Add interactive override update restriction flags
Add the ability for an interactive update to provide its own set of
update restriction flags that are used instead of the normal set.
Bug: 66016687
Test: unit-tests, manual OTA
Exempt-From-Owner-Approval: cherry-pick from nyc-iot-dev
Change-Id: I7b80a0dadde6b163e8b5e2bc6fd197c2d7761646
(cherry picked from commit a6fbaa5323022080af2f711290e3fb953b9826a4)
diff --git a/binder_bindings/android/brillo/IUpdateEngine.aidl b/binder_bindings/android/brillo/IUpdateEngine.aidl
index c85c0dc..e549a4d 100644
--- a/binder_bindings/android/brillo/IUpdateEngine.aidl
+++ b/binder_bindings/android/brillo/IUpdateEngine.aidl
@@ -21,7 +21,7 @@
interface IUpdateEngine {
void SetUpdateAttemptFlags(in int flags);
- void AttemptUpdate(in String app_version, in String omaha_url, in int flags);
+ boolean AttemptUpdate(in String app_version, in String omaha_url, in int flags);
void AttemptRollback(in boolean powerwash);
boolean CanRollback();
void ResetStatus();
diff --git a/binder_service_brillo.cc b/binder_service_brillo.cc
index c57561c..3f01e42 100644
--- a/binder_service_brillo.cc
+++ b/binder_service_brillo.cc
@@ -63,11 +63,15 @@
}
Status BinderUpdateEngineBrilloService::AttemptUpdate(
- const String16& app_version, const String16& omaha_url, int flags) {
+ const String16& app_version,
+ const String16& omaha_url,
+ int flags,
+ bool* out_result) {
return CallCommonHandler(&UpdateEngineService::AttemptUpdate,
NormalString(app_version),
NormalString(omaha_url),
- flags);
+ flags,
+ out_result);
}
Status BinderUpdateEngineBrilloService::AttemptRollback(bool powerwash) {
diff --git a/binder_service_brillo.h b/binder_service_brillo.h
index 81a4e4c..c802fca 100644
--- a/binder_service_brillo.h
+++ b/binder_service_brillo.h
@@ -54,7 +54,8 @@
android::binder::Status SetUpdateAttemptFlags(int flags) override;
android::binder::Status AttemptUpdate(const android::String16& app_version,
const android::String16& omaha_url,
- int flags) override;
+ int flags,
+ bool* out_result) override;
android::binder::Status AttemptRollback(bool powerwash) override;
android::binder::Status CanRollback(bool* out_can_rollback) override;
android::binder::Status ResetStatus() override;
diff --git a/client_library/client_binder.cc b/client_library/client_binder.cc
index b4625e4..54b33ed 100644
--- a/client_library/client_binder.cc
+++ b/client_library/client_binder.cc
@@ -48,11 +48,13 @@
bool BinderUpdateEngineClient::AttemptUpdate(const string& in_app_version,
const string& in_omaha_url,
bool at_user_request) {
+ bool started;
return service_
->AttemptUpdate(
String16{in_app_version.c_str()},
String16{in_omaha_url.c_str()},
- at_user_request ? 0 : UpdateAttemptFlags::kFlagNonInteractive)
+ at_user_request ? 0 : UpdateAttemptFlags::kFlagNonInteractive,
+ &started)
.isOk();
}
diff --git a/common_service.cc b/common_service.cc
index 38a3096..370587b 100644
--- a/common_service.cc
+++ b/common_service.cc
@@ -88,16 +88,20 @@
bool UpdateEngineService::AttemptUpdate(ErrorPtr* /* error */,
const string& in_app_version,
const string& in_omaha_url,
- int32_t in_flags_as_int) {
+ int32_t in_flags_as_int,
+ bool* out_result) {
auto flags = static_cast<UpdateAttemptFlags>(in_flags_as_int);
bool interactive = !(flags & UpdateAttemptFlags::kFlagNonInteractive);
+ bool restrict_downloads = (flags & UpdateAttemptFlags::kFlagRestrictDownload);
LOG(INFO) << "Attempt update: app_version=\"" << in_app_version << "\" "
<< "omaha_url=\"" << in_omaha_url << "\" "
<< "flags=0x" << std::hex << flags << " "
- << "interactive=" << (interactive ? "yes" : "no");
- system_state_->update_attempter()->CheckForUpdate(
- in_app_version, in_omaha_url, interactive);
+ << "interactive=" << (interactive ? "yes " : "no ")
+ << "RestrictDownload=" << (restrict_downloads ? "yes " : "no ");
+
+ *out_result = system_state_->update_attempter()->CheckForUpdate(
+ in_app_version, in_omaha_url, flags);
return true;
}
diff --git a/common_service.h b/common_service.h
index 261c9e8..544dd93 100644
--- a/common_service.h
+++ b/common_service.h
@@ -49,7 +49,8 @@
bool AttemptUpdate(brillo::ErrorPtr* error,
const std::string& in_app_version,
const std::string& in_omaha_url,
- int32_t in_flags_as_int);
+ int32_t in_flags_as_int,
+ bool* out_result);
bool AttemptRollback(brillo::ErrorPtr* error, bool in_powerwash);
diff --git a/common_service_unittest.cc b/common_service_unittest.cc
index 71e42d0..c124466 100644
--- a/common_service_unittest.cc
+++ b/common_service_unittest.cc
@@ -57,12 +57,32 @@
};
TEST_F(UpdateEngineServiceTest, AttemptUpdate) {
- EXPECT_CALL(*mock_update_attempter_, CheckForUpdate(
- "app_ver", "url", false /* interactive */));
- // The update is non-interactive when we pass the non-interactive flag.
- EXPECT_TRUE(common_service_.AttemptUpdate(
- &error_, "app_ver", "url", UpdateAttemptFlags::kFlagNonInteractive));
+ EXPECT_CALL(
+ *mock_update_attempter_,
+ CheckForUpdate("app_ver", "url", UpdateAttemptFlags::kFlagNonInteractive))
+ .WillOnce(Return(true));
+
+ // The non-interactive flag needs to be passed through to CheckForUpdate.
+ bool result = false;
+ EXPECT_TRUE(
+ common_service_.AttemptUpdate(&error_,
+ "app_ver",
+ "url",
+ UpdateAttemptFlags::kFlagNonInteractive,
+ &result));
EXPECT_EQ(nullptr, error_);
+ EXPECT_TRUE(result);
+}
+
+TEST_F(UpdateEngineServiceTest, AttemptUpdateReturnsFalse) {
+ EXPECT_CALL(*mock_update_attempter_,
+ CheckForUpdate("app_ver", "url", UpdateAttemptFlags::kNone))
+ .WillOnce(Return(false));
+ bool result = true;
+ EXPECT_TRUE(common_service_.AttemptUpdate(
+ &error_, "app_ver", "url", UpdateAttemptFlags::kNone, &result));
+ EXPECT_EQ(nullptr, error_);
+ EXPECT_FALSE(result);
}
// SetChannel is allowed when there's no device policy (the device is not
diff --git a/mock_update_attempter.h b/mock_update_attempter.h
index d64dcbe..d88b840 100644
--- a/mock_update_attempter.h
+++ b/mock_update_attempter.h
@@ -44,9 +44,10 @@
MOCK_METHOD0(GetCurrentUpdateAttemptFlags, UpdateAttemptFlags(void));
- MOCK_METHOD3(CheckForUpdate, void(const std::string& app_version,
- const std::string& omaha_url,
- bool is_interactive));
+ MOCK_METHOD3(CheckForUpdate,
+ bool(const std::string& app_version,
+ const std::string& omaha_url,
+ UpdateAttemptFlags flags));
MOCK_METHOD0(RefreshDevicePolicy, void(void));
diff --git a/update_attempter.cc b/update_attempter.cc
index e40dfe7..a6f9fa5 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -773,9 +773,20 @@
return BootControlInterface::kInvalidSlot;
}
-void UpdateAttempter::CheckForUpdate(const string& app_version,
+bool UpdateAttempter::CheckForUpdate(const string& app_version,
const string& omaha_url,
- bool interactive) {
+ UpdateAttemptFlags flags) {
+ bool interactive = !(flags & UpdateAttemptFlags::kFlagNonInteractive);
+
+ if (interactive && status_ != UpdateStatus::IDLE) {
+ // An update check is either in-progress, or an update has completed and the
+ // system is in UPDATED_NEED_REBOOT. Either way, don't do an interactive
+ // update at this time
+ LOG(INFO) << "Refusing to do an interactive update with an update already "
+ "in progress";
+ return false;
+ }
+
LOG(INFO) << "Forced update check requested.";
forced_app_version_.clear();
forced_omaha_url_.clear();
@@ -797,12 +808,22 @@
forced_omaha_url_ = constants::kOmahaDefaultAUTestURL;
}
+ if (interactive) {
+ // Use the passed-in update attempt flags for this update attempt instead
+ // of the previously set ones.
+ current_update_attempt_flags_ = flags;
+ // Note: The caching for non-interactive update checks happens in
+ // OnUpdateScheduled().
+ }
+
if (forced_update_pending_callback_.get()) {
// Make sure that a scheduling request is made prior to calling the forced
// update pending callback.
ScheduleUpdates();
forced_update_pending_callback_->Run(true, interactive);
}
+
+ return true;
}
bool UpdateAttempter::RebootIfNeeded() {
@@ -860,9 +881,12 @@
<< (params.is_interactive ? "interactive" : "periodic")
<< " update.";
- // Cache the update attempt flags that will be used by this update attempt
- // so that they can't be changed mid-way through.
- current_update_attempt_flags_ = update_attempt_flags_;
+ if (!params.is_interactive) {
+ // Cache the update attempt flags that will be used by this update attempt
+ // so that they can't be changed mid-way through.
+ current_update_attempt_flags_ = update_attempt_flags_;
+ }
+
LOG(INFO) << "Update attempt flags in use = 0x" << std::hex
<< current_update_attempt_flags_;
diff --git a/update_attempter.h b/update_attempter.h
index 758494a..d2fe3ec 100644
--- a/update_attempter.h
+++ b/update_attempter.h
@@ -148,9 +148,11 @@
// This is the internal entry point for going through an
// update. If the current status is idle invokes Update.
// This is called by the DBus implementation.
- virtual void CheckForUpdate(const std::string& app_version,
+ // This returns true if an update check was started, false if a check or an
+ // update was already in progress.
+ virtual bool CheckForUpdate(const std::string& app_version,
const std::string& omaha_url,
- bool is_interactive);
+ UpdateAttemptFlags flags);
// This is the internal entry point for going through a rollback. This will
// attempt to run the postinstall on the non-active partition and set it as
diff --git a/update_attempter_unittest.cc b/update_attempter_unittest.cc
index d54c867..bdc0196 100644
--- a/update_attempter_unittest.cc
+++ b/update_attempter_unittest.cc
@@ -1049,14 +1049,14 @@
TEST_F(UpdateAttempterTest, CheckForUpdateAUTest) {
fake_system_state_.fake_hardware()->SetIsOfficialBuild(true);
fake_system_state_.fake_hardware()->SetAreDevFeaturesEnabled(false);
- attempter_.CheckForUpdate("", "autest", true);
+ attempter_.CheckForUpdate("", "autest", UpdateAttemptFlags::kNone);
EXPECT_EQ(constants::kOmahaDefaultAUTestURL, attempter_.forced_omaha_url());
}
TEST_F(UpdateAttempterTest, CheckForUpdateScheduledAUTest) {
fake_system_state_.fake_hardware()->SetIsOfficialBuild(true);
fake_system_state_.fake_hardware()->SetAreDevFeaturesEnabled(false);
- attempter_.CheckForUpdate("", "autest-scheduled", true);
+ attempter_.CheckForUpdate("", "autest-scheduled", UpdateAttemptFlags::kNone);
EXPECT_EQ(constants::kOmahaDefaultAUTestURL, attempter_.forced_omaha_url());
}
@@ -1135,4 +1135,25 @@
attempter_.GetCurrentUpdateAttemptFlags());
}
+TEST_F(UpdateAttempterTest, InteractiveUpdateUsesPassedRestrictions) {
+ attempter_.SetUpdateAttemptFlags(UpdateAttemptFlags::kFlagRestrictDownload);
+
+ attempter_.CheckForUpdate("", "", UpdateAttemptFlags::kNone);
+ EXPECT_EQ(UpdateAttemptFlags::kNone,
+ attempter_.GetCurrentUpdateAttemptFlags());
+}
+
+TEST_F(UpdateAttempterTest, NonInteractiveUpdateUsesSetRestrictions) {
+ attempter_.SetUpdateAttemptFlags(UpdateAttemptFlags::kNone);
+
+ // This tests that when CheckForUpdate() is called with the non-interactive
+ // flag set, that it doesn't change the current UpdateAttemptFlags.
+ attempter_.CheckForUpdate("",
+ "",
+ UpdateAttemptFlags::kFlagNonInteractive |
+ UpdateAttemptFlags::kFlagRestrictDownload);
+ EXPECT_EQ(UpdateAttemptFlags::kNone,
+ attempter_.GetCurrentUpdateAttemptFlags());
+}
+
} // namespace chromeos_update_engine