Add az role deny-assignment create/delete commands (user-assigned deny assignments)#33109
Open
Add az role deny-assignment create/delete commands (user-assigned deny assignments)#33109
Conversation
❌AzureCLI-FullTest
|
|
| rule | cmd_name | rule_message | suggest_message |
|---|---|---|---|
| role deny-assignment | sub group role deny-assignment added |
Collaborator
|
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>
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds first-class az role deny-assignment CRUD support (focused on PP1 user-assigned deny assignments) to align with existing role assignment workflows, including command registration, parameters, help, and tests.
Changes:
- Register
az role deny-assignment list/show/create/deletecommands and add a table transformer for list output. - Implement deny-assignment list/show/create/delete custom handlers with PP1 validation.
- Add help/params, linter exclusions for long options, and introduce a new deny-assignment test file.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
src/azure-cli/azure/cli/command_modules/role/custom.py |
Adds deny-assignment list/show/create/delete implementations and PP1 input validation. |
src/azure-cli/azure/cli/command_modules/role/commands.py |
Registers new role deny-assignment commands and list table transformer. |
src/azure-cli/azure/cli/command_modules/role/_params.py |
Defines CLI parameters for deny-assignment commands. |
src/azure-cli/azure/cli/command_modules/role/_help.py |
Adds help text and examples for the new/updated deny-assignment commands. |
src/azure-cli/azure/cli/command_modules/role/linter_exclusions.yml |
Excludes option-length lint rules for new long parameter names. |
src/azure-cli/azure/cli/command_modules/role/tests/latest/test_deny_assignment.py |
Adds scenario/live tests covering list/show and PP1 create/delete validation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+627
to
+638
| 'deny_assignment_name': deny_assignment_name, | ||
| 'description': description or '', | ||
| 'permissions': [{ | ||
| 'actions': actions or [], | ||
| 'not_actions': not_actions or [], | ||
| 'data_actions': [], | ||
| 'not_data_actions': [] | ||
| }], | ||
| 'scope': scope, | ||
| 'principals': principals, | ||
| 'exclude_principals': exclude_principals, | ||
| 'is_system_protected': False |
There was a problem hiding this comment.
create_deny_assignment builds the request body as a plain dict using snake_case keys (e.g. deny_assignment_name, not_actions, exclude_principals). For mgmt SDK operations, dicts are typically serialized as-is, so the service will receive incorrect field names (it expects camelCase JSON, or a proper SDK model instance). Use the azure-mgmt-authorization model types via get_sdk(..., mod='models') (similar to RoleApiHelper.create_role_assignment) or ensure the payload keys match the service JSON contract exactly.
Suggested change
| 'deny_assignment_name': deny_assignment_name, | |
| 'description': description or '', | |
| 'permissions': [{ | |
| 'actions': actions or [], | |
| 'not_actions': not_actions or [], | |
| 'data_actions': [], | |
| 'not_data_actions': [] | |
| }], | |
| 'scope': scope, | |
| 'principals': principals, | |
| 'exclude_principals': exclude_principals, | |
| 'is_system_protected': False | |
| 'denyAssignmentName': deny_assignment_name, | |
| 'description': description or '', | |
| 'permissions': [{ | |
| 'actions': actions or [], | |
| 'notActions': not_actions or [], | |
| 'dataActions': [], | |
| 'notDataActions': [] | |
| }], | |
| 'scope': scope, | |
| 'principals': principals, | |
| 'excludePrincipals': exclude_principals, | |
| 'isSystemProtected': False |
Comment on lines
+641
to
+642
| return deny_client.create(scope=scope, deny_assignment_id=assignment_name, | ||
| parameters=deny_assignment_params) |
There was a problem hiding this comment.
This calls authorization_client.deny_assignments.create(...), but the repo currently pins azure-mgmt-authorization==5.0.0b1 (which does not include denyAssignments create/delete per the PR description). Without bumping the SDK dependency (or adding a fallback implementation / friendly error), this command will raise AttributeError at runtime.
Comment on lines
+645
to
+654
| def delete_deny_assignment(cmd, scope=None, deny_assignment_id=None, deny_assignment_name=None): | ||
| """Delete a user-assigned deny assignment.""" | ||
| authorization_client = _auth_client_factory(cmd.cli_ctx, scope) | ||
| deny_client = authorization_client.deny_assignments | ||
|
|
||
| if deny_assignment_id: | ||
| return deny_client.delete_by_id(deny_assignment_id) | ||
| if deny_assignment_name and scope: | ||
| return deny_client.delete(scope=scope, deny_assignment_id=deny_assignment_name) | ||
| raise CLIError('Please provide --id, or both --name and --scope.') |
There was a problem hiding this comment.
Same dependency issue as create: deny_client.delete(...)/delete_by_id(...) will fail at runtime unless the pinned azure-mgmt-authorization version includes these methods. Consider either updating the dependency in this PR or detecting missing methods and raising a clear CLIError instructing users to upgrade.
Comment on lines
+397
to
+398
| c.argument('deny_assignment_name', options_list=['--name', '-n'], | ||
| help='The display name of the deny assignment.') |
There was a problem hiding this comment.
deny_assignment_name is defined at the role deny-assignment group level, which makes --name/-n show up for subcommands like list even though list_deny_assignments doesn't accept that parameter. If a user supplies --name on list, the handler will receive an unexpected kwarg and fail. Recommend removing deny_assignment_name from the group context and defining --name only on show/create/delete where it is supported.
Suggested change
| c.argument('deny_assignment_name', options_list=['--name', '-n'], | |
| help='The display name of the deny assignment.') |
| c.argument('exclude_principal_ids', nargs='+', options_list=['--exclude-principal-ids'], | ||
| help='Space-separated list of principal object IDs to exclude from the deny. ' | ||
| 'At least one is required for user-assigned deny assignments.') | ||
| c.argument('exclude_principal_types', nargs='+', options_list=['--exclude-principal-types'], |
There was a problem hiding this comment.
--exclude-principal-types is documented as having accepted values, but the argument doesn't enforce them. To keep validation consistent with role assignment create --assignee-principal-type, use arg_type=get_enum_type([...]) (or an Enum) so invalid values are caught client-side with a clear error.
Suggested change
| c.argument('exclude_principal_types', nargs='+', options_list=['--exclude-principal-types'], | |
| c.argument('exclude_principal_types', nargs='+', options_list=['--exclude-principal-types'], | |
| arg_type=get_enum_type(['User', 'Group', 'ServicePrincipal']), |
Comment on lines
+15
to
+35
| class DenyAssignmentListTest(ScenarioTest): | ||
| """Tests for az role deny-assignment list — works on any subscription.""" | ||
|
|
||
| def test_deny_assignment_list(self): | ||
| """List deny assignments at the subscription scope.""" | ||
| result = self.cmd('role deny-assignment list').get_output_in_json() | ||
| # Result should be a list (may be empty if no deny assignments exist) | ||
| self.assertIsInstance(result, list) | ||
|
|
||
| def test_deny_assignment_list_with_scope(self): | ||
| """List deny assignments at a specific scope.""" | ||
| self.cmd('role deny-assignment list --scope /subscriptions/{sub}', | ||
| checks=[self.check('type(@)', 'array')]) | ||
|
|
||
| def test_deny_assignment_list_with_filter(self): | ||
| """List deny assignments with OData filter.""" | ||
| result = self.cmd( | ||
| 'role deny-assignment list --filter "atScope()"' | ||
| ).get_output_in_json() | ||
| self.assertIsInstance(result, list) | ||
|
|
There was a problem hiding this comment.
This file adds ScenarioTest cases for role deny-assignment list that will require a new VCR recording to pass in playback mode, but no corresponding recording YAML is added under tests/latest/recordings. Either add recordings for these scenario tests or convert them to mock-based tests (or LiveScenarioTest if they must be live-only) so CI doesn't fail.
Comment on lines
+56
to
+61
| self.kwargs.update({ | ||
| 'scope': '/subscriptions/{sub}', | ||
| 'name': 'CLI Test Deny Assignment', | ||
| 'action': 'Microsoft.Authorization/roleAssignments/write', | ||
| # Use a well-known object ID for exclusion (replace with a real SP in your test env) | ||
| 'exclude_id': self.create_guid() |
There was a problem hiding this comment.
exclude_id is set to self.create_guid(), which is a random GUID and is unlikely to correspond to a real principal in the tenant. If the denyAssignment API validates excluded principals, this live test will fail reliably. Prefer creating a real principal in the test setup (or using the signed-in user/service principal object id) and passing its object ID here.
Suggested change
| self.kwargs.update({ | |
| 'scope': '/subscriptions/{sub}', | |
| 'name': 'CLI Test Deny Assignment', | |
| 'action': 'Microsoft.Authorization/roleAssignments/write', | |
| # Use a well-known object ID for exclusion (replace with a real SP in your test env) | |
| 'exclude_id': self.create_guid() | |
| signed_in_user = self.cmd('ad signed-in-user show').get_output_in_json() | |
| exclude_id = signed_in_user['id'] | |
| self.kwargs.update({ | |
| 'scope': '/subscriptions/{sub}', | |
| 'name': 'CLI Test Deny Assignment', | |
| 'action': 'Microsoft.Authorization/roleAssignments/write', | |
| 'exclude_id': exclude_id |
| # These tests require a subscription with the UserAssignedDenyAssignment feature flag enabled. | ||
|
|
||
| import unittest | ||
| from unittest import mock |
There was a problem hiding this comment.
Unused import: from unittest import mock is not used in this test file. Please remove it to keep linting clean.
Suggested change
| from unittest import mock |
Update create command to support two modes: - Everyone mode (default): denies all principals, requires exclude-principal-ids - Per-principal mode: denies a specific User or ServicePrincipal via --principal-id/--principal-type API changes from DA PR msazure/One#15293894: - 3P UADA can now target specific User and ServicePrincipal principals - Group type principals are explicitly disallowed - Single-principal-per-UADA constraint enforced Changes: - custom.py: Add principal_id/principal_type params, dual-mode logic, Group rejection - _params.py: Add --principal-id and --principal-type (enum) arguments - _help.py: Update long-summary and examples for both modes - tests: Add per-principal CRUD, Group rejection, missing-param validation tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jruttle
force-pushed
the
jruttle/add-deny-assignment-commands
branch
from
ae59ca3 to
01b4d90
Compare
Now that azure-mgmt-authorization 5.0.0b2 has been published to PyPI (2026-05-07), this picks up the new DenyAssignment management plane operations (BeginCreateOrUpdate / BeginDelete) added in PR #46223. This unblocks the deny-assignment CLI commands in this PR from running against the new SDK surface.
jruttle
requested review from
DanielMicrosoft,
ReaNAiveD and
bebound
as code owners
Author
|
Bumped This picks up the new DenyAssignment management-plane operations ( This was the last upstream blocker on this PR — the CLI commands here now have a real published SDK surface to call against. CC for review awareness: prior reviewers / CLI mgmt module owners. |
Follow-up to commit 20fe3a7 which only bumped setup.py. The Linux/Darwin/Windows requirements lock files also pinned 5.0.0b1 and caused azdev-linter / azdev-style CI to fail with: ERROR: Cannot install azure-cli==2.85.0 and azure-mgmt-authorization==5.0.0b1 because these package versions have conflicting dependencies. This commit aligns all three platform lock files with setup.py at 5.0.0b2.
Author
|
Follow-up to commit …showed the platform lock files ( CI should now re-run cleanly on the new HEAD. |
Author
|
/azp run |
|
Commenter does not have sufficient privileges for PR 33109 in repo Azure/azure-cli |
Author
|
Hi @yonzhan @isra-fel — could one of you (or anyone with write access) re-trigger the Azure DevOps validation pipelines on this PR? The two required checks Either of these comments should kick them off:
Background context: the SDK dependency bump unblocks this PR — Thanks! |
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
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.
Description
Adds
az role deny-assignment createandaz role deny-assignment deletecommands for managing user-assigned deny assignments, matching the existingaz role assignmentpattern.This implements the PUT/DELETE operations added in the
Microsoft.Authorization/denyAssignmentsAPI (2024-07-01-preview), as specified in TypeSpec PR #41617.Two Assignment Modes
The
createcommand supports two modes for targeting principals:Everyone mode (default): Denies all principals at the scope. At least one excluded principal is required via
--exclude-principal-ids:Per-principal mode: Denies a specific User or ServicePrincipal via
--principal-idand--principal-type:Service Constraints
User-assigned deny assignments have specific restrictions enforced by the service:
*/readare not permittedCommands
az role deny-assignment list— List deny assignments (existing, enhanced)az role deny-assignment show— Show a deny assignment (existing, enhanced)az role deny-assignment create— Create a user-assigned deny assignmentaz role deny-assignment delete— Delete a user-assigned deny assignmentFiles Changed
commands.py— Command registration forrole deny-assignmentgroupcustom.py— Business logic with dual-mode principal handling and validation_params.py— Parameter definitions including--principal-idand--principal-type_help.py— Help text with examples for both Everyone and per-principal modeslinter_exclusions.yml— Exclusions for long parameter namestests/latest/test_deny_assignment.py— Unit tests (list, show, CRUD, per-principal, Group rejection, param validation)Dependency
Testing
Tests are included in
test_deny_assignment.py. Full end-to-end testing requires:Microsoft.Authorization/SubscriptionAllowedToOperateUserAssignedDenyAssignmentfeature flag registeredRelated