From b0da55496643f245485ac3c19bd2e3efef09b88c Mon Sep 17 00:00:00 2001 From: mgunnala Date: Mon, 18 Nov 2024 11:49:37 -0500 Subject: [PATCH] Address review comments --- azurelinuxagent/ga/exthandlers.py | 30 +++++++++--- azurelinuxagent/ga/policy/policy_engine.py | 5 +- tests/ga/test_extension.py | 2 +- tests_e2e/tests/ext_policy/ext_policy.py | 48 ++++++++++++------- .../ext_policy_with_dependencies.py | 4 +- .../ext_policy/policy_dependencies_cases.py | 14 +++--- 6 files changed, 67 insertions(+), 36 deletions(-) diff --git a/azurelinuxagent/ga/exthandlers.py b/azurelinuxagent/ga/exthandlers.py index 18b520357c..e7378ad631 100644 --- a/azurelinuxagent/ga/exthandlers.py +++ b/azurelinuxagent/ga/exthandlers.py @@ -510,9 +510,8 @@ def handle_ext_handlers(self, goal_state_id): policy_op, policy_err_code = policy_err_map.get(ext_handler.state) if policy_error is not None: err = ExtensionPolicyError(msg="", inner=policy_error, code=policy_err_code) - self.__handle_and_report_ext_handler_errors(handler_i, err, - report_op=handler_i.operation, - message=ustr(err), extension=extension, report=True) + self.__handle_and_report_policy_error(handler_i, err, report_op=handler_i.operation, message=ustr(err), + extension=extension, report=True) continue extension_allowed = policy_engine.should_allow_extension(ext_handler.name) @@ -522,9 +521,8 @@ def handle_ext_handlers(self, goal_state_id): ext_handler.name, conf.get_policy_file_path()) err = ExtensionPolicyError(msg, code=policy_err_code) - self.__handle_and_report_ext_handler_errors(handler_i, err, - report_op=handler_i.operation, - message=ustr(err), extension=extension, report=True) + self.__handle_and_report_policy_error(handler_i, err, report_op=handler_i.operation, message=ustr(err), + extension=extension, report=True) # In case of extensions disabled, we skip processing extensions. But CRP is still waiting for some status # back for the skipped extensions. In order to propagate the status back to CRP, we will report status back @@ -736,6 +734,26 @@ def __handle_and_report_ext_handler_errors(ext_handler_i, error, report_op, mess add_event(name=name, version=handler_version, op=report_op, is_success=False, log_event=True, message=message) + @staticmethod + def __handle_and_report_policy_error(ext_handler_i, error, report_op, message, report=True, extension=None): + # TODO: Consider merging this function with __handle_and_report_ext_handler_errors() above. + + # Set handler status for all extensions + ext_handler_i.set_handler_status(message=message, code=error.code) + + # Create status file for only extensions with settings. Since extensions are not processed in the case of + # policy-related failures, no extension status file is created. For CRP to report status, we need to create the + # file with failure on behalf of the extension. This should be done for both multi-config and single-config extensions. + if extension is not None: + ext_handler_i.create_status_file_if_not_exist(extension, status=ExtensionStatusValue.error, code=error.code, + operation=report_op, message=message) + + if report: + name = ext_handler_i.get_extension_full_name(extension) + handler_version = ext_handler_i.ext_handler.version + add_event(name=name, version=handler_version, op=report_op, is_success=False, log_event=True, + message=message) + def handle_enable(self, ext_handler_i, extension): """ 1- Ensure the handler is installed diff --git a/azurelinuxagent/ga/policy/policy_engine.py b/azurelinuxagent/ga/policy/policy_engine.py index 11f3d9c45e..3a0add6f8c 100644 --- a/azurelinuxagent/ga/policy/policy_engine.py +++ b/azurelinuxagent/ga/policy/policy_engine.py @@ -49,9 +49,8 @@ class ExtensionPolicyError(ExtensionError): """ Error raised during agent extension policy enforcement. """ - # TODO: when CRP adds terminal error code for policy-related extension failures, set that as the default code. - def __init__(self, msg, inner=None, code=-1): - msg = "Extension is disallowed by agent policy and will not be processed: {0}".format(msg) + def __init__(self, msg, code, inner=None): + msg = "Extension will not be processed: {0}".format(msg) super(ExtensionPolicyError, self).__init__(msg, inner, code) diff --git a/tests/ga/test_extension.py b/tests/ga/test_extension.py index a4883add17..0423fd585b 100644 --- a/tests/ga/test_extension.py +++ b/tests/ga/test_extension.py @@ -3588,7 +3588,7 @@ def test_should_fail_extension_if_error_thrown_during_policy_engine_init(self): } with patch('azurelinuxagent.ga.policy.policy_engine.ExtensionPolicyEngine.__init__', side_effect=Exception("mock exception")): - expected_msg = "Extension is disallowed by agent policy and will not be processed: \nInner error: mock exception" + expected_msg = "Extension will not be processed: \nInner error: mock exception" self._test_policy_failure(policy=policy, op=ExtensionRequestedState.Enabled, expected_status_code=ExtensionErrorCodes.PluginEnableProcessingFailed, expected_handler_status='NotReady', expected_status_msg=expected_msg) diff --git a/tests_e2e/tests/ext_policy/ext_policy.py b/tests_e2e/tests/ext_policy/ext_policy.py index 2d1c509176..cce68d85b6 100644 --- a/tests_e2e/tests/ext_policy/ext_policy.py +++ b/tests_e2e/tests/ext_policy/ext_policy.py @@ -88,7 +88,7 @@ def _operation_should_fail(operation, extension_case): fail(f"The agent should have reported an error trying to {operation} {extension_case.extension.__str__()} " f"because the extension is disallowed by policy.") except Exception as error: - assert_that("Extension is disallowed by agent policy and will not be processed" in str(error)) \ + assert_that("Extension will not be processed" in str(error)) \ .described_as( f"Error message should communicate that extension is disallowed by policy, but actual error " f"was: {error}").is_true() @@ -109,11 +109,6 @@ def run(self): resource_name="RunCommandHandler"), {'source': {'script': f"echo '{unique}' > /tmp/{test_file}"}} ) - azure_monitor = ExtPolicy.TestCase( - VirtualMachineExtensionClient(self._context.vm, VmExtensionIds.AzureMonitorLinuxAgent, - resource_name="AzureMonitorLinuxAgent"), - None - ) unique2 = str(uuid.uuid4()) test_file2 = f"waagent-test.{unique2}" run_command_2 = ExtPolicy.TestCase( @@ -121,13 +116,18 @@ def run(self): resource_name="RunCommandHandler2"), {'source': {'script': f"echo '{unique2}' > /tmp/{test_file2}"}} ) + azure_monitor = ExtPolicy.TestCase( + VirtualMachineExtensionClient(self._context.vm, VmExtensionIds.AzureMonitorLinuxAgent, + resource_name="AzureMonitorLinuxAgent"), + None + ) # Enable policy via conf log.info("Enabling policy via conf file on the test VM [%s]", self._context.vm.name) self._ssh_client.run_command("update-waagent-conf Debug.EnableExtensionPolicy=y", use_sudo=True) - # Test case 1: should only enable allowlisted extensions - # CustomScript should be enabled, RunCommand and AzureMonitor should fail. + # Only allowlisted extensions should be processed. + # We only allowlist CustomScript: CustomScript should be enabled, RunCommand and AzureMonitor should fail. policy = \ { "policyVersion": "0.1.0", @@ -145,8 +145,8 @@ def run(self): if VmExtensionIds.AzureMonitorLinuxAgent.supports_distro((self._ssh_client.run_command("get_distro.py").rstrip())): self._operation_should_fail("enable", azure_monitor) - # Test case 2: turn allowlist off - # RunCommand should be successfully enabled and then deleted. + # When allowlist is turned off, all extensions should be processed. + # RunCommand and AzureMonitorLinuxAgent should be successfully enabled and then deleted. policy = \ { "policyVersion": "0.1.0", @@ -163,8 +163,8 @@ def run(self): self._operation_should_succeed("enable", azure_monitor) self._operation_should_succeed("delete", azure_monitor) - # Test case 3: uninstall should fail when disallowed - # Remove CustomScript from allowlist and try to uninstall, should fail. + # Should not uninstall disallowed extensions. + # CustomScript is removed from the allowlist: delete operation should fail. policy = \ { "policyVersion": "0.1.0", @@ -175,16 +175,30 @@ def run(self): } } self._create_policy_file(policy) - # Known CRP issue - delete/uninstall operation times out instead of reporting an error. # TODO: uncomment this test case after issue is resolved # self._operation_should_fail("delete", custom_script) - # Test case 4: both instances in a multiconfig extension should fail, if disallowed. - # Disallow RunCommand and try to install two instances, both should fail fast. + # If a multiconfig extension is disallowed, no instances should be processed. + # RunCommand is not allowed - if we try to enable two instances, both should fail fast. self._operation_should_fail("enable", run_command) self._operation_should_fail("enable", run_command_2) + # If single-config extension is initially blocked, and policy is updated to allow it, extension should be + # successfully enabled and report status correctly. + self._operation_should_fail("enable", custom_script) + policy = \ + { + "policyVersion": "0.1.0", + "extensionPolicies": { + "allowListedExtensionsOnly": False, + "signatureRequired": False, + "extensions": {} + } + } + self._create_policy_file(policy) + self._operation_should_succeed("enable", custom_script) + def get_ignore_error_rules(self) -> List[Dict[str, Any]]: ignore_rules = [ # @@ -198,10 +212,10 @@ def get_ignore_error_rules(self) -> List[Dict[str, Any]]: { 'message': r"Skipping processing of extensions since execution of dependent extension .* failed" }, - # 2024-10-24T17:34:20.808235Z ERROR ExtHandler ExtHandler Event: name=Microsoft.Azure.Monitor.AzureMonitorLinuxAgent, op=None, message=[ExtensionPolicyError] Extension is disallowed by agent policy and will not be processed: failed to enable extension 'Microsoft.Azure.Monitor.AzureMonitorLinuxAgent' because extension is not specified in allowlist. To enable, add extension to the allowed list in the policy file ('/etc/waagent_policy.json')., duration=0 + # 2024-10-24T17:34:20.808235Z ERROR ExtHandler ExtHandler Event: name=Microsoft.Azure.Monitor.AzureMonitorLinuxAgent, op=None, message=[ExtensionPolicyError] Extension will not be processed: failed to enable extension 'Microsoft.Azure.Monitor.AzureMonitorLinuxAgent' because extension is not specified in allowlist. To enable, add extension to the allowed list in the policy file ('/etc/waagent_policy.json')., duration=0 # We intentionally block extensions with policy and expect this failure message { - 'message': r"Extension is disallowed by agent policy and will not be processed" + 'message': r"Extension will not be processed" } ] return ignore_rules diff --git a/tests_e2e/tests/ext_policy/ext_policy_with_dependencies.py b/tests_e2e/tests/ext_policy/ext_policy_with_dependencies.py index 7882f31df9..403734b230 100644 --- a/tests_e2e/tests/ext_policy/ext_policy_with_dependencies.py +++ b/tests_e2e/tests/ext_policy/ext_policy_with_dependencies.py @@ -269,10 +269,10 @@ def get_ignore_error_rules(self) -> List[Dict[str, Any]]: { 'message': r"Skipping processing of extensions since execution of dependent extension .* failed" }, - # 2024-10-24T17:34:20.808235Z ERROR ExtHandler ExtHandler Event: name=Microsoft.Azure.Monitor.AzureMonitorLinuxAgent, op=None, message=[ExtensionPolicyError] Extension is disallowed by agent policy and will not be processed: failed to enable extension 'Microsoft.Azure.Monitor.AzureMonitorLinuxAgent' because extension is not specified in allowlist. To enable, add extension to the allowed list in the policy file ('/etc/waagent_policy.json')., duration=0 + # 2024-10-24T17:34:20.808235Z ERROR ExtHandler ExtHandler Event: name=Microsoft.Azure.Monitor.AzureMonitorLinuxAgent, op=None, message=[ExtensionPolicyError] Extension will not be processed: failed to enable extension 'Microsoft.Azure.Monitor.AzureMonitorLinuxAgent' because extension is not specified in allowlist. To enable, add extension to the allowed list in the policy file ('/etc/waagent_policy.json')., duration=0 # We intentionally block extensions with policy and expect this failure message { - 'message': r"Extension is disallowed by agent policy and will not be processed" + 'message': r"Extension will not be processed" } ] return ignore_rules diff --git a/tests_e2e/tests/ext_policy/policy_dependencies_cases.py b/tests_e2e/tests/ext_policy/policy_dependencies_cases.py index 1abc3234a1..6eabd67a51 100644 --- a/tests_e2e/tests/ext_policy/policy_dependencies_cases.py +++ b/tests_e2e/tests/ext_policy/policy_dependencies_cases.py @@ -56,7 +56,7 @@ def _should_fail_single_config_depends_on_disallowed_single_config(): } } expected_errors = [ - "Extension is disallowed by agent policy and will not be processed: failed to enable extension 'Microsoft.OSTCExtensions.VMAccessForLinux' because extension is not specified in allowlist", + "Extension will not be processed: failed to enable extension 'Microsoft.OSTCExtensions.VMAccessForLinux' because extension is not specified in allowlist", "'CustomScript' is marked as failed since it depends upon the VM Extension 'VMAccessForLinux' which has failed" ] deletion_order = [VmExtensionIds.CustomScript, VmExtensionIds.VmAccess] @@ -80,7 +80,7 @@ def _should_fail_single_config_depends_on_disallowed_no_config(): } } expected_errors = [ - "Extension is disallowed by agent policy and will not be processed: failed to enable extension 'Microsoft.Azure.Monitor.AzureMonitorLinuxAgent' because extension is not specified in allowlist", + "Extension will not be processed: failed to enable extension 'Microsoft.Azure.Monitor.AzureMonitorLinuxAgent' because extension is not specified in allowlist", "'CustomScript' is marked as failed since it depends upon the VM Extension 'AzureMonitorLinuxAgent' which has failed" ] deletion_order = [VmExtensionIds.CustomScript, VmExtensionIds.AzureMonitorLinuxAgent] @@ -103,7 +103,7 @@ def _should_fail_single_config_depends_on_disallowed_multi_config(): } } expected_errors = [ - "Extension is disallowed by agent policy and will not be processed: failed to enable extension 'Microsoft.CPlat.Core.RunCommandHandlerLinux' because extension is not specified in allowlist", + "Extension will not be processed: failed to enable extension 'Microsoft.CPlat.Core.RunCommandHandlerLinux' because extension is not specified in allowlist", "'CustomScript' is marked as failed since it depends upon the VM Extension 'RunCommandHandlerLinux' which has failed" ] deletion_order = [VmExtensionIds.CustomScript, VmExtensionIds.RunCommandHandler] @@ -126,7 +126,7 @@ def _should_fail_multi_config_depends_on_disallowed_single_config(): } } expected_errors = [ - "Extension is disallowed by agent policy and will not be processed: failed to enable extension 'Microsoft.Azure.Extensions.CustomScript' because extension is not specified in allowlist", + "Extension will not be processed: failed to enable extension 'Microsoft.Azure.Extensions.CustomScript' because extension is not specified in allowlist", "VM has reported a failure when processing extension 'RunCommandHandlerLinux' (publisher 'Microsoft.CPlat.Core' and type 'RunCommandHandlerLinux'). Error message: 'Skipping processing of extensions since execution of dependent extension Microsoft.Azure.Extensions.CustomScript failed'." ] deletion_order = [VmExtensionIds.RunCommandHandler, VmExtensionIds.CustomScript] @@ -149,7 +149,7 @@ def _should_fail_multi_config_depends_on_disallowed_no_config(): } } expected_errors = [ - "Extension is disallowed by agent policy and will not be processed: failed to enable extension 'Microsoft.Azure.Monitor.AzureMonitorLinuxAgent' because extension is not specified in allowlist", + "Extension will not be processed: failed to enable extension 'Microsoft.Azure.Monitor.AzureMonitorLinuxAgent' because extension is not specified in allowlist", "VM has reported a failure when processing extension 'RunCommandHandlerLinux' (publisher 'Microsoft.CPlat.Core' and type 'RunCommandHandlerLinux'). Error message: 'Skipping processing of extensions since execution of dependent extension Microsoft.Azure.Monitor.AzureMonitorLinuxAgent failed'." ] deletion_order = [VmExtensionIds.RunCommandHandler, VmExtensionIds.AzureMonitorLinuxAgent] @@ -236,8 +236,8 @@ def _should_no_dependencies(): } } expected_errors = [ - "Extension is disallowed by agent policy and will not be processed: failed to enable extension 'Microsoft.OSTCExtensions.VMAccessForLinux' because extension is not specified in allowlist", - "Extension is disallowed by agent policy and will not be processed: failed to enable extension 'Microsoft.Azure.Extensions.CustomScript' because extension is not specified in allowlist" + "Extension will not be processed: failed to enable extension 'Microsoft.OSTCExtensions.VMAccessForLinux' because extension is not specified in allowlist", + "Extension will not be processed: failed to enable extension 'Microsoft.Azure.Extensions.CustomScript' because extension is not specified in allowlist" ] deletion_order = [VmExtensionIds.VmAccess, VmExtensionIds.CustomScript] return policy, template, expected_errors, deletion_order \ No newline at end of file