Deploy: Add secrets, workload identity, ACR support, and fix identity role assignments#468
Open
gambtho wants to merge 3 commits intoAzure:mainfrom
Open
Deploy: Add secrets, workload identity, ACR support, and fix identity role assignments#468gambtho wants to merge 3 commits intoAzure:mainfrom
gambtho wants to merge 3 commits intoAzure:mainfrom
Conversation
gambtho
changed the title
There was a problem hiding this comment.
Pull request overview
Adds Deploy Wizard support for (1) marking environment variables as secrets (emitted as a Kubernetes Secret + valueFrom.secretKeyRef) and (2) configuring Azure Workload Identity (managed identity + federated credential flow), plus updates to pipeline agent templates/tests to account for the expanded container config shape.
Changes:
- Extend
ContainerConfigto includeenvVars[].isSecretand Workload Identity fields, and update fixtures/stories/tests accordingly. - Update Deploy Wizard UI + YAML generator to create Secret/ServiceAccount manifests and label/pod spec wiring for workload identity.
- Add Azure CLI helpers and a new hook to automate managed identity + role assignment + OIDC issuer lookup + federated credential creation.
Reviewed changes
Copilot reviewed 33 out of 33 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| plugins/aks-desktop/src/utils/azure/az-cli.ts | Adds AKS OIDC issuer lookup, managed identity listing, and K8s federated credential creation helpers. |
| plugins/aks-desktop/src/components/GitHubPipeline/utils/pipelineOrchestration.test.ts | Updates tests to include the new envVars[].isSecret field. |
| plugins/aks-desktop/src/components/GitHubPipeline/utils/agentTemplates.ts | Adds pipeline workflow instructions for workload identity and surfaces it in generated agent config. |
| plugins/aks-desktop/src/components/GitHubPipeline/utils/agentTemplates.test.ts | Updates agent template tests for the new env var shape. |
| plugins/aks-desktop/src/components/GitHubPipeline/fixtures/pipelineConfig.ts | Extends fixture defaults with isSecret and workload identity fields. |
| plugins/aks-desktop/src/components/DeployWizard/utils/yamlGenerator.ts | Emits Secret + Workload Identity ServiceAccount/labels/serviceAccountName into generated YAML. |
| plugins/aks-desktop/src/components/DeployWizard/hooks/useDeployWorkloadIdentity.ts | New hook to set up managed identity + role assignment + federated credentials for a K8s service account. |
| plugins/aks-desktop/src/components/DeployWizard/hooks/useContainerConfiguration.ts | Extends ContainerConfig type and default state to include secrets + workload identity fields. |
| plugins/aks-desktop/src/components/DeployWizard/components/DeployWizardPure.stories.tsx | Updates story stub config for new fields. |
| plugins/aks-desktop/src/components/DeployWizard/components/ConfigureContainer.tsx | Adds env var secret toggle UI and a new Workload Identity step with “create”/“existing” identity flows. |
| plugins/aks-desktop/src/components/DeployWizard/DeployWizard.tsx | Passes Azure context/namespace into ConfigureContainer for workload identity configuration. |
| plugins/aks-desktop/src/components/DeployTab/utils/extractContainerConfig.ts | Extracts secret env vars and workload identity label/serviceAccountName from live deployments. |
| plugins/aks-desktop/src/components/DeployTab/utils/extractContainerConfig.test.ts | Updates extraction tests for isSecret. |
| plugins/aks-desktop/src/components/DeployTab/components/ClusterDeployCard.tsx | Supplies Azure context to Deploy Wizard from the Deploy Tab. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
plugins/aks-desktop/src/components/DeployWizard/components/ConfigureContainer.tsx
Outdated
Show resolved
Hide resolved
plugins/aks-desktop/src/components/DeployWizard/components/ConfigureContainer.tsx
Show resolved
Hide resolved
plugins/aks-desktop/src/components/DeployWizard/utils/yamlGenerator.ts
Outdated
Show resolved
Hide resolved
plugins/aks-desktop/src/components/DeployWizard/components/ConfigureContainer.tsx
Show resolved
Hide resolved
plugins/aks-desktop/src/components/DeployWizard/components/ConfigureContainer.tsx
Show resolved
Hide resolved
gambtho
force-pushed
the
thgamble/managedidentity
branch
3 times, most recently
from
2e27c8d to
1517b6b
Compare
gambtho
changed the title
gambtho
force-pushed
the
thgamble/managedidentity
branch
3 times, most recently
from
7d3dc54 to
ec8f6f5
Compare
Add per-variable secret toggle in environment variables step and a new Workload Identity stepper step that creates Azure managed identities with federated credentials for the cluster's OIDC issuer.
gambtho
force-pushed
the
thgamble/managedidentity
branch
from
ec8f6f5 to
55d85f2
Compare
Assign the full required role matrix (AcrPush, Container Registry Tasks Contributor, AKS RBAC Writer, AKS Namespace User) scoped correctly for both normal and managed namespaces. Add ACR selection/creation to the pipeline wizard and extract shared identity setup into reusable utilities.
gambtho
force-pushed
the
thgamble/managedidentity
branch
from
55d85f2 to
10a4895
Compare
There was a problem hiding this comment.
Pull request overview
This PR extends AKS Desktop’s Deploy Wizard and GitHub Pipeline flows to support per-env-var Kubernetes Secrets, Azure Workload Identity (with shared identity/role setup utilities), and optional Azure Container Registry (ACR) integration for build/push workflows, while correcting managed identity role assignment behavior.
Changes:
- Introduces shared Azure identity setup utilities (
identitySetup,identityRoles,identityWithRoles) and updates pipeline/deploy hooks to use multi-role assignment. - Adds Workload Identity configuration to deploy/pipeline flows (OIDC issuer discovery, federated credential creation, ServiceAccount + pod config generation).
- Adds ACR selection/creation support and threads ACR context into pipeline secrets and agent instructions; adds per-variable secret env var support in YAML generation + extraction.
Reviewed changes
Copilot reviewed 48 out of 48 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| plugins/aks-desktop/src/utils/azure/identityWithRoles.ts | New shared orchestration for identity creation + required role assignments |
| plugins/aks-desktop/src/utils/azure/identitySetup.ts | New utility to ensure RG + identity exist |
| plugins/aks-desktop/src/utils/azure/identitySetup.test.ts | Tests for identity setup utility |
| plugins/aks-desktop/src/utils/azure/identityRoles.ts | New role-matrix computation for normal vs managed namespaces + ACR + Azure RBAC |
| plugins/aks-desktop/src/utils/azure/identityRoles.test.ts | Tests for role computation |
| plugins/aks-desktop/src/utils/azure/az-cli.ts | Adds ACR typing, multi-role assignment, OIDC issuer discovery, identity listing, K8s federated credential creation, ACR creation helpers |
| plugins/aks-desktop/src/components/GitHubPipeline/utils/pipelineOrchestration.ts | Adds ACR name secret creation for workflows |
| plugins/aks-desktop/src/components/GitHubPipeline/utils/pipelineOrchestration.test.ts | Updates env var fixtures for new isSecret field |
| plugins/aks-desktop/src/components/GitHubPipeline/utils/agentTemplates.ts | Adds workload identity + ACR build/push instructions and summary output |
| plugins/aks-desktop/src/components/GitHubPipeline/utils/agentTemplates.test.ts | Updates env var fixtures for new isSecret field |
| plugins/aks-desktop/src/components/GitHubPipeline/types.ts | Extends pipeline config with ACR fields |
| plugins/aks-desktop/src/components/GitHubPipeline/hooks/useWorkloadIdentitySetup.ts | Switches to shared identity+roles utility; adds cluster/ACR/namespace context |
| plugins/aks-desktop/src/components/GitHubPipeline/hooks/useWorkloadIdentitySetup.test.ts | Updates mocks/expectations for multi-role assignment + shared utility |
| plugins/aks-desktop/src/components/GitHubPipeline/components/WorkloadIdentitySetup.tsx | Updates UI step wording/status for role assignments and threads new inputs |
| plugins/aks-desktop/src/components/GitHubPipeline/components/WorkloadIdentitySetup.test.tsx | Adds clusterName prop to fixture |
| plugins/aks-desktop/src/components/GitHubPipeline/components/AcrSelector.tsx | New UI component to select/create/skip ACR |
| plugins/aks-desktop/src/components/GitHubPipeline/fixtures/pipelineConfig.ts | Updates fixture for isSecret and workload identity fields |
| plugins/aks-desktop/src/components/GitHubPipeline/GitHubPipelineWizard.tsx | Threads clusterName into workload identity setup component |
| plugins/aks-desktop/src/components/DeployWizard/utils/yamlGenerator.ts | Emits Secret + secretKeyRef env vars; emits ServiceAccount + WI pod config |
| plugins/aks-desktop/src/components/DeployWizard/hooks/useDeployWorkloadIdentity.ts | New deploy-side workload identity setup hook (OIDC issuer + K8s federated cred) |
| plugins/aks-desktop/src/components/DeployWizard/hooks/useContainerConfiguration.ts | Extends container config with isSecret env vars and workload identity fields |
| plugins/aks-desktop/src/components/DeployWizard/components/DeployWizardPure.stories.tsx | Updates stub config for new fields |
| plugins/aks-desktop/src/components/DeployWizard/components/ConfigureContainer.tsx | Adds env var secret toggle/masking; adds workload identity step (create/select identity) |
| plugins/aks-desktop/src/components/DeployWizard/DeployWizard.tsx | Threads Azure context + namespace into container configuration UI |
| plugins/aks-desktop/src/components/DeployTab/utils/extractContainerConfig.ts | Adds round-trip support for secretKeyRef env vars and WI pod config detection |
| plugins/aks-desktop/src/components/DeployTab/utils/extractContainerConfig.test.ts | Updates expected env var structure with isSecret |
| plugins/aks-desktop/src/components/DeployTab/components/ClusterDeployCard.tsx | Threads Azure context into Deploy Wizard |
| Localize/locales/en/plugin-translation.json | Updates localized strings set |
| Localize/locales/en/frontend-translation.json | Adds new localized string key |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
+1474
to
+1482
| if (value) { | ||
| const saName = getServiceAccountName( | ||
| containerConfig.config.appName || 'app' | ||
| ); | ||
| containerConfig.setConfig(c => ({ | ||
| ...c, | ||
| workloadIdentityClientId: value.clientId, | ||
| workloadIdentityServiceAccount: saName, | ||
| })); |
Comment on lines
+66
to
+78
| let managedNamespaceResourceId: string | undefined; | ||
| if (isManagedNamespace && namespaceName) { | ||
| const nsResult = await getManagedNamespaceResourceId({ | ||
| clusterName, | ||
| resourceGroup, | ||
| namespaceName, | ||
| subscriptionId, | ||
| }); | ||
| if (!nsResult.success || !nsResult.resourceId) { | ||
| throw new Error(nsResult.error ?? 'Failed to get managed namespace resource ID'); | ||
| } | ||
| managedNamespaceResourceId = nsResult.resourceId; | ||
| } |
Comment on lines
+3540
to
+3545
| if (!isValidGuid(subscriptionId)) { | ||
| return { success: false, error: 'Invalid subscription ID format' }; | ||
| } | ||
| if (!isValidAzResourceName(resourceGroup)) { | ||
| return { success: false, error: 'Invalid resource group format' }; | ||
| } |
Comment on lines
+202
to
+212
| <TextField | ||
| label={t('Registry Name')} | ||
| size="small" | ||
| value={newRegistryName} | ||
| onChange={e => setNewRegistryName(e.target.value)} | ||
| fullWidth | ||
| helperText={t( | ||
| 'Globally unique name. Only alphanumeric characters, 5-50 characters.' | ||
| )} | ||
| sx={{ mb: 2 }} | ||
| /> |
Comment on lines
+1470
to
+1494
| <Autocomplete | ||
| options={existingIdentities} | ||
| getOptionLabel={option => `${option.name} (${option.clientId})`} | ||
| onChange={(_e, value) => { | ||
| if (value) { | ||
| const saName = getServiceAccountName( | ||
| containerConfig.config.appName || 'app' | ||
| ); | ||
| containerConfig.setConfig(c => ({ | ||
| ...c, | ||
| workloadIdentityClientId: value.clientId, | ||
| workloadIdentityServiceAccount: saName, | ||
| })); | ||
| } | ||
| }} | ||
| renderInput={params => ( | ||
| <TextField | ||
| {...params} | ||
| label={t('Select managed identity')} | ||
| helperText={t( | ||
| 'Choose an existing user-assigned managed identity. You will still need to create federated credentials manually.' | ||
| )} | ||
| /> | ||
| )} | ||
| /> |
Comment on lines
+69
to
+80
| return ` | ||
| - Before applying manifests, create a Kubernetes ServiceAccount for workload identity: | ||
| \`kubectl apply -f - <<EOF | ||
| apiVersion: v1 | ||
| kind: ServiceAccount | ||
| metadata: | ||
| name: ${saName} | ||
| namespace: \${{ inputs.namespace }} | ||
| annotations: | ||
| azure.workload.identity/client-id: "${cc.workloadIdentityClientId}" | ||
| EOF\` | ||
| - In the Deployment manifest, set \`spec.template.metadata.labels["azure.workload.identity/use"]: "true"\` and \`spec.template.spec.serviceAccountName: ${saName}\` |
Comment on lines
+146
to
+149
| if (config.acrLoginServer) { | ||
| // Extract ACR name from login server (e.g., "myregistry.azurecr.io" → "myregistry") | ||
| secrets.AZURE_ACR_NAME = config.acrLoginServer.split('.')[0]; | ||
| } |
Comment on lines
+38
to
+105
| export async function ensureIdentityWithRoles( | ||
| config: EnsureIdentityWithRolesConfig | ||
| ): Promise<IdentitySetupResult> { | ||
| const { | ||
| subscriptionId, | ||
| resourceGroup, | ||
| identityResourceGroup, | ||
| identityName, | ||
| clusterName, | ||
| acrResourceId, | ||
| isManagedNamespace = false, | ||
| namespaceName, | ||
| azureRbacEnabled, | ||
| onStatusChange, | ||
| } = config; | ||
|
|
||
| // Steps 1-3: Ensure RG + identity | ||
| const identity = await ensureIdentityAndResourceGroup({ | ||
| subscriptionId, | ||
| resourceGroup, | ||
| identityResourceGroup, | ||
| identityName, | ||
| onStatusChange, | ||
| }); | ||
|
|
||
| // Step 4: Compute and assign required roles | ||
| onStatusChange('assigning-roles'); | ||
|
|
||
| let managedNamespaceResourceId: string | undefined; | ||
| if (isManagedNamespace && namespaceName) { | ||
| const nsResult = await getManagedNamespaceResourceId({ | ||
| clusterName, | ||
| resourceGroup, | ||
| namespaceName, | ||
| subscriptionId, | ||
| }); | ||
| if (!nsResult.success || !nsResult.resourceId) { | ||
| throw new Error(nsResult.error ?? 'Failed to get managed namespace resource ID'); | ||
| } | ||
| managedNamespaceResourceId = nsResult.resourceId; | ||
| } | ||
|
|
||
| const roles = computeRequiredRoles({ | ||
| subscriptionId, | ||
| resourceGroup, | ||
| clusterName, | ||
| acrResourceId, | ||
| isManagedNamespace, | ||
| managedNamespaceResourceId, | ||
| azureRbacEnabled, | ||
| }); | ||
|
|
||
| const roleResult = await assignRolesToIdentity({ | ||
| principalId: identity.principalId, | ||
| subscriptionId, | ||
| roles, | ||
| }); | ||
|
|
||
| if (!roleResult.success) { | ||
| const failedRoles = roleResult.results | ||
| .filter(r => !r.success) | ||
| .map(r => `${r.role}: ${r.error}`) | ||
| .join('; '); | ||
| throw new Error(`Failed to assign roles: ${failedRoles}`); | ||
| } | ||
|
|
||
| return identity; | ||
| } |
Comment on lines
+169
to
+175
| const wiEnabled = config.enableWorkloadIdentity && config.workloadIdentityClientId; | ||
| const saName = config.workloadIdentityServiceAccount || `${name}-sa`; | ||
|
|
||
| const workloadIdentityLabel = wiEnabled ? `\n azure.workload.identity/use: "true"` : ''; | ||
|
|
||
| const serviceAccountYaml = wiEnabled ? `\n serviceAccountName: ${saName}` : ''; | ||
|
|
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
Secretresource, and referenced viavalueFrom.secretKeyRefin the Deployment manifestServiceAccountwithazure.workload.identity/client-idannotation plus pod labels/serviceAccountName on the DeploymentType of Change
Related Issues
Closes #467
Changes Made
Secrets support
ContainerConfig.envVarstype withisSecret: booleanConfigureContainer.tsxyamlGenerator.tsto emit aSecretresource andsecretKeyRefentries for secret env varsextractContainerConfig.tsto round-tripsecretKeyRefentriesWorkload Identity
getAksOidcIssuerUrl,listManagedIdentities,createK8sFederatedCredentialtoaz-cli.tsuseDeployWorkloadIdentityhook (adapts existing GitHub Pipeline pattern)yamlGenerator.tsto emitServiceAccount, pod label, andserviceAccountNameClusterDeployCardthroughDeployWizardtoConfigureContainerIdentity role assignment fixes
assignRolesToIdentity()— generalized multi-role assignment function replacing the single hardcoded rolecomputeRequiredRoles()in newidentityRoles.ts— computes the correct role set based on namespace type (normal vs managed), ACR presence, and Azure RBAC statusensureIdentityAndResourceGroup()into sharedidentitySetup.ts— eliminates ~80% duplication between the two identity hooksgetManagedNamespaceResourceId()andbuildClusterScope()helpers toaz-cli.tsACR support in pipeline wizard
AcrSelectorcomponent — lists existing ACRs, supports creating new registries, shows a warning if skippedcreateContainerRegistry()toaz-cli.tsacrResourceIdandacrLoginServertoPipelineConfigaz acr buildinstructions when ACR is configuredAZURE_ACR_NAMEwhen ACR is selectedPipeline integration
generateWorkloadIdentityWorkflowInstructionstoagentTemplates.tsTesting
Test Cases
identityRoles.test.ts(10 tests),identitySetup.test.ts(8 tests)Checklist
Performance Impact
Reviewer Notes
Secretresource is placed first in the generated YAML so it exists before the Deployment that references itassignRoleToIdentity()function is preserved for backward compatibility but should be removed once all callers are migratedgetClusterCapabilities()already returnsazureRbacEnabled— callers just need to thread it through to the identity hooks