feat: support modern Fleet GitOps schema (fleets/, apple_settings, reports:, depth-aware PayloadDisplayName)#37
Open
robbiet480 wants to merge 4 commits into
Conversation
Fleet upstream renamed the per-team YAML directory from teams/ to fleets/
in #40726 ("Migrating teams to fleets and queries to reports"). New repos
generated by `fleetctl new` use fleets/. fleet-plan previously hard-coded
teams/ everywhere, so it could not be used against current Fleet GitOps
repos.
Add an internal/teamdir package that resolves the directory at runtime,
preferring fleets/ when both exist. Wire it into the parser, git scope
detector, baseline checkout, and CLI error messages. Existing teams/
repos continue to work unchanged.
Tests cover both layouts and the precedence rule.
Three additions to keep up with current fleetdm/fleet GitOps yaml: 1. controls.apple_settings.configuration_profiles — the unified replacement for controls.macos_settings.custom_settings. Entries may be .mobileconfig, .json (DDM declarations), or .xml. Platform is inferred from the lib/macos/ vs lib/ipados/ vs lib/ios/ path segment. Diff identity remains the embedded profile name. 2. Top-level `settings:` and `reports:` keys are now accepted (parsed as opaque maps). Field-level diffing of these sections isn't implemented yet, but the absence of an error keeps the rest of the diff trustworthy. 3. labels_include_any / labels_exclude_any / labels_include_all sibling keys on profile refs are now captured by the raw struct. Without these, fleet-plan against current Fleet GitOps repos parses profiles=0 for every team and reports every live Fleet profile as "REMOVED" — a false-positive that makes the PR comment dangerously misleading. Tests: TestParseRepoAppleSettings covers .mobileconfig + .json DDM + platform inference; TestParseRepoAcceptsSettingsAndReportsKeys covers the new top-level keys.
extractMobileconfigName previously took the LAST PayloadDisplayName in the file, assuming the top-level one always appears after PayloadContent. That holds for plists emitted by ProfileCreator and similar tools, but Apple Configurator (and many hand-edited profiles) emit the top-level PayloadDisplayName BEFORE PayloadContent. With that ordering, the "last" occurrence is a nested payload's display name -- which is often a templated literal shared across many files. Concrete symptom: a fleet-gitops repo with 50+ per-location activation code profiles, each with a unique outer PayloadDisplayName like "Zoom Room Activation Code — University of Pennsylvania (Room2)" and two inner payloads named "Zoom Room Activation Code (Mac)". fleet-plan reported every profile as "Zoom Room Activation Code (Mac)", collapsing 50 distinct profiles into one duplicate name and producing a fake ADDED/REMOVED storm. Fix: stream through the XML with a <dict> nesting depth counter and only accept PayloadDisplayName at depth 1 (direct child of the root dict). Position within the file no longer matters. Existing tests for top-level-last ordering still pass; new test case covers the top-level-first ordering from real-world activation profiles.
Fleet's fleetdm/fleet#40726 renamed two top-level GitOps keys: teams/ -> fleets/ AND queries: -> reports:. The teams/->fleets/ half was already handled here; the queries:->reports: half was previously accepted as opaque so it wouldn't error, but its path refs were silently dropped. Symptom: a fleets/zoom-rooms.yml that lists 2 queries under reports: parsed with 0 queries on the proposed side. diffQueries then flagged both live Fleet queries as REMOVED -- a false signal that looks like real drift. Fix: add rawTeamFile.Reports (and the matching field in the default.yml rawStruct) and append them to the same resolveQueryRef loop. Both keys remain valid; `fleetctl gitops` accepts either or both during the transition, so we do too. Tests: replaced TestParseRepoAcceptsSettingsAndReportsKeys (which only asserted no unknown-key error and used a bogus inline body) with two focused tests -- TestParseRepoAcceptsSettingsKey for the opaque settings: block, and TestParseRepoReportsAliasesQueries that verifies a query defined under reports: actually shows up in the team's Queries list. End-to-end verified against the real Fleet instance: a fleet-gitops repo where two queries are listed under reports: now diffs as "no changes" (matches live state) instead of "2 deleted". A PR that adds one profile shows exactly "1 added" -- the actual change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Brings fleet-plan up to date with the current Fleet GitOps schema so it runs cleanly against repos generated by recent
fleetctl gitops. Five independent changes; happy to split into separate PRs if you'd prefer to land them piecemeal.1.
fleets/directory support (commit aa72a42)Fleet renamed the per-team YAML directory from
teams/tofleets/in fleetdm/fleet#40726 ("Migrating teams to fleets and queries to reports"). New repos generated byfleetctl newusefleets/. fleet-plan was hard-codingteams/everywhere.Adds a new
internal/teamdirpackage as the single source of truth that resolves the directory at runtime, preferringfleets/when both exist. Four call sites consult it:parser.ParseRepo,git.ResolveScope,git.CheckoutBaseline, and the CLI's "no teams found" error.2.
controls.apple_settings.configuration_profilesparsing (commit c07e212)The unified Apple-platform block introduced in Fleet 4.x. Replaces the legacy
controls.macos_settings.custom_settings. Entries may be.mobileconfig(macOS/iOS/iPadOS),.json(DDM declarations), or.xml. Without this, every team in a modern repo parses withprofiles=0, anddiffProfilesreports every live Fleet profile as REMOVED — silent data-loss-shaped output.Platform on the parsed profile is inferred from the path segment:
lib/macos/→ darwin,lib/ipados/→ ipados,lib/ios/→ ios, everything else → darwin. The diff identity (profile name from the file content or filename) is unchanged.3. Top-level
settings:key (commit c07e212)Valid in current Fleet GitOps yaml but fleet-plan was rejecting it as
unknown top-level key. Now parsed opaquely — field-level diffing of this section isn't implemented yet, but the absence of an error keeps the rest of the diff (profiles, policies, queries, software) trustworthy.Also captures
labels_include_any/labels_exclude_any/labels_include_allsibling keys on profile refs (previously dropped silently).4. Depth-aware top-level
PayloadDisplayNameextraction (commit 4fc400e)extractMobileconfigNamepreviously kept the last PayloadDisplayName in the file, on the assumption that the top-level one always appears afterPayloadContent. That holds for plists produced by some generators (ProfileCreator), but Apple Configurator and most hand-edited profiles put the top-level PayloadDisplayName before PayloadContent — where the "last" occurrence is a deeply-nested payload's display name, not the profile's identity.The bug surfaces dramatically with template-derived profiles: e.g. 50 per-location Zoom Room activation profiles with unique outer names (
"… — University of Pennsylvania (Room2)") but identical nested payload names ("Zoom Room Activation Code (Mac)") all collapse to one duplicate name, producing a fake ADDED/REMOVED storm.New extractor streams through the XML with a
<dict>nesting depth counter and only accepts PayloadDisplayName at depth 1 (direct child of the root dict). Position within the file no longer matters. Existing tests covering top-level-last ordering still pass.5.
reports:aliasesqueries:(commit 32770f0)Same fleetdm/fleet#40726 that renamed
teams/→fleets/also renamed the team-yaml keyqueries:→reports:.fleetctl gitopsaccepts both during the transition, so fleet-plan should too. Previouslyreports:was accepted opaquely (no error), but the querypath:refs underneath were silently dropped — so a team listing 2 queries underreports:parsed with 0 queries on the proposed side, and live Fleet's queries showed as REMOVED.rawTeamFile.Reportsand the matching field in thedefault.ymlraw struct now feed the sameresolveQueryRefloop. Both keys remain valid and produce equivalent output.Why now
I wired fleet-plan into a real Fleet GitOps repo's PR workflow. Each substantive PR surfaced another schema gap:
controls.macos_settings.custom_settings→controls.apple_settings.configuration_profilesrename (False positive diffs in config and software comparison #2 above) plus the unknown top-level keys (Fix fleet-maintained app inference when API returns null #3).extractMobileconfigName(fix: Improve fleet-maintained app inference (#3) #4).queries:→reports:rename (fix: Add prefix matching for FMA catalog name mismatches (#3) #5).The schema gaps are load-bearing — without them the tool is dangerous, not just incomplete.
The existing
teams/, legacy-controls, and legacy-queries:code paths are all unchanged and still exercised by every pre-existing test.End-to-end validation against a real Fleet instance:
No changes detected. Your branch matches the current Fleet state.✅1 added(just the new profile) ✅Test plan
go build ./...go test ./...— existing tests still pass, plus new tests:internal/teamdir: resolve / precedence /HasPrefixparser.TestParseRepoFleetsDirparser.TestParseRepoFleetsTakesPriorityOverTeamsparser.TestParseRepoAppleSettings— covers.mobileconfig+ DDM.json+ platform inferenceparser.TestParseRepoAcceptsSettingsKeyparser.TestParseRepoReportsAliasesQueries— verifiesreports:path refs become team queriesparser.TestExtractMobileconfigName— adds a top-level-first case alongside the existing top-level-last and real-world casesgit.TestIsTeamYAMLgit.TestResolveScope— new cases forfleets/team-file and resource changesfleets/-based repo parse with zero errors, profile and query counts match live Fleet state, and the diff output for a substantive PR is now accurate.Happy to adjust naming, structure, or split this up however you'd like. Thanks for the tool!