Add x-ms-verify-from-artifacts-blob to extensionsArtifact calls #2655

narrieta · 2022-08-19T23:24:07Z

The HostGAPlugin requires the x-ms-verify-from-artifacts-blob header in extensionsArtifact requests for Fast Track Goal states.

This PR adds that header. Most of the changes in the PR are code refactoring that makes this simple, plus some refactoring that has been delayed in previous changes, as well as a few changes for Python 3.10

narrieta · 2022-08-19T23:25:24Z

azurelinuxagent/common/protocol/extensions_goal_state.py

@@ -17,7 +17,7 @@
 # Requires Python 2.6+ and Openssl 1.0+
 import datetime

-import azurelinuxagent.common.logger as logger
+from azurelinuxagent.common import logger


These changes in import statements address new pylint warnings for Python 3.10, there are several of them in the PR

codecov · 2022-08-19T23:29:52Z

Codecov Report

Merging #2655 (8d86688) into develop (51b27a1) will decrease coverage by 0.00%.
The diff coverage is 94.11%.

@@             Coverage Diff             @@
##           develop    #2655      +/-   ##
===========================================
- Coverage    71.97%   71.96%   -0.01%     
===========================================
  Files          103      103              
  Lines        15688    15679       -9     
  Branches      2485     2486       +1     
===========================================
- Hits         11291    11283       -8     
+ Misses        3881     3879       -2     
- Partials       516      517       +1

