Index CLI override validation
This commit is contained in:
@@ -310,7 +310,7 @@ Validate: `go test ./internal/engine -run 'Secret|Sensitivity|Inheritance|Histor
|
|||||||
relevant root facade secret/inheritance tests, engine race tests, and the everyday
|
relevant root facade secret/inheritance tests, engine race tests, and the everyday
|
||||||
check. Record collection and propagation outcomes separately.
|
check. Record collection and propagation outcomes separately.
|
||||||
|
|
||||||
## Stage 12: Index CLI overlaps and validation reads
|
## Stage 12: Index CLI overlaps and validation reads — Complete
|
||||||
|
|
||||||
Roadmap candidate 4. Owner: engine `override.go`.
|
Roadmap candidate 4. Owner: engine `override.go`.
|
||||||
|
|
||||||
|
|||||||
@@ -50,12 +50,16 @@ func prepareOverrides(base *composedDocument, sources []SourceInfo, overrides []
|
|||||||
}
|
}
|
||||||
|
|
||||||
seen := make(map[string]int, len(overrides))
|
seen := make(map[string]int, len(overrides))
|
||||||
pathAccepted := make([]bool, len(overrides))
|
var acceptedPaths pathPrefixIndex
|
||||||
|
var baseLookup *documentLookup
|
||||||
|
if len(overrides) != 0 {
|
||||||
|
baseLookup = buildDocumentLookup(base.root)
|
||||||
|
}
|
||||||
var usedBytes int64
|
var usedBytes int64
|
||||||
usedNodes := 0
|
usedNodes := 0
|
||||||
for index, override := range overrides {
|
for index, override := range overrides {
|
||||||
location := &Location{SourceID: prepared[index].source.ID}
|
location := &Location{SourceID: prepared[index].source.ID}
|
||||||
if issue := validateOverridePath(override.Path, base.root); issue != nil {
|
if issue := validateOverridePath(override.Path, base.root, baseLookup); issue != nil {
|
||||||
issue.Primary = location
|
issue.Primary = location
|
||||||
diagnostics = append(diagnostics, *issue)
|
diagnostics = append(diagnostics, *issue)
|
||||||
continue
|
continue
|
||||||
@@ -70,7 +74,7 @@ func prepareOverrides(base *composedDocument, sources []SourceInfo, overrides []
|
|||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
seen[pointer] = index
|
seen[pointer] = index
|
||||||
if conflicting := overlappingOverride(overrides[:index], pathAccepted[:index], override.Path); conflicting >= 0 {
|
if conflicting, overlaps := acceptedPaths.earliestIntersectingID(pointer); overlaps {
|
||||||
diagnostics = append(diagnostics, Diagnostic{
|
diagnostics = append(diagnostics, Diagnostic{
|
||||||
Code: CodeOverrideOverlap, Severity: SeverityError, Path: override.Path.clone(),
|
Code: CodeOverrideOverlap, Severity: SeverityError, Path: override.Path.clone(),
|
||||||
Message: "configuration override paths overlap", Primary: location,
|
Message: "configuration override paths overlap", Primary: location,
|
||||||
@@ -92,7 +96,7 @@ func prepareOverrides(base *composedDocument, sources []SourceInfo, overrides []
|
|||||||
})
|
})
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
pathAccepted[index] = true
|
acceptedPaths.insert(pointer, index)
|
||||||
|
|
||||||
builder := valueBuilder{sourceID: prepared[index].source.ID, active: make(map[valueVisit]bool)}
|
builder := valueBuilder{sourceID: prepared[index].source.ID, active: make(map[valueVisit]bool)}
|
||||||
value, issue := builder.build(override.Value, override.Path, len(override.Path.segments))
|
value, issue := builder.build(override.Value, override.Path, len(override.Path.segments))
|
||||||
@@ -150,7 +154,7 @@ func validOverrideLabel(label string) bool {
|
|||||||
return true
|
return true
|
||||||
}
|
}
|
||||||
|
|
||||||
func validateOverridePath(path Path, root *document.Node) *Diagnostic {
|
func validateOverridePath(path Path, root *document.Node, lookup *documentLookup) *Diagnostic {
|
||||||
if !path.valid() || len(path.segments) == 0 {
|
if !path.valid() || len(path.segments) == 0 {
|
||||||
return &Diagnostic{Code: CodeOverrideInvalidPath, Severity: SeverityError, Path: path.clone(), Message: "configuration override path must be a valid non-root mapping path"}
|
return &Diagnostic{Code: CodeOverrideInvalidPath, Severity: SeverityError, Path: path.clone(), Message: "configuration override path must be a valid non-root mapping path"}
|
||||||
}
|
}
|
||||||
@@ -165,7 +169,7 @@ func validateOverridePath(path Path, root *document.Node) *Diagnostic {
|
|||||||
if current.Kind != document.Mapping {
|
if current.Kind != document.Mapping {
|
||||||
return &Diagnostic{Code: CodeOverrideInvalidPath, Severity: SeverityError, Path: path.clone(), Message: "configuration override path traverses a non-mapping value"}
|
return &Diagnostic{Code: CodeOverrideInvalidPath, Severity: SeverityError, Path: path.clone(), Message: "configuration override path traverses a non-mapping value"}
|
||||||
}
|
}
|
||||||
position := documentEntryIndex(current.Mapping, segment.key)
|
position := lookup.find(current, segment.key)
|
||||||
if index == len(path.segments)-1 {
|
if index == len(path.segments)-1 {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
@@ -178,15 +182,6 @@ func validateOverridePath(path Path, root *document.Node) *Diagnostic {
|
|||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
func overlappingOverride(previous []Override, valid []bool, path Path) int {
|
|
||||||
for index, override := range previous {
|
|
||||||
if valid[index] && (pathContains(override.Path, path) || pathContains(path, override.Path)) {
|
|
||||||
return index
|
|
||||||
}
|
|
||||||
}
|
|
||||||
return -1
|
|
||||||
}
|
|
||||||
|
|
||||||
func pathContainsDirective(path Path) bool {
|
func pathContainsDirective(path Path) bool {
|
||||||
for _, segment := range path.segments {
|
for _, segment := range path.segments {
|
||||||
if segment.key == directiveKey {
|
if segment.key == directiveKey {
|
||||||
|
|||||||
@@ -206,6 +206,132 @@ func TestOverrideValidationRejectsPathsOverlapValuesAndProtectedTargets(t *testi
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestOverrideOverlapIndexPreservesFirstAcceptedSource(t *testing.T) {
|
||||||
|
type config struct {
|
||||||
|
Nested map[string]any `yaml:"nested"`
|
||||||
|
}
|
||||||
|
base := Bytes("override-overlap.yml", []byte("nested: {}\n"))
|
||||||
|
_, err := testLoad(base, Options[config]{Overrides: []Override{
|
||||||
|
{Path: Keys("nested"), Value: map[string]any{}, Flag: "--nested"},
|
||||||
|
{Path: Keys("nested", "first"), Value: "one", Flag: "--first"},
|
||||||
|
{Path: Keys("nested", "second"), Value: "two", Flag: "--second"},
|
||||||
|
}})
|
||||||
|
if err == nil {
|
||||||
|
t.Fatal("load succeeded with overlapping overrides")
|
||||||
|
}
|
||||||
|
var loadErr *LoadError
|
||||||
|
if !errors.As(err, &loadErr) {
|
||||||
|
t.Fatalf("error = %T %v, want *LoadError", err, err)
|
||||||
|
}
|
||||||
|
diagnostics := loadErr.Diagnostics()
|
||||||
|
var conflicts []Diagnostic
|
||||||
|
for _, diagnostic := range diagnostics {
|
||||||
|
if diagnostic.Code == CodeOverrideOverlap {
|
||||||
|
conflicts = append(conflicts, diagnostic)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if len(conflicts) != 2 {
|
||||||
|
t.Fatalf("overlap diagnostics = %#v, want two conflicts", diagnostics)
|
||||||
|
}
|
||||||
|
for index, path := range []string{"/nested/first", "/nested/second"} {
|
||||||
|
if conflicts[index].Path.String() != path || len(conflicts[index].Related) != 1 || conflicts[index].Related[0].SourceID != "source-2" {
|
||||||
|
t.Fatalf("conflict[%d] = %#v, want path %q related to first CLI source", index, conflicts[index], path)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestOverrideOverlapIndexIncludesAcceptedPathWithInvalidValue(t *testing.T) {
|
||||||
|
type config struct {
|
||||||
|
Nested map[string]any `yaml:"nested"`
|
||||||
|
}
|
||||||
|
base := Bytes("override-value-overlap.yml", []byte("nested: {}\n"))
|
||||||
|
_, err := testLoad(base, Options[config]{Overrides: []Override{
|
||||||
|
{Path: Keys("nested"), Value: time.Second, Flag: "--nested"},
|
||||||
|
{Path: Keys("nested", "child"), Value: "value", Flag: "--child"},
|
||||||
|
}})
|
||||||
|
if err == nil {
|
||||||
|
t.Fatal("load succeeded with an invalid value and overlapping path")
|
||||||
|
}
|
||||||
|
var loadErr *LoadError
|
||||||
|
if !errors.As(err, &loadErr) {
|
||||||
|
t.Fatalf("error = %T %v, want *LoadError", err, err)
|
||||||
|
}
|
||||||
|
var invalidValue, overlap bool
|
||||||
|
for _, diagnostic := range loadErr.Diagnostics() {
|
||||||
|
switch diagnostic.Code {
|
||||||
|
case CodeOverrideInvalidValue:
|
||||||
|
invalidValue = diagnostic.Path.String() == "/nested"
|
||||||
|
case CodeOverrideOverlap:
|
||||||
|
overlap = diagnostic.Path.String() == "/nested/child" && len(diagnostic.Related) == 1 && diagnostic.Related[0].SourceID == "source-2"
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if !invalidValue || !overlap {
|
||||||
|
t.Fatalf("diagnostics = %#v, want invalid parent value and child overlap related to its source", loadErr.Diagnostics())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestRejectedOverridePathDoesNotCauseOverlap(t *testing.T) {
|
||||||
|
type config struct {
|
||||||
|
Scalar string `yaml:"scalar"`
|
||||||
|
}
|
||||||
|
base := Bytes("override-rejected-overlap.yml", []byte("scalar: value\n"))
|
||||||
|
_, err := testLoad(base, Options[config]{Overrides: []Override{
|
||||||
|
{Path: Keys("scalar", "child"), Value: "invalid", Flag: "--child"},
|
||||||
|
{Path: Keys("scalar"), Value: "valid", Flag: "--scalar"},
|
||||||
|
}})
|
||||||
|
if err == nil {
|
||||||
|
t.Fatal("load succeeded with a path traversing a scalar")
|
||||||
|
}
|
||||||
|
var loadErr *LoadError
|
||||||
|
if !errors.As(err, &loadErr) {
|
||||||
|
t.Fatalf("error = %T %v, want *LoadError", err, err)
|
||||||
|
}
|
||||||
|
diagnostics := loadErr.Diagnostics()
|
||||||
|
if len(diagnostics) != 1 || diagnostics[0].Code != CodeOverrideInvalidPath || diagnostics[0].Path.String() != "/scalar/child" {
|
||||||
|
t.Fatalf("diagnostics = %#v, want only the invalid descendant path", diagnostics)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestVersionRejectedOverridePathDoesNotCauseOverlap(t *testing.T) {
|
||||||
|
type config struct {
|
||||||
|
Nested map[string]any `yaml:"nested"`
|
||||||
|
}
|
||||||
|
base := Bytes("override-version-overlap.yml", []byte("nested: {}\n"))
|
||||||
|
version := &VersionRule{Path: Keys("nested", "locked"), Check: func(Version, *Validation) error { return nil }}
|
||||||
|
_, err := testLoad(base, Options[config]{
|
||||||
|
Definition: Definition[config]{Version: version},
|
||||||
|
Overrides: []Override{
|
||||||
|
{Path: Keys("nested"), Value: map[string]any{}, Flag: "--nested"},
|
||||||
|
{Path: Keys("nested", "other"), Value: "allowed", Flag: "--other"},
|
||||||
|
},
|
||||||
|
})
|
||||||
|
if err == nil {
|
||||||
|
t.Fatal("load succeeded despite an override of a schema-version ancestor")
|
||||||
|
}
|
||||||
|
var loadErr *LoadError
|
||||||
|
if !errors.As(err, &loadErr) {
|
||||||
|
t.Fatalf("error = %T %v, want *LoadError", err, err)
|
||||||
|
}
|
||||||
|
diagnostics := loadErr.Diagnostics()
|
||||||
|
if len(diagnostics) != 1 || diagnostics[0].Code != CodeOverrideVersion || diagnostics[0].Path.String() != "/nested" {
|
||||||
|
t.Fatalf("diagnostics = %#v, want only the protected ancestor path", diagnostics)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestEscapedOverrideKeysStayDistinctFromMultipleSegments(t *testing.T) {
|
||||||
|
type config struct {
|
||||||
|
Nested map[string]any `yaml:"nested"`
|
||||||
|
}
|
||||||
|
base := Bytes("override-escaped-keys.yml", []byte("nested: {}\n"))
|
||||||
|
_, err := testLoad(base, Options[config]{Overrides: []Override{
|
||||||
|
{Path: Keys("nested", "a/b"), Value: "single key", Flag: "--single"},
|
||||||
|
{Path: Keys("nested", "a", "b"), Value: "nested key", Flag: "--nested"},
|
||||||
|
}})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("load rejected distinct escaped path tokens: %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestOverrideCannotReplaceSchemaVersionAncestor(t *testing.T) {
|
func TestOverrideCannotReplaceSchemaVersionAncestor(t *testing.T) {
|
||||||
type schemaConfig struct {
|
type schemaConfig struct {
|
||||||
Version string `yaml:"version"`
|
Version string `yaml:"version"`
|
||||||
|
|||||||
Reference in New Issue
Block a user