update_engine: Add PayloadState Exclusion Logic

|PayloadState| will exclude payloads based on specific update failures.
This is to prevent critical platform updates from being blocked by less
critical updates (e.g. DLCs). A layer of robustness is added in
protecting CrOS devices from falling off the update train.

Some important changes to mention:
 - Only during updates will update_engine exclude non-critical payloads
 - |OmahaRequestAction|, the current precursor |Action| to
 |OmahaResponseHandlerAction|, during a update will exclude
 faulty/excluded payloads prior to setting the |OmahaResponse| as an
 output object for suqsequent bonded |Action| to consume
 - When all payloads are excluded for an update, the |ErrorCode| will
 be indicated as |OmahaResponseInvalid| as this case is not a valid
 Omaha response update_engine should ever run into because non-critical
 updates must tag alongside a critical update

BUG=chromium:928805
TEST=FEATURES=test emerge-$B update_engine update_engine-client

Change-Id: I0551a228d0b84defb4d59966e8ed46a5d9278d60
Reviewed-on: https://chromium-review.googlesource.com/c/aosp/platform/system/update_engine/+/2190237
Tested-by: Jae Hoon Kim <kimjae@chromium.org>
Auto-Submit: Jae Hoon Kim <kimjae@chromium.org>
Reviewed-by: Amin Hassani <ahassani@chromium.org>
Commit-Queue: Jae Hoon Kim <kimjae@chromium.org>

1 files changed, 104 insertions, 8 deletions

diff --git a/payload_state_unittest.cc b/payload_state_unittest.cc
index 4a0afcfe..bf9aed44 100644
--- a/payload_state_unittest.cc
+++ b/payload_state_unittest.cc
@@ -23,9 +23,11 @@
 #include <gtest/gtest.h>
 
 #include "update_engine/common/constants.h"
+#include "update_engine/common/excluder_interface.h"
 #include "update_engine/common/fake_clock.h"
 #include "update_engine/common/fake_hardware.h"
 #include "update_engine/common/fake_prefs.h"
+#include "update_engine/common/mock_excluder.h"
 #include "update_engine/common/mock_prefs.h"
 #include "update_engine/common/prefs.h"
 #include "update_engine/common/test_utils.h"
@@ -44,6 +46,7 @@ using testing::Mock;
 using testing::NiceMock;
 using testing::Return;
 using testing::SetArgPointee;
