diff --git a/pkg/env/select.go b/pkg/env/select.go index 77893d3..86d302f 100644 --- a/pkg/env/select.go +++ b/pkg/env/select.go @@ -148,6 +148,16 @@ func mergeYAMLTopLevel(a, b []byte) ([]byte, error) { bKey := bRoot.Content[i] bVal := bRoot.Content[i+1] if idx := findYAMLTopLevelKey(aRoot, bKey.Value); idx >= 0 { + // Both sides selected the same top-level key — a simple dot-path + // on one side and a JMESPath on the other. Merge them instead of + // letting the JMESPath side win: replacing dropped everything the + // simple selector asked for, the same collapse that lost base + // profiles' binaries in composed profiles. Recursing also keeps + // a's comments on the keys b does not touch. + if aRoot.Content[idx+1].Kind == yaml.MappingNode && bVal.Kind == yaml.MappingNode { + mergeYAMLMappings(aRoot.Content[idx+1], bVal) + continue + } aRoot.Content[idx] = bKey aRoot.Content[idx+1] = bVal continue @@ -443,6 +453,10 @@ func mergeYAMLMappings(dst, src *yaml.Node) { found = true if dst.Content[j+1].Kind == yaml.MappingNode && srcVal.Kind == yaml.MappingNode { mergeYAMLMappings(dst.Content[j+1], srcVal) + } else { + // Not both mappings — there is nothing to compose, so the + // documented precedence applies: the JMESPath result wins. + dst.Content[j+1] = srcVal } break } diff --git a/pkg/env/select_jmespath.go b/pkg/env/select_jmespath.go index c4dc9a0..b688a5b 100644 --- a/pkg/env/select_jmespath.go +++ b/pkg/env/select_jmespath.go @@ -3,6 +3,7 @@ package env import ( "encoding/json" "fmt" + "reflect" "strings" "github.com/jmespath-community/go-jmespath" @@ -109,11 +110,15 @@ func splitSelectorsByComplexity(selectors []string) (simple, complex []string) { // Merge semantics: // // - If an expression returns a map[string]interface{}, its entries are -// merged into the result (later expressions override earlier ones for -// the same key). +// merged into the result. Several expressions contributing to the SAME key +// union rather than overwrite: nested maps merge key-by-key, lists union +// (deduplicated), and only scalars replace. This is what lets a resolved +// profile chain select several slices of one file. // // - If an expression returns something else (scalar, array), it is -// wrapped under a key chosen by `wrapKeyFor`. The key is selected +// wrapped under a key chosen by `wrapKeyFor` — and unioned the same way, +// since wrapKeyFor deliberately sends every `binaries[?…]` variant to +// `binaries`. The key is selected // by a small fallback chain: a leading identifier followed by // JMESPath grammar (e.g. `binaries[?...]` → `binaries`), the // trailing identifier of a simple dot-path (`database.host` → @@ -149,17 +154,85 @@ func runJMESPathSelectors( continue } if m, ok := val.(map[string]interface{}); ok { - for k, v := range m { - merged[k] = v - } + mergeMapInto(merged, m) continue } - // Non-map result: wrap under a sensible key. - merged[wrapKeyFor(sel)] = val + // Non-map result: wrap under a sensible key — through the same + // union, since wrapKeyFor deliberately maps every `binaries[?…]` + // variant to `binaries`, so a profile chain of bare filters lands + // several lists on one key. + mergeValueInto(merged, wrapKeyFor(sel), val) } return marshal(merged) } +// mergeMapInto deep-merges src into dst, key by key. +func mergeMapInto(dst, src map[string]interface{}) { + for k, v := range src { + mergeValueInto(dst, k, v) + } +} + +// mergeValueInto unions v into dst[k]. Nested maps merge key-by-key, lists +// union, and anything else is replaced. Several selectors contributing to the +// SAME key is the normal shape once a profile chain is resolved — each +// projects a different slice of one file — and a plain assignment kept only +// the last one, which is how core binaries went missing from every composed +// profile. +func mergeValueInto(dst map[string]interface{}, k string, v interface{}) { + existing, present := dst[k] + if !present { + dst[k] = v + return + } + switch src := v.(type) { + case []interface{}: + if cur, ok := existing.([]interface{}); ok { + dst[k] = unionLists(cur, src) + return + } + case map[string]interface{}: + if cur, ok := existing.(map[string]interface{}); ok { + // Copy before descending: cur may alias the parsed document, and + // mutating it in place would corrupt what later selectors search. + clone := make(map[string]interface{}, len(cur)+len(src)) + for ck, cv := range cur { + clone[ck] = cv + } + mergeMapInto(clone, src) + dst[k] = clone + return + } + } + dst[k] = v +} + +// unionLists appends the elements of b that a does not already hold. +// Deduplicating matters: an entry matching two selectors is projected twice +// with the same value, and plain concatenation turned `groups: [local, core]` +// into `[local, core, local, core]`. The result is always a fresh slice — +// appending in place could write into the parsed document's backing array. +func unionLists(a, b []interface{}) []interface{} { + out := make([]interface{}, 0, len(a)+len(b)) + out = append(out, a...) + for _, item := range b { + if !containsValue(out, item) { + out = append(out, item) + } + } + return out +} + +// containsValue reports whether list already holds an element deep-equal to v. +func containsValue(list []interface{}, v interface{}) bool { + for _, item := range list { + if reflect.DeepEqual(item, v) { + return true + } + } + return false +} + // wrapKeyFor picks a top-level key under which to place a non-map // JMESPath result. The fallback chain is: // diff --git a/pkg/env/select_jmespath_test.go b/pkg/env/select_jmespath_test.go index bd7ee54..d7f1880 100644 --- a/pkg/env/select_jmespath_test.go +++ b/pkg/env/select_jmespath_test.go @@ -1,8 +1,11 @@ package env import ( + "sort" "strings" "testing" + + "gopkg.in/yaml.v3" ) // --- classifier --- @@ -419,3 +422,131 @@ func TestWrapKeyFor(t *testing.T) { } } } + +func TestFilterYAMLJMESPath_SameKeyFromTwoSelectorsUnions(t *testing.T) { + // A resolved profile chain hands us one selector per group, each projecting + // the same top-level `binaries` key. A shallow merge kept only the last + // group's map, which is how core tools went missing from every composed + // profile. + content := []byte(`binaries: + argsh: + groups: [core] + kubectl: + groups: [core] + kustomize: + groups: [kustomize] + kind: + groups: [local] +`) + selectors := []string{ + "{binaries: from_items(items(binaries)[?[1].groups && contains([1].groups, 'core')])}", + "{binaries: from_items(items(binaries)[?[1].groups && contains([1].groups, 'kustomize')])}", + } + + out, err := filterYAMLJMESPath(content, selectors) + if err != nil { + t.Fatalf("filter: %v", err) + } + + var got struct { + Binaries map[string]interface{} `yaml:"binaries"` + } + if err := yaml.Unmarshal(out, &got); err != nil { + t.Fatalf("unmarshal %s: %v", out, err) + } + for _, want := range []string{"argsh", "kubectl", "kustomize"} { + if _, ok := got.Binaries[want]; !ok { + t.Errorf("binaries.%s missing — got %v (both selectors' results must survive)", want, keysOf(got.Binaries)) + } + } + if _, ok := got.Binaries["kind"]; ok { + t.Error("binaries.kind leaked in — no selector asked for the local group") + } +} + +func keysOf(m map[string]interface{}) []string { + out := make([]string, 0, len(m)) + for k := range m { + out = append(out, k) + } + sort.Strings(out) + return out +} + +// The seam that matters is filterContent: it routes selectors to the simple +// Node path, the JMESPath path, or both, and then merges. Testing the +// individual filters misses collapses that only the merge introduces. +func TestFilterContent_SimpleAndComplexSelectorsBothSurvive(t *testing.T) { + content := []byte(`binaries: + argsh: + groups: [core] + kustomize: + groups: [kustomize] + kind: + groups: [local] +`) + selectors := []string{ + "binaries.argsh", + "{binaries: from_items(items(binaries)[?[1].groups && contains([1].groups, 'kustomize')])}", + } + + out, err := filterContent(content, selectors, "b.yaml") + if err != nil { + t.Fatalf("filterContent: %v", err) + } + + var got struct { + Binaries map[string]interface{} `yaml:"binaries"` + } + if err := yaml.Unmarshal(out, &got); err != nil { + t.Fatalf("unmarshal %s: %v", out, err) + } + for _, want := range []string{"argsh", "kustomize"} { + if _, ok := got.Binaries[want]; !ok { + t.Errorf("binaries.%s missing — got %v (the JMESPath side must not replace the simple side's key)", want, keysOf(got.Binaries)) + } + } + if _, ok := got.Binaries["kind"]; ok { + t.Error("binaries.kind leaked in — no selector asked for the local group") + } +} + +func TestFilterContent_BareFilterSelectorsUnion(t *testing.T) { + // Bare filters return LISTS, and wrapKeyFor sends every `binaries[?…]` + // variant to the same `binaries` key — so a chain of them used to keep + // only the last group. + content := []byte(`binaries: + - name: argsh + groups: [core] + - name: kustomize + groups: [kustomize] + - name: kind + groups: [local] +`) + selectors := []string{ + "binaries[?groups && contains(groups, 'core')]", + "binaries[?groups && contains(groups, 'kustomize')]", + } + + out, err := filterContent(content, selectors, "b.yaml") + if err != nil { + t.Fatalf("filterContent: %v", err) + } + + var got struct { + Binaries []struct { + Name string `yaml:"name"` + } `yaml:"binaries"` + } + if err := yaml.Unmarshal(out, &got); err != nil { + t.Fatalf("unmarshal %s: %v", out, err) + } + var names []string + for _, b := range got.Binaries { + names = append(names, b.Name) + } + sort.Strings(names) + if len(names) != 2 || names[0] != "argsh" || names[1] != "kustomize" { + t.Errorf("binaries = %v, want both filters' results [argsh kustomize]", names) + } +} diff --git a/pkg/env/splice.go b/pkg/env/splice.go index 9876c57..803bd14 100644 --- a/pkg/env/splice.go +++ b/pkg/env/splice.go @@ -70,23 +70,137 @@ func spliceSelectedScope(local, merged []byte, selectors []string, filePath stri } } -// topLevelKeysFromSelectors returns the set of top-level YAML keys that are +// topLevelKeysFromSelectors returns the set of top-level YAML keys named by the +// selectors. Only SIMPLE dot-paths contribute — see spliceYAML for why a complex +// expression's key cannot be trusted. Returns the set of top-level YAML keys that are // within the selector scope. A selector like "binaries" or ".binaries" maps // to {binaries}; a nested selector like "database.host" or ".database.host" // maps to {database}. func topLevelKeysFromSelectors(selectors []string) map[string]bool { keys := make(map[string]bool, len(selectors)) for _, sel := range selectors { - key := strings.TrimPrefix(sel, ".") + for _, key := range selectorTopLevelKeys(sel) { + keys[key] = true + } + } + return keys +} + +// topLevelKeysFromMerged returns the top-level keys of a merged YAML document, +// or nil when it does not parse as a mapping (the conflict-marker case). +func topLevelKeysFromMerged(merged []byte) map[string]bool { + var doc yaml.Node + if err := yaml.Unmarshal(merged, &doc); err != nil { + return nil + } + if len(doc.Content) == 0 || doc.Content[0].Kind != yaml.MappingNode { + return nil + } + root := doc.Content[0] + keys := make(map[string]bool, len(root.Content)/2) + for i := 0; i+1 < len(root.Content); i += 2 { + keys[root.Content[i].Value] = true + } + return keys +} + +// selectorTopLevelKeys returns the top-level keys a selector contributes to +// the merged document. It MUST agree with what the JMESPath layer actually +// emits (runJMESPathSelectors / wrapKeyFor) — a scope key that names nothing +// in `merged` makes the splice a silent no-op, which is how a complex +// selector like +// +// {binaries: from_items(items(binaries)[?[1].groups && …])} +// +// used to leave the consumer's file untouched: treated as a literal dot-path +// it yielded the nonsense key `{binaries: from_items(items(binaries)[?[1]`, +// nothing matched, and every binary the profile selected went missing. +func selectorTopLevelKeys(sel string) []string { + if keys, ok := multiSelectHashKeys(sel); ok { + return keys + } + if !isSimpleDotPath(sel) { + // Any other complex expression: the merge wraps its result under + // wrapKeyFor, so that is exactly the key in scope. + return []string{wrapKeyFor(sel)} + } + key := strings.TrimPrefix(sel, ".") + if key == "" { + return nil + } + if i := strings.Index(key, "."); i >= 0 { + key = key[:i] + } + return []string{key} +} + +// multiSelectHashKeys extracts the keys of a JMESPath multi-select hash — +// `{a: expr, b: expr}` → [a b]. Those keys ARE the top-level keys of the +// result. Reports false for anything that is not a well-formed hash so the +// caller can fall back. +func multiSelectHashKeys(sel string) ([]string, bool) { + body := strings.TrimSpace(sel) + if len(body) < 2 || body[0] != '{' || body[len(body)-1] != '}' { + return nil, false + } + parts := splitTopLevel(body[1:len(body)-1], ',') + if parts == nil { + return nil, false + } + var keys []string + for _, part := range parts { + // The key is everything up to the FIRST top-level colon; the value + // expression may contain colons of its own. + pair := splitTopLevel(part, ':') + if len(pair) < 2 { + return nil, false + } + key := strings.Trim(strings.TrimSpace(pair[0]), `"'`) if key == "" { - continue + return nil, false } - if i := strings.Index(key, "."); i >= 0 { - key = key[:i] + keys = append(keys, key) + } + if len(keys) == 0 { + return nil, false + } + return keys, true +} + +// splitTopLevel splits on sep, ignoring separators nested inside brackets, +// braces, parens or quotes. +func splitTopLevel(s string, sep byte) []string { + var ( + out []string + depth int + quote byte + start int + ) + for i := 0; i < len(s); i++ { + c := s[i] + switch { + case quote != 0: + if c == quote { + quote = 0 + } + case c == '\'' || c == '"' || c == '`': + quote = c + case c == '{' || c == '[' || c == '(': + depth++ + case c == '}' || c == ']' || c == ')': + depth-- + if depth < 0 { + // Unbalanced — e.g. `{a: b} | {c: d}`, where letting depth go + // negative would rebalance and hide the second brace group. + // Bail out so the caller falls back instead of splitting wrong. + return nil + } + case c == sep && depth == 0: + out = append(out, s[start:i]) + start = i + 1 } - keys[key] = true } - return keys + return append(out, s[start:]) } // usesCRLF reports whether the file should be treated as having @@ -159,7 +273,34 @@ func containsConflictMarkers(b []byte) bool { // preferred whitespace and quoting style — even for keys the splice // didn't touch. func spliceYAML(local, merged []byte, selectors []string) ([]byte, error) { - scope := topLevelKeysFromSelectors(selectors) + // Scope = the keys the merge actually emitted, plus the keys SIMPLE + // selectors name. See the branches below for why the distinction matters. + // Keys the merge actually emitted, when it parses at all. + scope := topLevelKeysFromMerged(merged) + if scope == nil { + // `merged` carries git conflict markers, so there is nothing to read + // keys from. Selector names are all we have. + scope = make(map[string]bool, len(selectors)) + } + // Plus the keys of SIMPLE selectors. For a plain dot-path the name is + // authoritative — the consumer's key is in scope whether or not the merge + // emitted it, so an upstream removal propagates (see + // TestSpliceYAMLStructural_RemovesScopedKeyAbsentInMerge). + // + // Complex selectors contribute nothing here, on BOTH paths. Their landing + // key is not derivable from the expression: anything returning a map goes + // through the expression's own structure, and the wrapKeyFor fallback merely + // guesses "result". Trusting that guess deletes whatever the consumer keeps + // under that name — including on the conflict path, where it also swallowed + // the markers it was supposed to be writing. + for _, sel := range selectors { + if !isSimpleDotPath(sel) { + continue + } + for _, k := range selectorTopLevelKeys(sel) { + scope[k] = true + } + } // Path 1: byte-level splice. Requires valid YAML on both sides // (no conflict markers in `merged`) and a parseable diff --git a/pkg/env/splice_json.go b/pkg/env/splice_json.go index 4947cba..b34275f 100644 --- a/pkg/env/splice_json.go +++ b/pkg/env/splice_json.go @@ -24,12 +24,11 @@ import ( // resolve manually. This is a deliberate scope limit, not a data-loss // risk: the caller never writes the partial result. func spliceJSON(local, merged []byte, selectors []string) ([]byte, error) { - // Reject complex JMESPath selectors. topLevelKeysFromSelectors - // is a literal-string operation, so an expression like - // `from_items(items(binaries))` would be treated as a key - // literally named "from_items(items(binaries))" and the splice - // would silently look for it (and skip the file). The JSON - // splice only supports simple dot-paths today; the caller + // Reject complex JMESPath selectors. The YAML splice handles them by + // reading the merged document's own keys; the JSON splice has no such + // step, and selectorTopLevelKeys deliberately contributes nothing for a + // complex expression, so the scope would come out empty and the file + // would be skipped in silence. Simple dot-paths only here — the caller // should drop the select or move the data to YAML. for _, s := range selectors { if !isSimpleDotPath(s) { diff --git a/pkg/env/splice_test.go b/pkg/env/splice_test.go index 0f06243..2578818 100644 --- a/pkg/env/splice_test.go +++ b/pkg/env/splice_test.go @@ -649,3 +649,120 @@ func TestSpliceSelectedScope_NoSelectors(t *testing.T) { t.Errorf("no-selectors splice should equal merged, got %q", out) } } + +// The splice is where a wrong scope key becomes invisible: it silently writes +// nothing. These tests sit at spliceSelectedScope, the seam the sync actually +// calls, with the selector shapes real profiles use. + +func TestSpliceSelectedScope_MapReturningComplexSelector(t *testing.T) { + // A complex selector whose result is a MAP does not go through wrapKeyFor, + // so its scope key cannot be predicted from the expression text — it has to + // come from the merged document. + local := []byte("binaries: {}\nprofiles:\n keep: {}\n") + merged := []byte("binaries:\n argsh: {}\n") + sel := []string{"from_items(items(binaries)[?[1].groups])"} + + out, err := spliceSelectedScope(local, merged, sel, "b.yaml") + if err != nil { + t.Fatalf("splice: %v", err) + } + if !strings.Contains(string(out), "argsh") { + t.Errorf("merged content was not spliced in — the splice was a silent no-op:\n%s", out) + } + if !strings.Contains(string(out), "profiles") { + t.Errorf("out-of-scope profiles was dropped:\n%s", out) + } +} + +func TestSpliceSelectedScope_MultiSelectHashSelector(t *testing.T) { + local := []byte("binaries: {}\nprofiles:\n keep: {}\n") + merged := []byte("binaries:\n argsh: {}\n") + sel := []string{"{binaries: from_items(items(binaries)[?[1].groups && contains([1].groups, 'core')])}"} + + out, err := spliceSelectedScope(local, merged, sel, "b.yaml") + if err != nil { + t.Fatalf("splice: %v", err) + } + if !strings.Contains(string(out), "argsh") { + t.Errorf("hash-wrapped selector spliced nothing:\n%s", out) + } + if !strings.Contains(string(out), "profiles") { + t.Errorf("out-of-scope profiles was dropped:\n%s", out) + } +} + +func TestSpliceSelectedScope_UnbalancedSelectorStillSplices(t *testing.T) { + // `{a: b} | {c: d}` is not a plain hash. Key extraction must refuse it + // rather than confidently returning the wrong key; the merged document then + // supplies the real scope. + local := []byte("binaries: {}\nprofiles:\n keep: {}\n") + merged := []byte("result:\n x: 1\n") + sel := []string{"{a: binaries} | {result: @}"} + + out, err := spliceSelectedScope(local, merged, sel, "b.yaml") + if err != nil { + t.Fatalf("splice: %v", err) + } + if !strings.Contains(string(out), "result") { + t.Errorf("merged key `result` was not spliced in:\n%s", out) + } + if !strings.Contains(string(out), "profiles") { + t.Errorf("out-of-scope profiles was dropped:\n%s", out) + } +} + +func TestSpliceSelectedScope_ComplexSelectorMatchingNothingIsANoOp(t *testing.T) { + // A complex selector that matches nothing must LEAVE THE LOCAL FILE ALONE. + // Its landing key is not derivable from the expression, so treating + // "absent from merged" as "delete it" would wipe a block on a selector that + // simply found no matches. + local := []byte("binaries:\n local-only: {}\nprofiles:\n keep: {}\n") + merged := []byte("{}\n") + sel := []string{"{binaries: from_items(items(binaries)[?[1].groups && contains([1].groups, 'nosuchgroup')])}"} + + out, err := spliceSelectedScope(local, merged, sel, "b.yaml") + if err != nil { + t.Fatalf("splice: %v", err) + } + if !strings.Contains(string(out), "local-only") { + t.Errorf("an empty projection deleted the consumer's binaries block:\n%s", out) + } + if !strings.Contains(string(out), "profiles") { + t.Errorf("out-of-scope profiles was dropped:\n%s", out) + } +} + +func TestSpliceSelectedScope_ComplexSelectorCannotTouchAnUnrelatedKey(t *testing.T) { + // wrapKeyFor sends function-style selectors to "result". That guess must + // never put a consumer-owned `result:` key in scope. + local := []byte("result: consumer-owned\nbinaries:\n keep: {}\n") + merged := []byte("{}\n") + sel := []string{"from_items(items(binaries)[?[1].groups])"} + + out, err := spliceSelectedScope(local, merged, sel, "b.yaml") + if err != nil { + t.Fatalf("splice: %v", err) + } + if !strings.Contains(string(out), "consumer-owned") { + t.Errorf("a phantom `result` scope key deleted a consumer-owned key:\n%s", out) + } +} + +func TestSpliceSelectedScope_ConflictPathLeavesUnrelatedKeysAlone(t *testing.T) { + // When `merged` carries git conflict markers there are no keys to read, and + // the scope falls back to the selectors. A complex selector's wrapKeyFor + // guess ("result") must not reach a consumer-owned `result:` there either — + // on that path it both deleted the key and swallowed the markers it was + // supposed to write. + local := []byte("result: consumer-owned\nbinaries:\n keep: {}\n") + merged := []byte("<<<<<<< local\nbinaries: {}\n=======\nbinaries:\n up: {}\n>>>>>>> upstream\n") + sel := []string{"from_items(items(binaries)[?[1].groups])"} + + out, err := spliceSelectedScope(local, merged, sel, "b.yaml") + if err != nil { + t.Fatalf("splice: %v", err) + } + if !strings.Contains(string(out), "consumer-owned") { + t.Errorf("the conflict path deleted a consumer-owned key via the `result` guess:\n%s", out) + } +} diff --git a/pkg/state/resolve.go b/pkg/state/resolve.go index 83f775b..28dfdae 100644 --- a/pkg/state/resolve.go +++ b/pkg/state/resolve.go @@ -23,7 +23,9 @@ func ResolveProfileIncludes(profile *EnvEntry, allProfiles EnvList) (*EnvEntry, return nil, err } - // Merge in order: earlier profiles are base, later override + // Merge in order: earlier profiles are the base. Scalars and Dest are + // last-wins; Select and Ignore COMPOSE across the chain (mergeGlobConfig), + // because each profile selects its own slice of a shared file. merged := &EnvEntry{ Key: profile.Key, Description: profile.Description, // never inherited @@ -31,9 +33,15 @@ func ResolveProfileIncludes(profile *EnvEntry, allProfiles EnvList) (*EnvEntry, } for _, p := range order { - // Merge files (later wins for same glob) + // A glob declared by several profiles in the include chain is + // COMPOSED, not replaced: each contributes its own selectors. Letting + // the last one win silently drops the base profiles' selection — e.g. + // lok8s' `kubeone` profile includes `local`, which includes `core` and + // `kustomize`, and all four select a DIFFERENT binaries group out of + // the same `.bin/b.yaml`. Replacement kept only the kubeone group, so + // core tools (argsh, sops, kubectl) were never installed. for glob, gc := range p.Files { - merged.Files[glob] = gc + merged.Files[glob] = mergeGlobConfig(merged.Files[glob], gc) } // Concatenate ignores @@ -65,6 +73,21 @@ func ResolveProfileIncludes(profile *EnvEntry, allProfiles EnvList) (*EnvEntry, return merged, nil } +// mergeGlobConfig composes two configs for the SAME glob, base first. Select +// and Ignore union (order-preserving, so the base profile's keys stay first); +// Dest is last-non-empty, since one file can only land in one place. +func mergeGlobConfig(base, override envmatch.GlobConfig) envmatch.GlobConfig { + out := envmatch.GlobConfig{ + Dest: base.Dest, + Ignore: appendUnique(base.Ignore, override.Ignore), + Select: appendUnique(base.Select, override.Select), + } + if override.Dest != "" { + out.Dest = override.Dest + } + return out +} + // collectIncludes performs a post-order DFS with proper cycle detection. // visited = fully processed nodes (skip), stack = current recursion path (cycle). func collectIncludes(key string, profiles EnvList, visited, stack map[string]bool, order *[]*EnvEntry) error { diff --git a/pkg/state/resolve_test.go b/pkg/state/resolve_test.go index d4d091b..47ab1b2 100644 --- a/pkg/state/resolve_test.go +++ b/pkg/state/resolve_test.go @@ -251,3 +251,79 @@ func TestResolveProfileIncludes_DescriptionNotInherited(t *testing.T) { t.Errorf("description = %q, want 'Top description'", resolved.Description) } } + +func TestResolveProfileIncludes_SameGlobSelectsUnion(t *testing.T) { + // The lok8s shape: every profile in the chain selects a DIFFERENT binaries + // group out of the SAME .bin/b.yaml. Replacing the glob config kept only + // the outermost profile's selector, so `kubeone` installed the kubeone + // group and none of core's (argsh, sops, kubectl, jq, yq). + core := &EnvEntry{ + Key: "core", + Files: map[string]envmatch.GlobConfig{".bin/b.yaml": {Select: []string{"{binaries: core}"}}}, + } + kustomize := &EnvEntry{ + Key: "kustomize", + Files: map[string]envmatch.GlobConfig{".bin/b.yaml": {Select: []string{"{binaries: kustomize}"}}}, + } + local := &EnvEntry{ + Key: "local", + Includes: []string{"core", "kustomize"}, + Files: map[string]envmatch.GlobConfig{".bin/b.yaml": {Select: []string{"{binaries: local}"}}}, + } + kubeone := &EnvEntry{ + Key: "kubeone", + Includes: []string{"local"}, + Files: map[string]envmatch.GlobConfig{".bin/b.yaml": {Select: []string{"{binaries: kubeone}"}}}, + } + + resolved, err := ResolveProfileIncludes(kubeone, EnvList{core, kustomize, local, kubeone}) + if err != nil { + t.Fatalf("resolve: %v", err) + } + + got := resolved.Files[".bin/b.yaml"].Select + want := []string{ + "{binaries: core}", + "{binaries: kustomize}", + "{binaries: local}", + "{binaries: kubeone}", + } + if len(got) != len(want) { + t.Fatalf("selectors for .bin/b.yaml = %v, want all four groups %v", got, want) + } + for i, w := range want { + if got[i] != w { + t.Errorf("selector %d = %q, want %q (base profiles must come first)", i, got[i], w) + } + } +} + +func TestResolveProfileIncludes_SameGlobDestAndIgnoreMerge(t *testing.T) { + base := &EnvEntry{ + Key: "base", + Files: map[string]envmatch.GlobConfig{ + "x/**": {Dest: "base/", Ignore: []string{"*.tmp"}}, + }, + } + child := &EnvEntry{ + Key: "child", + Includes: []string{"base"}, + Files: map[string]envmatch.GlobConfig{ + "x/**": {Dest: "child/", Ignore: []string{"*.bak", "*.tmp"}}, + }, + } + + resolved, err := ResolveProfileIncludes(child, EnvList{base, child}) + if err != nil { + t.Fatalf("resolve: %v", err) + } + + gc := resolved.Files["x/**"] + // One file can only land in one place, so Dest stays last-wins. + if gc.Dest != "child/" { + t.Errorf("Dest = %q, want child/ (override wins)", gc.Dest) + } + if len(gc.Ignore) != 2 || gc.Ignore[0] != "*.tmp" || gc.Ignore[1] != "*.bak" { + t.Errorf("Ignore = %v, want [*.tmp *.bak] unioned without duplicates", gc.Ignore) + } +}