From 3d120c2d2426b35fe8b27c89aad13971045bbc3c Mon Sep 17 00:00:00 2001 From: Jeroen Soeters Date: Fri, 28 Aug 2026 23:27:10 -0700 Subject: [PATCH 1/5] feat(schema): add the generator forma kind Introduces Generator as a new top-level forma kind alongside Stacks, Targets, Resources, and Policies: a source of a generated value that secrets will later reference. It belongs to a stack (like a resource) but has no target, NativeID, provider, or Read, and is referenced by other nodes rather than attached to a stack (like a policy). This slice only declares and round-trips the kind through all five ingestion surfaces (the forma union, FormaRender, the JSON schema plugin, and the extraction generator) plus the Go model. Nothing schedules, resolves, or rotates a generated value yet. Adds the first concrete generator, PasswordGenerator, with eval-time validation so an unsatisfiable spec (every character class disabled, or excludeCharacters emptying an enabled class) fails at PKL eval rather than during unattended rejection sampling later. --- internal/schema/json/json.go | 20 ++-- .../schema/pkl/generator/pklGenerator.pkl | 85 +++++++++++++- internal/schema/pkl/pkl_generate_test.go | 61 ++++++++++ internal/schema/pkl/pkl_test.go | 49 ++++++++ internal/schema/pkl/schema/forma.pkl | 3 +- internal/schema/pkl/schema/formae.pkl | 81 +++++++++++++ .../forma/generator_all_flags_false_test.pkl | 29 +++++ .../generator_exclude_empties_class_test.pkl | 30 +++++ .../pkl/testdata/forma/generator_test.pkl | 31 +++++ pkg/model/forma.go | 44 ++++++- pkg/model/forma_test.go | 108 ++++++++++++++++++ pkg/model/generator.go | 77 +++++++++++++ pkg/model/generator_test.go | 103 +++++++++++++++++ 13 files changed, 709 insertions(+), 12 deletions(-) create mode 100644 internal/schema/pkl/testdata/forma/generator_all_flags_false_test.pkl create mode 100644 internal/schema/pkl/testdata/forma/generator_exclude_empties_class_test.pkl create mode 100644 internal/schema/pkl/testdata/forma/generator_test.pkl create mode 100644 pkg/model/forma_test.go create mode 100644 pkg/model/generator.go create mode 100644 pkg/model/generator_test.go diff --git a/internal/schema/json/json.go b/internal/schema/json/json.go index 4443fac58..1810d7945 100644 --- a/internal/schema/json/json.go +++ b/internal/schema/json/json.go @@ -74,17 +74,19 @@ func (j JSON) SerializeForma(forma *model.Forma, options *schema.SerializeOption data = simplifiedResources } else { - // Full structure with Stacks, Targets, Resources, and Policies + // Full structure with Stacks, Targets, Resources, Policies, and Generators data = struct { - Stacks []model.Stack `json:"Stacks,omitempty"` - Targets []model.Target `json:"Targets,omitempty"` - Policies []json.RawMessage `json:"Policies,omitempty"` - Resources []model.Resource `json:"Resources,omitempty"` + Stacks []model.Stack `json:"Stacks,omitempty"` + Targets []model.Target `json:"Targets,omitempty"` + Policies []json.RawMessage `json:"Policies,omitempty"` + Generators []json.RawMessage `json:"Generators,omitempty"` + Resources []model.Resource `json:"Resources,omitempty"` }{ - Stacks: forma.Stacks, - Targets: forma.Targets, - Policies: forma.Policies, - Resources: forma.Resources, + Stacks: forma.Stacks, + Targets: forma.Targets, + Policies: forma.Policies, + Generators: forma.Generators, + Resources: forma.Resources, } } diff --git a/internal/schema/pkl/generator/pklGenerator.pkl b/internal/schema/pkl/generator/pklGenerator.pkl index 3fd853c0a..9aaf66995 100644 --- a/internal/schema/pkl/generator/pklGenerator.pkl +++ b/internal/schema/pkl/generator/pklGenerator.pkl @@ -255,6 +255,60 @@ function parsePolicies(policiesData: List>): String = )) policiesString.join("\n") +/// Parses standalone generators and generates PKL output. Mirrors +/// parsePolicies: generators are always declared standalone (never nested +/// inside a stack block), but unlike policies they carry their own `stack` +/// reference, resolved here the same way parseResources resolves +/// target/stack references on resources. +function parseGenerators(generatorsData: List>, stackLabelMap: Map): String = + let (generatorsString = generatorsData.map((generatorData) -> + let (generatorType = generatorData["Type"] as String) + let (label = generatorData["Label"] as String) + let (camelCaseLabel = toCamelCase(label)) + let (stackLabel = generatorData.getOrNull("Stack") as String?) + let (stackLine = if (stackLabel != null) + "\n stack = \(stackLabelMap.getOrNull(stackLabel) ?? toCamelCase(stackLabel)).res" + else + "" + ) + let (everySeconds = generatorData.getOrNull("EverySeconds") as Number?) + let (rotationLine = if (everySeconds != null) + "\n rotation { every = \(everySeconds).s }" + else + "" + ) + + if (generatorType == "password") + let (length = generatorData["Length"] as Number) + let (uppercase = generatorData["Uppercase"] as Boolean) + let (lowercase = generatorData["Lowercase"] as Boolean) + let (digits = generatorData["Digits"] as Boolean) + let (symbols = generatorData["Symbols"] as Boolean) + let (excludeCharacters = generatorData.getOrNull("ExcludeCharacters") as String? ?? "") + let (requireEachIncludedType = generatorData.getOrNull("RequireEachIncludedType") as Boolean? ?? true) + """ + + local \(camelCaseLabel) = new formae.PasswordGenerator { + label = "\(label)"\(stackLine)\(rotationLine) + length = \(length) + uppercase = \(uppercase) + lowercase = \(lowercase) + digits = \(digits) + symbols = \(symbols) + excludeCharacters = "\(excludeCharacters)" + requireEachIncludedType = \(requireEachIncludedType) + } + \(camelCaseLabel) + """ + else + // Unknown generator type - skip with comment + """ + + // Skipped unknown generator type: \(generatorType) (label: \(label)) + """ + )) + generatorsString.join("\n") + function parseTargets(targetsData: List>): String = let (targetsString = targetsData.map((targetData) -> let (label = targetData["label"]) @@ -393,6 +447,26 @@ function generateFormaFile(parsed: json.Value): String = List() ) + // Extract standalone generators from the parsed JSON + let (generatorsFromJson = if (parsed.getPropertyOrNull("Generators") != null) + parsed.Generators.toList().map((generator) -> Map( + "Type", generator.Type, + "Label", generator.Label, + "Stack", generator.getPropertyOrNull("Stack"), + "EverySeconds", generator.getPropertyOrNull("EverySeconds"), + "Length", generator.getPropertyOrNull("Length"), + "Uppercase", generator.getPropertyOrNull("Uppercase"), + "Lowercase", generator.getPropertyOrNull("Lowercase"), + "Digits", generator.getPropertyOrNull("Digits"), + "Symbols", generator.getPropertyOrNull("Symbols"), + "ExcludeCharacters", generator.getPropertyOrNull("ExcludeCharacters"), + "RequireEachIncludedType", generator.getPropertyOrNull("RequireEachIncludedType") + ) + ) + else + List() + ) + // Create mapping from original labels to camelCase labels for targets let (targetLabelMap = targetsFromJson.fold(Map(), (acc: Map, targetData) -> let (originalLabel = targetData["label"]) @@ -431,6 +505,15 @@ function generateFormaFile(parsed: json.Value): String = "" ) + // Generate generators section (only if there are generators). Placed + // after stacks (it emits `stack = .res` references) and + // before resources. + let (generatorsSection = if (generatorsFromJson.length > 0) + parseGenerators(generatorsFromJson, stackLabelMap) + "\n" + else + "" + ) + """ \(headerImports()) @@ -439,7 +522,7 @@ function generateFormaFile(parsed: json.Value): String = forma {\(policiesSection) \(parseStacks(stacksFromJson, policyLabelMap)) - + \(generatorsSection) \(parseTargets(targetsFromJson)) \(parseResources(resources, targetLabelMap, stackLabelMap)) diff --git a/internal/schema/pkl/pkl_generate_test.go b/internal/schema/pkl/pkl_generate_test.go index 1ba71f5b8..155c90f30 100644 --- a/internal/schema/pkl/pkl_generate_test.go +++ b/internal/schema/pkl/pkl_generate_test.go @@ -7,6 +7,7 @@ package pkl import ( + "encoding/json" "os" "path/filepath" "strings" @@ -115,3 +116,63 @@ func TestGenerateSourceCode_HashedSecretCount_ZeroForNonHashed(t *testing.T) { assert.Equal(t, 0, res.HashedSecretCount, "HashedSecretCount must be 0 when no hashed opaque fields are present") } + +// TestGenerateSourceCode_Generator_RoundTrips verifies that a forma carrying +// a standalone generator (as it would when re-serialized from stored state, +// the same way standalone policies already are) round-trips through +// GenerateSourceCode into a .pkl file that declares an equivalent +// formae.PasswordGenerator referencing its stack. +func TestGenerateSourceCode_Generator_RoundTrips(t *testing.T) { + deps, pluginDir := fakeawsDeps(t) + + forma := &model.Forma{ + Stacks: []model.Stack{{Label: "default"}}, + Targets: []model.Target{fakeawsTarget()}, + Resources: []model.Resource{{ + Label: "plain-secret", + Type: "FakeAWS::SecretsManager::Secret", + Stack: "default", + Target: "aws", + Properties: []byte(`{"SecretString":{"$value":"plaintext","$visibility":"Opaque","$strategy":"Update"}}`), + }}, + Generators: []json.RawMessage{ + []byte(`{ + "Type": "password", + "Label": "db-password", + "Stack": "default", + "EverySeconds": 2592000, + "Length": 24, + "Uppercase": true, + "Lowercase": true, + "Digits": true, + "Symbols": false, + "ExcludeCharacters": "oO0", + "RequireEachIncludedType": true + }`), + }, + } + + dir := t.TempDir() + targetPath := filepath.Join(dir, "out.pkl") + + options := &schema.SerializeOptions{ + Schema: "pkl", + SchemaLocation: schema.SchemaLocationLocal, + LocalPluginDir: pluginDir, + Dependencies: deps, + } + + _, err := PKL{}.GenerateSourceCode(forma, targetPath, nil, options) + require.NoError(t, err) + + written, err := os.ReadFile(targetPath) + require.NoError(t, err) + generated := string(written) + + assert.Contains(t, generated, "new formae.PasswordGenerator {") + assert.Contains(t, generated, `label = "db-password"`) + assert.Contains(t, generated, "stack = default.res") + assert.Contains(t, generated, "rotation { every = 2592000.s }") + assert.Contains(t, generated, "length = 24") + assert.Contains(t, generated, `excludeCharacters = "oO0"`) +} diff --git a/internal/schema/pkl/pkl_test.go b/internal/schema/pkl/pkl_test.go index 2efc3a092..2b7f0f444 100644 --- a/internal/schema/pkl/pkl_test.go +++ b/internal/schema/pkl/pkl_test.go @@ -269,6 +269,55 @@ func TestPkl_SecretShapeMisuse_BareMapSecretValueFailsEval(t *testing.T) { assert.ErrorContains(t, err, "SecretMapAccessor") } +// TestPkl_Generator_Evaluate verifies that a forma declaring PasswordGenerator +// instances evaluates and renders a Generators listing carrying the fields +// PasswordGenerator.render() produces, including EverySeconds when a +// rotation cadence is attached and its absence when it is not. +func TestPkl_Generator_Evaluate(t *testing.T) { + p := PKL{} + forma, err := p.Evaluate("./testdata/forma/generator_test.pkl", model.CommandApply, model.FormaApplyModeReconcile, nil) + require.NoError(t, err) + + jsonString := forma.ToJSON() + + assert.Equal(t, "password", gjson.Get(jsonString, "Generators.0.Type").String()) + assert.Equal(t, "db-password", gjson.Get(jsonString, "Generators.0.Label").String()) + assert.Equal(t, "generator-test-stack", gjson.Get(jsonString, "Generators.0.Stack").String()) + assert.Equal(t, int64(24), gjson.Get(jsonString, "Generators.0.Length").Int()) + assert.True(t, gjson.Get(jsonString, "Generators.0.Uppercase").Bool()) + assert.True(t, gjson.Get(jsonString, "Generators.0.Lowercase").Bool()) + assert.True(t, gjson.Get(jsonString, "Generators.0.Digits").Bool()) + assert.False(t, gjson.Get(jsonString, "Generators.0.Symbols").Bool()) + assert.Equal(t, "oO0", gjson.Get(jsonString, "Generators.0.ExcludeCharacters").String()) + assert.True(t, gjson.Get(jsonString, "Generators.0.RequireEachIncludedType").Bool()) + assert.False(t, gjson.Get(jsonString, "Generators.0.EverySeconds").Exists(), + "EverySeconds must be absent when no rotation is attached") + + assert.Equal(t, "rotating-password", gjson.Get(jsonString, "Generators.1.Label").String()) + assert.Equal(t, int64(2592000), gjson.Get(jsonString, "Generators.1.EverySeconds").Int(), + "a 30-day rotation must render as 2592000 seconds") +} + +// TestPkl_Generator_AllClassFlagsFalseFailsEval verifies that a +// PasswordGenerator with every character-class flag false fails at PKL eval, +// not at runtime — the spec has no alphabet to draw from. +func TestPkl_Generator_AllClassFlagsFalseFailsEval(t *testing.T) { + p := PKL{} + _, err := p.Evaluate("./testdata/forma/generator_all_flags_false_test.pkl", model.CommandApply, model.FormaApplyModeReconcile, nil) + require.Error(t, err) + assert.ErrorContains(t, err, "at least one of uppercase, lowercase, digits, symbols must be true") +} + +// TestPkl_Generator_ExcludeCharactersEmptiesClassFailsEval verifies that +// excludeCharacters removing every character of an enabled class fails at +// PKL eval, not at runtime. +func TestPkl_Generator_ExcludeCharactersEmptiesClassFailsEval(t *testing.T) { + p := PKL{} + _, err := p.Evaluate("./testdata/forma/generator_exclude_empties_class_test.pkl", model.CommandApply, model.FormaApplyModeReconcile, nil) + require.Error(t, err) + assert.ErrorContains(t, err, "excludeCharacters removes every digit") +} + func TestTranslateResourcePluginConfig(t *testing.T) { p := PKL{} config, err := p.FormaeConfig("./testdata/config/test_resource_plugin_config.pkl") diff --git a/internal/schema/pkl/schema/forma.pkl b/internal/schema/pkl/schema/forma.pkl index f5492aff1..64bfbd98b 100644 --- a/internal/schema/pkl/schema/forma.pkl +++ b/internal/schema/pkl/schema/forma.pkl @@ -14,7 +14,7 @@ properties: Any? /// fill: entry point used by the self-injecting output below; constructs a /// typed instance of the user's properties class from external `prop:` values. function fill(clazz: Class): Typed = formae.fillProps(clazz) -hidden forma: Listing +hidden forma: Listing /// defaultStack: returns the first defined Stack hidden defaultStack: formae.StackResolvable = @@ -56,6 +56,7 @@ output { Stacks = forma.toList().filterIsInstance(formae.Stack).toListing() Targets = forma.toList().filterIsInstance(formae.Target).toListing() Policies = forma.toList().filterIsInstance(formae.Policy).map((p) -> p.render()).toListing() + Generators = forma.toList().filterIsInstance(formae.Generator).map((g) -> g.render()).toListing() Resolvables = forma.toList().filterIsInstance(formae.Resolvable).toListing() Resources = new Listing { for (res in forma.toList().filterIsInstance(formae.Resource).toListing()) { diff --git a/internal/schema/pkl/schema/formae.pkl b/internal/schema/pkl/schema/formae.pkl index 27e07eec0..178faeb12 100644 --- a/internal/schema/pkl/schema/formae.pkl +++ b/internal/schema/pkl/schema/formae.pkl @@ -167,6 +167,86 @@ open class AutoReconcilePolicy extends Policy { } } +/// A source of generated values. Belongs to a stack; referenced by the +/// resources whose properties take the generated value. +abstract class Generator { + label: String + stack: StackResolvable? + /// When absent the generator produces a value on first apply and never + /// again on its own. + hidden rotation: RotationSpec? + + abstract function type(): String + abstract function render(): Dynamic +} + +/// How often the agent produces a new value for a generator. +open class RotationSpec { + /// Required: there is no default cadence, so attaching rotation always + /// states the interval. + every: Duration +} + +open class PasswordGenerator extends Generator { + length: Int(this >= 16) = 32 + uppercase: Boolean = true + lowercase: Boolean = true + digits: Boolean = true + symbols: Boolean = false + /// Characters the destination cannot accept, removed from the alphabet. + excludeCharacters: String = "" + /// Guarantee at least one character from every enabled class. + requireEachIncludedType: Boolean = true + + local self = this + + // Canonical alphabets, before excludeCharacters is applied. Eval-time + // validation below checks emptiness against these; the scheduler that + // actually draws characters must draw from the same sets, or a spec this + // accepts could still starve during unattended rejection sampling. + local uppercaseChars: String = "ABCDEFGHIJKLMNOPQRSTUVWXYZ" + local lowercaseChars: String = "abcdefghijklmnopqrstuvwxyz" + local digitChars: String = "0123456789" + local symbolChars: String = "!@#$%^&*()-_=+[]{}<>?/|~" + + local function remaining(alphabet: String): String = + alphabet.chars.filter((c) -> !self.excludeCharacters.contains(c)).join("") + + // Eval-time validation: a spec the scheduler could never satisfy (every + // class flag false, or excludeCharacters emptying an enabled class) fails + // here, not during unattended rejection sampling later. Referenced from + // render()'s return type so it is always forced, even though PKL would + // otherwise never evaluate an unread local. + local function validated(): Boolean = + if (!(self.uppercase || self.lowercase || self.digits || self.symbols)) + throw("PasswordGenerator \"\(self.label)\": at least one of uppercase, lowercase, digits, symbols must be true") + else if (self.uppercase && remaining(uppercaseChars).isEmpty) + throw("PasswordGenerator \"\(self.label)\": excludeCharacters removes every uppercase character") + else if (self.lowercase && remaining(lowercaseChars).isEmpty) + throw("PasswordGenerator \"\(self.label)\": excludeCharacters removes every lowercase character") + else if (self.digits && remaining(digitChars).isEmpty) + throw("PasswordGenerator \"\(self.label)\": excludeCharacters removes every digit") + else if (self.symbols && remaining(symbolChars).isEmpty) + throw("PasswordGenerator \"\(self.label)\": excludeCharacters removes every symbol character") + else + true + + function type(): String = "password" + function render(): Dynamic(validated()) = new { + Type = "password" + Label = self.label + Stack = self.stack + EverySeconds = self.rotation?.every?.toUnit("s")?.value?.toInt() + Length = length + Uppercase = uppercase + Lowercase = lowercase + Digits = digits + Symbols = symbols + ExcludeCharacters = excludeCharacters + RequireEachIncludedType = requireEachIncludedType + } +} + /// DEPRECATED: Use aws#Tag instead. /// This class will be removed in a future version. /// AWS resources should use `aws.Tag` from `@aws/aws.pkl`. @@ -660,6 +740,7 @@ class FormaRender { Stacks: Listing Targets: Listing Policies: Listing? + Generators: Listing? Resources: Listing(validate(this)) hidden Resolvables: Listing diff --git a/internal/schema/pkl/testdata/forma/generator_all_flags_false_test.pkl b/internal/schema/pkl/testdata/forma/generator_all_flags_false_test.pkl new file mode 100644 index 000000000..b92c306a8 --- /dev/null +++ b/internal/schema/pkl/testdata/forma/generator_all_flags_false_test.pkl @@ -0,0 +1,29 @@ +/* + * © 2025 Platform Engineering Labs Inc. + * + * SPDX-License-Identifier: FSL-1.1-ALv2 + */ + +// Shape-misuse test: a PasswordGenerator with every character-class flag set +// to false has no alphabet to draw from at all. PKL eval fails because the +// scheduler that later reads this spec runs unattended and would otherwise +// spin forever in rejection sampling. + +amends "@formae/forma.pkl" +import "@formae/formae.pkl" + +forma { + local testStack = new formae.Stack { + label = "generator-test-stack" + } + testStack + + new formae.PasswordGenerator { + label = "unsatisfiable" + stack = testStack.res + uppercase = false + lowercase = false + digits = false + symbols = false + } +} diff --git a/internal/schema/pkl/testdata/forma/generator_exclude_empties_class_test.pkl b/internal/schema/pkl/testdata/forma/generator_exclude_empties_class_test.pkl new file mode 100644 index 000000000..153f82f9c --- /dev/null +++ b/internal/schema/pkl/testdata/forma/generator_exclude_empties_class_test.pkl @@ -0,0 +1,30 @@ +/* + * © 2025 Platform Engineering Labs Inc. + * + * SPDX-License-Identifier: FSL-1.1-ALv2 + */ + +// Shape-misuse test: excludeCharacters removes every digit while digits is +// the only enabled class, leaving nothing for the enabled class to draw +// from. PKL eval fails at the same eval-time check as the all-flags-false +// case, for the same unattended-scheduler reason. + +amends "@formae/forma.pkl" +import "@formae/formae.pkl" + +forma { + local testStack = new formae.Stack { + label = "generator-test-stack" + } + testStack + + new formae.PasswordGenerator { + label = "starved-digits" + stack = testStack.res + uppercase = false + lowercase = false + digits = true + symbols = false + excludeCharacters = "0123456789" + } +} diff --git a/internal/schema/pkl/testdata/forma/generator_test.pkl b/internal/schema/pkl/testdata/forma/generator_test.pkl new file mode 100644 index 000000000..00ddca61e --- /dev/null +++ b/internal/schema/pkl/testdata/forma/generator_test.pkl @@ -0,0 +1,31 @@ +/* + * © 2025 Platform Engineering Labs Inc. + * + * SPDX-License-Identifier: FSL-1.1-ALv2 + */ + +amends "@formae/forma.pkl" +import "@formae/formae.pkl" + +forma { + local testStack = new formae.Stack { + label = "generator-test-stack" + } + testStack + + new formae.PasswordGenerator { + label = "db-password" + stack = testStack.res + length = 24 + excludeCharacters = "oO0" + } + + new formae.PasswordGenerator { + label = "rotating-password" + stack = testStack.res + symbols = true + rotation { + every = 30.d + } + } +} diff --git a/pkg/model/forma.go b/pkg/model/forma.go index f621f41a3..9c341f966 100644 --- a/pkg/model/forma.go +++ b/pkg/model/forma.go @@ -14,7 +14,8 @@ type Forma struct { Stacks []Stack `json:"Stacks,omitempty"` Targets []Target `json:"Targets,omitempty"` Resources []Resource `json:"Resources,omitempty"` - Policies []json.RawMessage `json:"Policies,omitempty"` // Standalone policies + Policies []json.RawMessage `json:"Policies,omitempty"` // Standalone policies + Generators []json.RawMessage `json:"Generators,omitempty"` // Generators, keyed to a stack by their own Stack field } type Prop struct { @@ -73,6 +74,47 @@ func (f *Forma) SplitByStack() []Forma { } } + // Generators are raw JSON (like Policies), so their Stack has to be read + // out rather than accessed as a Go field. Unlike Policies, a generator + // names its own stack directly (mirroring Resource.Stack), so — unlike + // Policies, which this function drops entirely today — each generator is + // routed to the stack it names. + for _, raw := range f.Generators { + var header struct { + Stack string `json:"Stack"` + } + if err := json.Unmarshal(raw, &header); err != nil { + continue // malformed generator; nothing sane to route it to + } + + if existing, ok := stacks[header.Stack]; ok { + existing.Generators = append(existing.Generators, raw) + continue + } + + var stack *Stack + for _, s := range f.Stacks { + if s.Label == header.Stack { + stack = &s + } + } + + if stack != nil { + stacks[header.Stack] = &Forma{ + Properties: f.Properties, + Stacks: []Stack{*stack}, + Generators: []json.RawMessage{raw}, + } + } else { + // Not present in forma.Stacks - create a minimal stack + stacks[header.Stack] = &Forma{ + Properties: f.Properties, + Stacks: []Stack{{Label: header.Stack}}, + Generators: []json.RawMessage{raw}, + } + } + } + for _, value := range stacks { result = append(result, *value) } diff --git a/pkg/model/forma_test.go b/pkg/model/forma_test.go new file mode 100644 index 000000000..2c338cbe1 --- /dev/null +++ b/pkg/model/forma_test.go @@ -0,0 +1,108 @@ +// © 2025 Platform Engineering Labs Inc. +// +// SPDX-License-Identifier: FSL-1.1-ALv2 + +package model + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestSplitByStack_GeneratorLandsOnNamedStack verifies that a generator +// belonging to a stack that also has resources is carried into that stack's +// split Forma, alongside the resources already routed there. +func TestSplitByStack_GeneratorLandsOnNamedStack(t *testing.T) { + generator := json.RawMessage(`{ + "Type": "password", + "Label": "db-password", + "Stack": "app", + "Length": 24, + "Uppercase": true, + "Lowercase": true, + "Digits": true, + "Symbols": false, + "RequireEachIncludedType": true + }`) + + forma := Forma{ + Stacks: []Stack{{Label: "app"}}, + Resources: []Resource{ + {Label: "bucket", Type: "FakeAWS::S3::Bucket", Stack: "app", Target: "aws"}, + }, + Generators: []json.RawMessage{generator}, + } + + split := forma.SplitByStack() + require.Len(t, split, 1) + + appForma := split[0] + require.Len(t, appForma.Resources, 1) + require.Len(t, appForma.Generators, 1) + assert.JSONEq(t, string(generator), string(appForma.Generators[0])) +} + +// TestSplitByStack_GeneratorOnlyStack verifies that a generator whose stack +// carries no resources still produces a split Forma for that stack — a +// generator-only stack is a legitimate shape, not just an appendage to a +// resource-bearing one. +func TestSplitByStack_GeneratorOnlyStack(t *testing.T) { + generator := json.RawMessage(`{ + "Type": "password", + "Label": "seed", + "Stack": "secrets", + "Length": 16, + "Uppercase": true, + "Lowercase": true, + "Digits": true, + "Symbols": false, + "RequireEachIncludedType": true + }`) + + forma := Forma{ + Stacks: []Stack{{Label: "secrets", Description: "generator-only stack"}}, + Generators: []json.RawMessage{generator}, + } + + split := forma.SplitByStack() + require.Len(t, split, 1) + + secretsForma := split[0] + assert.Empty(t, secretsForma.Resources) + require.Len(t, secretsForma.Generators, 1) + assert.JSONEq(t, string(generator), string(secretsForma.Generators[0])) + require.Len(t, secretsForma.Stacks, 1) + assert.Equal(t, "secrets", secretsForma.Stacks[0].Label) + assert.Equal(t, "generator-only stack", secretsForma.Stacks[0].Description) +} + +// TestSplitByStack_TwoGeneratorsDifferentStacks verifies that generators +// naming different stacks are routed to their own split Forma, not merged. +func TestSplitByStack_TwoGeneratorsDifferentStacks(t *testing.T) { + genA := json.RawMessage(`{"Type": "password", "Label": "a", "Stack": "stack-a", "Length": 16, "Uppercase": true, "Lowercase": true, "Digits": true, "Symbols": false, "RequireEachIncludedType": true}`) + genB := json.RawMessage(`{"Type": "password", "Label": "b", "Stack": "stack-b", "Length": 16, "Uppercase": true, "Lowercase": true, "Digits": true, "Symbols": false, "RequireEachIncludedType": true}`) + + forma := Forma{ + Stacks: []Stack{{Label: "stack-a"}, {Label: "stack-b"}}, + Generators: []json.RawMessage{genA, genB}, + } + + split := forma.SplitByStack() + require.Len(t, split, 2) + + byStack := map[string]Forma{} + for _, f := range split { + require.Len(t, f.Stacks, 1) + byStack[f.Stacks[0].Label] = f + } + + require.Contains(t, byStack, "stack-a") + require.Contains(t, byStack, "stack-b") + require.Len(t, byStack["stack-a"].Generators, 1) + require.Len(t, byStack["stack-b"].Generators, 1) + assert.JSONEq(t, string(genA), string(byStack["stack-a"].Generators[0])) + assert.JSONEq(t, string(genB), string(byStack["stack-b"].Generators[0])) +} diff --git a/pkg/model/generator.go b/pkg/model/generator.go new file mode 100644 index 000000000..fe7262208 --- /dev/null +++ b/pkg/model/generator.go @@ -0,0 +1,77 @@ +// © 2025 Platform Engineering Labs Inc. +// +// SPDX-License-Identifier: FSL-1.1-ALv2 + +package model + +import ( + "encoding/json" + "fmt" +) + +// Generator is a source of a generated value that secrets will later +// reference. It is neither a resource nor a policy: it has no target, no +// NativeID, no provider and no Read, so it is never discovered and never +// drifts. It belongs to a stack, exactly as a resource does. +type Generator interface { + GetLabel() string + GetType() string + GetStack() string + SetStack(stack string) +} + +// PasswordGenerator produces a random password value. Fields mirror the +// PKL PasswordGenerator.render() output. +type PasswordGenerator struct { + Type string `json:"Type"` // "password" + Label string `json:"Label"` + Stack string `json:"Stack,omitempty"` + EverySeconds *int64 `json:"EverySeconds,omitempty"` + Length int `json:"Length"` + Uppercase bool `json:"Uppercase"` + Lowercase bool `json:"Lowercase"` + Digits bool `json:"Digits"` + Symbols bool `json:"Symbols"` + ExcludeCharacters string `json:"ExcludeCharacters,omitempty"` + RequireEachIncludedType bool `json:"RequireEachIncludedType"` +} + +func (g *PasswordGenerator) GetLabel() string { return g.Label } +func (g *PasswordGenerator) GetType() string { return "password" } +func (g *PasswordGenerator) GetStack() string { return g.Stack } +func (g *PasswordGenerator) SetStack(stack string) { g.Stack = stack } + +// ParseGenerator parses a single generator from JSON, dispatching on the +// discriminated Type field the same way ParsePolicy does. +func ParseGenerator(raw json.RawMessage) (Generator, error) { + var header struct { + Type string `json:"Type"` + } + if err := json.Unmarshal(raw, &header); err != nil { + return nil, fmt.Errorf("failed to parse generator type: %w", err) + } + + switch header.Type { + case "password": + var g PasswordGenerator + if err := json.Unmarshal(raw, &g); err != nil { + return nil, fmt.Errorf("failed to parse password generator: %w", err) + } + return &g, nil + default: + return nil, fmt.Errorf("unknown generator type: %s", header.Type) + } +} + +// ParseGenerators parses multiple generators from JSON. +func ParseGenerators(rawGenerators []json.RawMessage) ([]Generator, error) { + generators := make([]Generator, 0, len(rawGenerators)) + for i, raw := range rawGenerators { + generator, err := ParseGenerator(raw) + if err != nil { + return nil, fmt.Errorf("failed to parse generator at index %d: %w", i, err) + } + generators = append(generators, generator) + } + return generators, nil +} diff --git a/pkg/model/generator_test.go b/pkg/model/generator_test.go new file mode 100644 index 000000000..cb261126f --- /dev/null +++ b/pkg/model/generator_test.go @@ -0,0 +1,103 @@ +// © 2025 Platform Engineering Labs Inc. +// +// SPDX-License-Identifier: FSL-1.1-ALv2 + +package model + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestParseGenerator_Password(t *testing.T) { + raw := json.RawMessage(`{ + "Type": "password", + "Label": "db-password", + "Stack": "default", + "Length": 24, + "Uppercase": true, + "Lowercase": true, + "Digits": true, + "Symbols": false, + "ExcludeCharacters": "oO0", + "RequireEachIncludedType": true + }`) + + generator, err := ParseGenerator(raw) + require.NoError(t, err) + + password, ok := generator.(*PasswordGenerator) + require.True(t, ok, "expected *PasswordGenerator") + + assert.Equal(t, "password", password.GetType()) + assert.Equal(t, "db-password", password.GetLabel()) + assert.Equal(t, "default", password.GetStack()) + assert.Equal(t, 24, password.Length) + assert.True(t, password.Uppercase) + assert.True(t, password.Lowercase) + assert.True(t, password.Digits) + assert.False(t, password.Symbols) + assert.Equal(t, "oO0", password.ExcludeCharacters) + assert.True(t, password.RequireEachIncludedType) + assert.Nil(t, password.EverySeconds) +} + +// TestParseGenerator_Password_RoundTrip checks that marshaling a parsed +// PasswordGenerator back to JSON and re-parsing it produces an identical +// value, so the same shape written by the PKL schema and read by this parser +// survives a full round trip (as it does when a generator is stored, then +// reloaded to be re-applied). +func TestParseGenerator_Password_RoundTrip(t *testing.T) { + raw := json.RawMessage(`{ + "Type": "password", + "Label": "api-key-seed", + "Stack": "secrets-stack", + "EverySeconds": 2592000, + "Length": 40, + "Uppercase": true, + "Lowercase": true, + "Digits": true, + "Symbols": true, + "ExcludeCharacters": "", + "RequireEachIncludedType": false + }`) + + generator, err := ParseGenerator(raw) + require.NoError(t, err) + + marshaled, err := json.Marshal(generator) + require.NoError(t, err) + + roundTripped, err := ParseGenerator(marshaled) + require.NoError(t, err) + + assert.Equal(t, generator, roundTripped) + + password := roundTripped.(*PasswordGenerator) + require.NotNil(t, password.EverySeconds) + assert.Equal(t, int64(2592000), *password.EverySeconds) +} + +func TestParseGenerator_UnknownType(t *testing.T) { + raw := json.RawMessage(`{"Type": "unknown-generator", "Label": "x"}`) + + _, err := ParseGenerator(raw) + require.Error(t, err) + assert.ErrorContains(t, err, "unknown generator type") +} + +func TestParseGenerators_Multiple(t *testing.T) { + raw := []json.RawMessage{ + json.RawMessage(`{"Type": "password", "Label": "one", "Length": 16, "Uppercase": true, "Lowercase": true, "Digits": true, "Symbols": false, "RequireEachIncludedType": true}`), + json.RawMessage(`{"Type": "password", "Label": "two", "Length": 32, "Uppercase": true, "Lowercase": true, "Digits": true, "Symbols": false, "RequireEachIncludedType": true}`), + } + + generators, err := ParseGenerators(raw) + require.NoError(t, err) + require.Len(t, generators, 2) + assert.Equal(t, "one", generators[0].GetLabel()) + assert.Equal(t, "two", generators[1].GetLabel()) +} From 9a9af8122477522918b87658bcf169705f4c5a7f Mon Sep 17 00:00:00 2001 From: Jeroen Soeters Date: Fri, 28 Aug 2026 23:46:45 -0700 Subject: [PATCH 2/5] feat(datastore): persist the generator forma kind Add the generators table (sqlite, postgres, mssql migrations), the Datastore CreateGenerator/UpdateGenerator/DeleteGenerator/GetGenerator/ LoadGeneratorsByStack methods, and a SQLite implementation. A generator is always owned by exactly one stack: unlike a policy it has no standalone form, so the table carries stack_id NOT NULL and there is no junction table or attach/detach. Identity is the row's KSUID, stable across an update found by (label, stack), so a rename does not read as delete-then-create. The behavioural test suite lives in internal/datastore/dstest as suite_generators.go, following the existing RunAll pattern, so Postgres, Aurora and MSSQL can be held to the same six behaviours once implemented. Those three backends and the hand-rolled test mocks get stub/panic methods for now to keep the Datastore interface satisfied. --- .../datastore/aurora/aurora_generators.go | 37 ++++ internal/datastore/datastore.go | 22 ++ internal/datastore/dstest/dstest.go | 16 ++ internal/datastore/dstest/suite_generators.go | 195 ++++++++++++++++++ internal/datastore/generator_data.go | 38 ++++ .../00024_generators_table.sql | 40 ++++ .../00025_generators_table.sql | 33 +++ .../00024_generators_table.sql | 33 +++ internal/datastore/mock_datastore_test.go | 15 +- internal/datastore/mssql/mssql_generators.go | 36 ++++ .../datastore/postgres/postgres_generators.go | 36 ++++ internal/datastore/sqlite/sqlite.go | 172 +++++++++++++++ internal/datastore/sqlite/sqlite_test.go | 12 ++ .../metastructure/extract_resources_test.go | 15 ++ .../metastructure/resource_summaries_test.go | 15 ++ 15 files changed, 714 insertions(+), 1 deletion(-) create mode 100644 internal/datastore/aurora/aurora_generators.go create mode 100644 internal/datastore/dstest/suite_generators.go create mode 100644 internal/datastore/generator_data.go create mode 100644 internal/datastore/migrations_mssql/00024_generators_table.sql create mode 100644 internal/datastore/migrations_postgres/00025_generators_table.sql create mode 100644 internal/datastore/migrations_sqlite/00024_generators_table.sql create mode 100644 internal/datastore/mssql/mssql_generators.go create mode 100644 internal/datastore/postgres/postgres_generators.go diff --git a/internal/datastore/aurora/aurora_generators.go b/internal/datastore/aurora/aurora_generators.go new file mode 100644 index 000000000..0b48c95e9 --- /dev/null +++ b/internal/datastore/aurora/aurora_generators.go @@ -0,0 +1,37 @@ +// © 2026 Platform Engineering Labs Inc. +// +// SPDX-License-Identifier: FSL-1.1-ALv2 + +package aurora + +import ( + "fmt" + + pkgmodel "github.com/platform-engineering-labs/formae/pkg/model" +) + +// Generator persistence for Aurora is not yet implemented. The generators +// table exists (see migrations_postgres, which Aurora Data API also runs) so +// schema stays in lockstep across backends, but these methods exist only to +// satisfy datastore.Datastore until a following change implements them +// against the shared dstest suite. + +func (d *DatastoreAuroraDataAPI) CreateGenerator(_ pkgmodel.Generator, _ string) (string, error) { + return "", fmt.Errorf("generator persistence is not yet implemented for aurora") +} + +func (d *DatastoreAuroraDataAPI) UpdateGenerator(_ pkgmodel.Generator, _ string) (string, error) { + return "", fmt.Errorf("generator persistence is not yet implemented for aurora") +} + +func (d *DatastoreAuroraDataAPI) DeleteGenerator(_, _ string) (string, error) { + return "", fmt.Errorf("generator persistence is not yet implemented for aurora") +} + +func (d *DatastoreAuroraDataAPI) GetGenerator(_, _ string) (pkgmodel.Generator, error) { + return nil, fmt.Errorf("generator persistence is not yet implemented for aurora") +} + +func (d *DatastoreAuroraDataAPI) LoadGeneratorsByStack(_ string) ([]pkgmodel.Generator, error) { + return nil, fmt.Errorf("generator persistence is not yet implemented for aurora") +} diff --git a/internal/datastore/datastore.go b/internal/datastore/datastore.go index 5bf2c4efa..6d808e606 100644 --- a/internal/datastore/datastore.go +++ b/internal/datastore/datastore.go @@ -486,6 +486,28 @@ type Datastore interface { // that are not in a terminal state (Success, Failed, Canceled) StackHasActiveCommands(stackLabel string) (bool, error) + // Generator operations - a generator produces a value (e.g. a random + // password) that a secret will later reference. Unlike a policy, a + // generator has no standalone form: it is always owned by exactly one + // stack, so there is no stack_generators junction table and no + // attach/detach. + + // CreateGenerator persists a new generator (returns version string) + CreateGenerator(gen pkgmodel.Generator, commandID string) (string, error) + // UpdateGenerator persists a new version of an existing generator, found + // by label and stack (returns version string) + UpdateGenerator(gen pkgmodel.Generator, commandID string) (string, error) + // DeleteGenerator soft-deletes the generator with the given label on the + // given stack (returns version string). A label with no live match is a + // no-op success that returns an empty version. + DeleteGenerator(label, stackLabel string) (string, error) + // GetGenerator retrieves the current generator with the given label on + // the given stack. Returns nil, nil if no live generator is found. + GetGenerator(label, stackLabel string) (pkgmodel.Generator, error) + // LoadGeneratorsByStack returns all non-deleted generators owned by a + // stack. + LoadGeneratorsByStack(stackLabel string) ([]pkgmodel.Generator, error) + // Close releases database connections Close() diff --git a/internal/datastore/dstest/dstest.go b/internal/datastore/dstest/dstest.go index 1c5aaa5ff..3bc9c142c 100644 --- a/internal/datastore/dstest/dstest.go +++ b/internal/datastore/dstest/dstest.go @@ -97,6 +97,15 @@ type TestDatastore struct { // Backends that don't provide it leave it nil and the relevant tests // t.Skip(). NullFormaCommandSubjectForTest func(commandID string) error + // GeneratorIDForTest returns the internal KSUID identity (the id column, + // stable across CreateGenerator/UpdateGenerator) of the current + // (max-version) generator row with the given label on the given stack, or + // "" if none exists. Generator has no public API that exposes this id — + // the Datastore interface returns only version strings — so the suite + // needs a direct accessor to prove the id survives an update unchanged. + // Backends that don't provide it leave it nil and the relevant tests + // t.Skip(). + GeneratorIDForTest func(label, stackLabel string) (string, error) } // RunAll runs the full datastore test suite against the provided factory. @@ -231,6 +240,13 @@ func RunAll(t *testing.T, newDS func(t *testing.T) TestDatastore) { RunDeleteInlinePolicyClearsExpiry(t, newDS) RunDeleteInlinePolicyThenRecreate(t, newDS) + RunCreateGeneratorThenGet(t, newDS) + RunGetGeneratorAbsentReturnsNil(t, newDS) + RunUpdateGeneratorBumpsVersionAndReadBackReflectsIt(t, newDS) + RunDeleteGeneratorThenGetReturnsNil(t, newDS) + RunLoadGeneratorsByStackReturnsOnlyThatStacksGenerators(t, newDS) + RunGeneratorKSUIDStableAcrossUpdate(t, newDS) + RunFindResourcesDependingOn(t, newDS) RunFindResourcesDependingOnMultipleRefs(t, newDS) RunFindResourcesDependingOnNoRefs(t, newDS) diff --git a/internal/datastore/dstest/suite_generators.go b/internal/datastore/dstest/suite_generators.go new file mode 100644 index 000000000..7731f194a --- /dev/null +++ b/internal/datastore/dstest/suite_generators.go @@ -0,0 +1,195 @@ +// © 2026 Platform Engineering Labs Inc. +// +// SPDX-License-Identifier: FSL-1.1-ALv2 + +//go:build unit + +package dstest + +import ( + "testing" + + "github.com/platform-engineering-labs/formae/internal/datastore" + pkgmodel "github.com/platform-engineering-labs/formae/pkg/model" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// createGeneratorStack creates a stack for the generator suite tests and +// returns its label. A generator is always inline to a stack — unlike a +// policy it has no standalone form — so every test needs one. +func createGeneratorStack(t *testing.T, ds datastore.Datastore, label string) string { + t.Helper() + stack := &pkgmodel.Stack{Label: label, Description: "generator lookup"} + _, err := ds.CreateStack(stack, "cmd-stack") + require.NoError(t, err) + return stack.Label +} + +// testPasswordGenerator returns a password generator on the given stack, with +// Length as the one field the suite varies to observe an update. +func testPasswordGenerator(label, stack string, length int) *pkgmodel.PasswordGenerator { + return &pkgmodel.PasswordGenerator{ + Type: "password", + Label: label, + Stack: stack, + Length: length, + Uppercase: true, + Lowercase: true, + Digits: true, + RequireEachIncludedType: true, + } +} + +// RunCreateGeneratorThenGet verifies that a created generator is retrievable +// by label and stack, with its data intact. +func RunCreateGeneratorThenGet(t *testing.T, newDS func(t *testing.T) TestDatastore) { + t.Run("CreateGenerator_ThenGet", func(t *testing.T) { + td := newDS(t) + ds := td.Datastore + defer td.CleanUpFn() //nolint:errcheck + + stack := createGeneratorStack(t, ds, "generator-owner") + gen := testPasswordGenerator("db-password", stack, 24) + + version, err := ds.CreateGenerator(gen, "cmd-create") + require.NoError(t, err) + require.NotEmpty(t, version) + + got, err := ds.GetGenerator("db-password", stack) + require.NoError(t, err) + require.NotNil(t, got) + assert.Equal(t, "db-password", got.GetLabel()) + assert.Equal(t, "password", got.GetType()) + assert.Equal(t, stack, got.GetStack()) + pw, ok := got.(*pkgmodel.PasswordGenerator) + require.True(t, ok, "GetGenerator must return the concrete password generator type") + assert.Equal(t, 24, pw.Length) + }) +} + +// RunGetGeneratorAbsentReturnsNil verifies that looking up a generator that +// was never created returns nil, nil rather than an error. +func RunGetGeneratorAbsentReturnsNil(t *testing.T, newDS func(t *testing.T) TestDatastore) { + t.Run("GetGenerator_AbsentReturnsNil", func(t *testing.T) { + td := newDS(t) + ds := td.Datastore + defer td.CleanUpFn() //nolint:errcheck + + stack := createGeneratorStack(t, ds, "generator-empty") + + got, err := ds.GetGenerator("never-created", stack) + require.NoError(t, err) + assert.Nil(t, got) + }) +} + +// RunUpdateGeneratorBumpsVersionAndReadBackReflectsIt verifies that updating a +// generator returns a new version distinct from creation, and that a +// subsequent GetGenerator reflects the updated data. +func RunUpdateGeneratorBumpsVersionAndReadBackReflectsIt(t *testing.T, newDS func(t *testing.T) TestDatastore) { + t.Run("UpdateGenerator_BumpsVersionAndReadBackReflectsIt", func(t *testing.T) { + td := newDS(t) + ds := td.Datastore + defer td.CleanUpFn() //nolint:errcheck + + stack := createGeneratorStack(t, ds, "generator-updated") + createVersion, err := ds.CreateGenerator(testPasswordGenerator("api-key", stack, 16), "cmd-create") + require.NoError(t, err) + + updateVersion, err := ds.UpdateGenerator(testPasswordGenerator("api-key", stack, 32), "cmd-update") + require.NoError(t, err) + + assert.NotEqual(t, createVersion, updateVersion, "an update must mint a new version") + + got, err := ds.GetGenerator("api-key", stack) + require.NoError(t, err) + require.NotNil(t, got) + pw, ok := got.(*pkgmodel.PasswordGenerator) + require.True(t, ok) + assert.Equal(t, 32, pw.Length, "the read-back generator must reflect the update") + }) +} + +// RunDeleteGeneratorThenGetReturnsNil verifies that a deleted generator is no +// longer returned by GetGenerator. +func RunDeleteGeneratorThenGetReturnsNil(t *testing.T, newDS func(t *testing.T) TestDatastore) { + t.Run("DeleteGenerator_ThenGetReturnsNil", func(t *testing.T) { + td := newDS(t) + ds := td.Datastore + defer td.CleanUpFn() //nolint:errcheck + + stack := createGeneratorStack(t, ds, "generator-deleted") + _, err := ds.CreateGenerator(testPasswordGenerator("temp-secret", stack, 20), "cmd-create") + require.NoError(t, err) + + version, err := ds.DeleteGenerator("temp-secret", stack) + require.NoError(t, err) + require.NotEmpty(t, version) + + got, err := ds.GetGenerator("temp-secret", stack) + require.NoError(t, err) + assert.Nil(t, got) + }) +} + +// RunLoadGeneratorsByStackReturnsOnlyThatStacksGenerators verifies that +// loading by stack does not leak another stack's generators. +func RunLoadGeneratorsByStackReturnsOnlyThatStacksGenerators(t *testing.T, newDS func(t *testing.T) TestDatastore) { + t.Run("LoadGeneratorsByStack_ReturnsOnlyThatStacksGenerators", func(t *testing.T) { + td := newDS(t) + ds := td.Datastore + defer td.CleanUpFn() //nolint:errcheck + + owner := createGeneratorStack(t, ds, "generator-load-owner") + other := createGeneratorStack(t, ds, "generator-load-other") + + _, err := ds.CreateGenerator(testPasswordGenerator("owner-a", owner, 12), "cmd-create") + require.NoError(t, err) + _, err = ds.CreateGenerator(testPasswordGenerator("owner-b", owner, 12), "cmd-create") + require.NoError(t, err) + _, err = ds.CreateGenerator(testPasswordGenerator("other-a", other, 12), "cmd-create") + require.NoError(t, err) + + generators, err := ds.LoadGeneratorsByStack(owner) + require.NoError(t, err) + + labels := make([]string, 0, len(generators)) + for _, g := range generators { + labels = append(labels, g.GetLabel()) + assert.Equal(t, owner, g.GetStack()) + } + assert.ElementsMatch(t, []string{"owner-a", "owner-b"}, labels) + }) +} + +// RunGeneratorKSUIDStableAcrossUpdate verifies that a generator's internal +// KSUID identity does not change when it is updated — only the label and +// stack are looked up, and the same id is carried forward onto the new +// version row. This is load-bearing: a later slice derives generator cadence +// per id, so a rename must not read as a delete plus a fresh generator. +func RunGeneratorKSUIDStableAcrossUpdate(t *testing.T, newDS func(t *testing.T) TestDatastore) { + t.Run("Generator_KSUIDStableAcrossUpdate", func(t *testing.T) { + td := newDS(t) + if td.GeneratorIDForTest == nil { + t.Skip("backend does not provide GeneratorIDForTest") + } + ds := td.Datastore + defer td.CleanUpFn() //nolint:errcheck + + stack := createGeneratorStack(t, ds, "generator-ksuid-stable") + _, err := ds.CreateGenerator(testPasswordGenerator("stable-id", stack, 16), "cmd-create") + require.NoError(t, err) + + idBeforeUpdate, err := td.GeneratorIDForTest("stable-id", stack) + require.NoError(t, err) + require.NotEmpty(t, idBeforeUpdate) + + _, err = ds.UpdateGenerator(testPasswordGenerator("stable-id", stack, 40), "cmd-update") + require.NoError(t, err) + + idAfterUpdate, err := td.GeneratorIDForTest("stable-id", stack) + require.NoError(t, err) + assert.Equal(t, idBeforeUpdate, idAfterUpdate, "the generator's KSUID identity must survive an update") + }) +} diff --git a/internal/datastore/generator_data.go b/internal/datastore/generator_data.go new file mode 100644 index 000000000..b62f94d9b --- /dev/null +++ b/internal/datastore/generator_data.go @@ -0,0 +1,38 @@ +// © 2026 Platform Engineering Labs Inc. +// +// SPDX-License-Identifier: FSL-1.1-ALv2 + +package datastore + +import ( + "encoding/json" + "fmt" + + pkgmodel "github.com/platform-engineering-labs/formae/pkg/model" +) + +// GeneratorData builds the generator_data payload for a generator. +// +// Unlike TTLPolicyData, no field needs to be stripped or reassembled: a +// generator carries no one-of and pkgmodel.ParseGenerator already dispatches +// on the same discriminated Type field the generator marshals itself with, so +// the full generator value round-trips through it directly. Centralized here +// so the four backends agree on the same byte-level format rather than each +// deciding independently. +func GeneratorData(gen pkgmodel.Generator) ([]byte, error) { + data, err := json.Marshal(gen) + if err != nil { + return nil, fmt.Errorf("failed to marshal generator data: %w", err) + } + return data, nil +} + +// GeneratorFromData rebuilds a generator from a stored generator_data +// payload. +func GeneratorFromData(data []byte) (pkgmodel.Generator, error) { + gen, err := pkgmodel.ParseGenerator(data) + if err != nil { + return nil, fmt.Errorf("failed to unmarshal generator data: %w", err) + } + return gen, nil +} diff --git a/internal/datastore/migrations_mssql/00024_generators_table.sql b/internal/datastore/migrations_mssql/00024_generators_table.sql new file mode 100644 index 000000000..f1fe97e2d --- /dev/null +++ b/internal/datastore/migrations_mssql/00024_generators_table.sql @@ -0,0 +1,40 @@ +-- © 2026 Platform Engineering Labs Inc. +-- +-- SPDX-License-Identifier: FSL-1.1-ALv2 + +-- +goose Up +-- Generators produce values (e.g. random passwords) that secrets will later +-- reference. Unlike policies, a generator has no standalone form: it is +-- always owned by exactly one stack, so stack_id is NOT NULL and there is no +-- stack_generators junction table and no attach/detach. +-- +-- Identity is the KSUID in id, not the label: a label is unique only within +-- its stack, and generator cadence will later be derived per generator id, so +-- a rename must not read as a delete plus a fresh generator. +-- +goose StatementBegin +IF NOT EXISTS (SELECT 1 FROM sys.tables WHERE name = 'generators') +BEGIN + CREATE TABLE generators ( + id nvarchar(450) COLLATE Latin1_General_BIN2 NOT NULL, + version nvarchar(450) COLLATE Latin1_General_BIN2 NOT NULL, + valid_from datetime2 DEFAULT SYSUTCDATETIME(), + command_id nvarchar(450) COLLATE Latin1_General_BIN2, + operation nvarchar(450) NOT NULL, + label nvarchar(450) NOT NULL, + generator_type nvarchar(450) NOT NULL, + stack_id nvarchar(450) COLLATE Latin1_General_BIN2 NOT NULL, + generator_data nvarchar(max) NOT NULL DEFAULT '{}', + PRIMARY KEY (id, version) + ); + CREATE INDEX idx_generators_stack_id ON generators (stack_id); + CREATE INDEX idx_generators_generator_type ON generators (generator_type); +END; +-- +goose StatementEnd + +-- +goose Down +-- +goose StatementBegin +IF EXISTS (SELECT 1 FROM sys.tables WHERE name = 'generators') +BEGIN + DROP TABLE generators; +END; +-- +goose StatementEnd diff --git a/internal/datastore/migrations_postgres/00025_generators_table.sql b/internal/datastore/migrations_postgres/00025_generators_table.sql new file mode 100644 index 000000000..3803869a7 --- /dev/null +++ b/internal/datastore/migrations_postgres/00025_generators_table.sql @@ -0,0 +1,33 @@ +-- © 2026 Platform Engineering Labs Inc. +-- +-- SPDX-License-Identifier: FSL-1.1-ALv2 + +-- +goose Up +-- Generators produce values (e.g. random passwords) that secrets will later +-- reference. Unlike policies, a generator has no standalone form: it is +-- always owned by exactly one stack, so stack_id is NOT NULL and there is no +-- stack_generators junction table and no attach/detach. +-- +-- Identity is the KSUID in id, not the label: a label is unique only within +-- its stack, and generator cadence will later be derived per generator id, so +-- a rename must not read as a delete plus a fresh generator. +CREATE TABLE IF NOT EXISTS generators ( + id TEXT NOT NULL, + version TEXT NOT NULL, + valid_from TIMESTAMP DEFAULT CURRENT_TIMESTAMP, + command_id TEXT, + operation TEXT NOT NULL, + label TEXT NOT NULL, + generator_type TEXT NOT NULL, + stack_id TEXT NOT NULL, + generator_data JSONB NOT NULL DEFAULT '{}', + PRIMARY KEY (id, version) +); + +CREATE INDEX IF NOT EXISTS idx_generators_stack_id ON generators(stack_id); +CREATE INDEX IF NOT EXISTS idx_generators_generator_type ON generators(generator_type); + +-- +goose Down +DROP INDEX IF EXISTS idx_generators_generator_type; +DROP INDEX IF EXISTS idx_generators_stack_id; +DROP TABLE IF EXISTS generators; diff --git a/internal/datastore/migrations_sqlite/00024_generators_table.sql b/internal/datastore/migrations_sqlite/00024_generators_table.sql new file mode 100644 index 000000000..6caa0f26c --- /dev/null +++ b/internal/datastore/migrations_sqlite/00024_generators_table.sql @@ -0,0 +1,33 @@ +-- © 2026 Platform Engineering Labs Inc. +-- +-- SPDX-License-Identifier: FSL-1.1-ALv2 + +-- +goose Up +-- Generators produce values (e.g. random passwords) that secrets will later +-- reference. Unlike policies, a generator has no standalone form: it is +-- always owned by exactly one stack, so stack_id is NOT NULL and there is no +-- stack_generators junction table and no attach/detach. +-- +-- Identity is the KSUID in id, not the label: a label is unique only within +-- its stack, and generator cadence will later be derived per generator id, so +-- a rename must not read as a delete plus a fresh generator. +CREATE TABLE IF NOT EXISTS generators ( + id TEXT NOT NULL, + version TEXT NOT NULL, + valid_from TIMESTAMP DEFAULT CURRENT_TIMESTAMP, + command_id TEXT, + operation TEXT NOT NULL, + label TEXT NOT NULL, + generator_type TEXT NOT NULL, + stack_id TEXT NOT NULL, + generator_data TEXT NOT NULL DEFAULT '{}', + PRIMARY KEY (id, version) +); + +CREATE INDEX IF NOT EXISTS idx_generators_stack_id ON generators(stack_id); +CREATE INDEX IF NOT EXISTS idx_generators_generator_type ON generators(generator_type); + +-- +goose Down +DROP INDEX IF EXISTS idx_generators_generator_type; +DROP INDEX IF EXISTS idx_generators_stack_id; +DROP TABLE IF EXISTS generators; diff --git a/internal/datastore/mock_datastore_test.go b/internal/datastore/mock_datastore_test.go index f76c309ac..c2d604242 100644 --- a/internal/datastore/mock_datastore_test.go +++ b/internal/datastore/mock_datastore_test.go @@ -177,7 +177,20 @@ func (m *mockDatastore) GetResourcesAtLastReconcile(_ string) ([]ResourceSnapsho return nil, nil } func (m *mockDatastore) StackHasActiveCommands(_ string) (bool, error) { return false, nil } -func (m *mockDatastore) Close() {} +func (m *mockDatastore) CreateGenerator(_ pkgmodel.Generator, _ string) (string, error) { + return "", nil +} +func (m *mockDatastore) UpdateGenerator(_ pkgmodel.Generator, _ string) (string, error) { + return "", nil +} +func (m *mockDatastore) DeleteGenerator(_, _ string) (string, error) { return "", nil } +func (m *mockDatastore) GetGenerator(_, _ string) (pkgmodel.Generator, error) { + return nil, nil +} +func (m *mockDatastore) LoadGeneratorsByStack(_ string) ([]pkgmodel.Generator, error) { + return nil, nil +} +func (m *mockDatastore) Close() {} func (m *mockDatastore) BulkStoreResourceUpdates(_ string, _ []resource_update.ResourceUpdate) error { return nil } diff --git a/internal/datastore/mssql/mssql_generators.go b/internal/datastore/mssql/mssql_generators.go new file mode 100644 index 000000000..047391632 --- /dev/null +++ b/internal/datastore/mssql/mssql_generators.go @@ -0,0 +1,36 @@ +// © 2026 Platform Engineering Labs Inc. +// +// SPDX-License-Identifier: FSL-1.1-ALv2 + +package mssql + +import ( + "fmt" + + pkgmodel "github.com/platform-engineering-labs/formae/pkg/model" +) + +// Generator persistence for MSSQL is not yet implemented. The generators +// table exists (see migrations_mssql) so schema stays in lockstep across +// backends, but these methods exist only to satisfy datastore.Datastore until +// a following change implements them against the shared dstest suite. + +func (d *DatastoreMSSQL) CreateGenerator(_ pkgmodel.Generator, _ string) (string, error) { + return "", fmt.Errorf("generator persistence is not yet implemented for mssql") +} + +func (d *DatastoreMSSQL) UpdateGenerator(_ pkgmodel.Generator, _ string) (string, error) { + return "", fmt.Errorf("generator persistence is not yet implemented for mssql") +} + +func (d *DatastoreMSSQL) DeleteGenerator(_, _ string) (string, error) { + return "", fmt.Errorf("generator persistence is not yet implemented for mssql") +} + +func (d *DatastoreMSSQL) GetGenerator(_, _ string) (pkgmodel.Generator, error) { + return nil, fmt.Errorf("generator persistence is not yet implemented for mssql") +} + +func (d *DatastoreMSSQL) LoadGeneratorsByStack(_ string) ([]pkgmodel.Generator, error) { + return nil, fmt.Errorf("generator persistence is not yet implemented for mssql") +} diff --git a/internal/datastore/postgres/postgres_generators.go b/internal/datastore/postgres/postgres_generators.go new file mode 100644 index 000000000..68c012270 --- /dev/null +++ b/internal/datastore/postgres/postgres_generators.go @@ -0,0 +1,36 @@ +// © 2026 Platform Engineering Labs Inc. +// +// SPDX-License-Identifier: FSL-1.1-ALv2 + +package postgres + +import ( + "fmt" + + pkgmodel "github.com/platform-engineering-labs/formae/pkg/model" +) + +// Generator persistence for Postgres is not yet implemented. The generators +// table exists (see migrations_postgres) so schema stays in lockstep across +// backends, but these methods exist only to satisfy datastore.Datastore until +// a following change implements them against the shared dstest suite. + +func (d DatastorePostgres) CreateGenerator(_ pkgmodel.Generator, _ string) (string, error) { + return "", fmt.Errorf("generator persistence is not yet implemented for postgres") +} + +func (d DatastorePostgres) UpdateGenerator(_ pkgmodel.Generator, _ string) (string, error) { + return "", fmt.Errorf("generator persistence is not yet implemented for postgres") +} + +func (d DatastorePostgres) DeleteGenerator(_, _ string) (string, error) { + return "", fmt.Errorf("generator persistence is not yet implemented for postgres") +} + +func (d DatastorePostgres) GetGenerator(_, _ string) (pkgmodel.Generator, error) { + return nil, fmt.Errorf("generator persistence is not yet implemented for postgres") +} + +func (d DatastorePostgres) LoadGeneratorsByStack(_ string) ([]pkgmodel.Generator, error) { + return nil, fmt.Errorf("generator persistence is not yet implemented for postgres") +} diff --git a/internal/datastore/sqlite/sqlite.go b/internal/datastore/sqlite/sqlite.go index cc2f67789..f86518903 100644 --- a/internal/datastore/sqlite/sqlite.go +++ b/internal/datastore/sqlite/sqlite.go @@ -2792,6 +2792,178 @@ func (d DatastoreSQLite) DeletePoliciesForStack(stackID string, commandID string return nil } +// CreateGenerator persists a new generator. Unlike CreatePolicy, the stack is +// never NULL: a generator is always inline to exactly one stack, read off +// gen.GetStack() rather than threaded as a separate argument. +func (d DatastoreSQLite) CreateGenerator(gen pkgmodel.Generator, commandID string) (string, error) { + _, span := sqliteTracer.Start(context.Background(), "CreateGenerator") + defer span.End() + + id := mksuid.New().String() + version := mksuid.New().String() + + data, err := datastore.GeneratorData(gen) + if err != nil { + return "", err + } + + query := `INSERT INTO generators (id, version, command_id, operation, label, generator_type, stack_id, generator_data) + VALUES (?, ?, ?, ?, ?, ?, ?, ?)` + _, err = d.conn.Exec(query, id, version, commandID, "create", gen.GetLabel(), gen.GetType(), gen.GetStack(), string(data)) + if err != nil { + slog.Error("Failed to create generator", "error", err, "label", gen.GetLabel()) + return "", err + } + + return version, nil +} + +// UpdateGenerator persists a new version of an existing generator. The +// existing row is found by label and stack — a generator has no standalone +// form, so unlike UpdatePolicy there is no NULL-stack branch — and the new +// version row carries forward the same id. +func (d DatastoreSQLite) UpdateGenerator(gen pkgmodel.Generator, commandID string) (string, error) { + _, span := sqliteTracer.Start(context.Background(), "UpdateGenerator") + defer span.End() + + var id string + err := d.conn.QueryRow( + `SELECT id FROM generators WHERE label = ? AND stack_id = ? ORDER BY version DESC LIMIT 1`, + gen.GetLabel(), gen.GetStack(), + ).Scan(&id) + if err != nil { + return "", fmt.Errorf("failed to find existing generator: %w", err) + } + + version := mksuid.New().String() + data, err := datastore.GeneratorData(gen) + if err != nil { + return "", err + } + + insertQuery := `INSERT INTO generators (id, version, command_id, operation, label, generator_type, stack_id, generator_data) + VALUES (?, ?, ?, ?, ?, ?, ?, ?)` + _, err = d.conn.Exec(insertQuery, id, version, commandID, "update", gen.GetLabel(), gen.GetType(), gen.GetStack(), string(data)) + if err != nil { + slog.Error("Failed to update generator", "error", err, "label", gen.GetLabel()) + return "", err + } + + return version, nil +} + +// DeleteGenerator soft-deletes the generator with the given label on the +// given stack. A label with no live match is a no-op success that returns an +// empty version, mirroring DeletePolicy. +func (d DatastoreSQLite) DeleteGenerator(label, stackLabel string) (string, error) { + _, span := sqliteTracer.Start(context.Background(), "DeleteGenerator") + defer span.End() + + query := ` + WITH latest_generators AS ( + SELECT id, generator_type, operation, + ROW_NUMBER() OVER (PARTITION BY id ORDER BY version DESC) as rn + FROM generators + WHERE stack_id = ? AND label = ? + ) + SELECT id, generator_type + FROM latest_generators + WHERE rn = 1 AND operation != 'delete' + ` + var id, generatorType string + err := d.conn.QueryRow(query, stackLabel, label).Scan(&id, &generatorType) + if err != nil { + if errors.Is(err, sql.ErrNoRows) { + return "", nil + } + return "", fmt.Errorf("failed to get generator for deletion: %w", err) + } + + version := mksuid.New().String() + insertQuery := `INSERT INTO generators (id, version, command_id, operation, label, generator_type, stack_id, generator_data) VALUES (?, ?, ?, ?, ?, ?, ?, ?)` + _, err = d.conn.Exec(insertQuery, id, version, "", "delete", label, generatorType, stackLabel, "{}") + if err != nil { + return "", fmt.Errorf("failed to delete generator: %w", err) + } + + slog.Debug("Deleted generator", "label", label, "id", id, "stackLabel", stackLabel) + + return version, nil +} + +// GetGenerator retrieves the current (latest, non-deleted) generator with the +// given label on the given stack. Returns nil, nil if no live generator +// matches. +func (d DatastoreSQLite) GetGenerator(label, stackLabel string) (pkgmodel.Generator, error) { + _, span := sqliteTracer.Start(context.Background(), "GetGenerator") + defer span.End() + + query := ` + WITH latest_generators AS ( + SELECT generator_data, operation, + ROW_NUMBER() OVER (PARTITION BY id ORDER BY version DESC) as rn + FROM generators + WHERE stack_id = ? AND label = ? + ) + SELECT generator_data + FROM latest_generators + WHERE rn = 1 AND operation != 'delete' + ` + var dataStr string + err := d.conn.QueryRow(query, stackLabel, label).Scan(&dataStr) + if err != nil { + if errors.Is(err, sql.ErrNoRows) { + return nil, nil + } + return nil, fmt.Errorf("failed to get generator: %w", err) + } + + return datastore.GeneratorFromData([]byte(dataStr)) +} + +// LoadGeneratorsByStack returns all non-deleted generators owned by a stack. +func (d DatastoreSQLite) LoadGeneratorsByStack(stackLabel string) ([]pkgmodel.Generator, error) { + _, span := sqliteTracer.Start(context.Background(), "LoadGeneratorsByStack") + defer span.End() + + query := ` + WITH latest_generators AS ( + SELECT id, generator_data, operation, + ROW_NUMBER() OVER (PARTITION BY id ORDER BY version DESC) as rn + FROM generators + WHERE stack_id = ? + ) + SELECT generator_data + FROM latest_generators + WHERE rn = 1 AND operation != 'delete' + ` + rows, err := d.conn.Query(query, stackLabel) + if err != nil { + return nil, err + } + defer func() { _ = rows.Close() }() + + var generators []pkgmodel.Generator + for rows.Next() { + var dataStr string + if err := rows.Scan(&dataStr); err != nil { + return nil, err + } + gen, err := datastore.GeneratorFromData([]byte(dataStr)) + if err != nil { + slog.Warn("Failed to deserialize generator, skipping", "error", err, "stackLabel", stackLabel) + continue + } + generators = append(generators, gen) + } + + if err := rows.Err(); err != nil { + return nil, err + } + + return generators, nil +} + // deserializePolicy creates a Policy from stored data func deserializePolicy(label, policyType, policyDataStr, stackID string) (pkgmodel.Policy, error) { switch policyType { diff --git a/internal/datastore/sqlite/sqlite_test.go b/internal/datastore/sqlite/sqlite_test.go index 36543cabd..f599a0940 100644 --- a/internal/datastore/sqlite/sqlite_test.go +++ b/internal/datastore/sqlite/sqlite_test.go @@ -142,6 +142,18 @@ func TestDatastore(t *testing.T) { ) return err }, + GeneratorIDForTest: func(label, stackLabel string) (string, error) { + conn := d.Conn() + var id string + err := conn.QueryRow( + `SELECT id FROM generators WHERE label = ? AND stack_id = ? ORDER BY version DESC LIMIT 1`, + label, stackLabel, + ).Scan(&id) + if err == sql.ErrNoRows { + return "", nil + } + return id, err + }, } }) } diff --git a/internal/metastructure/extract_resources_test.go b/internal/metastructure/extract_resources_test.go index 1f472bded..93f42c434 100644 --- a/internal/metastructure/extract_resources_test.go +++ b/internal/metastructure/extract_resources_test.go @@ -275,6 +275,21 @@ func (m *mockExtractDatastore) GetResourcesAtLastReconcile(_ string) ([]datastor func (m *mockExtractDatastore) StackHasActiveCommands(_ string) (bool, error) { panic("not implemented") } +func (m *mockExtractDatastore) CreateGenerator(_ pkgmodel.Generator, _ string) (string, error) { + panic("not implemented") +} +func (m *mockExtractDatastore) UpdateGenerator(_ pkgmodel.Generator, _ string) (string, error) { + panic("not implemented") +} +func (m *mockExtractDatastore) DeleteGenerator(_, _ string) (string, error) { + panic("not implemented") +} +func (m *mockExtractDatastore) GetGenerator(_, _ string) (pkgmodel.Generator, error) { + panic("not implemented") +} +func (m *mockExtractDatastore) LoadGeneratorsByStack(_ string) ([]pkgmodel.Generator, error) { + panic("not implemented") +} func (m *mockExtractDatastore) Close() {} func (m *mockExtractDatastore) BulkStoreResourceUpdates(_ string, _ []resource_update.ResourceUpdate) error { panic("not implemented") diff --git a/internal/metastructure/resource_summaries_test.go b/internal/metastructure/resource_summaries_test.go index 1c0584f62..6561f433b 100644 --- a/internal/metastructure/resource_summaries_test.go +++ b/internal/metastructure/resource_summaries_test.go @@ -262,6 +262,21 @@ func (m *mockSummaryDatastore) GetResourcesAtLastReconcile(_ string) ([]datastor func (m *mockSummaryDatastore) StackHasActiveCommands(_ string) (bool, error) { panic("not implemented") } +func (m *mockSummaryDatastore) CreateGenerator(_ pkgmodel.Generator, _ string) (string, error) { + panic("not implemented") +} +func (m *mockSummaryDatastore) UpdateGenerator(_ pkgmodel.Generator, _ string) (string, error) { + panic("not implemented") +} +func (m *mockSummaryDatastore) DeleteGenerator(_, _ string) (string, error) { + panic("not implemented") +} +func (m *mockSummaryDatastore) GetGenerator(_, _ string) (pkgmodel.Generator, error) { + panic("not implemented") +} +func (m *mockSummaryDatastore) LoadGeneratorsByStack(_ string) ([]pkgmodel.Generator, error) { + panic("not implemented") +} func (m *mockSummaryDatastore) Close() {} func (m *mockSummaryDatastore) BulkStoreResourceUpdates(_ string, _ []resource_update.ResourceUpdate) error { panic("not implemented") From 758e90633d9cbb99a5b39b9b38008b8545b5f0cc Mon Sep 17 00:00:00 2001 From: Jeroen Soeters Date: Fri, 28 Aug 2026 23:58:59 -0700 Subject: [PATCH 3/5] fix(datastore): store the stack KSUID in generators.stack_id, not the label policies.stack_id holds a resolved stack KSUID; generators.stack_id was storing the stack's label instead, a schema-level mismatch between two sibling tables sharing a column name with different semantics. Add StackID (with GetStackID/SetStackID) to the Generator model alongside the existing label-carrying GetStack/SetStack, mirroring how Policy carries both a label field and a resolved StackID. CreateGenerator and UpdateGenerator now persist gen.GetStackID() into stack_id. GetGenerator/DeleteGenerator/LoadGeneratorsByStack keep their public, label-scoped signatures but resolve the label to its current stack row internally before querying, the same way the datastore already resolves a stack by label elsewhere. The dstest generator suite sets StackID directly from the stack it creates, the same way the policy suite sets StackID on a TTLPolicy -- resolving a label to an ID during a real apply is deferred to the generator_update lifecycle, not part of this change. --- internal/datastore/dstest/suite_generators.go | 41 +++++++----- internal/datastore/sqlite/sqlite.go | 65 ++++++++++++++----- internal/datastore/sqlite/sqlite_test.go | 10 ++- pkg/model/generator.go | 5 ++ 4 files changed, 85 insertions(+), 36 deletions(-) diff --git a/internal/datastore/dstest/suite_generators.go b/internal/datastore/dstest/suite_generators.go index 7731f194a..634186d5b 100644 --- a/internal/datastore/dstest/suite_generators.go +++ b/internal/datastore/dstest/suite_generators.go @@ -16,23 +16,30 @@ import ( ) // createGeneratorStack creates a stack for the generator suite tests and -// returns its label. A generator is always inline to a stack — unlike a -// policy it has no standalone form — so every test needs one. -func createGeneratorStack(t *testing.T, ds datastore.Datastore, label string) string { +// returns it with its generated ID populated. A generator is always inline +// to a stack — unlike a policy it has no standalone form — so every test +// needs one, and generators.stack_id stores the stack's KSUID, so tests need +// the ID as well as the label. +func createGeneratorStack(t *testing.T, ds datastore.Datastore, label string) *pkgmodel.Stack { t.Helper() stack := &pkgmodel.Stack{Label: label, Description: "generator lookup"} _, err := ds.CreateStack(stack, "cmd-stack") require.NoError(t, err) - return stack.Label + require.NotEmpty(t, stack.ID) + return stack } // testPasswordGenerator returns a password generator on the given stack, with -// Length as the one field the suite varies to observe an update. -func testPasswordGenerator(label, stack string, length int) *pkgmodel.PasswordGenerator { +// Length as the one field the suite varies to observe an update. StackID is +// set directly from the resolved stack, the way the policy suite sets +// StackID on a TTLPolicy — the label-to-ID resolution a real apply performs +// is out of scope here. +func testPasswordGenerator(label string, stack *pkgmodel.Stack, length int) *pkgmodel.PasswordGenerator { return &pkgmodel.PasswordGenerator{ Type: "password", Label: label, - Stack: stack, + Stack: stack.Label, + StackID: stack.ID, Length: length, Uppercase: true, Lowercase: true, @@ -56,12 +63,12 @@ func RunCreateGeneratorThenGet(t *testing.T, newDS func(t *testing.T) TestDatast require.NoError(t, err) require.NotEmpty(t, version) - got, err := ds.GetGenerator("db-password", stack) + got, err := ds.GetGenerator("db-password", stack.Label) require.NoError(t, err) require.NotNil(t, got) assert.Equal(t, "db-password", got.GetLabel()) assert.Equal(t, "password", got.GetType()) - assert.Equal(t, stack, got.GetStack()) + assert.Equal(t, stack.Label, got.GetStack()) pw, ok := got.(*pkgmodel.PasswordGenerator) require.True(t, ok, "GetGenerator must return the concrete password generator type") assert.Equal(t, 24, pw.Length) @@ -78,7 +85,7 @@ func RunGetGeneratorAbsentReturnsNil(t *testing.T, newDS func(t *testing.T) Test stack := createGeneratorStack(t, ds, "generator-empty") - got, err := ds.GetGenerator("never-created", stack) + got, err := ds.GetGenerator("never-created", stack.Label) require.NoError(t, err) assert.Nil(t, got) }) @@ -102,7 +109,7 @@ func RunUpdateGeneratorBumpsVersionAndReadBackReflectsIt(t *testing.T, newDS fun assert.NotEqual(t, createVersion, updateVersion, "an update must mint a new version") - got, err := ds.GetGenerator("api-key", stack) + got, err := ds.GetGenerator("api-key", stack.Label) require.NoError(t, err) require.NotNil(t, got) pw, ok := got.(*pkgmodel.PasswordGenerator) @@ -123,11 +130,11 @@ func RunDeleteGeneratorThenGetReturnsNil(t *testing.T, newDS func(t *testing.T) _, err := ds.CreateGenerator(testPasswordGenerator("temp-secret", stack, 20), "cmd-create") require.NoError(t, err) - version, err := ds.DeleteGenerator("temp-secret", stack) + version, err := ds.DeleteGenerator("temp-secret", stack.Label) require.NoError(t, err) require.NotEmpty(t, version) - got, err := ds.GetGenerator("temp-secret", stack) + got, err := ds.GetGenerator("temp-secret", stack.Label) require.NoError(t, err) assert.Nil(t, got) }) @@ -151,13 +158,13 @@ func RunLoadGeneratorsByStackReturnsOnlyThatStacksGenerators(t *testing.T, newDS _, err = ds.CreateGenerator(testPasswordGenerator("other-a", other, 12), "cmd-create") require.NoError(t, err) - generators, err := ds.LoadGeneratorsByStack(owner) + generators, err := ds.LoadGeneratorsByStack(owner.Label) require.NoError(t, err) labels := make([]string, 0, len(generators)) for _, g := range generators { labels = append(labels, g.GetLabel()) - assert.Equal(t, owner, g.GetStack()) + assert.Equal(t, owner.Label, g.GetStack()) } assert.ElementsMatch(t, []string{"owner-a", "owner-b"}, labels) }) @@ -181,14 +188,14 @@ func RunGeneratorKSUIDStableAcrossUpdate(t *testing.T, newDS func(t *testing.T) _, err := ds.CreateGenerator(testPasswordGenerator("stable-id", stack, 16), "cmd-create") require.NoError(t, err) - idBeforeUpdate, err := td.GeneratorIDForTest("stable-id", stack) + idBeforeUpdate, err := td.GeneratorIDForTest("stable-id", stack.Label) require.NoError(t, err) require.NotEmpty(t, idBeforeUpdate) _, err = ds.UpdateGenerator(testPasswordGenerator("stable-id", stack, 40), "cmd-update") require.NoError(t, err) - idAfterUpdate, err := td.GeneratorIDForTest("stable-id", stack) + idAfterUpdate, err := td.GeneratorIDForTest("stable-id", stack.Label) require.NoError(t, err) assert.Equal(t, idBeforeUpdate, idAfterUpdate, "the generator's KSUID identity must survive an update") }) diff --git a/internal/datastore/sqlite/sqlite.go b/internal/datastore/sqlite/sqlite.go index f86518903..4c18219a1 100644 --- a/internal/datastore/sqlite/sqlite.go +++ b/internal/datastore/sqlite/sqlite.go @@ -2792,9 +2792,10 @@ func (d DatastoreSQLite) DeletePoliciesForStack(stackID string, commandID string return nil } -// CreateGenerator persists a new generator. Unlike CreatePolicy, the stack is -// never NULL: a generator is always inline to exactly one stack, read off -// gen.GetStack() rather than threaded as a separate argument. +// CreateGenerator persists a new generator. stack_id stores the stack's +// resolved KSUID — like policy_id on an inline policy, not the label — read +// off gen.GetStackID(). Unlike CreatePolicy the column is never NULL: a +// generator is always inline to exactly one stack. func (d DatastoreSQLite) CreateGenerator(gen pkgmodel.Generator, commandID string) (string, error) { _, span := sqliteTracer.Start(context.Background(), "CreateGenerator") defer span.End() @@ -2809,7 +2810,7 @@ func (d DatastoreSQLite) CreateGenerator(gen pkgmodel.Generator, commandID strin query := `INSERT INTO generators (id, version, command_id, operation, label, generator_type, stack_id, generator_data) VALUES (?, ?, ?, ?, ?, ?, ?, ?)` - _, err = d.conn.Exec(query, id, version, commandID, "create", gen.GetLabel(), gen.GetType(), gen.GetStack(), string(data)) + _, err = d.conn.Exec(query, id, version, commandID, "create", gen.GetLabel(), gen.GetType(), gen.GetStackID(), string(data)) if err != nil { slog.Error("Failed to create generator", "error", err, "label", gen.GetLabel()) return "", err @@ -2819,9 +2820,9 @@ func (d DatastoreSQLite) CreateGenerator(gen pkgmodel.Generator, commandID strin } // UpdateGenerator persists a new version of an existing generator. The -// existing row is found by label and stack — a generator has no standalone -// form, so unlike UpdatePolicy there is no NULL-stack branch — and the new -// version row carries forward the same id. +// existing row is found by label and stack ID — a generator has no +// standalone form, so unlike UpdatePolicy there is no NULL-stack branch — +// and the new version row carries forward the same id. func (d DatastoreSQLite) UpdateGenerator(gen pkgmodel.Generator, commandID string) (string, error) { _, span := sqliteTracer.Start(context.Background(), "UpdateGenerator") defer span.End() @@ -2829,7 +2830,7 @@ func (d DatastoreSQLite) UpdateGenerator(gen pkgmodel.Generator, commandID strin var id string err := d.conn.QueryRow( `SELECT id FROM generators WHERE label = ? AND stack_id = ? ORDER BY version DESC LIMIT 1`, - gen.GetLabel(), gen.GetStack(), + gen.GetLabel(), gen.GetStackID(), ).Scan(&id) if err != nil { return "", fmt.Errorf("failed to find existing generator: %w", err) @@ -2843,7 +2844,7 @@ func (d DatastoreSQLite) UpdateGenerator(gen pkgmodel.Generator, commandID strin insertQuery := `INSERT INTO generators (id, version, command_id, operation, label, generator_type, stack_id, generator_data) VALUES (?, ?, ?, ?, ?, ?, ?, ?)` - _, err = d.conn.Exec(insertQuery, id, version, commandID, "update", gen.GetLabel(), gen.GetType(), gen.GetStack(), string(data)) + _, err = d.conn.Exec(insertQuery, id, version, commandID, "update", gen.GetLabel(), gen.GetType(), gen.GetStackID(), string(data)) if err != nil { slog.Error("Failed to update generator", "error", err, "label", gen.GetLabel()) return "", err @@ -2853,12 +2854,22 @@ func (d DatastoreSQLite) UpdateGenerator(gen pkgmodel.Generator, commandID strin } // DeleteGenerator soft-deletes the generator with the given label on the -// given stack. A label with no live match is a no-op success that returns an -// empty version, mirroring DeletePolicy. +// given stack. The stack is resolved from its label the same way +// GetGenerator does; a stack that doesn't exist has nothing to delete. A +// label with no live match is a no-op success that returns an empty version, +// mirroring DeletePolicy. func (d DatastoreSQLite) DeleteGenerator(label, stackLabel string) (string, error) { _, span := sqliteTracer.Start(context.Background(), "DeleteGenerator") defer span.End() + stack, err := d.GetStackByLabel(stackLabel) + if err != nil { + return "", fmt.Errorf("failed to resolve stack %q: %w", stackLabel, err) + } + if stack == nil { + return "", nil + } + query := ` WITH latest_generators AS ( SELECT id, generator_type, operation, @@ -2871,7 +2882,7 @@ func (d DatastoreSQLite) DeleteGenerator(label, stackLabel string) (string, erro WHERE rn = 1 AND operation != 'delete' ` var id, generatorType string - err := d.conn.QueryRow(query, stackLabel, label).Scan(&id, &generatorType) + err = d.conn.QueryRow(query, stack.ID, label).Scan(&id, &generatorType) if err != nil { if errors.Is(err, sql.ErrNoRows) { return "", nil @@ -2881,7 +2892,7 @@ func (d DatastoreSQLite) DeleteGenerator(label, stackLabel string) (string, erro version := mksuid.New().String() insertQuery := `INSERT INTO generators (id, version, command_id, operation, label, generator_type, stack_id, generator_data) VALUES (?, ?, ?, ?, ?, ?, ?, ?)` - _, err = d.conn.Exec(insertQuery, id, version, "", "delete", label, generatorType, stackLabel, "{}") + _, err = d.conn.Exec(insertQuery, id, version, "", "delete", label, generatorType, stack.ID, "{}") if err != nil { return "", fmt.Errorf("failed to delete generator: %w", err) } @@ -2892,12 +2903,22 @@ func (d DatastoreSQLite) DeleteGenerator(label, stackLabel string) (string, erro } // GetGenerator retrieves the current (latest, non-deleted) generator with the -// given label on the given stack. Returns nil, nil if no live generator -// matches. +// given label on the given stack. The stack label is resolved to its +// current KSUID first, since generators.stack_id stores the stack's id, not +// its label — mirroring how a policy's inline lookups are scoped by stack +// ID. Returns nil, nil if no live stack or no live generator matches. func (d DatastoreSQLite) GetGenerator(label, stackLabel string) (pkgmodel.Generator, error) { _, span := sqliteTracer.Start(context.Background(), "GetGenerator") defer span.End() + stack, err := d.GetStackByLabel(stackLabel) + if err != nil { + return nil, fmt.Errorf("failed to resolve stack %q: %w", stackLabel, err) + } + if stack == nil { + return nil, nil + } + query := ` WITH latest_generators AS ( SELECT generator_data, operation, @@ -2910,7 +2931,7 @@ func (d DatastoreSQLite) GetGenerator(label, stackLabel string) (pkgmodel.Genera WHERE rn = 1 AND operation != 'delete' ` var dataStr string - err := d.conn.QueryRow(query, stackLabel, label).Scan(&dataStr) + err = d.conn.QueryRow(query, stack.ID, label).Scan(&dataStr) if err != nil { if errors.Is(err, sql.ErrNoRows) { return nil, nil @@ -2922,10 +2943,20 @@ func (d DatastoreSQLite) GetGenerator(label, stackLabel string) (pkgmodel.Genera } // LoadGeneratorsByStack returns all non-deleted generators owned by a stack. +// The stack label is resolved to its current KSUID first, for the same +// reason GetGenerator does. A stack that doesn't exist owns no generators. func (d DatastoreSQLite) LoadGeneratorsByStack(stackLabel string) ([]pkgmodel.Generator, error) { _, span := sqliteTracer.Start(context.Background(), "LoadGeneratorsByStack") defer span.End() + stack, err := d.GetStackByLabel(stackLabel) + if err != nil { + return nil, fmt.Errorf("failed to resolve stack %q: %w", stackLabel, err) + } + if stack == nil { + return nil, nil + } + query := ` WITH latest_generators AS ( SELECT id, generator_data, operation, @@ -2937,7 +2968,7 @@ func (d DatastoreSQLite) LoadGeneratorsByStack(stackLabel string) ([]pkgmodel.Ge FROM latest_generators WHERE rn = 1 AND operation != 'delete' ` - rows, err := d.conn.Query(query, stackLabel) + rows, err := d.conn.Query(query, stack.ID) if err != nil { return nil, err } diff --git a/internal/datastore/sqlite/sqlite_test.go b/internal/datastore/sqlite/sqlite_test.go index f599a0940..8e1148dd6 100644 --- a/internal/datastore/sqlite/sqlite_test.go +++ b/internal/datastore/sqlite/sqlite_test.go @@ -145,9 +145,15 @@ func TestDatastore(t *testing.T) { GeneratorIDForTest: func(label, stackLabel string) (string, error) { conn := d.Conn() var id string + // generators.stack_id stores the stack's KSUID, not its label, so + // the stack is resolved by label first (its own current row), the + // same way the datastore's own Get/DeleteGenerator do. err := conn.QueryRow( - `SELECT id FROM generators WHERE label = ? AND stack_id = ? ORDER BY version DESC LIMIT 1`, - label, stackLabel, + `SELECT g.id FROM generators g + JOIN (SELECT id FROM stacks WHERE label = ? ORDER BY version DESC LIMIT 1) s ON g.stack_id = s.id + WHERE g.label = ? + ORDER BY g.version DESC LIMIT 1`, + stackLabel, label, ).Scan(&id) if err == sql.ErrNoRows { return "", nil diff --git a/pkg/model/generator.go b/pkg/model/generator.go index fe7262208..373d6ff24 100644 --- a/pkg/model/generator.go +++ b/pkg/model/generator.go @@ -18,6 +18,8 @@ type Generator interface { GetType() string GetStack() string SetStack(stack string) + GetStackID() string + SetStackID(id string) } // PasswordGenerator produces a random password value. Fields mirror the @@ -26,6 +28,7 @@ type PasswordGenerator struct { Type string `json:"Type"` // "password" Label string `json:"Label"` Stack string `json:"Stack,omitempty"` + StackID string `json:"-"` // Set during processing, not from PKL EverySeconds *int64 `json:"EverySeconds,omitempty"` Length int `json:"Length"` Uppercase bool `json:"Uppercase"` @@ -40,6 +43,8 @@ func (g *PasswordGenerator) GetLabel() string { return g.Label } func (g *PasswordGenerator) GetType() string { return "password" } func (g *PasswordGenerator) GetStack() string { return g.Stack } func (g *PasswordGenerator) SetStack(stack string) { g.Stack = stack } +func (g *PasswordGenerator) GetStackID() string { return g.StackID } +func (g *PasswordGenerator) SetStackID(id string) { g.StackID = id } // ParseGenerator parses a single generator from JSON, dispatching on the // discriminated Type field the same way ParsePolicy does. From 160fb6e06d4547bd5bbe418f57d695af0871ae97 Mon Sep 17 00:00:00 2001 From: Jeroen Soeters Date: Sat, 29 Aug 2026 00:13:40 -0700 Subject: [PATCH 4/5] feat(datastore): implement generator persistence for postgres, mssql and aurora Replaces the "not yet implemented" generator stubs in the three remaining Datastore backends with real implementations that mirror each backend's existing policy methods: Postgres and MSSQL follow their own CreatePolicy/ UpdatePolicy/DeletePolicy idiom directly, and Aurora gets its own rdsdata-based implementation (it does not delegate to Postgres). Write methods trust the caller's resolved stack KSUID; the read methods resolve a stack label to its KSUID internally, matching how the policy methods already behave. Wires GeneratorIDForTest into each backend's dstest harness so the shared KSUID-stability test runs instead of skipping. --- .../datastore/aurora/aurora_generators.go | 312 +++++++++++++++++- internal/datastore/aurora/aurora_test.go | 3 + internal/datastore/mssql/mssql_dstest_test.go | 18 + internal/datastore/mssql/mssql_generators.go | 214 +++++++++++- .../datastore/postgres/postgres_generators.go | 216 +++++++++++- internal/datastore/postgres/postgres_test.go | 18 + 6 files changed, 738 insertions(+), 43 deletions(-) diff --git a/internal/datastore/aurora/aurora_generators.go b/internal/datastore/aurora/aurora_generators.go index 0b48c95e9..a3b224b31 100644 --- a/internal/datastore/aurora/aurora_generators.go +++ b/internal/datastore/aurora/aurora_generators.go @@ -5,33 +5,315 @@ package aurora import ( + "context" "fmt" + "log/slog" + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/service/rdsdata/types" + "github.com/demula/mksuid/v2" + + "github.com/platform-engineering-labs/formae/internal/datastore" pkgmodel "github.com/platform-engineering-labs/formae/pkg/model" ) -// Generator persistence for Aurora is not yet implemented. The generators -// table exists (see migrations_postgres, which Aurora Data API also runs) so -// schema stays in lockstep across backends, but these methods exist only to -// satisfy datastore.Datastore until a following change implements them -// against the shared dstest suite. +// CreateGenerator persists a new generator. stack_id stores the stack's +// resolved KSUID — like policy_id on an inline policy, not the label — read +// off gen.GetStackID(). Unlike CreatePolicy the column is never NULL: a +// generator is always inline to exactly one stack. +func (d *DatastoreAuroraDataAPI) CreateGenerator(gen pkgmodel.Generator, commandID string) (string, error) { + ctx := context.Background() + + id := mksuid.New().String() + version := mksuid.New().String() + + data, err := datastore.GeneratorData(gen) + if err != nil { + return "", err + } + + query := `INSERT INTO generators (id, version, command_id, operation, label, generator_type, stack_id, generator_data) + VALUES (:id, :version, :command_id, :operation, :label, :generator_type, :stack_id, :generator_data)` + params := []types.SqlParameter{ + {Name: aws.String("id"), Value: &types.FieldMemberStringValue{Value: id}}, + {Name: aws.String("version"), Value: &types.FieldMemberStringValue{Value: version}}, + {Name: aws.String("command_id"), Value: &types.FieldMemberStringValue{Value: commandID}}, + {Name: aws.String("operation"), Value: &types.FieldMemberStringValue{Value: "create"}}, + {Name: aws.String("label"), Value: &types.FieldMemberStringValue{Value: gen.GetLabel()}}, + {Name: aws.String("generator_type"), Value: &types.FieldMemberStringValue{Value: gen.GetType()}}, + {Name: aws.String("stack_id"), Value: &types.FieldMemberStringValue{Value: gen.GetStackID()}}, + {Name: aws.String("generator_data"), Value: &types.FieldMemberStringValue{Value: string(data)}}, + } -func (d *DatastoreAuroraDataAPI) CreateGenerator(_ pkgmodel.Generator, _ string) (string, error) { - return "", fmt.Errorf("generator persistence is not yet implemented for aurora") + _, err = d.executeStatement(ctx, query, params) + if err != nil { + slog.Error("Failed to create generator", "error", err, "label", gen.GetLabel()) + return "", err + } + + return version, nil } -func (d *DatastoreAuroraDataAPI) UpdateGenerator(_ pkgmodel.Generator, _ string) (string, error) { - return "", fmt.Errorf("generator persistence is not yet implemented for aurora") +// UpdateGenerator persists a new version of an existing generator. The +// existing row is found by label and stack ID — a generator has no +// standalone form, so unlike UpdatePolicy there is no NULL-stack branch — +// and the new version row carries forward the same id. +func (d *DatastoreAuroraDataAPI) UpdateGenerator(gen pkgmodel.Generator, commandID string) (string, error) { + ctx := context.Background() + + selectQuery := ` + SELECT id FROM generators + WHERE label = :label AND stack_id = :stack_id + ORDER BY version COLLATE "C" DESC + LIMIT 1 + ` + selectParams := []types.SqlParameter{ + {Name: aws.String("label"), Value: &types.FieldMemberStringValue{Value: gen.GetLabel()}}, + {Name: aws.String("stack_id"), Value: &types.FieldMemberStringValue{Value: gen.GetStackID()}}, + } + + result, err := d.executeStatement(ctx, selectQuery, selectParams) + if err != nil { + return "", fmt.Errorf("failed to find existing generator: %w", err) + } + if len(result.Records) == 0 { + return "", fmt.Errorf("generator not found: %s", gen.GetLabel()) + } + + id, err := getStringField(result.Records[0][0]) + if err != nil { + return "", fmt.Errorf("failed to get generator id: %w", err) + } + + version := mksuid.New().String() + data, err := datastore.GeneratorData(gen) + if err != nil { + return "", err + } + + insertQuery := `INSERT INTO generators (id, version, command_id, operation, label, generator_type, stack_id, generator_data) + VALUES (:id, :version, :command_id, :operation, :label, :generator_type, :stack_id, :generator_data)` + insertParams := []types.SqlParameter{ + {Name: aws.String("id"), Value: &types.FieldMemberStringValue{Value: id}}, + {Name: aws.String("version"), Value: &types.FieldMemberStringValue{Value: version}}, + {Name: aws.String("command_id"), Value: &types.FieldMemberStringValue{Value: commandID}}, + {Name: aws.String("operation"), Value: &types.FieldMemberStringValue{Value: "update"}}, + {Name: aws.String("label"), Value: &types.FieldMemberStringValue{Value: gen.GetLabel()}}, + {Name: aws.String("generator_type"), Value: &types.FieldMemberStringValue{Value: gen.GetType()}}, + {Name: aws.String("stack_id"), Value: &types.FieldMemberStringValue{Value: gen.GetStackID()}}, + {Name: aws.String("generator_data"), Value: &types.FieldMemberStringValue{Value: string(data)}}, + } + + _, err = d.executeStatement(ctx, insertQuery, insertParams) + if err != nil { + slog.Error("Failed to update generator", "error", err, "label", gen.GetLabel()) + return "", err + } + + return version, nil } -func (d *DatastoreAuroraDataAPI) DeleteGenerator(_, _ string) (string, error) { - return "", fmt.Errorf("generator persistence is not yet implemented for aurora") +// DeleteGenerator soft-deletes the generator with the given label on the +// given stack. The stack is resolved from its label the same way +// GetGenerator does; a stack that doesn't exist has nothing to delete. A +// label with no live match is a no-op success that returns an empty version, +// mirroring DeletePolicy. +func (d *DatastoreAuroraDataAPI) DeleteGenerator(label, stackLabel string) (string, error) { + ctx := context.Background() + + stack, err := d.GetStackByLabel(stackLabel) + if err != nil { + return "", fmt.Errorf("failed to resolve stack %q: %w", stackLabel, err) + } + if stack == nil { + return "", nil + } + + query := ` + WITH latest_generators AS ( + SELECT id, generator_type, operation, + ROW_NUMBER() OVER (PARTITION BY id ORDER BY version COLLATE "C" DESC) as rn + FROM generators + WHERE stack_id = :stack_id AND label = :label + ) + SELECT id, generator_type + FROM latest_generators + WHERE rn = 1 AND operation != 'delete' + ` + params := []types.SqlParameter{ + {Name: aws.String("stack_id"), Value: &types.FieldMemberStringValue{Value: stack.ID}}, + {Name: aws.String("label"), Value: &types.FieldMemberStringValue{Value: label}}, + } + result, err := d.executeStatement(ctx, query, params) + if err != nil { + return "", fmt.Errorf("failed to get generator for deletion: %w", err) + } + if len(result.Records) == 0 { + return "", nil + } + + record := result.Records[0] + id, _ := getStringField(record[0]) + generatorType, _ := getStringField(record[1]) + + version := mksuid.New().String() + insertQuery := `INSERT INTO generators (id, version, command_id, operation, label, generator_type, stack_id, generator_data) + VALUES (:id, :version, :command_id, :operation, :label, :generator_type, :stack_id, :generator_data)` + insertParams := []types.SqlParameter{ + {Name: aws.String("id"), Value: &types.FieldMemberStringValue{Value: id}}, + {Name: aws.String("version"), Value: &types.FieldMemberStringValue{Value: version}}, + {Name: aws.String("command_id"), Value: &types.FieldMemberStringValue{Value: ""}}, + {Name: aws.String("operation"), Value: &types.FieldMemberStringValue{Value: "delete"}}, + {Name: aws.String("label"), Value: &types.FieldMemberStringValue{Value: label}}, + {Name: aws.String("generator_type"), Value: &types.FieldMemberStringValue{Value: generatorType}}, + {Name: aws.String("stack_id"), Value: &types.FieldMemberStringValue{Value: stack.ID}}, + {Name: aws.String("generator_data"), Value: &types.FieldMemberStringValue{Value: "{}"}}, + } + _, err = d.executeStatement(ctx, insertQuery, insertParams) + if err != nil { + return "", fmt.Errorf("failed to delete generator: %w", err) + } + + slog.Debug("Deleted generator", "label", label, "id", id, "stackLabel", stackLabel) + + return version, nil } -func (d *DatastoreAuroraDataAPI) GetGenerator(_, _ string) (pkgmodel.Generator, error) { - return nil, fmt.Errorf("generator persistence is not yet implemented for aurora") +// GetGenerator retrieves the current (latest, non-deleted) generator with the +// given label on the given stack. The stack label is resolved to its +// current KSUID first, since generators.stack_id stores the stack's id, not +// its label — mirroring how a policy's inline lookups are scoped by stack +// ID. Returns nil, nil if no live stack or no live generator matches. +func (d *DatastoreAuroraDataAPI) GetGenerator(label, stackLabel string) (pkgmodel.Generator, error) { + ctx := context.Background() + + stack, err := d.GetStackByLabel(stackLabel) + if err != nil { + return nil, fmt.Errorf("failed to resolve stack %q: %w", stackLabel, err) + } + if stack == nil { + return nil, nil + } + + query := ` + WITH latest_generators AS ( + SELECT generator_data, operation, + ROW_NUMBER() OVER (PARTITION BY id ORDER BY version COLLATE "C" DESC) as rn + FROM generators + WHERE stack_id = :stack_id AND label = :label + ) + SELECT generator_data + FROM latest_generators + WHERE rn = 1 AND operation != 'delete' + ` + params := []types.SqlParameter{ + {Name: aws.String("stack_id"), Value: &types.FieldMemberStringValue{Value: stack.ID}}, + {Name: aws.String("label"), Value: &types.FieldMemberStringValue{Value: label}}, + } + result, err := d.executeStatement(ctx, query, params) + if err != nil { + return nil, fmt.Errorf("failed to get generator: %w", err) + } + if len(result.Records) == 0 { + return nil, nil + } + + dataStr, err := getStringField(result.Records[0][0]) + if err != nil { + return nil, fmt.Errorf("failed to get generator data: %w", err) + } + + return datastore.GeneratorFromData([]byte(dataStr)) } -func (d *DatastoreAuroraDataAPI) LoadGeneratorsByStack(_ string) ([]pkgmodel.Generator, error) { - return nil, fmt.Errorf("generator persistence is not yet implemented for aurora") +// LoadGeneratorsByStack returns all non-deleted generators owned by a stack. +// The stack label is resolved to its current KSUID first, for the same +// reason GetGenerator does. A stack that doesn't exist owns no generators. +func (d *DatastoreAuroraDataAPI) LoadGeneratorsByStack(stackLabel string) ([]pkgmodel.Generator, error) { + ctx := context.Background() + + stack, err := d.GetStackByLabel(stackLabel) + if err != nil { + return nil, fmt.Errorf("failed to resolve stack %q: %w", stackLabel, err) + } + if stack == nil { + return nil, nil + } + + query := ` + WITH latest_generators AS ( + SELECT id, generator_data, operation, + ROW_NUMBER() OVER (PARTITION BY id ORDER BY version COLLATE "C" DESC) as rn + FROM generators + WHERE stack_id = :stack_id + ) + SELECT generator_data + FROM latest_generators + WHERE rn = 1 AND operation != 'delete' + ` + params := []types.SqlParameter{ + {Name: aws.String("stack_id"), Value: &types.FieldMemberStringValue{Value: stack.ID}}, + } + result, err := d.executeStatement(ctx, query, params) + if err != nil { + return nil, err + } + + var generators []pkgmodel.Generator + for _, record := range result.Records { + if len(record) < 1 { + continue + } + dataStr, err := getStringField(record[0]) + if err != nil { + slog.Warn("Failed to read generator data, skipping", "error", err, "stackLabel", stackLabel) + continue + } + gen, err := datastore.GeneratorFromData([]byte(dataStr)) + if err != nil { + slog.Warn("Failed to deserialize generator, skipping", "error", err, "stackLabel", stackLabel) + continue + } + generators = append(generators, gen) + } + + return generators, nil +} + +// GeneratorIDForTesting returns the internal KSUID identity (the id column, +// stable across CreateGenerator/UpdateGenerator) of the current (max-version) +// generator row with the given label on the given stack, or "" if none +// exists. Generator has no public API that exposes this id — the Datastore +// interface returns only version strings — so the dstest suite needs a +// direct accessor to prove the id survives an update unchanged. +func (d *DatastoreAuroraDataAPI) GeneratorIDForTesting(label, stackLabel string) (string, error) { + ctx := context.Background() + + stack, err := d.GetStackByLabel(stackLabel) + if err != nil { + return "", fmt.Errorf("failed to resolve stack %q: %w", stackLabel, err) + } + if stack == nil { + return "", nil + } + + query := ` + SELECT id FROM generators + WHERE stack_id = :stack_id AND label = :label + ORDER BY version COLLATE "C" DESC + LIMIT 1 + ` + params := []types.SqlParameter{ + {Name: aws.String("stack_id"), Value: &types.FieldMemberStringValue{Value: stack.ID}}, + {Name: aws.String("label"), Value: &types.FieldMemberStringValue{Value: label}}, + } + result, err := d.executeStatement(ctx, query, params) + if err != nil { + return "", err + } + if len(result.Records) == 0 { + return "", nil + } + + return getStringField(result.Records[0][0]) } diff --git a/internal/datastore/aurora/aurora_test.go b/internal/datastore/aurora/aurora_test.go index 7d701586b..f00fc4f2b 100644 --- a/internal/datastore/aurora/aurora_test.go +++ b/internal/datastore/aurora/aurora_test.go @@ -76,6 +76,9 @@ func TestDatastore(t *testing.T) { NullFormaCommandSubjectForTest: func(commandID string) error { return d.NullFormaCommandSubjectForTesting(commandID) }, + GeneratorIDForTest: func(label, stackLabel string) (string, error) { + return d.GeneratorIDForTesting(label, stackLabel) + }, } }) } diff --git a/internal/datastore/mssql/mssql_dstest_test.go b/internal/datastore/mssql/mssql_dstest_test.go index 8d560fd66..89d77779c 100644 --- a/internal/datastore/mssql/mssql_dstest_test.go +++ b/internal/datastore/mssql/mssql_dstest_test.go @@ -9,6 +9,7 @@ package mssql_test import ( "context" "database/sql" + "errors" "fmt" "testing" "time" @@ -155,6 +156,23 @@ func TestDatastore(t *testing.T) { ) return err }, + GeneratorIDForTest: func(label, stackLabel string) (string, error) { + var id string + // generators.stack_id stores the stack's KSUID, not its label, so + // the stack is resolved by label first (its own current row), the + // same way the datastore's own Get/DeleteGenerator do. + err := conn.QueryRow( + `SELECT TOP (1) g.id FROM generators g + JOIN (SELECT TOP (1) id FROM stacks WHERE label = @p1 ORDER BY version COLLATE Latin1_General_BIN2 DESC) s ON g.stack_id = s.id + WHERE g.label = @p2 + ORDER BY g.version COLLATE Latin1_General_BIN2 DESC`, + stackLabel, label, + ).Scan(&id) + if errors.Is(err, sql.ErrNoRows) { + return "", nil + } + return id, err + }, CleanUpFn: func() error { ds.Close() m, err := sql.Open("sqlserver", dstestMSSQLBase+"&database=master") diff --git a/internal/datastore/mssql/mssql_generators.go b/internal/datastore/mssql/mssql_generators.go index 047391632..886c0df3f 100644 --- a/internal/datastore/mssql/mssql_generators.go +++ b/internal/datastore/mssql/mssql_generators.go @@ -5,32 +5,218 @@ package mssql import ( + "context" + "database/sql" + "errors" "fmt" + "log/slog" + "github.com/demula/mksuid/v2" + + "github.com/platform-engineering-labs/formae/internal/datastore" pkgmodel "github.com/platform-engineering-labs/formae/pkg/model" ) -// Generator persistence for MSSQL is not yet implemented. The generators -// table exists (see migrations_mssql) so schema stays in lockstep across -// backends, but these methods exist only to satisfy datastore.Datastore until -// a following change implements them against the shared dstest suite. +// CreateGenerator persists a new generator. stack_id stores the stack's +// resolved KSUID — like policy_id on an inline policy, not the label — read +// off gen.GetStackID(). Unlike CreatePolicy the column is never NULL: a +// generator is always inline to exactly one stack. +func (d *DatastoreMSSQL) CreateGenerator(gen pkgmodel.Generator, commandID string) (string, error) { + ctx, span := mssqlTracer.Start(context.Background(), "CreateGenerator") + defer span.End() + + id := mksuid.New().String() + version := mksuid.New().String() + + data, err := datastore.GeneratorData(gen) + if err != nil { + return "", err + } -func (d *DatastoreMSSQL) CreateGenerator(_ pkgmodel.Generator, _ string) (string, error) { - return "", fmt.Errorf("generator persistence is not yet implemented for mssql") + query := `INSERT INTO generators (id, version, command_id, operation, label, generator_type, stack_id, generator_data) + VALUES (@p1, @p2, @p3, @p4, @p5, @p6, @p7, @p8)` + _, err = d.conn.ExecContext(ctx, query, id, version, commandID, "create", + gen.GetLabel(), gen.GetType(), gen.GetStackID(), string(data)) + if err != nil { + slog.Error("Failed to create generator", "error", err, "label", gen.GetLabel()) + return "", err + } + + return version, nil } -func (d *DatastoreMSSQL) UpdateGenerator(_ pkgmodel.Generator, _ string) (string, error) { - return "", fmt.Errorf("generator persistence is not yet implemented for mssql") +// UpdateGenerator persists a new version of an existing generator. The +// existing row is found by label and stack ID — a generator has no +// standalone form, so unlike UpdatePolicy there is no NULL-stack branch — +// and the new version row carries forward the same id. +func (d *DatastoreMSSQL) UpdateGenerator(gen pkgmodel.Generator, commandID string) (string, error) { + ctx, span := mssqlTracer.Start(context.Background(), "UpdateGenerator") + defer span.End() + + query := ` + SELECT TOP (1) id FROM generators + WHERE label = @p1 AND stack_id = @p2 + ORDER BY version COLLATE Latin1_General_BIN2 DESC` + var id string + err := d.conn.QueryRowContext(ctx, query, gen.GetLabel(), gen.GetStackID()).Scan(&id) + if err != nil { + return "", fmt.Errorf("failed to find existing generator: %w", err) + } + + version := mksuid.New().String() + data, err := datastore.GeneratorData(gen) + if err != nil { + return "", err + } + + insertQuery := `INSERT INTO generators (id, version, command_id, operation, label, generator_type, stack_id, generator_data) + VALUES (@p1, @p2, @p3, @p4, @p5, @p6, @p7, @p8)` + _, err = d.conn.ExecContext(ctx, insertQuery, id, version, commandID, "update", + gen.GetLabel(), gen.GetType(), gen.GetStackID(), string(data)) + if err != nil { + slog.Error("Failed to update generator", "error", err, "label", gen.GetLabel()) + return "", err + } + + return version, nil } -func (d *DatastoreMSSQL) DeleteGenerator(_, _ string) (string, error) { - return "", fmt.Errorf("generator persistence is not yet implemented for mssql") +// DeleteGenerator soft-deletes the generator with the given label on the +// given stack. The stack is resolved from its label the same way +// GetGenerator does; a stack that doesn't exist has nothing to delete. A +// label with no live match is a no-op success that returns an empty version, +// mirroring DeletePolicy. +func (d *DatastoreMSSQL) DeleteGenerator(label, stackLabel string) (string, error) { + ctx, span := mssqlTracer.Start(context.Background(), "DeleteGenerator") + defer span.End() + + stack, err := d.GetStackByLabel(stackLabel) + if err != nil { + return "", fmt.Errorf("failed to resolve stack %q: %w", stackLabel, err) + } + if stack == nil { + return "", nil + } + + query := ` + WITH latest_generators AS ( + SELECT id, generator_type, operation, + ROW_NUMBER() OVER (PARTITION BY id ORDER BY version COLLATE Latin1_General_BIN2 DESC) as rn + FROM generators + WHERE stack_id = @p1 AND label = @p2 + ) + SELECT id, generator_type + FROM latest_generators + WHERE rn = 1 AND operation != 'delete'` + var id, generatorType string + err = d.conn.QueryRowContext(ctx, query, stack.ID, label).Scan(&id, &generatorType) + if err != nil { + if errors.Is(err, sql.ErrNoRows) { + return "", nil + } + return "", fmt.Errorf("failed to get generator for deletion: %w", err) + } + + version := mksuid.New().String() + insertQuery := `INSERT INTO generators (id, version, command_id, operation, label, generator_type, stack_id, generator_data) + VALUES (@p1, @p2, @p3, @p4, @p5, @p6, @p7, @p8)` + _, err = d.conn.ExecContext(ctx, insertQuery, id, version, "", "delete", label, generatorType, stack.ID, "{}") + if err != nil { + return "", fmt.Errorf("failed to delete generator: %w", err) + } + + slog.Debug("Deleted generator", "label", label, "id", id, "stackLabel", stackLabel) + + return version, nil } -func (d *DatastoreMSSQL) GetGenerator(_, _ string) (pkgmodel.Generator, error) { - return nil, fmt.Errorf("generator persistence is not yet implemented for mssql") +// GetGenerator retrieves the current (latest, non-deleted) generator with the +// given label on the given stack. The stack label is resolved to its +// current KSUID first, since generators.stack_id stores the stack's id, not +// its label — mirroring how a policy's inline lookups are scoped by stack +// ID. Returns nil, nil if no live stack or no live generator matches. +func (d *DatastoreMSSQL) GetGenerator(label, stackLabel string) (pkgmodel.Generator, error) { + ctx, span := mssqlTracer.Start(context.Background(), "GetGenerator") + defer span.End() + + stack, err := d.GetStackByLabel(stackLabel) + if err != nil { + return nil, fmt.Errorf("failed to resolve stack %q: %w", stackLabel, err) + } + if stack == nil { + return nil, nil + } + + query := ` + WITH latest_generators AS ( + SELECT generator_data, operation, + ROW_NUMBER() OVER (PARTITION BY id ORDER BY version COLLATE Latin1_General_BIN2 DESC) as rn + FROM generators + WHERE stack_id = @p1 AND label = @p2 + ) + SELECT generator_data + FROM latest_generators + WHERE rn = 1 AND operation != 'delete'` + var dataStr string + err = d.conn.QueryRowContext(ctx, query, stack.ID, label).Scan(&dataStr) + if err != nil { + if errors.Is(err, sql.ErrNoRows) { + return nil, nil + } + return nil, fmt.Errorf("failed to get generator: %w", err) + } + + return datastore.GeneratorFromData([]byte(dataStr)) } -func (d *DatastoreMSSQL) LoadGeneratorsByStack(_ string) ([]pkgmodel.Generator, error) { - return nil, fmt.Errorf("generator persistence is not yet implemented for mssql") +// LoadGeneratorsByStack returns all non-deleted generators owned by a stack. +// The stack label is resolved to its current KSUID first, for the same +// reason GetGenerator does. A stack that doesn't exist owns no generators. +func (d *DatastoreMSSQL) LoadGeneratorsByStack(stackLabel string) ([]pkgmodel.Generator, error) { + ctx, span := mssqlTracer.Start(context.Background(), "LoadGeneratorsByStack") + defer span.End() + + stack, err := d.GetStackByLabel(stackLabel) + if err != nil { + return nil, fmt.Errorf("failed to resolve stack %q: %w", stackLabel, err) + } + if stack == nil { + return nil, nil + } + + query := ` + WITH latest_generators AS ( + SELECT id, generator_data, operation, + ROW_NUMBER() OVER (PARTITION BY id ORDER BY version COLLATE Latin1_General_BIN2 DESC) as rn + FROM generators + WHERE stack_id = @p1 + ) + SELECT generator_data + FROM latest_generators + WHERE rn = 1 AND operation != 'delete'` + rows, err := d.conn.QueryContext(ctx, query, stack.ID) + if err != nil { + return nil, err + } + defer func() { _ = rows.Close() }() + + var generators []pkgmodel.Generator + for rows.Next() { + var dataStr string + if err := rows.Scan(&dataStr); err != nil { + return nil, err + } + gen, err := datastore.GeneratorFromData([]byte(dataStr)) + if err != nil { + slog.Warn("Failed to deserialize generator, skipping", "error", err, "stackLabel", stackLabel) + continue + } + generators = append(generators, gen) + } + + if err := rows.Err(); err != nil { + return nil, err + } + + return generators, nil } diff --git a/internal/datastore/postgres/postgres_generators.go b/internal/datastore/postgres/postgres_generators.go index 68c012270..39a6439cb 100644 --- a/internal/datastore/postgres/postgres_generators.go +++ b/internal/datastore/postgres/postgres_generators.go @@ -5,32 +5,220 @@ package postgres import ( + "context" + "errors" "fmt" + "log/slog" + "github.com/demula/mksuid/v2" + "github.com/jackc/pgx/v5" + + "github.com/platform-engineering-labs/formae/internal/datastore" pkgmodel "github.com/platform-engineering-labs/formae/pkg/model" ) -// Generator persistence for Postgres is not yet implemented. The generators -// table exists (see migrations_postgres) so schema stays in lockstep across -// backends, but these methods exist only to satisfy datastore.Datastore until -// a following change implements them against the shared dstest suite. +// CreateGenerator persists a new generator. stack_id stores the stack's +// resolved KSUID — like policy_id on an inline policy, not the label — read +// off gen.GetStackID(). Unlike CreatePolicy the column is never NULL: a +// generator is always inline to exactly one stack. +func (d DatastorePostgres) CreateGenerator(gen pkgmodel.Generator, commandID string) (string, error) { + ctx, span := tracer.Start(context.Background(), "CreateGenerator") + defer span.End() + + id := mksuid.New().String() + version := mksuid.New().String() + + data, err := datastore.GeneratorData(gen) + if err != nil { + return "", err + } -func (d DatastorePostgres) CreateGenerator(_ pkgmodel.Generator, _ string) (string, error) { - return "", fmt.Errorf("generator persistence is not yet implemented for postgres") + query := `INSERT INTO generators (id, version, command_id, operation, label, generator_type, stack_id, generator_data) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8)` + _, err = d.pool.Exec(ctx, query, id, version, commandID, "create", gen.GetLabel(), gen.GetType(), gen.GetStackID(), string(data)) + if err != nil { + slog.Error("Failed to create generator", "error", err, "label", gen.GetLabel()) + return "", err + } + + return version, nil } -func (d DatastorePostgres) UpdateGenerator(_ pkgmodel.Generator, _ string) (string, error) { - return "", fmt.Errorf("generator persistence is not yet implemented for postgres") +// UpdateGenerator persists a new version of an existing generator. The +// existing row is found by label and stack ID — a generator has no +// standalone form, so unlike UpdatePolicy there is no NULL-stack branch — +// and the new version row carries forward the same id. +func (d DatastorePostgres) UpdateGenerator(gen pkgmodel.Generator, commandID string) (string, error) { + ctx, span := tracer.Start(context.Background(), "UpdateGenerator") + defer span.End() + + query := ` + SELECT id FROM generators + WHERE label = $1 AND stack_id = $2 + ORDER BY version COLLATE "C" DESC + LIMIT 1 + ` + var id string + err := d.pool.QueryRow(ctx, query, gen.GetLabel(), gen.GetStackID()).Scan(&id) + if err != nil { + return "", fmt.Errorf("failed to find existing generator: %w", err) + } + + version := mksuid.New().String() + data, err := datastore.GeneratorData(gen) + if err != nil { + return "", err + } + + insertQuery := `INSERT INTO generators (id, version, command_id, operation, label, generator_type, stack_id, generator_data) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8)` + _, err = d.pool.Exec(ctx, insertQuery, id, version, commandID, "update", gen.GetLabel(), gen.GetType(), gen.GetStackID(), string(data)) + if err != nil { + slog.Error("Failed to update generator", "error", err, "label", gen.GetLabel()) + return "", err + } + + return version, nil } -func (d DatastorePostgres) DeleteGenerator(_, _ string) (string, error) { - return "", fmt.Errorf("generator persistence is not yet implemented for postgres") +// DeleteGenerator soft-deletes the generator with the given label on the +// given stack. The stack is resolved from its label the same way +// GetGenerator does; a stack that doesn't exist has nothing to delete. A +// label with no live match is a no-op success that returns an empty version, +// mirroring DeletePolicy. +func (d DatastorePostgres) DeleteGenerator(label, stackLabel string) (string, error) { + ctx, span := tracer.Start(context.Background(), "DeleteGenerator") + defer span.End() + + stack, err := d.GetStackByLabel(stackLabel) + if err != nil { + return "", fmt.Errorf("failed to resolve stack %q: %w", stackLabel, err) + } + if stack == nil { + return "", nil + } + + query := ` + WITH latest_generators AS ( + SELECT id, generator_type, operation, + ROW_NUMBER() OVER (PARTITION BY id ORDER BY version COLLATE "C" DESC) as rn + FROM generators + WHERE stack_id = $1 AND label = $2 + ) + SELECT id, generator_type + FROM latest_generators + WHERE rn = 1 AND operation != 'delete' + ` + var id, generatorType string + err = d.pool.QueryRow(ctx, query, stack.ID, label).Scan(&id, &generatorType) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + return "", nil + } + return "", fmt.Errorf("failed to get generator for deletion: %w", err) + } + + version := mksuid.New().String() + insertQuery := `INSERT INTO generators (id, version, command_id, operation, label, generator_type, stack_id, generator_data) VALUES ($1, $2, $3, $4, $5, $6, $7, $8)` + _, err = d.pool.Exec(ctx, insertQuery, id, version, "", "delete", label, generatorType, stack.ID, "{}") + if err != nil { + return "", fmt.Errorf("failed to delete generator: %w", err) + } + + slog.Debug("Deleted generator", "label", label, "id", id, "stackLabel", stackLabel) + + return version, nil } -func (d DatastorePostgres) GetGenerator(_, _ string) (pkgmodel.Generator, error) { - return nil, fmt.Errorf("generator persistence is not yet implemented for postgres") +// GetGenerator retrieves the current (latest, non-deleted) generator with the +// given label on the given stack. The stack label is resolved to its +// current KSUID first, since generators.stack_id stores the stack's id, not +// its label — mirroring how a policy's inline lookups are scoped by stack +// ID. Returns nil, nil if no live stack or no live generator matches. +func (d DatastorePostgres) GetGenerator(label, stackLabel string) (pkgmodel.Generator, error) { + ctx, span := tracer.Start(context.Background(), "GetGenerator") + defer span.End() + + stack, err := d.GetStackByLabel(stackLabel) + if err != nil { + return nil, fmt.Errorf("failed to resolve stack %q: %w", stackLabel, err) + } + if stack == nil { + return nil, nil + } + + query := ` + WITH latest_generators AS ( + SELECT generator_data, operation, + ROW_NUMBER() OVER (PARTITION BY id ORDER BY version COLLATE "C" DESC) as rn + FROM generators + WHERE stack_id = $1 AND label = $2 + ) + SELECT generator_data + FROM latest_generators + WHERE rn = 1 AND operation != 'delete' + ` + var dataStr string + err = d.pool.QueryRow(ctx, query, stack.ID, label).Scan(&dataStr) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + return nil, nil + } + return nil, fmt.Errorf("failed to get generator: %w", err) + } + + return datastore.GeneratorFromData([]byte(dataStr)) } -func (d DatastorePostgres) LoadGeneratorsByStack(_ string) ([]pkgmodel.Generator, error) { - return nil, fmt.Errorf("generator persistence is not yet implemented for postgres") +// LoadGeneratorsByStack returns all non-deleted generators owned by a stack. +// The stack label is resolved to its current KSUID first, for the same +// reason GetGenerator does. A stack that doesn't exist owns no generators. +func (d DatastorePostgres) LoadGeneratorsByStack(stackLabel string) ([]pkgmodel.Generator, error) { + ctx, span := tracer.Start(context.Background(), "LoadGeneratorsByStack") + defer span.End() + + stack, err := d.GetStackByLabel(stackLabel) + if err != nil { + return nil, fmt.Errorf("failed to resolve stack %q: %w", stackLabel, err) + } + if stack == nil { + return nil, nil + } + + query := ` + WITH latest_generators AS ( + SELECT id, generator_data, operation, + ROW_NUMBER() OVER (PARTITION BY id ORDER BY version COLLATE "C" DESC) as rn + FROM generators + WHERE stack_id = $1 + ) + SELECT generator_data + FROM latest_generators + WHERE rn = 1 AND operation != 'delete' + ` + rows, err := d.pool.Query(ctx, query, stack.ID) + if err != nil { + return nil, err + } + defer rows.Close() + + var generators []pkgmodel.Generator + for rows.Next() { + var dataStr string + if err := rows.Scan(&dataStr); err != nil { + return nil, err + } + gen, err := datastore.GeneratorFromData([]byte(dataStr)) + if err != nil { + slog.Warn("Failed to deserialize generator, skipping", "error", err, "stackLabel", stackLabel) + continue + } + generators = append(generators, gen) + } + + if err := rows.Err(); err != nil { + return nil, err + } + + return generators, nil } diff --git a/internal/datastore/postgres/postgres_test.go b/internal/datastore/postgres/postgres_test.go index a1650180a..a4774dc48 100644 --- a/internal/datastore/postgres/postgres_test.go +++ b/internal/datastore/postgres/postgres_test.go @@ -9,6 +9,7 @@ package postgres_test import ( "context" "encoding/json" + "errors" "fmt" "testing" "time" @@ -449,6 +450,23 @@ func TestDatastore(t *testing.T) { ) return err }, + GeneratorIDForTest: func(label, stackLabel string) (string, error) { + var id string + // generators.stack_id stores the stack's KSUID, not its label, so + // the stack is resolved by label first (its own current row), the + // same way the datastore's own Get/DeleteGenerator do. + err := d.Pool().QueryRow(context.Background(), + `SELECT g.id FROM generators g + JOIN (SELECT id FROM stacks WHERE label = $1 ORDER BY version COLLATE "C" DESC LIMIT 1) s ON g.stack_id = s.id + WHERE g.label = $2 + ORDER BY g.version COLLATE "C" DESC LIMIT 1`, + stackLabel, label, + ).Scan(&id) + if errors.Is(err, pgx.ErrNoRows) { + return "", nil + } + return id, err + }, } }) } From 6819e9b7802e8db06550715a57eb828dd296a409 Mon Sep 17 00:00:00 2001 From: Jeroen Soeters Date: Sat, 29 Aug 2026 00:41:19 -0700 Subject: [PATCH 5/5] fix(schema): stop generators from wiping stack resources on reconcile SplitByStack routed a generator-only stack into the split even though the stack had no resources, which the reconcile generator reads as "desired state has zero resources" and deletes every existing resource on that stack. Drop the generator branch from SplitByStack entirely: nothing reads split.Generators, so this is removing speculative surface. Also finishes off other loose ends from the generator-kind slice: require a generator's stack (with actual eval-time enforcement, since a StackResolvable's own fields are all optional and silently default-construct otherwise), cut the unused rotation/RotationSpec/ EverySeconds knob (it evaluated and persisted but nothing ever read it), stop PasswordGenerator.Type from being settable out of band with GetType(), make the generator round-trip test actually evaluate its output, escape excludeCharacters when emitting PKL, and align two datastore error-handling spots with their sibling backends. --- .../datastore/aurora/aurora_generators.go | 10 +- internal/datastore/dstest/suite_generators.go | 1 - internal/datastore/sqlite/sqlite_test.go | 3 +- ...esource_update_generator_reconcile_test.go | 68 +++++++++++ .../schema/pkl/generator/pklGenerator.pkl | 23 ++-- internal/schema/pkl/pkl_generate_test.go | 8 +- internal/schema/pkl/pkl_test.go | 19 +-- internal/schema/pkl/schema/formae.pkl | 26 ++--- .../forma/generator_no_stack_test.pkl | 17 +++ .../pkl/testdata/forma/generator_test.pkl | 9 -- pkg/model/forma.go | 41 ------- pkg/model/forma_test.go | 108 ------------------ pkg/model/generator.go | 20 +++- pkg/model/generator_test.go | 20 +++- 14 files changed, 168 insertions(+), 205 deletions(-) create mode 100644 internal/schema/pkl/testdata/forma/generator_no_stack_test.pkl delete mode 100644 pkg/model/forma_test.go diff --git a/internal/datastore/aurora/aurora_generators.go b/internal/datastore/aurora/aurora_generators.go index a3b224b31..40b45db68 100644 --- a/internal/datastore/aurora/aurora_generators.go +++ b/internal/datastore/aurora/aurora_generators.go @@ -153,8 +153,14 @@ func (d *DatastoreAuroraDataAPI) DeleteGenerator(label, stackLabel string) (stri } record := result.Records[0] - id, _ := getStringField(record[0]) - generatorType, _ := getStringField(record[1]) + id, err := getStringField(record[0]) + if err != nil { + return "", fmt.Errorf("failed to get generator id: %w", err) + } + generatorType, err := getStringField(record[1]) + if err != nil { + return "", fmt.Errorf("failed to get generator type: %w", err) + } version := mksuid.New().String() insertQuery := `INSERT INTO generators (id, version, command_id, operation, label, generator_type, stack_id, generator_data) diff --git a/internal/datastore/dstest/suite_generators.go b/internal/datastore/dstest/suite_generators.go index 634186d5b..e2fa25dcd 100644 --- a/internal/datastore/dstest/suite_generators.go +++ b/internal/datastore/dstest/suite_generators.go @@ -36,7 +36,6 @@ func createGeneratorStack(t *testing.T, ds datastore.Datastore, label string) *p // is out of scope here. func testPasswordGenerator(label string, stack *pkgmodel.Stack, length int) *pkgmodel.PasswordGenerator { return &pkgmodel.PasswordGenerator{ - Type: "password", Label: label, Stack: stack.Label, StackID: stack.ID, diff --git a/internal/datastore/sqlite/sqlite_test.go b/internal/datastore/sqlite/sqlite_test.go index 8e1148dd6..31b985633 100644 --- a/internal/datastore/sqlite/sqlite_test.go +++ b/internal/datastore/sqlite/sqlite_test.go @@ -10,6 +10,7 @@ import ( "context" "database/sql" "encoding/json" + "errors" "fmt" "strings" "testing" @@ -155,7 +156,7 @@ func TestDatastore(t *testing.T) { ORDER BY g.version DESC LIMIT 1`, stackLabel, label, ).Scan(&id) - if err == sql.ErrNoRows { + if errors.Is(err, sql.ErrNoRows) { return "", nil } return id, err diff --git a/internal/metastructure/resource_update/resource_update_generator_reconcile_test.go b/internal/metastructure/resource_update/resource_update_generator_reconcile_test.go index 4f006824a..95cfb6c4a 100644 --- a/internal/metastructure/resource_update/resource_update_generator_reconcile_test.go +++ b/internal/metastructure/resource_update/resource_update_generator_reconcile_test.go @@ -1116,6 +1116,74 @@ func TestGenerateResourceUpdatesForReconcile_ImplicitDelete(t *testing.T) { assert.Equal(t, "my-s3-bucket-delete", updates[0].DesiredState.Label) } +// TestGenerateResourceUpdatesForReconcile_GeneratorOnlyStackKeepsExistingResources +// verifies that reconciling a forma which declares only a generator on a +// stack that already holds a managed resource does not delete that +// resource. A generator carries no resources of its own, so the split +// Forma for its stack must never stand in for an empty desired resource +// set. +func TestGenerateResourceUpdatesForReconcile_GeneratorOnlyStackKeepsExistingResources(t *testing.T) { + ds, _ := GetDeps(t) + + resource := pkgmodel.Resource{ + Label: "my-s3-bucket", + Type: "AWS::S3::Bucket", + Stack: "infrastructure", + Target: "test-target", + Schema: pkgmodel.Schema{ + Identifier: "BucketName", + Hints: map[string]pkgmodel.FieldHint{ + "BucketName": { + CreateOnly: true, + }, + }, + Fields: []string{"BucketName"}, + }, + Properties: json.RawMessage(`{"BucketName": "my-unique-bucket-name"}`), + Managed: true, + } + + // First persist the stack with its resource. + existingStack := &pkgmodel.Forma{ + Stacks: []pkgmodel.Stack{{Label: "infrastructure"}}, + Resources: []pkgmodel.Resource{resource}, + } + _, err := ds.StoreStack(existingStack, "test-command-1") + assert.NoError(t, err) + + generator := json.RawMessage(`{ + "Type": "password", + "Label": "db-password", + "Stack": "infrastructure", + "Length": 24, + "Uppercase": true, + "Lowercase": true, + "Digits": true, + "Symbols": false, + "RequireEachIncludedType": true + }`) + + // Apply a forma that declares only the generator on the same stack - + // the resource is not repeated in the desired state. + mode := pkgmodel.FormaApplyModeReconcile + forma := &pkgmodel.Forma{ + Stacks: []pkgmodel.Stack{{Label: "infrastructure"}}, + Generators: []json.RawMessage{generator}, + } + + targetMap := map[string]*pkgmodel.Target{ + "test-target": { + Label: "test-target", + Config: json.RawMessage(`{"Region": "us-west-2"}`), + Namespace: "aws", + }, + } + + updates, err := generateResourceUpdatesForApply(forma, mode, FormaCommandSourceUser, targetMap, targetMap, ds, nil, false) + assert.NoError(t, err) + assert.Empty(t, updates) +} + func TestGenerateResourceUpdatesForReconcile_Update(t *testing.T) { ds, _ := GetDeps(t) diff --git a/internal/schema/pkl/generator/pklGenerator.pkl b/internal/schema/pkl/generator/pklGenerator.pkl index 9aaf66995..eb5f60455 100644 --- a/internal/schema/pkl/generator/pklGenerator.pkl +++ b/internal/schema/pkl/generator/pklGenerator.pkl @@ -255,6 +255,17 @@ function parsePolicies(policiesData: List>): String = )) policiesString.join("\n") +// Escapes a value about to be interpolated into an emitted PKL string +// literal. Mirrors gen.pkl's escapeString (not importable here: that +// function is local to gen.pkl). +local function escapeGeneratorString(str: String): String = + str + .replaceAll("\\", "\\\\") + .replaceAll("\"", "\\\"") + .replaceAll("\n", "\\n") + .replaceAll("\r", "\\r") + .replaceAll("\t", "\\t") + /// Parses standalone generators and generates PKL output. Mirrors /// parsePolicies: generators are always declared standalone (never nested /// inside a stack block), but unlike policies they carry their own `stack` @@ -271,13 +282,6 @@ function parseGenerators(generatorsData: List>, stackLabelMap: else "" ) - let (everySeconds = generatorData.getOrNull("EverySeconds") as Number?) - let (rotationLine = if (everySeconds != null) - "\n rotation { every = \(everySeconds).s }" - else - "" - ) - if (generatorType == "password") let (length = generatorData["Length"] as Number) let (uppercase = generatorData["Uppercase"] as Boolean) @@ -289,13 +293,13 @@ function parseGenerators(generatorsData: List>, stackLabelMap: """ local \(camelCaseLabel) = new formae.PasswordGenerator { - label = "\(label)"\(stackLine)\(rotationLine) + label = "\(label)"\(stackLine) length = \(length) uppercase = \(uppercase) lowercase = \(lowercase) digits = \(digits) symbols = \(symbols) - excludeCharacters = "\(excludeCharacters)" + excludeCharacters = "\(escapeGeneratorString(excludeCharacters))" requireEachIncludedType = \(requireEachIncludedType) } \(camelCaseLabel) @@ -453,7 +457,6 @@ function generateFormaFile(parsed: json.Value): String = "Type", generator.Type, "Label", generator.Label, "Stack", generator.getPropertyOrNull("Stack"), - "EverySeconds", generator.getPropertyOrNull("EverySeconds"), "Length", generator.getPropertyOrNull("Length"), "Uppercase", generator.getPropertyOrNull("Uppercase"), "Lowercase", generator.getPropertyOrNull("Lowercase"), diff --git a/internal/schema/pkl/pkl_generate_test.go b/internal/schema/pkl/pkl_generate_test.go index 155c90f30..9c94d6310 100644 --- a/internal/schema/pkl/pkl_generate_test.go +++ b/internal/schema/pkl/pkl_generate_test.go @@ -121,7 +121,8 @@ func TestGenerateSourceCode_HashedSecretCount_ZeroForNonHashed(t *testing.T) { // a standalone generator (as it would when re-serialized from stored state, // the same way standalone policies already are) round-trips through // GenerateSourceCode into a .pkl file that declares an equivalent -// formae.PasswordGenerator referencing its stack. +// formae.PasswordGenerator referencing its stack, and that the emitted file +// itself evaluates. func TestGenerateSourceCode_Generator_RoundTrips(t *testing.T) { deps, pluginDir := fakeawsDeps(t) @@ -140,7 +141,6 @@ func TestGenerateSourceCode_Generator_RoundTrips(t *testing.T) { "Type": "password", "Label": "db-password", "Stack": "default", - "EverySeconds": 2592000, "Length": 24, "Uppercase": true, "Lowercase": true, @@ -172,7 +172,9 @@ func TestGenerateSourceCode_Generator_RoundTrips(t *testing.T) { assert.Contains(t, generated, "new formae.PasswordGenerator {") assert.Contains(t, generated, `label = "db-password"`) assert.Contains(t, generated, "stack = default.res") - assert.Contains(t, generated, "rotation { every = 2592000.s }") assert.Contains(t, generated, "length = 24") assert.Contains(t, generated, `excludeCharacters = "oO0"`) + + _, err = PKL{}.Evaluate(targetPath, model.CommandApply, model.FormaApplyModeReconcile, nil) + require.NoError(t, err, "emitted PKL must itself evaluate") } diff --git a/internal/schema/pkl/pkl_test.go b/internal/schema/pkl/pkl_test.go index 2b7f0f444..725e95b69 100644 --- a/internal/schema/pkl/pkl_test.go +++ b/internal/schema/pkl/pkl_test.go @@ -269,10 +269,9 @@ func TestPkl_SecretShapeMisuse_BareMapSecretValueFailsEval(t *testing.T) { assert.ErrorContains(t, err, "SecretMapAccessor") } -// TestPkl_Generator_Evaluate verifies that a forma declaring PasswordGenerator -// instances evaluates and renders a Generators listing carrying the fields -// PasswordGenerator.render() produces, including EverySeconds when a -// rotation cadence is attached and its absence when it is not. +// TestPkl_Generator_Evaluate verifies that a forma declaring a +// PasswordGenerator evaluates and renders a Generators listing carrying the +// fields PasswordGenerator.render() produces. func TestPkl_Generator_Evaluate(t *testing.T) { p := PKL{} forma, err := p.Evaluate("./testdata/forma/generator_test.pkl", model.CommandApply, model.FormaApplyModeReconcile, nil) @@ -290,12 +289,14 @@ func TestPkl_Generator_Evaluate(t *testing.T) { assert.False(t, gjson.Get(jsonString, "Generators.0.Symbols").Bool()) assert.Equal(t, "oO0", gjson.Get(jsonString, "Generators.0.ExcludeCharacters").String()) assert.True(t, gjson.Get(jsonString, "Generators.0.RequireEachIncludedType").Bool()) - assert.False(t, gjson.Get(jsonString, "Generators.0.EverySeconds").Exists(), - "EverySeconds must be absent when no rotation is attached") +} - assert.Equal(t, "rotating-password", gjson.Get(jsonString, "Generators.1.Label").String()) - assert.Equal(t, int64(2592000), gjson.Get(jsonString, "Generators.1.EverySeconds").Int(), - "a 30-day rotation must render as 2592000 seconds") +// TestPkl_Generator_NoStackFailsEval verifies that a Generator with no stack +// set fails at PKL eval — stack is required, not defaulted. +func TestPkl_Generator_NoStackFailsEval(t *testing.T) { + p := PKL{} + _, err := p.Evaluate("./testdata/forma/generator_no_stack_test.pkl", model.CommandApply, model.FormaApplyModeReconcile, nil) + require.Error(t, err) } // TestPkl_Generator_AllClassFlagsFalseFailsEval verifies that a diff --git a/internal/schema/pkl/schema/formae.pkl b/internal/schema/pkl/schema/formae.pkl index 178faeb12..93c5fb3d9 100644 --- a/internal/schema/pkl/schema/formae.pkl +++ b/internal/schema/pkl/schema/formae.pkl @@ -171,22 +171,23 @@ open class AutoReconcilePolicy extends Policy { /// resources whose properties take the generated value. abstract class Generator { label: String - stack: StackResolvable? - /// When absent the generator produces a value on first apply and never - /// again on its own. - hidden rotation: RotationSpec? + stack: StackResolvable + + local self = this + + // Every StackResolvable property defaults (Resolvable's fields are all + // optional), so an unset `stack` silently default-constructs to an + // all-null instance instead of failing to evaluate. This is the actual + // enforcement that a generator names its stack; subclasses must fold it + // into their render() the same way PasswordGenerator folds in its own + // eval-time validation. + function hasStack(): Boolean = + self.stack.label != null || throw("Generator \"\(self.label)\": stack is required") abstract function type(): String abstract function render(): Dynamic } -/// How often the agent produces a new value for a generator. -open class RotationSpec { - /// Required: there is no default cadence, so attaching rotation always - /// states the interval. - every: Duration -} - open class PasswordGenerator extends Generator { length: Int(this >= 16) = 32 uppercase: Boolean = true @@ -232,11 +233,10 @@ open class PasswordGenerator extends Generator { true function type(): String = "password" - function render(): Dynamic(validated()) = new { + function render(): Dynamic(validated() && self.hasStack()) = new { Type = "password" Label = self.label Stack = self.stack - EverySeconds = self.rotation?.every?.toUnit("s")?.value?.toInt() Length = length Uppercase = uppercase Lowercase = lowercase diff --git a/internal/schema/pkl/testdata/forma/generator_no_stack_test.pkl b/internal/schema/pkl/testdata/forma/generator_no_stack_test.pkl new file mode 100644 index 000000000..27da00003 --- /dev/null +++ b/internal/schema/pkl/testdata/forma/generator_no_stack_test.pkl @@ -0,0 +1,17 @@ +/* + * © 2025 Platform Engineering Labs Inc. + * + * SPDX-License-Identifier: FSL-1.1-ALv2 + */ + +// Shape-misuse test: a Generator with no stack set has nowhere to persist. +// stack is required (non-null), so PKL eval fails when it is left unset. + +amends "@formae/forma.pkl" +import "@formae/formae.pkl" + +forma { + new formae.PasswordGenerator { + label = "stackless" + } +} diff --git a/internal/schema/pkl/testdata/forma/generator_test.pkl b/internal/schema/pkl/testdata/forma/generator_test.pkl index 00ddca61e..5d0603baa 100644 --- a/internal/schema/pkl/testdata/forma/generator_test.pkl +++ b/internal/schema/pkl/testdata/forma/generator_test.pkl @@ -19,13 +19,4 @@ forma { length = 24 excludeCharacters = "oO0" } - - new formae.PasswordGenerator { - label = "rotating-password" - stack = testStack.res - symbols = true - rotation { - every = 30.d - } - } } diff --git a/pkg/model/forma.go b/pkg/model/forma.go index 9c341f966..0aa0f2fdc 100644 --- a/pkg/model/forma.go +++ b/pkg/model/forma.go @@ -74,47 +74,6 @@ func (f *Forma) SplitByStack() []Forma { } } - // Generators are raw JSON (like Policies), so their Stack has to be read - // out rather than accessed as a Go field. Unlike Policies, a generator - // names its own stack directly (mirroring Resource.Stack), so — unlike - // Policies, which this function drops entirely today — each generator is - // routed to the stack it names. - for _, raw := range f.Generators { - var header struct { - Stack string `json:"Stack"` - } - if err := json.Unmarshal(raw, &header); err != nil { - continue // malformed generator; nothing sane to route it to - } - - if existing, ok := stacks[header.Stack]; ok { - existing.Generators = append(existing.Generators, raw) - continue - } - - var stack *Stack - for _, s := range f.Stacks { - if s.Label == header.Stack { - stack = &s - } - } - - if stack != nil { - stacks[header.Stack] = &Forma{ - Properties: f.Properties, - Stacks: []Stack{*stack}, - Generators: []json.RawMessage{raw}, - } - } else { - // Not present in forma.Stacks - create a minimal stack - stacks[header.Stack] = &Forma{ - Properties: f.Properties, - Stacks: []Stack{{Label: header.Stack}}, - Generators: []json.RawMessage{raw}, - } - } - } - for _, value := range stacks { result = append(result, *value) } diff --git a/pkg/model/forma_test.go b/pkg/model/forma_test.go deleted file mode 100644 index 2c338cbe1..000000000 --- a/pkg/model/forma_test.go +++ /dev/null @@ -1,108 +0,0 @@ -// © 2025 Platform Engineering Labs Inc. -// -// SPDX-License-Identifier: FSL-1.1-ALv2 - -package model - -import ( - "encoding/json" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -// TestSplitByStack_GeneratorLandsOnNamedStack verifies that a generator -// belonging to a stack that also has resources is carried into that stack's -// split Forma, alongside the resources already routed there. -func TestSplitByStack_GeneratorLandsOnNamedStack(t *testing.T) { - generator := json.RawMessage(`{ - "Type": "password", - "Label": "db-password", - "Stack": "app", - "Length": 24, - "Uppercase": true, - "Lowercase": true, - "Digits": true, - "Symbols": false, - "RequireEachIncludedType": true - }`) - - forma := Forma{ - Stacks: []Stack{{Label: "app"}}, - Resources: []Resource{ - {Label: "bucket", Type: "FakeAWS::S3::Bucket", Stack: "app", Target: "aws"}, - }, - Generators: []json.RawMessage{generator}, - } - - split := forma.SplitByStack() - require.Len(t, split, 1) - - appForma := split[0] - require.Len(t, appForma.Resources, 1) - require.Len(t, appForma.Generators, 1) - assert.JSONEq(t, string(generator), string(appForma.Generators[0])) -} - -// TestSplitByStack_GeneratorOnlyStack verifies that a generator whose stack -// carries no resources still produces a split Forma for that stack — a -// generator-only stack is a legitimate shape, not just an appendage to a -// resource-bearing one. -func TestSplitByStack_GeneratorOnlyStack(t *testing.T) { - generator := json.RawMessage(`{ - "Type": "password", - "Label": "seed", - "Stack": "secrets", - "Length": 16, - "Uppercase": true, - "Lowercase": true, - "Digits": true, - "Symbols": false, - "RequireEachIncludedType": true - }`) - - forma := Forma{ - Stacks: []Stack{{Label: "secrets", Description: "generator-only stack"}}, - Generators: []json.RawMessage{generator}, - } - - split := forma.SplitByStack() - require.Len(t, split, 1) - - secretsForma := split[0] - assert.Empty(t, secretsForma.Resources) - require.Len(t, secretsForma.Generators, 1) - assert.JSONEq(t, string(generator), string(secretsForma.Generators[0])) - require.Len(t, secretsForma.Stacks, 1) - assert.Equal(t, "secrets", secretsForma.Stacks[0].Label) - assert.Equal(t, "generator-only stack", secretsForma.Stacks[0].Description) -} - -// TestSplitByStack_TwoGeneratorsDifferentStacks verifies that generators -// naming different stacks are routed to their own split Forma, not merged. -func TestSplitByStack_TwoGeneratorsDifferentStacks(t *testing.T) { - genA := json.RawMessage(`{"Type": "password", "Label": "a", "Stack": "stack-a", "Length": 16, "Uppercase": true, "Lowercase": true, "Digits": true, "Symbols": false, "RequireEachIncludedType": true}`) - genB := json.RawMessage(`{"Type": "password", "Label": "b", "Stack": "stack-b", "Length": 16, "Uppercase": true, "Lowercase": true, "Digits": true, "Symbols": false, "RequireEachIncludedType": true}`) - - forma := Forma{ - Stacks: []Stack{{Label: "stack-a"}, {Label: "stack-b"}}, - Generators: []json.RawMessage{genA, genB}, - } - - split := forma.SplitByStack() - require.Len(t, split, 2) - - byStack := map[string]Forma{} - for _, f := range split { - require.Len(t, f.Stacks, 1) - byStack[f.Stacks[0].Label] = f - } - - require.Contains(t, byStack, "stack-a") - require.Contains(t, byStack, "stack-b") - require.Len(t, byStack["stack-a"].Generators, 1) - require.Len(t, byStack["stack-b"].Generators, 1) - assert.JSONEq(t, string(genA), string(byStack["stack-a"].Generators[0])) - assert.JSONEq(t, string(genB), string(byStack["stack-b"].Generators[0])) -} diff --git a/pkg/model/generator.go b/pkg/model/generator.go index 373d6ff24..d19450016 100644 --- a/pkg/model/generator.go +++ b/pkg/model/generator.go @@ -23,13 +23,13 @@ type Generator interface { } // PasswordGenerator produces a random password value. Fields mirror the -// PKL PasswordGenerator.render() output. +// PKL PasswordGenerator.render() output. Type is not a field: it is a +// constant discriminator, injected by MarshalJSON, so there is exactly one +// place that says what type this generator is. type PasswordGenerator struct { - Type string `json:"Type"` // "password" Label string `json:"Label"` Stack string `json:"Stack,omitempty"` StackID string `json:"-"` // Set during processing, not from PKL - EverySeconds *int64 `json:"EverySeconds,omitempty"` Length int `json:"Length"` Uppercase bool `json:"Uppercase"` Lowercase bool `json:"Lowercase"` @@ -46,6 +46,20 @@ func (g *PasswordGenerator) SetStack(stack string) { g.Stack = stack } func (g *PasswordGenerator) GetStackID() string { return g.StackID } func (g *PasswordGenerator) SetStackID(id string) { g.StackID = id } +// MarshalJSON injects the "Type": "password" discriminator that +// ParseGenerator dispatches on, so callers never set Type by hand and there +// is no way for the marshalled Type to disagree with GetType(). +func (g *PasswordGenerator) MarshalJSON() ([]byte, error) { + type alias PasswordGenerator + return json.Marshal(struct { + Type string `json:"Type"` + alias + }{ + Type: g.GetType(), + alias: alias(*g), + }) +} + // ParseGenerator parses a single generator from JSON, dispatching on the // discriminated Type field the same way ParsePolicy does. func ParseGenerator(raw json.RawMessage) (Generator, error) { diff --git a/pkg/model/generator_test.go b/pkg/model/generator_test.go index cb261126f..2496af98c 100644 --- a/pkg/model/generator_test.go +++ b/pkg/model/generator_test.go @@ -42,7 +42,6 @@ func TestParseGenerator_Password(t *testing.T) { assert.False(t, password.Symbols) assert.Equal(t, "oO0", password.ExcludeCharacters) assert.True(t, password.RequireEachIncludedType) - assert.Nil(t, password.EverySeconds) } // TestParseGenerator_Password_RoundTrip checks that marshaling a parsed @@ -55,7 +54,6 @@ func TestParseGenerator_Password_RoundTrip(t *testing.T) { "Type": "password", "Label": "api-key-seed", "Stack": "secrets-stack", - "EverySeconds": 2592000, "Length": 40, "Uppercase": true, "Lowercase": true, @@ -75,10 +73,22 @@ func TestParseGenerator_Password_RoundTrip(t *testing.T) { require.NoError(t, err) assert.Equal(t, generator, roundTripped) +} + +// TestParseGenerator_Password_MarshalInjectsType verifies that Type is not a +// second source of truth: a PasswordGenerator built without ever setting a +// Type field still marshals "Type": "password", matching GetType(). +func TestParseGenerator_Password_MarshalInjectsType(t *testing.T) { + password := &PasswordGenerator{Label: "unset-type", Length: 16} - password := roundTripped.(*PasswordGenerator) - require.NotNil(t, password.EverySeconds) - assert.Equal(t, int64(2592000), *password.EverySeconds) + marshaled, err := json.Marshal(password) + require.NoError(t, err) + + var decoded struct { + Type string `json:"Type"` + } + require.NoError(t, json.Unmarshal(marshaled, &decoded)) + assert.Equal(t, password.GetType(), decoded.Type) } func TestParseGenerator_UnknownType(t *testing.T) {