{Core} Remove Opt-In for --acquire-policy-token Flag#32883
Conversation
notyashhh
requested review from
DanielMicrosoft,
ReaNAiveD,
bebound,
calvinhzy,
jiasli,
jsntcy,
kairu-ms,
necusjz and
zhoxing-ms
as code owners
️✔️AzureCLI-FullTest
|
️✔️AzureCLI-BreakingChangeTest
|
|
Thank you for your contribution! We will review the pull request and get back to you soon. |
|
The git hooks are available for azure-cli and azure-cli-extensions repos. They could help you run required checks before creating the PR. Please sync the latest code with latest dev branch (for azure-cli) or main branch (for azure-cli-extensions). pip install azdev --upgrade
azdev setup -c <your azure-cli repo path> -r <your azure-cli-extensions repo path>
|
There was a problem hiding this comment.
Pull request overview
This PR removes the config-based feature flag gating for the “Global Policy” arguments, making the global policy argument registration always active in azure-cli-core.
Changes:
- Removed the
core.enable_policy_tokenconfig check. - Always registers the
EVENT_INVOKER_POST_CMD_TBL_CREATEhook to add--change-referenceand--acquire-policy-tokento eligible commands.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| from knack import events | ||
| cli_ctx.register_event(events.EVENT_INVOKER_POST_CMD_TBL_CREATE, add_global_policy_argument) |
There was a problem hiding this comment.
PR description is currently the template and doesn’t document the impact of making --change-reference/--acquire-policy-token always available (potential CLI surface-area/breaking-change implications) or how it was tested. Please fill in the related command(s), rationale, and a concrete testing guide so reviewers can validate the behavior change.
| from knack import events | ||
| cli_ctx.register_event(events.EVENT_INVOKER_POST_CMD_TBL_CREATE, add_global_policy_argument) |
There was a problem hiding this comment.
Removing the feature-flag means these global arguments are now registered for every command table load. There are no unit tests covering that the new global options are present where expected (and skipped for list/show). Consider adding a core test (e.g., in azure/cli/core/tests/test_parser.py or test_help.py) that builds a parser for a representative command and asserts the presence/absence of --change-reference and --acquire-policy-token.
|
shall we add the context in the description of that pr? |
notyashhh
changed the title
notyashhh
changed the title
|
@necusjz, Can you please review? |
necusjz
changed the title
|
|
|
According to the source branch of a PR should reside in your forked repo. |
notyashhh
changed the title
Related command
All mutating
azcommands (global change in azure-cli-core)Example:
az keyvault create,az keyvault set-policy,az network vnet create, etc.Description
core.enable_policy_tokenconfiguration flag that previously gated the Change Safety (Azure Policy Invoke) feature behind an opt-in.core.enable_policy_token=trueto access the--acquire-policy-tokenand--change-referenceglobal arguments.list/show) without any configuration needed.Testing Guide
Confirm --acquire-policy-token and --change-reference appear under "Global Policy Arguments".
Confirm the "Global Policy Arguments" section is absent.
Confirm no change in behavior or output.
History Notes
[Core]
az *: Make--acquire-policy-tokenand--change-referenceglobal arguments available by default, removing the need forcore.enable_policy_tokenconfigurationThis 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.