This PR fixes a few minor errors in scripts - #9
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates several Bash scripts across the AKS samples to correct Azure CLI flag usage, fix script execution issues, and tighten/complete Key Vault RBAC configuration so the Key Vault CSI driver samples work reliably (including in non-interactive/CI environments).
Changes:
- Fixes Azure CLI flag usage for PostgreSQL flexible server firewall rules and database commands.
- Fixes malformed shebangs and improves non-interactive
kubectl execusage/quoting in CSI driver sample scripts. - Enables Key Vault RBAC authorization and updates role assignments toward least-privilege access; adds a missing role assignment step for the Key Vault Secrets Provider add-on identity.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/01-user-assigned-managed-identity.sh | Adds role assignment for the Key Vault Secrets Provider add-on identity (but currently grants an overly privileged role and has incorrect exit codes in new error paths). |
| samples/web-app-postgresql-flexible-server/scripts/01-deploy-resources.sh | Corrects az postgres flexible-server flags to match the MySQL sample and current CLI expectations. |
| samples/key-vault-csi-driver/workload-identity/02-create-key-vault-and-secrets.sh | Enables RBAC authorization on Key Vault creation for RBAC-based data-plane access. |
| samples/key-vault-csi-driver/workload-identity/03-create-managed-identity.sh | Fixes shebang, switches to least-privilege Key Vault role, and fails early if OIDC issuer URL is missing. |
| samples/key-vault-csi-driver/workload-identity/04-create-secret-provider-class.sh | Fixes malformed shebang. |
| samples/key-vault-csi-driver/workload-identity/05-create-demo-pod.sh | Fixes malformed shebang. |
| samples/key-vault-csi-driver/workload-identity/06-list-secrets.sh | Removes interactive kubectl exec flags and improves quoting for CI/non-interactive shells. |
| samples/key-vault-csi-driver/user-assigned-managed-identity/02-create-key-vault-and-secrets.sh | Enables RBAC authorization on Key Vault creation for RBAC-based data-plane access. |
| samples/key-vault-csi-driver/user-assigned-managed-identity/03-create-role-assignment.sh | Fixes shebang and updates role assignment to least-privilege Key Vault role. |
| samples/key-vault-csi-driver/user-assigned-managed-identity/04-create-secret-provider-class.sh | Fixes malformed shebang. |
| samples/key-vault-csi-driver/user-assigned-managed-identity/05-create-demo-pod.sh | Fixes malformed shebang. |
| samples/key-vault-csi-driver/user-assigned-managed-identity/06-list-secrets.sh | Removes interactive kubectl exec flags and improves quoting for CI/non-interactive shells. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| echo "Successfully retrieved the objectId for the Azure Key Vault Secrets Provider identity in the [$aks_cluster_name] AKS cluster" | ||
| else | ||
| echo "Failed to retrieve the objectId for the Azure Key Vault Secrets Provider identity in the [$aks_cluster_name] AKS cluster" | ||
| exit |
| echo "Successfully retrieved the resourceId for the Azure Key Vault Secrets Provider identity in the [$aks_cluster_name] AKS cluster" | ||
| else | ||
| echo "Failed to retrieve the resourceId for the Azure Key Vault Secrets Provider identity in the [$aks_cluster_name] AKS cluster" | ||
| exit |
Comment on lines
+783
to
+787
| kv_secret_provider_managed_identity_name=$(basename $kv_secret_provider_managed_identity_resource_id) | ||
|
|
||
| # Assign the Key Vault Administrator role to the managed identity on the node resource group | ||
| role="Key Vault Administrator" | ||
| managed_identity_name="$kv_secret_provider_managed_identity_name" |
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.
Motivation
Addresses the GitHub Copilot review feedback on PR #8 (Azure Key Vault Provider for Secrets Store CSI Driver samples). The original scripts contained incorrect Azure CLI flags, malformed shebangs, overly permissive role assignments, missing RBAC configuration, and a missing role grant that would cause
SecretProviderClassmounts to fail with authorization errors.Changes
samples/web-app-postgresql-flexible-server/scripts/01-deploy-resources.sh: correctedaz postgres flexible-serverflags to match the MySQL sample —firewall-rule createnow uses--name/--rule-name, anddb show/db createuse--database-name.#/bin/bash→#!/bin/bash) in six CSI driver scripts:03-create-managed-identity.sh,04-create-secret-provider-class.sh,05-create-demo-pod.sh(workload-identity) and03-create-role-assignment.sh,04-create-secret-provider-class.sh,05-create-demo-pod.sh(user-assigned-managed-identity).--enable-rbac-authorization trueto both02-create-key-vault-and-secrets.shscripts so subsequent RBAC role assignments grant data-plane access.Key Vault Administratorwith the least-privilegeKey Vault Secrets Userrole inuser-assigned-managed-identity/03-create-role-assignment.shandworkload-identity/03-create-managed-identity.sh, corrected the misleading "node resource group" comments, and removed the redundantPRINCIPAL_ID="$PRINCIPAL_ID"assignment.AKS_OIDC_ISSUER_URLinworkload-identity/03-create-managed-identity.shto fail early with an actionable message when the OIDC issuer is not enabled.--stdin --ttyfromkubectl execand quoted variables/paths in both06-list-secrets.shscripts so they run in non-interactive/CI shells.scripts/01-user-assigned-managed-identity.sh: added a role assignment step granting theazureKeyvaultSecretsProvideradd-on managed identity theKey Vault Secrets Userrole on$key_vault_id, mirroring the system-assigned script.