From 9b910f844c606fc642803f316e303628dc8e160d Mon Sep 17 00:00:00 2001 From: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Sun, 25 Oct 2026 11:16:38 -0400 Subject: [PATCH] fix(deploy): a key the environment declares may-be-empty is stored empty service-secrets.may-be-empty only tolerated an empty value already in the store; nothing ever wrote one. A property absent from the store that no file, generator or other key supplies was therefore required of the operator even when the environment had declared its correct value empty, and the only way through was a hand write to the store. The declaration now plans it as 'empty' and the apply stores it, so the ExternalSecret reading it materializes; a re-plan keeps it. Co-Authored-By: Claude Opus 5.5 --- pkg/deploysecrets/plan.go | 22 ++++++++++---- pkg/deploysecrets/plan_test.go | 55 ++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 5 deletions(-) diff --git a/pkg/deploysecrets/plan.go b/pkg/deploysecrets/plan.go index e25b932c..e36a31d1 100644 --- a/pkg/deploysecrets/plan.go +++ b/pkg/deploysecrets/plan.go @@ -64,6 +64,10 @@ const ( // ActionRequire: the property is absent and nothing can produce it; the // operator supplies it. ActionRequire Action = "require" + // ActionEmpty: the property is absent, nothing supplies it, and the + // environment declares in service-secrets.may-be-empty that its correct value + // here is empty; it is stored empty so the workload's secret materializes. + ActionEmpty Action = "empty" // ActionUnverified: the remote key exists but was not read, so whether it // holds the property is unknown. Source says what would happen if it does not. ActionUnverified Action = "unverified" @@ -273,7 +277,7 @@ func Build(ctx context.Context, in *Inputs) (*Plan, error) { for _, property := range state.secret.Properties { location := propertyLocation{state.secret.RemoteKey, property.Property} propertyPlan, value, err := planConfigured(state, property, states, generators, generated, in.Configured, - aliases[location], rotating) + aliases[location], rotating, in.MayBeEmpty) if err != nil { return nil, err } @@ -282,7 +286,7 @@ func Build(ctx context.Context, in *Inputs) (*Plan, error) { Source: "derived: the stored URL's password follows rotated " + strings.Join(rewrite.from, ", ")} value = rewrite.value } - if value != "" { + if value != "" || propertyPlan.Action == ActionEmpty { changes[property.Property] = value } secretPlan.Properties = append(secretPlan.Properties, propertyPlan) @@ -531,7 +535,7 @@ func refuseSharedSigningKey(state *remoteState, property gitops.RenderedSecretPr // only consulted for a property nothing holds. func planConfigured(state *remoteState, property gitops.RenderedSecretProperty, states []*remoteState, generators map[string]environments.EnvironmentSecretGenerator, generated map[string]string, - configured map[string]ConfiguredValue, keys []string, rotating map[string]bool) (PropertyPlan, string, error) { + configured map[string]ConfiguredValue, keys []string, rotating map[string]bool, mayBeEmpty []string) (PropertyPlan, string, error) { lookup := property lookup.Keys = keys declaredKey, generator, err := declaredGenerator(lookup, generators) @@ -596,7 +600,7 @@ func planConfigured(state *remoteState, property gitops.RenderedSecretProperty, return PropertyPlan{Property: property.Property, Keys: property.Keys, Action: ActionKeep, Source: "stored"}, "", nil } } - fallback, value, err := fallbackSource(lookup, generators, generated, configuredValue, hasConfigured) + fallback, value, err := fallbackSource(lookup, generators, generated, configuredValue, hasConfigured, mayBeEmpty) if err != nil { return PropertyPlan{}, "", err } @@ -636,7 +640,7 @@ func planConfigured(state *remoteState, property gitops.RenderedSecretProperty, // A property read as two keys covered by two different generators has no answer: // whichever won would be an accident of ordering, so it is refused. func fallbackSource(property gitops.RenderedSecretProperty, generators map[string]environments.EnvironmentSecretGenerator, - generated map[string]string, configured ConfiguredValue, hasConfigured bool) (PropertyPlan, string, error) { + generated map[string]string, configured ConfiguredValue, hasConfigured bool, mayBeEmpty []string) (PropertyPlan, string, error) { if hasConfigured { return PropertyPlan{Property: property.Property, Keys: property.Keys, Action: ActionConfigured, Source: "configured in " + configured.File}, configured.value, nil @@ -645,6 +649,14 @@ func fallbackSource(property gitops.RenderedSecretProperty, generators map[strin if err != nil { return PropertyPlan{}, "", err } + if declaredKey == "" && declaredEmpty(property, mayBeEmpty) { + // The declaration is the value: an environment that says a key is + // legitimately empty here has said what to store. Requiring it instead + // leaves the property absent, and the ExternalSecret reading it never + // materializes its Secret. + return PropertyPlan{Property: property.Property, Keys: property.Keys, Action: ActionEmpty, + Source: "service-secrets.may-be-empty declares " + strings.Join(property.Keys, ", ") + " empty here"}, "", nil + } if declaredKey == "" { return PropertyPlan{Property: property.Property, Keys: property.Keys, Action: ActionRequire, Source: "no key holds it, no configuration file supplies it, and service-secrets.generate declares no generator for " + diff --git a/pkg/deploysecrets/plan_test.go b/pkg/deploysecrets/plan_test.go index 78abee51..0a8635f7 100644 --- a/pkg/deploysecrets/plan_test.go +++ b/pkg/deploysecrets/plan_test.go @@ -583,6 +583,61 @@ func TestBuildAcceptsAnEmptyPropertyTheEnvironmentDeclares(t *testing.T) { } } +// A key the environment declares empty and nothing holds is written empty, not +// required: the declaration says what the value is, and leaving the property +// absent keeps the ExternalSecret reading it from ever materializing. +func TestBuildWritesADeclaredEmptyPropertyNothingHolds(t *testing.T) { + rendered := fixture() + documents := seeded() + delete(documents["host-accounts"], clientSecret) + store := newFakeStore(documents) + ctx := context.Background() + plan, err := Build(ctx, &Inputs{Rendered: rendered, + Generators: generators(), Services: []string{"tasks/store", "tasks/worker"}, + Store: store, ReadPayloads: true, MayBeEmpty: []string{clientSecret}}) + if err != nil { + t.Fatal(err) + } + for _, required := range plan.Required() { + if strings.Contains(required, clientSecret) { + t.Fatalf("Required() names %s, want it planned empty: the environment declares it", clientSecret) + } + } + if _, err = plan.Apply(ctx, store, true); err != nil { + t.Fatal(err) + } + value, present := store.documents["host-accounts"][clientSecret] + if !present || value != "" { + t.Errorf("apply stored (%q, present=%v), want the property present and empty", value, present) + } + // A second plan keeps it: the stored empty is the declared one. + again, err := Build(ctx, &Inputs{Rendered: rendered, + Generators: generators(), Services: []string{"tasks/store", "tasks/worker"}, + Store: store, ReadPayloads: true, MayBeEmpty: []string{clientSecret}}) + if err != nil { + t.Fatal(err) + } + if slices.Contains(again.Changes(), "host-accounts") { + t.Errorf("a re-plan rewrites host-accounts, want the stored empty kept") + } +} + +// Without the declaration the same absent property is still the operator's. +func TestBuildRequiresAnAbsentPropertyNotDeclaredEmpty(t *testing.T) { + rendered := fixture() + documents := seeded() + delete(documents["host-accounts"], clientSecret) + plan, err := Build(context.Background(), &Inputs{Rendered: rendered, + Generators: generators(), Services: []string{"tasks/store", "tasks/worker"}, + Store: newFakeStore(documents), ReadPayloads: true}) + if err != nil { + t.Fatal(err) + } + if !slices.ContainsFunc(plan.Required(), func(required string) bool { return strings.Contains(required, clientSecret) }) { + t.Errorf("Required() = %v, want the undeclared %s required", plan.Required(), clientSecret) + } +} + // Declaring one of a property's keys is not declaring the property: a property // two services read under two keys is only safely empty when neither needs one. func TestBuildStillRefusesWhenOnlySomeOfAPropertysKeysAreDeclared(t *testing.T) {