Merge "aconfig storage daemon: add local override removal capability" into main

5 files changed, 238 insertions, 41 deletions

diff --git a/aconfigd/aconfigd.cpp b/aconfigd/aconfigd.cpp
index 96d3490..ff5411e 100644
--- a/aconfigd/aconfigd.cpp
+++ b/aconfigd/aconfigd.cpp
@@ -205,7 +205,8 @@ Result<bool> AddOrUpdateStorageForContainer(const std::string& container,
   auto it = persist_storage_records.find(container);
   if (it == persist_storage_records.end() || it->second.timestamp != *timestamp) {
     // copy flag value file
-    auto target_value_file = std::string("/metadata/aconfig/flags/") + container + ".val";
+    auto flags_dir = std::string("/metadata/aconfig/flags/");
+    auto target_value_file = flags_dir + container + ".val";
     auto copy_result = CopyFile(value_file, target_value_file, 0644);
     if (!copy_result.ok()) {
       return Error() << "CopyFile failed for " << value_file << " :"
@@ -233,6 +234,7 @@ Result<bool> AddOrUpdateStorageForContainer(const std::string& container,
     record.flag_map = flag_file;
     record.flag_val = target_value_file;
     record.flag_info = flag_info_file;
+    record.local_overrides = flags_dir + container + "_local_overrides.pb";
     record.timestamp = *timestamp;
 
     // write to persistent storage records file
@@ -277,7 +279,8 @@ Result<void> HandleLocalFlagOverride(const std::string& package,
                                      const std::string& flag_value) {
   auto container = mapped_files_manager.GetContainer(package);
   if (!container.ok()) {
-      return container.error();
+      return Error() << "Failed to find package " << package << ": "
+                     << container.error();
   }
 
   auto pb_file = persist_storage_records[*container].local_overrides;
@@ -307,7 +310,7 @@ Result<void> HandleLocalFlagOverride(const std::string& package,
 
     // mark override sticky
     auto& mapped_files = mapped_files_manager.get_mapped_files(*container);
-    auto update = mapped_files.MarkStickyOverride(package, flag);
+    auto update = mapped_files.MarkHasLocalOverride(package, flag, true);
     if (!update.ok()) {
       return Error() << "Failed to mark flag " << package + "." + flag << " sticky: "
                      << update.error();
@@ -328,7 +331,8 @@ Result<void> HandleServerFlagOverride(const std::string& package,
                                       const std::string& flag_value) {
   auto container = mapped_files_manager.GetContainer(package);
   if (!container.ok()) {
-      return container.error();
+      return Error() << "Failed to find package " << package << ": "
+                     << container.error();
   }
   auto& mapped_files = mapped_files_manager.get_mapped_files(*container);
   return  mapped_files.UpdatePersistFlag(package, flag, flag_value);
@@ -362,10 +366,29 @@ void HandlePersistFlagQuery(const StorageRequestMessage::FlagQueryMessage& msg,
   auto container = mapped_files_manager.GetContainer(msg.package_name());
   if (!container.ok()) {
     auto* errmsg = return_msg.mutable_error_message();
-    *errmsg = container.error().message();
+    *errmsg = "Failed to find package " + msg.package_name() + ": "
+              + container.error().message();
     return;
   }
 
+  // get flag local override value if local override exists
+  auto const& entry = persist_storage_records[*container];
+  auto overrides_pb = ReadPbFromFile<LocalFlagOverrides>(entry.local_overrides);
+  if (!overrides_pb.ok()) {
+    auto* errmsg = return_msg.mutable_error_message();
+    *errmsg = "Unable to read local overrides pb: " + overrides_pb.error().message();
+    return;
+  }
+
+  auto local_override_value = std::string();
+  for (auto& entry : overrides_pb->overrides()) {
+    if (msg.package_name() == entry.package_name()
+        && msg.flag_name() == entry.flag_name()) {
+      local_override_value = entry.flag_value();
+    }
+  }
+
+  // get flag server override value
   auto& mapped_files = mapped_files_manager.get_mapped_files(*container);
   auto result = mapped_files.GetPersistFlagValueAndInfo(
       msg.package_name(), msg.flag_name());
@@ -375,10 +398,100 @@ void HandlePersistFlagQuery(const StorageRequestMessage::FlagQueryMessage& msg,
     *errmsg = result.error().message();
   } else {
     auto result_msg = return_msg.mutable_flag_query_message();
-    result_msg->set_flag_value(result->first);
-    result_msg->set_is_sticky(result->second & FlagInfoBit::IsSticky);
+    result_msg->set_server_flag_value(result->first);
+    result_msg->set_local_flag_value(local_override_value);
+    result_msg->set_has_server_override(result->second & FlagInfoBit::HasServerOverride);
     result_msg->set_is_readwrite(result->second & FlagInfoBit::IsReadWrite);
-    result_msg->set_has_override(result->second & FlagInfoBit::HasOverride);
+    result_msg->set_has_local_override(result->second & FlagInfoBit::HasLocalOverride);
+  }
+}
+
+/// Remove all local overrides
+Result<void> RemoveAllLocalOverrides() {
+  for (auto const& [container, record] : persist_storage_records) {
+    auto overrides_pb = ReadPbFromFile<LocalFlagOverrides>(record.local_overrides);
+    if (!overrides_pb.ok()) {
+      return Error() << "Unable to read local overrides pb: " << overrides_pb.error();
+    }
+
+    for (auto& entry : overrides_pb->overrides()) {
+      auto& mapped_files = mapped_files_manager.get_mapped_files(container);
+      auto update = mapped_files.MarkHasLocalOverride(
+          entry.package_name(), entry.flag_name(), false);
+      if (!update.ok()) {
+        return Error() << "Failed to mark flag " << entry.package_name() + "." +
+            entry.flag_name() << " sticky: " << update.error();
+      }
+    }
+
+    if (unlink(record.local_overrides.c_str()) == -1) {
+      return ErrnoError() << "unlink() failed for " << record.local_overrides;
+    }
+  }
+
+  return {};
+}
+
+/// Remove a local override
+Result<void> RemoveFlagLocalOverride(const std::string& package,
+                                     const std::string& flag) {
+  auto container = mapped_files_manager.GetContainer(package);
+  if (!container.ok()) {
+      return Error() << "Failed to find package " << package << ": "
+                     << container.error();
+  }
+
+  auto const& record = persist_storage_records[*container];
+  auto overrides_pb = ReadPbFromFile<LocalFlagOverrides>(record.local_overrides);
+  if (!overrides_pb.ok()) {
+    return Error() << "Unable to read local overrides pb: " << overrides_pb.error();
+  }
+
+  auto updated_overrides = LocalFlagOverrides();
+  for (auto entry : overrides_pb->overrides()) {
+    if (entry.package_name() == package && entry.flag_name() == flag) {
+      auto& mapped_files = mapped_files_manager.get_mapped_files(*container);
+      auto update = mapped_files.MarkHasLocalOverride(
+          entry.package_name(), entry.flag_name(), false);
+      if (!update.ok()) {
+        return Error() << "Failed to mark flag " << entry.package_name() + "." +
+            entry.flag_name() << " sticky: " << update.error();
+      }
+      continue;
+    }
+    auto kept_override = updated_overrides.add_overrides();
+    kept_override->set_package_name(entry.package_name());
+    kept_override->set_flag_name(entry.flag_name());
+    kept_override->set_flag_value(entry.flag_value());
+  }
+
+  if (updated_overrides.overrides_size() != overrides_pb->overrides_size()) {
+    auto result = WritePbToFile<LocalFlagOverrides>(
+        updated_overrides, record.local_overrides);
+    if (!result.ok()) {
+      return base::Error() << result.error();
+    }
+  }
+
+  return {};
+}
+
+/// Handle override removal request
+void HandleLocalOverrideRemoval(
+    const StorageRequestMessage::RemoveLocalOverrideMessage& msg,
+    StorageReturnMessage& return_msg) {
+  auto result = Result<void>();
+  if (msg.remove_all()) {
+    result = RemoveAllLocalOverrides();
+  } else {
+    result = RemoveFlagLocalOverride(msg.package_name(), msg.flag_name());
+  }
+
+  if (!result.ok()) {
+    auto* errmsg = return_msg.mutable_error_message();
+    *errmsg = result.error().message();
+  } else {
+    return_msg.mutable_remove_local_override_message();
   }
 }
 
@@ -454,6 +567,12 @@ void HandleSocketRequest(const StorageRequestMessage& message,
       HandlePersistFlagQuery(msg, return_message);
       break;
     }
+    case StorageRequestMessage::kRemoveLocalOverrideMessage: {
+      LOG(INFO) << "received a local override removal request";
+      auto msg = message.remove_local_override_message();
+      HandleLocalOverrideRemoval(msg, return_message);
+      break;
+    }
     default:
       auto* errmsg = return_message.mutable_error_message();
       *errmsg = "Unknown message type from aconfigd socket";
diff --git a/aconfigd/aconfigd.proto b/aconfigd/aconfigd.proto
index 4adfa26..ff3c832 100644
--- a/aconfigd/aconfigd.proto
+++ b/aconfigd/aconfigd.proto
@@ -35,6 +35,13 @@ message StorageRequestMessage {
     optional bool is_local = 4;
   }
 
+  // request to remove local flag override
+  message RemoveLocalOverrideMessage {
+    optional bool remove_all = 1;
+    optional string package_name = 2;
+    optional string flag_name = 3;
+  }
+
   // query persistent flag value and info
   message FlagQueryMessage {
     optional string package_name = 1;
@@ -45,6 +52,7 @@ message StorageRequestMessage {
     NewStorageMessage new_storage_message = 1;
     FlagOverrideMessage flag_override_message = 2;
     FlagQueryMessage flag_query_message = 3;
+    RemoveLocalOverrideMessage remove_local_override_message = 4;
   };
 }
 
@@ -59,17 +67,21 @@ message StorageReturnMessage {
   message FlagOverrideReturnMessage {}
 
   message FlagQueryReturnMessage {
-    optional string flag_value = 1;
-    optional bool is_sticky = 2;
-    optional bool is_readwrite = 3;
-    optional bool has_override = 4;
+    optional string server_flag_value = 1;
+    optional string local_flag_value = 2;
+    optional bool has_server_override = 3;
+    optional bool is_readwrite = 4;
+    optional bool has_local_override = 5;
   }
 
+  message RemoveLocalOverrideReturnMessage {}
+
   oneof msg {
     NewStorageReturnMessage new_storage_message = 1;
     FlagOverrideReturnMessage flag_override_message = 2;
     FlagQueryReturnMessage flag_query_message = 3;
-    string error_message = 4;
+    RemoveLocalOverrideReturnMessage remove_local_override_message = 4;
+    string error_message = 5;
   };
 }
 
diff --git a/aconfigd/aconfigd_mapped_file.cpp b/aconfigd/aconfigd_mapped_file.cpp
index ad16a30..89f1b2b 100644
--- a/aconfigd/aconfigd_mapped_file.cpp
+++ b/aconfigd/aconfigd_mapped_file.cpp
@@ -283,10 +283,11 @@ namespace android {
         }
 
         // update flag info
-        update_result = set_flag_has_override(
+        update_result = set_flag_has_server_override(
             **flag_info_file, value_type, flag_index, true);
         if (!update_result.ok()) {
-          return base::Error() << "Failed to update flag has override: " << update_result.error();
+          return base::Error() << "Failed to update flag has server override: "
+                               << update_result.error();
         }
         break;
       }
@@ -297,9 +298,10 @@ namespace android {
     return {};
   }
 
-  /// mark this flag has sticky override
-  base::Result<void> MappedFiles::MarkStickyOverride(const std::string& package,
-                                                     const std::string& flag) {
+  /// mark this flag has local override
+  base::Result<void> MappedFiles::MarkHasLocalOverride(const std::string& package,
+                                                       const std::string& flag,
+                                                       bool has_local_override) {
     // find flag value type and index
     auto type_and_index = GetFlagTypeAndIndex(package, flag);
     if (!type_and_index.ok()) {
@@ -319,18 +321,11 @@ namespace android {
       return base::Error() << flag_info_file.error();
     }
 
-    // update flag info, has override
-    auto update_result = set_flag_has_override(
-        **flag_info_file, value_type, flag_index, true);
+    // update flag info, has local override
+    auto update_result = set_flag_has_local_override(
+        **flag_info_file, value_type, flag_index, has_local_override);
     if (!update_result.ok()) {
-      return base::Error() << "Failed to update flag has override: " << update_result.error();
-    }
-
-    // update flag info, is sticky
-    update_result = set_flag_is_sticky(
-        **flag_info_file, value_type, flag_index, true);
-    if (!update_result.ok()) {
-      return base::Error() << "Failed to update flag is sticky: " << update_result.error();
+      return base::Error() << "Failed to update flag has local override: " << update_result.error();
     }
 
     return {};
diff --git a/aconfigd/aconfigd_mapped_file.h b/aconfigd/aconfigd_mapped_file.h
index b615690..eb31d7f 100644
--- a/aconfigd/aconfigd_mapped_file.h
+++ b/aconfigd/aconfigd_mapped_file.h
@@ -55,9 +55,10 @@ namespace android {
                                            const std::string& flag,
                                            const std::string& value);
 
-      /// mark this flag has sticky override
-      base::Result<void> MarkStickyOverride(const std::string& package,
-                                            const std::string& flag);
+      /// mark this flag has local override
+      base::Result<void> MarkHasLocalOverride(const std::string& package,
+                                              const std::string& flag,
+                                              bool has_local_override);
 
       /// get persistent flag value and info
       base::Result<std::pair<std::string, uint8_t>> GetPersistFlagValueAndInfo(
diff --git a/aconfigd/aconfigd_test.cpp b/aconfigd/aconfigd_test.cpp
index c1026a1..17b6582 100644
--- a/aconfigd/aconfigd_test.cpp
+++ b/aconfigd/aconfigd_test.cpp
@@ -153,7 +153,7 @@ TEST(aconfigd_socket, new_storage_message) {
   ASSERT_TRUE(found);
 }
 
-TEST(aconfigd_socket, flag_override_message) {
+TEST(aconfigd_socket, flag_server_override_message) {
   auto new_storage_result = send_new_storage_message();
   ASSERT_TRUE(new_storage_result.ok()) << new_storage_result.error();
   ASSERT_EQ(new_storage_result->msgs_size(), 1);
@@ -175,10 +175,11 @@ TEST(aconfigd_socket, flag_override_message) {
   return_message = flag_query_result->msgs(0);
   ASSERT_TRUE(return_message.has_flag_query_message());
   auto query = return_message.flag_query_message();
-  ASSERT_EQ(query.flag_value(), "true");
-  ASSERT_EQ(query.is_sticky(), false);
+  ASSERT_EQ(query.server_flag_value(), "true");
+  ASSERT_EQ(query.local_flag_value(), "");
   ASSERT_EQ(query.is_readwrite(), true);
-  ASSERT_EQ(query.has_override(), true);
+  ASSERT_EQ(query.has_server_override(), true);
+  ASSERT_EQ(query.has_local_override(), false);
 
   flag_override_result = send_flag_override_message(
       "com.android.aconfig.storage.test_1", "enabled_rw", "false", false);
@@ -194,10 +195,60 @@ TEST(aconfigd_socket, flag_override_message) {
   return_message = flag_query_result->msgs(0);
   ASSERT_TRUE(return_message.has_flag_query_message());
   query = return_message.flag_query_message();
-  ASSERT_EQ(query.flag_value(), "false");
-  ASSERT_EQ(query.is_sticky(), false);
+  ASSERT_EQ(query.server_flag_value(), "false");
+  ASSERT_EQ(query.local_flag_value(), "");
   ASSERT_EQ(query.is_readwrite(), true);
-  ASSERT_EQ(query.has_override(), true);
+  ASSERT_EQ(query.has_server_override(), true);
+  ASSERT_EQ(query.has_local_override(), false);
+}
+
+TEST(aconfigd_socket, flag_local_override_message) {
+  auto new_storage_result = send_new_storage_message();
+  ASSERT_TRUE(new_storage_result.ok()) << new_storage_result.error();
+  ASSERT_EQ(new_storage_result->msgs_size(), 1);
+  auto return_message = new_storage_result->msgs(0);
+  ASSERT_TRUE(return_message.has_new_storage_message());
+
+  auto flag_override_result = send_flag_override_message(
+      "com.android.aconfig.storage.test_1", "disabled_rw", "true", true);
+  ASSERT_TRUE(flag_override_result.ok()) << flag_override_result.error();
+  ASSERT_EQ(flag_override_result->msgs_size(), 1);
+  return_message = flag_override_result->msgs(0);
+  ASSERT_TRUE(return_message.has_flag_override_message())
+      << return_message.error_message();
+
+  auto flag_query_result = send_flag_query_message(
+      "com.android.aconfig.storage.test_1", "disabled_rw");
+  ASSERT_TRUE(flag_query_result.ok()) << flag_query_result.error();
+  ASSERT_EQ(flag_query_result->msgs_size(), 1);
+  return_message = flag_query_result->msgs(0);
+  ASSERT_TRUE(return_message.has_flag_query_message());
+  auto query = return_message.flag_query_message();
+  ASSERT_EQ(query.server_flag_value(), "false");
+  ASSERT_EQ(query.local_flag_value(), "true");
+  ASSERT_EQ(query.is_readwrite(), true);
+  ASSERT_EQ(query.has_server_override(), false);
+  ASSERT_EQ(query.has_local_override(), true);
+
+  flag_override_result = send_flag_override_message(
+      "com.android.aconfig.storage.test_1", "disabled_rw", "false", true);
+  ASSERT_TRUE(flag_override_result.ok()) << flag_override_result.error();
+  ASSERT_EQ(flag_override_result->msgs_size(), 1);
+  return_message = flag_override_result->msgs(0);
+  ASSERT_TRUE(return_message.has_flag_override_message());
+
+  flag_query_result = send_flag_query_message(
+      "com.android.aconfig.storage.test_1", "disabled_rw");
+  ASSERT_TRUE(flag_query_result.ok()) << flag_query_result.error();
+  ASSERT_EQ(flag_query_result->msgs_size(), 1);
+  return_message = flag_query_result->msgs(0);
+  ASSERT_TRUE(return_message.has_flag_query_message());
+  query = return_message.flag_query_message();
+  ASSERT_EQ(query.server_flag_value(), "false");
+  ASSERT_EQ(query.local_flag_value(), "false");
+  ASSERT_EQ(query.is_readwrite(), true);
+  ASSERT_EQ(query.has_server_override(), false);
+  ASSERT_EQ(query.has_local_override(), true);
 }
 
 TEST(aconfigd_socket, nonexist_flag_override_message) {
@@ -208,12 +259,21 @@ TEST(aconfigd_socket, nonexist_flag_override_message) {
   ASSERT_TRUE(return_message.has_new_storage_message());
 
   auto flag_override_result = send_flag_override_message(
-      "com.android.aconfig.storage.test_1", "unknown", "true", false);
+      "unknown", "enabled_rw", "true", false);
   ASSERT_TRUE(flag_override_result.ok()) << flag_override_result.error();
   ASSERT_EQ(flag_override_result->msgs_size(), 1);
   return_message = flag_override_result->msgs(0);
   ASSERT_TRUE(return_message.has_error_message());
   auto errmsg = return_message.error_message();
+  ASSERT_TRUE(errmsg.find("Failed to find package unknown") != std::string::npos);
+
+  flag_override_result = send_flag_override_message(
+      "com.android.aconfig.storage.test_1", "unknown", "true", false);
+  ASSERT_TRUE(flag_override_result.ok()) << flag_override_result.error();
+  ASSERT_EQ(flag_override_result->msgs_size(), 1);
+  return_message = flag_override_result->msgs(0);
+  ASSERT_TRUE(return_message.has_error_message());
+  errmsg = return_message.error_message();
   ASSERT_TRUE(errmsg.find("Failed to find flag unknown") != std::string::npos);
 }
 
@@ -225,13 +285,23 @@ TEST(aconfigd_socket, nonexist_flag_query_message) {
   ASSERT_TRUE(return_message.has_new_storage_message());
 
   auto flag_query_result = send_flag_query_message(
-      "com.android.aconfig.storage.test_1", "unknown");
+      "unknown", "enabled_rw");
   ASSERT_TRUE(flag_query_result.ok()) << flag_query_result.error();
   ASSERT_EQ(flag_query_result->msgs_size(), 1);
   return_message = flag_query_result->msgs(0);
   ASSERT_TRUE(return_message.has_error_message());
   auto query = return_message.flag_query_message();
   auto errmsg = return_message.error_message();
+  ASSERT_TRUE(errmsg.find("Failed to find package unknown") != std::string::npos);
+
+ flag_query_result = send_flag_query_message(
+      "com.android.aconfig.storage.test_1", "unknown");
+  ASSERT_TRUE(flag_query_result.ok()) << flag_query_result.error();
+  ASSERT_EQ(flag_query_result->msgs_size(), 1);
+  return_message = flag_query_result->msgs(0);
+  ASSERT_TRUE(return_message.has_error_message());
+  query = return_message.flag_query_message();
+  errmsg = return_message.error_message();
   ASSERT_TRUE(errmsg.find("Failed to find flag unknown") != std::string::npos);
 }