From a2fab66abadf66ec84f29ff0e59a3a927f25dcc7 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Wed, 14 Oct 2026 13:19:31 +0000 Subject: [PATCH] Carry destinations through imported configuration --- docs/roadmap/implementation.md | 9 +- internal/engine/destination_import_test.go | 208 +++++++++++++++++++++ internal/engine/imports.go | 48 +++-- internal/engine/load_input.go | 3 +- 4 files changed, 245 insertions(+), 23 deletions(-) create mode 100644 internal/engine/destination_import_test.go diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 42c206c..45da0be 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -228,7 +228,7 @@ private integration step. ## Stage 4 — Carry placement through import acquisition -Status: planned. Scope: source-local directives and placed imported applications. +Status: complete. Scope: source-local directives and placed imported applications. 1. Thread a normalized destination through `loadImportedDocumentsTracked`, `loadImportedWithAccessTracked`, and `importTraversal`, retaining root defaults @@ -246,9 +246,10 @@ Status: planned. Scope: source-local directives and placed imported applications paths. Verify a repeated file at two destinations in one group still fails, including an import/direct-input alias. Reuse cycle/containment regressions. -Validate: focused destination/import/glob/grouped tests and everyday check. -Done: every retained imported contribution has effective placement and physical -source identity, with no double wrapping or application-level directive leakage. +Validated with `go test ./internal/engine -run 'Destination|PlaceDocument|Import|Grouped'` +and `./scripts/check.sh` (go1.26.8 linux/amd64); both passed. Retained imported +contributions have effective placement and physical source identity, with no +double wrapping or application-level directive leakage. ## Stage 5 — Complete strict tiers and main version ownership diff --git a/internal/engine/destination_import_test.go b/internal/engine/destination_import_test.go new file mode 100644 index 0000000..ab8ab99 --- /dev/null +++ b/internal/engine/destination_import_test.go @@ -0,0 +1,208 @@ +package engine + +import ( + "path/filepath" + "testing" + "testing/fstest" +) + +type importDestinationConfig struct { + Mounted map[string]string `yaml:"mounted"` + Collections map[string]map[string]string `yaml:"collections"` +} + +func TestLayeredHostImportsInheritInputDestinationTransitively(t *testing.T) { + t.Parallel() + + directory := t.TempDir() + mainPath := writeImportFile(t, directory, "main.yml", `configkit: + imports: + - path: first.yml + - path: second.yml +root: main +`) + firstPath := writeImportFile(t, directory, "first.yml", `configkit: + imports: + - path: nested/third.yml +first: one +`) + thirdPath := writeImportFile(t, directory, "nested/third.yml", "nested: three\n") + secondPath := writeImportFile(t, directory, "second.yml", "second: two\n") + + loaded, err := LoadLayered( + Input{Source: File(mainPath), Imports: true, Destination: Keys("mounted")}, + testSecretBinding(), + LayeredOptions[importDestinationConfig]{}, + ) + if err != nil { + if loadError, ok := err.(*LoadError); ok { + t.Fatalf("LoadLayered diagnostics: %#v", loadError.Diagnostics()) + } + t.Fatalf("LoadLayered: %v", err) + } + result := testLoaded(loaded) + want := map[string]string{"root": "main", "first": "one", "nested": "three", "second": "two"} + if len(result.Config.Mounted) != len(want) { + t.Fatalf("mounted values = %#v", result.Config.Mounted) + } + for key, value := range want { + if result.Config.Mounted[key] != value { + t.Errorf("mounted[%q] = %q, want %q", key, result.Config.Mounted[key], value) + } + } + if _, ok := result.Metadata().Lookup(Keys("mounted", "configkit")); ok { + t.Fatal("source-root import directive leaked into application content") + } + sources := result.Sources() + wantPaths := []string{canonicalTestPath(t, mainPath), canonicalTestPath(t, firstPath), canonicalTestPath(t, thirdPath), canonicalTestPath(t, secondPath)} + if len(sources) != len(wantPaths) { + t.Fatalf("sources = %#v", sources) + } + for index, source := range sources { + if source.Destination.String() != "/mounted" { + t.Errorf("source %d destination = %q", index, source.Destination.String()) + } + if source.Path != wantPaths[index] { + t.Errorf("source %d path = %q, want %q", index, source.Path, wantPaths[index]) + } + if index > 0 && source.ImportedFrom == nil { + t.Errorf("imported source %d has no physical import location", index) + } + } +} + +func TestLayeredNativeFilesystemImportsInheritInputDestination(t *testing.T) { + t.Parallel() + + native := fstest.MapFS{ + "main.yml": &fstest.MapFile{Data: []byte("configkit:\n imports:\n - path: first.yml\nroot: main\n")}, + "first.yml": &fstest.MapFile{Data: []byte("configkit: {imports: [{path: nested.yml}]}\nfirst: imported\n")}, + "nested.yml": &fstest.MapFile{Data: []byte("nested: imported\n")}, + } + namespace := newNativeNamespace("bundle", native) + loaded, err := LoadLayered( + Input{Source: filesystemFile(namespace, "main.yml"), Imports: true, Destination: Keys("mounted")}, + testSecretBinding(), + LayeredOptions[importDestinationConfig]{}, + ) + if err != nil { + if loadError, ok := err.(*LoadError); ok { + t.Fatalf("LoadLayered diagnostics: %#v", loadError.Diagnostics()) + } + t.Fatalf("LoadLayered: %v", err) + } + result := testLoaded(loaded) + if result.Config.Mounted["root"] != "main" || result.Config.Mounted["first"] != "imported" || result.Config.Mounted["nested"] != "imported" { + t.Fatalf("mounted native filesystem values = %#v", result.Config.Mounted) + } + sources := result.Sources() + if len(sources) != 3 || sources[0].Destination.String() != "/mounted" || sources[1].Destination.String() != "/mounted" || sources[2].Destination.String() != "/mounted" { + t.Fatalf("native source destinations = %#v", sources) + } +} + +func TestLayeredImportOnlyInputMaterializesDestination(t *testing.T) { + t.Parallel() + + directory := t.TempDir() + mainPath := writeImportFile(t, directory, "main.yml", `configkit: + imports: + - path: optional.yml + optional: true +`) + loaded, err := LoadLayered( + Input{Source: File(mainPath), Imports: true, Destination: Keys("collections", "empty")}, + testSecretBinding(), + LayeredOptions[importDestinationConfig]{}, + ) + if err != nil { + t.Fatalf("LoadLayered: %v", err) + } + result := testLoaded(loaded) + info, ok := result.Metadata().Lookup(Keys("collections", "empty")) + if !ok || !info.Explicit { + t.Fatalf("import-only body destination = %#v, %v", info, ok) + } + sources := result.Sources() + if len(sources) != 1 || sources[0].Destination.String() != "/collections/empty" { + t.Fatalf("sources = %#v", sources) + } +} + +func TestLayeredImportedVersionClaimsUseEffectiveDestination(t *testing.T) { + t.Parallel() + + for _, test := range []struct { + name string + rulePath Path + destination Path + wantCode Code + }{ + {name: "protected effective path", rulePath: Keys("mounted", "version"), destination: Keys("mounted"), wantCode: CodeImportVersion}, + {name: "ordinary nested domain version", rulePath: Keys("version"), destination: Keys("collections", "entry")}, + } { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + directory := t.TempDir() + writeImportFile(t, directory, "main.yml", "configkit: {imports: [{path: fragment.yml}]}\n") + writeImportFile(t, directory, "fragment.yml", "version: v2\n") + rule := &VersionRule{Path: test.rulePath, Check: func(Version, *Validation) error { return nil }} + loaded, err := LoadLayered( + Input{Source: NoSource()}, + testSecretBinding(), + LayeredOptions[importDestinationConfig]{ + Definition: Definition[importDestinationConfig]{Version: rule}, + Underlays: []Underlay{{Inputs: []Input{{ + Source: File(filepath.Join(directory, "main.yml")), Imports: true, Destination: test.destination, + }}}}, + }, + ) + if test.wantCode != "" { + if err == nil { + t.Fatal("protected imported version claim was accepted") + } + loadError, ok := err.(*LoadError) + if !ok || len(loadError.Diagnostics()) == 0 || loadError.Diagnostics()[0].Code != test.wantCode { + t.Fatalf("load error = %#v", err) + } + if test.name == "protected effective path" { + sources := loadError.Sources() + if len(sources) != 2 || sources[0].Destination.String() != "/mounted" || sources[1].Destination.String() != "/mounted" { + t.Fatalf("error source destinations = %#v", sources) + } + } + return + } + if err != nil { + t.Fatalf("ordinary nested version was rejected: %v", err) + } + result := testLoaded(loaded) + if got := result.Config.Collections["entry"]["version"]; got != "v2" { + t.Fatalf("nested version = %q", got) + } + }) + } +} + +func TestLayeredDestinationsDoNotChangeRepeatedSourceIdentity(t *testing.T) { + t.Parallel() + + directory := t.TempDir() + writeImportFile(t, directory, "shared.yml", "value: shared\n") + importerPath := writeImportFile(t, directory, "importer.yml", "configkit: {imports: [{path: shared.yml}]}\n") + _, err := LoadLayered( + Input{Source: NoSource()}, + testSecretBinding(), + LayeredOptions[importDestinationConfig]{Underlays: []Underlay{{Inputs: []Input{ + {Source: File(importerPath), Imports: true, Destination: Keys("imported")}, + {Source: File(filepath.Join(directory, "shared.yml")), Destination: Keys("direct")}, + }}}}, + ) + if err == nil { + t.Fatal("the same physical source was accepted at two destinations in one group") + } + loadError, ok := err.(*LoadError) + if !ok || len(loadError.Diagnostics()) == 0 || loadError.Diagnostics()[0].Code != CodeImportRepeated { + t.Fatalf("load error = %#v", err) + } +} diff --git a/internal/engine/imports.go b/internal/engine/imports.go index bb56e62..0b9b7ae 100644 --- a/internal/engine/imports.go +++ b/internal/engine/imports.go @@ -45,10 +45,11 @@ type importAccounting struct { } type importTraversal struct { - access *filesystemAccess - accounting *importAccounting - limits importLimits - rule *VersionRule + access *filesystemAccess + accounting *importAccounting + limits importLimits + rule *VersionRule + destination Path documents []*document.Node sources []SourceInfo @@ -58,16 +59,16 @@ type importTraversal struct { } func loadImportedDocuments(source Source, context *FilesystemContext, main parsedSource, rule *VersionRule, limits importLimits) (*composedDocument, []SourceInfo, []Diagnostic) { - return loadImportedDocumentsTracked(source, context, main, rule, limits, &importAccounting{}, nil, nil) + return loadImportedDocumentsTracked(source, context, main, Path{}, rule, limits, &importAccounting{}, nil, nil) } -func loadImportedDocumentsTracked(source Source, context *FilesystemContext, main parsedSource, rule *VersionRule, limits importLimits, accounting *importAccounting, visited map[resourceIdentity]importRoute, hostToken *namespaceToken) (*composedDocument, []SourceInfo, []Diagnostic) { +func loadImportedDocumentsTracked(source Source, context *FilesystemContext, main parsedSource, destination Path, rule *VersionRule, limits importLimits, accounting *importAccounting, visited map[resourceIdentity]importRoute, hostToken *namespaceToken) (*composedDocument, []SourceInfo, []Diagnostic) { if nativeContext(source, context) { access, basePath, mainPath, diagnostic := nativeFilesystemAccess(source, context) if diagnostic != nil { return nil, sourceInfos(main.source), []Diagnostic{*diagnostic} } - return loadImportedWithAccessTracked(access, basePath, mainPath, main, rule, limits, accounting, visited) + return loadImportedWithAccessTracked(access, basePath, mainPath, main, destination, rule, limits, accounting, visited) } rootPath, basePath, mainPath, diagnostic := importFilesystem(source, context, main) if diagnostic != nil { @@ -82,14 +83,14 @@ func loadImportedDocumentsTracked(source Source, context *FilesystemContext, mai if hostToken != nil { access.token = hostToken } - return loadImportedWithAccessTracked(access, basePath, mainPath, main, rule, limits, accounting, visited) + return loadImportedWithAccessTracked(access, basePath, mainPath, main, destination, rule, limits, accounting, visited) } func loadImportedWithAccess(access *filesystemAccess, basePath, mainPath string, main parsedSource, rule *VersionRule, limits importLimits, accounting *importAccounting) (*composedDocument, []SourceInfo, []Diagnostic) { - return loadImportedWithAccessTracked(access, basePath, mainPath, main, rule, limits, accounting, nil) + return loadImportedWithAccessTracked(access, basePath, mainPath, main, Path{}, rule, limits, accounting, nil) } -func loadImportedWithAccessTracked(access *filesystemAccess, basePath, mainPath string, main parsedSource, rule *VersionRule, limits importLimits, accounting *importAccounting, visited map[resourceIdentity]importRoute) (*composedDocument, []SourceInfo, []Diagnostic) { +func loadImportedWithAccessTracked(access *filesystemAccess, basePath, mainPath string, main parsedSource, destination Path, rule *VersionRule, limits importLimits, accounting *importAccounting, visited map[resourceIdentity]importRoute) (*composedDocument, []SourceInfo, []Diagnostic) { if !access.contains(basePath) || (mainPath != "" && !access.contains(mainPath)) { _ = access.close() return nil, sourceInfos(main.source), []Diagnostic{importDiagnostic(CodeImportOutsideRoot, "selected source and import base must stay inside the configuration root", nil)} @@ -102,6 +103,7 @@ func loadImportedWithAccessTracked(access *filesystemAccess, basePath, mainPath } } mainInfo := cloneSourceInfo(*main.source) + mainInfo.Destination = destination.clone() if mainPath != "" { mainInfo.Path = mainPath } @@ -118,9 +120,10 @@ func loadImportedWithAccessTracked(access *filesystemAccess, basePath, mainPath accounting.nodes += countInputNodes(main.document) state := importTraversal{ access: access, accounting: accounting, limits: limits, rule: rule, - documents: make([]*document.Node, 0, 1), - sources: []SourceInfo{mainInfo}, - active: make(map[resourceIdentity]int), visited: visited, + destination: destination.clone(), + documents: make([]*document.Node, 0, 1), + sources: []SourceInfo{mainInfo}, + active: make(map[resourceIdentity]int), visited: visited, } if state.visited == nil { state.visited = make(map[resourceIdentity]importRoute) @@ -140,8 +143,13 @@ func loadImportedWithAccessTracked(access *filesystemAccess, basePath, mainPath state.stack = append(state.stack, mainRoute) body, declarations, diagnostics := splitImportDirectives(main.document) if len(diagnostics) == 0 { - state.documents = append(state.documents, body) - diagnostics = state.walkDeclarations(basePath, declarations, 0) + _, placed, err := placeDocument(body, state.destination, body.Location, maxDocumentDepth, maxDocumentNodes) + if err != nil { + diagnostics = []Diagnostic{sourceDiagnostic(CodeSourceLimitExceeded, main.source.ID, "configuration exceeds the source depth or node limit after destination placement")} + } else { + state.documents = append(state.documents, placed) + diagnostics = state.walkDeclarations(basePath, declarations, 0) + } } closeErr := access.close() @@ -260,7 +268,7 @@ func (s *importTraversal) visitPath(basePath string, declaration importDeclarati kind = SourceKindFilesystem label = s.access.label + "/" + canonical } - info := SourceInfo{ID: sourceID, Label: label, Path: canonical, Layer: LayerBase, Kind: kind, ImportedFrom: &location} + info := SourceInfo{ID: sourceID, Label: label, Path: canonical, Destination: s.destination.clone(), Layer: LayerBase, Kind: kind, ImportedFrom: &location} if kind == SourceKindFilesystem { info.namespace = s.access.token } @@ -291,7 +299,11 @@ func (s *importTraversal) visitPath(basePath string, declaration importDeclarati if len(diagnostics) != 0 { return diagnostics } - if versionNode := fragmentVersionClaim(body, s.rule); versionNode != nil { + _, placed, err := placeDocument(body, s.destination, body.Location, maxDocumentDepth, maxDocumentNodes) + if err != nil { + return []Diagnostic{importDiagnostic(CodeImportLimitExceeded, "configuration exceeds the source depth or node limit after destination placement", &location)} + } + if versionNode := fragmentVersionClaim(placed, s.rule); versionNode != nil { versionLocation := publicLocation(versionNode.Location) return []Diagnostic{{ Code: CodeImportVersion, Severity: SeverityError, Path: s.rule.Path.clone(), @@ -300,7 +312,7 @@ func (s *importTraversal) visitPath(basePath string, declaration importDeclarati } route := importRoute{canonical: canonical, locations: currentRoute.locations} - s.documents = append(s.documents, body) + s.documents = append(s.documents, placed) s.active[identity] = len(s.stack) s.visited[identity] = route s.stack = append(s.stack, route) diff --git a/internal/engine/load_input.go b/internal/engine/load_input.go index eee0b0d..93a0d31 100644 --- a/internal/engine/load_input.go +++ b/internal/engine/load_input.go @@ -32,7 +32,8 @@ func assembleBaseInputTracked[T any](source Source, options Options[T], destinat return input, diagnostics } if options.Imports { - input.composition, input.sources, diagnostics = loadImportedDocumentsTracked(source, options.Filesystem, parsed, options.Definition.Version, limits, accounting, visited, hostToken) + input.composition, input.sources, diagnostics = loadImportedDocumentsTracked(source, options.Filesystem, parsed, destination, options.Definition.Version, limits, accounting, visited, hostToken) + setSourceDestinations(input.sources, destination) if len(diagnostics) != 0 { return input, diagnostics }