Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Block extensions disallowed by policy #3259

Merged
merged 43 commits into from
Jan 22, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
c2cc2c6
Block disallowed extension processing
mgunnala Nov 8, 2024
151081d
Enable policy e2e tests
mgunnala Nov 8, 2024
edec2af
Pylint
mgunnala Nov 8, 2024
a37508f
Fix e2e test failures
mgunnala Nov 11, 2024
b0da554
Address review comments
mgunnala Nov 18, 2024
a4f5cab
Merge branch 'develop' into allowlist_2
mgunnala Nov 18, 2024
699b9ba
Address review comments
mgunnala Nov 20, 2024
86de0c5
Address test review comments
mgunnala Nov 21, 2024
c3e9b89
Remove status file for single-config
mgunnala Nov 22, 2024
65d7034
Add back status file for single-config
mgunnala Nov 22, 2024
95f247a
Run e2e tests on all endorsed
mgunnala Nov 22, 2024
3b18519
Fix UT failures
mgunnala Nov 23, 2024
63da127
Pylint
mgunnala Nov 26, 2024
471cd59
Merge branch 'develop' into allowlist_2
narrieta Nov 26, 2024
8ea989b
Address review comments for agent code
mgunnala Dec 3, 2024
83f6ff0
Tests
mgunnala Dec 3, 2024
b037e41
Revert "Tests"
mgunnala Dec 3, 2024
ba3869c
Address test comments
mgunnala Dec 6, 2024
dfcc158
Address test comments
mgunnala Dec 9, 2024
fe07ffa
Merge branch 'develop' into allowlist_2
mgunnala Dec 9, 2024
a31bdcf
Address test comments
mgunnala Dec 10, 2024
5198cf8
Cleanup existing extensions on test VMs
mgunnala Dec 12, 2024
4a0a4ef
Address comments and disable dependencies e2e tests
mgunnala Dec 16, 2024
daa8017
Merge branch 'develop' into allowlist_2
mgunnala Dec 16, 2024
bacc425
Add fixes for e2e tests
mgunnala Dec 17, 2024
3319916
Add back delete failure test case
mgunnala Dec 17, 2024
8c31798
Address comments round 3
mgunnala Dec 17, 2024
32ef5c1
Address comments
mgunnala Dec 17, 2024
f0895b7
Merge branch 'develop' into allowlist_2
mgunnala Dec 17, 2024
0c9f1c7
Pylint
mgunnala Dec 18, 2024
fc2de23
Merge branch 'develop' into allowlist_2
mgunnala Jan 13, 2025
c3aac0f
Report status for single-config ext
mgunnala Jan 13, 2025
be42640
Small e2e test cleanups
mgunnala Jan 13, 2025
7069a0b
Address agent code comments
mgunnala Jan 15, 2025
8231234
Merge branch 'develop' into allowlist_2
mgunnala Jan 15, 2025
ce5cf20
Address test comments
mgunnala Jan 17, 2025
e222aa7
Merge branch 'develop' into allowlist_2
mgunnala Jan 17, 2025
5abbfc6
Small formatting fix
mgunnala Jan 17, 2025
6abd3f8
Merge branch 'develop' into allowlist_2
narrieta Jan 18, 2025
9fa38ea
Fix comments
mgunnala Jan 21, 2025
7ebbd2b
Merge branch 'develop' into allowlist_2
mgunnala Jan 21, 2025
1e6a82d
Merge branch 'develop' into allowlist_2
mgunnala Jan 21, 2025
8cd64d4
Address review comments
mgunnala Jan 22, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 31 additions & 33 deletions azurelinuxagent/ga/exthandlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,12 @@
# This is the default sequence number we use when there are no settings available for Handlers
_DEFAULT_SEQ_NO = "0"

