Adding support for constrained extensions - #3668
maddieford wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds “constrained extensions” runtime policy support: it reads extension-specific runtimePolicy from /etc/waagent_policy.json (when policy enforcement is enabled) and synchronizes a per-extension waagent_runtime_policy.json in the extension’s config directory, gated by a new supportsPolicy manifest flag. It also adds unit and E2E coverage for the new behavior.
Changes:
- Add runtime policy retrieval to the policy engine and runtime policy file synchronization in
ExtHandlersHandler. - Extend handler manifest parsing/validation with
supportsPolicy(boolean-only) and update tests/fixtures accordingly. - Add a new E2E suite validating runtime policy behavior with supported and unsupported CSE versions.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
azurelinuxagent/ga/policy/policy_engine.py |
Adds ExtensionRuntimePolicyError and get_extension_runtime_policy() for retrieving per-extension runtime policy. |
azurelinuxagent/ga/exthandlers.py |
Synchronizes runtime policy file before enable/install; adds runtime policy file helpers on ExtHandlerInstance; adds supportsPolicy parsing and validation. |
tests/ga/test_policy_engine.py |
Adds unit tests for get_extension_runtime_policy() and case-insensitive extension name handling. |
tests/ga/test_exthandlers_exthandlerinstance.py |
Adds unit tests for runtime policy file write/remove error handling. |
tests/ga/test_extension.py |
Adds integration-style tests ensuring runtime policy file is created/updated/removed across enable cycles and supportsPolicy scenarios. |
tests/data/ext/handler_manifest/valid_manifest.json |
Adds supportsPolicy to valid manifest fixture. |
tests/data/ext/handler_manifest/manifest_boolean_fields_strings.json |
Adds supportsPolicy as a string to validate it’s treated as invalid/false. |
tests/data/ext/handler_manifest/manifest_boolean_fields_invalid.json |
Adds supportsPolicy invalid value fixture for invalid-boolean reporting. |
tests/data/ext/handler_manifest/manifest_boolean_fields_false.json |
Adds supportsPolicy false-string fixture. |
tests_e2e/tests/ext_runtime_policy/ext_runtime_policy.py |
New E2E test validating runtime policy delivery/update/removal and unsupported-extension blocking. |
tests_e2e/test_suites/ext_runtime_policy.yml |
Registers the new runtime policy E2E suite (canary-only location, cloud skips). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.
Suppressed comments (3)
Previously missed (2) — in code that hasn't changed since the last review.
tests/ga/test_policy_engine.py:579
- Docstring has an extra space in
get_extension_runtime_policy (), which reads like a typo and makes the method name harder to search for.
"""
Extension name is allowed to be any case. Test that should_allow(), should_enforce_signature_validation(), and
get_extension_runtime_policy () return expected results, even when the extension name does not match the case
of the name specified in policy.
"""
azurelinuxagent/ga/exthandlers.py:2549
- PR description says the agent writes
runtime_policy.json, but the implementation writeswaagent_runtime_policy.json. Please confirm the intended filename and align the PR description (or the implementation) so extension authors know what to read.
def get_runtime_policy_file(self):
return os.path.join(self.get_conf_dir(), 'waagent_runtime_policy.json')
azurelinuxagent/ga/policy/policy_engine.py:489
- The runtime-policy error message references the JSON path as "{extension}.runtimePolicy", but in the policy schema runtimePolicy is nested under extensionPolicies.extensions[]. This can confuse customers trying to remediate the issue; consider pointing to the correct path/property name.
raise ExtensionRuntimePolicyError(
"Runtime policy is specified for extension '{0}', but this extension does not support policy enforcement. "
"To continue enabling the extension without runtime policy, remove the entry '{0}.runtimePolicy' "
"from the policy file ({1}).".format(extension_name, conf.get_policy_file_path()))
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.
Suppressed comments (3)
Previously missed (1) — in code that hasn't changed since the last review.
azurelinuxagent/ga/exthandlers.py:2549
- PR description says the agent writes
runtime_policy.json, but the implementation writeswaagent_runtime_policy.json. Please align the external-facing name (either update the PR/docs/spec or rename the file) so extension authors know what to read.
def get_runtime_policy_file(self):
return os.path.join(self.get_conf_dir(), 'waagent_runtime_policy.json')
tests/ga/test_policy_engine.py:578
- Docstring has an extra space in the function name (
get_extension_runtime_policy ()), which makes the documentation inconsistent with the actual method name and harder to search/grep.
Extension name is allowed to be any case. Test that should_allow(), should_enforce_signature_validation(), and
get_extension_runtime_policy () return expected results, even when the extension name does not match the case
of the name specified in policy.
tests_e2e/test_suites/ext_runtime_policy.yml:10
- The inline comment has a typo in the Azure region name (
centraluseaupvscentraluseuap), which can be confusing when troubleshooting why the suite is pinned to that location.
locations: "AzureCloud:centraluseuap" # Test CSE 2.8.0 is currently available only in centraluseaup.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #3668 +/- ##
===========================================
+ Coverage 71.97% 73.73% +1.75%
===========================================
Files 103 122 +19
Lines 15692 18730 +3038
Branches 2486 2517 +31
===========================================
+ Hits 11295 13810 +2515
- Misses 3881 4317 +436
- Partials 516 603 +87 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ebf6ebe to
50f8c60
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.
Suppressed comments (3)
Previously missed (3) — in code that hasn't changed since the last review.
azurelinuxagent/ga/exthandlers.py:2549
- The PR description says the agent creates a file named
runtime_policy.json, but the implementation writeswaagent_runtime_policy.json. Ifruntime_policy.jsonis the intended public contract for extensions, the agent should write that filename; otherwise the PR description (and any related docs) should be updated to matchwaagent_runtime_policy.jsonto avoid integration confusion.
def get_runtime_policy_file(self):
return os.path.join(self.get_conf_dir(), 'waagent_runtime_policy.json')
azurelinuxagent/ga/exthandlers.py:2562
- update_runtime_policy() wraps the underlying IOError but drops it as an inner exception. Passing the original exception as
inneris consistent with other error paths in this file and preserves better diagnostics (the message formatting already supports an Inner error).
try:
fileutil.write_file(runtime_policy_file, json.dumps(runtime_policy))
except IOError as e:
raise ExtensionRuntimePolicyError(
"Failed to save extension runtime policy file: {0}. Error: {1}".format(runtime_policy_file, e))
azurelinuxagent/ga/exthandlers.py:2571
- remove_runtime_policy() similarly formats the OSError into the message but does not pass the original exception as an inner error. Passing it through improves troubleshooting and keeps error wrapping consistent with other ExtensionError/AgentError usage.
fileutil.rm_files(runtime_policy_file)
except OSError as e:
if not is_file_not_found_error(e):
raise ExtensionRuntimePolicyError(
"Failed to remove extension runtime policy file: {0}. Error: {1}".format(runtime_policy_file, e))
| # If an extension runtime policy exists, but the extension does not support policy, an exception will be raised | ||
| # and caught in handle_ext_handler to prevent the extension from being processed and report the appropriate | ||
| # status for the extension. This ensures that the policy is not silently ignored. |
| def get_runtime_policy_file(self): | ||
| return os.path.join(self.get_conf_dir(), 'waagent_runtime_policy.json') | ||
|
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
azurelinuxagent/ga/exthandlers.py:2562
- update_runtime_policy() writes waagent_runtime_policy.json without ensuring the extension's config directory exists. Unlike other writes in this class (e.g., HandlerStatus/state files), this can raise IOError if the handler's config dir hasn't been created yet, causing the extension to fail for an avoidable filesystem precondition.
def update_runtime_policy(self, runtime_policy):
runtime_policy_file = self.get_runtime_policy_file()
try:
fileutil.write_file(runtime_policy_file, json.dumps(runtime_policy))
except IOError as e:
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
tests_e2e/tests/ext_runtime_policy/ext_runtime_policy.py:97
- In this E2E test, setting auto_upgrade_minor_version=True can cause CSE to upgrade to a newer minor version between enables. Since the runtime-policy file path is derived from the installed type_handler_version later in the test, a minor upgrade can make the path stale and make these assertions flaky. Consider disabling minor auto-upgrades here (or recomputing the runtime_policy_file_path after each enable).
"auto_upgrade_minor_version": True,
Description
This PR adds support for the constrained extensions feature. The constrained extensions feature allows users to specify extension specific runtime policy via the existing /etc/waagent_policy.json configuration.
Each extension defines their own runtime policy schema and declares support for policy in its manifest. At each enable, the agent creates the runtime_policy.json for the extension in its config directory if the extension supports policy. The contents of the runtime_policy.json are the extension specific configuration provided by the customer in waagent_policy.json.
The extension is expected to pick up the policy from its config directory and apply it.
This PR introduces a new E2E test for the scenario which uses the CSE Test extension which supports policy.
Issue #
PR information
developbranch.Quality of Code and Contribution Guidelines
Distro maintenance information, if applicable