+using testing::StrictMock;
 
 namespace chromeos_update_engine {
 
@@ -1012,10 +1015,6 @@ TEST(PayloadStateTest, RollbackVersion) {
 
   NiceMock<MockPrefs>* mock_powerwash_safe_prefs =
       fake_system_state.mock_powerwash_safe_prefs();
-  EXPECT_TRUE(payload_state.Initialize(&fake_system_state));
-
-  // Verify pre-conditions are good.
-  EXPECT_TRUE(payload_state.GetRollbackVersion().empty());
 
   // Mock out the os version and make sure it's blacklisted correctly.
   string rollback_version = "2345.0.0";
@@ -1023,6 +1022,11 @@ TEST(PayloadStateTest, RollbackVersion) {
   params.Init(rollback_version, "", false);
   fake_system_state.set_request_params(&params);
 
+  EXPECT_TRUE(payload_state.Initialize(&fake_system_state));
+
+  // Verify pre-conditions are good.
+  EXPECT_TRUE(payload_state.GetRollbackVersion().empty());
+
   EXPECT_CALL(*mock_powerwash_safe_prefs,
               SetString(kPrefsRollbackVersion, rollback_version));
   payload_state.Rollback();
@@ -1353,15 +1357,15 @@ TEST(PayloadStateTest, PayloadTypeMetricWhenTypeIsForcedFull) {
   PayloadState payload_state;
   FakeSystemState fake_system_state;
 
-  EXPECT_TRUE(payload_state.Initialize(&fake_system_state));
-  SetupPayloadStateWith2Urls(
-      "Hash6437", true, false, &payload_state, &response);
-
   // Mock the request to a request where the delta was disabled.
   OmahaRequestParams params(&fake_system_state);
   params.set_delta_okay(false);
   fake_system_state.set_request_params(&params);
 
+  EXPECT_TRUE(payload_state.Initialize(&fake_system_state));
+  SetupPayloadStateWith2Urls(
+      "Hash6437", true, false, &payload_state, &response);
+
   // Simulate a successful download and update.
   payload_state.DownloadComplete();
 
@@ -1658,6 +1662,9 @@ TEST(PayloadStateTest, P2PStateVarsAreClearedOnNewResponse) {
 TEST(PayloadStateTest, NextPayloadResetsUrlIndex) {
   PayloadState payload_state;
   FakeSystemState fake_system_state;
+  StrictMock<MockExcluder> mock_excluder;
+  EXPECT_CALL(*fake_system_state.mock_update_attempter(), GetExcluder())
+      .WillOnce(Return(&mock_excluder));
   EXPECT_TRUE(payload_state.Initialize(&fake_system_state));
 
   OmahaResponse response;
@@ -1682,4 +1689,93 @@ TEST(PayloadStateTest, NextPayloadResetsUrlIndex) {
   EXPECT_EQ(payload_state.GetCurrentUrl(), "http://test1b");
 }
 
+TEST(PayloadStateTest, ExcludeNoopForNonExcludables) {
+  PayloadState payload_state;
+  FakeSystemState fake_system_state;
+  StrictMock<MockExcluder> mock_excluder;
+  EXPECT_CALL(*fake_system_state.mock_update_attempter(), GetExcluder())
+      .WillOnce(Return(&mock_excluder));
+  EXPECT_TRUE(payload_state.Initialize(&fake_system_state));
+
+  OmahaResponse response;
+  response.packages.push_back(
+      {.payload_urls = {"http://test1a", "http://test2a"},
+       .size = 123456789,
+       .metadata_size = 58123,
+       .metadata_signature = "msign",
+       .hash = "hash",
+       .can_exclude = false});
+  payload_state.SetResponse(response);
+
+  EXPECT_CALL(mock_excluder, Exclude(_)).Times(0);
+  payload_state.ExcludeCurrentPayload();
+}
+
+TEST(PayloadStateTest, ExcludeOnlyCanExcludables) {
+  PayloadState payload_state;
+  FakeSystemState fake_system_state;
+  StrictMock<MockExcluder> mock_excluder;
+  EXPECT_CALL(*fake_system_state.mock_update_attempter(), GetExcluder())
+      .WillOnce(Return(&mock_excluder));
+  EXPECT_TRUE(payload_state.Initialize(&fake_system_state));
+
+  OmahaResponse response;
+  response.packages.push_back(
+      {.payload_urls = {"http://test1a", "http://test2a"},
+       .size = 123456789,
+       .metadata_size = 58123,
+       .metadata_signature = "msign",
+       .hash = "hash",
+       .can_exclude = true});
+  payload_state.SetResponse(response);
+
+  EXPECT_CALL(mock_excluder, Exclude(utils::GetExclusionName("http://test1a")))
+      .WillOnce(Return(true));
+  payload_state.ExcludeCurrentPayload();
+}
+
+TEST(PayloadStateTest, IncrementFailureExclusionTest) {
+  PayloadState payload_state;
+  FakeSystemState fake_system_state;
+  StrictMock<MockExcluder> mock_excluder;
+  EXPECT_CALL(*fake_system_state.mock_update_attempter(), GetExcluder())
+      .WillOnce(Return(&mock_excluder));
+  EXPECT_TRUE(payload_state.Initialize(&fake_system_state));
+
+  OmahaResponse response;
+  // Critical package.
+  response.packages.push_back(
+      {.payload_urls = {"http://crit-test1a", "http://crit-test2a"},
+       .size = 123456789,
+       .metadata_size = 58123,
+       .metadata_signature = "msign",
+       .hash = "hash",
+       .can_exclude = false});
+  // Non-critical package.
+  response.packages.push_back(
+      {.payload_urls = {"http://test1a", "http://test2a"},
+       .size = 123456789,
+       .metadata_size = 58123,
+       .metadata_signature = "msign",
+       .hash = "hash",
+       .can_exclude = true});
+  response.max_failure_count_per_url = 2;
+  payload_state.SetResponse(response);
+
+  // Critical package won't be excluded.
+  // Increment twice as failure count allowed per URL is set to 2.
+  payload_state.IncrementFailureCount();
+  payload_state.IncrementFailureCount();
+
+  EXPECT_TRUE(payload_state.NextPayload());
+
+  // First increment failure should not exclude.
+  payload_state.IncrementFailureCount();
+
+  // Second increment failure should exclude.
+  EXPECT_CALL(mock_excluder, Exclude(utils::GetExclusionName("http://test1a")))
+      .WillOnce(Return(true));
+  payload_state.IncrementFailureCount();
+}
+
 }  // namespace chromeos_update_engine