Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
mgunnala committed Nov 18, 2024
1 parent a37508f commit b0da554
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 36 deletions.
30 changes: 24 additions & 6 deletions azurelinuxagent/ga/exthandlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
5 changes: 2 additions & 3 deletions azurelinuxagent/ga/policy/policy_engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)


Expand Down
2 changes: 1 addition & 1 deletion tests/ga/test_extension.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
48 changes: 31 additions & 17 deletions tests_e2e/tests/ext_policy/ext_policy.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -109,25 +109,25 @@ 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(
VirtualMachineExtensionClient(self._context.vm, VmExtensionIds.RunCommandHandler,
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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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 = [
#
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions tests_e2e/tests/ext_policy/ext_policy_with_dependencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 7 additions & 7 deletions tests_e2e/tests/ext_policy/policy_dependencies_cases.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand Down Expand Up @@ -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

0 comments on commit b0da554

Please sign in to comment.