Kickstart: pre-flight check for user deploy RBAC (cluster + ACR)#2165
Open
bosesuneha wants to merge 1 commit into
Open
Kickstart: pre-flight check for user deploy RBAC (cluster + ACR)#2165bosesuneha wants to merge 1 commit into
bosesuneha wants to merge 1 commit into
Conversation
Adds a centralized helper that checks whether the signed-in user holds the
Azure role assignments required for a successful Kickstart deploy on the
Normal Namespace path:
Cluster scope (Azure RBAC for Kubernetes, when enabled):
- AKS RBAC Writer / RBAC Admin / RBAC Cluster Admin (kubectl apply)
ACR scope:
- AcrPush (docker push)
- Container Registry Tasks Contributor (az acr build)
The check uses listForScope with the atScope() filter so RG / subscription /
MG-inherited assignments count. A 403 on enumeration sets a *Inconclusive
flag so the UI warns rather than blocks.
CONFIGURE phase renders results as a single markdown table; DEPLOY phase
blocks only when the user definitively lacks an AKS RBAC Writer-tier role
on an Azure-RBAC-enabled cluster.
Refactors aksContainerAssist/oidcSetup.ts to consume the same role-ID
constants and isAzureRbacEnabled helper from the new shared module.
Self-assign / fix-it commands deferred to a follow-up PR.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a centralized Kickstart pre-flight RBAC check for the signed-in user (cluster deploy roles + ACR push/tasks roles) and surfaces the results in the CONFIGURE phase as a markdown table, with a deploy-phase gate when Azure RBAC for Kubernetes is enabled and the user definitively lacks a deploy-capable AKS RBAC role.
Changes:
- Introduces
aksRbacHelpers.tswith role IDs and helper functions to detect Azure RBAC enablement and enumerate user role assignments at cluster/ACR scopes. - Extends Kickstart configuration/state to carry RBAC pre-flight results and renders them in CONFIGURE as a single markdown table.
- Adds a DEPLOY-phase blocker for Azure-RBAC-enabled clusters when the user is confirmed missing an AKS RBAC Writer/Admin/Cluster Admin role.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/commands/utils/aksRbacHelpers.ts | New shared helper for AKS/ACR RBAC pre-flight (role IDs, Azure RBAC detection, role assignment enumeration). |
| src/commands/aksKickstart/configure.ts | Fetch cluster once and run new RBAC pre-flight in parallel; plumbs result into KickstartConfiguration. |
| src/commands/aksContainerAssist/oidcSetup.ts | Deduplicates built-in role IDs and reuses shared aksRbacHelpers utilities. |
| src/chatParticipants/kickstart/state.ts | Extends ConfigData with RBAC pre-flight fields for rendering and gating. |
| src/chatParticipants/kickstart/phases/configure.ts | Renders consolidated pre-flight checks as a markdown table (including RBAC rows when applicable). |
| src/chatParticipants/kickstart/phases/deploy.ts | Blocks deployment when Azure RBAC for Kubernetes is enabled and the user conclusively lacks a deploy role. |
| src/tests/suite/kickstart.integration.test.ts | Updates integration test fixtures to include new ConfigData RBAC fields. |
| src/chatParticipants/kickstart/state.test.ts | Updates state tests for new RBAC fields in ConfigData. |
| src/chatParticipants/kickstart/phaseRunner.test.ts | Updates phase runner tests for new RBAC fields in ConfigData. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| scope: string, | ||
| userObjectId: string, | ||
| ): Promise<Errorable<ScopeRolesResult>> { | ||
| const listResult = await listAll(client.roleAssignments.listForScope(scope, { filter: "atScope()" })); |
Comment on lines
+72
to
+90
| ): Promise<Errorable<ScopeRolesResult>> { | ||
| const listResult = await listAll(client.roleAssignments.listForScope(scope, { filter: "atScope()" })); | ||
|
|
||
| if (failed(listResult)) { | ||
| if (isAuthorizationError(listResult.error)) { | ||
| return { | ||
| succeeded: true, | ||
| result: { roleIds: new Set(), inconclusive: true, reason: listResult.error }, | ||
| }; | ||
| } | ||
| return listResult; | ||
| } | ||
|
|
||
| const roleIds = new Set<string>(); | ||
| for (const ra of listResult.result) { | ||
| if (ra.principalId?.toLowerCase() !== userObjectId.toLowerCase()) continue; | ||
| const roleDefName = ra.roleDefinitionId?.split("/").pop(); | ||
| if (roleDefName) roleIds.add(roleDefName.toLowerCase()); | ||
| } |
Comment on lines
+8
to
+14
| /** Result of a single pre-flight check: granted ✅, missing ❌, or inconclusive ⚠️ (403). */ | ||
| type CheckStatus = "granted" | "missing" | "inconclusive"; | ||
|
|
||
| const STATUS_ICON: Record<CheckStatus, string> = { granted: "✅", missing: "❌", inconclusive: "⚠️" }; | ||
|
|
||
| function statusOf(granted: boolean, inconclusive: boolean): CheckStatus { | ||
| if (inconclusive) return "inconclusive"; |
Comment on lines
+191
to
+202
| const checks: PreflightCheck[] = [ | ||
| check("Cluster SKU", config.clusterSku === "Automatic" ? "inconclusive" : "granted", { | ||
| granted: "AKS Standard", | ||
| missing: "AKS Standard", | ||
| inconclusive: | ||
| "AKS Automatic — Azure manages node pools, scaling, and upgrades. Some Kickstart features may behave differently.", | ||
| }), | ||
| check("Kubeconfig access (you → cluster)", config.canGetKubeconfig ? "granted" : "missing", { | ||
| granted: "Available", | ||
| missing: "Denied. Ensure you have the **Azure Kubernetes Service Cluster User Role** on the cluster.", | ||
| inconclusive: "Could not verify.", | ||
| }), |
Collaborator
Fixes for Copilot review commentsI've addressed all 4 review comments locally. Here's the commit: Changes:1.
2.
3.
@bosesuneha Can you pull this into the PR? Branch: |
davidgamero
added a commit
that referenced
this pull request
May 26, 2026
* Add deploy RBAC pre-flight checks to Kickstart configure phase
Adds a centralized helper that checks whether the signed-in user holds the
Azure role assignments required for a successful Kickstart deploy on the
Normal Namespace path:
Cluster scope (Azure RBAC for Kubernetes, when enabled):
- AKS RBAC Writer / RBAC Admin / RBAC Cluster Admin (kubectl apply)
ACR scope:
- AcrPush (docker push)
- Container Registry Tasks Contributor (az acr build)
The check uses listForScope with the atScope() filter so RG / subscription /
MG-inherited assignments count. A 403 on enumeration sets a *Inconclusive
flag so the UI warns rather than blocks.
CONFIGURE phase renders results as a single markdown table; DEPLOY phase
blocks only when the user definitively lacks an AKS RBAC Writer-tier role
on an Azure-RBAC-enabled cluster.
Refactors aksContainerAssist/oidcSetup.ts to consume the same role-ID
constants and isAzureRbacEnabled helper from the new shared module.
Self-assign / fix-it commands deferred to a follow-up PR.
* fix: address Copilot review comments on PR #2165
- aksRbacHelpers: remove atScope() filter to include inherited roles;
use assignedTo server-side filter instead of in-memory principalId check
- configure: add 'warning' CheckStatus for AKS Automatic SKU info
(distinct from 'inconclusive' which means 403/auth failure)
- configure: render pre-flight table before kubeconfig failure so user
sees all check results even when kubeconfig access is denied
---------
Co-authored-by: Suneha Bose <suneha.bose@gmail.com>
Co-authored-by: David Gamero <davidgamero@users.noreply.github.com>
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.
Summary
Adds a centralized pre-flight check that verifies the signed-in user holds the Azure role assignments required for a successful Kickstart deploy on the Normal Namespace path, and surfaces the result as a markdown table in the CONFIGURE phase.
Cluster scope (only when Azure RBAC for Kubernetes is enabled — always on for AKS Automatic):
kubectl applyACR scope:
docker pushduring BUILDaz acr buildThe check uses
roleAssignments.listForScopewithatScope(), so RG / subscription / management-group-inherited assignments count.Behavior
azureRbacEnabledis true.*Inconclusiveflag is set → warning rendered, no block.Out of scope (deferred to follow-up PR)
Self-assign / fix-it commands for missing roles. The check results carry all the data needed; this PR is checks-only.
Files
src/commands/utils/aksRbacHelpers.ts— role IDs,isAzureRbacEnabled,isAuthorizationError,checkUserDeployRbacsrc/commands/aksContainerAssist/oidcSetup.ts— dedup constants, delegate to shared helpersrc/commands/aksKickstart/configure.ts— fetch cluster once, parallel checks,userDeployRbaconKickstartConfigurationsrc/chatParticipants/kickstart/state.ts—ConfigDataextended with RBAC fieldssrc/chatParticipants/kickstart/phases/configure.ts— markdown-table renderersrc/chatParticipants/kickstart/phases/deploy.ts— pre-deploy RBAC gateTesting
tsc --noEmitcleaneslintclean on all touched filesvalidatePrereqs) pre-exist onupstream/kickstartand are unrelated to this change (verified by stash + re-run)