From ebbe3830d0c6190168f90eebeea5ea4defd1ee18 Mon Sep 17 00:00:00 2001 From: Nicacio Oliveira Date: Wed, 20 May 2026 10:49:53 -0300 Subject: [PATCH 1/5] feat(buildsecrets): sync tenant build-secrets via ExternalSecret Adds a BuildSecretsReconciler that materialises an ExternalSecret per opted-in site namespace. The EE pulls `sites//build` from the cluster-local AWS Secrets Manager into a K8s Secret named `build-secrets`, which the builder Job already consumes via envFrom with optional:true (admin PR #3201). Opt-in is the namespace annotation `deco.sites/build-secrets-managed: enabled`. Removing it (or deleting the namespace) deletes the EE and ESO cascades the K8s Secret via `creationPolicy: Owner`. Encapsulates the sync mechanism behind a single buildsecrets package so swapping away from ESO touches one file. The Reconciler watches Namespace + ExternalSecret (unstructured) and re-reconciles on either. Force-sync recipes documented inline: - Per site: `kubectl annotate es build-secrets force-sync=$(date +%s) --overwrite` - All sites: filter by label `deco.sites/feature=build-secrets` --- .../clusterrole-operator-manager-role.yaml | 12 ++ cmd/main.go | 10 ++ config/rbac/role.yaml | 12 ++ internal/buildsecrets/externalsecret.go | 130 ++++++++++++++++++ internal/buildsecrets/externalsecret_test.go | 124 +++++++++++++++++ .../controller/buildsecrets_controller.go | 125 +++++++++++++++++ 6 files changed, 413 insertions(+) create mode 100644 internal/buildsecrets/externalsecret.go create mode 100644 internal/buildsecrets/externalsecret_test.go create mode 100644 internal/controller/buildsecrets_controller.go diff --git a/chart/templates/clusterrole-operator-manager-role.yaml b/chart/templates/clusterrole-operator-manager-role.yaml index f72f3fb..843372c 100644 --- a/chart/templates/clusterrole-operator-manager-role.yaml +++ b/chart/templates/clusterrole-operator-manager-role.yaml @@ -91,6 +91,18 @@ rules: - get - patch - update +- apiGroups: + - external-secrets.io + resources: + - externalsecrets + verbs: + - create + - delete + - get + - list + - patch + - update + - watch - apiGroups: - serving.knative.dev resources: diff --git a/cmd/main.go b/cmd/main.go index 699153c..0d81615 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -267,6 +267,16 @@ func main() { setupLog.Error(err, "unable to create controller", "controller", "Namespace") os.Exit(1) } + + bsReconciler := &controller.BuildSecretsReconciler{ + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + } + if err := bsReconciler.SetupWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create controller", "controller", "BuildSecrets") + os.Exit(1) + } + // Start Sentinel failover watcher if enabled and Sentinel is configured. // leaderElectedRunnable ensures only the active leader subscribes — prevents // redundant TriggerResyncAll calls from non-leader replicas. diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 879d81d..6a4e960 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -92,6 +92,18 @@ rules: - get - patch - update +- apiGroups: + - external-secrets.io + resources: + - externalsecrets + verbs: + - create + - delete + - get + - list + - patch + - update + - watch - apiGroups: - serving.knative.dev resources: diff --git a/internal/buildsecrets/externalsecret.go b/internal/buildsecrets/externalsecret.go new file mode 100644 index 0000000..61a849b --- /dev/null +++ b/internal/buildsecrets/externalsecret.go @@ -0,0 +1,130 @@ +/* +Copyright 2025. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 +*/ + +// Package buildsecrets owns the operator's interaction with the tenant +// build-time secrets sync. Today it materialises an `ExternalSecret` +// (external-secrets.io/v1) per opted-in site namespace; swapping the +// sync mechanism away from ESO touches only this file. +package buildsecrets + +import ( + "context" + "fmt" + + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +const ( + // SecretName is the K8s Secret materialised by ESO from AWS Secrets + // Manager. Builders in each site namespace consume it via envFrom with + // optional:true (admin PR #3201) so its absence is a no-op. + SecretName = "build-secrets" + + // ManagedByLabel + FeatureLabel let operators bulk-filter the EE for + // force-sync via `kubectl annotate es -l ... force-sync=$(date +%s)`. + ManagedByLabel = "deco.sites/managed-by" + FeatureLabel = "deco.sites/feature" + + refreshInterval = "1h" + clusterSecretStoreName = "aws-secrets-manager" + awsSecretsManagerKeyFmt = "sites/%s/build" +) + +// GVK is exported so the reconciler can register a Watches() on the same +// type without re-declaring the kind tuple. +var GVK = schema.GroupVersionKind{ + Group: "external-secrets.io", + Version: "v1", + Kind: "ExternalSecret", +} + +// Ensure creates or updates the ExternalSecret pulling `sites//build` +// from the cluster-local AWS Secrets Manager into the K8s Secret named +// `build-secrets`. Spec drift is corrected on every reconcile. +func Ensure(ctx context.Context, c client.Client, namespace, site string) error { + desired := newExternalSecret(namespace, site) + + existing := &unstructured.Unstructured{} + existing.SetGroupVersionKind(GVK) + key := types.NamespacedName{Name: SecretName, Namespace: namespace} + err := c.Get(ctx, key, existing) + switch { + case errors.IsNotFound(err): + return c.Create(ctx, desired) + case err != nil: + return fmt.Errorf("get externalsecret: %w", err) + } + + existing.Object["spec"] = desired.Object["spec"] + labels := existing.GetLabels() + if labels == nil { + labels = map[string]string{} + } + for k, v := range desired.GetLabels() { + labels[k] = v + } + existing.SetLabels(labels) + if err := c.Update(ctx, existing); err != nil { + return fmt.Errorf("update externalsecret: %w", err) + } + return nil +} + +// Remove deletes the ExternalSecret. ESO's `creationPolicy: Owner` makes +// the materialised K8s Secret cascade with it. Idempotent on NotFound. +func Remove(ctx context.Context, c client.Client, namespace string) error { + obj := &unstructured.Unstructured{} + obj.SetGroupVersionKind(GVK) + obj.SetName(SecretName) + obj.SetNamespace(namespace) + if err := c.Delete(ctx, obj); err != nil && !errors.IsNotFound(err) { + return fmt.Errorf("delete externalsecret: %w", err) + } + return nil +} + +func newExternalSecret(namespace, site string) *unstructured.Unstructured { + return &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": GVK.GroupVersion().String(), + "kind": GVK.Kind, + "metadata": map[string]interface{}{ + "name": SecretName, + "namespace": namespace, + "labels": map[string]interface{}{ + ManagedByLabel: "operator", + FeatureLabel: "build-secrets", + }, + }, + "spec": map[string]interface{}{ + "refreshInterval": refreshInterval, + "secretStoreRef": map[string]interface{}{ + "name": clusterSecretStoreName, + "kind": "ClusterSecretStore", + }, + "target": map[string]interface{}{ + "name": SecretName, + "creationPolicy": "Owner", + }, + "dataFrom": []interface{}{ + map[string]interface{}{ + "extract": map[string]interface{}{ + "key": fmt.Sprintf(awsSecretsManagerKeyFmt, site), + }, + }, + }, + }, + }, + } +} diff --git a/internal/buildsecrets/externalsecret_test.go b/internal/buildsecrets/externalsecret_test.go new file mode 100644 index 0000000..2c4d47e --- /dev/null +++ b/internal/buildsecrets/externalsecret_test.go @@ -0,0 +1,124 @@ +/* +Copyright 2025. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 +*/ + +package buildsecrets + +import ( + "context" + "testing" + + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func newClient(t *testing.T) client.Client { + t.Helper() + scheme := runtime.NewScheme() + return fake.NewClientBuilder().WithScheme(scheme).Build() +} + +func getExternalSecret(t *testing.T, c client.Client, namespace string) *unstructured.Unstructured { + t.Helper() + got := &unstructured.Unstructured{} + got.SetGroupVersionKind(GVK) + if err := c.Get(context.Background(), types.NamespacedName{Name: SecretName, Namespace: namespace}, got); err != nil { + t.Fatalf("Get externalsecret: %v", err) + } + return got +} + +func TestEnsureCreates(t *testing.T) { + c := newClient(t) + if err := Ensure(context.Background(), c, "sites-acme", "acme"); err != nil { + t.Fatalf("Ensure: %v", err) + } + + got := getExternalSecret(t, c, "sites-acme") + if v := got.GetLabels()[ManagedByLabel]; v != "operator" { + t.Fatalf("label %s = %q, want operator", ManagedByLabel, v) + } + if v := got.GetLabels()[FeatureLabel]; v != "build-secrets" { + t.Fatalf("label %s = %q, want build-secrets", FeatureLabel, v) + } + + dataFrom, _, _ := unstructured.NestedSlice(got.Object, "spec", "dataFrom") + if len(dataFrom) != 1 { + t.Fatalf("dataFrom length = %d, want 1", len(dataFrom)) + } + extract := dataFrom[0].(map[string]interface{})["extract"].(map[string]interface{}) + if got := extract["key"]; got != "sites/acme/build" { + t.Fatalf("extract.key = %q, want sites/acme/build", got) + } + target, _, _ := unstructured.NestedMap(got.Object, "spec", "target") + if got := target["creationPolicy"]; got != "Owner" { + t.Fatalf("creationPolicy = %q, want Owner", got) + } +} + +func TestEnsureIsIdempotent(t *testing.T) { + c := newClient(t) + ctx := context.Background() + if err := Ensure(ctx, c, "sites-acme", "acme"); err != nil { + t.Fatalf("first Ensure: %v", err) + } + if err := Ensure(ctx, c, "sites-acme", "acme"); err != nil { + t.Fatalf("second Ensure: %v", err) + } +} + +func TestEnsureCorrectsDrift(t *testing.T) { + c := newClient(t) + ctx := context.Background() + if err := Ensure(ctx, c, "sites-acme", "acme"); err != nil { + t.Fatalf("Ensure: %v", err) + } + + drifted := getExternalSecret(t, c, "sites-acme") + if err := unstructured.SetNestedField(drifted.Object, "999h", "spec", "refreshInterval"); err != nil { + t.Fatalf("SetNestedField: %v", err) + } + if err := c.Update(ctx, drifted); err != nil { + t.Fatalf("Update drift: %v", err) + } + + if err := Ensure(ctx, c, "sites-acme", "acme"); err != nil { + t.Fatalf("Ensure after drift: %v", err) + } + healed := getExternalSecret(t, c, "sites-acme") + got, _, _ := unstructured.NestedString(healed.Object, "spec", "refreshInterval") + if got != refreshInterval { + t.Fatalf("refreshInterval = %q, want %q (drift not corrected)", got, refreshInterval) + } +} + +func TestRemove(t *testing.T) { + c := newClient(t) + ctx := context.Background() + if err := Remove(ctx, c, "sites-acme"); err != nil { + t.Fatalf("Remove on empty: %v", err) + } + if err := Ensure(ctx, c, "sites-acme", "acme"); err != nil { + t.Fatalf("Ensure: %v", err) + } + if err := Remove(ctx, c, "sites-acme"); err != nil { + t.Fatalf("Remove: %v", err) + } + + got := &unstructured.Unstructured{} + got.SetGroupVersionKind(GVK) + err := c.Get(ctx, types.NamespacedName{Name: SecretName, Namespace: "sites-acme"}, got) + if !errors.IsNotFound(err) { + t.Fatalf("Get after Remove: err = %v, want NotFound", err) + } +} diff --git a/internal/controller/buildsecrets_controller.go b/internal/controller/buildsecrets_controller.go new file mode 100644 index 0000000..312adc4 --- /dev/null +++ b/internal/controller/buildsecrets_controller.go @@ -0,0 +1,125 @@ +/* +Copyright 2025. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 +*/ + +package controller + +import ( + "context" + "strings" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/handler" + logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/deco-sites/decofile-operator/internal/buildsecrets" +) + +const ( + buildSecretsAnnotation = "deco.sites/build-secrets-managed" + buildSecretsAnnotationValue = "enabled" +) + +// +kubebuilder:rbac:groups=external-secrets.io,resources=externalsecrets,verbs=get;list;watch;create;update;patch;delete + +// BuildSecretsReconciler keeps an ExternalSecret in sync with each +// opted-in site namespace. Opt-in is the annotation +// `deco.sites/build-secrets-managed: enabled` on the Namespace. +// +// Removing the annotation (or deleting the Namespace) deletes the EE, +// which cascades into the K8s Secret via ESO's `creationPolicy: Owner`. +// Builds revert silently because admin's envFrom is `optional: true`. +// +// # Force sync recipes +// +// Re-fetch ONE site from AWS Secrets Manager immediately, without +// waiting for the EE's refreshInterval: +// +// kubectl annotate es build-secrets -n sites- \ +// force-sync=$(date +%s) --overwrite +// +// Re-fetch ALL managed sites at once: +// +// kubectl get es -A -l deco.sites/feature=build-secrets -o name \ +// | xargs -I{} kubectl annotate {} force-sync=$(date +%s) --overwrite +// +// Bump the operator (re-applies the EE spec template across all +// managed namespaces — use after editing this controller): +// +// kubectl annotate ns -l deco.sites/build-secrets-managed=enabled \ +// deco.sites/build-secrets-sync=$(date +%s) --overwrite +type BuildSecretsReconciler struct { + client.Client + Scheme *runtime.Scheme +} + +func (r *BuildSecretsReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + log := logf.FromContext(ctx).WithName("buildsecrets").WithValues("namespace", req.Name) + + if !strings.HasPrefix(req.Name, siteNamespacePrefix) { + return ctrl.Result{}, nil + } + + ns := &corev1.Namespace{} + if err := r.Get(ctx, req.NamespacedName, ns); err != nil { + return ctrl.Result{}, client.IgnoreNotFound(err) + } + + site := siteNameFromNamespace(ns.Name) + if site == "" { + return ctrl.Result{}, nil + } + + optedIn := ns.Annotations[buildSecretsAnnotation] == buildSecretsAnnotationValue + if !optedIn || !ns.DeletionTimestamp.IsZero() { + if err := buildsecrets.Remove(ctx, r.Client, ns.Name); err != nil { + return ctrl.Result{}, err + } + log.V(1).Info("ExternalSecret removed", "site", site) + return ctrl.Result{}, nil + } + + if err := buildsecrets.Ensure(ctx, r.Client, ns.Name, site); err != nil { + return ctrl.Result{}, err + } + log.V(1).Info("ExternalSecret ensured", "site", site) + return ctrl.Result{}, nil +} + +func (r *BuildSecretsReconciler) SetupWithManager(mgr ctrl.Manager) error { + // Map ExternalSecret events back to the parent Namespace so deletions + // or out-of-band edits trigger a re-reconcile that restores the spec. + esToNamespace := handler.EnqueueRequestsFromMapFunc( + func(_ context.Context, obj client.Object) []reconcile.Request { + if obj.GetName() != buildsecrets.SecretName { + return nil + } + return []reconcile.Request{ + {NamespacedName: types.NamespacedName{Name: obj.GetNamespace()}}, + } + }, + ) + + es := &unstructured.Unstructured{} + es.SetGroupVersionKind(buildsecrets.GVK) + + return ctrl.NewControllerManagedBy(mgr). + Named("buildsecrets"). + For(&corev1.Namespace{}). + Watches(es, esToNamespace). + WithOptions(controller.Options{MaxConcurrentReconciles: 4}). + Complete(r) +} From 9ff4a14488ac440a828351a9facc3a8d4d739e36 Mon Sep 17 00:00:00 2001 From: Nicacio Oliveira Date: Wed, 20 May 2026 12:12:56 -0300 Subject: [PATCH 2/5] fix(buildsecrets): decouple site-name derivation from Valkey reserved-name filter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit siteNameFromNamespace also strips Valkey-reserved usernames ('default', 'redis-root') — irrelevant to build-secrets reconciliation. A site namespace called 'sites-default' would have been silently skipped from sync due to the Valkey coupling. Strip the prefix inline instead. Identified by cubic. --- internal/controller/buildsecrets_controller.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/controller/buildsecrets_controller.go b/internal/controller/buildsecrets_controller.go index 312adc4..3593289 100644 --- a/internal/controller/buildsecrets_controller.go +++ b/internal/controller/buildsecrets_controller.go @@ -69,7 +69,12 @@ type BuildSecretsReconciler struct { func (r *BuildSecretsReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { log := logf.FromContext(ctx).WithName("buildsecrets").WithValues("namespace", req.Name) - if !strings.HasPrefix(req.Name, siteNamespacePrefix) { + // Derive site from namespace by stripping the `sites-` prefix. We do NOT + // reuse siteNameFromNamespace from namespace_controller.go because that + // helper also filters out Valkey-reserved usernames ("default", + // "redis-root"), which has no bearing on build-secrets reconciliation. + site := strings.TrimPrefix(req.Name, siteNamespacePrefix) + if site == req.Name || site == "" { return ctrl.Result{}, nil } @@ -78,11 +83,6 @@ func (r *BuildSecretsReconciler) Reconcile(ctx context.Context, req ctrl.Request return ctrl.Result{}, client.IgnoreNotFound(err) } - site := siteNameFromNamespace(ns.Name) - if site == "" { - return ctrl.Result{}, nil - } - optedIn := ns.Annotations[buildSecretsAnnotation] == buildSecretsAnnotationValue if !optedIn || !ns.DeletionTimestamp.IsZero() { if err := buildsecrets.Remove(ctx, r.Client, ns.Name); err != nil { From 6f7c57a4959f94294e396185cea7a5cca09460c1 Mon Sep 17 00:00:00 2001 From: Nicacio Oliveira Date: Wed, 20 May 2026 16:31:29 -0300 Subject: [PATCH 3/5] refactor(buildsecrets): operator manages K8s Secret directly, no ESO MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pivot away from the ESO ExternalSecret intermediary. The reconciler now talks to the upstream secret backend (AWS Secrets Manager today) through a small Source interface and materialises the K8s Secret `build-secrets` itself. Why: - Per-tenant secrets have a different lifecycle than the shared secrets that justify ESO: "upstream not provisioned yet" is a normal state, not an error to surface in `kubectl get es`. - One component in the critical path instead of two. Easier to reason about, fewer failure modes. - The Source interface lets us swap the backend (GCP Secret Manager, Vault, even ESO if we ever want to revert) with a single new file — the rest of the operator and the K8s Secret contract stay put. What changed: - internal/buildsecrets/source.go — interface + constants - internal/buildsecrets/aws_source.go — AWS SM implementation - internal/buildsecrets/secret.go — Sync/Remove of the K8s Secret - internal/buildsecrets/*_test.go — fake Source + mock SM client - internal/controller/buildsecrets_controller.go — uses Source, watches Namespace + Secret - cmd/main.go — instantiates Source via --build-secrets-backend (env BUILD_SECRETS_BACKEND, default "aws"; "disabled" bypasses the reconciler entirely) - RBAC: drops externalsecrets.external-secrets.io, adds Secret update delete patch (regenerated via `make manifests && make helm`) - go.mod: adds aws-sdk-go-v2/service/secretsmanager, bumps the SDK Adoption safety: a Secret named `build-secrets` that lacks the operator's labels is treated as user-managed. The reconciler will not adopt, update, or delete it — useful while migrating away from the manual Secrets we already created by hand (e.g. frigidaire-*). Force-sync interface (annotation on the Namespace) preserved. Companion PR (deploys before this): IAM in terraform-eks-cluster adding secretsmanager:GetSecretValue to the operator Pod Identity. --- .../clusterrole-operator-manager-role.yaml | 22 +- cmd/main.go | 48 +++- config/rbac/role.yaml | 22 +- go.mod | 18 +- go.sum | 28 +- internal/buildsecrets/aws_source.go | 77 ++++++ internal/buildsecrets/aws_source_test.go | 108 ++++++++ internal/buildsecrets/externalsecret.go | 130 ---------- internal/buildsecrets/externalsecret_test.go | 124 --------- internal/buildsecrets/secret.go | 137 ++++++++++ internal/buildsecrets/secret_test.go | 245 ++++++++++++++++++ internal/buildsecrets/source.go | 56 ++++ .../controller/buildsecrets_controller.go | 79 +++--- 13 files changed, 729 insertions(+), 365 deletions(-) create mode 100644 internal/buildsecrets/aws_source.go create mode 100644 internal/buildsecrets/aws_source_test.go delete mode 100644 internal/buildsecrets/externalsecret.go delete mode 100644 internal/buildsecrets/externalsecret_test.go create mode 100644 internal/buildsecrets/secret.go create mode 100644 internal/buildsecrets/secret_test.go create mode 100644 internal/buildsecrets/source.go diff --git a/chart/templates/clusterrole-operator-manager-role.yaml b/chart/templates/clusterrole-operator-manager-role.yaml index 843372c..3b3cb0a 100644 --- a/chart/templates/clusterrole-operator-manager-role.yaml +++ b/chart/templates/clusterrole-operator-manager-role.yaml @@ -7,6 +7,7 @@ rules: - "" resources: - configmaps + - secrets verbs: - create - delete @@ -33,15 +34,6 @@ rules: - get - list - watch -- apiGroups: - - "" - resources: - - secrets - verbs: - - create - - get - - list - - watch - apiGroups: - "" resources: @@ -91,18 +83,6 @@ rules: - get - patch - update -- apiGroups: - - external-secrets.io - resources: - - externalsecrets - verbs: - - create - - delete - - get - - list - - patch - - update - - watch - apiGroups: - serving.knative.dev resources: diff --git a/cmd/main.go b/cmd/main.go index 0d81615..0b112d8 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -46,6 +46,7 @@ import ( decositesv1alpha1 "github.com/deco-sites/decofile-operator/api/v1alpha1" "github.com/deco-sites/decofile-operator/internal/build" + "github.com/deco-sites/decofile-operator/internal/buildsecrets" "github.com/deco-sites/decofile-operator/internal/controller" "github.com/deco-sites/decofile-operator/internal/valkey" webhookv1 "github.com/deco-sites/decofile-operator/internal/webhook/v1" @@ -81,6 +82,8 @@ func main() { var valkeyAdminPassword string var valkeyResyncPeriod time.Duration var valkeyWatchFailover bool + var buildSecretsBackend string + var buildSecretsResyncPeriod time.Duration flag.StringVar(&metricsAddr, "metrics-bind-address", "0", "The address the metrics endpoint binds to. "+ "Use :8443 for HTTPS or :8080 for HTTP, or leave as 0 to disable the metrics service.") flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.") @@ -114,6 +117,12 @@ func main() { os.Getenv("VALKEY_WATCH_FAILOVER") != "false", "Subscribe to Sentinel +switch-master events and trigger immediate ACL resync on failover. "+ "Enabled by default when VALKEY_SENTINEL_URLS is set. Set VALKEY_WATCH_FAILOVER=false to disable.") + flag.StringVar(&buildSecretsBackend, "build-secrets-backend", + getEnvOrDefault("BUILD_SECRETS_BACKEND", "aws"), + "Backend for tenant build-secrets sync. Supported: aws | disabled.") + flag.DurationVar(&buildSecretsResyncPeriod, "build-secrets-resync-period", + parseDuration(os.Getenv("BUILD_SECRETS_RESYNC_PERIOD"), controller.DefaultBuildSecretsResyncPeriod), + "How often to re-fetch upstream build-secrets even when nothing changed in the cluster.") opts := zap.Options{ Development: false, } @@ -268,14 +277,26 @@ func main() { os.Exit(1) } - bsReconciler := &controller.BuildSecretsReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - } - if err := bsReconciler.SetupWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create controller", "controller", "BuildSecrets") + buildSecretsSource, err := newBuildSecretsSource(context.Background(), buildSecretsBackend) + if err != nil { + setupLog.Error(err, "unable to initialize build-secrets source", "backend", buildSecretsBackend) os.Exit(1) } + if buildSecretsSource != nil { + bsReconciler := &controller.BuildSecretsReconciler{ + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + Source: buildSecretsSource, + ResyncPeriod: buildSecretsResyncPeriod, + } + if err := bsReconciler.SetupWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create controller", "controller", "BuildSecrets") + os.Exit(1) + } + setupLog.Info("Build-secrets sync enabled", "backend", buildSecretsBackend, "resyncPeriod", buildSecretsResyncPeriod) + } else { + setupLog.Info("Build-secrets sync disabled (BUILD_SECRETS_BACKEND=disabled)") + } // Start Sentinel failover watcher if enabled and Sentinel is configured. // leaderElectedRunnable ensures only the active leader subscribes — prevents @@ -421,3 +442,18 @@ func getEnvOrDefault(key, defaultVal string) string { } return defaultVal } + +// newBuildSecretsSource picks an upstream secret backend for the +// BuildSecretsReconciler. `disabled` returns (nil, nil) so callers can +// skip wiring the reconciler entirely — useful for local dev and for +// clusters where this feature is not yet enabled. +func newBuildSecretsSource(ctx context.Context, backend string) (buildsecrets.Source, error) { + switch backend { + case "aws": + return buildsecrets.NewAWSSource(ctx) + case "disabled", "": + return nil, nil + default: + return nil, fmt.Errorf("unknown build-secrets backend %q (supported: aws | disabled)", backend) + } +} diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 6a4e960..0700ea7 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -8,6 +8,7 @@ rules: - "" resources: - configmaps + - secrets verbs: - create - delete @@ -34,15 +35,6 @@ rules: - get - list - watch -- apiGroups: - - "" - resources: - - secrets - verbs: - - create - - get - - list - - watch - apiGroups: - "" resources: @@ -92,18 +84,6 @@ rules: - get - patch - update -- apiGroups: - - external-secrets.io - resources: - - externalsecrets - verbs: - - create - - delete - - get - - list - - patch - - update - - watch - apiGroups: - serving.knative.dev resources: diff --git a/go.mod b/go.mod index 4dbf225..7b5b185 100644 --- a/go.mod +++ b/go.mod @@ -4,10 +4,10 @@ go 1.24.0 require ( github.com/andybalholm/brotli v1.2.0 - github.com/aws/aws-sdk-go-v2 v1.36.3 + github.com/aws/aws-sdk-go-v2 v1.41.7 github.com/aws/aws-sdk-go-v2/config v1.29.14 - github.com/aws/aws-sdk-go-v2/credentials v1.17.67 - github.com/aws/aws-sdk-go-v2/service/s3 v1.78.2 + github.com/aws/aws-sdk-go-v2/service/secretsmanager v1.41.7 + github.com/go-logr/logr v1.4.3 github.com/google/uuid v1.6.0 github.com/onsi/ginkgo/v2 v2.22.0 github.com/onsi/gomega v1.36.1 @@ -23,20 +23,17 @@ require ( require ( cel.dev/expr v0.24.0 // indirect github.com/antlr4-go/antlr/v4 v4.13.0 // indirect - github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.6.10 // indirect + github.com/aws/aws-sdk-go-v2/credentials v1.17.67 // indirect github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.30 // indirect - github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.34 // indirect - github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.34 // indirect + github.com/aws/aws-sdk-go-v2/internal/configsources v1.4.23 // indirect + github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.7.23 // indirect github.com/aws/aws-sdk-go-v2/internal/ini v1.8.3 // indirect - github.com/aws/aws-sdk-go-v2/internal/v4a v1.3.34 // indirect github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.12.3 // indirect - github.com/aws/aws-sdk-go-v2/service/internal/checksum v1.7.0 // indirect github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.12.15 // indirect - github.com/aws/aws-sdk-go-v2/service/internal/s3shared v1.18.15 // indirect github.com/aws/aws-sdk-go-v2/service/sso v1.25.3 // indirect github.com/aws/aws-sdk-go-v2/service/ssooidc v1.30.1 // indirect github.com/aws/aws-sdk-go-v2/service/sts v1.33.19 // indirect - github.com/aws/smithy-go v1.22.2 // indirect + github.com/aws/smithy-go v1.25.1 // indirect github.com/beorn7/perks v1.0.1 // indirect github.com/blang/semver/v4 v4.0.0 // indirect github.com/blendle/zapdriver v1.3.1 // indirect @@ -49,7 +46,6 @@ require ( github.com/felixge/httpsnoop v1.0.4 // indirect github.com/fsnotify/fsnotify v1.7.0 // indirect github.com/fxamacker/cbor/v2 v2.7.0 // indirect - github.com/go-logr/logr v1.4.3 // indirect github.com/go-logr/stdr v1.2.2 // indirect github.com/go-logr/zapr v1.3.0 // indirect github.com/go-openapi/jsonpointer v0.21.0 // indirect diff --git a/go.sum b/go.sum index b731f34..66013cc 100644 --- a/go.sum +++ b/go.sum @@ -4,42 +4,34 @@ github.com/andybalholm/brotli v1.2.0 h1:ukwgCxwYrmACq68yiUqwIWnGY0cTPox/M94sVwTo github.com/andybalholm/brotli v1.2.0/go.mod h1:rzTDkvFWvIrjDXZHkuS16NPggd91W3kUSvPlQ1pLaKY= github.com/antlr4-go/antlr/v4 v4.13.0 h1:lxCg3LAv+EUK6t1i0y1V6/SLeUi0eKEKdhQAlS8TVTI= github.com/antlr4-go/antlr/v4 v4.13.0/go.mod h1:pfChB/xh/Unjila75QW7+VU4TSnWnnk9UTnmpPaOR2g= -github.com/aws/aws-sdk-go-v2 v1.36.3 h1:mJoei2CxPutQVxaATCzDUjcZEjVRdpsiiXi2o38yqWM= -github.com/aws/aws-sdk-go-v2 v1.36.3/go.mod h1:LLXuLpgzEbD766Z5ECcRmi8AzSwfZItDtmABVkRLGzg= -github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.6.10 h1:zAybnyUQXIZ5mok5Jqwlf58/TFE7uvd3IAsa1aF9cXs= -github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.6.10/go.mod h1:qqvMj6gHLR/EXWZw4ZbqlPbQUyenf4h82UQUlKc+l14= +github.com/aws/aws-sdk-go-v2 v1.41.7 h1:DWpAJt66FmnnaRIOT/8ASTucrvuDPZASqhhLey6tLY8= +github.com/aws/aws-sdk-go-v2 v1.41.7/go.mod h1:4LAfZOPHNVNQEckOACQx60Y8pSRjIkNZQz1w92xpMJc= github.com/aws/aws-sdk-go-v2/config v1.29.14 h1:f+eEi/2cKCg9pqKBoAIwRGzVb70MRKqWX4dg1BDcSJM= github.com/aws/aws-sdk-go-v2/config v1.29.14/go.mod h1:wVPHWcIFv3WO89w0rE10gzf17ZYy+UVS1Geq8Iei34g= github.com/aws/aws-sdk-go-v2/credentials v1.17.67 h1:9KxtdcIA/5xPNQyZRgUSpYOE6j9Bc4+D7nZua0KGYOM= github.com/aws/aws-sdk-go-v2/credentials v1.17.67/go.mod h1:p3C44m+cfnbv763s52gCqrjaqyPikj9Sg47kUVaNZQQ= github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.30 h1:x793wxmUWVDhshP8WW2mlnXuFrO4cOd3HLBroh1paFw= github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.30/go.mod h1:Jpne2tDnYiFascUEs2AWHJL9Yp7A5ZVy3TNyxaAjD6M= -github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.34 h1:ZK5jHhnrioRkUNOc+hOgQKlUL5JeC3S6JgLxtQ+Rm0Q= -github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.34/go.mod h1:p4VfIceZokChbA9FzMbRGz5OV+lekcVtHlPKEO0gSZY= -github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.34 h1:SZwFm17ZUNNg5Np0ioo/gq8Mn6u9w19Mri8DnJ15Jf0= -github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.34/go.mod h1:dFZsC0BLo346mvKQLWmoJxT+Sjp+qcVR1tRVHQGOH9Q= +github.com/aws/aws-sdk-go-v2/internal/configsources v1.4.23 h1:GpT/TrnBYuE5gan2cZbTtvP+JlHsutdmlV2YfEyNde0= +github.com/aws/aws-sdk-go-v2/internal/configsources v1.4.23/go.mod h1:xYWD6BS9ywC5bS3sz9Xh04whO/hzK2plt2Zkyrp4JuA= +github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.7.23 h1:bpd8vxhlQi2r1hiueOw02f/duEPTMK59Q4QMAoTTtTo= +github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.7.23/go.mod h1:15DfR2nw+CRHIk0tqNyifu3G1YdAOy68RftkhMDDwYk= github.com/aws/aws-sdk-go-v2/internal/ini v1.8.3 h1:bIqFDwgGXXN1Kpp99pDOdKMTTb5d2KyU5X/BZxjOkRo= github.com/aws/aws-sdk-go-v2/internal/ini v1.8.3/go.mod h1:H5O/EsxDWyU+LP/V8i5sm8cxoZgc2fdNR9bxlOFrQTo= -github.com/aws/aws-sdk-go-v2/internal/v4a v1.3.34 h1:ZNTqv4nIdE/DiBfUUfXcLZ/Spcuz+RjeziUtNJackkM= -github.com/aws/aws-sdk-go-v2/internal/v4a v1.3.34/go.mod h1:zf7Vcd1ViW7cPqYWEHLHJkS50X0JS2IKz9Cgaj6ugrs= github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.12.3 h1:eAh2A4b5IzM/lum78bZ590jy36+d/aFLgKF/4Vd1xPE= github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.12.3/go.mod h1:0yKJC/kb8sAnmlYa6Zs3QVYqaC8ug2AbnNChv5Ox3uA= -github.com/aws/aws-sdk-go-v2/service/internal/checksum v1.7.0 h1:lguz0bmOoGzozP9XfRJR1QIayEYo+2vP/No3OfLF0pU= -github.com/aws/aws-sdk-go-v2/service/internal/checksum v1.7.0/go.mod h1:iu6FSzgt+M2/x3Dk8zhycdIcHjEFb36IS8HVUVFoMg0= github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.12.15 h1:dM9/92u2F1JbDaGooxTq18wmmFzbJRfXfVfy96/1CXM= github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.12.15/go.mod h1:SwFBy2vjtA0vZbjjaFtfN045boopadnoVPhu4Fv66vY= -github.com/aws/aws-sdk-go-v2/service/internal/s3shared v1.18.15 h1:moLQUoVq91LiqT1nbvzDukyqAlCv89ZmwaHw/ZFlFZg= -github.com/aws/aws-sdk-go-v2/service/internal/s3shared v1.18.15/go.mod h1:ZH34PJUc8ApjBIfgQCFvkWcUDBtl/WTD+uiYHjd8igA= -github.com/aws/aws-sdk-go-v2/service/s3 v1.78.2 h1:jIiopHEV22b4yQP2q36Y0OmwLbsxNWdWwfZRR5QRRO4= -github.com/aws/aws-sdk-go-v2/service/s3 v1.78.2/go.mod h1:U5SNqwhXB3Xe6F47kXvWihPl/ilGaEDe8HD/50Z9wxc= +github.com/aws/aws-sdk-go-v2/service/secretsmanager v1.41.7 h1:JUGKqUnJHbXpS8uyuICP/zpQ+vXUIXW2zTEqjMLCqrY= +github.com/aws/aws-sdk-go-v2/service/secretsmanager v1.41.7/go.mod h1:l/cqI7ujYqBuTR6Ll13d9/gG/uUdlVzJ1UDltEEBTOo= github.com/aws/aws-sdk-go-v2/service/sso v1.25.3 h1:1Gw+9ajCV1jogloEv1RRnvfRFia2cL6c9cuKV2Ps+G8= github.com/aws/aws-sdk-go-v2/service/sso v1.25.3/go.mod h1:qs4a9T5EMLl/Cajiw2TcbNt2UNo/Hqlyp+GiuG4CFDI= github.com/aws/aws-sdk-go-v2/service/ssooidc v1.30.1 h1:hXmVKytPfTy5axZ+fYbR5d0cFmC3JvwLm5kM83luako= github.com/aws/aws-sdk-go-v2/service/ssooidc v1.30.1/go.mod h1:MlYRNmYu/fGPoxBQVvBYr9nyr948aY/WLUvwBMBJubs= github.com/aws/aws-sdk-go-v2/service/sts v1.33.19 h1:1XuUZ8mYJw9B6lzAkXhqHlJd/XvaX32evhproijJEZY= github.com/aws/aws-sdk-go-v2/service/sts v1.33.19/go.mod h1:cQnB8CUnxbMU82JvlqjKR2HBOm3fe9pWorWBza6MBJ4= -github.com/aws/smithy-go v1.22.2 h1:6D9hW43xKFrRx/tXXfAlIZc4JI+yQe6snnWcQyxSyLQ= -github.com/aws/smithy-go v1.22.2/go.mod h1:irrKGvNn1InZwb2d7fkIRNucdfwR8R+Ts3wxYa/cJHg= +github.com/aws/smithy-go v1.25.1 h1:J8ERsGSU7d+aCmdQur5Txg6bVoYelvQJgtZehD12GkI= +github.com/aws/smithy-go v1.25.1/go.mod h1:YE2RhdIuDbA5E5bTdciG9KrW3+TiEONeUWCqxX9i1Fc= github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= github.com/blang/semver/v4 v4.0.0 h1:1PFHFE6yCCTv8C1TeyNNarDzntLi7wMI5i/pzqYIsAM= diff --git a/internal/buildsecrets/aws_source.go b/internal/buildsecrets/aws_source.go new file mode 100644 index 0000000..38d5d53 --- /dev/null +++ b/internal/buildsecrets/aws_source.go @@ -0,0 +1,77 @@ +/* +Copyright 2025. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 +*/ + +package buildsecrets + +import ( + "context" + "encoding/json" + "errors" + "fmt" + + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/config" + "github.com/aws/aws-sdk-go-v2/service/secretsmanager" + smtypes "github.com/aws/aws-sdk-go-v2/service/secretsmanager/types" +) + +// awsSecretsManagerAPI captures the AWS Secrets Manager surface this +// package uses. Defined so tests can swap a mock without depending on +// the real SDK client. +type awsSecretsManagerAPI interface { + GetSecretValue(ctx context.Context, params *secretsmanager.GetSecretValueInput, optFns ...func(*secretsmanager.Options)) (*secretsmanager.GetSecretValueOutput, error) +} + +// AWSSource resolves keys against AWS Secrets Manager. Credentials come +// from the ambient environment (Pod Identity / IRSA in-cluster; default +// chain locally). Region is taken from the AWS_REGION env var or the +// default chain — usually set by the EKS Pod Identity webhook. +type AWSSource struct { + api awsSecretsManagerAPI +} + +// NewAWSSource builds a Source backed by the live Secrets Manager +// client. The reconciler keeps the same instance for its lifetime. +func NewAWSSource(ctx context.Context) (*AWSSource, error) { + cfg, err := config.LoadDefaultConfig(ctx) + if err != nil { + return nil, fmt.Errorf("load AWS config: %w", err) + } + return &AWSSource{api: secretsmanager.NewFromConfig(cfg)}, nil +} + +// Get fetches the JSON-encoded secret at `key` and decodes it into a +// flat map. A missing key returns (nil, false, nil) — the natural +// "not provisioned yet" state for tenants that have opted in but not +// supplied data. +func (s *AWSSource) Get(ctx context.Context, key string) (map[string]string, bool, error) { + out, err := s.api.GetSecretValue(ctx, &secretsmanager.GetSecretValueInput{ + SecretId: aws.String(key), + }) + if err != nil { + var nfe *smtypes.ResourceNotFoundException + if errors.As(err, &nfe) { + return nil, false, nil + } + return nil, false, fmt.Errorf("get secret %q: %w", key, err) + } + + if out.SecretString == nil { + // Binary secrets are not supported — builds inject env vars, + // not files, so a JSON object of strings is the only shape. + return nil, false, fmt.Errorf("secret %q has no SecretString (binary secret?)", key) + } + + var data map[string]string + if err := json.Unmarshal([]byte(*out.SecretString), &data); err != nil { + return nil, false, fmt.Errorf("parse %q as JSON object of strings: %w", key, err) + } + return data, true, nil +} diff --git a/internal/buildsecrets/aws_source_test.go b/internal/buildsecrets/aws_source_test.go new file mode 100644 index 0000000..3b9a5b1 --- /dev/null +++ b/internal/buildsecrets/aws_source_test.go @@ -0,0 +1,108 @@ +/* +Copyright 2025. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 +*/ + +package buildsecrets + +import ( + "context" + "errors" + "testing" + + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/service/secretsmanager" + smtypes "github.com/aws/aws-sdk-go-v2/service/secretsmanager/types" +) + +type mockSMClient struct { + out *secretsmanager.GetSecretValueOutput + err error +} + +func (m *mockSMClient) GetSecretValue(_ context.Context, _ *secretsmanager.GetSecretValueInput, _ ...func(*secretsmanager.Options)) (*secretsmanager.GetSecretValueOutput, error) { + return m.out, m.err +} + +func TestAWSSourceGet_Found(t *testing.T) { + src := &AWSSource{api: &mockSMClient{ + out: &secretsmanager.GetSecretValueOutput{ + SecretString: aws.String(`{"DENO_AUTH_TOKENS":"github_pat_xxx@raw.githubusercontent.com","FOO":"bar"}`), + }, + }} + + data, exists, err := src.Get(context.Background(), "sites/acme/build") + if err != nil { + t.Fatalf("Get: %v", err) + } + if !exists { + t.Fatal("exists = false, want true") + } + if data["DENO_AUTH_TOKENS"] != "github_pat_xxx@raw.githubusercontent.com" { + t.Fatalf("DENO_AUTH_TOKENS = %q", data["DENO_AUTH_TOKENS"]) + } + if data["FOO"] != "bar" { + t.Fatalf("FOO = %q", data["FOO"]) + } +} + +func TestAWSSourceGet_NotFoundIsNotAnError(t *testing.T) { + src := &AWSSource{api: &mockSMClient{ + err: &smtypes.ResourceNotFoundException{}, + }} + + data, exists, err := src.Get(context.Background(), "sites/acme/build") + if err != nil { + t.Fatalf("Get returned error: %v", err) + } + if exists { + t.Fatal("exists = true on missing key, want false") + } + if data != nil { + t.Fatalf("data = %v, want nil", data) + } +} + +func TestAWSSourceGet_BinaryRejected(t *testing.T) { + src := &AWSSource{api: &mockSMClient{ + out: &secretsmanager.GetSecretValueOutput{ + SecretBinary: []byte{0x01, 0x02}, + }, + }} + + _, _, err := src.Get(context.Background(), "sites/acme/build") + if err == nil { + t.Fatal("expected error for binary secret, got nil") + } +} + +func TestAWSSourceGet_InvalidJSON(t *testing.T) { + src := &AWSSource{api: &mockSMClient{ + out: &secretsmanager.GetSecretValueOutput{ + SecretString: aws.String(`not json`), + }, + }} + + _, _, err := src.Get(context.Background(), "sites/acme/build") + if err == nil { + t.Fatal("expected JSON parse error, got nil") + } +} + +func TestAWSSourceGet_OtherErrorPropagates(t *testing.T) { + sentinel := errors.New("boom") + src := &AWSSource{api: &mockSMClient{err: sentinel}} + + _, _, err := src.Get(context.Background(), "sites/acme/build") + if err == nil { + t.Fatal("expected error, got nil") + } + if !errors.Is(err, sentinel) { + t.Fatalf("error did not wrap sentinel: %v", err) + } +} diff --git a/internal/buildsecrets/externalsecret.go b/internal/buildsecrets/externalsecret.go deleted file mode 100644 index 61a849b..0000000 --- a/internal/buildsecrets/externalsecret.go +++ /dev/null @@ -1,130 +0,0 @@ -/* -Copyright 2025. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 -*/ - -// Package buildsecrets owns the operator's interaction with the tenant -// build-time secrets sync. Today it materialises an `ExternalSecret` -// (external-secrets.io/v1) per opted-in site namespace; swapping the -// sync mechanism away from ESO touches only this file. -package buildsecrets - -import ( - "context" - "fmt" - - "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/types" - "sigs.k8s.io/controller-runtime/pkg/client" -) - -const ( - // SecretName is the K8s Secret materialised by ESO from AWS Secrets - // Manager. Builders in each site namespace consume it via envFrom with - // optional:true (admin PR #3201) so its absence is a no-op. - SecretName = "build-secrets" - - // ManagedByLabel + FeatureLabel let operators bulk-filter the EE for - // force-sync via `kubectl annotate es -l ... force-sync=$(date +%s)`. - ManagedByLabel = "deco.sites/managed-by" - FeatureLabel = "deco.sites/feature" - - refreshInterval = "1h" - clusterSecretStoreName = "aws-secrets-manager" - awsSecretsManagerKeyFmt = "sites/%s/build" -) - -// GVK is exported so the reconciler can register a Watches() on the same -// type without re-declaring the kind tuple. -var GVK = schema.GroupVersionKind{ - Group: "external-secrets.io", - Version: "v1", - Kind: "ExternalSecret", -} - -// Ensure creates or updates the ExternalSecret pulling `sites//build` -// from the cluster-local AWS Secrets Manager into the K8s Secret named -// `build-secrets`. Spec drift is corrected on every reconcile. -func Ensure(ctx context.Context, c client.Client, namespace, site string) error { - desired := newExternalSecret(namespace, site) - - existing := &unstructured.Unstructured{} - existing.SetGroupVersionKind(GVK) - key := types.NamespacedName{Name: SecretName, Namespace: namespace} - err := c.Get(ctx, key, existing) - switch { - case errors.IsNotFound(err): - return c.Create(ctx, desired) - case err != nil: - return fmt.Errorf("get externalsecret: %w", err) - } - - existing.Object["spec"] = desired.Object["spec"] - labels := existing.GetLabels() - if labels == nil { - labels = map[string]string{} - } - for k, v := range desired.GetLabels() { - labels[k] = v - } - existing.SetLabels(labels) - if err := c.Update(ctx, existing); err != nil { - return fmt.Errorf("update externalsecret: %w", err) - } - return nil -} - -// Remove deletes the ExternalSecret. ESO's `creationPolicy: Owner` makes -// the materialised K8s Secret cascade with it. Idempotent on NotFound. -func Remove(ctx context.Context, c client.Client, namespace string) error { - obj := &unstructured.Unstructured{} - obj.SetGroupVersionKind(GVK) - obj.SetName(SecretName) - obj.SetNamespace(namespace) - if err := c.Delete(ctx, obj); err != nil && !errors.IsNotFound(err) { - return fmt.Errorf("delete externalsecret: %w", err) - } - return nil -} - -func newExternalSecret(namespace, site string) *unstructured.Unstructured { - return &unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": GVK.GroupVersion().String(), - "kind": GVK.Kind, - "metadata": map[string]interface{}{ - "name": SecretName, - "namespace": namespace, - "labels": map[string]interface{}{ - ManagedByLabel: "operator", - FeatureLabel: "build-secrets", - }, - }, - "spec": map[string]interface{}{ - "refreshInterval": refreshInterval, - "secretStoreRef": map[string]interface{}{ - "name": clusterSecretStoreName, - "kind": "ClusterSecretStore", - }, - "target": map[string]interface{}{ - "name": SecretName, - "creationPolicy": "Owner", - }, - "dataFrom": []interface{}{ - map[string]interface{}{ - "extract": map[string]interface{}{ - "key": fmt.Sprintf(awsSecretsManagerKeyFmt, site), - }, - }, - }, - }, - }, - } -} diff --git a/internal/buildsecrets/externalsecret_test.go b/internal/buildsecrets/externalsecret_test.go deleted file mode 100644 index 2c4d47e..0000000 --- a/internal/buildsecrets/externalsecret_test.go +++ /dev/null @@ -1,124 +0,0 @@ -/* -Copyright 2025. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 -*/ - -package buildsecrets - -import ( - "context" - "testing" - - "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/types" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/client/fake" -) - -func newClient(t *testing.T) client.Client { - t.Helper() - scheme := runtime.NewScheme() - return fake.NewClientBuilder().WithScheme(scheme).Build() -} - -func getExternalSecret(t *testing.T, c client.Client, namespace string) *unstructured.Unstructured { - t.Helper() - got := &unstructured.Unstructured{} - got.SetGroupVersionKind(GVK) - if err := c.Get(context.Background(), types.NamespacedName{Name: SecretName, Namespace: namespace}, got); err != nil { - t.Fatalf("Get externalsecret: %v", err) - } - return got -} - -func TestEnsureCreates(t *testing.T) { - c := newClient(t) - if err := Ensure(context.Background(), c, "sites-acme", "acme"); err != nil { - t.Fatalf("Ensure: %v", err) - } - - got := getExternalSecret(t, c, "sites-acme") - if v := got.GetLabels()[ManagedByLabel]; v != "operator" { - t.Fatalf("label %s = %q, want operator", ManagedByLabel, v) - } - if v := got.GetLabels()[FeatureLabel]; v != "build-secrets" { - t.Fatalf("label %s = %q, want build-secrets", FeatureLabel, v) - } - - dataFrom, _, _ := unstructured.NestedSlice(got.Object, "spec", "dataFrom") - if len(dataFrom) != 1 { - t.Fatalf("dataFrom length = %d, want 1", len(dataFrom)) - } - extract := dataFrom[0].(map[string]interface{})["extract"].(map[string]interface{}) - if got := extract["key"]; got != "sites/acme/build" { - t.Fatalf("extract.key = %q, want sites/acme/build", got) - } - target, _, _ := unstructured.NestedMap(got.Object, "spec", "target") - if got := target["creationPolicy"]; got != "Owner" { - t.Fatalf("creationPolicy = %q, want Owner", got) - } -} - -func TestEnsureIsIdempotent(t *testing.T) { - c := newClient(t) - ctx := context.Background() - if err := Ensure(ctx, c, "sites-acme", "acme"); err != nil { - t.Fatalf("first Ensure: %v", err) - } - if err := Ensure(ctx, c, "sites-acme", "acme"); err != nil { - t.Fatalf("second Ensure: %v", err) - } -} - -func TestEnsureCorrectsDrift(t *testing.T) { - c := newClient(t) - ctx := context.Background() - if err := Ensure(ctx, c, "sites-acme", "acme"); err != nil { - t.Fatalf("Ensure: %v", err) - } - - drifted := getExternalSecret(t, c, "sites-acme") - if err := unstructured.SetNestedField(drifted.Object, "999h", "spec", "refreshInterval"); err != nil { - t.Fatalf("SetNestedField: %v", err) - } - if err := c.Update(ctx, drifted); err != nil { - t.Fatalf("Update drift: %v", err) - } - - if err := Ensure(ctx, c, "sites-acme", "acme"); err != nil { - t.Fatalf("Ensure after drift: %v", err) - } - healed := getExternalSecret(t, c, "sites-acme") - got, _, _ := unstructured.NestedString(healed.Object, "spec", "refreshInterval") - if got != refreshInterval { - t.Fatalf("refreshInterval = %q, want %q (drift not corrected)", got, refreshInterval) - } -} - -func TestRemove(t *testing.T) { - c := newClient(t) - ctx := context.Background() - if err := Remove(ctx, c, "sites-acme"); err != nil { - t.Fatalf("Remove on empty: %v", err) - } - if err := Ensure(ctx, c, "sites-acme", "acme"); err != nil { - t.Fatalf("Ensure: %v", err) - } - if err := Remove(ctx, c, "sites-acme"); err != nil { - t.Fatalf("Remove: %v", err) - } - - got := &unstructured.Unstructured{} - got.SetGroupVersionKind(GVK) - err := c.Get(ctx, types.NamespacedName{Name: SecretName, Namespace: "sites-acme"}, got) - if !errors.IsNotFound(err) { - t.Fatalf("Get after Remove: err = %v, want NotFound", err) - } -} diff --git a/internal/buildsecrets/secret.go b/internal/buildsecrets/secret.go new file mode 100644 index 0000000..8274759 --- /dev/null +++ b/internal/buildsecrets/secret.go @@ -0,0 +1,137 @@ +/* +Copyright 2025. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 +*/ + +package buildsecrets + +import ( + "context" + "fmt" + "maps" + "reflect" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// Sync reconciles the K8s Secret `build-secrets` in `namespace` against +// the upstream Source for `site`. Four cases: +// +// 1. Upstream missing + no local Secret: no-op. +// 2. Upstream missing + local Secret owned by operator: delete locally. +// 3. Upstream present + no local Secret: create with operator labels. +// 4. Upstream present + local Secret owned by operator: update if data +// drifted. +// +// In all cases where a local Secret exists *without* operator labels, +// returns ErrNotOwned and makes no changes. +func Sync(ctx context.Context, c client.Client, namespace, site string, src Source) error { + data, exists, err := src.Get(ctx, fmt.Sprintf(KeyTemplate, site)) + if err != nil { + return fmt.Errorf("source.Get: %w", err) + } + + existing := &corev1.Secret{} + getErr := c.Get(ctx, types.NamespacedName{Name: SecretName, Namespace: namespace}, existing) + switch { + case errors.IsNotFound(getErr): + existing = nil + case getErr != nil: + return fmt.Errorf("get secret: %w", getErr) + } + + if !exists { + if existing == nil { + return nil + } + if !isManagedByUs(existing) { + return ErrNotOwned + } + if err := c.Delete(ctx, existing); err != nil && !errors.IsNotFound(err) { + return fmt.Errorf("delete secret: %w", err) + } + return nil + } + + desired := newSecret(namespace, data) + + if existing == nil { + if err := c.Create(ctx, desired); err != nil { + return fmt.Errorf("create secret: %w", err) + } + return nil + } + + if !isManagedByUs(existing) { + return ErrNotOwned + } + + if !dataChanged(existing.Data, desired.Data) { + return nil + } + + existing.Data = desired.Data + existing.StringData = nil + existing.Labels = maps.Clone(desired.Labels) + if err := c.Update(ctx, existing); err != nil { + return fmt.Errorf("update secret: %w", err) + } + return nil +} + +// Remove deletes the operator-managed K8s Secret in `namespace`. +// Refuses (ErrNotOwned) if the Secret lacks operator labels. +// Idempotent on NotFound. +func Remove(ctx context.Context, c client.Client, namespace string) error { + existing := &corev1.Secret{} + err := c.Get(ctx, types.NamespacedName{Name: SecretName, Namespace: namespace}, existing) + if errors.IsNotFound(err) { + return nil + } + if err != nil { + return fmt.Errorf("get secret: %w", err) + } + if !isManagedByUs(existing) { + return ErrNotOwned + } + if err := c.Delete(ctx, existing); err != nil && !errors.IsNotFound(err) { + return fmt.Errorf("delete secret: %w", err) + } + return nil +} + +func newSecret(namespace string, data map[string]string) *corev1.Secret { + out := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: SecretName, + Namespace: namespace, + Labels: map[string]string{ + ManagedByLabel: "operator", + FeatureLabel: "build-secrets", + }, + }, + Data: make(map[string][]byte, len(data)), + Type: corev1.SecretTypeOpaque, + } + for k, v := range data { + out.Data[k] = []byte(v) + } + return out +} + +func isManagedByUs(s *corev1.Secret) bool { + return s.Labels[ManagedByLabel] == "operator" && s.Labels[FeatureLabel] == "build-secrets" +} + +func dataChanged(current, desired map[string][]byte) bool { + return !reflect.DeepEqual(current, desired) +} diff --git a/internal/buildsecrets/secret_test.go b/internal/buildsecrets/secret_test.go new file mode 100644 index 0000000..8deee20 --- /dev/null +++ b/internal/buildsecrets/secret_test.go @@ -0,0 +1,245 @@ +/* +Copyright 2025. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 +*/ + +package buildsecrets + +import ( + "context" + "errors" + "testing" + + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +// mockSource implements Source for tests without touching AWS. +type mockSource struct { + data map[string]string + exists bool + err error +} + +func (m *mockSource) Get(ctx context.Context, key string) (map[string]string, bool, error) { + if m.err != nil { + return nil, false, m.err + } + return m.data, m.exists, nil +} + +func newClient(t *testing.T, objs ...client.Object) client.Client { + t.Helper() + scheme := runtime.NewScheme() + _ = clientgoscheme.AddToScheme(scheme) + return fake.NewClientBuilder().WithScheme(scheme).WithObjects(objs...).Build() +} + +func getSecret(t *testing.T, c client.Client, namespace string) *corev1.Secret { + t.Helper() + got := &corev1.Secret{} + if err := c.Get(context.Background(), types.NamespacedName{Name: SecretName, Namespace: namespace}, got); err != nil { + t.Fatalf("get secret: %v", err) + } + return got +} + +func TestSyncCreatesWhenUpstreamExists(t *testing.T) { + c := newClient(t) + src := &mockSource{exists: true, data: map[string]string{"DENO_AUTH_TOKENS": "github_pat_xxx@raw.githubusercontent.com"}} + + if err := Sync(context.Background(), c, "sites-acme", "acme", src); err != nil { + t.Fatalf("Sync: %v", err) + } + + got := getSecret(t, c, "sites-acme") + if string(got.Data["DENO_AUTH_TOKENS"]) != "github_pat_xxx@raw.githubusercontent.com" { + t.Fatalf("data not written: %v", got.Data) + } + if got.Labels[ManagedByLabel] != "operator" || got.Labels[FeatureLabel] != "build-secrets" { + t.Fatalf("labels missing: %v", got.Labels) + } +} + +func TestSyncNoopWhenUpstreamMissing(t *testing.T) { + c := newClient(t) + src := &mockSource{exists: false} + + if err := Sync(context.Background(), c, "sites-acme", "acme", src); err != nil { + t.Fatalf("Sync: %v", err) + } + + got := &corev1.Secret{} + err := c.Get(context.Background(), types.NamespacedName{Name: SecretName, Namespace: "sites-acme"}, got) + if !apierrors.IsNotFound(err) { + t.Fatalf("expected NotFound, got %v", err) + } +} + +func TestSyncUpdatesOnDrift(t *testing.T) { + managed := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: SecretName, + Namespace: "sites-acme", + Labels: map[string]string{ + ManagedByLabel: "operator", + FeatureLabel: "build-secrets", + }, + }, + Data: map[string][]byte{"OLD": []byte("value")}, + } + c := newClient(t, managed) + src := &mockSource{exists: true, data: map[string]string{"NEW": "value"}} + + if err := Sync(context.Background(), c, "sites-acme", "acme", src); err != nil { + t.Fatalf("Sync: %v", err) + } + + got := getSecret(t, c, "sites-acme") + if _, ok := got.Data["OLD"]; ok { + t.Fatal("old key not removed") + } + if string(got.Data["NEW"]) != "value" { + t.Fatalf("new key not written: %v", got.Data) + } +} + +func TestSyncIdempotentWhenDataMatches(t *testing.T) { + managed := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: SecretName, + Namespace: "sites-acme", + Labels: map[string]string{ + ManagedByLabel: "operator", + FeatureLabel: "build-secrets", + }, + ResourceVersion: "999", + }, + Data: map[string][]byte{"FOO": []byte("bar")}, + } + c := newClient(t, managed) + src := &mockSource{exists: true, data: map[string]string{"FOO": "bar"}} + + if err := Sync(context.Background(), c, "sites-acme", "acme", src); err != nil { + t.Fatalf("Sync: %v", err) + } + + got := getSecret(t, c, "sites-acme") + if got.ResourceVersion != "999" { + t.Fatalf("ResourceVersion changed (write should have been skipped): %s", got.ResourceVersion) + } +} + +func TestSyncDeletesWhenUpstreamRemoved(t *testing.T) { + managed := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: SecretName, + Namespace: "sites-acme", + Labels: map[string]string{ + ManagedByLabel: "operator", + FeatureLabel: "build-secrets", + }, + }, + Data: map[string][]byte{"FOO": []byte("bar")}, + } + c := newClient(t, managed) + src := &mockSource{exists: false} + + if err := Sync(context.Background(), c, "sites-acme", "acme", src); err != nil { + t.Fatalf("Sync: %v", err) + } + + got := &corev1.Secret{} + err := c.Get(context.Background(), types.NamespacedName{Name: SecretName, Namespace: "sites-acme"}, got) + if !apierrors.IsNotFound(err) { + t.Fatalf("expected Secret deleted, got err = %v", err) + } +} + +func TestSyncRefusesUnownedSecret(t *testing.T) { + unowned := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: SecretName, + Namespace: "sites-acme", + }, + Data: map[string][]byte{"USER_TOKEN": []byte("hand-crafted")}, + } + c := newClient(t, unowned) + src := &mockSource{exists: true, data: map[string]string{"FROM_SM": "value"}} + + err := Sync(context.Background(), c, "sites-acme", "acme", src) + if !errors.Is(err, ErrNotOwned) { + t.Fatalf("expected ErrNotOwned, got %v", err) + } + + got := getSecret(t, c, "sites-acme") + if string(got.Data["USER_TOKEN"]) != "hand-crafted" { + t.Fatalf("unowned secret data was mutated: %v", got.Data) + } + if _, ok := got.Data["FROM_SM"]; ok { + t.Fatal("operator wrote into unowned secret") + } +} + +func TestRemoveDeletesManagedSecret(t *testing.T) { + managed := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: SecretName, + Namespace: "sites-acme", + Labels: map[string]string{ + ManagedByLabel: "operator", + FeatureLabel: "build-secrets", + }, + }, + } + c := newClient(t, managed) + + if err := Remove(context.Background(), c, "sites-acme"); err != nil { + t.Fatalf("Remove: %v", err) + } + + got := &corev1.Secret{} + err := c.Get(context.Background(), types.NamespacedName{Name: SecretName, Namespace: "sites-acme"}, got) + if !apierrors.IsNotFound(err) { + t.Fatalf("expected NotFound, got %v", err) + } +} + +func TestRemoveRefusesUnowned(t *testing.T) { + unowned := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: SecretName, + Namespace: "sites-acme", + }, + } + c := newClient(t, unowned) + + err := Remove(context.Background(), c, "sites-acme") + if !errors.Is(err, ErrNotOwned) { + t.Fatalf("expected ErrNotOwned, got %v", err) + } + + // Secret still there + got := getSecret(t, c, "sites-acme") + if got.Name != SecretName { + t.Fatal("unowned secret was deleted") + } +} + +func TestRemoveIdempotentOnNotFound(t *testing.T) { + c := newClient(t) + if err := Remove(context.Background(), c, "sites-acme"); err != nil { + t.Fatalf("Remove on empty: %v", err) + } +} diff --git a/internal/buildsecrets/source.go b/internal/buildsecrets/source.go new file mode 100644 index 0000000..cb2de0a --- /dev/null +++ b/internal/buildsecrets/source.go @@ -0,0 +1,56 @@ +/* +Copyright 2025. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 +*/ + +// Package buildsecrets owns the operator's per-tenant build-time secret +// sync. Reconciler keeps a K8s Secret named `build-secrets` in opted-in +// site namespaces aligned with an upstream secret backend (AWS Secrets +// Manager today; other backends slot in by implementing Source). +package buildsecrets + +import ( + "context" + "errors" +) + +const ( + // SecretName is the K8s Secret consumed by the builder Job via + // envFrom (admin PR #3201 — `optional: true` so absence is a no-op). + SecretName = "build-secrets" + + // ManagedByLabel + FeatureLabel mark Secrets the operator created so + // it knows what is safe to update or delete. A Secret without these + // labels is treated as user-managed and left alone. + ManagedByLabel = "deco.sites/managed-by" + FeatureLabel = "deco.sites/feature" + + // KeyTemplate is the path convention in the upstream backend. AWS + // Secrets Manager stores `sites//build` as a JSON object whose + // keys/values land verbatim in the K8s Secret data. + KeyTemplate = "sites/%s/build" +) + +// ErrNotOwned signals the K8s Secret `build-secrets` exists in the +// namespace without the operator's labels. Sync and Remove refuse to +// touch it so we don't trample on a Secret a human created by hand. +var ErrNotOwned = errors.New("build-secrets Secret exists without operator labels; refusing to manage it") + +// Source abstracts the upstream secret backend behind a tiny shape that +// hides AWS-specifics from the reconciler. Implementations: +// +// - AWSSource (this package): AWS Secrets Manager via aws-sdk-go-v2 +// - future GCPSource / VaultSource: same interface, different SDK +// +// Get returns (data, true, nil) when the upstream key exists; (nil, +// false, nil) when it does not — *not* an error, just "no upstream +// data yet" (normal state for un-provisioned tenants). Network or +// permission failures bubble up via the error return. +type Source interface { + Get(ctx context.Context, key string) (data map[string]string, exists bool, err error) +} diff --git a/internal/controller/buildsecrets_controller.go b/internal/controller/buildsecrets_controller.go index 3593289..ebe0f33 100644 --- a/internal/controller/buildsecrets_controller.go +++ b/internal/controller/buildsecrets_controller.go @@ -12,10 +12,11 @@ package controller import ( "context" + "errors" "strings" + "time" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" @@ -31,48 +32,55 @@ import ( const ( buildSecretsAnnotation = "deco.sites/build-secrets-managed" buildSecretsAnnotationValue = "enabled" + + // DefaultBuildSecretsResyncPeriod is the safety-net interval at which + // the reconciler re-fetches the upstream backend even when nothing + // changed in the cluster — picks up out-of-band edits to AWS Secrets + // Manager. Configurable via BUILD_SECRETS_RESYNC_PERIOD. + DefaultBuildSecretsResyncPeriod = 15 * time.Minute ) -// +kubebuilder:rbac:groups=external-secrets.io,resources=externalsecrets,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch;create;update;patch;delete -// BuildSecretsReconciler keeps an ExternalSecret in sync with each -// opted-in site namespace. Opt-in is the annotation -// `deco.sites/build-secrets-managed: enabled` on the Namespace. +// BuildSecretsReconciler keeps the K8s Secret `build-secrets` in each +// opted-in site namespace aligned with the upstream backend (AWS +// Secrets Manager today; see buildsecrets.Source for the abstraction). // -// Removing the annotation (or deleting the Namespace) deletes the EE, -// which cascades into the K8s Secret via ESO's `creationPolicy: Owner`. -// Builds revert silently because admin's envFrom is `optional: true`. +// Opt-in is the annotation `deco.sites/build-secrets-managed: enabled` +// on the Namespace. The reconciler is fully event-driven (no polling) +// but requeues at ResyncPeriod as a safety net for upstream edits the +// operator did not observe (manual aws CLI rotation, etc.). // -// # Force sync recipes +// # State machine // -// Re-fetch ONE site from AWS Secrets Manager immediately, without -// waiting for the EE's refreshInterval: +// annotation off → ensure no operator-managed Secret exists +// annotation on, no upstream → no Secret (status: upstream-missing) +// annotation on, upstream → Secret created/updated with data // -// kubectl annotate es build-secrets -n sites- \ -// force-sync=$(date +%s) --overwrite +// # Force-sync recipes // -// Re-fetch ALL managed sites at once: +// Re-fetch ONE site from the upstream immediately (instead of waiting +// for ResyncPeriod): // -// kubectl get es -A -l deco.sites/feature=build-secrets -o name \ -// | xargs -I{} kubectl annotate {} force-sync=$(date +%s) --overwrite +// kubectl annotate ns sites- \ +// deco.sites/build-secrets-sync=$(date +%s) --overwrite // -// Bump the operator (re-applies the EE spec template across all -// managed namespaces — use after editing this controller): +// Re-fetch ALL managed sites at once: // // kubectl annotate ns -l deco.sites/build-secrets-managed=enabled \ // deco.sites/build-secrets-sync=$(date +%s) --overwrite type BuildSecretsReconciler struct { client.Client - Scheme *runtime.Scheme + Scheme *runtime.Scheme + Source buildsecrets.Source + ResyncPeriod time.Duration } func (r *BuildSecretsReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { log := logf.FromContext(ctx).WithName("buildsecrets").WithValues("namespace", req.Name) - // Derive site from namespace by stripping the `sites-` prefix. We do NOT - // reuse siteNameFromNamespace from namespace_controller.go because that - // helper also filters out Valkey-reserved usernames ("default", - // "redis-root"), which has no bearing on build-secrets reconciliation. + // Strip the `sites-` prefix inline; we deliberately avoid the + // valkey-specific helper that filters reserved usernames. site := strings.TrimPrefix(req.Name, siteNamespacePrefix) if site == req.Name || site == "" { return ctrl.Result{}, nil @@ -86,23 +94,29 @@ func (r *BuildSecretsReconciler) Reconcile(ctx context.Context, req ctrl.Request optedIn := ns.Annotations[buildSecretsAnnotation] == buildSecretsAnnotationValue if !optedIn || !ns.DeletionTimestamp.IsZero() { if err := buildsecrets.Remove(ctx, r.Client, ns.Name); err != nil { + if errors.Is(err, buildsecrets.ErrNotOwned) { + log.Info("Secret exists without operator labels — leaving it alone", "site", site) + return ctrl.Result{}, nil + } return ctrl.Result{}, err } - log.V(1).Info("ExternalSecret removed", "site", site) return ctrl.Result{}, nil } - if err := buildsecrets.Ensure(ctx, r.Client, ns.Name, site); err != nil { + if err := buildsecrets.Sync(ctx, r.Client, ns.Name, site, r.Source); err != nil { + if errors.Is(err, buildsecrets.ErrNotOwned) { + log.Info("Skipping unowned Secret build-secrets — operator will not adopt it", "site", site) + return ctrl.Result{RequeueAfter: r.ResyncPeriod}, nil + } return ctrl.Result{}, err } - log.V(1).Info("ExternalSecret ensured", "site", site) - return ctrl.Result{}, nil + return ctrl.Result{RequeueAfter: r.ResyncPeriod}, nil } func (r *BuildSecretsReconciler) SetupWithManager(mgr ctrl.Manager) error { - // Map ExternalSecret events back to the parent Namespace so deletions - // or out-of-band edits trigger a re-reconcile that restores the spec. - esToNamespace := handler.EnqueueRequestsFromMapFunc( + // Map Secret events back to the parent Namespace so deletions or + // out-of-band edits trigger a re-reconcile that restores state. + secretToNamespace := handler.EnqueueRequestsFromMapFunc( func(_ context.Context, obj client.Object) []reconcile.Request { if obj.GetName() != buildsecrets.SecretName { return nil @@ -113,13 +127,10 @@ func (r *BuildSecretsReconciler) SetupWithManager(mgr ctrl.Manager) error { }, ) - es := &unstructured.Unstructured{} - es.SetGroupVersionKind(buildsecrets.GVK) - return ctrl.NewControllerManagedBy(mgr). Named("buildsecrets"). For(&corev1.Namespace{}). - Watches(es, esToNamespace). + Watches(&corev1.Secret{}, secretToNamespace). WithOptions(controller.Options{MaxConcurrentReconciles: 4}). Complete(r) } From 82347d3b4db31f70ae1ec0cf1060cd9dc0736a5e Mon Sep 17 00:00:00 2001 From: Nicacio Oliveira Date: Wed, 20 May 2026 17:04:14 -0300 Subject: [PATCH 4/5] fix(buildsecrets): silence unparam lint on getSecret helper All tests use the same testNamespace; param was always 'sites-acme'. --- internal/buildsecrets/secret_test.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/internal/buildsecrets/secret_test.go b/internal/buildsecrets/secret_test.go index 8deee20..7bb862c 100644 --- a/internal/buildsecrets/secret_test.go +++ b/internal/buildsecrets/secret_test.go @@ -46,10 +46,12 @@ func newClient(t *testing.T, objs ...client.Object) client.Client { return fake.NewClientBuilder().WithScheme(scheme).WithObjects(objs...).Build() } -func getSecret(t *testing.T, c client.Client, namespace string) *corev1.Secret { +const testNamespace = "sites-acme" + +func getSecret(t *testing.T, c client.Client) *corev1.Secret { t.Helper() got := &corev1.Secret{} - if err := c.Get(context.Background(), types.NamespacedName{Name: SecretName, Namespace: namespace}, got); err != nil { + if err := c.Get(context.Background(), types.NamespacedName{Name: SecretName, Namespace: testNamespace}, got); err != nil { t.Fatalf("get secret: %v", err) } return got @@ -63,7 +65,7 @@ func TestSyncCreatesWhenUpstreamExists(t *testing.T) { t.Fatalf("Sync: %v", err) } - got := getSecret(t, c, "sites-acme") + got := getSecret(t, c) if string(got.Data["DENO_AUTH_TOKENS"]) != "github_pat_xxx@raw.githubusercontent.com" { t.Fatalf("data not written: %v", got.Data) } @@ -106,7 +108,7 @@ func TestSyncUpdatesOnDrift(t *testing.T) { t.Fatalf("Sync: %v", err) } - got := getSecret(t, c, "sites-acme") + got := getSecret(t, c) if _, ok := got.Data["OLD"]; ok { t.Fatal("old key not removed") } @@ -135,7 +137,7 @@ func TestSyncIdempotentWhenDataMatches(t *testing.T) { t.Fatalf("Sync: %v", err) } - got := getSecret(t, c, "sites-acme") + got := getSecret(t, c) if got.ResourceVersion != "999" { t.Fatalf("ResourceVersion changed (write should have been skipped): %s", got.ResourceVersion) } @@ -183,7 +185,7 @@ func TestSyncRefusesUnownedSecret(t *testing.T) { t.Fatalf("expected ErrNotOwned, got %v", err) } - got := getSecret(t, c, "sites-acme") + got := getSecret(t, c) if string(got.Data["USER_TOKEN"]) != "hand-crafted" { t.Fatalf("unowned secret data was mutated: %v", got.Data) } @@ -231,7 +233,7 @@ func TestRemoveRefusesUnowned(t *testing.T) { } // Secret still there - got := getSecret(t, c, "sites-acme") + got := getSecret(t, c) if got.Name != SecretName { t.Fatal("unowned secret was deleted") } From 152a4b66ef06f49112d5834a26d935fc891a51fd Mon Sep 17 00:00:00 2001 From: Nicacio Oliveira Date: Wed, 20 May 2026 23:07:58 -0300 Subject: [PATCH 5/5] fix(buildsecrets): reject JSON null payload from AWS Secrets Manager MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit json.Unmarshal('null') into a map[string]string leaves the map nil without erroring. Previously we'd return (nil, true, nil) and the reconciler would happily create an empty K8s Secret, which is not what the tenant meant by 'null' in their payload. Treat it as a malformed upstream and surface the error. Empty object {} stays valid — yields a non-nil empty map and a materialised but empty Secret. Identified by cubic. --- internal/buildsecrets/aws_source.go | 5 ++++ internal/buildsecrets/aws_source_test.go | 35 ++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/internal/buildsecrets/aws_source.go b/internal/buildsecrets/aws_source.go index 38d5d53..d07ff01 100644 --- a/internal/buildsecrets/aws_source.go +++ b/internal/buildsecrets/aws_source.go @@ -73,5 +73,10 @@ func (s *AWSSource) Get(ctx context.Context, key string) (map[string]string, boo if err := json.Unmarshal([]byte(*out.SecretString), &data); err != nil { return nil, false, fmt.Errorf("parse %q as JSON object of strings: %w", key, err) } + // Reject JSON `null` (Unmarshal leaves the map nil without erroring). + // An empty object {} is fine and yields a non-nil empty map. + if data == nil { + return nil, false, fmt.Errorf("secret %q payload is JSON null; expected an object of strings", key) + } return data, true, nil } diff --git a/internal/buildsecrets/aws_source_test.go b/internal/buildsecrets/aws_source_test.go index 3b9a5b1..29a4791 100644 --- a/internal/buildsecrets/aws_source_test.go +++ b/internal/buildsecrets/aws_source_test.go @@ -94,6 +94,41 @@ func TestAWSSourceGet_InvalidJSON(t *testing.T) { } } +func TestAWSSourceGet_NullJSONRejected(t *testing.T) { + src := &AWSSource{api: &mockSMClient{ + out: &secretsmanager.GetSecretValueOutput{ + SecretString: aws.String(`null`), + }, + }} + + _, _, err := src.Get(context.Background(), "sites/acme/build") + if err == nil { + t.Fatal("expected error for null payload, got nil") + } +} + +func TestAWSSourceGet_EmptyObjectAccepted(t *testing.T) { + src := &AWSSource{api: &mockSMClient{ + out: &secretsmanager.GetSecretValueOutput{ + SecretString: aws.String(`{}`), + }, + }} + + data, exists, err := src.Get(context.Background(), "sites/acme/build") + if err != nil { + t.Fatalf("Get: %v", err) + } + if !exists { + t.Fatal("empty object should be treated as existing") + } + if data == nil { + t.Fatal("empty object should yield non-nil empty map") + } + if len(data) != 0 { + t.Fatalf("data should be empty, got %v", data) + } +} + func TestAWSSourceGet_OtherErrorPropagates(t *testing.T) { sentinel := errors.New("boom") src := &AWSSource{api: &mockSMClient{err: sentinel}}