diff --git a/cmd/fleet-plan/cmd_test.go b/cmd/fleet-plan/cmd_test.go index 17b2df0..ed67bd7 100644 --- a/cmd/fleet-plan/cmd_test.go +++ b/cmd/fleet-plan/cmd_test.go @@ -13,6 +13,7 @@ import ( "testing" "github.com/CampusTech/fleet-plan/internal/git" + "github.com/CampusTech/fleet-plan/internal/parser" ) // ---------- version command ---------- @@ -755,3 +756,38 @@ func TestRunExitCodes(t *testing.T) { }) } } + +func TestHasNoTeam(t *testing.T) { + tests := []struct { + name string + teams []parser.ParsedTeam + want bool + }{ + {name: "no teams at all"}, + { + name: "ordinary teams only", + teams: []parser.ParsedTeam{{Name: "Workstations", SourceFile: "teams/workstations.yml"}}, + }, + { + name: "teams layout no-team file", + teams: []parser.ParsedTeam{ + {Name: "Workstations", SourceFile: "teams/workstations.yml"}, + {Name: "No team", SourceFile: "teams/no-team.yml"}, + }, + want: true, + }, + { + name: "fleets layout unassigned file", + teams: []parser.ParsedTeam{{Name: "Unassigned", SourceFile: "fleets/unassigned.yml"}}, + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := hasNoTeam(tt.teams); got != tt.want { + t.Errorf("got %v, want %v", got, tt.want) + } + }) + } +} diff --git a/cmd/fleet-plan/main.go b/cmd/fleet-plan/main.go index ae521e7..e66d560 100644 --- a/cmd/fleet-plan/main.go +++ b/cmd/fleet-plan/main.go @@ -168,7 +168,10 @@ func runDiff(cmd *cobra.Command, _ []string) error { } fmt.Fprintf(os.Stderr, "Fetching Fleet state from %s...\n", auth.URL) - state, err := client.FetchAll(ctx, repo.Global != nil) + state, err := client.FetchAll(ctx, api.FetchOptions{ + Global: repo.Global != nil, + NoTeam: hasNoTeam(repo.Teams), + }) if err != nil { return err } @@ -228,6 +231,18 @@ func runDiff(cmd *cobra.Command, _ []string) error { return nil } +// hasNoTeam reports whether the repo configures Fleet's "hosts on no team" +// bucket. Fetching that bucket costs extra API calls, so it is only requested +// when a file describes it. +func hasNoTeam(teams []parser.ParsedTeam) bool { + for _, t := range teams { + if parser.IsNoTeam(t.Name, t.SourceFile) { + return true + } + } + return false +} + // errChangesDetected signals --detailed-exitcodes exit status 2. It is not a // failure: main translates it to the exit code after runDiff's defers run. var errChangesDetected = errors.New("changes detected") diff --git a/docs/API-Endpoints.md b/docs/API-Endpoints.md index 2b683ba..b3c47da 100644 --- a/docs/API-Endpoints.md +++ b/docs/API-Endpoints.md @@ -9,6 +9,7 @@ All read-only. fleet-plan never writes to your Fleet server. | `GET` | `/api/v1/fleet/labels` | Label validation and host counts | | `GET` | `/api/v1/fleet/teams/{id}/policies` | Per-team policies | | `GET` | `/api/v1/fleet/global/policies` | Global policies (when default.yml parsed) | +| `GET` | `/api/v1/fleet/teams/0/policies` | Policies for hosts on no team (when the repo has a no-team file) | | `GET` | `/api/v1/fleet/queries` | Per-team and global queries | | `GET` | `/api/v1/fleet/configuration_profiles` | MDM configuration profiles | | `GET` | `/api/v1/fleet/software/titles` | Managed software titles (paginated) | @@ -19,4 +20,6 @@ All read-only. fleet-plan never writes to your Fleet server. Global endpoints (`/config`, `/global/policies`, `/queries` with teamID=0) are only called when `default.yml` defines global sections. +The "hosts on no team" bucket is fetched only when the repo has a no-team file (`teams/no-team.yml` or `fleets/unassigned.yml`). Its resources live behind `team_id=0` on `/configuration_profiles` and `/scripts`, and behind `/teams/0/policies` for policies — note that `/global/policies` is a *different* set. Fleet does not report configured software for this bucket, so software is not diffed there. + HTTPS enforced unless `FLEET_PLAN_INSECURE=1`. diff --git a/docs/Architecture.md b/docs/Architecture.md index 90ff4dc..841d322 100644 --- a/docs/Architecture.md +++ b/docs/Architecture.md @@ -89,6 +89,8 @@ Walks `teams/*.yml`, resolves `path:` references, produces `ParsedRepo`. Also pa Compares `FleetState` (API) vs `ParsedRepo` (YAML). Produces `[]DiffResult` per team + a `(global)` result when `default.yml` is present. +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. + | Resource | Match key | Diff fields | |----------|-----------|-------------| | Config sections (global) | dot-path key | old/new value (skips `$VAR` placeholders) | diff --git a/internal/api/client.go b/internal/api/client.go index bcc3760..bb02019 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -122,6 +122,32 @@ type FleetState struct { Config map[string]any // from GET /api/v1/fleet/config GlobalPolicies []Policy // from GET /api/v1/fleet/global/policies (teamID=0) GlobalQueries []Query // from GET /api/v1/fleet/queries (teamID=0) + NoTeam *NoTeam // "hosts on no team" bucket, when requested +} + +// NoTeam holds the current state of Fleet's "hosts on no team" bucket, which +// the /teams endpoint does not return. Its resources live behind team_id=0. +// +// Queries are absent by design: Fleet scopes queries to a real team or to the +// global scope, so a no-team file cannot define them. Managed software is +// absent too — Fleet only reports configured packages through the teams list, +// which excludes this bucket. +type NoTeam struct { + Policies []Policy + Profiles []Profile + Scripts []Script + PoliciesUnavailable bool + ProfilesUnavailable bool + ScriptsUnavailable bool +} + +// FetchOptions selects the optional scopes FetchAll retrieves. Both cost extra +// round trips, so callers ask for them only when the repo defines them. +type FetchOptions struct { + // Global fetches /config, /global/policies, and global queries. + Global bool + // NoTeam fetches the "hosts on no team" bucket (team_id=0). + NoTeam bool } // Team represents a Fleet team with its associated resources. @@ -415,6 +441,35 @@ func (c *Client) GetPolicies(ctx context.Context, teamID uint) ([]Policy, error) return all, nil } +// GetNoTeamPolicies fetches the policies that belong to Fleet's "hosts on no +// team" bucket. This is a different endpoint from GetPolicies(0), which +// returns global policies. The response also carries inherited_policies (the +// global ones, which apply to no-team hosts as well); those are deliberately +// ignored, since a no-team YAML file does not own them. +func (c *Client) GetNoTeamPolicies(ctx context.Context) ([]Policy, error) { + var all []Policy + page := 0 + for { + q := url.Values{ + "per_page": {"250"}, + "page": {strconv.Itoa(page)}, + } + var resp policiesResponse + if err := c.get(ctx, "/api/v1/fleet/teams/0/policies", q, &resp); err != nil { + return nil, fmt.Errorf("fetching no-team policies: %w", err) + } + all = append(all, resp.Policies...) + if len(resp.Policies) < 250 { + break + } + page++ + if page > 100 { // safety: max 25k policies + break + } + } + return all, nil +} + // GetQueries fetches queries, optionally filtered by team, with pagination. func (c *Client) GetQueries(ctx context.Context, teamID uint) ([]Query, error) { var all []Query @@ -443,7 +498,9 @@ func (c *Client) GetQueries(ctx context.Context, teamID uint) ([]Query, error) { return all, nil } -// GetSoftware fetches managed (available_for_install) software titles for a team. +// GetSoftware fetches managed (available_for_install) software titles for a +// team. team_id is always sent; see GetProfiles for why teamID 0 must not be +// omitted. // Uses available_for_install=true to exclude detected-only titles (OS packages, // browser extensions, etc.) and only return software deployed via Fleet/GitOps. // Paginates to collect all results. @@ -456,9 +513,7 @@ func (c *Client) GetSoftware(ctx context.Context, teamID uint) ([]SoftwareTitle, "page": {strconv.Itoa(page)}, "available_for_install": {"true"}, } - if teamID > 0 { - q.Set("team_id", strconv.FormatUint(uint64(teamID), 10)) - } + q.Set("team_id", strconv.FormatUint(uint64(teamID), 10)) var resp softwareResponse if err := c.get(ctx, "/api/v1/fleet/software/titles", q, &resp); err != nil { return nil, fmt.Errorf("fetching software (team %d): %w", teamID, err) @@ -570,7 +625,9 @@ func (c *Client) GetLabels(ctx context.Context) ([]Label, error) { return all, nil } -// GetProfiles fetches MDM profiles for a team with pagination. +// GetProfiles fetches MDM profiles for a team with pagination. team_id is +// always sent: Fleet reads teamID 0 as the "hosts on no team" bucket, whereas +// omitting the parameter returns every team's profiles. func (c *Client) GetProfiles(ctx context.Context, teamID uint) ([]Profile, error) { var all []Profile page := 0 @@ -579,9 +636,7 @@ func (c *Client) GetProfiles(ctx context.Context, teamID uint) ([]Profile, error "per_page": {"250"}, "page": {strconv.Itoa(page)}, } - if teamID > 0 { - q.Set("team_id", strconv.FormatUint(uint64(teamID), 10)) - } + q.Set("team_id", strconv.FormatUint(uint64(teamID), 10)) var resp profilesResponse if err := c.get(ctx, "/api/v1/fleet/configuration_profiles", q, &resp); err != nil { return nil, fmt.Errorf("fetching profiles (team %d): %w", teamID, err) @@ -598,7 +653,8 @@ func (c *Client) GetProfiles(ctx context.Context, teamID uint) ([]Profile, error return all, nil } -// GetScripts fetches scripts for a team with pagination. +// GetScripts fetches scripts for a team with pagination. team_id is always +// sent; see GetProfiles for why teamID 0 must not be omitted. func (c *Client) GetScripts(ctx context.Context, teamID uint) ([]Script, error) { var all []Script page := 0 @@ -607,9 +663,7 @@ func (c *Client) GetScripts(ctx context.Context, teamID uint) ([]Script, error) "per_page": {"250"}, "page": {strconv.Itoa(page)}, } - if teamID > 0 { - q.Set("team_id", strconv.FormatUint(uint64(teamID), 10)) - } + q.Set("team_id", strconv.FormatUint(uint64(teamID), 10)) var resp scriptsResponse if err := c.get(ctx, "/api/v1/fleet/scripts", q, &resp); err != nil { return nil, fmt.Errorf("fetching scripts (team %d): %w", teamID, err) @@ -680,9 +734,13 @@ func (c *Client) getScriptContent(ctx context.Context, scriptID uint) (string, e // FetchAll concurrently fetches the complete Fleet state. Uses errgroup for // parallel requests. If fetchGlobal is true, also fetches global config, // policies, and queries (for default.yml diffing). -func (c *Client) FetchAll(ctx context.Context, fetchGlobal ...bool) (*FleetState, error) { +func (c *Client) FetchAll(ctx context.Context, opts ...FetchOptions) (*FleetState, error) { state := &FleetState{} - wantGlobal := len(fetchGlobal) > 0 && fetchGlobal[0] + var o FetchOptions + if len(opts) > 0 { + o = opts[0] + } + wantGlobal := o.Global teams, err := c.GetTeams(ctx) if err != nil { @@ -743,6 +801,49 @@ func (c *Client) FetchAll(ctx context.Context, fetchGlobal ...bool) (*FleetState }) } + // noTeam is written only by the goroutines below, then attached to state + // after g.Wait(). + var noTeam *NoTeam + if o.NoTeam { + noTeam = &NoTeam{} + g.Go(func() error { + policies, err := c.GetNoTeamPolicies(gctx) + if err != nil { + if !isPermissionError(err) { + return err + } + noTeam.PoliciesUnavailable = true + return nil + } + noTeam.Policies = policies + return nil + }) + g.Go(func() error { + profiles, err := c.GetProfiles(gctx, 0) + if err != nil { + if !isPermissionError(err) { + return err + } + noTeam.ProfilesUnavailable = true + return nil + } + noTeam.Profiles = profiles + return nil + }) + g.Go(func() error { + scripts, err := c.GetScripts(gctx, 0) + if err != nil { + if !isPermissionError(err) { + return err + } + noTeam.ScriptsUnavailable = true + return nil + } + noTeam.Scripts = scripts + return nil + }) + } + // teamPartials holds per-goroutine results indexed by team slot. // Each field is written by exactly one goroutine, so there is no data race. type teamPartial struct { @@ -845,6 +946,13 @@ func (c *Client) FetchAll(ctx context.Context, fetchGlobal ...bool) (*FleetState teamResults[i].ScriptsUnavailable = p.scriptsUnavailable } + if noTeam != nil { + if !noTeam.ScriptsUnavailable && len(noTeam.Scripts) > 0 { + c.EnrichScriptContents(ctx, noTeam.Scripts) + } + state.NoTeam = noTeam + } + // Enrich script contents (second pass, needs script IDs from first pass) for i := range teamResults { if !teamResults[i].ScriptsUnavailable && len(teamResults[i].Scripts) > 0 { diff --git a/internal/api/client_test.go b/internal/api/client_test.go index 6952b0f..3955fc3 100644 --- a/internal/api/client_test.go +++ b/internal/api/client_test.go @@ -6,6 +6,7 @@ import ( "fmt" "net/http" "net/http/httptest" + "strings" "testing" ) @@ -368,7 +369,10 @@ func TestGetProfilesTeamID(t *testing.T) { teamID uint wantTeamID string }{ - {name: "global profiles", teamID: 0, wantTeamID: ""}, + // teamID 0 is Fleet's "hosts on no team" bucket, so the parameter + // must be sent explicitly. Omitting it would return every team's + // profiles instead. + {name: "no-team profiles", teamID: 0, wantTeamID: "0"}, {name: "team 5 profiles", teamID: 5, wantTeamID: "5"}, } @@ -595,7 +599,7 @@ func TestFetchAllWithGlobal(t *testing.T) { defer ts.Close() c := testClient(t, ts, "tok") - state, err := c.FetchAll(context.Background(), true) + state, err := c.FetchAll(context.Background(), FetchOptions{Global: true}) if err != nil { t.Fatalf("FetchAll: %v", err) } @@ -1091,3 +1095,214 @@ func TestFetchAllFleetMaintainedFallback(t *testing.T) { }) } } + +// ---------- no-team bucket ---------- + +func TestGetNoTeamPolicies(t *testing.T) { + // /teams/0/policies returns the bucket's own policies plus the global ones + // it inherits. Only the former belong to a no-team YAML file. + const body = `{ + "policies": [{"id": 1, "name": "No-team policy", "query": "SELECT 1;"}], + "inherited_policies": [{"id": 99, "name": "Global policy", "query": "SELECT 2;"}] + }` + + var gotPath string + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.Path + fmt.Fprint(w, body) + })) + defer ts.Close() + + policies, err := testClient(t, ts, "tok").GetNoTeamPolicies(context.Background()) + if err != nil { + t.Fatalf("GetNoTeamPolicies: %v", err) + } + if gotPath != "/api/v1/fleet/teams/0/policies" { + t.Errorf("path: got %q", gotPath) + } + if len(policies) != 1 || policies[0].Name != "No-team policy" { + t.Fatalf("policies: got %+v, want only the no-team policy", policies) + } +} + +func TestFetchAllNoTeam(t *testing.T) { + var noTeamParams struct{ profiles, scripts string } + mux := http.NewServeMux() + mux.HandleFunc("/api/v1/fleet/teams", func(w http.ResponseWriter, _ *http.Request) { + fmt.Fprint(w, `{"teams":[]}`) + }) + mux.HandleFunc("/api/v1/fleet/labels", func(w http.ResponseWriter, _ *http.Request) { + fmt.Fprint(w, `{"labels":[]}`) + }) + mux.HandleFunc("/api/v1/fleet/software/fleet_maintained_apps", func(w http.ResponseWriter, _ *http.Request) { + fmt.Fprint(w, `{"fleet_maintained_apps":[]}`) + }) + mux.HandleFunc("/api/v1/fleet/teams/0/policies", func(w http.ResponseWriter, _ *http.Request) { + fmt.Fprint(w, `{"policies":[{"id":1,"name":"No-team policy"}],"inherited_policies":[{"id":9,"name":"Global"}]}`) + }) + mux.HandleFunc("/api/v1/fleet/configuration_profiles", func(w http.ResponseWriter, r *http.Request) { + noTeamParams.profiles = r.URL.Query().Get("team_id") + fmt.Fprint(w, `{"profiles":[{"profile_uuid":"u1","name":"Conditional access"}]}`) + }) + mux.HandleFunc("/api/v1/fleet/scripts", func(w http.ResponseWriter, r *http.Request) { + noTeamParams.scripts = r.URL.Query().Get("team_id") + fmt.Fprint(w, `{"scripts":[{"id":0,"name":"uninstall.sh"}]}`) + }) + + ts := httptest.NewServer(mux) + defer ts.Close() + + t.Run("requested", func(t *testing.T) { + state, err := testClient(t, ts, "tok").FetchAll(context.Background(), FetchOptions{NoTeam: true}) + if err != nil { + t.Fatalf("FetchAll: %v", err) + } + if state.NoTeam == nil { + t.Fatal("NoTeam: got nil, want the fetched bucket") + } + if len(state.NoTeam.Policies) != 1 || state.NoTeam.Policies[0].Name != "No-team policy" { + t.Errorf("policies: got %+v", state.NoTeam.Policies) + } + if len(state.NoTeam.Profiles) != 1 || len(state.NoTeam.Scripts) != 1 { + t.Errorf("profiles=%d scripts=%d, want 1 each", len(state.NoTeam.Profiles), len(state.NoTeam.Scripts)) + } + // team_id=0 must be sent explicitly; omitting it returns every team's + // resources instead of the no-team bucket's. + if noTeamParams.profiles != "0" || noTeamParams.scripts != "0" { + t.Errorf("team_id params: profiles=%q scripts=%q, want 0 for both", + noTeamParams.profiles, noTeamParams.scripts) + } + }) + + t.Run("not requested", func(t *testing.T) { + state, err := testClient(t, ts, "tok").FetchAll(context.Background()) + if err != nil { + t.Fatalf("FetchAll: %v", err) + } + if state.NoTeam != nil { + t.Errorf("NoTeam: got %+v, want nil when not requested", state.NoTeam) + } + }) +} + +func TestFetchAllNoTeamPermissionErrors(t *testing.T) { + // A gitops-scoped token may be refused on some of these endpoints. That + // must degrade to "skipped", not fail the whole diff. + mux := http.NewServeMux() + mux.HandleFunc("/api/v1/fleet/teams", func(w http.ResponseWriter, _ *http.Request) { + fmt.Fprint(w, `{"teams":[]}`) + }) + mux.HandleFunc("/api/v1/fleet/labels", func(w http.ResponseWriter, _ *http.Request) { + fmt.Fprint(w, `{"labels":[]}`) + }) + mux.HandleFunc("/api/v1/fleet/software/fleet_maintained_apps", func(w http.ResponseWriter, _ *http.Request) { + fmt.Fprint(w, `{"fleet_maintained_apps":[]}`) + }) + mux.HandleFunc("/api/v1/fleet/teams/0/policies", func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusForbidden) + }) + mux.HandleFunc("/api/v1/fleet/configuration_profiles", func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusForbidden) + }) + mux.HandleFunc("/api/v1/fleet/scripts", func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + }) + + ts := httptest.NewServer(mux) + defer ts.Close() + + state, err := testClient(t, ts, "tok").FetchAll(context.Background(), FetchOptions{NoTeam: true}) + if err != nil { + t.Fatalf("FetchAll: %v", err) + } + if state.NoTeam == nil { + t.Fatal("NoTeam: got nil") + } + if !state.NoTeam.PoliciesUnavailable || !state.NoTeam.ProfilesUnavailable || !state.NoTeam.ScriptsUnavailable { + t.Errorf("unavailable flags: got %+v, want all true", state.NoTeam) + } +} + +func TestGetNoTeamPoliciesPagination(t *testing.T) { + // 250 results means "there may be more"; the client keeps paging until a + // short page comes back. + var pages []string + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + page := r.URL.Query().Get("page") + pages = append(pages, page) + n := 250 + if page != "0" { + n = 2 + } + policies := make([]Policy, n) + for i := range policies { + policies[i] = Policy{ID: uint(i + 1), Name: fmt.Sprintf("p%s-%d", page, i)} + } + _ = json.NewEncoder(w).Encode(policiesResponse{Policies: policies}) + })) + defer ts.Close() + + policies, err := testClient(t, ts, "tok").GetNoTeamPolicies(context.Background()) + if err != nil { + t.Fatalf("GetNoTeamPolicies: %v", err) + } + if len(policies) != 252 { + t.Errorf("got %d policies, want 252", len(policies)) + } + if strings.Join(pages, ",") != "0,1" { + t.Errorf("pages requested: got %v, want [0 1]", pages) + } +} + +func TestFetchAllNoTeamFatalErrors(t *testing.T) { + // A 403/404 degrades to "unavailable", but any other failure is a real + // problem and must not be reported as an empty bucket. + tests := []struct { + name string + failPath string + }{ + {"policies", "/api/v1/fleet/teams/0/policies"}, + {"profiles", "/api/v1/fleet/configuration_profiles"}, + {"scripts", "/api/v1/fleet/scripts"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/api/v1/fleet/teams", func(w http.ResponseWriter, _ *http.Request) { + fmt.Fprint(w, `{"teams":[]}`) + }) + mux.HandleFunc("/api/v1/fleet/labels", func(w http.ResponseWriter, _ *http.Request) { + fmt.Fprint(w, `{"labels":[]}`) + }) + mux.HandleFunc("/api/v1/fleet/software/fleet_maintained_apps", func(w http.ResponseWriter, _ *http.Request) { + fmt.Fprint(w, `{"fleet_maintained_apps":[]}`) + }) + // Every no-team endpoint succeeds except the one under test, + // which returns a server error rather than a 403/404. + ok := map[string]string{ + "/api/v1/fleet/teams/0/policies": `{"policies":[]}`, + "/api/v1/fleet/configuration_profiles": `{"profiles":[]}`, + "/api/v1/fleet/scripts": `{"scripts":[]}`, + } + for path, body := range ok { + if path == tt.failPath { + mux.HandleFunc(path, func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + }) + continue + } + mux.HandleFunc(path, func(w http.ResponseWriter, _ *http.Request) { + fmt.Fprint(w, body) + }) + } + + ts := httptest.NewServer(mux) + defer ts.Close() + + if _, err := testClient(t, ts, "tok").FetchAll(context.Background(), FetchOptions{NoTeam: true}); err == nil { + t.Fatalf("expected FetchAll to fail when %s returns 500", tt.name) + } + }) + } +} diff --git a/internal/diff/differ.go b/internal/diff/differ.go index a794bc9..8cd03cb 100644 --- a/internal/diff/differ.go +++ b/internal/diff/differ.go @@ -173,6 +173,87 @@ func noTeamSummary(t parser.ParsedTeam) string { return strings.Join(parts, ", ") } +// diffNoTeam fills in the diff for Fleet's "hosts on no team" bucket, whose +// resources live behind team_id=0 rather than in the /teams list. +// +// When the bucket was not fetched (older Fleet, or the caller did not ask for +// it), it falls back to reporting what the repo configures, so nothing +// silently disappears from the plan. +func diffNoTeam(result *DiffResult, current *api.NoTeam, proposed parser.ParsedTeam, changedFiles []string, cfg diffOptions) { + if current == nil { + if summary := noTeamSummary(proposed); summary != "" { + result.Errors = append(result.Errors, + fmt.Sprintf("%s configured (no API diff available for hosts on no team)", summary)) + } + return + } + + if current.PoliciesUnavailable { + result.Errors = append(result.Errors, "policies diff skipped: API token lacks permission to read no-team policies") + } else { + result.Policies = diffPolicies(current.Policies, proposed.Policies) + } + + if current.ProfilesUnavailable { + result.Errors = append(result.Errors, "profiles diff skipped: API token lacks permission to read profiles") + } else { + var warnings []string + result.Profiles, warnings = diffProfiles(current.Profiles, proposed.Profiles, changedFiles) + result.Errors = append(result.Errors, warnings...) + } + + if current.ScriptsUnavailable { + result.Errors = append(result.Errors, "scripts diff skipped: API token lacks permission to read scripts") + } else { + result.Scripts = diffScripts(current.Scripts, proposed.Scripts) + } + + // Fleet reports configured software only through the teams list, which + // excludes this bucket, so there is nothing to compare against. Say so + // rather than reporting every configured item as an addition. + if n := len(proposed.Software.Packages) + len(proposed.Software.FleetMaintained) + len(proposed.Software.AppStoreApps); n > 0 { + result.Errors = append(result.Errors, + fmt.Sprintf("software diff skipped: %d software items configured, but Fleet does not report software for hosts on no team", n)) + } + + // Fleet scopes queries to a real team or to the global scope, so a + // no-team file cannot own them. The parser still accepts `queries:` and + // `reports:` in any team file, so say plainly that these are not diffed + // instead of dropping them without a word. + if n := len(proposed.Queries); n > 0 { + result.Errors = append(result.Errors, + fmt.Sprintf("queries diff skipped: %d queries configured, but Fleet has no query scope for hosts on no team", n)) + } + + // Subtract changes that already exist between the base branch and Fleet, + // so a no-team change that is merged but not yet deployed is not reported + // again on every later MR. + if cfg.baseline != nil { + if baseTeam, ok := findBaselineNoTeam(cfg.baseline); ok { + base := DiffResult{} + if !current.PoliciesUnavailable { + base.Policies = diffPolicies(current.Policies, baseTeam.Policies) + } + if !current.ProfilesUnavailable { + base.Profiles, _ = diffProfiles(current.Profiles, baseTeam.Profiles, nil) + } + if !current.ScriptsUnavailable { + base.Scripts = diffScripts(current.Scripts, baseTeam.Scripts) + } + result.Policies = subtractResourceDiff(result.Policies, base.Policies) + result.Profiles = subtractResourceDiff(result.Profiles, base.Profiles) + result.Scripts = subtractResourceDiff(result.Scripts, base.Scripts) + vlog(cfg.verbose, "[%s] after baseline subtraction: policies=%s profiles=%s scripts=%s", + proposed.Name, rdSummary(result.Policies), rdSummary(result.Profiles), rdSummary(result.Scripts)) + } else { + vlog(cfg.verbose, "[%s] no baseline no-team file found", proposed.Name) + } + } + + vlog(cfg.verbose, "[%s] no-team diff: policies=%s profiles=%s scripts=%s", + proposed.Name, rdSummary(result.Policies), rdSummary(result.Profiles), rdSummary(result.Scripts)) +} + // rdNames returns names of changes for debugging. func rdNames(rd ResourceDiff) string { var names []string @@ -277,13 +358,7 @@ func Diff(current *api.FleetState, proposed *parser.ParsedRepo, teamFilters []st // returned by the /teams API endpoint. Skip the "will be created" // warning for it, and don't list its resources as additions. if parser.IsNoTeam(proposedTeam.Name, proposedTeam.SourceFile) { - // Can't deep-diff against API state since it's not in the teams - // list. Just report what the repo configures for it, so nothing - // silently disappears from the plan. - if summary := noTeamSummary(proposedTeam); summary != "" { - result.Errors = append(result.Errors, - fmt.Sprintf("%s configured (no API diff available for hosts on no team)", summary)) - } + diffNoTeam(&result, current.NoTeam, proposedTeam, changedFiles, cfg) } else { // Genuinely new team for _, p := range proposedTeam.Policies { @@ -499,6 +574,19 @@ func filterChanges(changes []ResourceChange, keep func(string) bool) []ResourceC // ---------- Baseline subtraction ---------- // findBaselineTeam looks up a team by name in the baseline parsed repo. +// findBaselineNoTeam returns the baseline's no-team file. It matches on the +// no-team identity rather than the display name, because the base branch and +// the MR branch may spell it differently ("No team" vs "Unassigned") -- for +// instance in the MR that migrates a repo from the teams/ layout to fleets/. +func findBaselineNoTeam(baseline *parser.ParsedRepo) (parser.ParsedTeam, bool) { + for _, t := range baseline.Teams { + if parser.IsNoTeam(t.Name, t.SourceFile) { + return t, true + } + } + return parser.ParsedTeam{}, false +} + func findBaselineTeam(baseline *parser.ParsedRepo, name string) (parser.ParsedTeam, bool) { for _, t := range baseline.Teams { if strings.EqualFold(t.Name, name) { diff --git a/internal/diff/differ_test.go b/internal/diff/differ_test.go index d00c26e..4cb5518 100644 --- a/internal/diff/differ_test.go +++ b/internal/diff/differ_test.go @@ -2654,3 +2654,209 @@ func TestDiffTestdataTeamSettings(t *testing.T) { } } } + +// When the no-team bucket has been fetched (team_id=0), it is diffed like any +// other team rather than being summarized. +func TestDiffNoTeamDeepDiff(t *testing.T) { + current := &api.FleetState{ + Teams: []api.Team{}, + Labels: []api.Label{}, + NoTeam: &api.NoTeam{ + Policies: []api.Policy{ + // Same name, different query → modified. + {Name: "RingCentral uninstalled", Query: "SELECT 1;", Platform: "darwin"}, + // Not in the YAML → deleted. + {Name: "Retired policy", Query: "SELECT 2;", PassingHostCount: 5}, + }, + Profiles: []api.Profile{{Name: "Conditional access", Platform: "darwin"}}, + Scripts: []api.Script{ + {ID: 1, Name: "uninstall-ringcentral.sh", Content: "echo one\n"}, + {ID: 2, Name: "gone.ps1", Content: "Write-Host removed\n"}, + }, + }, + } + + proposed := &parser.ParsedRepo{Teams: []parser.ParsedTeam{{ + Name: "Unassigned", + SourceFile: "fleets/unassigned.yml", + Policies: []parser.ParsedPolicy{ + {Name: "RingCentral uninstalled", Query: "SELECT 42;", Platform: "darwin"}, + {Name: "Brand new policy", Query: "SELECT 3;"}, + }, + Profiles: []parser.ParsedProfile{ + {Name: "Conditional access", Platform: "darwin", Path: "lib/profiles/ca.mobileconfig"}, + {Name: "Newly added profile", Platform: "darwin", Path: "lib/profiles/new.mobileconfig"}, + }, + Scripts: []parser.ParsedScript{ + {Name: "uninstall-ringcentral.sh", Content: "echo one\necho two\n"}, + }, + }}} + + results := Diff(current, proposed, nil, nil) + if len(results) != 1 { + t.Fatalf("got %d results, want 1", len(results)) + } + r := results[0] + + checks := []struct { + what string + got ResourceDiff + a int + m int + d int + }{ + {"policies", r.Policies, 1, 1, 1}, + {"profiles", r.Profiles, 1, 0, 0}, + {"scripts", r.Scripts, 0, 1, 1}, + } + for _, c := range checks { + if len(c.got.Added) != c.a || len(c.got.Modified) != c.m || len(c.got.Deleted) != c.d { + t.Errorf("%s: got +%d ~%d -%d, want +%d ~%d -%d", c.what, + len(c.got.Added), len(c.got.Modified), len(c.got.Deleted), c.a, c.m, c.d) + } + } + + // The summary fallback must not appear once a real diff is available. + for _, e := range r.Errors { + if strings.Contains(e, "no API diff available") { + t.Errorf("summary fallback still reported: %q", e) + } + if strings.Contains(e, "does not exist in Fleet yet") { + t.Errorf("no-team reported as a new team: %q", e) + } + } +} + +func TestDiffNoTeamUnavailableResources(t *testing.T) { + current := &api.FleetState{ + Teams: []api.Team{}, + Labels: []api.Label{}, + NoTeam: &api.NoTeam{ + PoliciesUnavailable: true, + ProfilesUnavailable: true, + ScriptsUnavailable: true, + }, + } + proposed := &parser.ParsedRepo{Teams: []parser.ParsedTeam{{ + Name: "No team", + SourceFile: "teams/no-team.yml", + Policies: []parser.ParsedPolicy{{Name: "P"}}, + }}} + + r := Diff(current, proposed, nil, nil)[0] + + // Nothing may be reported as an addition when the API side is unreadable: + // that would claim Fleet has none of it, which is not known. + if !r.Policies.IsEmpty() || !r.Profiles.IsEmpty() || !r.Scripts.IsEmpty() { + t.Errorf("expected empty diffs, got policies=%+v profiles=%+v scripts=%+v", + r.Policies, r.Profiles, r.Scripts) + } + for _, want := range []string{"policies diff skipped", "profiles diff skipped", "scripts diff skipped"} { + found := false + for _, e := range r.Errors { + if strings.Contains(e, want) { + found = true + } + } + if !found { + t.Errorf("missing %q in %v", want, r.Errors) + } + } +} + +func TestDiffNoTeamSoftwareIsReportedAsSkipped(t *testing.T) { + current := &api.FleetState{Teams: []api.Team{}, Labels: []api.Label{}, NoTeam: &api.NoTeam{}} + proposed := &parser.ParsedRepo{Teams: []parser.ParsedTeam{{ + Name: "Unassigned", + SourceFile: "fleets/unassigned.yml", + Software: parser.ParsedSoftware{ + Packages: []parser.ParsedSoftwarePackage{{URL: "https://example.com/a.pkg"}}, + FleetMaintained: []parser.ParsedFleetApp{{Slug: "zoom/darwin"}}, + }, + }}} + + r := Diff(current, proposed, nil, nil)[0] + + if !r.Software.IsEmpty() { + t.Errorf("software: got %+v, want empty (nothing to compare against)", r.Software) + } + found := false + for _, e := range r.Errors { + if strings.Contains(e, "software diff skipped: 2 software items configured") { + found = true + } + } + if !found { + t.Errorf("missing software skip note in %v", r.Errors) + } +} + +// A no-team change that is already merged to the base branch but not yet +// deployed must not be reported again on every later MR. +func TestDiffNoTeamBaselineSubtraction(t *testing.T) { + current := &api.FleetState{ + Teams: []api.Team{}, + Labels: []api.Label{}, + NoTeam: &api.NoTeam{ + Policies: []api.Policy{{Name: "Existing", Query: "SELECT 1;"}}, + Scripts: []api.Script{{ID: 1, Name: "keep.sh", Content: "echo one\n"}}, + }, + } + + // Already on the base branch: the added policy and the edited script. + baseline := &parser.ParsedRepo{Teams: []parser.ParsedTeam{{ + Name: "No team", + SourceFile: "teams/no-team.yml", + Policies: []parser.ParsedPolicy{ + {Name: "Existing", Query: "SELECT 1;"}, + {Name: "Merged not deployed", Query: "SELECT 2;"}, + }, + Scripts: []parser.ParsedScript{{Name: "keep.sh", Content: "echo one\necho two\n"}}, + }}} + + // The MR adds one more policy on top of the base branch's state. The + // branch spells the bucket differently, which must not defeat matching. + proposed := &parser.ParsedRepo{Teams: []parser.ParsedTeam{{ + Name: "Unassigned", + SourceFile: "fleets/unassigned.yml", + Policies: []parser.ParsedPolicy{ + {Name: "Existing", Query: "SELECT 1;"}, + {Name: "Merged not deployed", Query: "SELECT 2;"}, + {Name: "New in this MR", Query: "SELECT 3;"}, + }, + Scripts: []parser.ParsedScript{{Name: "keep.sh", Content: "echo one\necho two\n"}}, + }}} + + r := Diff(current, proposed, nil, nil, WithBaseline(baseline))[0] + + if len(r.Policies.Added) != 1 || r.Policies.Added[0].Name != "New in this MR" { + t.Errorf("policies added: got %+v, want only the MR's own addition", r.Policies.Added) + } + if !r.Scripts.IsEmpty() { + t.Errorf("scripts: got %+v, want empty (the edit is already on the base branch)", r.Scripts) + } +} + +func TestDiffNoTeamQueriesAreReportedAsSkipped(t *testing.T) { + current := &api.FleetState{Teams: []api.Team{}, Labels: []api.Label{}, NoTeam: &api.NoTeam{}} + proposed := &parser.ParsedRepo{Teams: []parser.ParsedTeam{{ + Name: "No team", + SourceFile: "teams/no-team.yml", + Queries: []parser.ParsedQuery{{Name: "Q1"}, {Name: "Q2"}}, + }}} + + r := Diff(current, proposed, nil, nil)[0] + + if !r.Queries.IsEmpty() { + t.Errorf("queries: got %+v, want empty (Fleet has no no-team query scope)", r.Queries) + } + found := false + for _, e := range r.Errors { + if strings.Contains(e, "queries diff skipped: 2 queries configured") { + found = true + } + } + if !found { + t.Errorf("missing query skip note in %v", r.Errors) + } +}