From 1fd0adc5717465f87ea6b77c3e6034bf12a760af Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Mon, 2 Nov 2026 00:35:37 +0000 Subject: [PATCH] Allow configuration names in application paths --- docs/roadmap/implementation.md | 2 ++ internal/engine/destination.go | 2 +- .../engine/destination_diagnostic_test.go | 4 +-- internal/engine/destination_load_test.go | 4 +-- internal/engine/destination_test.go | 28 +++++++++++++------ internal/engine/override.go | 25 ++++------------- internal/engine/override_test.go | 26 +++++++++++++++-- internal/engine/sequence_registration_test.go | 17 ++++++++++- 8 files changed, 73 insertions(+), 35 deletions(-) diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 6c4acbf..0c84586 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -102,6 +102,8 @@ must not remove validation of fields inside the actual control block. ## Stage 3 — Permit application paths and CLI data +**Status:** Complete. Focused destination, registration, and override tests pass. + **Owners:** [destination.go](../../internal/engine/destination.go), [sequence_registration.go](../../internal/engine/sequence_registration.go), [override.go](../../internal/engine/override.go), and their existing tests. diff --git a/internal/engine/destination.go b/internal/engine/destination.go index db2b505..06536f2 100644 --- a/internal/engine/destination.go +++ b/internal/engine/destination.go @@ -28,7 +28,7 @@ func normalizeDestination(path Path) (Path, error) { if segment.kind != pathSegmentKey && segment.kind != pathSegmentParsed { return Path{}, errInvalidDestination } - if segment.key == "configkit" || segment.key == "extends" { + if segment.key == "extends" { return Path{}, errInvalidDestination } segments = append(segments, pathSegment{kind: pathSegmentKey, key: segment.key}) diff --git a/internal/engine/destination_diagnostic_test.go b/internal/engine/destination_diagnostic_test.go index 078790b..a801720 100644 --- a/internal/engine/destination_diagnostic_test.go +++ b/internal/engine/destination_diagnostic_test.go @@ -92,7 +92,7 @@ func TestLayeredDestinationDiagnosticsUseEffectivePathsAndAuthoredLocations(t *t t.Run("invalid destination has no fictional source location", func(t *testing.T) { t.Parallel() loaded, err := LoadLayered( - Input{Source: Bytes("main", []byte("value: present\n")), Destination: Keys("configkit")}, + Input{Source: Bytes("main", []byte("value: present\n")), Destination: Keys("extends")}, testSecretBinding(), LayeredOptions[map[string]any]{}, ) @@ -100,7 +100,7 @@ func TestLayeredDestinationDiagnosticsUseEffectivePathsAndAuthoredLocations(t *t t.Fatalf("invalid destination returned a result: %#v", loaded) } diagnostic := firstDestinationDiagnostic(t, err) - if diagnostic.Code != CodeOptionsInvalid || diagnostic.Path.String() != "/configkit" || diagnostic.Primary != nil { + if diagnostic.Code != CodeOptionsInvalid || diagnostic.Path.String() != "/extends" || diagnostic.Primary != nil { t.Fatalf("invalid destination diagnostic = %#v", diagnostic) } }) diff --git a/internal/engine/destination_load_test.go b/internal/engine/destination_load_test.go index a6d1ff5..4118791 100644 --- a/internal/engine/destination_load_test.go +++ b/internal/engine/destination_load_test.go @@ -190,7 +190,7 @@ func TestLayeredLoadPreflightsDestinationsBeforeReadingSources(t *testing.T) { Input{Source: NoSource()}, testSecretBinding(), LayeredOptions[destinationLoadConfig]{Underlays: []Underlay{{Inputs: []Input{{ - Source: Reader("reader", reader), Destination: Keys("configkit"), + Source: Reader("reader", reader), Destination: Keys("extends"), }}}}}, ) if err == nil { @@ -204,7 +204,7 @@ func TestLayeredLoadPreflightsDestinationsBeforeReadingSources(t *testing.T) { t.Fatalf("error = %#v", err) } diagnostic := loadError.Diagnostics()[0] - if diagnostic.Code != CodeOptionsInvalid || diagnostic.Path.String() != "/configkit" || diagnostic.Primary != nil { + if diagnostic.Code != CodeOptionsInvalid || diagnostic.Path.String() != "/extends" || diagnostic.Primary != nil { t.Fatalf("destination diagnostic = %#v", diagnostic) } } diff --git a/internal/engine/destination_test.go b/internal/engine/destination_test.go index f22651e..cb74824 100644 --- a/internal/engine/destination_test.go +++ b/internal/engine/destination_test.go @@ -40,6 +40,20 @@ func TestNormalizeDestinationUsesLiteralMappingKeys(t *testing.T) { } } +func TestNormalizeDestinationAllowsConfigApplicationPaths(t *testing.T) { + for _, path := range []Path{Keys("config"), Keys("configkit"), Keys("outer", "config"), Keys("outer", "configkit"), Keys("config", "relative")} { + normalized, err := normalizeDestination(path) + if err != nil || normalized.String() != path.String() { + t.Errorf("normalizeDestination(%q) = %q, %v", path.String(), normalized.String(), err) + } + } + parent := Keys("outer") + relative, err := normalizeDestination(Keys("config", "child")) + if err != nil || appendDestination(parent, relative).String() != "/outer/config/child" { + t.Fatalf("relative destination = %q, %v", relative.String(), err) + } +} + func TestMappingPlacementModeMatchesZeroValue(t *testing.T) { t.Parallel() @@ -95,13 +109,11 @@ func TestNormalizeDestinationRejectsIndicesAndReservedKeys(t *testing.T) { t.Parallel() for name, path := range map[string]Path{ - "explicit index": Keys("items").Index(0), - "negative index": Keys("items").Index(-1), - "invalid path": Path{invalid: true}, - "configkit": Keys("configkit"), - "extends": Keys("extends"), - "nested configkit": Keys("outer", "configkit"), - "nested extends": Keys("outer", "extends"), + "explicit index": Keys("items").Index(0), + "negative index": Keys("items").Index(-1), + "invalid path": Path{invalid: true}, + "extends": Keys("extends"), + "nested extends": Keys("outer", "extends"), } { t.Run(name, func(t *testing.T) { t.Parallel() @@ -111,7 +123,7 @@ func TestNormalizeDestinationRejectsIndicesAndReservedKeys(t *testing.T) { }) } - for _, key := range []string{"myextends", "configkit2", "", ".", "..", "../x", "a.b", "a/b", "~"} { + for _, key := range []string{"config", "configkit", "myextends", "configkit2", "", ".", "..", "../x", "a.b", "a/b", "~"} { t.Run("ordinary key "+key, func(t *testing.T) { t.Parallel() if _, err := normalizeDestination(Keys(key)); err != nil { diff --git a/internal/engine/override.go b/internal/engine/override.go index 45f2bb5..dddba6e 100644 --- a/internal/engine/override.go +++ b/internal/engine/override.go @@ -108,7 +108,7 @@ func prepareOverrideBatch(base overridePreparationInput, sources []SourceInfo, o }) continue } - if pathContainsDirective(override.Path) || pathContainsInheritanceDirective(override.Path) { + if pathContainsInheritanceDirective(override.Path) { diagnostics = append(diagnostics, Diagnostic{ Code: CodeOverrideDirective, Severity: SeverityError, Path: override.Path.clone(), Message: "configuration overrides cannot create configuration directives", Primary: location, @@ -125,7 +125,7 @@ func prepareOverrideBatch(base overridePreparationInput, sources []SourceInfo, o diagnostics = append(diagnostics, diagnostic) continue } - if relative, ok := overrideStructuralDirectivePath(value, Path{}); ok { + if relative, ok := overrideInheritancePath(value, Path{}); ok { diagnostics = append(diagnostics, Diagnostic{ Code: CodeOverrideDirective, Severity: SeverityError, Path: appendOverridePath(override.Path, relative), Message: "configuration overrides cannot create configuration directives", Primary: location, @@ -201,15 +201,6 @@ func validateOverridePath(path Path, root *document.Node, lookup *documentLookup return nil } -func pathContainsDirective(path Path) bool { - for _, segment := range path.segments { - if segment.key == directiveKey { - return true - } - } - return false -} - func pathContainsInheritanceDirective(path Path) bool { for _, segment := range path.segments { if segment.key == inheritanceKey { @@ -225,27 +216,23 @@ func appendOverridePath(base, relative Path) Path { return result } -func overrideStructuralDirectivePath(node *document.Node, path Path) (Path, bool) { - return overrideStructuralDirectivePathAt(node, path) -} - -func overrideStructuralDirectivePathAt(node *document.Node, path Path) (Path, bool) { +func overrideInheritancePath(node *document.Node, path Path) (Path, bool) { if node == nil { return Path{}, false } if node.Kind == document.Mapping { for _, entry := range node.Mapping { child := path.Key(entry.Key) - if entry.Key == directiveKey || entry.Key == inheritanceKey { + if entry.Key == inheritanceKey { return child, true } - if found, ok := overrideStructuralDirectivePathAt(entry.Value, child); ok { + if found, ok := overrideInheritancePath(entry.Value, child); ok { return found, true } } } else if node.Kind == document.Sequence { for index, child := range node.Sequence { - if found, ok := overrideStructuralDirectivePathAt(child, path.Index(index)); ok { + if found, ok := overrideInheritancePath(child, path.Index(index)); ok { return found, true } } diff --git a/internal/engine/override_test.go b/internal/engine/override_test.go index 635d8f6..a54d276 100644 --- a/internal/engine/override_test.go +++ b/internal/engine/override_test.go @@ -187,8 +187,8 @@ func TestOverrideValidationRejectsPathsOverlapValuesAndProtectedTargets(t *testi {name: "nonfinite", overrides: []Override{{Path: Keys("scalar"), Value: math.Inf(1), Flag: "--scalar"}}, code: CodeOverrideInvalidValue}, {name: "depth", overrides: []Override{{Path: Keys("nested"), Value: deep, Flag: "--nested"}}, code: CodeOverrideLimitExceeded}, {name: "record bytes", overrides: []Override{{Path: Keys("scalar"), Value: strings.Repeat("x", maxSourceBytes+1), Flag: "--scalar"}}, code: CodeOverrideLimitExceeded}, - {name: "directive path", overrides: []Override{{Path: Keys("configkit", "x"), Value: true, Flag: "--directive"}}, code: CodeOverrideDirective}, - {name: "directive value", overrides: []Override{{Path: Keys("nested"), Value: map[string]any{"configkit": map[string]any{}}, Flag: "--nested"}}, code: CodeOverrideDirective}, + {name: "inheritance path", overrides: []Override{{Path: Keys("extends", "x"), Value: true, Flag: "--directive"}}, code: CodeOverrideDirective}, + {name: "inheritance value", overrides: []Override{{Path: Keys("nested"), Value: map[string]any{"extends": map[string]any{}}, Flag: "--nested"}}, code: CodeOverrideDirective}, {name: "version", overrides: []Override{{Path: Keys("version"), Value: "v2", Flag: "--version"}}, code: CodeOverrideVersion}, {name: "version ancestor", overrides: []Override{{Path: Keys("version"), Value: map[string]any{"child": "v2"}, Flag: "--version"}}, code: CodeOverrideVersion}, {name: "label", overrides: []Override{{Path: Keys("scalar"), Value: "x", Flag: "\nCANARY"}}, code: CodeOverrideInvalidLabel}, @@ -206,6 +206,28 @@ func TestOverrideValidationRejectsPathsOverlapValuesAndProtectedTargets(t *testi } } +func TestOverrideAcceptsConfigApplicationData(t *testing.T) { + type config struct { + Config map[string]any `yaml:"config"` + ConfigKit map[string]any `yaml:"configkit"` + Nested map[string]any `yaml:"nested"` + } + result, err := testLoad(NoSource(), Options[config]{Overrides: []Override{ + {Path: Keys("config", "imports"), Value: "ordinary", Flag: "--imports"}, + {Path: Keys("configkit", "sequences"), Value: "ordinary", Flag: "--sequences"}, + {Path: Keys("nested"), Value: map[string]any{"config": map[string]any{"sequences": "ordinary"}, "configkit": "old"}, Flag: "--nested"}, + }}) + if err != nil { + t.Fatal(err) + } + if result.Config.Config["imports"] != "ordinary" || result.Config.ConfigKit["sequences"] != "ordinary" || result.Config.Nested["configkit"] != "old" { + t.Fatalf("overrides = %#v", result.Config) + } + if nested, ok := result.Config.Nested["config"].(map[string]any); !ok || nested["sequences"] != "ordinary" { + t.Fatalf("nested data = %#v", result.Config.Nested) + } +} + func TestOverrideOverlapIndexPreservesFirstAcceptedSource(t *testing.T) { type config struct { Nested map[string]any `yaml:"nested"` diff --git a/internal/engine/sequence_registration_test.go b/internal/engine/sequence_registration_test.go index d18fe02..612fd6d 100644 --- a/internal/engine/sequence_registration_test.go +++ b/internal/engine/sequence_registration_test.go @@ -226,6 +226,21 @@ func TestCompileSequenceIdentityPolicyAllowsDisjointSelectorsAndRoot(t *testing. } } +func TestSequenceIdentitySelectorsAllowConfigKeys(t *testing.T) { + policy, diagnostics := compileSequenceIdentityPolicy([]sequenceIdentityRegistration{ + {selector: sequenceIdentitySelector{literalSequenceSelector("config"), literalSequenceSelector("items")}, identityFields: []string{"id"}}, + {selector: sequenceIdentitySelector{literalSequenceSelector("configkit"), anyMappingSequenceSelector(), literalSequenceSelector("items")}, identityFields: []string{"name"}}, + }) + if len(diagnostics) != 0 { + t.Fatalf("diagnostics = %#v", diagnostics) + } + for _, path := range []Path{Keys("config", "items"), Keys("configkit", "config", "items")} { + if _, matched := policy.identityFieldsAt(path); !matched { + t.Errorf("selector failed to match %q", path.String()) + } + } +} + func TestCompileSequenceIdentityPolicySnapshotsSelectorAndFields(t *testing.T) { selector := sequenceIdentitySelector{literalSequenceSelector("items")} fields := []string{"id", "namespace"} @@ -242,7 +257,7 @@ func TestCompileSequenceIdentityPolicySnapshotsSelectorAndFields(t *testing.T) { func TestCompileSequenceIdentityPolicyRejectsInvalidSelectorsAndEmptyFields(t *testing.T) { _, diagnostics := compileSequenceIdentityPolicy([]sequenceIdentityRegistration{ - {selector: sequenceIdentitySelector{literalSequenceSelector("configkit")}, identityFields: []string{"id"}}, + {selector: sequenceIdentitySelector{literalSequenceSelector("extends")}, identityFields: []string{"id"}}, {selector: sequenceIdentitySelector{{kind: sequenceSelectorSegmentKind(99)}}, identityFields: []string{"id"}}, {selector: sequenceIdentitySelector{literalSequenceSelector("unused")}, identityFields: nil}, })