Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 35 additions & 19 deletions internal/diff/differ.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,15 @@ var wsRE = regexp.MustCompile(`\s+`)

// DiffResult holds the diff for a single team (or global scope).
type DiffResult struct {
Team string // "(global)" for default.yml scope
Policies ResourceDiff
Queries ResourceDiff
Software ResourceDiff
Profiles ResourceDiff
Scripts ResourceDiff
Labels LabelValidation
Config []ConfigChange // org_settings, agent_options, controls diffs
Errors []string
Team string // "(global)" for default.yml scope
Policies ResourceDiff
Queries ResourceDiff
Software ResourceDiff
Profiles ResourceDiff
Scripts ResourceDiff
Labels LabelValidation
Config []ConfigChange // org_settings, agent_options, controls diffs
Errors []string
SkippedConfigSections []string // config sections absent from API (e.g. "agent_options")
}

Expand Down Expand Up @@ -100,9 +100,9 @@ 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
}

Expand Down Expand Up @@ -299,7 +299,7 @@ func Diff(current *api.FleetState, proposed *parser.ParsedRepo, teamFilters []st
result.Errors = append(result.Errors, "profiles diff skipped: API token lacks permission to read profiles")
} else {
var profileWarnings []string
result.Profiles, profileWarnings = diffProfiles(currentTeam.Profiles, proposedTeam.Profiles)
result.Profiles, profileWarnings = diffProfiles(currentTeam.Profiles, proposedTeam.Profiles, changedFiles)
result.Errors = append(result.Errors, profileWarnings...)
}

Expand Down Expand Up @@ -329,7 +329,7 @@ func Diff(current *api.FleetState, proposed *parser.ParsedRepo, teamFilters []st
baseDiff.Software = diffSoftware(enrichedSoftware, baseTeam.Software)
}
if !currentTeam.ProfilesUnavailable {
baseDiff.Profiles, _ = diffProfiles(currentTeam.Profiles, baseTeam.Profiles)
baseDiff.Profiles, _ = diffProfiles(currentTeam.Profiles, baseTeam.Profiles, nil)
}
if !currentTeam.ScriptsUnavailable {
baseDiff.Scripts = diffScripts(currentTeam.Scripts, baseTeam.Scripts)
Expand Down Expand Up @@ -421,6 +421,7 @@ func buildSourceMap(team parser.ParsedTeam) map[string][]string {
for _, p := range team.Profiles {
add(p.Name, p.SourceFile)
add(p.Name, team.SourceFile)
add(p.Name, p.Path)
}
for _, s := range team.Scripts {
add(s.Name, s.SourceFile)
Expand Down Expand Up @@ -797,9 +798,9 @@ func diffSoftware(current api.TeamSoftware, proposed parser.ParsedSoftware) Reso
}
}
for _, sc := range []struct {
name string
curVal string
newVal string
name string
curVal string
newVal string
}{
{"install_script", cur.InstallScript, a.InstallScript},
{"uninstall_script", cur.UninstallScript, a.UninstallScript},
Expand Down Expand Up @@ -1080,7 +1081,7 @@ func normalizeFleetPlatform(platform string) string {
}
}

func diffProfiles(current []api.Profile, proposed []parser.ParsedProfile) (ResourceDiff, []string) {
func diffProfiles(current []api.Profile, proposed []parser.ParsedProfile, changedFiles []string) (ResourceDiff, []string) {
var diff ResourceDiff
var warnings []string

Expand All @@ -1089,6 +1090,12 @@ func diffProfiles(current []api.Profile, proposed []parser.ParsedProfile) (Resou
currentMap[p.Name] = p
}

// Build a set of changed file paths for fast lookup.
changedSet := make(map[string]bool, len(changedFiles))
for _, cf := range changedFiles {
changedSet[cf] = true
}

// Profile identity is determined by the name embedded in the file content
// (e.g., PayloadDisplayName for .mobileconfig), which is what Fleet uses.
// The parser extracts this into ParsedProfile.Name during parsing.
Expand All @@ -1110,6 +1117,16 @@ func diffProfiles(current []api.Profile, proposed []parser.ParsedProfile) (Resou
Name: name,
Fields: fields,
})
} else if changedSet[p.Path] {
// The Fleet API does not return profile content, so we cannot
// compare payloads. When the profile XML file appears in the
// git changed-files list, treat it as modified.
diff.Modified = append(diff.Modified, ResourceChange{
Name: name,
Fields: map[string]FieldDiff{
"path": {New: p.Path},
},
})
}
}

