Repository navigation
[AKS] az aks create/update: Add --kms-infrastructure-encryption for KMS platform-managed keys (PMK) - #34204
Merged
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Member
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
Bin Xia (bingosummer)
force-pushed
the
binxi/kms-pmk
branch
from
October 9, 2026 08:45
5bdabed to
16da9f4
Compare
Bin Xia (bingosummer)
marked this pull request as ready for review
October 9, 2026 08:45
Bin Xia (bingosummer)
requested review from
a team and
FumingZhang
as code owners
October 9, 2026 08:45
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Key and resource ID validation currently accepts malformed or nested resource identifiers.
1 open finding
What changed in this PR
Adds GA support for AKS KMS platform-managed key infrastructure encryption.
Changes:
- Adds
--kms-infrastructure-encryptionto AKS create/update. - Updates KMS model handling and Key Vault validation.
- Adds unit and live scenario coverage.
| File | Description |
|---|---|
_consts.py |
Defines the enabled value. |
_params.py |
Registers the new argument. |
_help.py |
Documents the option. |
custom.py |
Exposes it in command handlers. |
managed_cluster_decorator.py |
Applies and preserves encryption settings. |
_validators.py |
Updates Key Vault ID validation. |
linter_exclusions.yml |
Excludes the long option name. |
test_validators.py |
Tests validator changes. |
test_managed_cluster_decorator.py |
Tests create/update model handling. |
test_aks_commands.py |
Adds live PMK/CMK scenarios. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Member
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
FumingZhang
reviewed
Oct 12, 2026
| # infrastructure encryption (platform-managed keys) is enabled. | ||
| segments = key_id[len(https_prefix):].split("/") | ||
| if len(segments) != 4 or segments[1] != "keys": | ||
| if len(segments) < 3 or segments[1] != "keys": |
Member
There was a problem hiding this comment.
[P2] Reject key IDs with extra path segments
len(segments) < 3 accepts malformed IDs such as https://vault.vault.azure.net/keys/key/version/extra and forwards them unchanged to the service. Please accept exactly 3 or 4 segments and add a regression test for extra segments.
Member
Author
There was a problem hiding this comment.
Good catch — fixed in 9745a9a.
The check is now len(segments) not in (3, 4), so only the two legitimate forms are accepted:
- versionless:
{vault-url}/keys/{key-name}(3 segments) — required when KMS infrastructure encryption (PMK) is enabled - versioned:
{vault-url}/keys/{key-name}/{key-version}(4 segments)
Anything with extra path components, e.g. https://fakekeyvault.vault.azure.net/keys/name/version/extra, is now rejected client-side instead of being forwarded to the service.
Added a regression test test_invalid_azure_keyvault_kms_key_id_with_extra_segments in test_validators.py covering exactly that case; the existing 3-segment and 4-segment positive cases still pass.
…MS PMK Port KMS platform-managed keys (PMK) support from the aks-preview extension into the GA azure-cli ACS module ahead of the KMS PMK GA release. Adds a new `--kms-infrastructure-encryption` parameter to `az aks create` and `az aks update`, which sets `securityProfile.kubernetesResourceObjectEncryptionProfile.infrastructureEncryption` to enable encryption at rest of Kubernetes resource objects using service-managed keys. The stable API only defines `Enabled` for the `InfrastructureEncryption` enum, so the parameter accepts `Enabled` only and defaults to unset. When unset, the encryption profile is left untouched, so PMK is never implicitly disabled. Other changes: - PMK requires a versionless key ID, so the Key Vault key ID validator no longer rejects IDs without a version segment. - The Key Vault resource ID validator now also verifies the provider is Microsoft.KeyVault and the type is vaults or managedHsms. - PMK requires a Key Vault resource ID even when the Key Vault network access is Public, so that combination is now allowed and the resource ID is set in both the create and update paths. - Add unit tests for the new context getter, the create and update decorator paths and the updated validators, plus live scenario tests covering PMK alone, PMK with CMK, and enabling PMK on an existing cluster. - Add linter exclusions for the new option name and a HISTORY.rst entry. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 671828b6-e9d0-4bbe-95e3-576ec728c597
…n with aks-preview Remove the key vault network access constraints on --azure-keyvault-kms-key-vault-resource-id so that the behavior matches the aks-preview extension: * Drop the "resource id can not be empty if network access is Private" and "resource id must be empty if network access is Public" checks from _get_azure_keyvault_kms_key_vault_resource_id. KMS PMK requires a key vault resource id regardless of the network access, so these rules no longer hold. The "Private requires a resource id" guarantee is still enforced by _get_azure_keyvault_kms_key_vault_network_access. * Always assign key_vault_resource_id when CMK is enabled, instead of only when network access is Private or PMK is enabled, so that a resource id supplied with Public network access is no longer silently dropped. The update path keeps falling back to "" so that the field is still cleared when no resource id is given. Update the affected unit tests accordingly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 671828b6-e9d0-4bbe-95e3-576ec728c597
Address PR review feedback on --azure-keyvault-kms-key-id validation. Supporting versionless key IDs for KMS infrastructure encryption (PMK) relaxed the segment check from "len(segments) != 4" to "len(segments) < 3", which also accepted malformed key IDs with extra path components such as https://fakekeyvault.vault.azure.net/keys/name/version/extra. Only the versionless ({vault-url}/keys/{key-name}) and versioned ({vault-url}/keys/{key-name}/{key-version}) forms are valid, so accept exactly 3 or 4 segments and reject anything else in the CLI instead of forwarding it to the service. Add a regression test covering the extra path segment case. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 671828b6-e9d0-4bbe-95e3-576ec728c597
Bin Xia (bingosummer)
force-pushed
the
binxi/kms-pmk
branch
from
October 13, 2026 02:17
16da9f4 to
9745a9a
Compare
Member
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
FumingZhang
approved these changes
Oct 13, 2026
Zhecheng Li (lzhecheng)
approved these changes
Oct 13, 2026
Member
Author
|
@Azure/act-observability-squad Aditya Pujara (@a0x1ab) Yong Zhang (@yonzhan) could you please help review and merge the PR? Thanks |
JaysonTaiMicrosoft
added
the
Request X Engineering Agent
![x-engineering-agent[bot]](https://avatars.githubusercontent.com/in/4205534?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Review
All 53 upstream checks passed on 9745a9aed4b13. The create/update parameter wiring reaches the encryption profile, and the changed decorator tests assert PMK-only creation, PMK with CMK, unset-profile preservation, and preserving PMK when CMK is disabled. Private Key Vault access still requires a resource ID through the network-access validator.
Integration evidence needed
src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_aks_commands.py adds test_aks_create_with_kms_pmk_and_update_cmk, test_aks_create_with_kms_pmk_and_cmk, and test_aks_update_with_kms_pmk, all marked @live_only(). They exercise the new request/output field securityProfile.kubernetesResourceObjectEncryptionProfile.infrastructureEncryption, a versionless CMK key ID with Public Key Vault access, and PMK preservation across CMK transitions. No recording files changed, so recorded playback does not establish these service interactions. The description explicitly says the proposed focused test command was not executed; the upstream 130/130 summary does not establish that these live-only scenarios ran.
Before human approval, provide a current-head live result for those three scenarios and rerun the focused decorator/validator tests (azdev test acs --filter "test_managed_cluster_decorator or test_validators"). Keep the live-only designation if intentional; otherwise provide recordings for the new request/response behavior. Distinguish the unit run from the live integration result. This human PR has no Agent live-test request, so none was dispatched.
The description's History Note covers the new PMK flag. It should also document the customer-visible acceptance of versionless KMS key IDs and Public Key Vault resource IDs, and rejection of resource IDs outside Key Vault/Managed HSM, rather than leave those validator changes only in the implementation description.
Test validation
- Agent live test: Not run; this is a review-only PR. Check author-provided test evidence and upstream CI separately.
- Regression coverage: For
acs: 3 focused test file(s) changed. Scenario coverage is unverified; review the issue's required conditions, test setup, and assertions before approval.
Risk assessment
60/100 · High · High confidence
The High rating is driven by security-sensitive behavior, public CLI behavior, failure-handling behavior.
- Change scope: 10 changed files, 737 changed lines (
+692/-45), including 6 production files. - Affected components:
acs - Risk drivers: security-sensitive behavior (+28); public CLI behavior (+18); failure-handling behavior (+12)
- Regression evidence: Changed regression tests are included, reducing risk.
- Confidence: High because changed-line patches were available for every production file.
- Required review: Owning-squad review is required for
acsbefore merge.
Posted by x-engineering-agent (Reviewer)
x-engineering-agent
Bot
added
the
X Engineering Agent Reviewed
JaysonTaiMicrosoft
added
the
Request X Engineering Agent Live Test
JaysonTaiMicrosoft
approved these changes
Oct 14, 2026
Contributor
Live test results —
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 PR Validation —⚠️ Review suggested
Related command
az aks create,az aks updateDescription
Ports KMS platform-managed keys (PMK) support from the
aks-previewextension into the GAacsmodule ahead of the KMS PMK GA release.--kms-infrastructure-encryptiontoaz aks create/az aks update, which setssecurityProfile.kubernetesResourceObjectEncryptionProfile.infrastructureEncryptionto enable encryption at rest of Kubernetes resource objects using service-managed keys.Enabledfor theInfrastructureEncryptionenum, so the parameter acceptsEnabledonly and defaults to unset. When unset, the encryption profile is left untouched, so PMK is never implicitly disabled.Microsoft.KeyVaultand the type isvaultsormanagedHsms.Public, so that combination is now allowed and the resource ID is set in both the create and update paths.Testing Guide
Unit tests added for the new context getter, the create/update decorator paths and the updated validators, plus live scenario tests covering PMK alone, PMK with CMK, and enabling PMK on an existing cluster.
Unit test run:
(not executed in this environment — please rely on CI for verification)
History Notes
[AKS]
az aks create/update: Add--kms-infrastructure-encryptionto support KMS platform-managed keys (PMK)This checklist is used to make sure that common guidelines for a pull request are followed.
The PR title and description has followed the guideline in Submitting Pull Requests.
I adhere to the Command Guidelines.
I adhere to the Error Handling Guidelines.