update_engine: Delete UpdateDownloadAllowed Policy
Unused policy in update_engine code.
|UpdateDownloadAllowed| was a work in progress that was left over.
Network connection is checked in a different flow.
BUG=b:171829801
TEST=FEATURES=test emerge-$B update_engine
Change-Id: Ic726efd066c270be7ca0b594d5627ee884893c27
Reviewed-on: https://chromium-review.googlesource.com/c/aosp/platform/system/update_engine/+/2518383
Tested-by: Jae Hoon Kim <kimjae@chromium.org>
Commit-Queue: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Amin Hassani <ahassani@chromium.org>
diff --git a/update_manager/android_things_policy.cc b/update_manager/android_things_policy.cc
index c4fa75a..5759145 100644
--- a/update_manager/android_things_policy.cc
+++ b/update_manager/android_things_policy.cc
@@ -157,16 +157,6 @@
return EvalStatus::kSucceeded;
}
-// Always returns |EvalStatus::kSucceeded|
-EvalStatus AndroidThingsPolicy::UpdateDownloadAllowed(EvaluationContext* ec,
- State* state,
- string* error,
- bool* result) const {
- // By default, we allow updates.
- *result = true;
- return EvalStatus::kSucceeded;
-}
-
// P2P is always disabled. Returns |result|==|false| and
// |EvalStatus::kSucceeded|
EvalStatus AndroidThingsPolicy::P2PEnabled(EvaluationContext* ec,
diff --git a/update_manager/android_things_policy.h b/update_manager/android_things_policy.h
index 9fd8bc4..3b273ca 100644
--- a/update_manager/android_things_policy.h
+++ b/update_manager/android_things_policy.h
@@ -53,12 +53,6 @@
UpdateDownloadParams* result,
UpdateState update_state) const override;
- // Always returns |EvalStatus::kSucceeded|
- EvalStatus UpdateDownloadAllowed(EvaluationContext* ec,
- State* state,
- std::string* error,
- bool* result) const override;
-
// P2P is always disabled. Returns |result|==|false| and
// |EvalStatus::kSucceeded|
EvalStatus P2PEnabled(EvaluationContext* ec,
diff --git a/update_manager/chromeos_policy.cc b/update_manager/chromeos_policy.cc
index 4c651b7..262987e 100644
--- a/update_manager/chromeos_policy.cc
+++ b/update_manager/chromeos_policy.cc
@@ -465,88 +465,6 @@
return EvalStatus::kSucceeded;
}
-// TODO(garnold) Logic in this method is based on
-// ConnectionManager::IsUpdateAllowedOver(); be sure to deprecate the latter.
-//
-// TODO(garnold) The current logic generally treats the list of allowed
-// connections coming from the device policy as an allowlist, meaning that it
-// can only be used for enabling connections, but not disable them. Further,
-// certain connection types cannot be enabled even by policy.
-// In effect, the only thing that device policy can change is to enable
-// updates over a cellular network (disabled by default). We may want to
-// revisit this semantics, allowing greater flexibility in defining specific
-// permissions over all types of networks.
-EvalStatus ChromeOSPolicy::UpdateDownloadAllowed(EvaluationContext* ec,
- State* state,
- string* error,
- bool* result) const {
- // Get the current connection type.
- ShillProvider* const shill_provider = state->shill_provider();
- const ConnectionType* conn_type_p =
- ec->GetValue(shill_provider->var_conn_type());
- POLICY_CHECK_VALUE_AND_FAIL(conn_type_p, error);
- ConnectionType conn_type = *conn_type_p;
-
- // If we're tethering, treat it as a cellular connection.
- if (conn_type != ConnectionType::kCellular) {
- const ConnectionTethering* conn_tethering_p =
- ec->GetValue(shill_provider->var_conn_tethering());
- POLICY_CHECK_VALUE_AND_FAIL(conn_tethering_p, error);
- if (*conn_tethering_p == ConnectionTethering::kConfirmed)
- conn_type = ConnectionType::kCellular;
- }
-
- // By default, we allow updates for all connection types, with exceptions as
- // noted below. This also determines whether a device policy can override the
- // default.
- *result = true;
- bool device_policy_can_override = false;
- switch (conn_type) {
- case ConnectionType::kCellular:
- *result = false;
- device_policy_can_override = true;
- break;
-
- case ConnectionType::kUnknown:
- if (error)
- *error = "Unknown connection type";
- return EvalStatus::kFailed;
-
- default:
- break; // Nothing to do.
- }
-
- // If update is allowed, we're done.
- if (*result)
- return EvalStatus::kSucceeded;
-
- // Check whether the device policy specifically allows this connection.
- if (device_policy_can_override) {
- DevicePolicyProvider* const dp_provider = state->device_policy_provider();
- const bool* device_policy_is_loaded_p =
- ec->GetValue(dp_provider->var_device_policy_is_loaded());
- if (device_policy_is_loaded_p && *device_policy_is_loaded_p) {
- const set<ConnectionType>* allowed_conn_types_p =
- ec->GetValue(dp_provider->var_allowed_connection_types_for_update());
- if (allowed_conn_types_p) {
- if (allowed_conn_types_p->count(conn_type)) {
- *result = true;
- return EvalStatus::kSucceeded;
- }
- } else if (conn_type == ConnectionType::kCellular) {
- // Local user settings can allow updates over cellular iff a policy was
- // loaded but no allowed connections were specified in it.
- const bool* update_over_cellular_allowed_p =
- ec->GetValue(state->updater_provider()->var_cellular_enabled());
- if (update_over_cellular_allowed_p && *update_over_cellular_allowed_p)
- *result = true;
- }
- }
- }
-
- return (*result ? EvalStatus::kSucceeded : EvalStatus::kAskMeAgainLater);
-}
-
EvalStatus ChromeOSPolicy::P2PEnabled(EvaluationContext* ec,
State* state,
string* error,
diff --git a/update_manager/chromeos_policy.h b/update_manager/chromeos_policy.h
index ded5164..3c196da 100644
--- a/update_manager/chromeos_policy.h
+++ b/update_manager/chromeos_policy.h
@@ -72,11 +72,6 @@
UpdateDownloadParams* result,
UpdateState update_state) const override;
- EvalStatus UpdateDownloadAllowed(EvaluationContext* ec,
- State* state,
- std::string* error,
- bool* result) const override;
-
EvalStatus P2PEnabled(EvaluationContext* ec,
State* state,
std::string* error,
diff --git a/update_manager/chromeos_policy_unittest.cc b/update_manager/chromeos_policy_unittest.cc
index 996db2b..53e8a52 100644
--- a/update_manager/chromeos_policy_unittest.cc
+++ b/update_manager/chromeos_policy_unittest.cc
@@ -1406,107 +1406,6 @@
EXPECT_FALSE(result.do_increment_failures);
}
-TEST_F(UmChromeOSPolicyTest, UpdateDownloadAllowedEthernetDefault) {
- // Ethernet is always allowed.
-
- fake_state_.shill_provider()->var_conn_type()->reset(
- new ConnectionType(ConnectionType::kEthernet));
-
- bool result;
- ExpectPolicyStatus(
- EvalStatus::kSucceeded, &Policy::UpdateDownloadAllowed, &result);
- EXPECT_TRUE(result);
-}
-
-TEST_F(UmChromeOSPolicyTest, UpdateDownloadAllowedWifiDefault) {
- // Wifi is allowed if not tethered.
-
- fake_state_.shill_provider()->var_conn_type()->reset(
- new ConnectionType(ConnectionType::kWifi));
-
- bool result;
- ExpectPolicyStatus(
- EvalStatus::kSucceeded, &Policy::UpdateDownloadAllowed, &result);
- EXPECT_TRUE(result);
-}
-
-TEST_F(UmChromeOSPolicyTest,
- UpdateCurrentConnectionNotAllowedWifiTetheredDefault) {
- // Tethered wifi is not allowed by default.
-
- fake_state_.shill_provider()->var_conn_type()->reset(
- new ConnectionType(ConnectionType::kWifi));
- fake_state_.shill_provider()->var_conn_tethering()->reset(
- new ConnectionTethering(ConnectionTethering::kConfirmed));
-
- bool result;
- ExpectPolicyStatus(
- EvalStatus::kAskMeAgainLater, &Policy::UpdateDownloadAllowed, &result);
-}
-
-TEST_F(UmChromeOSPolicyTest, UpdateDownloadAllowedWifiTetheredPolicyOverride) {
- // Tethered wifi can be allowed by policy.
-
- fake_state_.shill_provider()->var_conn_type()->reset(
- new ConnectionType(ConnectionType::kWifi));
- fake_state_.shill_provider()->var_conn_tethering()->reset(
- new ConnectionTethering(ConnectionTethering::kConfirmed));
- set<ConnectionType> allowed_connections;
- allowed_connections.insert(ConnectionType::kCellular);
- fake_state_.device_policy_provider()
- ->var_allowed_connection_types_for_update()
- ->reset(new set<ConnectionType>(allowed_connections));
-
- bool result;
- ExpectPolicyStatus(
- EvalStatus::kSucceeded, &Policy::UpdateDownloadAllowed, &result);
- EXPECT_TRUE(result);
-}
-
-TEST_F(UmChromeOSPolicyTest, UpdateCurrentConnectionNotAllowedCellularDefault) {
- // Cellular is not allowed by default.
-
- fake_state_.shill_provider()->var_conn_type()->reset(
- new ConnectionType(ConnectionType::kCellular));
-
- bool result;
- ExpectPolicyStatus(
- EvalStatus::kAskMeAgainLater, &Policy::UpdateDownloadAllowed, &result);
-}
-
-TEST_F(UmChromeOSPolicyTest, UpdateDownloadAllowedCellularPolicyOverride) {
- // Update over cellular can be enabled by policy.
-
- fake_state_.shill_provider()->var_conn_type()->reset(
- new ConnectionType(ConnectionType::kCellular));
- set<ConnectionType> allowed_connections;
- allowed_connections.insert(ConnectionType::kCellular);
- fake_state_.device_policy_provider()
- ->var_allowed_connection_types_for_update()
- ->reset(new set<ConnectionType>(allowed_connections));
-
- bool result;
- ExpectPolicyStatus(
- EvalStatus::kSucceeded, &Policy::UpdateDownloadAllowed, &result);
- EXPECT_TRUE(result);
-}
-
-TEST_F(UmChromeOSPolicyTest, UpdateDownloadAllowedCellularUserOverride) {
- // Update over cellular can be enabled by user settings, but only if policy
- // is present and does not determine allowed connections.
-
- fake_state_.shill_provider()->var_conn_type()->reset(
- new ConnectionType(ConnectionType::kCellular));
- set<ConnectionType> allowed_connections;
- allowed_connections.insert(ConnectionType::kCellular);
- fake_state_.updater_provider()->var_cellular_enabled()->reset(new bool(true));
-
- bool result;
- ExpectPolicyStatus(
- EvalStatus::kSucceeded, &Policy::UpdateDownloadAllowed, &result);
- EXPECT_TRUE(result);
-}
-
TEST_F(UmChromeOSPolicyTest, UpdateCanStartAllowedScatteringSupressedDueToP2P) {
// The UpdateCanStart policy returns true; scattering should have applied, but
// P2P download is allowed. Scattering values are nonetheless returned, and so
diff --git a/update_manager/default_policy.cc b/update_manager/default_policy.cc
index 7ca414b..5a38180 100644
--- a/update_manager/default_policy.cc
+++ b/update_manager/default_policy.cc
@@ -89,14 +89,6 @@
return EvalStatus::kSucceeded;
}
-EvalStatus DefaultPolicy::UpdateDownloadAllowed(EvaluationContext* ec,
- State* state,
- std::string* error,
- bool* result) const {
- *result = true;
- return EvalStatus::kSucceeded;
-}
-
EvalStatus DefaultPolicy::P2PEnabled(EvaluationContext* ec,
State* state,
std::string* error,
diff --git a/update_manager/default_policy.h b/update_manager/default_policy.h
index 1b284f4..006bcb7 100644
--- a/update_manager/default_policy.h
+++ b/update_manager/default_policy.h
@@ -83,11 +83,6 @@
UpdateDownloadParams* result,
UpdateState update_state) const override;
- EvalStatus UpdateDownloadAllowed(EvaluationContext* ec,
- State* state,
- std::string* error,
- bool* result) const override;
-
EvalStatus P2PEnabled(EvaluationContext* ec,
State* state,
std::string* error,
diff --git a/update_manager/mock_policy.h b/update_manager/mock_policy.h
index 46b6c78..183130f 100644
--- a/update_manager/mock_policy.h
+++ b/update_manager/mock_policy.h
@@ -46,11 +46,6 @@
testing::_, testing::_, testing::_, testing::_, testing::_))
.WillByDefault(
testing::Invoke(&default_policy_, &DefaultPolicy::UpdateCanStart));
- ON_CALL(
- *this,
- UpdateDownloadAllowed(testing::_, testing::_, testing::_, testing::_))
- .WillByDefault(testing::Invoke(&default_policy_,
- &DefaultPolicy::UpdateDownloadAllowed));
ON_CALL(*this, P2PEnabled(testing::_, testing::_, testing::_, testing::_))
.WillByDefault(
testing::Invoke(&default_policy_, &DefaultPolicy::P2PEnabled));
diff --git a/update_manager/policy.h b/update_manager/policy.h
index ad6994c..7543ea9 100644
--- a/update_manager/policy.h
+++ b/update_manager/policy.h
@@ -227,9 +227,6 @@
if (reinterpret_cast<typeof(&Policy::UpdateCanStart)>(policy_method) ==
&Policy::UpdateCanStart)
return class_name + "UpdateCanStart";
- if (reinterpret_cast<typeof(&Policy::UpdateDownloadAllowed)>(
- policy_method) == &Policy::UpdateDownloadAllowed)
- return class_name + "UpdateDownloadAllowed";
if (reinterpret_cast<typeof(&Policy::P2PEnabled)>(policy_method) ==
&Policy::P2PEnabled)
return class_name + "P2PEnabled";
@@ -278,17 +275,6 @@
UpdateDownloadParams* result,
UpdateState update_state) const = 0;
- // Checks whether downloading of an update is allowed; currently, this checks
- // whether the network connection type is suitable for updating over. May
- // consult the shill provider as well as the device policy (if available).
- // Returns |EvalStatus::kSucceeded|, setting |result| according to whether or
- // not the current connection can be used; on error, returns
- // |EvalStatus::kFailed| and sets |error| accordingly.
- virtual EvalStatus UpdateDownloadAllowed(EvaluationContext* ec,
- State* state,
- std::string* error,
- bool* result) const = 0;
-
// Checks whether P2P is enabled. This may consult device policy and other
// global settings.
virtual EvalStatus P2PEnabled(EvaluationContext* ec,
diff --git a/update_manager/policy_utils.h b/update_manager/policy_utils.h
index dc606f2..aedb90c 100644
--- a/update_manager/policy_utils.h
+++ b/update_manager/policy_utils.h
@@ -92,13 +92,6 @@
return EvalStatus::kContinue;
};
- EvalStatus UpdateDownloadAllowed(EvaluationContext* ec,
- State* state,
- std::string* error,
- bool* result) const override {
- return EvalStatus::kContinue;
- };
-
EvalStatus P2PEnabled(EvaluationContext* ec,
State* state,
std::string* error,