Expand Down Expand Up @@ -1508,4 +1525,3 @@ func normalizeScript(s string) string {
func normalizeWS(s string) string {
return strings.TrimSpace(wsRE.ReplaceAllString(s, " "))
}

88 changes: 76 additions & 12 deletions internal/diff/differ_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,10 +266,10 @@ func TestDiffPolicyScenarios(t *testing.T) {
wantModified: 1, checkName: "Disk Encryption", checkFields: []string{"query", "critical"},
},
{
name: "policy deleted with hosts",
current: []api.Policy{{Name: "Keep", Platform: "darwin"}, {Name: "Delete This", Platform: "darwin", PassingHostCount: 50}},
proposed: []parser.ParsedPolicy{{Name: "Keep", Platform: "darwin"}},
wantDeleted: 1, checkName: "Delete This", checkWarning: true,
name: "policy deleted with hosts",
current: []api.Policy{{Name: "Keep", Platform: "darwin"}, {Name: "Delete This", Platform: "darwin", PassingHostCount: 50}},
proposed: []parser.ParsedPolicy{{Name: "Keep", Platform: "darwin"}},
wantDeleted: 1, checkName: "Delete This", checkWarning: true,
},
{
name: "no changes",
Expand Down Expand Up @@ -1213,7 +1213,7 @@ func TestDiffProfilesMatchByContentName(t *testing.T) {
{Path: "/repo/profiles/mac/wifi-corporate.mobileconfig", Name: "wifi-corporate", Platform: "darwin"},
}

diff, warnings := diffProfiles(current, proposed)
diff, warnings := diffProfiles(current, proposed, nil)
if len(warnings) > 0 {
t.Errorf("unexpected warnings: %v", warnings)
}
Expand All @@ -1236,7 +1236,7 @@ func TestDiffProfilesDetectsRealChanges(t *testing.T) {
{Path: "/repo/profiles/mac/new-profile.mobileconfig", Name: "new-profile", Platform: "darwin"},
}

diff, _ := diffProfiles(current, proposed)
diff, _ := diffProfiles(current, proposed, nil)
if len(diff.Added) != 1 || diff.Added[0].Name != "new-profile" {
t.Errorf("expected 1 added profile 'new-profile', got %v", diff.Added)
}
Expand All @@ -1249,12 +1249,12 @@ func TestDiffProfilesDetectsRealChanges(t *testing.T) {

func TestDiffGlobalConfig(t *testing.T) {
tests := []struct {
name string
apiConfig map[string]any
proposedOrg map[string]any
wantChanges int
wantKey string // key to find in changes (empty = skip check)
wantKeyAbsent string // key that must NOT appear in changes
name string
apiConfig map[string]any
proposedOrg map[string]any
wantChanges int
wantKey string // key to find in changes (empty = skip check)
wantKeyAbsent string // key that must NOT appear in changes
wantOld, wantNew string
}{
{
Expand Down Expand Up @@ -1882,6 +1882,70 @@ func assertChangesEqual(t *testing.T, label string, want, got []ResourceChange)
}
}

// TestDiffChangedFileFilterIncludesProfilePath verifies that modifying a profile
// XML file (not the team YAML) is detected by the changed-file filter.
func TestDiffChangedFileFilterIncludesProfilePath(t *testing.T) {
t.Parallel()

current := &api.FleetState{
Teams: []api.Team{{
ID: 1,
Name: "T",
Profiles: []api.Profile{
{ProfileUUID: "uuid-1", Name: "experience_windows", Platform: "windows"},
{ProfileUUID: "uuid-2", Name: "newsandinterests_windows", Platform: "windows"},
},
}},
}

proposed := &parser.ParsedRepo{
Teams: []parser.ParsedTeam{{
Name: "T",
Profiles: []parser.ParsedProfile{
{
Name: "experience_windows",
Path: "profiles/windows/experience_windows.xml",
Platform: "windows",
SourceFile: "/repo/teams/t.yml",
},
{
Name: "newsandinterests_windows",
Path: "profiles/windows/newsandinterests_windows.xml",
Platform: "windows",
SourceFile: "/repo/teams/t.yml",
},
},
}},
}

// Only the profile XML changed, not the team YAML.
changedFiles := []string{
"profiles/windows/experience_windows.xml",
}

results := Diff(current, proposed, nil, changedFiles)
if len(results) != 1 {
t.Fatalf("expected 1 result, got %d", len(results))
}
r := results[0]

// experience_windows should appear (its XML path is in changedFiles).
if len(r.Profiles.Modified) != 1 {
t.Fatalf("expected 1 modified profile (XML path match), got %d modified, %d added, %d deleted",
len(r.Profiles.Modified), len(r.Profiles.Added), len(r.Profiles.Deleted))
}
if r.Profiles.Modified[0].Name != "experience_windows" {
t.Errorf("expected experience_windows in modified, got %q", r.Profiles.Modified[0].Name)
}

// newsandinterests_windows should be filtered out (its XML is not in changedFiles).
for _, m := range r.Profiles.Modified {
if m.Name == "newsandinterests_windows" {
t.Errorf("newsandinterests_windows should be filtered out, but found in modified")
}
}
}

// 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()
Expand Down
Loading