# For policy-related errors, this mapping is used to generate user-friendly error messages and determine the appropriate
# terminal error code based on the blocked operation.
# For extension disallowed errors (e.g. blocked by policy, extensions disabled), this mapping is used to generate
# user-friendly error messages and determine the appropriate terminal error code based on the blocked operation.
# Format: {<ExtensionRequestedState>: (<str>, <ExtensionErrorCodes>)}
# - The first element of the tuple is a user-friendly operation name included in error messages.
# - The second element of the tuple is the CRP terminal error code for the operation.
_POLICY_ERROR_MAP = \
_EXT_DISALLOWED_ERROR_MAP = \
{
ExtensionRequestedState.Enabled: ('run', ExtensionErrorCodes.PluginEnableProcessingFailed),
# Note: currently, when uninstall is requested for an extension, CRP polls until the agent does not
Expand Down Expand Up @@ -297,8 +297,9 @@ class ExtHandlersHandler(object):
def __init__(self, protocol):
self.protocol = protocol
self.ext_handlers = None
# Maintain a list of extensions that are disallowed, and always report extension status for disallowed extensions.
self.disallowed_ext_handlers = []
# Maintain a list of extension handler objects that are disallowed (e.g. blocked by policy, extensions disabled, etc.).
# Extension status is always reported for the extensions in this list. List is reset for each goal state.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I think this comment should indicate that extension status is always reported for the extensions in the list if an extension status exists (i.e. it's not a no-config ext)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated this wording to

Extension status, if it exists, is always reported for the extensions in this list.

self.__disallowed_ext_handlers = []
# The GoalState Aggregate status needs to report the last status of the GoalState. Since we only process
# extensions on goal state change, we need to maintain its state.
# Setting the status to None here. This would be overridden as soon as the first GoalState is processed
Expand Down Expand Up @@ -513,39 +514,35 @@ def handle_ext_handlers(self, goal_state_id):
except Exception as ex:
policy_error = ex

self.__disallowed_ext_handlers = []

for extension, ext_handler in all_extensions:

handler_i = ExtHandlerInstance(ext_handler, self.protocol, extension=extension)

# Get user-friendly operation name and terminal error code to use in status messages if extension is disallowed
operation, error_code = _EXT_DISALLOWED_ERROR_MAP.get(ext_handler.state)

# 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
# here with an error message.
if not extensions_enabled:
self.disallowed_ext_handlers.append(ext_handler)
agent_conf_file_path = get_osutil().agent_conf_file_path
msg = "Extension will not be processed since extension processing is disabled. To enable extension " \
"processing, set Extensions.Enabled=y in '{0}'".format(agent_conf_file_path)
ext_full_name = handler_i.get_extension_full_name(extension)
logger.info('')
logger.info("{0}: {1}".format(ext_full_name, msg))
add_event(op=WALAEventOperation.ExtensionProcessing, message="{0}: {1}".format(ext_full_name, msg))
handler_i.set_handler_status(status=ExtHandlerStatusValue.not_ready, message=msg, code=-1)
handler_i.create_status_file(extension,
status=ExtensionStatusValue.error,
code=-1,
operation=handler_i.operation,
message=msg,
overwrite=False)
agent_conf_file_path = get_osutil().agent_conf_file_path
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: should probably use get_agent_conf_file_path() instead

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

msg = "Extension '{0}' will not be processed since extension processing is disabled. To enable extension " \
"processing, set Extensions.Enabled=y in '{1}'".format(ext_full_name, agent_conf_file_path)
# logger.info(msg)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left-over?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was removed in commit ce5cf20

self.__handle_ext_disallowed_error(handler_i, error_code, report_op=WALAEventOperation.ExtensionProcessing,
message=msg, extension=extension)
continue

# If an error was thrown during policy engine initialization, skip further processing of the extension.
# CRP is still waiting for status, so we report error status here.
operation, error_code = _POLICY_ERROR_MAP.get(ext_handler.state)
if policy_error is not None:
msg = "Extension will not be processed: {0}".format(ustr(policy_error))
self.__report_policy_error(ext_handler_i=handler_i, error_code=error_code,
report_op=WALAEventOperation.ExtensionPolicy, message=msg,
extension=extension)
self.__handle_ext_disallowed_error(ext_handler_i=handler_i, error_code=error_code,
report_op=WALAEventOperation.ExtensionPolicy, message=msg,
extension=extension)
continue

# In case of depends-on errors, we skip processing extensions if there was an error processing dependent extensions.
Expand Down Expand Up @@ -579,8 +576,8 @@ def handle_ext_handlers(self, goal_state_id):
"Extension will not be processed: failed to {0} extension '{1}' because it is not specified "
"as an allowed extension. To {0}, add the extension to the list of allowed extensions in the policy file ('{2}')."
).format(operation, ext_handler.name, conf.get_policy_file_path())
self.__report_policy_error(handler_i, error_code, report_op=WALAEventOperation.ExtensionPolicy,
message=msg, extension=extension)
self.__handle_ext_disallowed_error(handler_i, error_code, report_op=WALAEventOperation.ExtensionPolicy,
message=msg, extension=extension)
extension_success = False
else:
extension_success = self.handle_ext_handler(handler_i, extension, goal_state_id)
Expand Down Expand Up @@ -748,27 +745,28 @@ 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)

def __report_policy_error(self, ext_handler_i, error_code, report_op, message, extension):
# TODO: __handle_and_report_ext_handler_errors() does not create a status file for single-config extensions, this
# function was created as a temporary workaround. Consider merging the two functions function after assessing its impact.

