Repository: Azure/WALinuxAgent
Pull request #2378 (2021-10-07 02:26:12)

Set up supporting code to compare ExtensionsConfig with vmSettings
This PR prepares the code to compare ExtensionsConfig with vmSettings. 

For now, it only compares the RequiredFeatures; subsequent PRs will parse and compare other attributes.

codecov[bot]: # [Codecov](https://codecov.io/gh/Azure/WALinuxAgent/pull/2378?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure) Report
> Merging [#2378](https://codecov.io/gh/Azure/WALinuxAgent/pull/2378?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure) (c31c7f9) into [develop](https://codecov.io/gh/Azure/WALinuxAgent/commit/6884f7c0f548186bbf387c01e474237f73597e00?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure) (6884f7c) will **increase** coverage by `0.05%`.
> The diff coverage is `90.19%`.

[![Impacted file tree graph](https://codecov.io/gh/Azure/WALinuxAgent/pull/2378/graphs/tree.svg?width=650&height=150&src=pr&token=ae90E2KYCw&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure)](https://codecov.io/gh/Azure/WALinuxAgent/pull/2378?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure)

```diff
@@             Coverage Diff             @@
##           develop    #2378      +/-   ##
===========================================
+ Coverage    70.89%   70.94%   +0.05%     
===========================================
  Files           97       97              
  Lines        14107    14142      +35     
  Branches      2029     2033       +4     
===========================================
+ Hits         10001    10033      +32     
+ Misses        3657     3656       -1     
- Partials       449      453       +4     
```


| [Impacted Files](https://codecov.io/gh/Azure/WALinuxAgent/pull/2378?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure) | Coverage Δ | |
|---|---|---|
| [azurelinuxagent/common/protocol/restapi.py](https://codecov.io/gh/Azure/WALinuxAgent/pull/2378/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure#diff-YXp1cmVsaW51eGFnZW50L2NvbW1vbi9wcm90b2NvbC9yZXN0YXBpLnB5) | `97.38% <ø> (-0.06%)` | :arrow_down: |
| [azurelinuxagent/common/protocol/wire.py](https://codecov.io/gh/Azure/WALinuxAgent/pull/2378/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure#diff-YXp1cmVsaW51eGFnZW50L2NvbW1vbi9wcm90b2NvbC93aXJlLnB5) | `78.52% <81.81%> (-0.08%)` | :arrow_down: |
| [...inuxagent/common/protocol/extensions\_goal\_state.py](https://codecov.io/gh/Azure/WALinuxAgent/pull/2378/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure#diff-YXp1cmVsaW51eGFnZW50L2NvbW1vbi9wcm90b2NvbC9leHRlbnNpb25zX2dvYWxfc3RhdGUucHk=) | `90.08% <92.30%> (+0.55%)` | :arrow_up: |
| [azurelinuxagent/common/event.py](https://codecov.io/gh/Azure/WALinuxAgent/pull/2378/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure#diff-YXp1cmVsaW51eGFnZW50L2NvbW1vbi9ldmVudC5weQ==) | `85.32% <100.00%> (+0.03%)` | :arrow_up: |
| [azurelinuxagent/ga/exthandlers.py](https://codecov.io/gh/Azure/WALinuxAgent/pull/2378/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure#diff-YXp1cmVsaW51eGFnZW50L2dhL2V4dGhhbmRsZXJzLnB5) | `86.25% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/Azure/WALinuxAgent/pull/2378?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/Azure/WALinuxAgent/pull/2378?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure). Last update [6884f7c...c31c7f9](https://codecov.io/gh/Azure/WALinuxAgent/pull/2378?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure).


Patch:
diff --git a/azurelinuxagent/common/event.py b/azurelinuxagent/common/event.py
index 0887a924..aa1907ba 100644
--- a/azurelinuxagent/common/event.py
+++ b/azurelinuxagent/common/event.py
@@ -88,6 +88,7 @@ class WALAEventOperation:
     FetchGoalState = "FetchGoalState"
     Firewall = "Firewall"
     GoalState = "GoalState"
+    GoalStateMismatch = "GoalStateMismatch"
     GoalStateUnsupportedFeatures = "GoalStateUnsupportedFeatures"
     HealthCheck = "HealthCheck"
     HealthObservation = "HealthObservation"
diff --git a/azurelinuxagent/common/protocol/extensions_goal_state.py b/azurelinuxagent/common/protocol/extensions_goal_state.py
index 2d0a940f..c02f29e6 100644
--- a/azurelinuxagent/common/protocol/extensions_goal_state.py
+++ b/azurelinuxagent/common/protocol/extensions_goal_state.py
@@ -17,21 +17,31 @@
 # Requires Python 2.6+ and Openssl 1.0+
 
 import json
+import re
 
 from collections import defaultdict
+from operator import attrgetter
 
 import azurelinuxagent.common.logger as logger
 from azurelinuxagent.common.event import add_event, WALAEventOperation
 from azurelinuxagent.common.exception import ExtensionConfigError
 from azurelinuxagent.common.future import ustr
 from azurelinuxagent.common.protocol.restapi import Extension, ExtHandler, ExtHandlerList, ExtHandlerVersionUri, VMAgentManifest, \
-    VMAgentManifestList, VMAgentManifestUri, InVMGoalStateMetaData, RequiredFeature, ExtensionState
-from azurelinuxagent.common.utils.textutil import parse_doc, findall, find, findtext, getattrib, gettext
+    VMAgentManifestList, VMAgentManifestUri, InVMGoalStateMetaData, ExtensionState
+from azurelinuxagent.common.utils.textutil import parse_doc, findall, find, findtext, getattrib, gettext, format_exception
 
 
 class ExtensionsGoalState(object):
+    """
+    ExtensionsGoalState represents the extensions information in the goal state; that information can originate from
+    ExtensionsConfig when the goal state is retrieved from the WireServe or from vmSettings when it is retrieved from
+    the HostGAPlugin.
+
+    NOTE: Do not instantiate this class directly, use one of the create_* methods instead.
+    """
     def __init__(self):
-        self.xml_text = None
+        self._id = None
+        self._text = None
         self.ext_handlers = ExtHandlerList()
         self.vmagent_manifests = VMAgentManifestList()
         self.in_vm_gs_metadata = InVMGoalStateMetaData()
@@ -41,11 +51,62 @@ class ExtensionsGoalState(object):
         self.required_features = []
 
     @staticmethod
-    def from_extensions_config(xml_text):
-        extensions_goal_state = ExtensionsGoalState()
-        extensions_goal_state.xml_text = xml_text
+    def create_empty():
+        return _EmptyExtensionsGoalState()
+
+    @staticmethod
+    def create_from_extensions_config(incarnation, xml_text):
+        return _ExtensionsGoalStateFromExtensionsConfig(incarnation, xml_text)
+
+    @staticmethod
+    def create_from_vm_settings(etag, json_text):
+        return _ExtensionsGoalStateFromVmSettings(etag, json_text)
+
+    def get_id(self):
+        """
+        Returns the incarnation number if the ExtensionsGoalState was created from ExtensionsConfig, or the etag if it
+        was created from vmSettings.
+        """
+        return self._id
+
+    def get_redacted_text(self):
+        """
+        Returns the raw text (either the ExtensionsConfig or the vmSettings) with any confidential data removed, or None for empty goal states.
+        """
+        raise NotImplementedError()
+
+    @staticmethod
+    def compare(from_extensions_config, from_vm_settings):
+        """
+        Compares the two instances given as argument and logs a GoalStateMismatch message if they are different.
+
+        NOTE: The order of the two instances is important for the debug info to be logged correctly (ExtensionsConfig first, vmSettings second)
+        """
+        def report_difference(attribute):
+            message = "Mismatch in ExtensionsConfig (incarnation {0}) and vmSettings (etag {1}).\nAttribute: {2}\n{3} != {4}".format(
+                from_extensions_config.get_id(), from_vm_settings.get_id(),
+                attribute,
+                attrgetter(attribute)(from_extensions_config), attrgetter(attribute)(from_vm_settings)
+            )
+            logger.info("[GoalStateMismatch] {0}", message)
+            add_event(op=WALAEventOperation.GoalStateMismatch, is_success=False, message=message)
+
+        if from_extensions_config.required_features != from_vm_settings.required_features:
+            report_difference("required_features")
 
-        xml_doc = parse_doc(extensions_goal_state.xml_text)
+
+class _EmptyExtensionsGoalState(ExtensionsGoalState):
+    def get_redacted_text(self):
+        return None
+
+
+class _ExtensionsGoalStateFromExtensionsConfig(ExtensionsGoalState):
+    def __init__(self, incarnation, xml_text):
+        super(_ExtensionsGoalStateFromExtensionsConfig, self).__init__()
+        self._id = incarnation
+        self._text = xml_text
+
+        xml_doc = parse_doc(xml_text)
 
         ga_families_list = find(xml_doc, "GAFamilies")
         ga_families = findall(ga_families_list, "GAFamily")
@@ -59,44 +120,36 @@ class ExtensionsGoalState(object):
             for uri in uris:
                 manifest_uri = VMAgentManifestUri(uri=gettext(uri))
                 manifest.versionsManifestUris.append(manifest_uri)
-            extensions_goal_state.vmagent_manifests.vmAgentManifests.append(manifest)
+            self.vmagent_manifests.vmAgentManifests.append(manifest)
 
-        extensions_goal_state.__parse_plugins_and_settings_and_populate_ext_handlers(xml_doc)
+        self.__parse_plugins_and_settings_and_populate_ext_handlers(xml_doc)
 
         required_features_list = find(xml_doc, "RequiredFeatures")
         if required_features_list is not None:
-            extensions_goal_state._parse_required_features(required_features_list)
+            self._parse_required_features(required_features_list)
 
-        extensions_goal_state.status_upload_blob = findtext(xml_doc, "StatusUploadBlob")
-        extensions_goal_state.artifacts_profile_blob = findtext(xml_doc, "InVMArtifactsProfileBlob")
+        self.status_upload_blob = findtext(xml_doc, "StatusUploadBlob")
+        self.artifacts_profile_blob = findtext(xml_doc, "InVMArtifactsProfileBlob")
 
         status_upload_node = find(xml_doc, "StatusUploadBlob")
-        extensions_goal_state.status_upload_blob_type = getattrib(status_upload_node, "statusBlobType")
-        logger.verbose("Extension config shows status blob type as [{0}]", extensions_goal_state.status_upload_blob_type)
+        self.status_upload_blob_type = getattrib(status_upload_node, "statusBlobType")
+        logger.verbose("Extension config shows status blob type as [{0}]", self.status_upload_blob_type)
 
-        extensions_goal_state.in_vm_gs_metadata.parse_node(find(xml_doc, "InVMGoalStateMetaData"))
+        self.in_vm_gs_metadata.parse_node(find(xml_doc, "InVMGoalStateMetaData"))
 
-        return extensions_goal_state
-
-    @staticmethod
-    def from_vm_settings(json_text):
-        raise NotImplementedError()  # TODO: Implement parsing of vmSettings
+    def get_redacted_text(self):
+        text = self._text
+        for ext_handler in self.ext_handlers.extHandlers:
+            for extension in ext_handler.properties.extensions:
+                if extension.protectedSettings is not None:
+                    text = text.replace(extension.protectedSettings, "*** REDACTED ***")
+        return text
 
     def _parse_required_features(self, required_features_list):
         for required_feature in findall(required_features_list, "RequiredFeature"):
             feature_name = findtext(required_feature, "Name")
-            feature_value = findtext(required_feature, "Value")
-            self.required_features.append(RequiredFeature(name=feature_name, value=feature_value))
-
-    def get_redacted_xml_text(self):
-        if self.xml_text is None:
-            return "<None/>"
-        xml_text = self.xml_text
-        for ext_handler in self.ext_handlers.extHandlers:
-            for extension in ext_handler.properties.extensions:
-                if extension.protectedSettings is not None:
-                    xml_text = xml_text.replace(extension.protectedSettings, "*** REDACTED ***")
-        return xml_text
+            # per the documentation, RequiredFeatures also have a "Value" attribute but currently it is not being populated
+            self.required_features.append(feature_name)
 
     def __parse_plugins_and_settings_and_populate_ext_handlers(self, xml_doc):
         """
@@ -145,8 +198,8 @@ class ExtensionsGoalState(object):
         for plugin in plugins:
             ext_handler = ExtHandler()
             try:
-                ExtensionsGoalState._parse_plugin(ext_handler, plugin)
-                ExtensionsGoalState._parse_plugin_settings(ext_handler, plugin_settings)
+                _ExtensionsGoalStateFromExtensionsConfig._parse_plugin(ext_handler, plugin)
+                _ExtensionsGoalStateFromExtensionsConfig._parse_plugin_settings(ext_handler, plugin_settings)
             except ExtensionConfigError as error:
                 ext_handler.invalid_setting_reason = ustr(error)
 
@@ -287,11 +340,11 @@ class ExtensionsGoalState(object):
                     handler_name, version, len(runtime_settings_nodes))
                 raise ExtensionConfigError(msg)
             # Only Runtime settings available, parse that
-            ExtensionsGoalState.__parse_runtime_settings(plugin_settings_node, runtime_settings_nodes[0], handler_name,
+            _ExtensionsGoalStateFromExtensionsConfig.__parse_runtime_settings(plugin_settings_node, runtime_settings_nodes[0], handler_name,
                                                       ext_handler)
         elif extension_runtime_settings_nodes:
             # Parse the ExtensionRuntime settings for the given extension
-            ExtensionsGoalState.__parse_extension_runtime_settings(plugin_settings_node, extension_runtime_settings_nodes,
+            _ExtensionsGoalStateFromExtensionsConfig.__parse_extension_runtime_settings(plugin_settings_node, extension_runtime_settings_nodes,
                                                                 ext_handler)
 
     @staticmethod
@@ -335,8 +388,8 @@ class ExtensionsGoalState(object):
                 len(depends_on_nodes))
             raise ExtensionConfigError(msg)
         depends_on_node = depends_on_nodes[0] if depends_on_nodes else None
-        depends_on_level = ExtensionsGoalState.__get_dependency_level_from_node(depends_on_node, handler_name)
-        ExtensionsGoalState.__parse_and_add_extension_settings(runtime_settings_node, handler_name, ext_handler,
+        depends_on_level = _ExtensionsGoalStateFromExtensionsConfig.__get_dependency_level_from_node(depends_on_node, handler_name)
+        _ExtensionsGoalStateFromExtensionsConfig.__parse_and_add_extension_settings(runtime_settings_node, handler_name, ext_handler,
                                                             depends_on_level)
 
     @staticmethod
@@ -398,7 +451,7 @@ class ExtensionsGoalState(object):
             if extension_name in (None, ""):
                 raise ExtensionConfigError("No Name not specified for DependsOn object in ExtensionRuntimeSettings for MultiConfig!")
 
-            dependency_level = ExtensionsGoalState.__get_dependency_level_from_node(depends_on_node, extension_name)
+            dependency_level = _ExtensionsGoalStateFromExtensionsConfig.__get_dependency_level_from_node(depends_on_node, extension_name)
             dependency_levels[extension_name] = dependency_level
 
         ext_handler.supports_multi_config = True
@@ -410,7 +463,7 @@ class ExtensionsGoalState(object):
             # State can either be `ExtensionState.Enabled` (default) or `ExtensionState.Disabled`
             state = getattrib(extension_runtime_setting_node, "state")
             state = ustr(state.lower()) if state not in (None, "") else ExtensionState.Enabled
-            ExtensionsGoalState.__parse_and_add_extension_settings(extension_runtime_setting_node, extension_name,
+            _ExtensionsGoalStateFromExtensionsConfig.__parse_and_add_extension_settings(extension_runtime_setting_node, extension_name,
                                                                 ext_handler, dependency_levels[extension_name],
                                                                 state=state)
 
@@ -445,3 +498,31 @@ class ExtensionsGoalState(object):
             ext.certificateThumbprint = thumbprint
             ext_handler.properties.extensions.append(ext)
 
+
+class _ExtensionsGoalStateFromVmSettings(ExtensionsGoalState):
+    def __init__(self, etag, json_text):
+        super(_ExtensionsGoalStateFromVmSettings, self).__init__()
+        self._id = etag
+        self._text = json_text
+
+        try:
+            self._parse_vm_settings(json_text)
+        except Exception as e:
+            # TODO: Review code to determine whether we can rename ExtensionConfigError to InvalidExtensionsGoalStateError or
+            #       if we need to differentiate between errors in ExtensionsConfig and vmSettings.
+            raise ExtensionConfigError("Error parsing vmSettings (etag: {0}): {1}\n{2}".format(etag, format_exception(e), json_text))
+
+    def _parse_vm_settings(self, json_text):
+        vm_settings = json.loads(json_text)
+        self._parse_required_features(vm_settings)
+        # TODO: Parse all atttributes
+
+    def _parse_required_features(self, vm_settings):
+        required_features = vm_settings.get("requiredFeatures")
+        if required_features is not None:
+            if not isinstance(required_features, list):
+                raise Exception("requiredFeatures should be an array")
+            self.required_features.extend([feature["name"] for feature in required_features])
+
+    def get_redacted_text(self):
+        return re.sub(r'("protectedSettings"\s*:\s*)"[^"]+"', r'\1"*** REDACTED ***"', self._text)
diff --git a/azurelinuxagent/common/protocol/restapi.py b/azurelinuxagent/common/protocol/restapi.py
index 2f960b4a..5b1fc0c9 100644
--- a/azurelinuxagent/common/protocol/restapi.py
+++ b/azurelinuxagent/common/protocol/restapi.py
@@ -82,13 +82,6 @@ class VMAgentManifestList(DataContract):
         self.vmAgentManifests = DataContractList(VMAgentManifest)
 
 
-class RequiredFeature(DataContract):
-    def __init__(self, name, value=None):
-        self.name = name
-        # As per the docs, this is a reserved field and not currently in use.
-        self.value = value
-
-
 class ExtensionState(object):
     Enabled = ustr("enabled")
     Disabled = ustr("disabled")
diff --git a/azurelinuxagent/common/protocol/wire.py b/azurelinuxagent/common/protocol/wire.py
index 91fa1da0..78670007 100644
--- a/azurelinuxagent/common/protocol/wire.py
+++ b/azurelinuxagent/common/protocol/wire.py
@@ -19,7 +19,6 @@
 import json
 import os
 import random
-import re
 import time
 import uuid
 import xml.sax.saxutils as saxutils
@@ -124,9 +123,6 @@ class WireProtocol(DataContract):
     def get_incarnation(self):
         return self.client.get_goal_state().incarnation
 
-    def get_etag(self):
-        return self.client.get_etag()
-
     def get_in_vm_gs_metadata(self):
         return self.client.get_extensions_goal_state().in_vm_gs_metadata
 
@@ -578,9 +574,8 @@ class WireClient(object):
         logger.info("Wire server endpoint:{0}", endpoint)
         self._endpoint = endpoint
         self._goal_state = None
-        self._extensions_goal_state = None
-        self._vm_settings_text = None
-        self._vm_settings_etag = None
+        self._extensions_goal_state = None  # goal state from ExtensionsConfig
+        self._extensions_goal_state_from_vm_settings = None  # goal state from vmSettings
         self._host_plugin = None
         self.status_blob = StatusBlob(self)
         self.goal_state_flusher = StateFlusher(conf.get_lib_dir())
@@ -800,7 +795,9 @@ class WireClient(object):
             if force_update:
                 logger.info("Forcing an update of the goal state..")
 
-            if force_update or self._goal_state is None or self._goal_state.incarnation != goal_state.incarnation:
+            if force_update or \
+            self._goal_state is None or self._extensions_goal_state is None or \
+            self._goal_state.incarnation != goal_state.incarnation:
                 goal_state.fetch_full_goal_state(self)
                 self._goal_state = goal_state
                 #
@@ -808,21 +805,25 @@ class WireClient(object):
                 #       when not using fast track
                 # if not use_fast_track:
                 if self._goal_state.extensions_config_uri is None:
-                    self._extensions_goal_state = ExtensionsGoalState()
+                    self._extensions_goal_state = ExtensionsGoalState.create_empty()
                 else:
                     xml_text = self.fetch_config(self._goal_state.extensions_config_uri, self.get_header())
-                    self._extensions_goal_state = ExtensionsGoalState.from_extensions_config(xml_text)
+                    self._extensions_goal_state = ExtensionsGoalState.create_from_extensions_config(goal_state.incarnation, xml_text)
                 # END TODO
                 updated = True
 
             if use_fast_track:
-                #
-                # TODO: When Fast Track is fully enabled we should populate self._extensions_goal_state using the VmSettings
-                #
-                if self._update_vm_settings(force_update):
+                request_etag = None if force_update or self._extensions_goal_state_from_vm_settings is None else self._extensions_goal_state_from_vm_settings.get_id()
+                response_etag, vm_settings = self._fetch_vm_settings(request_etag)
+                if vm_settings is not None:
+                    # TODO: When Fast Track is fully enabled this should actually be assigned to self._extensions_goal_state
+                    self._extensions_goal_state_from_vm_settings = ExtensionsGoalState.create_from_vm_settings(response_etag, vm_settings)
                     updated = True
-
-                # TODO: Create an ExtensionsGoalState from the vmSettings and compare against self._extensions_goal_state
+                # TODO: When Fast Track is fully enabled this comparison should be removed
+                # If either goal state was updated, compare them
+                if updated:
+                    ExtensionsGoalState.compare(self._extensions_goal_state, self._extensions_goal_state_from_vm_settings)
+                # END TODO
 
             if updated:
                 self._save_goal_state()
@@ -830,9 +831,12 @@ class WireClient(object):
         except Exception as exception:
             raise ProtocolError("Error fetching goal state: {0}".format(ustr(exception)))
 
-    def _update_vm_settings(self, force_update):
+    def _fetch_vm_settings(self, etag):
+        """
+        Queries the vmSettings from the HostGAPlugin using the given etag, returns a tuple of the response's etag and content.
+        If the vmSettings have not been updated for the given etag returns (None, None).
+        """
         correlation_id = str(uuid.uuid4())
-        etag = None if force_update else self.get_etag()
 
         try:
             def get_vm_settings():
@@ -859,12 +863,11 @@ class WireClient(object):
                     break
 
             if response_etag is None:
-                return False
+                return None, None
 
             logger.info("Fetched new vmSettings [correlation ID: {0} New eTag: {1}]", correlation_id, response_etag)
-            self._vm_settings_text = vm_settings
-            self._vm_settings_etag = response_etag
-            return True
+
+            return response_etag, vm_settings
 
         except Exception as exception:
             raise ProtocolError("Error fetching vmSettings [correlation ID: {0} eTag: {1}]: {2}".format(correlation_id, etag, ustr(exception)))
@@ -891,12 +894,18 @@ class WireClient(object):
             save_if_not_none(self._goal_state.hosting_env, HOSTING_ENV_FILE_NAME)
             save_if_not_none(self._goal_state.shared_conf, SHARED_CONF_FILE_NAME)
             save_if_not_none(self._goal_state.remote_access, REMOTE_ACCESS_FILE_NAME.format(self._goal_state.incarnation))
-            if self._extensions_goal_state is not None and self._extensions_goal_state.xml_text is not None:
-                self._save_cache(self._extensions_goal_state.get_redacted_xml_text(), EXT_CONF_FILE_NAME.format(self._goal_state.incarnation))
-
-            if self._vm_settings_text is not None:
-                redacted_vm_settings_text = re.sub(r'("protectedSettings"\s*:\s*)"[^"]+"', r'\1"*** REDACTED ***"', self._vm_settings_text)
-                self._save_cache(redacted_vm_settings_text, VM_SETTINGS_FILE_NAME.format(self._vm_settings_etag))
+            if self._extensions_goal_state is not None:
+                # disable=assignment-from-none: E1128: Assigning result of a function call, where the function returns None
+                text = self._extensions_goal_state.get_redacted_text()  # pylint: disable=assignment-from-none
+                if text is not None:
+                    self._save_cache(text, EXT_CONF_FILE_NAME.format(self._extensions_goal_state.get_id()))
+            # TODO: When Fast Track is fully enabled self._extensions_goal_state_from_vm_settings will go away and this can be deleted
+            if self._extensions_goal_state_from_vm_settings is not None:
+                # disable=assignment-from-none: E1128: Assigning result of a function call, where the function returns None
+                text = self._extensions_goal_state_from_vm_settings.get_redacted_text()  # pylint: disable=assignment-from-none
+                if text is not None:
+                    self._save_cache(text, VM_SETTINGS_FILE_NAME.format(self._extensions_goal_state_from_vm_settings.get_id()))
+            # END TODO
 
         except Exception as e:
             logger.warn("Failed to save the goal state to disk: {0}", ustr(e))
@@ -906,14 +915,6 @@ class WireClient(object):
             logger.warn("Setting empty Host Plugin object!")
         self._host_plugin = new_host_plugin
 
-    def get_etag(self):
-        """
-        Returns the Etag of the current vmSettings, or None, if the vmSettings have not been retrieved.
-        """
-        if self._vm_settings_text is None:
-            return None
-        return self._vm_settings_etag
-
     def get_goal_state(self):
         if self._goal_state is None:
             raise ProtocolError("Trying to fetch goal state before initialization!")
diff --git a/azurelinuxagent/ga/exthandlers.py b/azurelinuxagent/ga/exthandlers.py
index 32e1195b..9b1ad98d 100644
--- a/azurelinuxagent/ga/exthandlers.py
+++ b/azurelinuxagent/ga/exthandlers.py
@@ -368,7 +368,7 @@ class ExtHandlersHandler(object):
     def __get_unsupported_features(self):
         required_features = self.protocol.get_required_features()
         supported_features = get_agent_supported_features_list_for_crp()
-        return [feature.name for feature in required_features if feature.name not in supported_features]
+        return [feature for feature in required_features if feature not in supported_features]
 
     def __process_and_handle_extensions(self, etag):
         try:


Test patch:
diff --git a/tests/ga/test_extension.py b/tests/ga/test_extension.py
index 3b9e06b9..8d5b341c 100644
--- a/tests/ga/test_extension.py
+++ b/tests/ga/test_extension.py
@@ -2592,11 +2592,7 @@ class TestExtension_Deprecated(AgentTestCase):
         required_features = protocol.get_required_features()
         self.assertEqual(3, len(required_features), "Incorrect features parsed")
         for i, feature in enumerate(required_features):
-            self.assertEqual(feature.name, "TestRequiredFeature{0}".format(i+1), "Name mismatch")
-            if i < 2:
-                self.assertIsNone(feature.value, "Value not present")
-            else:
-                self.assertEqual(feature.value, "3.0", "Value mismatch")
+            self.assertEqual(feature, "TestRequiredFeature{0}".format(i+1), "Name mismatch")
 
     def test_it_should_fail_goal_state_if_required_features_not_supported(self, mock_get, mock_crypt_util, *args):
         test_data = mockwiredata.WireProtocolData(mockwiredata.DATA_FILE_REQUIRED_FEATURES)
diff --git a/tests/ga/test_update.py b/tests/ga/test_update.py
index 781cb51a..2ed57218 100644
--- a/tests/ga/test_update.py
+++ b/tests/ga/test_update.py
@@ -1413,7 +1413,7 @@ class TestUpdate(UpdateTestCase):
         self.assertTrue(self._test_upgrade_available())
 
     def test_upgrade_available_handles_missing_family(self):
-        extensions_goal_state = ExtensionsGoalState.from_extensions_config(load_data("wire/ext_conf_missing_family.xml"))
+        extensions_goal_state = ExtensionsGoalState.create_from_extensions_config(123, load_data("wire/ext_conf_missing_family.xml"))
         protocol = ProtocolMock()
         protocol.family = "Prod"
         protocol.agent_manifests = extensions_goal_state.vmagent_manifests  # pylint: disable=attribute-defined-outside-init
diff --git a/tests/protocol/test_extensions_goal_state.py b/tests/protocol/test_extensions_goal_state.py
new file mode 100644
index 00000000..c070ddde
--- /dev/null
+++ b/tests/protocol/test_extensions_goal_state.py
@@ -0,0 +1,70 @@
+# Copyright (c) Microsoft Corporation. All rights reserved.
+# Licensed under the Apache License.
+import os.path
+
+from azurelinuxagent.common.event import WALAEventOperation
+from azurelinuxagent.common.future import httpclient
+from azurelinuxagent.common.protocol import hostplugin
+from azurelinuxagent.common.protocol.extensions_goal_state import ExtensionsGoalState
+from azurelinuxagent.common.utils import restutil, fileutil
+from tests.protocol.mocks import mock_wire_protocol
+from tests.protocol import mockwiredata
+from tests.protocol.mocks import HttpRequestPredicates, MockHttpResponse
+from tests.tools import AgentTestCase, data_dir, patch
+
+_original_http_request = restutil.http_request
+
+
+class ExtensionsGoalStateTestCase(HttpRequestPredicates, AgentTestCase):
+    def test_fetch_vm_settings_should_should_retry_on_resource_gone_error(self):
+        """
+        Requests to the hostgaplugin incude the Container ID and the RoleConfigName as headers; when the hostgaplugin returns GONE (HTTP status 410) the agent
+        needs to get a new goal state and retry the request with updated values for the Container ID and RoleConfigName headers.
+        """
+        with mock_wire_protocol(mockwiredata.DATA_FILE_VM_SETTINGS) as protocol:
+            # The mock_wire_protocol mocks azurelinuxagent.common.utils.restutil.http_request, but the GONE status is handled at a lower level, in the internal _http_request.
+            # To mock this, we bypass the mock http_request by calling the original http_request (function http_get_handler) and then mocking the internal _http_request (function
+            # http_get_vm_settings).
+            def http_get_handler(url, *_, **kwargs):
+                if self.is_host_plugin_vm_settings_request(url):
+                    return _original_http_request("GET", url, None, **kwargs)
+                return None
+            protocol.set_http_handlers(http_get_handler=http_get_handler)
+
+            request_headers = []  # we expect a retry with new headers and use this array to persist the headers of each request
+            def http_get_vm_settings(_method, _host, _relative_url, **kwargs):
+                request_headers.append(kwargs["headers"])
+                if len(request_headers) == 1:
+                    # Fail the first request with status GONE and update the mock data to return the new Container ID and RoleConfigName that should be
+                    # used in the headers of the retry request.
+                    protocol.mock_wire_data.set_container_id("GET_VM_SETTINGS_TEST_CONTAINER_ID")
+                    protocol.mock_wire_data.set_role_config_name("GET_VM_SETTINGS_TEST_ROLE_CONFIG_NAME")
+                    return MockHttpResponse(status=httpclient.GONE)
+                # For this test we are interested only on the retry logic, so the second request (the retry) is not important; we use NOT_MODIFIED (304) for simplicity.
+                return MockHttpResponse(status=httpclient.NOT_MODIFIED)
+
+            with patch("azurelinuxagent.common.utils.restutil._http_request", side_effect=http_get_vm_settings):
+                protocol.client._fetch_vm_settings(123)
+
+            self.assertEqual(2, len(request_headers), "We expected 2 requests for vmSettings: the original request and the retry request")
+            self.assertEqual("GET_VM_SETTINGS_TEST_CONTAINER_ID", request_headers[1][hostplugin._HEADER_CONTAINER_ID], "The retry request did not include the expected header for the ContainerId")
+            self.assertEqual("GET_VM_SETTINGS_TEST_ROLE_CONFIG_NAME", request_headers[1][hostplugin._HEADER_HOST_CONFIG_NAME], "The retry request did not include the expected header for the RoleConfigName")
+
+    def test_compare_should_report_mismatches_between_extensions_config_and_vm_settings(self):
+        from_vm_settings = ExtensionsGoalState.create_from_vm_settings("123", fileutil.read_file(os.path.join(data_dir, "hostgaplugin/vm_settings.json")))
+        from_extensions_config = ExtensionsGoalState.create_from_extensions_config("123", fileutil.read_file(os.path.join(data_dir, "hostgaplugin/ext_conf.xml")))
+
+        with patch("azurelinuxagent.common.protocol.extensions_goal_state.add_event") as add_event_patcher:
+            ExtensionsGoalState.compare(from_extensions_config, from_vm_settings)
+
+            mismatch_events = [kw["message"] for _, kw in add_event_patcher.call_args_list if kw['op'] == WALAEventOperation.GoalStateMismatch]
+            self.assertTrue(len(mismatch_events) == 0, "Did not expect any GoalStateMismatch messages, got: {0}".format(mismatch_events))
+
+        with patch("azurelinuxagent.common.protocol.extensions_goal_state.add_event") as add_event_patcher:
+            from_vm_settings.required_features = ['FORCE_A_MISMATCH_FEATURE']
+
+            ExtensionsGoalState.compare(from_extensions_config, from_vm_settings)
+
+            mismatch_events = [kw["message"] for _, kw in add_event_patcher.call_args_list if kw['op'] == WALAEventOperation.GoalStateMismatch]
+            self.assertTrue(len(mismatch_events) == 1 and 'required_features' in mismatch_events[0],
+                "Expected 1 difference in RequiredFeatures, got: {0}".format(mismatch_events))
diff --git a/tests/protocol/test_goal_state.py b/tests/protocol/test_goal_state.py
index d2568145..492fddcc 100644
--- a/tests/protocol/test_goal_state.py
+++ b/tests/protocol/test_goal_state.py
@@ -1,26 +1,15 @@
 # Copyright (c) Microsoft Corporation. All rights reserved.
 # Licensed under the Apache License.
 
-import json
-import os
-
-from azurelinuxagent.common.future import httpclient
-from azurelinuxagent.common import conf
 from azurelinuxagent.common.exception import IncompleteGoalStateError
-from azurelinuxagent.common.protocol.extensions_goal_state import ExtensionsGoalState
 from azurelinuxagent.common.protocol.goal_state import GoalState, _NUM_GS_FETCH_RETRIES
-from azurelinuxagent.common.protocol import hostplugin
-from azurelinuxagent.common.utils import restutil
 from tests.protocol.mocks import mock_wire_protocol
 from tests.protocol import mockwiredata
-from tests.protocol.mocks import HttpRequestPredicates, MockHttpResponse
 from tests.tools import AgentTestCase, patch
 
-_original_http_request = restutil.http_request
 
-@patch("azurelinuxagent.common.protocol.wire.conf.get_enable_fast_track", return_value=True)
-class GoalStateTestCase(HttpRequestPredicates, AgentTestCase):
-    def test_fetch_goal_state_should_raise_on_incomplete_goal_state(self, _):
+class GoalStateTestCase(AgentTestCase):
+    def test_fetch_goal_state_should_raise_on_incomplete_goal_state(self):
         with mock_wire_protocol(mockwiredata.DATA_FILE) as protocol:
             protocol.mock_wire_data.data_files = mockwiredata.DATA_FILE_NOOP_GS
             protocol.mock_wire_data.reload()
@@ -31,71 +20,3 @@ class GoalStateTestCase(HttpRequestPredicates, AgentTestCase):
                     GoalState(protocol.client)
                 self.assertEqual(_NUM_GS_FETCH_RETRIES, mock_sleep.call_count, "Unexpected number of retries")
 
-    def test_update_goal_state_should_save_goal_state(self, _):
-        with mock_wire_protocol(mockwiredata.DATA_FILE_VM_SETTINGS_PROTECTED_SETTINGS) as protocol:
-            protocol.mock_wire_data.set_incarnation(999)
-            protocol.mock_wire_data.set_etag(888)
-            protocol.update_goal_state()
-
-        extensions_config_file = os.path.join(conf.get_lib_dir(), "ExtensionsConfig.999.xml")
-        vm_settings_file = os.path.join(conf.get_lib_dir(), "VmSettings.888.json")
-        expected_files = [
-            os.path.join(conf.get_lib_dir(), "GoalState.999.xml"),
-            os.path.join(conf.get_lib_dir(), "SharedConfig.xml"),
-            os.path.join(conf.get_lib_dir(), "Certificates.xml"),
-            os.path.join(conf.get_lib_dir(), "HostingEnvironmentConfig.xml"),
-            extensions_config_file,
-            vm_settings_file
-        ]
-
-        for f in expected_files:
-            self.assertTrue(os.path.exists(f), "{0} was not saved".format(f))
-
-        with open(extensions_config_file, "r") as file_:
-            extensions_goal_state = ExtensionsGoalState.from_extensions_config(file_.read())
-        self.assertEqual(4, len(extensions_goal_state.ext_handlers.extHandlers), "Expected 4 extensions in the test ExtensionsConfig")
-        for e in extensions_goal_state.ext_handlers.extHandlers:
-            self.assertEqual(e.properties.extensions[0].protectedSettings, "*** REDACTED ***", "The protected settings for {0} were not redacted".format(e.name))
-
-        # TODO: Use azurelinuxagent.common.protocol.ExtensionsGoalState once it implements parsing
-        with open(vm_settings_file, "r") as file_:
-            vm_settings = json.load(file_)
-        extensions = vm_settings["extensionGoalStates"]
-        self.assertEqual(4, len(extensions), "Expected 4 extensions in the test vmSettings")
-        for e in extensions:
-            self.assertEqual(e["settings"][0]["protectedSettings"], "*** REDACTED ***", "The protected settings for {0} were not redacted".format(e["name"]))
-
-    def test_update_vm_settings_should_should_retry_on_resource_gone_error(self, _):
-        """
-        Requests to the hostgaplugin incude the Container ID and the RoleConfigName as headers; when the hostgaplugin returns GONE (HTTP status 410) the agent
-        needs to get a new goal state and retry the request with updated values for the Container ID and RoleConfigName headers.
-        """
-        with mock_wire_protocol(mockwiredata.DATA_FILE_VM_SETTINGS) as protocol:
-            # The mock_wire_protocol mocks azurelinuxagent.common.utils.restutil.http_request, but the GONE status is handled at a lower level, in the internal _http_request.
-            # To mock this, we bypass the mock http_request by calling the original http_request (function http_get_handler) and then mocking the internal _http_request (function
-            # http_get_vm_settings).
-            def http_get_handler(url, *_, **kwargs):
-                if self.is_host_plugin_vm_settings_request(url):
-                    return _original_http_request("GET", url, None, **kwargs)
-                return None
-            protocol.set_http_handlers(http_get_handler=http_get_handler)
-
-            request_headers = []  # we expect a retry with new headers and use this array to persist the headers of each request
-            def http_get_vm_settings(_method, _host, _relative_url, **kwargs):
-                request_headers.append(kwargs["headers"])
-                if len(request_headers) == 1:
-                    # Fail the first request with status GONE and update the mock data to return the new Container ID and RoleConfigName that should be
-                    # used in the headers of the retry request.
-                    protocol.mock_wire_data.set_container_id("GET_VM_SETTINGS_TEST_CONTAINER_ID")
-                    protocol.mock_wire_data.set_role_config_name("GET_VM_SETTINGS_TEST_ROLE_CONFIG_NAME")
-                    return MockHttpResponse(status=httpclient.GONE)
-                # For this test we are interested only on the retry logic, so the second request (the retry) is not important; we use NOT_MODIFIED (304) for simplicity.
-                return MockHttpResponse(status=httpclient.NOT_MODIFIED)
-
-            with patch("azurelinuxagent.common.utils.restutil._http_request", side_effect=http_get_vm_settings):
-                protocol.client._update_vm_settings(force_update=False)
-
-            self.assertEqual(2, len(request_headers), "We expected 2 requests for vmSettings: the original request and the retry request")
-            self.assertEqual("GET_VM_SETTINGS_TEST_CONTAINER_ID", request_headers[1][hostplugin._HEADER_CONTAINER_ID], "The retry request did not include the expected header for the ContainerId")
-            self.assertEqual("GET_VM_SETTINGS_TEST_ROLE_CONFIG_NAME", request_headers[1][hostplugin._HEADER_HOST_CONFIG_NAME], "The retry request did not include the expected header for the RoleConfigName")
-
diff --git a/tests/protocol/test_wire.py b/tests/protocol/test_wire.py
index ac6bcdf9..32350e84 100644
--- a/tests/protocol/test_wire.py
+++ b/tests/protocol/test_wire.py
@@ -1204,6 +1204,42 @@ class UpdateGoalStateTestCase(AgentTestCase):
                     len(protected_settings),
                     "Could not find the expected number of redacted settings. Expected {0}.\n{1}".format(len(protected_settings), extensions_config))
 
+    def test_update_goal_state_should_save_goal_state(self):
+        # Enable FastTrack to also save the vmSettings
+        with patch("azurelinuxagent.common.protocol.wire.conf.get_enable_fast_track", return_value=True):
+            with mock_wire_protocol(mockwiredata.DATA_FILE_VM_SETTINGS_PROTECTED_SETTINGS) as protocol:
+                protocol.mock_wire_data.set_incarnation(999)
+                protocol.mock_wire_data.set_etag(888)
+                protocol.update_goal_state()
+
+            extensions_config_file = os.path.join(conf.get_lib_dir(), "ExtensionsConfig.999.xml")
+            vm_settings_file = os.path.join(conf.get_lib_dir(), "VmSettings.888.json")
+            expected_files = [
+                os.path.join(conf.get_lib_dir(), "GoalState.999.xml"),
+                os.path.join(conf.get_lib_dir(), "SharedConfig.xml"),
+                os.path.join(conf.get_lib_dir(), "Certificates.xml"),
+                os.path.join(conf.get_lib_dir(), "HostingEnvironmentConfig.xml"),
+                extensions_config_file,
+                vm_settings_file
+            ]
+
+            for f in expected_files:
+                self.assertTrue(os.path.exists(f), "{0} was not saved".format(f))
+
+            with open(extensions_config_file, "r") as file_:
+                extensions_goal_state = ExtensionsGoalState.create_from_extensions_config(123, file_.read())
+            self.assertEqual(4, len(extensions_goal_state.ext_handlers.extHandlers), "Expected 4 extensions in the test ExtensionsConfig")
+            for e in extensions_goal_state.ext_handlers.extHandlers:
+                self.assertEqual(e.properties.extensions[0].protectedSettings, "*** REDACTED ***", "The protected settings for {0} were not redacted".format(e.name))
+
+            # TODO: Use azurelinuxagent.common.protocol.ExtensionsGoalState once it implements parsing
+            with open(vm_settings_file, "r") as file_:
+                vm_settings = json.load(file_)
+            extensions = vm_settings["extensionGoalStates"]
+            self.assertEqual(4, len(extensions), "Expected 4 extensions in the test vmSettings")
+            for e in extensions:
+                self.assertEqual(e["settings"][0]["protectedSettings"], "*** REDACTED ***", "The protected settings for {0} were not redacted".format(e["name"]))
+
 
 class UpdateHostPluginFromGoalStateTestCase(AgentTestCase):
     """