{AKS} AIManager print manual grant commands when --no-wait skips role assignment - #10239
{AKS} AIManager print manual grant commands when --no-wait skips role assignment#10239PugDeveloper wants to merge 3 commits into
Conversation
… assignment Under --no-wait the caller role grant is skipped (success cannot be confirmed). The old warning told the user to "Re-run without --no-wait", which is blocked by the create existence guard, or "assign the roles manually" with no command. The warning now prints the exact `az role assignment create` commands (resolved caller object id, role names, scope) for both roles. Dedupe the command string into a shared builder used by the permission-denied warning too. Bump 1.5.0 -> 1.5.1.
|
Hi PugDeveloper, |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
microsoft-github-policy-service
Bot
added
Auto-Assign
|
/azp run |
|
Azure Pipelines: Successfully started running 2 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
Updates the aimanager Azure CLI extension to improve the --no-wait user experience for az aimanager create and az aimanager namespace add by emitting copy/paste-ready az role assignment create remediation commands when automatic role assignment is skipped.
Changes:
- Add a shared helper to build
az role assignment createcommand strings and reuse it across warnings. - Replace the old
--no-waitwarning with one that prints exact manual role-assignment commands (including resolved caller object id when available). - Bump extension version to
1.5.1and add a changelog entry; update/extend unit tests accordingly.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/aimanager/setup.py | Bumps extension version to 1.5.1. |
| src/aimanager/HISTORY.rst | Documents the --no-wait warning improvement in release notes. |
| src/aimanager/azext_aimanager/custom.py | Wires --no-wait flow to the new shared warning helper. |
| src/aimanager/azext_aimanager/_roleassignments.py | Adds command-string helper and new warn_roles_skipped_no_wait warning that prints runnable remediation commands. |
| src/aimanager/azext_aimanager/tests/latest/test_roleassignments.py | Adds tests validating exact command output and fallback behavior when caller OID is unknown. |
| src/aimanager/azext_aimanager/tests/latest/test_aimanager.py | Updates wiring tests to assert the new warning helper is invoked under --no-wait. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| def _warn_assignment_failed(scope, role_id, object_id): | ||
| def _role_assignment_command(assignee, role_id, scope): | ||
| """Build the exact 'az role assignment create' command that grants one role to the caller.""" | ||
| role_name = AIMANAGER_ROLE_NAMES.get(role_id, role_id) |
There was a problem hiding this comment.
qq: it seems like this map AIMANAGER_ROLE_NAMES will try to get the role name based on the mapping on role_id, so the second role_id is just for default, which means if there is no either AIMANAGER_CONTRIBUTOR_ROLE_ID or AIMANAGER_RBAC_READER_ROLE_ID, but it assigns to role_name so sort of confused.
AIMANAGER_ROLE_NAMES = {
AIMANAGER_CONTRIBUTOR_ROLE_ID: "Azure AIManager Contributor",
AIMANAGER_RBAC_READER_ROLE_ID: "Azure AIManager and namespace RBAC Reader",
}
There was a problem hiding this comment.
Yeah, can see the confusion. role_id being both the key and the default made that line read weird. Pulled it into a small _role_name() helper so it's obvious what's happening: if a role id isn't in the map it just falls back to the GUID, which az role assignment create --role takes fine anyway. It's only ever called with the two known roles so the fallback never actually kicks in, but the helper makes the intent clear now.
There was a problem hiding this comment.
Actually I see now. az role assignment create with --role parameter accepts either a role name or a role definition GUID - Azure CLI resolves a name to its GUID for you. That's a general Azure CLI capability.
…back Address review feedback: AIMANAGER_ROLE_NAMES.get(role_id, role_id) read oddly (same value as key and default). Extract a named _role_name(role_id) helper that documents the fallback (an unmapped id falls back to the raw GUID, which az role assignment create --role also accepts, so the best-effort warning never raises) and dedupes the two identical lookups. No behavior change.
|
/azp run |
|
Azure Pipelines: Successfully started running 2 pipeline(s). |
…ning Address review nit: the --no-wait warning hands out az role assignment create commands but did not say they need elevated rights. Fold "requires Owner or User Access Administrator" into the message (matching the permission-denied warning) and drop the redundant "safe to run after creation completes" phrasing, so it is not longer. Assert the caveat in the unit test.
|
/azp run |
|
Azure Pipelines: Successfully started running 2 pipeline(s). |
Ximeng Zhao (xmzhao0822)
left a comment
Ximeng Zhao (xmzhao0822)
left a comment
There was a problem hiding this comment.
Looks good, approved.
|
Julie Zhu (@yanzhudd) Please help to merge the PR, thank you :) |
🤖 PR Validation — ️✔️ All clear
Related command
az aimanager create,az aimanager namespace addDescription
Under
--no-wait, the caller role grant is skipped because the command returns before creation is confirmed. The old warning suggested "Re-run without--no-wait" (blocked by the create existence guard) or "assign the roles manually" (no command given).The warning now prints the exact
az role assignment createcommands for both roles, with the resolved caller object id, role names, and scope, so the user can copy-paste them once the resource is created. The command string is now built by a shared helper reused by the permission-denied warning.No behavior change to the default (waited) path. Follow-up to #10237.
Extension version: aimanager
1.5.0->1.5.1(patch).General Guidelines
azdev style aimanagerlocally? (PASSED)python scripts/ci/test_index.py -qlocally? (OK)About Extension Publish
Bumped
setup.pyVERSION to1.5.1and added aHISTORY.rstentry.