Impacted Files	Coverage Δ
azurelinuxagent/common/protocol/hostplugin.py	`87.23% <66.66%> (-0.44%)`	⬇️
azurelinuxagent/common/protocol/goal_state.py	`94.58% <89.36%> (-0.74%)`	⬇️
...inuxagent/common/protocol/extensions_goal_state.py	`74.46% <100.00%> (ø)`
...ol/extensions_goal_state_from_extensions_config.py	`92.05% <100.00%> (ø)`
...protocol/extensions_goal_state_from_vm_settings.py	`83.45% <100.00%> (ø)`
azurelinuxagent/common/protocol/restapi.py	`97.50% <100.00%> (ø)`
azurelinuxagent/common/protocol/wire.py	`77.83% <100.00%> (-0.71%)`	⬇️
azurelinuxagent/common/utils/archive.py	`81.92% <100.00%> (+0.20%)`	⬆️
azurelinuxagent/ga/exthandlers.py	`85.70% <100.00%> (+0.01%)`	⬆️
azurelinuxagent/ga/update.py	`89.34% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

narrieta · 2022-08-19T23:29:52Z

azurelinuxagent/common/protocol/extensions_goal_state.py

@@ -126,7 +126,7 @@ def on_hold(self):
        raise NotImplementedError()

    @property
-    def agent_manifests(self):
+    def agent_families(self):


There was a class in restapi.py named VMAgentManifest which did not represent a manifest, but rather the GAFamilies element in ExtensionConfigs (which includes the name of the family (Prod vs Test), the required agent version, and the URIs of the actual manifests). This naming made the code extremely confusing, especially in code that deals with VMAgentManifest and actual agent manifest. I renamed VMAgentManifest to VMAgentFamily so you will see those renames throughout this PR.

narrieta · 2022-08-19T23:31:13Z

azurelinuxagent/common/protocol/restapi.py

-class VMAgentManifest(object):
-    def __init__(self, family, version=None):
-        self.family = family
+class VMAgentFamily(object):


I renamed VMAgentManifest to VMAgentFamily because this class does not represent a manifest, but rather the GAFamilies element in ExtensionConfigs (which includes the name of the family (Prod vs Test), the required agent version, and the URIs of the actual manifests). This naming made the code extremely confusing, especially in code that deals with VMAgentManifest and actual agent manifest. You will see those renames throughout this PR.

narrieta · 2022-08-19T23:34:26Z

azurelinuxagent/common/protocol/goal_state.py

@@ -129,6 +129,36 @@ def shared_conf(self):
    def remote_access(self):
        return self._remote_access

+    def fetch_agent_manifest(self, family_name, uris):


The extensionsArtifact API requires the use of the x-ms-verify-from-artifacts-blob header for FastTrack goal states. I added these convenience functions to the GoalState class to make this easier.

narrieta · 2022-08-19T23:35:22Z

azurelinuxagent/common/protocol/goal_state.py

@@ -562,3 +592,46 @@ def _parse_user(user):
        remote_access_user = RemoteAccessUser(name, encrypted_password, expiration)
        return remote_access_user

+


This class was moved here from wire.py

narrieta · 2022-08-19T23:41:46Z

azurelinuxagent/common/protocol/wire.py

@@ -111,22 +109,6 @@ def get_certs(self):
        certificates = self.client.get_certs()
        return certificates.cert_list

-    def get_vmagent_manifests(self):


WireClient maintains a reference to the goal state (returned by get_goal_state). WireClient is held within a WireProtocol, which is returned by ProtocolUtil, which is a singleton. Basically, this makes the goal state a global variable, and it has introduced threading issues in the past. We have been removing the use of WireClient.get_goal_state() over time. This PR presented an opportunity to remove a couple of them.

narrieta · 2022-08-19T23:43:48Z

azurelinuxagent/common/protocol/hostplugin.py

@@ -55,6 +55,7 @@
 _HEADER_HOST_CONFIG_NAME = "x-ms-host-config-name"
 _HEADER_ARTIFACT_LOCATION = "x-ms-artifact-location"
 _HEADER_ARTIFACT_MANIFEST_LOCATION = "x-ms-artifact-manifest-location"
+_HEADER_VERIFY_FROM_ARTIFACTS_BLOB = "x-ms-verify-from-artifacts-blob"


This is the header required by the HostGAPlugin for Fast Track goal states. It's value is arbitrary and it is set to "true", but the HostGAPlugin just checks for its presence, not its value.

narrieta · 2022-08-19T23:45:15Z

azurelinuxagent/common/protocol/wire.py

@@ -808,30 +790,11 @@ def get_certs(self):
            raise ProtocolError("Trying to fetch Certificates before initialization!")
        return self._goal_state.certs

-    def get_ext_manifest(self, ext_handler):


Move to the GoalState class

narrieta · 2022-08-19T23:45:31Z

azurelinuxagent/common/protocol/wire.py

@@ -1186,48 +1149,6 @@ def get_supported(self):
        return self.supported


-class ExtensionManifest(object): 


moved to goal_state.py

narrieta · 2022-08-19T23:46:14Z

azurelinuxagent/common/utils/archive.py

@@ -306,3 +306,6 @@ def save_hosting_env(self, text):

    def save_shared_conf(self, text):
        self.save(text, SHARED_CONF_FILE_NAME)
+


Convenience function to save manifests to the history folder

narrieta · 2022-08-19T23:47:01Z

azurelinuxagent/ga/exthandlers.py

@@ -949,9 +949,9 @@ def get_ext_handlers_status_debug_info(self, vm_status):
        if status_blob_text is None:
            status_blob_text = ""

-        support_multi_config = dict()
+        support_multi_config = {}


pylint warning about dict() vs {}

narrieta · 2022-08-19T23:48:35Z

azurelinuxagent/ga/exthandlers.py

@@ -1259,7 +1260,7 @@ def download(self):
                self.logger.info("The existing extension package is invalid, will ignore it.")

        if not package_exists:
-            self.protocol.client.download_extension(self.pkg.uris, destination, on_downloaded=lambda: self._unzip_extension_package(destination, self.get_base_dir()))
+            self.protocol.get_goal_state().download_extension(self.pkg.uris, destination, on_downloaded=lambda: self._unzip_extension_package(destination, self.get_base_dir()))


We need to get rid of WireProtocol.get_goal_state(), since it returns a global. That requires deeper refactoring, using it for the moment

narrieta · 2022-08-19T23:49:19Z

azurelinuxagent/ga/update.py

@@ -576,7 +575,7 @@ def handle_updates_for_requested_version():
            # The call to get_latest_agent_greater_than_daemon() also finds all agents in directory and sets the self.agents property.
            # This state is used to find the GuestAgent object with the current version later if requested version is available in last GS.
            available_agent = self.get_latest_agent_greater_than_daemon()
-            requested_version, _ = self.__get_requested_version_and_manifest_from_last_gs(protocol)
+            requested_version, _ = self.__get_requested_version_and_agent_family_from_last_gs()


Part of renaming "agent manifest" to "agent family"

narrieta · 2022-08-19T23:51:44Z

azurelinuxagent/ga/update.py

@@ -986,7 +985,7 @@ def _is_orphaned(self):

    def _load_agents(self):
        path = os.path.join(conf.get_lib_dir(), "{0}-*".format(AGENT_NAME))
-        return [GuestAgent(path=agent_dir)
+        return [GuestAgent(is_fast_track_goal_state=self._goal_state.extensions_goal_state.source == GoalStateSource.FastTrack, path=agent_dir)


Class GuestAgent actually downloads the agent packages. That download still needs to be refactored into the common download logic. For the moment, this sets up the x-ms-verify-from-artifacts-blob header

narrieta · 2022-08-19T23:52:35Z

ci/3.6.pylintrc

@@ -3,18 +3,22 @@
 [MESSAGES CONTROL]

 disable=C, # (C) convention, for programming standard violation
+    broad-except, # (W0703): *Catching too general exception %s*


I sorted the warnings alphabetically and added redundant-u-string-prefix (which is pointed out by Python 3.10)

narrieta · 2022-08-19T23:54:40Z

tests/ga/test_extension.py

@@ -1361,7 +1361,7 @@ def test_ext_handler_report_status_resource_gone(self, mock_add_event, *args):
    def test_ext_handler_download_failure_permanent_ProtocolError(self, mock_add_event, mock_error_state, *args):
        test_data = mockwiredata.WireProtocolData(mockwiredata.DATA_FILE)
        exthandlers_handler, protocol = self._create_mock(test_data, *args)  # pylint: disable=no-value-for-parameter
-        protocol.get_ext_handler_pkgs = Mock(side_effect=ProtocolError)
+        protocol.get_goal_state().fetch_extension_manifest = Mock(side_effect=ProtocolError)


These mocks in the tests use knowledge of internals of the code they are testing. They need to be rewritten. For the moment I just changed them to match the new production code.

narrieta · 2022-08-19T23:56:40Z

tests/ga/test_update.py

 class GoalStateMock(object):
-    def __init__(self, incarnation):
+    def __init__(self, incarnation, family, versions):


Some of the logic in the production code was moved from WireProtocol to GoalState. Updating the mocks to match those changes. These mocks need to be rewritten.

nagworld9 · 2022-08-24T14:40:25Z

azurelinuxagent/common/protocol/wire.py

@@ -597,14 +579,14 @@ def call_storage_service(http_req, *args, **kwargs):
        return http_req(*args, **kwargs)

    def fetch_artifacts_profile_blob(self, uri):
-        return self._fetch_content("artifacts profile blob", [uri])[1]  # _fetch_content returns a (uri, content) tuple
+        return self._fetch_content("artifacts profile blob", [uri], use_verify_header=False)[1]  # _fetch_content returns a (uri, content) tuple


is this called only on fabric goal state?

The header is needed only when downloading manifests or packages (extension's or agent's). I'll add a comment in the code,

And also, you are right, this is called only for Fabric. We need it to get the onHold property, which comes in HGAP's vmSettings for FT goal states.

Add x-ms-verify-from-artifacts-blob to extensionsArtifact calls

8b13620

narrieta requested review from ZhidongPeng and nagworld9 as code owners August 19, 2022 23:24

narrieta commented Aug 19, 2022

View reviewed changes

nagworld9 reviewed Aug 24, 2022

View reviewed changes

narrieta and others added 3 commits September 1, 2022 15:18

Check for null goal state

fc3d319

Merge branch 'develop' into verify-header

f0b5939

Improve API interface

8d86688

nagworld9 approved these changes Sep 2, 2022

View reviewed changes

narrieta merged commit ef996e2 into Azure:develop Sep 3, 2022

narrieta deleted the verify-header branch September 3, 2022 00:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add x-ms-verify-from-artifacts-blob to extensionsArtifact calls #2655

Add x-ms-verify-from-artifacts-blob to extensionsArtifact calls #2655

narrieta commented Aug 19, 2022

narrieta Aug 19, 2022

codecov bot commented Aug 19, 2022 •

edited

Loading

narrieta Aug 19, 2022

narrieta Aug 19, 2022

narrieta Aug 19, 2022

narrieta Aug 19, 2022

narrieta Aug 19, 2022

narrieta Aug 19, 2022 •

edited

Loading

narrieta Aug 19, 2022

narrieta Aug 19, 2022

narrieta Aug 19, 2022

narrieta Aug 19, 2022

narrieta Aug 19, 2022

narrieta Aug 19, 2022

narrieta Aug 19, 2022

narrieta Aug 19, 2022 •

edited

Loading

narrieta Aug 19, 2022

narrieta Aug 19, 2022

nagworld9 Aug 24, 2022

narrieta Sep 1, 2022

		@@ -562,3 +592,46 @@ def _parse_user(user):
		remote_access_user = RemoteAccessUser(name, encrypted_password, expiration)
		return remote_access_user

		@@ -1186,48 +1149,6 @@ def get_supported(self):
		return self.supported


		class ExtensionManifest(object):

		@@ -306,3 +306,6 @@ def save_hosting_env(self, text):

		def save_shared_conf(self, text):
		self.save(text, SHARED_CONF_FILE_NAME)

Add x-ms-verify-from-artifacts-blob to extensionsArtifact calls #2655

Add x-ms-verify-from-artifacts-blob to extensionsArtifact calls #2655

Conversation

narrieta commented Aug 19, 2022

Choose a reason for hiding this comment

codecov bot commented Aug 19, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

narrieta Aug 19, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

narrieta Aug 19, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Aug 19, 2022 •

edited

Loading

narrieta Aug 19, 2022 •

edited

Loading

narrieta Aug 19, 2022 •

edited

Loading