From 37085e8cadff228d912e4edec1d0df90f2a14fd7 Mon Sep 17 00:00:00 2001 From: Dan Tsekhanskiy <28414793+TsekNet@users.noreply.github.com> Date: Fri, 20 Mar 2026 08:34:41 -0400 Subject: [PATCH 01/12] fix: Subtract base-branch baseline from --git diffs (#24) When using --git in CI with staged/delayed deployments, fleet-plan reported changes already merged to main but not yet deployed. This trained users to ignore fleet-plan comments. Now in --git mode, fleet-plan extracts the base branch YAML via git show, diffs it against live Fleet, and subtracts those pre-existing deltas. Only the incremental changes from the current MR are reported. Ref #23 --- cmd/fleet-plan/main.go | 40 +++++++- internal/diff/differ.go | 114 ++++++++++++++++++++++ internal/diff/differ_test.go | 172 +++++++++++++++++++++++++++++++++ internal/git/baseline.go | 153 ++++++++++++++++++++++++++++++ internal/git/baseline_test.go | 174 ++++++++++++++++++++++++++++++++++ 5 files changed, 651 insertions(+), 2 deletions(-) create mode 100644 internal/git/baseline.go create mode 100644 internal/git/baseline_test.go diff --git a/cmd/fleet-plan/main.go b/cmd/fleet-plan/main.go index 6f81474..01d999a 100644 --- a/cmd/fleet-plan/main.go +++ b/cmd/fleet-plan/main.go @@ -136,6 +136,24 @@ func runDiff(cmd *cobra.Command, _ []string) error { return fmt.Errorf("no teams found in %s/teams/\nAre you in a fleet-gitops repo? Try --repo /path/to/repo", flagRepo) } + // Parse baseline (base branch) for subtraction when in --git mode. + var baseline *parser.ParsedRepo + if flagGit && len(changedFiles) > 0 && ci.DiffBaseSHA != "" { + baseRoot, baseCleanup, err := git.CheckoutBaseline(flagRepo, ci.DiffBaseSHA, changedFiles) + if err != nil { + fmt.Fprintf(os.Stderr, "Warning: could not extract baseline (%v), skipping baseline subtraction\n", err) + } else { + defer baseCleanup() + baseDefaultFile := resolveBaselineDefault(baseRoot, flagBase, flagEnv) + baseParsed, err := parser.ParseRepo(baseRoot, teams, baseDefaultFile) + if err != nil { + fmt.Fprintf(os.Stderr, "Warning: could not parse baseline (%v), skipping baseline subtraction\n", err) + } else { + baseline = baseParsed + } + } + } + client, err := api.NewClient(auth.URL, auth.Token) if err != nil { return err @@ -149,8 +167,11 @@ func runDiff(cmd *cobra.Command, _ []string) error { return err } - results := diff.Diff(state, repo, teams, changedFiles, - diff.WithScriptEnricher(client)) + diffOpts := []diff.DiffOption{diff.WithScriptEnricher(client)} + if baseline != nil { + diffOpts = append(diffOpts, diff.WithBaseline(baseline)) + } + results := diff.Diff(state, repo, teams, changedFiles, diffOpts...) elapsed := time.Since(start) hasChanges := output.HasChanges(results) @@ -234,6 +255,21 @@ func resolveDefaultFile(repo, base, env string) (path string, cleanup func(), er return tmpPath, func() { os.Remove(tmpPath) }, nil } +// resolveBaselineDefault returns the path to default.yml within the baseline +// temp dir, if it exists. For baseline parsing, we use the base branch's own +// default.yml (no merge with env overlay, since the overlay may have changed +// in the MR). +func resolveBaselineDefault(baseRoot, base, env string) string { + // If base+env merge was used, check if default.yml exists in the baseline. + if base != "" && env != "" { + candidate := filepath.Join(baseRoot, "default.yml") + if _, err := os.Stat(candidate); err == nil { + return candidate + } + } + return "" +} + // buildHeading returns the default CI heading using the Fleet server URL. func buildHeading(fleetURL string) string { display := strings.TrimPrefix(fleetURL, "https://") diff --git a/internal/diff/differ.go b/internal/diff/differ.go index 7d14ba4..93951f6 100644 --- a/internal/diff/differ.go +++ b/internal/diff/differ.go @@ -100,6 +100,7 @@ type DiffOption func(*diffOptions) type diffOptions struct { enricher ScriptEnricher + baseline *parser.ParsedRepo } // WithScriptEnricher enables script-level diffing for fleet-maintained apps. @@ -107,6 +108,14 @@ func WithScriptEnricher(e ScriptEnricher) DiffOption { return func(o *diffOptions) { o.enricher = e } } +// WithBaseline provides a parsed base-branch repo. When set, Diff subtracts +// changes that already exist between the base branch and Fleet (i.e. changes +// merged to main but not yet deployed) so that only the incremental changes +// introduced by the current MR are reported. +func WithBaseline(b *parser.ParsedRepo) DiffOption { + return func(o *diffOptions) { o.baseline = b } +} + // Diff computes the diff between current Fleet state and proposed YAML for all // teams. If teamFilters is non-empty, only matching teams are diffed. // If changedFiles is non-empty, only resources whose SourceFile matches are @@ -228,6 +237,30 @@ func Diff(current *api.FleetState, proposed *parser.ParsedRepo, teamFilters []st result.Scripts = filterResourceDiff(result.Scripts, sourceNames, changedFiles) } + // Subtract baseline: remove changes that already exist between the + // base branch and Fleet (merged but not yet deployed). + if cfg.baseline != nil && exists { + if baseTeam, ok := findBaselineTeam(cfg.baseline, proposedTeam.Name); ok { + baseDiff := DiffResult{} + baseDiff.Policies = diffPolicies(currentTeam.Policies, baseTeam.Policies) + baseDiff.Queries = diffQueries(currentTeam.Queries, baseTeam.Queries) + if !currentTeam.SoftwareUnavailable { + baseDiff.Software = diffSoftware(currentTeam.Software, baseTeam.Software) + } + if !currentTeam.ProfilesUnavailable { + baseDiff.Profiles, _ = diffProfiles(currentTeam.Profiles, baseTeam.Profiles) + } + if !currentTeam.ScriptsUnavailable { + baseDiff.Scripts = diffScripts(currentTeam.Scripts, baseTeam.Scripts) + } + result.Policies = subtractResourceDiff(result.Policies, baseDiff.Policies) + result.Queries = subtractResourceDiff(result.Queries, baseDiff.Queries) + result.Software = subtractResourceDiff(result.Software, baseDiff.Software) + result.Profiles = subtractResourceDiff(result.Profiles, baseDiff.Profiles) + result.Scripts = subtractResourceDiff(result.Scripts, baseDiff.Scripts) + } + } + result.Labels = validateLabels(proposedTeam, labelMap, changedNames(result.Policies)) results = append(results, result) } @@ -315,6 +348,87 @@ func filterChanges(changes []ResourceChange, keep func(string) bool) []ResourceC return out } +// ---------- Baseline subtraction ---------- + +// findBaselineTeam looks up a team by name in the baseline parsed repo. +func findBaselineTeam(baseline *parser.ParsedRepo, name string) (parser.ParsedTeam, bool) { + for _, t := range baseline.Teams { + if strings.EqualFold(t.Name, name) { + return t, true + } + } + return parser.ParsedTeam{}, false +} + +// subtractResourceDiff removes changes from "total" that also appear in +// "baseline". A change is considered the same if it has the same Name and +// change type (added/modified/deleted). +// +// For modified resources, if the resource appears in both diffs but with +// different field changes, it is kept (the MR introduced additional changes +// beyond what the baseline already had). +func subtractResourceDiff(total, baseline ResourceDiff) ResourceDiff { + return ResourceDiff{ + Added: subtractChanges(total.Added, baseline.Added), + Modified: subtractModified(total.Modified, baseline.Modified), + Deleted: subtractChanges(total.Deleted, baseline.Deleted), + } +} + +// subtractChanges removes entries from "total" whose Name matches an entry in +// "baseline". Used for Added and Deleted lists where name-match is sufficient. +func subtractChanges(total, baseline []ResourceChange) []ResourceChange { + if len(baseline) == 0 { + return total + } + baseNames := make(map[string]bool, len(baseline)) + for _, b := range baseline { + baseNames[b.Name] = true + } + var out []ResourceChange + for _, c := range total { + if !baseNames[c.Name] { + out = append(out, c) + } + } + return out +} + +// subtractModified removes entries from "total" that have the exact same field +// diffs in "baseline". If a resource is modified in both but with different +// fields or values, it is kept (the MR changed it further). +func subtractModified(total, baseline []ResourceChange) []ResourceChange { + if len(baseline) == 0 { + return total + } + baseFields := make(map[string]map[string]FieldDiff, len(baseline)) + for _, b := range baseline { + baseFields[b.Name] = b.Fields + } + var out []ResourceChange + for _, c := range total { + bf, exists := baseFields[c.Name] + if !exists || !sameFieldDiffs(c.Fields, bf) { + out = append(out, c) + } + } + return out +} + +// sameFieldDiffs returns true if two field diff maps are identical. +func sameFieldDiffs(a, b map[string]FieldDiff) bool { + if len(a) != len(b) { + return false + } + for k, av := range a { + bv, ok := b[k] + if !ok || av.Old != bv.Old || av.New != bv.New { + return false + } + } + return true +} + // ---------- Per-resource diffing ---------- func diffPolicies(current []api.Policy, proposed []parser.ParsedPolicy) ResourceDiff { diff --git a/internal/diff/differ_test.go b/internal/diff/differ_test.go index d3e0783..fd8885b 100644 --- a/internal/diff/differ_test.go +++ b/internal/diff/differ_test.go @@ -1710,6 +1710,178 @@ func TestDiffScriptChangedFileFilter(t *testing.T) { } } +// ---------- Baseline subtraction tests ---------- + +func TestSubtractResourceDiff(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + total ResourceDiff + baseline ResourceDiff + want ResourceDiff + }{ + { + name: "empty baseline changes nothing", + total: ResourceDiff{Added: []ResourceChange{{Name: "A"}}}, + baseline: ResourceDiff{}, + want: ResourceDiff{Added: []ResourceChange{{Name: "A"}}}, + }, + { + name: "subtract matching delete", + total: ResourceDiff{Deleted: []ResourceChange{{Name: "old-policy"}, {Name: "mr-policy"}}}, + baseline: ResourceDiff{Deleted: []ResourceChange{{Name: "old-policy"}}}, + want: ResourceDiff{Deleted: []ResourceChange{{Name: "mr-policy"}}}, + }, + { + name: "subtract matching add", + total: ResourceDiff{Added: []ResourceChange{{Name: "base-add"}, {Name: "mr-add"}}}, + baseline: ResourceDiff{Added: []ResourceChange{{Name: "base-add"}}}, + want: ResourceDiff{Added: []ResourceChange{{Name: "mr-add"}}}, + }, + { + name: "subtract identical modify", + total: ResourceDiff{Modified: []ResourceChange{ + {Name: "same-mod", Fields: map[string]FieldDiff{"query": {Old: "a", New: "b"}}}, + {Name: "mr-mod", Fields: map[string]FieldDiff{"query": {Old: "x", New: "y"}}}, + }}, + baseline: ResourceDiff{Modified: []ResourceChange{ + {Name: "same-mod", Fields: map[string]FieldDiff{"query": {Old: "a", New: "b"}}}, + }}, + want: ResourceDiff{Modified: []ResourceChange{ + {Name: "mr-mod", Fields: map[string]FieldDiff{"query": {Old: "x", New: "y"}}}, + }}, + }, + { + name: "keep modify with different fields", + total: ResourceDiff{Modified: []ResourceChange{ + {Name: "evolved", Fields: map[string]FieldDiff{"query": {Old: "a", New: "c"}}}, + }}, + baseline: ResourceDiff{Modified: []ResourceChange{ + {Name: "evolved", Fields: map[string]FieldDiff{"query": {Old: "a", New: "b"}}}, + }}, + want: ResourceDiff{Modified: []ResourceChange{ + {Name: "evolved", Fields: map[string]FieldDiff{"query": {Old: "a", New: "c"}}}, + }}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := subtractResourceDiff(tt.total, tt.baseline) + assertResourceDiffEqual(t, tt.want, got) + }) + } +} + +func TestDiffWithBaselineSubtraction(t *testing.T) { + t.Parallel() + + // Simulate: base branch already removed "old-policy" and modified "shared-query". + // MR adds "new-query" and removes "mr-removed-policy". + // Only the MR's changes should appear. + + current := &api.FleetState{ + Teams: []api.Team{ + { + ID: 1, + Name: "TestTeam", + Policies: []api.Policy{ + {Name: "old-policy", Query: "SELECT 1;", Platform: "linux"}, + {Name: "mr-removed-policy", Query: "SELECT 2;", Platform: "windows"}, + }, + Queries: []api.Query{ + {Name: "shared-query", Query: "SELECT old;", Interval: 3600}, + {Name: "existing-query", Query: "SELECT x;", Interval: 300}, + }, + }, + }, + } + + // MR branch: old-policy gone (already removed in base), mr-removed-policy gone (MR removes it), + // shared-query modified to "SELECT new;" (already modified in base to same value), + // new-query added (MR adds it). + proposed := &parser.ParsedRepo{ + Teams: []parser.ParsedTeam{ + { + Name: "TestTeam", + SourceFile: "teams/test.yml", + Queries: []parser.ParsedQuery{ + {Name: "shared-query", Query: "SELECT new;", Interval: 3600}, + {Name: "existing-query", Query: "SELECT x;", Interval: 300}, + {Name: "new-query", Query: "SELECT fresh;", Interval: 600, SourceFile: "queries/new.yml"}, + }, + }, + }, + } + + // Base branch: old-policy already removed, shared-query already modified, + // but mr-removed-policy still present. + baseline := &parser.ParsedRepo{ + Teams: []parser.ParsedTeam{ + { + Name: "TestTeam", + SourceFile: "teams/test.yml", + Policies: []parser.ParsedPolicy{ + {Name: "mr-removed-policy", Query: "SELECT 2;", Platform: "windows"}, + }, + Queries: []parser.ParsedQuery{ + {Name: "shared-query", Query: "SELECT new;", Interval: 3600}, + {Name: "existing-query", Query: "SELECT x;", Interval: 300}, + }, + }, + }, + } + + results := Diff(current, proposed, nil, nil, WithBaseline(baseline)) + if len(results) != 1 { + t.Fatalf("expected 1 result, got %d", len(results)) + } + r := results[0] + + // old-policy deletion should be subtracted (already in base). + if len(r.Policies.Deleted) != 1 { + t.Fatalf("expected 1 deleted policy (mr-removed-policy), got %d: %+v", len(r.Policies.Deleted), r.Policies.Deleted) + } + if r.Policies.Deleted[0].Name != "mr-removed-policy" { + t.Errorf("deleted policy name: got %q, want mr-removed-policy", r.Policies.Deleted[0].Name) + } + + // shared-query modification should be subtracted (same diff in base). + if len(r.Queries.Modified) != 0 { + t.Errorf("expected 0 modified queries (subtracted), got %d: %+v", len(r.Queries.Modified), r.Queries.Modified) + } + + // new-query addition should remain (not in base). + if len(r.Queries.Added) != 1 { + t.Fatalf("expected 1 added query (new-query), got %d", len(r.Queries.Added)) + } + if r.Queries.Added[0].Name != "new-query" { + t.Errorf("added query name: got %q, want new-query", r.Queries.Added[0].Name) + } +} + +func assertResourceDiffEqual(t *testing.T, want, got ResourceDiff) { + t.Helper() + assertChangesEqual(t, "Added", want.Added, got.Added) + assertChangesEqual(t, "Modified", want.Modified, got.Modified) + assertChangesEqual(t, "Deleted", want.Deleted, got.Deleted) +} + +func assertChangesEqual(t *testing.T, label string, want, got []ResourceChange) { + t.Helper() + if len(want) != len(got) { + t.Errorf("%s: want %d changes, got %d", label, len(want), len(got)) + return + } + for i := range want { + if want[i].Name != got[i].Name { + t.Errorf("%s[%d].Name: want %q, got %q", label, i, want[i].Name, got[i].Name) + } + } +} + // findTeam locates a DiffResult by team name, failing the test if not found. func findTeam(t *testing.T, results []DiffResult, name string) *DiffResult { t.Helper() diff --git a/internal/git/baseline.go b/internal/git/baseline.go new file mode 100644 index 0000000..dd0fc4c --- /dev/null +++ b/internal/git/baseline.go @@ -0,0 +1,153 @@ +package git + +import ( + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" +) + +// CheckoutBaseline extracts the base-branch versions of the given files into a +// temporary directory that mirrors the repo layout. It uses "git show :" +// so it works in shallow clones without a full checkout. +// +// The caller must call the returned cleanup function to remove the temp dir. +// If the base ref is unavailable or a file doesn't exist at the base ref, that +// file is silently skipped (it may be newly added in the MR). +func CheckoutBaseline(repoRoot string, baseRef string, files []string) (tmpRoot string, cleanup func(), err error) { + if baseRef == "" { + return "", nil, fmt.Errorf("no base ref provided") + } + + tmpRoot, err = os.MkdirTemp("", "fleet-plan-baseline-*") + if err != nil { + return "", nil, fmt.Errorf("creating temp dir: %w", err) + } + cleanup = func() { os.RemoveAll(tmpRoot) } + + // Resolve which files we need: the explicitly changed files, plus any + // files they reference (path: directives in team YAML). We start with + // the team files themselves, then do a second pass for references. + needed := collectBaselineFiles(repoRoot, baseRef, files) + + var extracted int + for _, f := range needed { + content, err := gitShow(repoRoot, baseRef, f) + if err != nil { + // File doesn't exist at base ref (newly added), skip. + continue + } + + dst := filepath.Join(tmpRoot, f) + if err := os.MkdirAll(filepath.Dir(dst), 0o755); err != nil { + cleanup() + return "", nil, fmt.Errorf("creating dir for %s: %w", f, err) + } + if err := os.WriteFile(dst, content, 0o644); err != nil { + cleanup() + return "", nil, fmt.Errorf("writing %s: %w", f, err) + } + extracted++ + } + + if extracted == 0 { + cleanup() + return "", nil, fmt.Errorf("no baseline files could be extracted") + } + + return tmpRoot, cleanup, nil +} + +// collectBaselineFiles returns the full set of files needed for baseline parsing. +// This includes the changed team files and any path: references they contain. +func collectBaselineFiles(repoRoot, baseRef string, changedFiles []string) []string { + seen := make(map[string]bool) + var result []string + + add := func(f string) { + if !seen[f] { + seen[f] = true + result = append(result, f) + } + } + + // Start with all fleet-relevant changed files. + for _, f := range changedFiles { + add(f) + } + + // For each team file, extract it from base ref and scan for path: references. + // Also extract any referenced resource files so the parser can resolve them. + for _, f := range changedFiles { + if !strings.HasPrefix(f, "teams/") { + continue + } + content, err := gitShow(repoRoot, baseRef, f) + if err != nil { + continue + } + for _, ref := range extractPathRefs(content, f) { + add(ref) + // Resource YAML files may reference scripts, profiles, etc. + refContent, err := gitShow(repoRoot, baseRef, ref) + if err != nil { + continue + } + for _, nested := range extractPathRefs(refContent, ref) { + add(nested) + } + } + } + + // Also include default.yml and base.yml if they exist, since the parser + // may need them for global config context. + for _, f := range []string{"default.yml", "base.yml"} { + if _, err := gitShow(repoRoot, baseRef, f); err == nil { + add(f) + } + } + + return result +} + +// extractPathRefs scans YAML content for "path:" directives and returns the +// resolved file paths relative to repo root. +func extractPathRefs(content []byte, sourceFile string) []string { + var refs []string + sourceDir := filepath.Dir(sourceFile) + for _, line := range strings.Split(string(content), "\n") { + trimmed := strings.TrimSpace(line) + if !strings.HasPrefix(trimmed, "- path:") && !strings.HasPrefix(trimmed, "path:") { + continue + } + parts := strings.SplitN(trimmed, ":", 2) + if len(parts) != 2 { + continue + } + ref := strings.TrimSpace(parts[1]) + // Remove quotes if present. + ref = strings.Trim(ref, `"'`) + if ref == "" { + continue + } + // Resolve relative to the source file's directory. + resolved := filepath.Join(sourceDir, ref) + resolved = filepath.Clean(resolved) + if !strings.HasPrefix(resolved, "..") { + refs = append(refs, resolved) + } + } + return refs +} + +// gitShow runs "git show :" and returns the file content. +func gitShow(repoRoot, ref, path string) ([]byte, error) { + cmd := exec.Command("git", "show", ref+":"+path) + cmd.Dir = repoRoot + out, err := cmd.Output() + if err != nil { + return nil, fmt.Errorf("git show %s:%s: %w", ref, path, err) + } + return out, nil +} diff --git a/internal/git/baseline_test.go b/internal/git/baseline_test.go new file mode 100644 index 0000000..02f5714 --- /dev/null +++ b/internal/git/baseline_test.go @@ -0,0 +1,174 @@ +package git + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +func TestExtractPathRefs(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + content string + sourceFile string + want []string + }{ + { + name: "policy path refs", + content: " - path: ../policies/windows/filevault.yml\n - path: ../policies/linux/ssh.yml\n", + sourceFile: "teams/workstations.yml", + want: []string{"policies/windows/filevault.yml", "policies/linux/ssh.yml"}, + }, + { + name: "software path ref", + content: " path: ../software/mac/slack/slack.yml\n", + sourceFile: "teams/workstations.yml", + want: []string{"software/mac/slack/slack.yml"}, + }, + { + name: "quoted path", + content: " - path: \"../queries/windows/hardware.yml\"\n", + sourceFile: "teams/test.yml", + want: []string{"queries/windows/hardware.yml"}, + }, + { + name: "no path refs", + content: "name: TestTeam\nagent_options:\n config:\n options:\n logger_plugin: tls\n", + sourceFile: "teams/test.yml", + want: nil, + }, + { + name: "nested ref from software yaml", + content: "path: install.ps1\n", + sourceFile: "software/windows/agent/agent.yml", + want: []string{"software/windows/agent/install.ps1"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := extractPathRefs([]byte(tt.content), tt.sourceFile) + if len(got) != len(tt.want) { + t.Fatalf("got %d refs, want %d: %v", len(got), len(tt.want), got) + } + for i := range tt.want { + if got[i] != tt.want[i] { + t.Errorf("ref[%d]: got %q, want %q", i, got[i], tt.want[i]) + } + } + }) + } +} + +func TestCheckoutBaseline_NoBaseRef(t *testing.T) { + t.Parallel() + _, _, err := CheckoutBaseline("/tmp", "", []string{"teams/test.yml"}) + if err == nil { + t.Fatal("expected error for empty base ref") + } +} + +func TestCheckoutBaseline_ExtractsFiles(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + gitRun(t, dir, "init") + gitRun(t, dir, "config", "user.email", "test@test.com") + gitRun(t, dir, "config", "user.name", "Test") + + // Create initial commit with a team file and a policy file. + os.MkdirAll(filepath.Join(dir, "teams"), 0o755) + os.MkdirAll(filepath.Join(dir, "policies", "linux"), 0o755) + os.WriteFile(filepath.Join(dir, "teams", "test.yml"), []byte("name: Test\npolicies:\n - path: ../policies/linux/ssh.yml\n"), 0o644) + os.WriteFile(filepath.Join(dir, "policies", "linux", "ssh.yml"), []byte("name: SSH\nquery: SELECT 1;\n"), 0o644) + gitRun(t, dir, "add", "-A") + gitRun(t, dir, "commit", "-m", "init") + + baseSHA := gitOutput(t, dir, "rev-parse", "HEAD") + + // Modify the team file on a new "branch" (just a new commit). + os.WriteFile(filepath.Join(dir, "teams", "test.yml"), []byte("name: Test\npolicies:\n - path: ../policies/linux/ssh.yml\nqueries:\n - path: ../queries/new.yml\n"), 0o644) + gitRun(t, dir, "add", "-A") + gitRun(t, dir, "commit", "-m", "add query") + + // CheckoutBaseline should extract the base version. + tmpRoot, cleanup, err := CheckoutBaseline(dir, baseSHA, []string{"teams/test.yml"}) + if err != nil { + t.Fatalf("CheckoutBaseline: %v", err) + } + defer cleanup() + + // Verify the team file was extracted with the base content. + content, err := os.ReadFile(filepath.Join(tmpRoot, "teams", "test.yml")) + if err != nil { + t.Fatalf("reading extracted team file: %v", err) + } + if !strings.Contains(string(content), "name: Test") { + t.Errorf("expected team file content, got: %s", content) + } + if strings.Contains(string(content), "queries") { + t.Errorf("base version should not contain queries section, got: %s", content) + } + + // Verify the referenced policy file was also extracted. + policyContent, err := os.ReadFile(filepath.Join(tmpRoot, "policies", "linux", "ssh.yml")) + if err != nil { + t.Fatalf("reading extracted policy file: %v", err) + } + if !strings.Contains(string(policyContent), "name: SSH") { + t.Errorf("expected policy file content, got: %s", policyContent) + } +} + +func TestCollectBaselineFiles_IncludesDefaultYml(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + gitRun(t, dir, "init") + gitRun(t, dir, "config", "user.email", "test@test.com") + gitRun(t, dir, "config", "user.name", "Test") + + os.MkdirAll(filepath.Join(dir, "teams"), 0o755) + os.WriteFile(filepath.Join(dir, "default.yml"), []byte("org_settings:\n"), 0o644) + os.WriteFile(filepath.Join(dir, "teams", "test.yml"), []byte("name: Test\n"), 0o644) + gitRun(t, dir, "add", "-A") + gitRun(t, dir, "commit", "-m", "init") + + sha := gitOutput(t, dir, "rev-parse", "HEAD") + files := collectBaselineFiles(dir, sha, []string{"teams/test.yml"}) + + found := false + for _, f := range files { + if f == "default.yml" { + found = true + } + } + if !found { + t.Errorf("expected default.yml in collected files, got: %v", files) + } +} + +func gitRun(t *testing.T, dir string, args ...string) { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = dir + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git %v: %v\n%s", args, err, out) + } +} + +func gitOutput(t *testing.T, dir string, args ...string) string { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = dir + out, err := cmd.Output() + if err != nil { + t.Fatalf("git %v: %v", args, err) + } + return strings.TrimSpace(string(out)) +} From d7ab3f299c69633f687a5c18cd147e69e2b830bf Mon Sep 17 00:00:00 2001 From: Dan Tsekhanskiy Date: Fri, 20 Mar 2026 08:38:21 -0400 Subject: [PATCH 02/12] fix: Use enriched software state for baseline subtraction The baseline diff was using raw API software state without fleet-maintained app script enrichment, causing fleet-maintained app diffs (e.g. blender uninstall_script) to not be subtracted properly. --- internal/diff/differ.go | 59 ++++++++++++++++++++++------------------- 1 file changed, 31 insertions(+), 28 deletions(-) diff --git a/internal/diff/differ.go b/internal/diff/differ.go index 93951f6..749dea1 100644 --- a/internal/diff/differ.go +++ b/internal/diff/differ.go @@ -193,11 +193,14 @@ func Diff(current *api.FleetState, proposed *parser.ParsedRepo, teamFilters []st result.Policies = diffPolicies(currentTeam.Policies, proposedTeam.Policies) result.Queries = diffQueries(currentTeam.Queries, proposedTeam.Queries) + // enrichedSoftware holds the API software state with fleet-maintained + // app scripts populated. Hoisted here so the baseline subtraction + // can reuse the same enriched state. + enrichedSoftware := currentTeam.Software + if currentTeam.SoftwareUnavailable { result.Errors = append(result.Errors, "software diff skipped: API token lacks permission to read software titles") } else { - currentSoftware := currentTeam.Software - // Fleet's /teams API may return fleet_maintained_apps: null, or // return a partial list (e.g., only macOS FMAs while Windows FMAs // are merged into packages). Infer from software titles + catalog @@ -207,10 +210,10 @@ func Diff(current *api.FleetState, proposed *parser.ParsedRepo, teamFilters []st if cfg.enricher != nil && len(inferred) > 0 { cfg.enricher.EnrichFleetAppScripts(context.Background(), inferred) } - currentSoftware.FleetMaintained = mergeFleetApps(currentTeam.Software.FleetMaintained, inferred) + enrichedSoftware.FleetMaintained = mergeFleetApps(currentTeam.Software.FleetMaintained, inferred) } - result.Software = diffSoftware(currentSoftware, proposedTeam.Software) + result.Software = diffSoftware(enrichedSoftware, proposedTeam.Software) } if currentTeam.ProfilesUnavailable { @@ -226,6 +229,30 @@ func Diff(current *api.FleetState, proposed *parser.ParsedRepo, teamFilters []st } else { result.Scripts = diffScripts(currentTeam.Scripts, proposedTeam.Scripts) } + + // Subtract baseline: remove changes that already exist between the + // base branch and Fleet (merged but not yet deployed). + if cfg.baseline != nil { + if baseTeam, ok := findBaselineTeam(cfg.baseline, proposedTeam.Name); ok { + baseDiff := DiffResult{} + baseDiff.Policies = diffPolicies(currentTeam.Policies, baseTeam.Policies) + baseDiff.Queries = diffQueries(currentTeam.Queries, baseTeam.Queries) + if !currentTeam.SoftwareUnavailable { + baseDiff.Software = diffSoftware(enrichedSoftware, baseTeam.Software) + } + if !currentTeam.ProfilesUnavailable { + baseDiff.Profiles, _ = diffProfiles(currentTeam.Profiles, baseTeam.Profiles) + } + if !currentTeam.ScriptsUnavailable { + baseDiff.Scripts = diffScripts(currentTeam.Scripts, baseTeam.Scripts) + } + result.Policies = subtractResourceDiff(result.Policies, baseDiff.Policies) + result.Queries = subtractResourceDiff(result.Queries, baseDiff.Queries) + result.Software = subtractResourceDiff(result.Software, baseDiff.Software) + result.Profiles = subtractResourceDiff(result.Profiles, baseDiff.Profiles) + result.Scripts = subtractResourceDiff(result.Scripts, baseDiff.Scripts) + } + } } if len(changedFiles) > 0 { @@ -237,30 +264,6 @@ func Diff(current *api.FleetState, proposed *parser.ParsedRepo, teamFilters []st result.Scripts = filterResourceDiff(result.Scripts, sourceNames, changedFiles) } - // Subtract baseline: remove changes that already exist between the - // base branch and Fleet (merged but not yet deployed). - if cfg.baseline != nil && exists { - if baseTeam, ok := findBaselineTeam(cfg.baseline, proposedTeam.Name); ok { - baseDiff := DiffResult{} - baseDiff.Policies = diffPolicies(currentTeam.Policies, baseTeam.Policies) - baseDiff.Queries = diffQueries(currentTeam.Queries, baseTeam.Queries) - if !currentTeam.SoftwareUnavailable { - baseDiff.Software = diffSoftware(currentTeam.Software, baseTeam.Software) - } - if !currentTeam.ProfilesUnavailable { - baseDiff.Profiles, _ = diffProfiles(currentTeam.Profiles, baseTeam.Profiles) - } - if !currentTeam.ScriptsUnavailable { - baseDiff.Scripts = diffScripts(currentTeam.Scripts, baseTeam.Scripts) - } - result.Policies = subtractResourceDiff(result.Policies, baseDiff.Policies) - result.Queries = subtractResourceDiff(result.Queries, baseDiff.Queries) - result.Software = subtractResourceDiff(result.Software, baseDiff.Software) - result.Profiles = subtractResourceDiff(result.Profiles, baseDiff.Profiles) - result.Scripts = subtractResourceDiff(result.Scripts, baseDiff.Scripts) - } - } - result.Labels = validateLabels(proposedTeam, labelMap, changedNames(result.Policies)) results = append(results, result) } From 2e2ecbca67d4161868c45b4a42601ec01ed6c7a0 Mon Sep 17 00:00:00 2001 From: Dan Tsekhanskiy Date: Fri, 20 Mar 2026 08:55:21 -0400 Subject: [PATCH 03/12] fix: Include team source file in resource filter map When an MR adds a path: reference to teams/foo.yml, the referenced resource's SourceFile (e.g. queries/windows/hardware.yml) isn't in the MR's changed files list. The filter dropped these resources because it only checked the individual SourceFile, not the team file that includes them. Now buildSourceMap registers the team's SourceFile for all resource types (policies, queries, software, profiles, scripts), matching the existing behavior for fleet-maintained apps. --- internal/diff/differ.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/internal/diff/differ.go b/internal/diff/differ.go index 749dea1..cb67b0d 100644 --- a/internal/diff/differ.go +++ b/internal/diff/differ.go @@ -280,9 +280,11 @@ func buildSourceMap(team parser.ParsedTeam) map[string][]string { } for _, p := range team.Policies { add(p.Name, p.SourceFile) + add(p.Name, team.SourceFile) } for _, q := range team.Queries { add(q.Name, q.SourceFile) + add(q.Name, team.SourceFile) } for _, p := range team.Software.Packages { key := parser.NormalizeSoftwarePath(p.RefPath) @@ -294,6 +296,7 @@ func buildSourceMap(team parser.ParsedTeam) map[string][]string { } if key != "" { add(parser.NormalizeSoftwarePath(key), p.SourceFile) + add(parser.NormalizeSoftwarePath(key), team.SourceFile) for _, sf := range p.SourceFiles { add(parser.NormalizeSoftwarePath(key), sf) } @@ -312,9 +315,11 @@ func buildSourceMap(team parser.ParsedTeam) map[string][]string { } for _, p := range team.Profiles { add(p.Name, p.SourceFile) + add(p.Name, team.SourceFile) } for _, s := range team.Scripts { add(s.Name, s.SourceFile) + add(s.Name, team.SourceFile) } return m } From 221996753c289ba6b9354a833a6f7b681f0ddc15 Mon Sep 17 00:00:00 2001 From: Dan Tsekhanskiy <28414793+TsekNet@users.noreply.github.com> Date: Fri, 20 Mar 2026 12:53:48 -0400 Subject: [PATCH 04/12] fix: Add baseline subtraction for global config diffs (#26) Global config changes (default.yml/base.yml) were not subtracted, causing MRs touching global config to show all pending undeployed changes. Now applies subtractConfigChanges and subtractResourceDiff to the global diff block. Also merges base branch base.yml with env overlay for baseline global parsing. Ref #25 --- cmd/fleet-plan/main.go | 46 ++++++++++++++++++++++++++++++++--------- internal/diff/differ.go | 34 ++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 10 deletions(-) diff --git a/cmd/fleet-plan/main.go b/cmd/fleet-plan/main.go index 01d999a..dffe793 100644 --- a/cmd/fleet-plan/main.go +++ b/cmd/fleet-plan/main.go @@ -144,7 +144,7 @@ func runDiff(cmd *cobra.Command, _ []string) error { fmt.Fprintf(os.Stderr, "Warning: could not extract baseline (%v), skipping baseline subtraction\n", err) } else { defer baseCleanup() - baseDefaultFile := resolveBaselineDefault(baseRoot, flagBase, flagEnv) + baseDefaultFile := resolveBaselineDefault(baseRoot, flagRepo, flagBase, flagEnv) baseParsed, err := parser.ParseRepo(baseRoot, teams, baseDefaultFile) if err != nil { fmt.Fprintf(os.Stderr, "Warning: could not parse baseline (%v), skipping baseline subtraction\n", err) @@ -255,17 +255,43 @@ func resolveDefaultFile(repo, base, env string) (path string, cleanup func(), er return tmpPath, func() { os.Remove(tmpPath) }, nil } -// resolveBaselineDefault returns the path to default.yml within the baseline -// temp dir, if it exists. For baseline parsing, we use the base branch's own -// default.yml (no merge with env overlay, since the overlay may have changed -// in the MR). -func resolveBaselineDefault(baseRoot, base, env string) string { - // If base+env merge was used, check if default.yml exists in the baseline. +// resolveBaselineDefault returns the path to default.yml for baseline parsing. +// When --base and --env are used, it merges the base branch's base.yml with +// the env overlay (from the MR branch, since env overlays rarely change). +// Falls back to the baseline's default.yml if present. +func resolveBaselineDefault(baseRoot, repoRoot, base, env string) string { if base != "" && env != "" { - candidate := filepath.Join(baseRoot, "default.yml") - if _, err := os.Stat(candidate); err == nil { - return candidate + baseFile := filepath.Join(baseRoot, base) + if _, err := os.Stat(baseFile); err != nil { + // base.yml doesn't exist at base ref, skip + return "" + } + // Use the env overlay from the MR branch (repoRoot), not the baseline. + envFile := env + if !filepath.IsAbs(envFile) { + envFile = filepath.Join(repoRoot, envFile) } + if _, err := os.Stat(envFile); err != nil { + return "" + } + tmp, err := os.CreateTemp("", "fleet-plan-baseline-default-*.yml") + if err != nil { + return "" + } + tmpPath := tmp.Name() + tmp.Close() + if err := merge.MergeFiles(baseFile, envFile, tmpPath); err != nil { + os.Remove(tmpPath) + return "" + } + // Note: this temp file leaks if the caller doesn't clean it up. + // It's small and short-lived (CI job lifetime), acceptable tradeoff. + return tmpPath + } + // No base+env: check for plain default.yml in the baseline. + candidate := filepath.Join(baseRoot, "default.yml") + if _, err := os.Stat(candidate); err == nil { + return candidate } return "" } diff --git a/internal/diff/differ.go b/internal/diff/differ.go index cb67b0d..a0a8c0e 100644 --- a/internal/diff/differ.go +++ b/internal/diff/differ.go @@ -148,6 +148,20 @@ func Diff(current *api.FleetState, proposed *parser.ParsedRepo, teamFilters []st // Diff global queries globalResult.Queries = diffQueries(current.GlobalQueries, proposed.Global.Queries) + // Subtract baseline for global scope + if cfg.baseline != nil && cfg.baseline.Global != nil { + var baseConfig []ConfigChange + if current.Config != nil { + baseConfig, _ = diffConfig(current.Config, cfg.baseline.Global) + } + basePolicies := diffPolicies(current.GlobalPolicies, cfg.baseline.Global.Policies) + baseQueries := diffQueries(current.GlobalQueries, cfg.baseline.Global.Queries) + + globalResult.Config = subtractConfigChanges(globalResult.Config, baseConfig) + globalResult.Policies = subtractResourceDiff(globalResult.Policies, basePolicies) + globalResult.Queries = subtractResourceDiff(globalResult.Queries, baseQueries) + } + results = append(results, globalResult) } @@ -437,6 +451,26 @@ func sameFieldDiffs(a, b map[string]FieldDiff) bool { return true } +// subtractConfigChanges removes ConfigChange entries from "total" that also +// appear in "baseline" with the same Section, Key, Old, and New values. +func subtractConfigChanges(total, baseline []ConfigChange) []ConfigChange { + if len(baseline) == 0 { + return total + } + type configKey struct{ Section, Key, Old, New string } + baseSet := make(map[configKey]bool, len(baseline)) + for _, b := range baseline { + baseSet[configKey{b.Section, b.Key, b.Old, b.New}] = true + } + var out []ConfigChange + for _, c := range total { + if !baseSet[configKey{c.Section, c.Key, c.Old, c.New}] { + out = append(out, c) + } + } + return out +} + // ---------- Per-resource diffing ---------- func diffPolicies(current []api.Policy, proposed []parser.ParsedPolicy) ResourceDiff { From 0cbf60c218066347b4138bae02b06947712edb28 Mon Sep 17 00:00:00 2001 From: Dan Tsekhanskiy Date: Fri, 20 Mar 2026 13:15:12 -0400 Subject: [PATCH 05/12] fix: Extract path refs from base.yml for baseline parsing collectBaselineFiles only scanned team files for path: references. Global config files (base.yml, default.yml) also have path: refs to queries and policies that need to be in the baseline temp dir for the parser to resolve them. --- internal/git/baseline.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/internal/git/baseline.go b/internal/git/baseline.go index dd0fc4c..f49c4fc 100644 --- a/internal/git/baseline.go +++ b/internal/git/baseline.go @@ -77,10 +77,11 @@ func collectBaselineFiles(repoRoot, baseRef string, changedFiles []string) []str add(f) } - // For each team file, extract it from base ref and scan for path: references. - // Also extract any referenced resource files so the parser can resolve them. + // For each team or config file, extract it from base ref and scan for path: + // references. Also extract any referenced resource files so the parser can + // resolve them. for _, f := range changedFiles { - if !strings.HasPrefix(f, "teams/") { + if !strings.HasPrefix(f, "teams/") && f != "base.yml" && f != "default.yml" { continue } content, err := gitShow(repoRoot, baseRef, f) From 41065646f3a7ede85d34e1aa566b9871b74fbaad Mon Sep 17 00:00:00 2001 From: Dan Tsekhanskiy Date: Fri, 20 Mar 2026 13:19:00 -0400 Subject: [PATCH 06/12] fix: Place baseline merged default inside temp dir The merged base.yml+env overlay was written to /tmp, causing path: references (./queries/...) to resolve against /tmp instead of the baseline temp dir where the query files live. --- cmd/fleet-plan/main.go | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/cmd/fleet-plan/main.go b/cmd/fleet-plan/main.go index dffe793..b285cf9 100644 --- a/cmd/fleet-plan/main.go +++ b/cmd/fleet-plan/main.go @@ -274,18 +274,12 @@ func resolveBaselineDefault(baseRoot, repoRoot, base, env string) string { if _, err := os.Stat(envFile); err != nil { return "" } - tmp, err := os.CreateTemp("", "fleet-plan-baseline-default-*.yml") - if err != nil { - return "" - } - tmpPath := tmp.Name() - tmp.Close() + // Place the merged file inside the baseline temp dir so the parser + // resolves path: refs (./queries/...) relative to the baseline root. + tmpPath := filepath.Join(baseRoot, "default.yml") if err := merge.MergeFiles(baseFile, envFile, tmpPath); err != nil { - os.Remove(tmpPath) return "" } - // Note: this temp file leaks if the caller doesn't clean it up. - // It's small and short-lived (CI job lifetime), acceptable tradeoff. return tmpPath } // No base+env: check for plain default.yml in the baseline. From cab5a30dedb0f4e12fca21de8b631a987dc659cd Mon Sep 17 00:00:00 2001 From: Dan Tsekhanskiy Date: Fri, 20 Mar 2026 13:21:58 -0400 Subject: [PATCH 07/12] debug: Add baseline global stderr output --- internal/diff/differ.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/internal/diff/differ.go b/internal/diff/differ.go index a0a8c0e..10d3a4e 100644 --- a/internal/diff/differ.go +++ b/internal/diff/differ.go @@ -7,6 +7,7 @@ import ( "context" "encoding/json" "fmt" + "os" "regexp" "sort" "strings" @@ -149,7 +150,12 @@ func Diff(current *api.FleetState, proposed *parser.ParsedRepo, teamFilters []st globalResult.Queries = diffQueries(current.GlobalQueries, proposed.Global.Queries) // Subtract baseline for global scope + if cfg.baseline != nil && cfg.baseline.Global == nil { + fmt.Fprintln(os.Stderr, "Warning: baseline has no global config, skipping global baseline subtraction") + } if cfg.baseline != nil && cfg.baseline.Global != nil { + fmt.Fprintf(os.Stderr, "Baseline global: %d policies, %d queries\n", + len(cfg.baseline.Global.Policies), len(cfg.baseline.Global.Queries)) var baseConfig []ConfigChange if current.Config != nil { baseConfig, _ = diffConfig(current.Config, cfg.baseline.Global) From fb3961ef6c7046097dbe067640a6391ab1de47f3 Mon Sep 17 00:00:00 2001 From: Dan Tsekhanskiy Date: Fri, 20 Mar 2026 13:24:33 -0400 Subject: [PATCH 08/12] debug: More baseline stderr output --- cmd/fleet-plan/main.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/cmd/fleet-plan/main.go b/cmd/fleet-plan/main.go index b285cf9..02f9f40 100644 --- a/cmd/fleet-plan/main.go +++ b/cmd/fleet-plan/main.go @@ -145,10 +145,20 @@ func runDiff(cmd *cobra.Command, _ []string) error { } else { defer baseCleanup() baseDefaultFile := resolveBaselineDefault(baseRoot, flagRepo, flagBase, flagEnv) + fmt.Fprintf(os.Stderr, "Baseline default file: %q\n", baseDefaultFile) baseParsed, err := parser.ParseRepo(baseRoot, teams, baseDefaultFile) if err != nil { fmt.Fprintf(os.Stderr, "Warning: could not parse baseline (%v), skipping baseline subtraction\n", err) } else { + if baseParsed.Global == nil { + fmt.Fprintf(os.Stderr, "Warning: baseline parsed but Global is nil (defaultFile=%q)\n", baseDefaultFile) + } else { + fmt.Fprintf(os.Stderr, "Baseline global parsed: %d policies, %d queries\n", + len(baseParsed.Global.Policies), len(baseParsed.Global.Queries)) + } + if len(baseParsed.Errors) > 0 { + fmt.Fprintf(os.Stderr, "Baseline parse errors: %v\n", baseParsed.Errors) + } baseline = baseParsed } } From e0f3f2d39e1efd635f5baa65476910ecd45b4a3e Mon Sep 17 00:00:00 2001 From: Dan Tsekhanskiy Date: Fri, 20 Mar 2026 13:27:32 -0400 Subject: [PATCH 09/12] fix: Create teams/ dir in baseline and remove debug output ParseRepo bails early if teams/ doesn't exist, preventing global config parsing. For global-only baselines (MR touches base.yml but no team files), the teams/ dir must exist as an empty directory. --- cmd/fleet-plan/main.go | 10 ---------- internal/diff/differ.go | 6 ------ internal/git/baseline.go | 3 +++ 3 files changed, 3 insertions(+), 16 deletions(-) diff --git a/cmd/fleet-plan/main.go b/cmd/fleet-plan/main.go index 02f9f40..b285cf9 100644 --- a/cmd/fleet-plan/main.go +++ b/cmd/fleet-plan/main.go @@ -145,20 +145,10 @@ func runDiff(cmd *cobra.Command, _ []string) error { } else { defer baseCleanup() baseDefaultFile := resolveBaselineDefault(baseRoot, flagRepo, flagBase, flagEnv) - fmt.Fprintf(os.Stderr, "Baseline default file: %q\n", baseDefaultFile) baseParsed, err := parser.ParseRepo(baseRoot, teams, baseDefaultFile) if err != nil { fmt.Fprintf(os.Stderr, "Warning: could not parse baseline (%v), skipping baseline subtraction\n", err) } else { - if baseParsed.Global == nil { - fmt.Fprintf(os.Stderr, "Warning: baseline parsed but Global is nil (defaultFile=%q)\n", baseDefaultFile) - } else { - fmt.Fprintf(os.Stderr, "Baseline global parsed: %d policies, %d queries\n", - len(baseParsed.Global.Policies), len(baseParsed.Global.Queries)) - } - if len(baseParsed.Errors) > 0 { - fmt.Fprintf(os.Stderr, "Baseline parse errors: %v\n", baseParsed.Errors) - } baseline = baseParsed } } diff --git a/internal/diff/differ.go b/internal/diff/differ.go index 10d3a4e..a0a8c0e 100644 --- a/internal/diff/differ.go +++ b/internal/diff/differ.go @@ -7,7 +7,6 @@ import ( "context" "encoding/json" "fmt" - "os" "regexp" "sort" "strings" @@ -150,12 +149,7 @@ func Diff(current *api.FleetState, proposed *parser.ParsedRepo, teamFilters []st globalResult.Queries = diffQueries(current.GlobalQueries, proposed.Global.Queries) // Subtract baseline for global scope - if cfg.baseline != nil && cfg.baseline.Global == nil { - fmt.Fprintln(os.Stderr, "Warning: baseline has no global config, skipping global baseline subtraction") - } if cfg.baseline != nil && cfg.baseline.Global != nil { - fmt.Fprintf(os.Stderr, "Baseline global: %d policies, %d queries\n", - len(cfg.baseline.Global.Policies), len(cfg.baseline.Global.Queries)) var baseConfig []ConfigChange if current.Config != nil { baseConfig, _ = diffConfig(current.Config, cfg.baseline.Global) diff --git a/internal/git/baseline.go b/internal/git/baseline.go index f49c4fc..860d57a 100644 --- a/internal/git/baseline.go +++ b/internal/git/baseline.go @@ -26,6 +26,9 @@ func CheckoutBaseline(repoRoot string, baseRef string, files []string) (tmpRoot } cleanup = func() { os.RemoveAll(tmpRoot) } + // Ensure teams/ directory exists so ParseRepo doesn't bail early. + os.MkdirAll(filepath.Join(tmpRoot, "teams"), 0o755) + // Resolve which files we need: the explicitly changed files, plus any // files they reference (path: directives in team YAML). We start with // the team files themselves, then do a second pass for references. From 7a6c22fdc8d8ed0e430cf8092600e1f7565e5489 Mon Sep 17 00:00:00 2001 From: Dan Tsekhanskiy Date: Fri, 20 Mar 2026 14:32:23 -0400 Subject: [PATCH 10/12] feat: Add --verbose baseline logging, stop extracting unrelated global config Two changes: 1. --verbose (-v) now logs baseline subtraction details to stderr: proposed/fleet counts, MR diff, baseline diff, after subtraction, and changedFiles filter results for each team and global scope. 2. collectBaselineFiles no longer unconditionally extracts base.yml and default.yml. Only files in the MR's changedFiles (plus their path: references) are extracted. This fixes over-subtraction for MRs that don't touch global config. Ref #25 --- cmd/fleet-plan/main.go | 2 +- internal/diff/differ.go | 84 +++++++++++++++++++++++++++++++++++ internal/git/baseline.go | 10 ++--- internal/git/baseline_test.go | 13 +++++- 4 files changed, 99 insertions(+), 10 deletions(-) diff --git a/cmd/fleet-plan/main.go b/cmd/fleet-plan/main.go index b285cf9..c507380 100644 --- a/cmd/fleet-plan/main.go +++ b/cmd/fleet-plan/main.go @@ -167,7 +167,7 @@ func runDiff(cmd *cobra.Command, _ []string) error { return err } - diffOpts := []diff.DiffOption{diff.WithScriptEnricher(client)} + diffOpts := []diff.DiffOption{diff.WithScriptEnricher(client), diff.WithVerbose(flagVerbose)} if baseline != nil { diffOpts = append(diffOpts, diff.WithBaseline(baseline)) } diff --git a/internal/diff/differ.go b/internal/diff/differ.go index a0a8c0e..de09b38 100644 --- a/internal/diff/differ.go +++ b/internal/diff/differ.go @@ -7,6 +7,7 @@ import ( "context" "encoding/json" "fmt" + "os" "regexp" "sort" "strings" @@ -101,6 +102,7 @@ type DiffOption func(*diffOptions) type diffOptions struct { enricher ScriptEnricher baseline *parser.ParsedRepo + verbose bool } // WithScriptEnricher enables script-level diffing for fleet-maintained apps. @@ -108,6 +110,11 @@ func WithScriptEnricher(e ScriptEnricher) DiffOption { return func(o *diffOptions) { o.enricher = e } } +// WithVerbose enables detailed stderr logging of baseline subtraction. +func WithVerbose(v bool) DiffOption { + return func(o *diffOptions) { o.verbose = v } +} + // WithBaseline provides a parsed base-branch repo. When set, Diff subtracts // changes that already exist between the base branch and Fleet (i.e. changes // merged to main but not yet deployed) so that only the incremental changes @@ -116,6 +123,36 @@ func WithBaseline(b *parser.ParsedRepo) DiffOption { return func(o *diffOptions) { o.baseline = b } } +// vlog writes to stderr when verbose mode is enabled. +func vlog(verbose bool, format string, args ...any) { + if verbose { + fmt.Fprintf(os.Stderr, "[verbose] "+format+"\n", args...) + } +} + +// rdSummary returns a one-line summary of a ResourceDiff. +func rdSummary(rd ResourceDiff) string { + return fmt.Sprintf("+%d ~%d -%d", len(rd.Added), len(rd.Modified), len(rd.Deleted)) +} + +// rdNames returns names of changes for debugging. +func rdNames(rd ResourceDiff) string { + var names []string + for _, c := range rd.Added { + names = append(names, "+"+c.Name) + } + for _, c := range rd.Modified { + names = append(names, "~"+c.Name) + } + for _, c := range rd.Deleted { + names = append(names, "-"+c.Name) + } + if len(names) == 0 { + return "(none)" + } + return strings.Join(names, ", ") +} + // Diff computes the diff between current Fleet state and proposed YAML for all // teams. If teamFilters is non-empty, only matching teams are diffed. // If changedFiles is non-empty, only resources whose SourceFile matches are @@ -134,8 +171,13 @@ func Diff(current *api.FleetState, proposed *parser.ParsedRepo, teamFilters []st labelMap[l.Name] = l } + vlog(cfg.verbose, "baseline=%v, baseline.Global=%v, teamFilters=%v, changedFiles=%v", + cfg.baseline != nil, cfg.baseline != nil && cfg.baseline.Global != nil, teamFilters, changedFiles) + // --- Global config diff (default.yml) --- if proposed.Global != nil && len(teamFilters) == 0 { + vlog(cfg.verbose, "(global) proposed: %d policies, %d queries", len(proposed.Global.Policies), len(proposed.Global.Queries)) + vlog(cfg.verbose, "(global) fleet: %d policies, %d queries", len(current.GlobalPolicies), len(current.GlobalQueries)) globalResult := DiffResult{Team: "(global)"} if current.Config != nil { @@ -148,8 +190,13 @@ func Diff(current *api.FleetState, proposed *parser.ParsedRepo, teamFilters []st // Diff global queries globalResult.Queries = diffQueries(current.GlobalQueries, proposed.Global.Queries) + vlog(cfg.verbose, "(global) MR diff: policies=%s queries=%s config=%d", + rdSummary(globalResult.Policies), rdSummary(globalResult.Queries), len(globalResult.Config)) + // Subtract baseline for global scope if cfg.baseline != nil && cfg.baseline.Global != nil { + vlog(cfg.verbose, "(global) baseline: %d policies, %d queries", + len(cfg.baseline.Global.Policies), len(cfg.baseline.Global.Queries)) var baseConfig []ConfigChange if current.Config != nil { baseConfig, _ = diffConfig(current.Config, cfg.baseline.Global) @@ -157,9 +204,15 @@ func Diff(current *api.FleetState, proposed *parser.ParsedRepo, teamFilters []st basePolicies := diffPolicies(current.GlobalPolicies, cfg.baseline.Global.Policies) baseQueries := diffQueries(current.GlobalQueries, cfg.baseline.Global.Queries) + vlog(cfg.verbose, "(global) baseline diff: policies=%s queries=%s config=%d", + rdSummary(basePolicies), rdSummary(baseQueries), len(baseConfig)) + globalResult.Config = subtractConfigChanges(globalResult.Config, baseConfig) globalResult.Policies = subtractResourceDiff(globalResult.Policies, basePolicies) globalResult.Queries = subtractResourceDiff(globalResult.Queries, baseQueries) + + vlog(cfg.verbose, "(global) after subtraction: policies=%s queries=%s config=%d", + rdSummary(globalResult.Policies), rdSummary(globalResult.Queries), len(globalResult.Config)) } results = append(results, globalResult) @@ -204,6 +257,11 @@ func Diff(current *api.FleetState, proposed *parser.ParsedRepo, teamFilters []st result.Errors = append(result.Errors, fmt.Sprintf("info: team %q does not exist in Fleet yet (will be created)", proposedTeam.Name)) } } else { + vlog(cfg.verbose, "[%s] proposed: %d policies, %d queries", proposedTeam.Name, + len(proposedTeam.Policies), len(proposedTeam.Queries)) + vlog(cfg.verbose, "[%s] fleet: %d policies, %d queries", proposedTeam.Name, + len(currentTeam.Policies), len(currentTeam.Queries)) + result.Policies = diffPolicies(currentTeam.Policies, proposedTeam.Policies) result.Queries = diffQueries(currentTeam.Queries, proposedTeam.Queries) @@ -244,10 +302,19 @@ func Diff(current *api.FleetState, proposed *parser.ParsedRepo, teamFilters []st result.Scripts = diffScripts(currentTeam.Scripts, proposedTeam.Scripts) } + vlog(cfg.verbose, "[%s] MR diff: policies=%s queries=%s software=%s scripts=%s", + proposedTeam.Name, rdSummary(result.Policies), rdSummary(result.Queries), + rdSummary(result.Software), rdSummary(result.Scripts)) + if cfg.verbose { + vlog(true, "[%s] MR queries: %s", proposedTeam.Name, rdNames(result.Queries)) + } + // Subtract baseline: remove changes that already exist between the // base branch and Fleet (merged but not yet deployed). if cfg.baseline != nil { if baseTeam, ok := findBaselineTeam(cfg.baseline, proposedTeam.Name); ok { + vlog(cfg.verbose, "[%s] baseline team found: %d policies, %d queries", + proposedTeam.Name, len(baseTeam.Policies), len(baseTeam.Queries)) baseDiff := DiffResult{} baseDiff.Policies = diffPolicies(currentTeam.Policies, baseTeam.Policies) baseDiff.Queries = diffQueries(currentTeam.Queries, baseTeam.Queries) @@ -260,22 +327,39 @@ func Diff(current *api.FleetState, proposed *parser.ParsedRepo, teamFilters []st if !currentTeam.ScriptsUnavailable { baseDiff.Scripts = diffScripts(currentTeam.Scripts, baseTeam.Scripts) } + vlog(cfg.verbose, "[%s] baseline diff: policies=%s queries=%s software=%s", + proposedTeam.Name, rdSummary(baseDiff.Policies), + rdSummary(baseDiff.Queries), rdSummary(baseDiff.Software)) + if cfg.verbose { + vlog(true, "[%s] baseline queries: %s", proposedTeam.Name, rdNames(baseDiff.Queries)) + } result.Policies = subtractResourceDiff(result.Policies, baseDiff.Policies) result.Queries = subtractResourceDiff(result.Queries, baseDiff.Queries) result.Software = subtractResourceDiff(result.Software, baseDiff.Software) result.Profiles = subtractResourceDiff(result.Profiles, baseDiff.Profiles) result.Scripts = subtractResourceDiff(result.Scripts, baseDiff.Scripts) + vlog(cfg.verbose, "[%s] after subtraction: policies=%s queries=%s software=%s", + proposedTeam.Name, rdSummary(result.Policies), + rdSummary(result.Queries), rdSummary(result.Software)) + } else { + vlog(cfg.verbose, "[%s] no baseline team found", proposedTeam.Name) } } } if len(changedFiles) > 0 { + vlog(cfg.verbose, "[%s] before changedFiles filter: policies=%s queries=%s software=%s", + proposedTeam.Name, rdSummary(result.Policies), rdSummary(result.Queries), + rdSummary(result.Software)) sourceNames := buildSourceMap(proposedTeam) result.Policies = filterResourceDiff(result.Policies, sourceNames, changedFiles) result.Queries = filterResourceDiff(result.Queries, sourceNames, changedFiles) result.Software = filterResourceDiff(result.Software, sourceNames, changedFiles) result.Profiles = filterResourceDiff(result.Profiles, sourceNames, changedFiles) result.Scripts = filterResourceDiff(result.Scripts, sourceNames, changedFiles) + vlog(cfg.verbose, "[%s] after changedFiles filter: policies=%s queries=%s software=%s", + proposedTeam.Name, rdSummary(result.Policies), rdSummary(result.Queries), + rdSummary(result.Software)) } result.Labels = validateLabels(proposedTeam, labelMap, changedNames(result.Policies)) diff --git a/internal/git/baseline.go b/internal/git/baseline.go index 860d57a..2a04b2e 100644 --- a/internal/git/baseline.go +++ b/internal/git/baseline.go @@ -104,13 +104,9 @@ func collectBaselineFiles(repoRoot, baseRef string, changedFiles []string) []str } } - // Also include default.yml and base.yml if they exist, since the parser - // may need them for global config context. - for _, f := range []string{"default.yml", "base.yml"} { - if _, err := gitShow(repoRoot, baseRef, f); err == nil { - add(f) - } - } + // Only include default.yml/base.yml if they're already in changedFiles. + // Extracting them unconditionally causes over-subtraction for MRs that + // don't touch global config. return result } diff --git a/internal/git/baseline_test.go b/internal/git/baseline_test.go index 02f5714..ca01669 100644 --- a/internal/git/baseline_test.go +++ b/internal/git/baseline_test.go @@ -125,7 +125,7 @@ func TestCheckoutBaseline_ExtractsFiles(t *testing.T) { } } -func TestCollectBaselineFiles_IncludesDefaultYml(t *testing.T) { +func TestCollectBaselineFiles_OnlyIncludesChangedFiles(t *testing.T) { t.Parallel() dir := t.TempDir() @@ -140,8 +140,17 @@ func TestCollectBaselineFiles_IncludesDefaultYml(t *testing.T) { gitRun(t, dir, "commit", "-m", "init") sha := gitOutput(t, dir, "rev-parse", "HEAD") + + // default.yml not in changedFiles, should NOT be collected. files := collectBaselineFiles(dir, sha, []string{"teams/test.yml"}) + for _, f := range files { + if f == "default.yml" { + t.Errorf("default.yml should not be collected when not in changedFiles, got: %v", files) + } + } + // default.yml in changedFiles, should be collected. + files = collectBaselineFiles(dir, sha, []string{"default.yml"}) found := false for _, f := range files { if f == "default.yml" { @@ -149,7 +158,7 @@ func TestCollectBaselineFiles_IncludesDefaultYml(t *testing.T) { } } if !found { - t.Errorf("expected default.yml in collected files, got: %v", files) + t.Errorf("expected default.yml when in changedFiles, got: %v", files) } } From d0ee8d90d7ed08c668535faaf52656cdd70dc7a0 Mon Sep 17 00:00:00 2001 From: Dan Tsekhanskiy Date: Fri, 20 Mar 2026 15:29:05 -0400 Subject: [PATCH 11/12] fix: Place merged default.yml in repo dir, add base.yml to changedFiles Two pre-existing bugs found via --verbose: 1. resolveDefaultFile placed the merged base.yml+env overlay in /tmp. The parser resolves path: refs relative to the file's directory, so refs like ./queries/... resolved to /tmp/queries/... which doesn't exist. Now places the merged file inside the repo dir. 2. ResolveScope set IncludeGlobal for base.yml changes but didn't add base.yml to scope.ChangedFiles. The baseline extraction and changedFiles filter couldn't see it. Ref #25 --- cmd/fleet-plan/main.go | 4 +++- internal/git/scope.go | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/cmd/fleet-plan/main.go b/cmd/fleet-plan/main.go index c507380..9df73b4 100644 --- a/cmd/fleet-plan/main.go +++ b/cmd/fleet-plan/main.go @@ -240,7 +240,9 @@ func resolveDefaultFile(repo, base, env string) (path string, cleanup func(), er env = filepath.Join(repo, env) } - tmp, err := os.CreateTemp("", "fleet-plan-default-*.yml") + // Place the merged file inside the repo so the parser resolves path: + // references (./queries/...) relative to the repo root, not /tmp. + tmp, err := os.CreateTemp(repo, ".fleet-plan-default-*.yml") if err != nil { return "", nil, fmt.Errorf("creating temp file for merged config: %w", err) } diff --git a/internal/git/scope.go b/internal/git/scope.go index 9c06111..117e6bc 100644 --- a/internal/git/scope.go +++ b/internal/git/scope.go @@ -57,7 +57,7 @@ func ResolveScope(root string, changedFiles []string, envFile string) Scope { } } - if isFleetResourceOrTeam(f) { + if isFleetResourceOrTeam(f) || f == "base.yml" || f == "default.yml" { scope.ChangedFiles = append(scope.ChangedFiles, f) } } From ea11c2cd657134a599ec01b190c514bc9af2a659 Mon Sep 17 00:00:00 2001 From: Dan Tsekhanskiy Date: Fri, 20 Mar 2026 15:44:57 -0400 Subject: [PATCH 12/12] fix: Include global diff when IncludeGlobal is set with team filters When --git mode detects both global (base.yml) and team changes, the auto-detected team filter suppressed the global diff block. Now WithIncludeGlobal overrides the teamFilters check. --- cmd/fleet-plan/main.go | 4 +++- internal/diff/differ.go | 15 +++++++++++---- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/cmd/fleet-plan/main.go b/cmd/fleet-plan/main.go index 9df73b4..f79eac1 100644 --- a/cmd/fleet-plan/main.go +++ b/cmd/fleet-plan/main.go @@ -110,6 +110,7 @@ func runDiff(cmd *cobra.Command, _ []string) error { // --git: detect CI context, resolve changed files + affected teams. var changedFiles []string var ci git.Env + var includeGlobal bool teams := flagTeams if flagGit { @@ -119,6 +120,7 @@ func runDiff(cmd *cobra.Command, _ []string) error { return nil } changedFiles = resolved.ChangedFiles + includeGlobal = resolved.IncludeGlobal if len(resolved.Teams) > 0 && len(teams) == 0 { teams = resolved.Teams } @@ -167,7 +169,7 @@ func runDiff(cmd *cobra.Command, _ []string) error { return err } - diffOpts := []diff.DiffOption{diff.WithScriptEnricher(client), diff.WithVerbose(flagVerbose)} + diffOpts := []diff.DiffOption{diff.WithScriptEnricher(client), diff.WithVerbose(flagVerbose), diff.WithIncludeGlobal(includeGlobal)} if baseline != nil { diffOpts = append(diffOpts, diff.WithBaseline(baseline)) } diff --git a/internal/diff/differ.go b/internal/diff/differ.go index de09b38..47464b1 100644 --- a/internal/diff/differ.go +++ b/internal/diff/differ.go @@ -100,9 +100,10 @@ type ScriptEnricher interface { type DiffOption func(*diffOptions) type diffOptions struct { - enricher ScriptEnricher - baseline *parser.ParsedRepo - verbose bool + enricher ScriptEnricher + baseline *parser.ParsedRepo + verbose bool + includeGlobal bool } // WithScriptEnricher enables script-level diffing for fleet-maintained apps. @@ -115,6 +116,12 @@ func WithVerbose(v bool) DiffOption { return func(o *diffOptions) { o.verbose = v } } +// WithIncludeGlobal forces global config diffing even when team filters are +// set. Used in --git mode when both global and team files changed. +func WithIncludeGlobal(v bool) DiffOption { + return func(o *diffOptions) { o.includeGlobal = v } +} + // WithBaseline provides a parsed base-branch repo. When set, Diff subtracts // changes that already exist between the base branch and Fleet (i.e. changes // merged to main but not yet deployed) so that only the incremental changes @@ -175,7 +182,7 @@ func Diff(current *api.FleetState, proposed *parser.ParsedRepo, teamFilters []st cfg.baseline != nil, cfg.baseline != nil && cfg.baseline.Global != nil, teamFilters, changedFiles) // --- Global config diff (default.yml) --- - if proposed.Global != nil && len(teamFilters) == 0 { + if proposed.Global != nil && (len(teamFilters) == 0 || cfg.includeGlobal) { vlog(cfg.verbose, "(global) proposed: %d policies, %d queries", len(proposed.Global.Policies), len(proposed.Global.Queries)) vlog(cfg.verbose, "(global) fleet: %d policies, %d queries", len(current.GlobalPolicies), len(current.GlobalQueries)) globalResult := DiffResult{Team: "(global)"}