Skip to content

Commit 5dd560f

Browse files
robbiet480claude
andcommitted
fix(diff): subtract baseline for no-team; report skipped no-team queries
Two review findings on #56: - The no-team diff ignored the baseline, so a policy, profile, or script change that was merged to the base branch but not yet deployed was reported again on every later MR. It is now subtracted like any other team's. The baseline's no-team file is matched on no-team identity rather than display name, since the base and MR branches can spell it differently ("No team" vs "Unassigned") -- exactly what happens in the MR that migrates a repo from the teams/ layout to fleets/. - `queries:` in a no-team file were dropped silently. Fleet scopes queries to a real team or to the global scope, so they cannot be diffed there; the plan now says so, matching how skipped software is reported. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEpzMNJnGaBLAfrPeqknCy
1 parent 4869ac1 commit 5dd560f

3 files changed

Lines changed: 118 additions & 1 deletion

File tree

docs/Architecture.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ Walks `teams/*.yml`, resolves `path:` references, produces `ParsedRepo`. Also pa
8989

9090
Compares `FleetState` (API) vs `ParsedRepo` (YAML). Produces `[]DiffResult` per team + a `(global)` result when `default.yml` is present.
9191

92-
Fleet's "hosts on no team" bucket is absent from `GET /teams`, so it is fetched separately (`team_id=0`) and diffed like any other team for policies, profiles, and scripts. Software is reported as skipped there: Fleet exposes configured software only through the teams list. When the bucket was not fetched, the diff falls back to summarizing what the repo configures for it.
92+
Fleet's "hosts on no team" bucket is absent from `GET /teams`, so it is fetched separately (`team_id=0`) and diffed like any other team for policies, profiles, and scripts, baseline subtraction included. Software and queries are reported as skipped there: Fleet exposes configured software only through the teams list, and scopes queries to a real team or the global scope. When the bucket was not fetched, the diff falls back to summarizing what the repo configures for it.
9393

9494
| Resource | Match key | Diff fields |
9595
|----------|-----------|-------------|

internal/diff/differ.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,40 @@ func diffNoTeam(result *DiffResult, current *api.NoTeam, proposed parser.ParsedT
216216
fmt.Sprintf("software diff skipped: %d software items configured, but Fleet does not report software for hosts on no team", n))
217217
}
218218

219+
// Fleet scopes queries to a real team or to the global scope, so a
220+
// no-team file cannot own them. The parser still accepts `queries:` and
221+
// `reports:` in any team file, so say plainly that these are not diffed
222+
// instead of dropping them without a word.
223+
if n := len(proposed.Queries); n > 0 {
224+
result.Errors = append(result.Errors,
225+
fmt.Sprintf("queries diff skipped: %d queries configured, but Fleet has no query scope for hosts on no team", n))
226+
}
227+
228+
// Subtract changes that already exist between the base branch and Fleet,
229+
// so a no-team change that is merged but not yet deployed is not reported
230+
// again on every later MR.
231+
if cfg.baseline != nil {
232+
if baseTeam, ok := findBaselineNoTeam(cfg.baseline); ok {
233+
base := DiffResult{}
234+
if !current.PoliciesUnavailable {
235+
base.Policies = diffPolicies(current.Policies, baseTeam.Policies)
236+
}
237+
if !current.ProfilesUnavailable {
238+
base.Profiles, _ = diffProfiles(current.Profiles, baseTeam.Profiles, nil)
239+
}
240+
if !current.ScriptsUnavailable {
241+
base.Scripts = diffScripts(current.Scripts, baseTeam.Scripts)
242+
}
243+
result.Policies = subtractResourceDiff(result.Policies, base.Policies)
244+
result.Profiles = subtractResourceDiff(result.Profiles, base.Profiles)
245+
result.Scripts = subtractResourceDiff(result.Scripts, base.Scripts)
246+
vlog(cfg.verbose, "[%s] after baseline subtraction: policies=%s profiles=%s scripts=%s",
247+
proposed.Name, rdSummary(result.Policies), rdSummary(result.Profiles), rdSummary(result.Scripts))
248+
} else {
249+
vlog(cfg.verbose, "[%s] no baseline no-team file found", proposed.Name)
250+
}
251+
}
252+
219253
vlog(cfg.verbose, "[%s] no-team diff: policies=%s profiles=%s scripts=%s",
220254
proposed.Name, rdSummary(result.Policies), rdSummary(result.Profiles), rdSummary(result.Scripts))
221255
}
@@ -537,6 +571,19 @@ func filterChanges(changes []ResourceChange, keep func(string) bool) []ResourceC
537571
// ---------- Baseline subtraction ----------
538572

