From b1f2d59cbe4524f151081d38898c5764c97f6c42 Mon Sep 17 00:00:00 2001 From: Anna Williamson Date: Tue, 28 Jul 2026 13:46:43 -0700 Subject: [PATCH 01/12] feat(azure): sync Key Vault to AKS via External Secrets Operator Install ESO (workload-identity to Key Vault) and a ClusterSecretStore on AKS when external_secrets_enabled is set, so Key Vault secrets sync into native k8s Secrets. team-operator is unchanged. Also fix CreateSecretIfNotExists quoting string values, and add a migration guide. --- docs/guides/external-secrets-aks.md | 153 +++++++++++++++++++++++++++ lib/azure/secretstore.go | 24 ++++- lib/azure/secretstore_test.go | 25 +++++ lib/steps/clusters.go | 12 +++ lib/steps/clusters_azure.go | 155 +++++++++++++++++++++++++++- lib/types/workload.go | 12 +++ 6 files changed, 378 insertions(+), 3 deletions(-) create mode 100644 docs/guides/external-secrets-aks.md diff --git a/docs/guides/external-secrets-aks.md b/docs/guides/external-secrets-aks.md new file mode 100644 index 00000000..e4818085 --- /dev/null +++ b/docs/guides/external-secrets-aks.md @@ -0,0 +1,153 @@ +# External Secrets on AKS (Azure Key Vault → Kubernetes) + +This guide describes how PTD syncs Azure Key Vault secrets into native Kubernetes +Secrets on AKS using the [External Secrets Operator (ESO)](https://external-secrets.io), +and the one-time procedure for migrating an **existing** Azure workload cluster onto +this model. + +## Overview + +On AKS, ESO replaces the previous approach of hand-applying Kubernetes Secrets (or +baking Key Vault values into Secrets at deploy time). It is the AKS counterpart to +the AWS Secrets Store CSI driver. team-operator is **unaware** of ESO — it continues +to read native Secrets by name (`SecretType: kubernetes`), so no operator changes are +required. + +ESO is installed by the `clusters` step when a cluster sets: + +```yaml +clusters: + "": + external_secrets_enabled: true +``` + +The step deploys the ESO controller into `posit-team-system` (authenticated to Key +Vault via workload identity) and creates a cluster-scoped `ClusterSecretStore` named +`azure-keyvault` pointing at the workload vault (`kv-ptd-`). + +## Key Vault naming convention + +Product secrets are stored in Key Vault as **1:1 entries** (not JSON blobs) named: + +``` +-- +``` + +- `` — the workload (target) name, e.g. the value returned by `Target.Name()`. +- `` — the site name (e.g. `main`). +- `` — the Kubernetes Secret key the workload expects (e.g. `dev-db-password`). + +Example (illustrative): `--dev-db-password`. + +A per-site `ExternalSecret` selects everything under `^--` and rewrites +the key to strip that prefix, so the resulting Secret keys match exactly what +team-operator reads: + +```yaml +apiVersion: external-secrets.io/v1 +kind: ExternalSecret +spec: + refreshInterval: 1m + secretStoreRef: + kind: ClusterSecretStore + name: azure-keyvault + target: + name: --posit-team # the Secret team-operator reads + creationPolicy: Owner + dataFrom: + - find: + name: + regexp: "^--" + rewrite: + - regexp: + source: "^--(.*)" + target: "$1" +``` + +**The `--` prefix (not a bare `-` prefix) is required**: the +infrastructure secrets (`-mimir-auth`, `-postgres-admin-secret`, +`-grafana-postgres-admin-secret`, `--postgres-grafana-user`) +also start with the compound name, and must **not** be swept into the product Secret. +Those remain deploy-time (Pulumi) managed for now. + +> AWS stores the same data as a **single JSON blob** per site +> (`-.posit.team`); the ESO equivalent there uses `dataFrom.extract` +> on that one secret rather than `find`. + +## Which secrets go in Key Vault + +Per site, the target Secret's keys come from two sources: + +| Source | Keys | Populated by | +| --- | --- | --- | +| **Code-generated** | `dev-db-password`, `keycloak-db-user`, `keycloak-db-password`, `pkg-db-password`, `pkg-secret-key`, `pub-db-password`, `pub-secret-key` | `secrets.NewSiteSecret` at the `bootstrap` step | +| **External / manual** | `dev-license`, `pub-license`, `pkg-license`, and (OIDC sites only) `dev-admin-token`, `dev-user-token` | created by hand / the license workflow | + +Notes: +- `dev-admin-token` / `dev-user-token` are Workbench API tokens that team-operator + only **reads** (mounted read-only), and only when Workbench auth is OIDC. They are + never generated by the operator, so storing them in Key Vault is safe. +- `home-auth-map` is deprecated and is **not** synced. + +## Secret value format + +Key Vault values must be stored as **raw strings** — no surrounding JSON quotes and no +trailing newline — because ESO syncs the bytes verbatim into the Kubernetes Secret. A +quoted or newline-padded value would break consumers (e.g. a DB password carrying +literal `"` characters). The `bootstrap` step stores strings verbatim; when setting +values by hand, always use `--file` (see below) rather than `--value`. + +## Migrating an existing cluster + +For a cluster that is **already running** with live Secrets, the **cluster is the +source of truth**. Do not let `bootstrap` regenerate the code-generated secrets under +the new names — it would create fresh random values that don't match the live database +and app state. Instead, seed the new-named Key Vault entries from the live cluster +first (`CreateSecretIfNotExists` then no-ops). + +For each existing Azure workload cluster: + +1. **Enumerate** the site's live Secret keys (the `--posit-team` Secret + and any separate product Secrets team-operator reads). +2. **Seed Key Vault from the cluster.** For every key — both code-generated and + external — copy the live value into `--`. Copy exact bytes via + a temp file so there is no quoting, no newline stripping, and no plaintext in the + process arguments: + ```bash + umask 077; tmp=$(mktemp) + ptd workon -- kubectl get secret -n posit-team \ + -o jsonpath="{.data.}" | base64 -d > "$tmp" + az keyvault secret set --vault-name --name "--" \ + --file "$tmp" --encoding utf-8 >/dev/null + rm -P "$tmp" + ``` + Include licenses, and (OIDC sites) the Workbench `dev-admin-token` / `dev-user-token`. +3. **Verify** each entry matches the cluster by hash (never print plaintext): + ```bash + # Key Vault side + az keyvault secret show --vault-name --name "--" \ + -o json | jq -j '.value' | shasum -a 256 | cut -c1-16 + # Cluster side + ptd workon -- kubectl get secret -n posit-team \ + -o jsonpath="{.data.}" | base64 -d | shasum -a 256 | cut -c1-16 + ``` + The two hashes must match before proceeding. +4. **Enable and apply**: set `external_secrets_enabled: true` on the cluster and run + `ptd ensure --only-steps clusters` (preview first). +5. **Confirm the sync**: the `ExternalSecret` should report `Ready=True` / + `SecretSynced`, and the reproduced Secret should match the original key-for-key + (hash each key as in step 3). +6. **Clean up**: once verified, delete the old-named Key Vault entries (e.g. the + `-` names and any `-lic` license names) that the new convention + replaces. + +### Greenfield clusters + +New clusters built after the naming change need no migration: `bootstrap` generates the +code-generated secrets under the correct names, and only the **external** secrets +(licenses and, for OIDC sites, the Workbench tokens) must be added to Key Vault by hand. + +## See also + +- External Secrets Operator docs: +- Azure Key Vault provider: diff --git a/lib/azure/secretstore.go b/lib/azure/secretstore.go index 2d2254b2..6366136b 100644 --- a/lib/azure/secretstore.go +++ b/lib/azure/secretstore.go @@ -102,14 +102,34 @@ func (s *SecretStore) CreateSecretIfNotExists(ctx context.Context, credentials t return err } - mSecret, err := json.Marshal(secret) + value, err := encodeSecretValue(secret) if err != nil { return err } if !s.SecretExists(ctx, azureCreds, secretName) { - return createSecret(ctx, azureCreds, s.vaultName, secretName, string(mSecret)) + return createSecret(ctx, azureCreds, s.vaultName, secretName, value) } return } + +// encodeSecretValue renders a secret payload for storage in Azure Key Vault. +// +// Key Vault stores plain string values, and these values are now synced verbatim +// into Kubernetes Secrets by the External Secrets Operator. A string is therefore +// stored as-is: json.Marshal would wrap it in literal double quotes (e.g. +// `"p4ssw0rd"`), which corrupts the value once synced — a DB password would carry +// the quote characters and fail auth, and cookie-signing keys would mismatch. +// Non-string values (structs/maps) are still JSON-encoded to preserve the existing +// blob behavior (e.g. the {fqdn,username,password} database secret). +func encodeSecretValue(secret any) (string, error) { + if str, ok := secret.(string); ok { + return str, nil + } + b, err := json.Marshal(secret) + if err != nil { + return "", err + } + return string(b), nil +} diff --git a/lib/azure/secretstore_test.go b/lib/azure/secretstore_test.go index 140f969b..bccf43d4 100644 --- a/lib/azure/secretstore_test.go +++ b/lib/azure/secretstore_test.go @@ -66,6 +66,31 @@ func TestSecretStoreCreateSecretIfNotExists(t *testing.T) { // We can't easily test the remaining cases without mocking Azure functions } +func TestEncodeSecretValue(t *testing.T) { + // Strings must be stored verbatim — NOT json-quoted — so values sync + // cleanly into k8s Secrets via External Secrets. Regression test for the + // bug where product secrets (e.g. main-dev-db-password) were stored as + // `"value"` with literal surrounding quotes. + t.Run("string stored verbatim without quotes", func(t *testing.T) { + v, err := encodeSecretValue("p4ssw0rd") + assert.NoError(t, err) + assert.Equal(t, "p4ssw0rd", v) + }) + + t.Run("string with special characters is untouched", func(t *testing.T) { + v, err := encodeSecretValue("a\"b\nc") + assert.NoError(t, err) + assert.Equal(t, "a\"b\nc", v) + }) + + // Non-string payloads keep the JSON-blob behavior. + t.Run("struct/map is JSON-encoded", func(t *testing.T) { + v, err := encodeSecretValue(map[string]string{"key": "value"}) + assert.NoError(t, err) + assert.Equal(t, `{"key":"value"}`, v) + }) +} + func TestSecretStoreGetSecretValue(t *testing.T) { secretStore := NewSecretStore("region", "vault") ctx := context.Background() diff --git a/lib/steps/clusters.go b/lib/steps/clusters.go index 32fdfb5b..d0075349 100644 --- a/lib/steps/clusters.go +++ b/lib/steps/clusters.go @@ -27,10 +27,22 @@ const ( azRoleReader = "acdd72a7-3385-48ef-bd42-f606fba81ae7" azRoleDNSZoneContributor = "befefa01-2a29-4197-83a8-272ff33ce314" azRoleStorageAccountContributor = "17d1049b-9a84-46fb-8f53-869881c3d3ab" + // azRoleKeyVaultSecretsUser grants read access to Key Vault secret contents. + // Built-in "Key Vault Secrets User" role. Used by the External Secrets Operator + // identity to read secrets from the workload Key Vault (RBAC-authorized vault). + azRoleKeyVaultSecretsUser = "4633458b-17de-408a-b874-0445c86b69e6" // Azure K8s namespaces for CertManager and Traefik clustersCertManagerNamespace = "cert-manager" clustersTraefikNamespace = "traefik" + // clustersExternalSecretsSA is the ESO controller service account name (matches + // the Helm chart's controller service account) that is federated to the + // workload-identity managed identity for Key Vault access. It lives in + // posit-team-system alongside team-operator. + clustersExternalSecretsSA = "external-secrets" + // clustersExternalSecretsStoreName is the ClusterSecretStore name pointing at + // the workload Key Vault. ExternalSecret resources reference this store. + clustersExternalSecretsStoreName = "azure-keyvault" ) // ClustersStep deploys the per-cluster resources (IAM roles, K8s operators, etc.) diff --git a/lib/steps/clusters_azure.go b/lib/steps/clusters_azure.go index f8c6adcf..906ed1fa 100644 --- a/lib/steps/clusters_azure.go +++ b/lib/steps/clusters_azure.go @@ -32,8 +32,10 @@ import ( type azureClustersParams struct { compoundName string subscriptionID string + tenantID string region string resourceGroupName string + keyVaultName string clusters map[string]types.AzureWorkloadClusterConfig kubeconfigsByCluster map[string]string dnsForwardDomains []types.DNSForwardDomainConfig @@ -191,8 +193,10 @@ func (s *ClustersStep) runAzureInlineGo(ctx context.Context, creds types.Credent params := azureClustersParams{ compoundName: s.DstTarget.Name(), subscriptionID: azTarget.SubscriptionID(), + tenantID: azTarget.TenantID(), region: s.DstTarget.Region(), resourceGroupName: azTarget.ResourceGroupName(), + keyVaultName: azTarget.VaultName(), clusters: cfg.Clusters, kubeconfigsByCluster: kubeconfigsByCluster, dnsForwardDomains: cfg.Network.DnsForwardDomains, @@ -353,7 +357,7 @@ func azureClustersDeploy(ctx *pulumi.Context, _ types.Target, params azureCluste "tag": pulumi.String(azureTeamOpImgTag), } } - _, err = helmv3.NewRelease(ctx, fmt.Sprintf("%s-%s-team-operator", name, release), &helmv3.ReleaseArgs{ + teamOpHelm, err := helmv3.NewRelease(ctx, fmt.Sprintf("%s-%s-team-operator", name, release), &helmv3.ReleaseArgs{ Name: pulumi.String("team-operator"), Chart: pulumi.String("oci://ghcr.io/posit-dev/charts/team-operator"), Version: pulumi.String(azureTeamOpChartVersion), @@ -664,6 +668,155 @@ func azureClustersDeploy(ctx *pulumi.Context, _ types.Target, params azureCluste } } + // ── External Secrets Operator (optional, AKS only) ───────────────────── + // Syncs Azure Key Vault secrets into native k8s Secrets via a + // ClusterSecretStore authenticated with workload identity. This is the AKS + // counterpart to the AWS Secrets Store CSI driver. team-operator consumes + // the resulting native Secrets unchanged (SecretType: kubernetes) and is + // unaware of ESO. The ExternalSecret resources that map specific Key Vault + // keys to named Secrets are authored per-workload via custom_k8s_resources/ + // (they reference the ClusterSecretStore created here by name). + if clusterCfg.ExternalSecretsEnabled { + esoVersion := clusterCfg.Components.ResolveAzureComponents().ExternalSecretsVersion + + // Managed identity for the ESO controller. + esoIdentityName := fmt.Sprintf("id-%s-%s-external-secrets", name, release) + esoIdentity, err := azmanagedidentity.NewUserAssignedIdentity(ctx, + esoIdentityName, + &azmanagedidentity.UserAssignedIdentityArgs{ + ResourceGroupName: pulumi.String(params.resourceGroupName), + Location: pulumi.String(params.region), + Tags: buildAzureRequiredTags(name, params.resourceTags), + }) + if err != nil { + return fmt.Errorf("clusters: failed to create external-secrets identity for %s: %w", release, err) + } + + // Key Vault Secrets User role scoped to the workload Key Vault. + // The vault is RBAC-authorized, so this role assignment (not an access + // policy) is what grants ESO read access to secret contents. + keyVaultScope := fmt.Sprintf( + "/subscriptions/%s/resourceGroups/%s/providers/Microsoft.KeyVault/vaults/%s", + params.subscriptionID, params.resourceGroupName, params.keyVaultName) + _, err = azauthorization.NewRoleAssignment(ctx, + fmt.Sprintf("%s-%s-kv-secrets-user-external-secrets", name, release), + &azauthorization.RoleAssignmentArgs{ + PrincipalId: esoIdentity.PrincipalId, + PrincipalType: pulumi.StringPtr("ServicePrincipal"), + RoleDefinitionId: pulumi.String(fmt.Sprintf("/providers/Microsoft.Authorization/roleDefinitions/%s", azRoleKeyVaultSecretsUser)), + Scope: pulumi.String(keyVaultScope), + }, pulumi.Parent(esoIdentity)) + if err != nil { + return fmt.Errorf("clusters: failed to create external-secrets key vault role for %s: %w", release, err) + } + + // ESO runs in posit-team-system, alongside team-operator. That namespace is + // created by the team-operator Helm release (CreateNamespace: true — i.e. + // created only if it does not already exist). We depend on that release so + // the namespace exists before these resources, rather than declaring a + // second Pulumi resource that would contend with team-operator for + // ownership of the namespace. + + // ESO controller ServiceAccount (annotated for workload identity). + // The Helm release below is configured with serviceAccount.create=false + // and this name, so it binds its controller Deployment to this SA. + esoSA, err := corev1.NewServiceAccount(ctx, + fmt.Sprintf("%s-%s-external-secrets-sa", name, release), + &corev1.ServiceAccountArgs{ + Metadata: &metav1.ObjectMetaArgs{ + Name: pulumi.String(clustersExternalSecretsSA), + Namespace: pulumi.String(clustersPositTeamSystemNamespace), + Annotations: pulumi.StringMap{ + "azure.workload.identity/client-id": esoIdentity.ClientId, + "azure.workload.identity/tenant-id": pulumi.String(params.tenantID), + }, + Labels: pulumi.StringMap{ + "azure.workload.identity/use": pulumi.String("true"), + }, + }, + }, k8sProviderOpt, + pulumi.DependsOn([]pulumi.Resource{teamOpHelm, esoIdentity})) + if err != nil { + return fmt.Errorf("clusters: failed to create external-secrets service account for %s: %w", release, err) + } + + // Federated identity credential binding the ESO SA to the identity. + if identityInfo != nil && identityInfo.OIDCIssuerURL != "" { + _, err = azmanagedidentity.NewFederatedIdentityCredential(ctx, + fmt.Sprintf("fedid-%s-%s-external-secrets", name, release), + &azmanagedidentity.FederatedIdentityCredentialArgs{ + ResourceName: esoIdentity.Name, + FederatedIdentityCredentialResourceName: pulumi.StringPtr(fmt.Sprintf("fedid-%s-%s-external-secrets", name, release)), + ResourceGroupName: pulumi.String(params.resourceGroupName), + Subject: pulumi.String(fmt.Sprintf("system:serviceaccount:%s:%s", clustersPositTeamSystemNamespace, clustersExternalSecretsSA)), + Issuer: pulumi.String(identityInfo.OIDCIssuerURL), + Audiences: pulumi.StringArray{pulumi.String("api://AzureADTokenExchange")}, + }, pulumi.Parent(esoIdentity)) + if err != nil { + return fmt.Errorf("clusters: failed to create external-secrets federated identity credential for %s: %w", release, err) + } + } + + // External Secrets Operator Helm release. + esoHelm, err := helmv3.NewRelease(ctx, + fmt.Sprintf("%s-%s-external-secrets", name, release), + &helmv3.ReleaseArgs{ + Name: pulumi.String("external-secrets"), + Chart: pulumi.String("external-secrets"), + Version: pulumi.String(esoVersion), + Namespace: pulumi.String(clustersPositTeamSystemNamespace), + RepositoryOpts: &helmv3.RepositoryOptsArgs{ + Repo: pulumi.String("https://charts.external-secrets.io"), + }, + Atomic: pulumi.Bool(true), + Values: pulumi.Map{ + "installCRDs": pulumi.Bool(true), + "serviceAccount": pulumi.Map{ + "create": pulumi.Bool(false), + "name": pulumi.String(clustersExternalSecretsSA), + }, + "podLabels": pulumi.Map{ + "azure.workload.identity/use": pulumi.String("true"), + }, + }, + }, k8sProviderOpt, + pulumi.DependsOn([]pulumi.Resource{teamOpHelm, esoSA})) + if err != nil { + return fmt.Errorf("clusters: failed to create external-secrets helm release for %s: %w", release, err) + } + + // ClusterSecretStore pointing at the workload Key Vault. ExternalSecret + // resources reference this store by name to materialize native Secrets. + // Depends on the Helm release so the ESO CRDs are registered first. + _, err = apiextensions.NewCustomResource(ctx, + fmt.Sprintf("%s-%s-external-secrets-store", name, release), + &apiextensions.CustomResourceArgs{ + ApiVersion: pulumi.String("external-secrets.io/v1"), + Kind: pulumi.String("ClusterSecretStore"), + Metadata: &metav1.ObjectMetaArgs{ + Name: pulumi.String(clustersExternalSecretsStoreName), + }, + OtherFields: kubernetes.UntypedArgs{ + "spec": map[string]interface{}{ + "provider": map[string]interface{}{ + "azurekv": map[string]interface{}{ + "authType": "WorkloadIdentity", + "vaultUrl": fmt.Sprintf("https://%s.vault.azure.net", params.keyVaultName), + "serviceAccountRef": map[string]interface{}{ + "name": clustersExternalSecretsSA, + "namespace": clustersPositTeamSystemNamespace, + }, + }, + }, + }, + }, + }, k8sProviderOpt, + pulumi.DependsOn([]pulumi.Resource{esoHelm})) + if err != nil { + return fmt.Errorf("clusters: failed to create external-secrets cluster store for %s: %w", release, err) + } + } + // ── Traefik ──────────────────────────────────────────────────────────── // Python: AzureTraefik component name is "{compound_name}-traefik" (no release suffix). traefikSubName := fmt.Sprintf("%s-traefik", name) diff --git a/lib/types/workload.go b/lib/types/workload.go index 8c67cabb..e1aed5da 100644 --- a/lib/types/workload.go +++ b/lib/types/workload.go @@ -563,6 +563,15 @@ type AzureWorkloadClusterConfig struct { // UseLetsEncrypt controls whether CertManager is deployed for this cluster. UseLetsEncrypt bool `yaml:"use_lets_encrypt"` + // ExternalSecretsEnabled controls whether the External Secrets Operator (ESO) + // is deployed for this cluster. When true, ESO is installed and a + // ClusterSecretStore is created pointing at the workload's Azure Key Vault, + // authenticated via workload identity. ExternalSecret resources that map + // specific Key Vault keys to named k8s Secrets are authored per-workload via + // custom_k8s_resources/. AKS only; AWS continues to use the Secrets Store CSI + // driver. + ExternalSecretsEnabled bool `yaml:"external_secrets_enabled"` + UserNodePools []AzureUserNodePoolConfig `yaml:"user_node_pools"` // Optional: Root disk size for system node pool in GB (defaults to 128) @@ -591,6 +600,7 @@ type AzureWorkloadClusterConfig struct { type AzureWorkloadClusterComponentConfig struct { SecretStoreCsiDriverAzureProviderVersion string `yaml:"secret_store_csi_driver_azure_provider_version"` + ExternalSecretsVersion *string `yaml:"external_secrets_version"` AlloyVersion *string `yaml:"alloy_version"` ExternalDnsVersion *string `yaml:"external_dns_version"` GrafanaVersion *string `yaml:"grafana_version"` @@ -606,6 +616,7 @@ type AzureWorkloadClusterComponentConfig struct { // ResolvedAzureComponents is the result of resolving AzureWorkloadClusterComponentConfig with defaults applied. type ResolvedAzureComponents struct { AlloyVersion string + ExternalSecretsVersion string ExternalDnsVersion string GrafanaVersion string KubeStateMetricsVersion string @@ -619,6 +630,7 @@ type ResolvedAzureComponents struct { func (c *AzureWorkloadClusterComponentConfig) ResolveAzureComponents() ResolvedAzureComponents { return ResolvedAzureComponents{ AlloyVersion: resolveString(c.AlloyVersion, "0.12.6"), + ExternalSecretsVersion: resolveString(c.ExternalSecretsVersion, "2.8.0"), ExternalDnsVersion: resolveString(c.ExternalDnsVersion, "1.14.4"), GrafanaVersion: resolveString(c.GrafanaVersion, "7.0.14"), KubeStateMetricsVersion: resolveString(c.KubeStateMetricsVersion, "5.30.1"), From f28174c1d245ccfb473c391f13372e48f3458b76 Mon Sep 17 00:00:00 2001 From: Anna Williamson Date: Thu, 30 Jul 2026 13:24:27 -0700 Subject: [PATCH 02/12] feat(azure): name bootstrap site secrets -- Prefix Azure site secret Key Vault names with the compound (workload) name so they follow the -- convention that ESO selects per site via dataFrom.find. Greenfield only; existing clusters migrate per docs/guides/external-secrets-aks.md. --- lib/steps/bootstrap.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/steps/bootstrap.go b/lib/steps/bootstrap.go index a7a46787..d6189708 100644 --- a/lib/steps/bootstrap.go +++ b/lib/steps/bootstrap.go @@ -279,8 +279,13 @@ func (s *BootstrapStep) runAzure(ctx context.Context, c types.Credentials, _ str // Create a KeyVault secret for each populated field in the map since Azure Secret Provider // doesn't support json blobs, we must create a KV entry for each field. + // + // Names are prefixed with the compound (workload) name so they follow the + // -- convention that the External Secrets Operator + // selects per site via `dataFrom.find { name.regexp: "^--" }`. + // See docs/guides/external-secrets-aks.md. for fieldName, fieldValue := range secretMap { - fieldSecretName := fmt.Sprintf("%s-%s", siteName, fieldName) + fieldSecretName := fmt.Sprintf("%s-%s-%s", s.DstTarget.Name(), siteName, fieldName) fieldValueStr := fmt.Sprintf("%v", fieldValue) if fieldValueStr == "" { continue From f9108d96376fcc8d5bde23a2e7a6283023bf954f Mon Sep 17 00:00:00 2001 From: Anna Williamson Date: Thu, 30 Jul 2026 13:47:22 -0700 Subject: [PATCH 03/12] feat(azure): generate per-site ExternalSecret from Key Vault For each site, create an ExternalSecret using dataFrom.find on ^-- with a rewrite to strip the prefix, producing the native Secret team-operator reads. Introduce azureSiteSecretName so the Site CR's secret.vaultName and the ExternalSecret target share one source of truth (hyphen form, matching live). --- lib/steps/clusters_azure.go | 63 +++++++++++++++++++++++++++++++++++-- lib/steps/sites.go | 10 +++++- lib/steps/sites_test.go | 2 +- 3 files changed, 71 insertions(+), 4 deletions(-) diff --git a/lib/steps/clusters_azure.go b/lib/steps/clusters_azure.go index 906ed1fa..825dc3a1 100644 --- a/lib/steps/clusters_azure.go +++ b/lib/steps/clusters_azure.go @@ -49,6 +49,9 @@ type azureClustersParams struct { // siteDomains is always the per-site domains (one per site), independent of root_domain. // Used for Traefik Ingresses, which must be created for every site domain. siteDomains []string + // siteNames is the sorted list of site names (keys of cfg.Sites). Used to + // generate one ExternalSecret per site when external secrets are enabled. + siteNames []string // siteTLSSecrets maps a site domain to its optional per-host TLS secrets. // When a domain has a non-empty entry, the Traefik ingress terminates TLS // with those secrets instead of the default single wildcard secret. @@ -205,6 +208,7 @@ func (s *ClustersStep) runAzureInlineGo(ctx context.Context, creds types.Credent clusterIdentityByCluster: clusterIdentityByCluster, certManagerDomains: certManagerDomains, siteDomains: siteDomains, + siteNames: helpers.SortedKeys(cfg.Sites), siteTLSSecrets: siteTLSSecrets, thirdPartyTelemetryEnabled: cfg.ThirdPartyTelemetryEnabled == nil || *cfg.ThirdPartyTelemetryEnabled, workloadDir: filepath.Join(helpers.GetTargetsConfigPath(), helpers.WorkDir, s.DstTarget.Name()), @@ -323,7 +327,7 @@ func azureClustersDeploy(ctx *pulumi.Context, _ types.Target, params azureCluste } // posit-team namespace (created inside TeamOperator in Python) - _, err = corev1.NewNamespace(ctx, fmt.Sprintf("%s-%s-%s", name, release, clustersPositTeamNamespace), &corev1.NamespaceArgs{ + positTeamNs, err := corev1.NewNamespace(ctx, fmt.Sprintf("%s-%s-%s", name, release, clustersPositTeamNamespace), &corev1.NamespaceArgs{ Metadata: &metav1.ObjectMetaArgs{ Name: pulumi.String(clustersPositTeamNamespace), }, @@ -788,7 +792,7 @@ func azureClustersDeploy(ctx *pulumi.Context, _ types.Target, params azureCluste // ClusterSecretStore pointing at the workload Key Vault. ExternalSecret // resources reference this store by name to materialize native Secrets. // Depends on the Helm release so the ESO CRDs are registered first. - _, err = apiextensions.NewCustomResource(ctx, + esoStore, err := apiextensions.NewCustomResource(ctx, fmt.Sprintf("%s-%s-external-secrets-store", name, release), &apiextensions.CustomResourceArgs{ ApiVersion: pulumi.String("external-secrets.io/v1"), @@ -815,6 +819,61 @@ func azureClustersDeploy(ctx *pulumi.Context, _ types.Target, params azureCluste if err != nil { return fmt.Errorf("clusters: failed to create external-secrets cluster store for %s: %w", release, err) } + + // One ExternalSecret per site. Each selects that site's Key Vault entries + // (^--) and rewrites the key to strip that prefix, so the + // resulting native Secret's keys are exactly what team-operator reads + // (dev-db-password, dev-license, …). creationPolicy: Owner means ESO owns + // the whole Secret — every key must therefore have a Key Vault source. + // target.name comes from azureSiteSecretName, the same helper the Site CR + // uses for secret.vaultName, so the two can never drift. + for _, siteName := range params.siteNames { + sitePrefix := fmt.Sprintf("%s-%s-", name, siteName) + _, err = apiextensions.NewCustomResource(ctx, + fmt.Sprintf("%s-%s-%s-external-secret", name, release, siteName), + &apiextensions.CustomResourceArgs{ + ApiVersion: pulumi.String("external-secrets.io/v1"), + Kind: pulumi.String("ExternalSecret"), + Metadata: &metav1.ObjectMetaArgs{ + Name: pulumi.String(fmt.Sprintf("%s-secrets", siteName)), + Namespace: pulumi.String(clustersPositTeamNamespace), + }, + OtherFields: kubernetes.UntypedArgs{ + "spec": map[string]interface{}{ + "refreshInterval": "1h", + "secretStoreRef": map[string]interface{}{ + "kind": "ClusterSecretStore", + "name": clustersExternalSecretsStoreName, + }, + "target": map[string]interface{}{ + "name": azureSiteSecretName(name, siteName), + "creationPolicy": "Owner", + }, + "dataFrom": []interface{}{ + map[string]interface{}{ + "find": map[string]interface{}{ + "name": map[string]interface{}{ + "regexp": "^" + sitePrefix, + }, + }, + "rewrite": []interface{}{ + map[string]interface{}{ + "regexp": map[string]interface{}{ + "source": "^" + sitePrefix + "(.*)", + "target": "$1", + }, + }, + }, + }, + }, + }, + }, + }, k8sProviderOpt, + pulumi.DependsOn([]pulumi.Resource{esoStore, positTeamNs})) + if err != nil { + return fmt.Errorf("clusters: failed to create external secret for %s/%s: %w", release, siteName, err) + } + } } // ── Traefik ──────────────────────────────────────────────────────────── diff --git a/lib/steps/sites.go b/lib/steps/sites.go index 4a414f48..627a88f2 100644 --- a/lib/steps/sites.go +++ b/lib/steps/sites.go @@ -434,12 +434,20 @@ func azureSitesDeploy(ctx *pulumi.Context, _ types.Target, params azureSiteParam return nil } +// azureSiteSecretName returns the name of the native Kubernetes Secret that +// team-operator reads for a site on AKS (secret.type: kubernetes). It is also the +// target of the site's ExternalSecret (see clusters_azure.go), so both the Site CR +// and the ExternalSecret derive the name from this single helper and cannot drift. +func azureSiteSecretName(compoundName, siteName string) string { + return fmt.Sprintf("%s-%s-posit-team", compoundName, siteName) +} + func buildAzureSiteSpec( params azureSiteParams, release, siteName string, siteConfig types.SiteConfigSpec, ) map[string]interface{} { - siteSecretName := params.compoundName + "-" + siteName + ".posit.team" + siteSecretName := azureSiteSecretName(params.compoundName, siteName) workloadSecretName := params.compoundName + ".posit.team" return map[string]interface{}{ diff --git a/lib/steps/sites_test.go b/lib/steps/sites_test.go index 0d1a739c..c7a3460c 100644 --- a/lib/steps/sites_test.go +++ b/lib/steps/sites_test.go @@ -378,7 +378,7 @@ func TestBuildAzureSiteSpec(t *testing.T) { secret := spec["secret"].(map[string]interface{}) assert.Equal(t, "kubernetes", secret["type"]) - assert.Equal(t, "myworkload-main.posit.team", secret["vaultName"]) + assert.Equal(t, "myworkload-main-posit-team", secret["vaultName"]) workloadSecret := spec["workloadSecret"].(map[string]interface{}) assert.Equal(t, "kubernetes", workloadSecret["type"]) From a8829f7952a8d48bab7dfdbbb91151bc9ada2302 Mon Sep 17 00:00:00 2001 From: Anna Williamson Date: Fri, 31 Jul 2026 08:40:07 -0700 Subject: [PATCH 04/12] refactor(azure): hyphenate workload secret name for consistency workloadSecret.vaultName now uses azureWorkloadSecretName (-posit-team), matching the live cluster naming and the site secret convention. AWS naming unchanged. --- lib/steps/sites.go | 10 +++++++++- lib/steps/sites_test.go | 2 +- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/lib/steps/sites.go b/lib/steps/sites.go index 627a88f2..6a2578cf 100644 --- a/lib/steps/sites.go +++ b/lib/steps/sites.go @@ -442,13 +442,21 @@ func azureSiteSecretName(compoundName, siteName string) string { return fmt.Sprintf("%s-%s-posit-team", compoundName, siteName) } +// azureWorkloadSecretName returns the name of the native Kubernetes Secret that +// team-operator reads for workload-level values on AKS. Hyphenated to match the +// live cluster naming (and the site secret convention); AWS keeps its own +// ".posit.team" Secrets Manager naming in buildAWSSiteSpec. +func azureWorkloadSecretName(compoundName string) string { + return fmt.Sprintf("%s-posit-team", compoundName) +} + func buildAzureSiteSpec( params azureSiteParams, release, siteName string, siteConfig types.SiteConfigSpec, ) map[string]interface{} { siteSecretName := azureSiteSecretName(params.compoundName, siteName) - workloadSecretName := params.compoundName + ".posit.team" + workloadSecretName := azureWorkloadSecretName(params.compoundName) return map[string]interface{}{ "clusterDate": release, diff --git a/lib/steps/sites_test.go b/lib/steps/sites_test.go index c7a3460c..5372ed4b 100644 --- a/lib/steps/sites_test.go +++ b/lib/steps/sites_test.go @@ -382,7 +382,7 @@ func TestBuildAzureSiteSpec(t *testing.T) { workloadSecret := spec["workloadSecret"].(map[string]interface{}) assert.Equal(t, "kubernetes", workloadSecret["type"]) - assert.Equal(t, "myworkload.posit.team", workloadSecret["vaultName"]) + assert.Equal(t, "myworkload-posit-team", workloadSecret["vaultName"]) ppm := spec["packageManager"].(map[string]interface{}) azureFiles := ppm["azureFiles"].(map[string]interface{}) From f767cf683efe5042d5db07be936ccf316b870ad6 Mon Sep 17 00:00:00 2001 From: Anna Williamson Date: Fri, 31 Jul 2026 09:06:10 -0700 Subject: [PATCH 05/12] feat(azure): preserve empty token keys in site ExternalSecret dev-admin-token and dev-user-token have empty values and therefore cannot be stored in Key Vault or matched by dataFrom.find. Emit them as empty literals via target.template with mergePolicy: Merge so the synced Secret keeps the same shape as the hand-applied one. --- lib/steps/clusters_azure.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/lib/steps/clusters_azure.go b/lib/steps/clusters_azure.go index 825dc3a1..324d0392 100644 --- a/lib/steps/clusters_azure.go +++ b/lib/steps/clusters_azure.go @@ -848,6 +848,19 @@ func azureClustersDeploy(ctx *pulumi.Context, _ types.Target, params azureCluste "target": map[string]interface{}{ "name": azureSiteSecretName(name, siteName), "creationPolicy": "Owner", + // Key Vault cannot store an empty value, so these keys have no + // Key Vault source and cannot come from `find`. They are emitted + // as empty literals to preserve the shape of the existing Secret + // (team-operator mounts them only for OIDC Workbench auth, and + // their live values are empty). mergePolicy: Merge keeps the + // find results and adds these on top. + "template": map[string]interface{}{ + "mergePolicy": "Merge", + "data": map[string]interface{}{ + "dev-admin-token": "", + "dev-user-token": "", + }, + }, }, "dataFrom": []interface{}{ map[string]interface{}{ From 60ccc76b8c40423dfdd2ac4461bb84cb134b2c0d Mon Sep 17 00:00:00 2001 From: Anna Williamson Date: Fri, 31 Jul 2026 09:23:09 -0700 Subject: [PATCH 06/12] docs(azure): document Key Vault secret ownership and cleanup steps Add tables for which Key Vault secrets PTD creates vs which must be created by hand (licenses, Workbench tokens, workload main-database-url, grafana-postgres-admin-secret, dockerhub creds), the reserved 'workload' site name, the Kubernetes Secrets that must stay out of ESO, and a post-migration cleanup checklist. Cross-reference from CONFIGURATION.md and the docs index. --- docs/CONFIGURATION.md | 31 ++++++++ docs/README.md | 1 + docs/guides/external-secrets-aks.md | 112 ++++++++++++++++++++++++---- 3 files changed, 131 insertions(+), 13 deletions(-) diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 887f343e..88a51bf8 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -407,8 +407,39 @@ clusters: **Default:** `false` (safety checks are respected during upgrades) +## Secret Management Options (Azure) + +### external_secrets_enabled + +Deploys the External Secrets Operator (ESO) to an AKS cluster and generates a per-site +`ExternalSecret`, so Azure Key Vault secrets sync into native Kubernetes Secrets instead +of being applied by hand. AKS only — AWS uses the Secrets Store CSI driver. + +```yaml +clusters: + "20250115": + external_secrets_enabled: true + components: + external_secrets_version: "2.8.0" # optional; chart version +``` + +Key Vault secrets follow the `--` convention; the site's +`ExternalSecret` selects `^--` and strips that prefix so the resulting +Secret keys match what team-operator reads. + +**Before enabling on an existing cluster**, the Key Vault entries must already exist under +the new names and match the live cluster values — otherwise the `ExternalSecret` (which +owns the target Secret) will reproduce it incompletely. Some Key Vault secrets are created +by PTD code and some must be created by hand. + +**Default:** `false` + +See [External Secrets on AKS](guides/external-secrets-aks.md) for the secret-ownership +tables, the migration procedure, and the post-migration cleanup checklist. + ## See Also - [Getting Started](GETTING_STARTED.md) - [CLI Reference](cli/PTD_CLI_REFERENCE.md) +- [External Secrets on AKS](guides/external-secrets-aks.md) - [Examples](../examples/) diff --git a/docs/README.md b/docs/README.md index 90925820..e01b5e06 100644 --- a/docs/README.md +++ b/docs/README.md @@ -23,6 +23,7 @@ Welcome to the Posit Team Dedicated (PTD) documentation. ### Infrastructure - [Kubernetes Guide](infrastructure/kubernetes.md) - Kubernetes-specific documentation +- [External Secrets on AKS](guides/external-secrets-aks.md) - Key Vault → Kubernetes Secret sync, secret ownership (code vs. hand-created), and the existing-cluster migration procedure ### Misc - [Known Issues](KNOWN_ISSUES.md) - Known issues and rough edges diff --git a/docs/guides/external-secrets-aks.md b/docs/guides/external-secrets-aks.md index e4818085..d559dbf1 100644 --- a/docs/guides/external-secrets-aks.md +++ b/docs/guides/external-secrets-aks.md @@ -74,21 +74,76 @@ Those remain deploy-time (Pulumi) managed for now. > (`-.posit.team`); the ESO equivalent there uses `dataFrom.extract` > on that one secret rather than `find`. -## Which secrets go in Key Vault +## Key Vault secret ownership -Per site, the target Secret's keys come from two sources: +Every Key Vault secret is either **created by PTD code** or **created by hand/CLI**. +Anything in the hand-created column will not exist on a new workload until someone adds +it, and it is never regenerated — so it must be seeded during a migration and recreated +during a rebuild. -| Source | Keys | Populated by | +### Created by PTD code + +| Key Vault secret | Step | Notes | +| --- | --- | --- | +| `--dev-db-password` | `bootstrap` | random, `secrets.NewSiteSecret` | +| `--keycloak-db-user` | `bootstrap` | derived from site name | +| `--keycloak-db-password` | `bootstrap` | random | +| `--pkg-db-password` | `bootstrap` | random | +| `--pkg-secret-key` | `bootstrap` | `rskey` generated | +| `--pub-db-password` | `bootstrap` | random | +| `--pub-secret-key` | `bootstrap` | `rskey` generated | +| `-postgres-admin-secret` | `persistent` | JSON `{fqdn,username,password}` | +| `-mimir-auth` | `persistent` | random password | +| `--postgres-grafana-user` | `postgres_config` | JSON `{database,password,role}`, per cluster | + +### Created by hand / CLI (never written by code) + +| Key Vault secret | Consumed by | Notes | | --- | --- | --- | -| **Code-generated** | `dev-db-password`, `keycloak-db-user`, `keycloak-db-password`, `pkg-db-password`, `pkg-secret-key`, `pub-db-password`, `pub-secret-key` | `secrets.NewSiteSecret` at the `bootstrap` step | -| **External / manual** | `dev-license`, `pub-license`, `pkg-license`, and (OIDC sites only) `dev-admin-token`, `dev-user-token` | created by hand / the license workflow | +| `--dev-license` | Workbench | license workflow | +| `--pub-license` | Connect | license workflow | +| `--pkg-license` | Package Manager | license workflow | +| `--dev-admin-token` | Workbench (OIDC only) | often empty; see below | +| `--dev-user-token` | Workbench (OIDC only) | often empty; see below | +| `-workload-main-database-url` | team-operator (workload secret) | see "Workload-level secrets" | +| `-grafana-postgres-admin-secret` | `postgres_config`, `helm` steps | **read-only in code** — the steps fail/warn if absent | +| `ptd-dockerhub-username` | ACR pull-through cache | shared, not per-workload | +| `ptd-dockerhub-oat` | ACR pull-through cache | shared, not per-workload | Notes: -- `dev-admin-token` / `dev-user-token` are Workbench API tokens that team-operator - only **reads** (mounted read-only), and only when Workbench auth is OIDC. They are - never generated by the operator, so storing them in Key Vault is safe. +- `dev-admin-token` / `dev-user-token` are Workbench API tokens that team-operator only + **reads** (mounted read-only), and only when Workbench auth is OIDC. Key Vault cannot + store an empty value, so when they are unused the site's ExternalSecret emits them as + empty literals via `target.template` (`mergePolicy: Merge`) to preserve the Secret's + shape. - `home-auth-map` is deprecated and is **not** synced. +### Workload-level secrets + +The Site CR also references a **workload** Secret (`workloadSecret.vaultName` → +`-posit-team`) holding `main-database-url`. On AWS the `persistent` step writes +this automatically; that code path is **AWS-only**, so on Azure it is hand-created. + +Its Key Vault entries use a `-workload-` prefix — **not** +`-`. A field named `main-database-url` under the bare compound prefix +would produce `-main-database-url`, which collides with the `^-main-` +site selector for a site named `main` (the default) and would be swept into the site +Secret. **`workload` is therefore a reserved site name.** + +> There is currently no ExternalSecret generated for the workload Secret — it remains +> hand-applied. Seeding `-workload-main-database-url` prepares for that. + +### Kubernetes Secrets NOT sourced from Key Vault + +Do not attempt to bring these under ESO: + +| Secret | Created by | +| --- | --- | +| `-connect-key`, `-packagemanager-key`, `-workbench-key`, `-workbench-config` | **team-operator** generates and owns these (keys, launcher PEM, DSN config). Leave them alone. | +| `azure-storage-account--secret` | `clusters` step, from the Azure Storage API | +| `external-dns/azure-config-file`, `grafana/grafana-db-url`, `alloy/mimir-auth` | `helm` step — Pulumi reads Key Vault and writes a *transformed* value (e.g. a connection string), so these are not 1:1 syncs | +| `-postgres-admin-secret` (in-cluster) | hand-applied today; could later use `dataFrom.extract` on the JSON blob | + ## Secret value format Key Vault values must be stored as **raw strings** — no surrounding JSON quotes and no @@ -137,15 +192,46 @@ For each existing Azure workload cluster: 5. **Confirm the sync**: the `ExternalSecret` should report `Ready=True` / `SecretSynced`, and the reproduced Secret should match the original key-for-key (hash each key as in step 3). -6. **Clean up**: once verified, delete the old-named Key Vault entries (e.g. the - `-` names and any `-lic` license names) that the new convention - replaces. +6. **Clean up**: see the checklist below. + +### Post-migration cleanup checklist + +Seed the new names *in parallel* and leave the old ones in place during the migration — +reverting is then just "delete the new entries". Once the ExternalSecret is confirmed +healthy and the reproduced Secret matches key-for-key, clean up: + +- [ ] **Old-named Key Vault product secrets** — the pre-migration `-` entries + (e.g. `main-dev-db-password`) that `--` replaces. +- [ ] **Old-named license entries** — the `-lic` spellings (e.g. `main-dev-lic`) replaced + by `--dev-license`. +- [ ] **Any test/scratch Key Vault secrets** created while validating the sync (e.g. a + throwaway `…-findtest-*` prefix or a single-value sync probe). +- [ ] **Any test ExternalSecrets and their target Secrets** in the cluster. Delete the + **ExternalSecret first** — otherwise ESO immediately recreates the Secret on its + next refresh and it looks like the delete failed. +- [ ] **The `external-secrets` namespace**, if the cluster was first deployed with ESO + there before it moved to `posit-team-system`. Pulumi removes it on the next + `clusters` apply; confirm it is gone. +- [ ] **Verify nothing else matched the selector** — list the Key Vault entries matching + `^--` and confirm each one is an intended product key: + ```bash + az keyvault secret list --vault-name \ + --query "[?starts_with(name,'--')].name" -o tsv + ``` + +Azure Key Vault deletes are **soft deletes**: the entries remain recoverable (and their +names reserved) for the vault's retention period. Purge only if a name must be reused +immediately. ### Greenfield clusters New clusters built after the naming change need no migration: `bootstrap` generates the -code-generated secrets under the correct names, and only the **external** secrets -(licenses and, for OIDC sites, the Workbench tokens) must be added to Key Vault by hand. +code-generated secrets under the correct names. Everything in +[Created by hand / CLI](#created-by-hand--cli-never-written-by-code) must still be added +to Key Vault manually — notably the three licenses, the workload +`main-database-url`, and `-grafana-postgres-admin-secret` (the `postgres_config` +and `helm` steps only ever *read* that one, so a missing entry surfaces as a step failure +or a warning rather than being created for you). ## See also From 7ba4fca8591c4fd7ee96fb6273cc8b19ae48cbac Mon Sep 17 00:00:00 2001 From: Anna Williamson Date: Fri, 31 Jul 2026 15:25:44 -0700 Subject: [PATCH 07/12] refactor: trim verbose comments in favor of doc references Reduce the ESO-related inline commentary from ~87 to ~39 lines, keeping only non-obvious rationale (RBAC-vs-access-policy, namespace ownership, CRD ordering, empty-value handling) and pointing at docs/guides/external-secrets-aks.md for the rest. --- lib/azure/secretstore.go | 11 ++----- lib/azure/secretstore_test.go | 5 +--- lib/steps/bootstrap.go | 7 ++--- lib/steps/clusters.go | 12 ++------ lib/steps/clusters_azure.go | 54 +++++++++-------------------------- lib/steps/sites.go | 12 +++----- lib/types/workload.go | 12 ++++---- 7 files changed, 32 insertions(+), 81 deletions(-) diff --git a/lib/azure/secretstore.go b/lib/azure/secretstore.go index 6366136b..f0a7cffb 100644 --- a/lib/azure/secretstore.go +++ b/lib/azure/secretstore.go @@ -115,14 +115,9 @@ func (s *SecretStore) CreateSecretIfNotExists(ctx context.Context, credentials t } // encodeSecretValue renders a secret payload for storage in Azure Key Vault. -// -// Key Vault stores plain string values, and these values are now synced verbatim -// into Kubernetes Secrets by the External Secrets Operator. A string is therefore -// stored as-is: json.Marshal would wrap it in literal double quotes (e.g. -// `"p4ssw0rd"`), which corrupts the value once synced — a DB password would carry -// the quote characters and fail auth, and cookie-signing keys would mismatch. -// Non-string values (structs/maps) are still JSON-encoded to preserve the existing -// blob behavior (e.g. the {fqdn,username,password} database secret). +// Strings are stored verbatim: External Secrets syncs Key Vault values byte-for-byte +// into Kubernetes Secrets, so json.Marshal's surrounding quotes would corrupt them. +// Non-strings are still JSON-encoded (e.g. the {fqdn,username,password} DB secret). func encodeSecretValue(secret any) (string, error) { if str, ok := secret.(string); ok { return str, nil diff --git a/lib/azure/secretstore_test.go b/lib/azure/secretstore_test.go index bccf43d4..35c90bd8 100644 --- a/lib/azure/secretstore_test.go +++ b/lib/azure/secretstore_test.go @@ -67,10 +67,7 @@ func TestSecretStoreCreateSecretIfNotExists(t *testing.T) { } func TestEncodeSecretValue(t *testing.T) { - // Strings must be stored verbatim — NOT json-quoted — so values sync - // cleanly into k8s Secrets via External Secrets. Regression test for the - // bug where product secrets (e.g. main-dev-db-password) were stored as - // `"value"` with literal surrounding quotes. + // Regression: strings were stored json-quoted, corrupting synced values. t.Run("string stored verbatim without quotes", func(t *testing.T) { v, err := encodeSecretValue("p4ssw0rd") assert.NoError(t, err) diff --git a/lib/steps/bootstrap.go b/lib/steps/bootstrap.go index d6189708..13af413d 100644 --- a/lib/steps/bootstrap.go +++ b/lib/steps/bootstrap.go @@ -279,11 +279,8 @@ func (s *BootstrapStep) runAzure(ctx context.Context, c types.Credentials, _ str // Create a KeyVault secret for each populated field in the map since Azure Secret Provider // doesn't support json blobs, we must create a KV entry for each field. - // - // Names are prefixed with the compound (workload) name so they follow the - // -- convention that the External Secrets Operator - // selects per site via `dataFrom.find { name.regexp: "^--" }`. - // See docs/guides/external-secrets-aks.md. + // Names follow --, the convention External Secrets + // selects per site. See docs/guides/external-secrets-aks.md. for fieldName, fieldValue := range secretMap { fieldSecretName := fmt.Sprintf("%s-%s-%s", s.DstTarget.Name(), siteName, fieldName) fieldValueStr := fmt.Sprintf("%v", fieldValue) diff --git a/lib/steps/clusters.go b/lib/steps/clusters.go index d0075349..b118420d 100644 --- a/lib/steps/clusters.go +++ b/lib/steps/clusters.go @@ -27,21 +27,15 @@ const ( azRoleReader = "acdd72a7-3385-48ef-bd42-f606fba81ae7" azRoleDNSZoneContributor = "befefa01-2a29-4197-83a8-272ff33ce314" azRoleStorageAccountContributor = "17d1049b-9a84-46fb-8f53-869881c3d3ab" - // azRoleKeyVaultSecretsUser grants read access to Key Vault secret contents. - // Built-in "Key Vault Secrets User" role. Used by the External Secrets Operator - // identity to read secrets from the workload Key Vault (RBAC-authorized vault). + // Built-in "Key Vault Secrets User" (getSecret + readMetadata). azRoleKeyVaultSecretsUser = "4633458b-17de-408a-b874-0445c86b69e6" // Azure K8s namespaces for CertManager and Traefik clustersCertManagerNamespace = "cert-manager" clustersTraefikNamespace = "traefik" - // clustersExternalSecretsSA is the ESO controller service account name (matches - // the Helm chart's controller service account) that is federated to the - // workload-identity managed identity for Key Vault access. It lives in - // posit-team-system alongside team-operator. + // ESO controller SA (in posit-team-system) federated for Key Vault access. clustersExternalSecretsSA = "external-secrets" - // clustersExternalSecretsStoreName is the ClusterSecretStore name pointing at - // the workload Key Vault. ExternalSecret resources reference this store. + // ClusterSecretStore that ExternalSecret resources reference by name. clustersExternalSecretsStoreName = "azure-keyvault" ) diff --git a/lib/steps/clusters_azure.go b/lib/steps/clusters_azure.go index 324d0392..da2b0098 100644 --- a/lib/steps/clusters_azure.go +++ b/lib/steps/clusters_azure.go @@ -49,8 +49,7 @@ type azureClustersParams struct { // siteDomains is always the per-site domains (one per site), independent of root_domain. // Used for Traefik Ingresses, which must be created for every site domain. siteDomains []string - // siteNames is the sorted list of site names (keys of cfg.Sites). Used to - // generate one ExternalSecret per site when external secrets are enabled. + // siteNames is the sorted list of site names, for per-site ExternalSecrets. siteNames []string // siteTLSSecrets maps a site domain to its optional per-host TLS secrets. // When a domain has a non-empty entry, the Traefik ingress terminates TLS @@ -673,17 +672,11 @@ func azureClustersDeploy(ctx *pulumi.Context, _ types.Target, params azureCluste } // ── External Secrets Operator (optional, AKS only) ───────────────────── - // Syncs Azure Key Vault secrets into native k8s Secrets via a - // ClusterSecretStore authenticated with workload identity. This is the AKS - // counterpart to the AWS Secrets Store CSI driver. team-operator consumes - // the resulting native Secrets unchanged (SecretType: kubernetes) and is - // unaware of ESO. The ExternalSecret resources that map specific Key Vault - // keys to named Secrets are authored per-workload via custom_k8s_resources/ - // (they reference the ClusterSecretStore created here by name). + // Syncs Key Vault into native k8s Secrets; team-operator is unaware of ESO. + // See docs/guides/external-secrets-aks.md. if clusterCfg.ExternalSecretsEnabled { esoVersion := clusterCfg.Components.ResolveAzureComponents().ExternalSecretsVersion - // Managed identity for the ESO controller. esoIdentityName := fmt.Sprintf("id-%s-%s-external-secrets", name, release) esoIdentity, err := azmanagedidentity.NewUserAssignedIdentity(ctx, esoIdentityName, @@ -696,9 +689,8 @@ func azureClustersDeploy(ctx *pulumi.Context, _ types.Target, params azureCluste return fmt.Errorf("clusters: failed to create external-secrets identity for %s: %w", release, err) } - // Key Vault Secrets User role scoped to the workload Key Vault. - // The vault is RBAC-authorized, so this role assignment (not an access - // policy) is what grants ESO read access to secret contents. + // The vault is RBAC-authorized, so access is granted by role assignment + // rather than an access policy. keyVaultScope := fmt.Sprintf( "/subscriptions/%s/resourceGroups/%s/providers/Microsoft.KeyVault/vaults/%s", params.subscriptionID, params.resourceGroupName, params.keyVaultName) @@ -714,16 +706,9 @@ func azureClustersDeploy(ctx *pulumi.Context, _ types.Target, params azureCluste return fmt.Errorf("clusters: failed to create external-secrets key vault role for %s: %w", release, err) } - // ESO runs in posit-team-system, alongside team-operator. That namespace is - // created by the team-operator Helm release (CreateNamespace: true — i.e. - // created only if it does not already exist). We depend on that release so - // the namespace exists before these resources, rather than declaring a - // second Pulumi resource that would contend with team-operator for - // ownership of the namespace. - - // ESO controller ServiceAccount (annotated for workload identity). - // The Helm release below is configured with serviceAccount.create=false - // and this name, so it binds its controller Deployment to this SA. + // ESO runs in posit-team-system, whose namespace the team-operator release + // already creates; DependsOn it rather than contending for ownership. + // The Helm release below sets serviceAccount.create=false to use this SA. esoSA, err := corev1.NewServiceAccount(ctx, fmt.Sprintf("%s-%s-external-secrets-sa", name, release), &corev1.ServiceAccountArgs{ @@ -761,7 +746,6 @@ func azureClustersDeploy(ctx *pulumi.Context, _ types.Target, params azureCluste } } - // External Secrets Operator Helm release. esoHelm, err := helmv3.NewRelease(ctx, fmt.Sprintf("%s-%s-external-secrets", name, release), &helmv3.ReleaseArgs{ @@ -789,9 +773,7 @@ func azureClustersDeploy(ctx *pulumi.Context, _ types.Target, params azureCluste return fmt.Errorf("clusters: failed to create external-secrets helm release for %s: %w", release, err) } - // ClusterSecretStore pointing at the workload Key Vault. ExternalSecret - // resources reference this store by name to materialize native Secrets. - // Depends on the Helm release so the ESO CRDs are registered first. + // DependsOn the Helm release so the ESO CRDs are registered first. esoStore, err := apiextensions.NewCustomResource(ctx, fmt.Sprintf("%s-%s-external-secrets-store", name, release), &apiextensions.CustomResourceArgs{ @@ -820,13 +802,9 @@ func azureClustersDeploy(ctx *pulumi.Context, _ types.Target, params azureCluste return fmt.Errorf("clusters: failed to create external-secrets cluster store for %s: %w", release, err) } - // One ExternalSecret per site. Each selects that site's Key Vault entries - // (^--) and rewrites the key to strip that prefix, so the - // resulting native Secret's keys are exactly what team-operator reads - // (dev-db-password, dev-license, …). creationPolicy: Owner means ESO owns - // the whole Secret — every key must therefore have a Key Vault source. - // target.name comes from azureSiteSecretName, the same helper the Site CR - // uses for secret.vaultName, so the two can never drift. + // One ExternalSecret per site: selects ^-- and strips the + // prefix to produce the keys team-operator reads. Owner policy prunes keys + // with no Key Vault source. See docs/guides/external-secrets-aks.md. for _, siteName := range params.siteNames { sitePrefix := fmt.Sprintf("%s-%s-", name, siteName) _, err = apiextensions.NewCustomResource(ctx, @@ -848,12 +826,8 @@ func azureClustersDeploy(ctx *pulumi.Context, _ types.Target, params azureCluste "target": map[string]interface{}{ "name": azureSiteSecretName(name, siteName), "creationPolicy": "Owner", - // Key Vault cannot store an empty value, so these keys have no - // Key Vault source and cannot come from `find`. They are emitted - // as empty literals to preserve the shape of the existing Secret - // (team-operator mounts them only for OIDC Workbench auth, and - // their live values are empty). mergePolicy: Merge keeps the - // find results and adds these on top. + // Key Vault cannot store empty values, so these are emitted as + // empty literals to preserve the Secret's shape. "template": map[string]interface{}{ "mergePolicy": "Merge", "data": map[string]interface{}{ diff --git a/lib/steps/sites.go b/lib/steps/sites.go index 6a2578cf..b762510e 100644 --- a/lib/steps/sites.go +++ b/lib/steps/sites.go @@ -434,18 +434,14 @@ func azureSitesDeploy(ctx *pulumi.Context, _ types.Target, params azureSiteParam return nil } -// azureSiteSecretName returns the name of the native Kubernetes Secret that -// team-operator reads for a site on AKS (secret.type: kubernetes). It is also the -// target of the site's ExternalSecret (see clusters_azure.go), so both the Site CR -// and the ExternalSecret derive the name from this single helper and cannot drift. +// azureSiteSecretName is the Secret team-operator reads for a site on AKS. Shared +// with the site's ExternalSecret target (clusters_azure.go) so the two cannot drift. func azureSiteSecretName(compoundName, siteName string) string { return fmt.Sprintf("%s-%s-posit-team", compoundName, siteName) } -// azureWorkloadSecretName returns the name of the native Kubernetes Secret that -// team-operator reads for workload-level values on AKS. Hyphenated to match the -// live cluster naming (and the site secret convention); AWS keeps its own -// ".posit.team" Secrets Manager naming in buildAWSSiteSpec. +// azureWorkloadSecretName is the workload-level equivalent. Hyphenated to match the +// live clusters; AWS keeps its own ".posit.team" naming in buildAWSSiteSpec. func azureWorkloadSecretName(compoundName string) string { return fmt.Sprintf("%s-posit-team", compoundName) } diff --git a/lib/types/workload.go b/lib/types/workload.go index e1aed5da..f6b5db40 100644 --- a/lib/types/workload.go +++ b/lib/types/workload.go @@ -563,13 +563,11 @@ type AzureWorkloadClusterConfig struct { // UseLetsEncrypt controls whether CertManager is deployed for this cluster. UseLetsEncrypt bool `yaml:"use_lets_encrypt"` - // ExternalSecretsEnabled controls whether the External Secrets Operator (ESO) - // is deployed for this cluster. When true, ESO is installed and a - // ClusterSecretStore is created pointing at the workload's Azure Key Vault, - // authenticated via workload identity. ExternalSecret resources that map - // specific Key Vault keys to named k8s Secrets are authored per-workload via - // custom_k8s_resources/. AKS only; AWS continues to use the Secrets Store CSI - // driver. + // ExternalSecretsEnabled deploys the External Secrets Operator and a per-site + // ExternalSecret, syncing Azure Key Vault into native k8s Secrets. AKS only; + // AWS uses the Secrets Store CSI driver. Requires the Key Vault entries to + // already exist under the expected names — see + // docs/guides/external-secrets-aks.md. ExternalSecretsEnabled bool `yaml:"external_secrets_enabled"` UserNodePools []AzureUserNodePoolConfig `yaml:"user_node_pools"` From 7490356040b9505893b8ae5763f7a93a6a06ebc5 Mon Sep 17 00:00:00 2001 From: Anna Williamson Date: Fri, 31 Jul 2026 15:36:29 -0700 Subject: [PATCH 08/12] fix(azure): address PR review feedback - EnsureWorkloadSecret now uses encodeSecretValue for consistency with CreateSecretIfNotExists - extract the always-empty site secret keys to a named var so the ExternalSecret template has one source of truth - reject a site named 'workload' when external secrets are enabled (collides with the reserved Key Vault prefix) - correct refreshInterval in the guide example to match the generated value (1h) --- docs/guides/external-secrets-aks.md | 2 +- lib/azure/secretstore.go | 5 ++++- lib/steps/clusters.go | 13 +++++++++++++ lib/steps/clusters_azure.go | 21 +++++++++++++++------ 4 files changed, 33 insertions(+), 8 deletions(-) diff --git a/docs/guides/external-secrets-aks.md b/docs/guides/external-secrets-aks.md index d559dbf1..dedacc69 100644 --- a/docs/guides/external-secrets-aks.md +++ b/docs/guides/external-secrets-aks.md @@ -47,7 +47,7 @@ team-operator reads: apiVersion: external-secrets.io/v1 kind: ExternalSecret spec: - refreshInterval: 1m + refreshInterval: 1h secretStoreRef: kind: ClusterSecretStore name: azure-keyvault diff --git a/lib/azure/secretstore.go b/lib/azure/secretstore.go index f0a7cffb..40f91ff1 100644 --- a/lib/azure/secretstore.go +++ b/lib/azure/secretstore.go @@ -61,7 +61,10 @@ func (s *SecretStore) EnsureWorkloadSecret(ctx context.Context, credentials type // Create a KeyVault secret for each populated field in the map since Azure Secret Provider // doesn't support json blobs, we must create a KV entry for each field. for fieldName, fieldValue := range secretMap { - fieldValueStr := fmt.Sprintf("%v", fieldValue) + fieldValueStr, err := encodeSecretValue(fieldValue) + if err != nil { + return fmt.Errorf("failed to encode field %s: %w", fieldName, err) + } if fieldValueStr == "" { continue } diff --git a/lib/steps/clusters.go b/lib/steps/clusters.go index b118420d..98285a51 100644 --- a/lib/steps/clusters.go +++ b/lib/steps/clusters.go @@ -37,8 +37,21 @@ const ( clustersExternalSecretsSA = "external-secrets" // ClusterSecretStore that ExternalSecret resources reference by name. clustersExternalSecretsStoreName = "azure-keyvault" + // clustersReservedWorkloadSiteName is reserved: workload-level Key Vault entries + // use the -workload- prefix, which a site of this name would collide + // with. See docs/guides/external-secrets-aks.md. + clustersReservedWorkloadSiteName = "workload" ) +// clustersAzureEmptySiteSecretKeys are site secret keys that are always empty, so +// Key Vault (which rejects empty values) holds no entry and `bootstrap` skips them. +// The site ExternalSecret emits them as empty literals to keep the Secret's shape. +// Keep in sync with the empty fields of secrets.NewSiteSecret. +var clustersAzureEmptySiteSecretKeys = []string{ + "dev-admin-token", + "dev-user-token", +} + // ClustersStep deploys the per-cluster resources (IAM roles, K8s operators, etc.) // for both AWS and Azure workloads. type ClustersStep struct { diff --git a/lib/steps/clusters_azure.go b/lib/steps/clusters_azure.go index da2b0098..d7b167ae 100644 --- a/lib/steps/clusters_azure.go +++ b/lib/steps/clusters_azure.go @@ -64,6 +64,16 @@ type azureClustersParams struct { rootDomain string } +// emptySiteSecretTemplateData renders clustersAzureEmptySiteSecretKeys as the +// ExternalSecret target.template data map. +func emptySiteSecretTemplateData() map[string]interface{} { + data := make(map[string]interface{}, len(clustersAzureEmptySiteSecretKeys)) + for _, k := range clustersAzureEmptySiteSecretKeys { + data[k] = "" + } + return data +} + // traefikIngressTLSEntry is a plain representation of a single Traefik ingress // `tls` entry (hosts + secret name). It is the pure, unit-testable form of the // Pulumi `tls` array built by traefikIngressTLS. @@ -806,6 +816,10 @@ func azureClustersDeploy(ctx *pulumi.Context, _ types.Target, params azureCluste // prefix to produce the keys team-operator reads. Owner policy prunes keys // with no Key Vault source. See docs/guides/external-secrets-aks.md. for _, siteName := range params.siteNames { + if siteName == clustersReservedWorkloadSiteName { + return fmt.Errorf("clusters: site name %q is reserved when external secrets are enabled: it collides with the %s-%s- Key Vault prefix", + siteName, name, clustersReservedWorkloadSiteName) + } sitePrefix := fmt.Sprintf("%s-%s-", name, siteName) _, err = apiextensions.NewCustomResource(ctx, fmt.Sprintf("%s-%s-%s-external-secret", name, release, siteName), @@ -826,14 +840,9 @@ func azureClustersDeploy(ctx *pulumi.Context, _ types.Target, params azureCluste "target": map[string]interface{}{ "name": azureSiteSecretName(name, siteName), "creationPolicy": "Owner", - // Key Vault cannot store empty values, so these are emitted as - // empty literals to preserve the Secret's shape. "template": map[string]interface{}{ "mergePolicy": "Merge", - "data": map[string]interface{}{ - "dev-admin-token": "", - "dev-user-token": "", - }, + "data": emptySiteSecretTemplateData(), }, }, "dataFrom": []interface{}{ From 80d69b93ef4f60f3a436c6b826851c0d346c052b Mon Sep 17 00:00:00 2001 From: Anna Williamson Date: Mon, 3 Aug 2026 14:08:20 -0700 Subject: [PATCH 09/12] fix(azure): use creationPolicy Orphan for site ExternalSecret Owner sets an ownerReference on the target Secret, so deleting the ExternalSecret (or the ESO CRDs) would cascade-delete a live product Secret. Orphan adopts, updates and prunes identically but omits the ownerReference. Verified on a test cluster: adoption, update-on-Key-Vault-change, pruning, and Secret survival after ExternalSecret deletion. --- docs/guides/external-secrets-aks.md | 21 ++++++++++++++++++++- lib/steps/clusters_azure.go | 9 ++++++--- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/docs/guides/external-secrets-aks.md b/docs/guides/external-secrets-aks.md index dedacc69..718631dd 100644 --- a/docs/guides/external-secrets-aks.md +++ b/docs/guides/external-secrets-aks.md @@ -53,7 +53,7 @@ spec: name: azure-keyvault target: name: --posit-team # the Secret team-operator reads - creationPolicy: Owner + creationPolicy: Orphan dataFrom: - find: name: @@ -74,6 +74,25 @@ Those remain deploy-time (Pulumi) managed for now. > (`-.posit.team`); the ESO equivalent there uses `dataFrom.extract` > on that one secret rather than `find`. +## Secret lifecycle policies + +The generated ExternalSecret uses `creationPolicy: Orphan` and the default +`deletionPolicy: Retain`. Together these mean the Kubernetes Secret is adopted, kept +updated, and pruned of keys with no Key Vault source — but it is never destroyed by ESO: + +| Event | Result | +| --- | --- | +| Key Vault value changes | Secret is updated on the next refresh | +| Key has no Key Vault source | Key is pruned from the Secret | +| Key Vault entry deleted | Secret is **retained** (`deletionPolicy: Retain`) | +| ExternalSecret deleted (e.g. `external_secrets_enabled: false`) | Secret is **retained** (`creationPolicy: Orphan` sets no `ownerReferences`) | + +`creationPolicy: Owner` is ESO's default and would otherwise be the natural choice, but +it sets an `ownerReference` on the Secret — so deleting the ExternalSecret, or removing +the ESO CRDs, would garbage-collect a live product Secret and break the workload. The +trade-off is that a Secret can outlive its ExternalSecret and go stale; deleting it is a +deliberate manual step. + ## Key Vault secret ownership Every Key Vault secret is either **created by PTD code** or **created by hand/CLI**. diff --git a/lib/steps/clusters_azure.go b/lib/steps/clusters_azure.go index d7b167ae..62fe5e19 100644 --- a/lib/steps/clusters_azure.go +++ b/lib/steps/clusters_azure.go @@ -813,8 +813,11 @@ func azureClustersDeploy(ctx *pulumi.Context, _ types.Target, params azureCluste } // One ExternalSecret per site: selects ^-- and strips the - // prefix to produce the keys team-operator reads. Owner policy prunes keys - // with no Key Vault source. See docs/guides/external-secrets-aks.md. + // prefix to produce the keys team-operator reads. Orphan policy still + // adopts, updates and prunes the Secret, but omits the ownerReference so + // deleting the ExternalSecret (e.g. disabling this feature) cannot + // cascade-delete a live product Secret. + // See docs/guides/external-secrets-aks.md. for _, siteName := range params.siteNames { if siteName == clustersReservedWorkloadSiteName { return fmt.Errorf("clusters: site name %q is reserved when external secrets are enabled: it collides with the %s-%s- Key Vault prefix", @@ -839,7 +842,7 @@ func azureClustersDeploy(ctx *pulumi.Context, _ types.Target, params azureCluste }, "target": map[string]interface{}{ "name": azureSiteSecretName(name, siteName), - "creationPolicy": "Owner", + "creationPolicy": "Orphan", "template": map[string]interface{}{ "mergePolicy": "Merge", "data": emptySiteSecretTemplateData(), From 0145f642ea61a3303ebca9207c1fe710a693c65f Mon Sep 17 00:00:00 2001 From: Anna Williamson Date: Tue, 4 Aug 2026 14:52:41 -0700 Subject: [PATCH 10/12] fix(azure): gate site secret rename behind external_secrets_enabled The -- Key Vault naming is now used only when a cluster in the workload has external secrets enabled; otherwise bootstrap keeps the historical - name. This leaves vaults on unmigrated Azure workloads untouched when this change merges. Adds AnyClusterExternalSecretsEnabled and azureSiteSecretKeyVaultName with unit tests, and documents that the flag must be set before the first bootstrap run on a new workload. --- docs/guides/external-secrets-aks.md | 19 +++++++++++++++++-- lib/steps/bootstrap.go | 25 ++++++++++++++++++++++--- lib/steps/bootstrap_test.go | 24 ++++++++++++++++++++++++ lib/types/workload.go | 13 +++++++++++++ lib/types/workload_test.go | 22 ++++++++++++++++++++++ 5 files changed, 98 insertions(+), 5 deletions(-) create mode 100644 lib/steps/bootstrap_test.go diff --git a/docs/guides/external-secrets-aks.md b/docs/guides/external-secrets-aks.md index 718631dd..648dcef8 100644 --- a/docs/guides/external-secrets-aks.md +++ b/docs/guides/external-secrets-aks.md @@ -39,6 +39,13 @@ Product secrets are stored in Key Vault as **1:1 entries** (not JSON blobs) name Example (illustrative): `--dev-db-password`. +> **This convention is gated on `external_secrets_enabled`.** The `bootstrap` step uses +> it only when at least one cluster in the workload has external secrets enabled; +> otherwise it keeps the historical `-` name so vaults on unmigrated +> workloads are left untouched. **Set the flag before running `bootstrap` on a new +> workload** — otherwise bootstrap writes legacy names that the site's ExternalSecret +> selector will not match. + A per-site `ExternalSecret` selects everything under `^--` and rewrites the key to strip that prefix, so the resulting Secret keys match exactly what team-operator reads: @@ -244,8 +251,16 @@ immediately. ### Greenfield clusters -New clusters built after the naming change need no migration: `bootstrap` generates the -code-generated secrets under the correct names. Everything in +New clusters need no migration, provided `external_secrets_enabled: true` is set +**before** the first `bootstrap` run — that is what selects the +`--` naming. `bootstrap` then generates the code-generated +secrets under the correct names. (If bootstrap already ran without the flag, the +secrets exist under `-`; because `CreateSecretIfNotExists` never +overwrites, enabling the flag and re-running creates a *second* set under the new +names with fresh random values — seed those from the cluster as in the migration +above, then delete the legacy entries.) + +Everything in [Created by hand / CLI](#created-by-hand--cli-never-written-by-code) must still be added to Key Vault manually — notably the three licenses, the workload `main-database-url`, and `-grafana-postgres-admin-secret` (the `postgres_config` diff --git a/lib/steps/bootstrap.go b/lib/steps/bootstrap.go index 13af413d..1d1a806e 100644 --- a/lib/steps/bootstrap.go +++ b/lib/steps/bootstrap.go @@ -151,6 +151,18 @@ func (s *BootstrapStep) runAws(ctx context.Context, creds types.Credentials, wor return nil } +// azureSiteSecretKeyVaultName returns the Key Vault name for one field of a site +// secret. With external secrets enabled the name is prefixed with the compound +// (workload) name so the site's ExternalSecret can select it via +// `^--`; otherwise the historical - name is kept so +// vaults on unmigrated workloads are left untouched. +func azureSiteSecretKeyVaultName(compoundName, siteName, fieldName string, externalSecretsEnabled bool) string { + if externalSecretsEnabled { + return fmt.Sprintf("%s-%s-%s", compoundName, siteName, fieldName) + } + return fmt.Sprintf("%s-%s", siteName, fieldName) +} + func (s *BootstrapStep) runAzure(ctx context.Context, c types.Credentials, _ string) error { azureCreds, err := azure.OnlyAzureCredentials(c) if err != nil { @@ -163,6 +175,9 @@ func (s *BootstrapStep) runAzure(ctx context.Context, c types.Credentials, _ str // resource group at creation time (below). These mirror the tags the persistent // step places on child resources. var resourceTags map[string]string + // externalSecretsEnabled selects the site secret Key Vault naming convention + // (see the site secrets loop below). + externalSecretsEnabled := false if rawConfig, cfgErr := helpers.ConfigForTarget(s.DstTarget); cfgErr != nil { // Don't fail bootstrap on this; the RG is still created (untagged), matching // the previous behavior. But warn so a missing/malformed config doesn't @@ -170,6 +185,7 @@ func (s *BootstrapStep) runAzure(ctx context.Context, c types.Credentials, _ str s.Log.Warn("could not load workload config for resource_tags; resource group will be created untagged", "err", cfgErr) } else if cfg, ok := rawConfig.(types.AzureWorkloadConfig); ok { resourceTags = cfg.ResourceTags + externalSecretsEnabled = cfg.AnyClusterExternalSecretsEnabled() } // Note: runAzure is only invoked for Azure workload targets, so the assertion // above always holds here. Control-room targets won't satisfy AzureWorkloadConfig @@ -279,10 +295,13 @@ func (s *BootstrapStep) runAzure(ctx context.Context, c types.Credentials, _ str // Create a KeyVault secret for each populated field in the map since Azure Secret Provider // doesn't support json blobs, we must create a KV entry for each field. - // Names follow --, the convention External Secrets - // selects per site. See docs/guides/external-secrets-aks.md. + // + // Only workloads with external secrets enabled use the + // -- convention that External Secrets selects per + // site; others keep - so unmigrated vaults are untouched. + // See docs/guides/external-secrets-aks.md. for fieldName, fieldValue := range secretMap { - fieldSecretName := fmt.Sprintf("%s-%s-%s", s.DstTarget.Name(), siteName, fieldName) + fieldSecretName := azureSiteSecretKeyVaultName(s.DstTarget.Name(), siteName, fieldName, externalSecretsEnabled) fieldValueStr := fmt.Sprintf("%v", fieldValue) if fieldValueStr == "" { continue diff --git a/lib/steps/bootstrap_test.go b/lib/steps/bootstrap_test.go new file mode 100644 index 00000000..7aca6d3b --- /dev/null +++ b/lib/steps/bootstrap_test.go @@ -0,0 +1,24 @@ +package steps + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestAzureSiteSecretKeyVaultName(t *testing.T) { + // With external secrets enabled the name carries the compound prefix so the + // site's ExternalSecret can select it via ^--. + t.Run("external secrets enabled uses compound prefix", func(t *testing.T) { + assert.Equal(t, + "myworkload-main-dev-db-password", + azureSiteSecretKeyVaultName("myworkload", "main", "dev-db-password", true)) + }) + + // Unmigrated workloads keep the historical name so their vaults are untouched. + t.Run("external secrets disabled keeps legacy name", func(t *testing.T) { + assert.Equal(t, + "main-dev-db-password", + azureSiteSecretKeyVaultName("myworkload", "main", "dev-db-password", false)) + }) +} diff --git a/lib/types/workload.go b/lib/types/workload.go index f6b5db40..94f04a6b 100644 --- a/lib/types/workload.go +++ b/lib/types/workload.go @@ -639,6 +639,19 @@ func (c *AzureWorkloadClusterComponentConfig) ResolveAzureComponents() ResolvedA } } +// AnyClusterExternalSecretsEnabled reports whether any cluster in the workload has +// external secrets enabled. The bootstrap step uses this to decide the Key Vault +// naming convention for site secrets, so unmigrated workloads are left untouched. +// See docs/guides/external-secrets-aks.md. +func (c AzureWorkloadConfig) AnyClusterExternalSecretsEnabled() bool { + for _, cluster := range c.Clusters { + if cluster.ExternalSecretsEnabled { + return true + } + } + return false +} + type SiteConfig struct { Spec SiteConfigSpec `json:"spec" yaml:"spec"` } diff --git a/lib/types/workload_test.go b/lib/types/workload_test.go index ca702c3e..91b36fa5 100644 --- a/lib/types/workload_test.go +++ b/lib/types/workload_test.go @@ -473,3 +473,25 @@ func TestUsesEksAccessEntries(t *testing.T) { }) } } + +func TestAnyClusterExternalSecretsEnabled(t *testing.T) { + t.Run("no clusters enabled", func(t *testing.T) { + cfg := AzureWorkloadConfig{Clusters: map[string]AzureWorkloadClusterConfig{ + "20250101": {}, + "20250201": {}, + }} + assert.False(t, cfg.AnyClusterExternalSecretsEnabled()) + }) + + t.Run("one of several enabled", func(t *testing.T) { + cfg := AzureWorkloadConfig{Clusters: map[string]AzureWorkloadClusterConfig{ + "20250101": {}, + "20250201": {ExternalSecretsEnabled: true}, + }} + assert.True(t, cfg.AnyClusterExternalSecretsEnabled()) + }) + + t.Run("no clusters at all", func(t *testing.T) { + assert.False(t, AzureWorkloadConfig{}.AnyClusterExternalSecretsEnabled()) + }) +} From ff11f22412ff34f188b431876aff9c36e5d1d066 Mon Sep 17 00:00:00 2001 From: Anna Williamson Date: Thu, 6 Aug 2026 08:22:24 -0700 Subject: [PATCH 11/12] fix(azure): retain External Secrets CRDs on rollback The chart renders CRDs as templates, so an Atomic rollback of a failed upgrade deletes them, cascade-deleting every ExternalSecret and ClusterSecretStore in the cluster (observed during testing). Set crds.annotations.helm.sh/resource-policy=keep, matching the team-operator release's crd.keep. Also document running --refresh after a failed apply, since a rollback can remove resources Pulumi still believes exist. --- docs/guides/external-secrets-aks.md | 13 +++++++++++++ lib/steps/clusters_azure.go | 9 +++++++++ 2 files changed, 22 insertions(+) diff --git a/docs/guides/external-secrets-aks.md b/docs/guides/external-secrets-aks.md index 648dcef8..4a64c6eb 100644 --- a/docs/guides/external-secrets-aks.md +++ b/docs/guides/external-secrets-aks.md @@ -100,6 +100,19 @@ the ESO CRDs, would garbage-collect a live product Secret and break the workload trade-off is that a Secret can outlive its ExternalSecret and go stale; deleting it is a deliberate manual step. +### CRDs are retained + +The chart renders its CRDs as ordinary templates, so a rollback of a failed `Atomic` +upgrade would delete them — and deleting a CRD cascade-deletes every `ExternalSecret` and +`ClusterSecretStore` in the cluster. The release therefore sets +`crds.annotations."helm.sh/resource-policy": keep` so Helm leaves the CRDs in place +(the team-operator release does the same via `crd.keep`). + +If a `clusters` apply involving ESO does fail, run +`ptd ensure --only-steps clusters --refresh` before re-applying: a rollback can +remove resources Pulumi still believes exist, and without a refresh they are not +recreated — leaving Secrets present but unmanaged. + ## Key Vault secret ownership Every Key Vault secret is either **created by PTD code** or **created by hand/CLI**. diff --git a/lib/steps/clusters_azure.go b/lib/steps/clusters_azure.go index 62fe5e19..e2ca157e 100644 --- a/lib/steps/clusters_azure.go +++ b/lib/steps/clusters_azure.go @@ -769,6 +769,15 @@ func azureClustersDeploy(ctx *pulumi.Context, _ types.Target, params azureCluste Atomic: pulumi.Bool(true), Values: pulumi.Map{ "installCRDs": pulumi.Bool(true), + // The chart renders CRDs as templates, so an Atomic rollback of a + // failed upgrade would delete them — cascade-deleting every + // ExternalSecret and ClusterSecretStore in the cluster. Retain them + // instead (the team-operator release does the same via crd.keep). + "crds": pulumi.Map{ + "annotations": pulumi.Map{ + "helm.sh/resource-policy": pulumi.String("keep"), + }, + }, "serviceAccount": pulumi.Map{ "create": pulumi.Bool(false), "name": pulumi.String(clustersExternalSecretsSA), From bcaffbebb264445f61978f352910316611880e4d Mon Sep 17 00:00:00 2001 From: Anna Williamson Date: Thu, 6 Aug 2026 08:42:32 -0700 Subject: [PATCH 12/12] fix(azure): address PR review comments - bootstrap: type-assert site secret field values instead of fmt.Sprintf("%v"), so a future non-string field fails loudly rather than being stored Go-formatted - use azRoleDefID for the Key Vault Secrets User assignment, consistent with the other role assignments in this file - document what breaks when the OIDC issuer is absent and the federated credential is skipped - explain why the always-empty site secret key list is curated rather than derived --- lib/steps/bootstrap.go | 8 +++++++- lib/steps/clusters.go | 7 ++++++- lib/steps/clusters_azure.go | 7 +++++-- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/lib/steps/bootstrap.go b/lib/steps/bootstrap.go index 1d1a806e..fac5ced1 100644 --- a/lib/steps/bootstrap.go +++ b/lib/steps/bootstrap.go @@ -302,7 +302,13 @@ func (s *BootstrapStep) runAzure(ctx context.Context, c types.Credentials, _ str // See docs/guides/external-secrets-aks.md. for fieldName, fieldValue := range secretMap { fieldSecretName := azureSiteSecretKeyVaultName(s.DstTarget.Name(), siteName, fieldName, externalSecretsEnabled) - fieldValueStr := fmt.Sprintf("%v", fieldValue) + // SiteSecret fields are all strings. Fail loudly rather than storing a + // Go-formatted value if that ever changes, since External Secrets syncs + // Key Vault values into Kubernetes Secrets byte-for-byte. + fieldValueStr, isStr := fieldValue.(string) + if !isStr { + return fmt.Errorf("site secret field %s has type %T, expected string", fieldName, fieldValue) + } if fieldValueStr == "" { continue } diff --git a/lib/steps/clusters.go b/lib/steps/clusters.go index 98285a51..cf7d09d9 100644 --- a/lib/steps/clusters.go +++ b/lib/steps/clusters.go @@ -46,7 +46,12 @@ const ( // clustersAzureEmptySiteSecretKeys are site secret keys that are always empty, so // Key Vault (which rejects empty values) holds no entry and `bootstrap` skips them. // The site ExternalSecret emits them as empty literals to keep the Secret's shape. -// Keep in sync with the empty fields of secrets.NewSiteSecret. +// +// This list is deliberately curated, not derived from secrets.SiteSecret: most fields +// that field-type inspection would report as empty (the licences, the chronicle and +// OIDC client secrets) DO have hand-created Key Vault entries and must come from +// `find`, while others (home-auth-map) are deprecated and intentionally dropped. +// Emitting empty literals for those would add spurious keys or mask real values. var clustersAzureEmptySiteSecretKeys = []string{ "dev-admin-token", "dev-user-token", diff --git a/lib/steps/clusters_azure.go b/lib/steps/clusters_azure.go index e2ca157e..23a9f7c7 100644 --- a/lib/steps/clusters_azure.go +++ b/lib/steps/clusters_azure.go @@ -709,7 +709,7 @@ func azureClustersDeploy(ctx *pulumi.Context, _ types.Target, params azureCluste &azauthorization.RoleAssignmentArgs{ PrincipalId: esoIdentity.PrincipalId, PrincipalType: pulumi.StringPtr("ServicePrincipal"), - RoleDefinitionId: pulumi.String(fmt.Sprintf("/providers/Microsoft.Authorization/roleDefinitions/%s", azRoleKeyVaultSecretsUser)), + RoleDefinitionId: pulumi.String(azRoleDefID(azRoleKeyVaultSecretsUser)), Scope: pulumi.String(keyVaultScope), }, pulumi.Parent(esoIdentity)) if err != nil { @@ -739,7 +739,10 @@ func azureClustersDeploy(ctx *pulumi.Context, _ types.Target, params azureCluste return fmt.Errorf("clusters: failed to create external-secrets service account for %s: %w", release, err) } - // Federated identity credential binding the ESO SA to the identity. + // Federated identity credential binding the ESO SA to the identity. Without + // it ESO installs but cannot authenticate to Key Vault: the + // ClusterSecretStore reports Invalid and Secrets silently go unmanaged. + // PTD enables the OIDC issuer on every AKS cluster, so this holds in practice. if identityInfo != nil && identityInfo.OIDCIssuerURL != "" { _, err = azmanagedidentity.NewFederatedIdentityCredential(ctx, fmt.Sprintf("fedid-%s-%s-external-secrets", name, release),