def __handle_ext_disallowed_error(self, ext_handler_i, error_code, report_op, message, extension):
# Report error for disallowed extensions (extensions blocked by policy or extensions disabled via config).
# If extension status exists, CRP ignores handler status and reports extension status. In the case of policy errors,
# we write a .status file to force CRP to fail fast - the agent will otherwise report a transitioning status.
# - For extensions without settings or uninstall errors: report at the handler level.
# - For extensions with settings (install/enable errors): report at both handler and extension levels.
#
# TODO: __handle_and_report_ext_handler_errors() does not create a status file for single-config extensions, this
# function was created as a temporary workaround. Consider merging the two functions function after assessing the impact.

# Keep a list of disallowed extensions so that report_ext_handler_status() can report status for them.
self.disallowed_ext_handlers.append(ext_handler_i.ext_handler)
self.__disallowed_ext_handlers.append(ext_handler_i.ext_handler)

# Set handler status for all extensions (with and without settings).
ext_handler_i.set_handler_status(message=message, code=error_code)
ext_handler_i.set_handler_status(status=ExtHandlerStatusValue.not_ready, message=message, code=error_code)

# For extensions with settings (install/enable errors), also update extension-level status.
# Overwrite any existing status file to reflect policy failures accurately.
if extension is not None and ext_handler_i.ext_handler.state == ExtensionRequestedState.Enabled:
# TODO: if extension is reporting heartbeat, it overwrites status. Consider overwriting heartbeat here
ext_handler_i.create_status_file(extension, status=ExtensionStatusValue.error, code=error_code,
operation=report_op, message=message, overwrite=True)
operation=ext_handler_i.operation, message=message, overwrite=True)

name = ext_handler_i.get_extension_full_name(extension)
handler_version = ext_handler_i.ext_handler.version
Expand Down Expand Up @@ -1068,7 +1066,7 @@ def report_ext_handler_status(self, vm_status, ext_handler, goal_state_changed):

handler_state = ext_handler_i.get_handler_state()
ext_handler_statuses = []
ext_disallowed = ext_handler in self.disallowed_ext_handlers
ext_disallowed = ext_handler in self.__disallowed_ext_handlers
# For MultiConfig, we need to report status per extension even for Handler level failures.
# If we have HandlerStatus for a MultiConfig handler and GS is requesting for it, we would report status per
# extension even if HandlerState == NotInstalled (Sample scenario: ExtensionsGoalStateError, DecideVersionError, etc)
Expand Down
8 changes: 4 additions & 4 deletions tests/ga/test_extension.py
Original file line number Diff line number Diff line change
Expand Up @@ -1652,13 +1652,13 @@ def test_extensions_disabled(self, _, *args):
vm_status = args[0]
self.assertEqual(1, len(vm_status.vmAgent.extensionHandlers))
exthandler = vm_status.vmAgent.extensionHandlers[0]
self.assertEqual(-1, exthandler.code)
self.assertEqual(ExtensionErrorCodes.PluginEnableProcessingFailed, exthandler.code)
self.assertEqual('NotReady', exthandler.status)
self.assertEqual("Extension will not be processed since extension processing is disabled. To enable extension processing, set Extensions.Enabled=y in '/etc/waagent.conf'", exthandler.message)
self.assertEqual("Extension 'OSTCExtensions.ExampleHandlerLinux' will not be processed since extension processing is disabled. To enable extension processing, set Extensions.Enabled=y in '/etc/waagent.conf'", exthandler.message)
ext_status = exthandler.extension_status
self.assertEqual(-1, ext_status.code)
self.assertEqual(ExtensionErrorCodes.PluginEnableProcessingFailed, ext_status.code)
self.assertEqual('error', ext_status.status)
self.assertEqual("Extension will not be processed since extension processing is disabled. To enable extension processing, set Extensions.Enabled=y in '/etc/waagent.conf'", ext_status.message)
self.assertEqual("Extension 'OSTCExtensions.ExampleHandlerLinux' will not be processed since extension processing is disabled. To enable extension processing, set Extensions.Enabled=y in '/etc/waagent.conf'", ext_status.message)

def test_extensions_deleted(self, *args):
# Ensure initial enable is successful
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ def run(self):
assert_that("VMExtensionProvisioningError" in str(error)) \
.described_as(f"Expected a VMExtensionProvisioningError error, but actual error was: {error}") \
.is_true()
assert_that("Extension will not be processed since extension processing is disabled" in str(error)) \
assert_that("will not be processed since extension processing is disabled" in str(error)) \
.described_as(
f"Error message should communicate that extension will not be processed, but actual error "
f"was: {error}").is_true()
Expand Down