Skip to content

Commit

Permalink
Remove unused needsadmin attribute
Browse files Browse the repository at this point in the history
The needsadmin attribute is not used by Flatcar and it also wouldn't
have a use case in the future.
Remove it from the code base to focus on what makes sense.
  • Loading branch information
pothos committed Jul 5, 2023
1 parent 4c11728 commit 677d950
Show file tree
Hide file tree
Showing 4 changed files with 1 addition and 19 deletions.
4 changes: 1 addition & 3 deletions src/update_engine/omaha_request_action.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ static const char* kTagMaxFailureCountPerUrl = "MaxFailureCountPerUrl";
// Deprecated: "MoreInfo"; See https://github.com/google/omaha/blob/main/doc/ServerProtocolV2.md
// "And on the Mac, the Google Software Update Agent supports the following optional attributes": Prompt, MoreInfo
// Deprecated: "Prompt"
static const char* kTagNeedsAdmin = "needsadmin";
// Deprecated: "needsadmin"; Not used by Flatcar
static const char* kTagSha256 = "sha256";

namespace {
Expand Down Expand Up @@ -591,8 +591,6 @@ bool OmahaRequestAction::ParseParams(xmlDoc* doc,
}

// Get the optional properties one by one.
output_object->needs_admin =
XmlGetProperty(pie_action_node, kTagNeedsAdmin) == "true";

string max = XmlGetProperty(pie_action_node, kTagMaxFailureCountPerUrl);
if (!strings::StringToUint(max, &output_object->max_failure_count_per_url))
Expand Down
11 changes: 0 additions & 11 deletions src/update_engine/omaha_request_action_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ string GetUpdateResponse2(const string& app_id,
const string& codebase,
const string& filename,
const string& hash,
const string& needsadmin,
const string& size) {
string response =
"<?xml version=\"1.0\" encoding=\"UTF-8\"?><response "
Expand All @@ -88,7 +87,6 @@ string GetUpdateResponse2(const string& app_id,
"IsDelta=\"true\" "
"IsDeltaPayload=\"true\" "
"sha256=\"" + hash + "\" "
"needsadmin=\"" + needsadmin + "\" " +
"/></actions></manifest></updatecheck></app></response>";
LOG(INFO) << "Response = " << response;
return response;
Expand All @@ -99,14 +97,12 @@ string GetUpdateResponse(const string& app_id,
const string& codebase,
const string& filename,
const string& hash,
const string& needsadmin,
const string& size) {
return GetUpdateResponse2(app_id,
display_version,
codebase,
filename,
hash,
needsadmin,
size);
}
} // namespace {}
Expand Down Expand Up @@ -207,7 +203,6 @@ TEST(OmahaRequestActionTest, ValidUpdateTest) {
"http://code/base/", // dl url
"file.signed", // file name
"HASH1234=", // checksum
"false", // needs admin
"123"), // size
-1,
false, // ping_only
Expand All @@ -220,7 +215,6 @@ TEST(OmahaRequestActionTest, ValidUpdateTest) {
EXPECT_EQ("http://code/base/file.signed", response.payload_urls[0]);
EXPECT_EQ("HASH1234=", response.hash);
EXPECT_EQ(123, response.size);
EXPECT_FALSE(response.needs_admin);
}

TEST(OmahaRequestActionTest, NoOutputPipeTest) {
Expand Down Expand Up @@ -340,7 +334,6 @@ TEST(OmahaRequestActionTest, MissingFieldTest) {
"IsDelta=\"true\" "
"IsDeltaPayload=\"false\" "
"sha256=\"lkq34j5345\" "
"needsadmin=\"true\" "
"/></actions></manifest></updatecheck></app></response>";
LOG(INFO) << "Input Response = " << input_response;

Expand All @@ -358,7 +351,6 @@ TEST(OmahaRequestActionTest, MissingFieldTest) {
EXPECT_EQ("http://missing/field/test/f", response.payload_urls[0]);
EXPECT_EQ("lkq34j5345", response.hash);
EXPECT_EQ(587, response.size);
EXPECT_TRUE(response.needs_admin);
}

TEST(OmahaRequestActionTest, ConcatUrlSlashTest) {
Expand All @@ -376,7 +368,6 @@ TEST(OmahaRequestActionTest, ConcatUrlSlashTest) {
"IsDelta=\"true\" "
"IsDeltaPayload=\"false\" "
"sha256=\"lkq34j5345\" "
"needsadmin=\"true\" "
"/></actions></manifest></updatecheck></app></response>";
LOG(INFO) << "Input Response = " << input_response;

Expand Down Expand Up @@ -490,7 +481,6 @@ TEST(OmahaRequestActionTest, XmlDecodeTest) {
"testthe&amp;codebase/", // dl url
"file.signed", // file name
"HASH1234=", // checksum
"false", // needs admin
"123"), // size
-1,
false, // ping_only
Expand All @@ -511,7 +501,6 @@ TEST(OmahaRequestActionTest, ParseIntTest) {
"thecodebase/", // dl url
"file.signed", // file name
"HASH1234=", // checksum
"false", // needs admin
// overflows int32:
"123123123123123"), // size
-1,
Expand Down
2 changes: 0 additions & 2 deletions src/update_engine/omaha_response.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ struct OmahaResponse {
poll_interval(0),
size(0),
max_failure_count_per_url(0),
needs_admin(false),
is_delta_payload(false),
disable_payload_backoff(false) {}

Expand All @@ -45,7 +44,6 @@ struct OmahaResponse {
// next URL in the current pass. This is a configurable value from the
// Omaha Response attribute, if ever we need to fine tune the behavior.
uint32_t max_failure_count_per_url;
bool needs_admin;

// True if the payload described in this response is a delta payload.
// False if it's a full payload.
Expand Down
3 changes: 0 additions & 3 deletions src/update_engine/omaha_response_handler_action_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ TEST_F(OmahaResponseHandlerActionTest, SimpleTest) {
in.payload_urls.push_back("http://foo/the_update_a.b.c.d.tgz");
in.hash = "HASH+";
in.size = 12;
in.needs_admin = true;
InstallPlan install_plan;
EXPECT_TRUE(DoTest(in, "/dev/sda3", &install_plan));
EXPECT_EQ(in.payload_urls[0], install_plan.download_url);
Expand All @@ -102,7 +101,6 @@ TEST_F(OmahaResponseHandlerActionTest, SimpleTest) {
in.payload_urls.push_back("http://foo/the_update_a.b.c.d.tgz");
in.hash = "HASHj+";
in.size = 12;
in.needs_admin = true;
InstallPlan install_plan;
EXPECT_TRUE(DoTest(in, "/dev/sda4", &install_plan));
EXPECT_EQ(in.payload_urls[0], install_plan.download_url);
Expand All @@ -117,7 +115,6 @@ TEST_F(OmahaResponseHandlerActionTest, SimpleTest) {
in.payload_urls.push_back(kLongName);
in.hash = "HASHj+";
in.size = 12;
in.needs_admin = true;
InstallPlan install_plan;
EXPECT_TRUE(DoTest(in, "/dev/sda3", &install_plan));
EXPECT_EQ(in.payload_urls[0], install_plan.download_url);
Expand Down

0 comments on commit 677d950

Please sign in to comment.