Add az role deny-assignment create/delete commands (user-assigned deny assignments)#33109
Add az role deny-assignment create/delete commands (user-assigned deny assignments)#33109
Conversation
|
Validation for Azure CLI Full Test Starting...
Thanks for your contribution! |
|
Validation for Breaking Change Starting...
Thanks for your contribution! |
|
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
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.
| '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.
| '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 |
| 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.
| 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.
| 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.
| 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.
| 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']), |
| 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.
| 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.
| 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.
| from unittest import mock |
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 swagger PR #41104.PP1 (Preview) Constraints
User-assigned deny assignments have specific restrictions enforced by the service:
Everyone(SystemDefined with00000000-0000-0000-0000-000000000000)*/readare not permittedCommands Added
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 (4 functions with PP1 validation)_params.py— Parameter definitions for all deny-assignment commands_help.py— Help text with examples for all 4 commands + grouplinter_exclusions.yml— Exclusions for long parameter namestests/latest/test_deny_assignment.py— Unit tests (list, show, CRUD)Dependency
Testing
Tests are included in
test_deny_assignment.py. Full end-to-end testing requires:Microsoft.Authorization/SubscriptionAllowedToOperateUserAssignedDenyAssignmentfeature flag registeredRelated