539573
// findBaselineTeam looks up a team by name in the baseline parsed repo.
574+
// findBaselineNoTeam returns the baseline's no-team file. It matches on the
575+
// no-team identity rather than the display name, because the base branch and
576+
// the MR branch may spell it differently ("No team" vs "Unassigned") -- for
577+
// instance in the MR that migrates a repo from the teams/ layout to fleets/.
578+
func findBaselineNoTeam(baseline *parser.ParsedRepo) (parser.ParsedTeam, bool) {
579+
for _, t := range baseline.Teams {
580+
if parser.IsNoTeam(t.Name, t.SourceFile) {
581+
return t, true
582+
}
583+
}
584+
return parser.ParsedTeam{}, false
585+
}
586+
540587
func findBaselineTeam(baseline *parser.ParsedRepo, name string) (parser.ParsedTeam, bool) {
541588
for _, t := range baseline.Teams {
542589
if strings.EqualFold(t.Name, name) {

internal/diff/differ_test.go

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2491,3 +2491,73 @@ func TestDiffNoTeamSoftwareIsReportedAsSkipped(t *testing.T) {
24912491
t.Errorf("missing software skip note in %v", r.Errors)
24922492
}
24932493
}
2494+
2495+
// A no-team change that is already merged to the base branch but not yet
2496+
// deployed must not be reported again on every later MR.
2497+
func TestDiffNoTeamBaselineSubtraction(t *testing.T) {
2498+
current := &api.FleetState{
2499+
Teams: []api.Team{},
2500+
Labels: []api.Label{},
2501+
NoTeam: &api.NoTeam{
2502+
Policies: []api.Policy{{Name: "Existing", Query: "SELECT 1;"}},
2503+
Scripts: []api.Script{{ID: 1, Name: "keep.sh", Content: "echo one\n"}},
2504+
},
2505+
}
2506+
2507+
// Already on the base branch: the added policy and the edited script.
2508+
baseline := &parser.ParsedRepo{Teams: []parser.ParsedTeam{{
2509+
Name: "No team",
2510+
SourceFile: "teams/no-team.yml",
2511+
Policies: []parser.ParsedPolicy{
2512+
{Name: "Existing", Query: "SELECT 1;"},
2513+
{Name: "Merged not deployed", Query: "SELECT 2;"},
2514+
},
2515+
Scripts: []parser.ParsedScript{{Name: "keep.sh", Content: "echo one\necho two\n"}},
2516+
}}}
2517+
2518+
// The MR adds one more policy on top of the base branch's state. The
2519+
// branch spells the bucket differently, which must not defeat matching.
2520+
proposed := &parser.ParsedRepo{Teams: []parser.ParsedTeam{{
2521+
Name: "Unassigned",
2522+
SourceFile: "fleets/unassigned.yml",
2523+
Policies: []parser.ParsedPolicy{
2524+
{Name: "Existing", Query: "SELECT 1;"},
2525+
{Name: "Merged not deployed", Query: "SELECT 2;"},
2526+
{Name: "New in this MR", Query: "SELECT 3;"},
2527+
},
2528+
Scripts: []parser.ParsedScript{{Name: "keep.sh", Content: "echo one\necho two\n"}},
2529+
}}}
2530+
2531+
r := Diff(current, proposed, nil, nil, WithBaseline(baseline))[0]
2532+
2533+
if len(r.Policies.Added) != 1 || r.Policies.Added[0].Name != "New in this MR" {
2534+
t.Errorf("policies added: got %+v, want only the MR's own addition", r.Policies.Added)
2535+
}
2536+
if !r.Scripts.IsEmpty() {
2537+
t.Errorf("scripts: got %+v, want empty (the edit is already on the base branch)", r.Scripts)
2538+
}
2539+
}
2540+
2541+
func TestDiffNoTeamQueriesAreReportedAsSkipped(t *testing.T) {
2542+
current := &api.FleetState{Teams: []api.Team{}, Labels: []api.Label{}, NoTeam: &api.NoTeam{}}
2543+
proposed := &parser.ParsedRepo{Teams: []parser.ParsedTeam{{
2544+
Name: "No team",
2545+
SourceFile: "teams/no-team.yml",
2546+
Queries: []parser.ParsedQuery{{Name: "Q1"}, {Name: "Q2"}},
2547+
}}}
2548+
2549+
r := Diff(current, proposed, nil, nil)[0]
2550+
2551+
if !r.Queries.IsEmpty() {
2552+
t.Errorf("queries: got %+v, want empty (Fleet has no no-team query scope)", r.Queries)
2553+
}
2554+
found := false
2555+
for _, e := range r.Errors {
2556+
if strings.Contains(e, "queries diff skipped: 2 queries configured") {
2557+
found = true
2558+
}
2559+
}
2560+
if !found {
2561+
t.Errorf("missing query skip note in %v", r.Errors)
2562+
}
2563+
}

0 commit comments

Comments
 (0)