Skip to content

Commit 50ea91b

Browse files
authored
fix: Track script source files in changed-file filter (#12)
Closes #11
1 parent baf8e67 commit 50ea91b

7 files changed

Lines changed: 230 additions & 10 deletions

File tree

‎internal/diff/differ.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,9 @@ func buildSourceMap(team parser.ParsedTeam) map[string][]string {
259259
}
260260
if key != "" {
261261
add(normalizeSoftwarePath(key), p.SourceFile)
262+
for _, sf := range p.SourceFiles {
263+
add(normalizeSoftwarePath(key), sf)
264+
}
262265
}
263266
}
264267
for _, f := range team.Software.FleetMaintained {

‎internal/diff/differ_test.go‎

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1453,6 +1453,127 @@ func TestStripArchSuffix(t *testing.T) {
14531453
}
14541454
}
14551455

1456+
// TestDiffChangedFileFilterIncludesScriptSourceFiles verifies that when only a
1457+
// script file changes (no YAML changes), the changed-file filter still includes
1458+
// the parent software package in the diff output. Regression test for #11.
1459+
func TestDiffChangedFileFilterIncludesScriptSourceFiles(t *testing.T) {
1460+
current := &api.FleetState{
1461+
Teams: []api.Team{
1462+
{
1463+
ID: 1,
1464+
Name: "Workstations",
1465+
Software: api.TeamSoftware{
1466+
Packages: []api.TeamSoftwarePackage{
1467+
{
1468+
ReferencedYAMLPath: "software/mac/printers-hq/printers-hq.yml",
1469+
URL: "https://example.com/printers-hq-1.0.pkg",
1470+
},
1471+
{
1472+
ReferencedYAMLPath: "software/mac/slack/slack.yml",
1473+
URL: "https://example.com/slack-old.dmg",
1474+
},
1475+
},
1476+
},
1477+
},
1478+
},
1479+
}
1480+
1481+
proposed := &parser.ParsedRepo{
1482+
Teams: []parser.ParsedTeam{
1483+
{
1484+
Name: "Workstations",
1485+
Software: parser.ParsedSoftware{
1486+
Packages: []parser.ParsedSoftwarePackage{
1487+
{
1488+
RefPath: "software/mac/printers-hq/printers-hq.yml",
1489+
URL: "https://example.com/printers-hq-2.0.pkg",
1490+
SourceFile: "/repo/software/mac/printers-hq/printers-hq.yml",
1491+
SourceFiles: []string{
1492+
"/repo/software/mac/printers-hq/printers-hq-install.sh",
1493+
"/repo/software/mac/printers-hq/printers-hq-uninstall.sh",
1494+
},
1495+
},
1496+
{
1497+
RefPath: "software/mac/slack/slack.yml",
1498+
URL: "https://example.com/slack-new.dmg",
1499+
SourceFile: "/repo/software/mac/slack/slack.yml",
1500+
},
1501+
},
1502+
},
1503+
},
1504+
},
1505+
}
1506+
1507+
// Only the install script changed, no YAML changes.
1508+
changedFiles := []string{
1509+
"software/mac/printers-hq/printers-hq-install.sh",
1510+
}
1511+
1512+
results := Diff(current, proposed, nil, changedFiles)
1513+
if len(results) != 1 {
1514+
t.Fatalf("expected 1 result, got %d", len(results))
1515+
}
1516+
r := results[0]
1517+
1518+
// printers-hq should appear (its install script is in changedFiles).
1519+
if len(r.Software.Modified) != 1 {
1520+
t.Fatalf("expected 1 modified package (printers-hq via script match), got %d modified, %d added, %d deleted",
1521+
len(r.Software.Modified), len(r.Software.Added), len(r.Software.Deleted))
1522+
}
1523+
if !strings.Contains(r.Software.Modified[0].Name, "printers-hq") {
1524+
t.Errorf("expected printers-hq in modified, got %q", r.Software.Modified[0].Name)
1525+
}
1526+
1527+
// slack should be filtered out (its YAML is not in changedFiles).
1528+
for _, m := range r.Software.Modified {
1529+
if strings.Contains(m.Name, "slack") {
1530+
t.Errorf("slack should be filtered out, but found in modified: %q", m.Name)
1531+
}
1532+
}
1533+
}
1534+
1535+
// TestDiffChangedFileFilterYAMLStillWorks verifies that the changed-file filter
1536+
// continues to work for YAML-only changes (no regression from script tracking).
1537+
func TestDiffChangedFileFilterYAMLStillWorks(t *testing.T) {
1538+
current := &api.FleetState{
1539+
Teams: []api.Team{
1540+
{
1541+
ID: 1,
1542+
Name: "T",
1543+
Software: api.TeamSoftware{
1544+
Packages: []api.TeamSoftwarePackage{
1545+
{ReferencedYAMLPath: "software/mac/app/app.yml", URL: "https://example.com/old.pkg"},
1546+
},
1547+
},
1548+
},
1549+
},
1550+
}
1551+
1552+
proposed := &parser.ParsedRepo{
1553+
Teams: []parser.ParsedTeam{
1554+
{
1555+
Name: "T",
1556+
Software: parser.ParsedSoftware{
1557+
Packages: []parser.ParsedSoftwarePackage{
1558+
{
1559+
RefPath: "software/mac/app/app.yml",
1560+
URL: "https://example.com/new.pkg",
1561+
SourceFile: "/repo/software/mac/app/app.yml",
1562+
SourceFiles: []string{"/repo/software/mac/app/install.sh"},
1563+
},
1564+
},
1565+
},
1566+
},
1567+
},
1568+
}
1569+
1570+
results := Diff(current, proposed, nil, []string{"software/mac/app/app.yml"})
1571+
r := results[0]
1572+
if len(r.Software.Modified) != 1 {
1573+
t.Fatalf("expected 1 modified (YAML match), got %d", len(r.Software.Modified))
1574+
}
1575+
}
1576+
14561577
// findTeam locates a DiffResult by team name, failing the test if not found.
14571578
func findTeam(t *testing.T, results []DiffResult, name string) *DiffResult {
14581579
t.Helper()

‎internal/parser/parser.go‎

Lines changed: 47 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -138,11 +138,12 @@ type ParsedSoftware struct {
138138

139139
// ParsedSoftwarePackage represents a custom software package.
140140
type ParsedSoftwarePackage struct {
141-
URL string `yaml:"url"`
142-
HashSHA256 string `yaml:"hash_sha256"`
143-
SelfService bool `yaml:"self_service"`
144-
SourceFile string `yaml:"-"`
145-
RefPath string `yaml:"-"`
141+
URL string `yaml:"url"`
142+
HashSHA256 string `yaml:"hash_sha256"`
143+
SelfService bool `yaml:"self_service"`
144+
SourceFile string `yaml:"-"`
145+
RefPath string `yaml:"-"`
146+
SourceFiles []string `yaml:"-"` // all referenced file paths (install/uninstall scripts, pre_install_query)
146147
}
147148

148149
// ParsedFleetApp represents a Fleet-maintained app.
@@ -232,6 +233,17 @@ type rawSoftwareRef struct {
232233
SelfService *bool `yaml:"self_service"`
233234
}
234235

236+
// rawSoftwarePackage captures script path: refs inside a software package YAML file.
237+
type rawSoftwarePackage struct {
238+
URL string `yaml:"url"`
239+
HashSHA256 string `yaml:"hash_sha256"`
240+
SelfService bool `yaml:"self_service"`
241+
InstallScript *rawPathRef `yaml:"install_script"`
242+
UninstallScript *rawPathRef `yaml:"uninstall_script"`
243+
PreInstallQuery *rawPathRef `yaml:"pre_install_query"`
244+
PostInstallScript *rawPathRef `yaml:"post_install_script"`
245+
}
246+
235247
type rawControls struct {
236248
MacOSSettings struct {
237249
CustomSettings []rawProfileRef `yaml:"custom_settings"`
@@ -522,19 +534,44 @@ func resolveQueryRef(baseDir, refPath, parentFile string) ([]ParsedQuery, []Pars
522534
return items, nil
523535
}
524536

525-
// resolveSoftwareRef reads a software package YAML file.
537+
// resolveSoftwareRef reads a software package YAML file and resolves any
538+
// install_script, uninstall_script, pre_install_query, or post_install_script
539+
// path: references within it. The resolved paths are tracked in SourceFiles
540+
// so the changed-file filter can match script-only MR changes.
526541
func resolveSoftwareRef(baseDir, refPath, parentFile string) ([]ParsedSoftwarePackage, []ParseError) {
527542
data, resolved, errs := readYAMLRef(baseDir, refPath, parentFile, "software ")
528543
if errs != nil {
529544
return nil, errs
530545
}
531546

532-
var pkg ParsedSoftwarePackage
533-
if err := yaml.Unmarshal(data, &pkg); err != nil {
547+
var raw rawSoftwarePackage
548+
if err := yaml.Unmarshal(data, &raw); err != nil {
534549
return nil, []ParseError{{File: resolved, Message: fmt.Sprintf("YAML parse error: %s", err)}}
535550
}
536-
pkg.SourceFile = resolved
537-
return []ParsedSoftwarePackage{pkg}, nil
551+
552+
pkg := ParsedSoftwarePackage{
553+
URL: raw.URL,
554+
HashSHA256: raw.HashSHA256,
555+
SelfService: raw.SelfService,
556+
SourceFile: resolved,
557+
}
558+
559+
pkgDir := filepath.Dir(resolved)
560+
for _, ref := range []*rawPathRef{raw.InstallScript, raw.UninstallScript, raw.PreInstallQuery, raw.PostInstallScript} {
561+
if ref == nil || ref.Path == "" {
562+
continue
563+
}
564+
scriptPath := filepath.Join(pkgDir, ref.Path)
565+
if repoRoot != "" {
566+
if err := safePath(repoRoot, scriptPath); err != nil {
567+
errs = append(errs, ParseError{File: resolved, Message: err.Error()})
568+
continue
569+
}
570+
}
571+
pkg.SourceFiles = append(pkg.SourceFiles, scriptPath)
572+
}
573+
574+
return []ParsedSoftwarePackage{pkg}, errs
538575
}
539576

540577
// resolveFleetApp resolves path: references in a fleet-maintained app entry,

‎internal/parser/parser_test.go‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -523,6 +523,57 @@ func TestValidateLogging(t *testing.T) {
523523
}
524524
}
525525

526+
// TestParseSoftwarePackageScriptSourceFiles verifies that install_script,
527+
// uninstall_script, and other path: refs inside a software package YAML are
528+
// resolved and tracked in SourceFiles. This enables the changed-file filter
529+
// to match MRs that only modify scripts (no YAML changes).
530+
func TestParseSoftwarePackageScriptSourceFiles(t *testing.T) {
531+
root := testutil.TestdataRoot(t)
532+
533+
repo, err := ParseRepo(root, []string{"Workstations"}, "")
534+
if err != nil {
535+
t.Fatalf("ParseRepo: %v", err)
536+
}
537+
if len(repo.Errors) > 0 {
538+
for _, e := range repo.Errors {
539+
t.Logf("parse error: %s", e)
540+
}
541+
t.Fatalf("expected zero parse errors, got %d", len(repo.Errors))
542+
}
543+
544+
ws := &repo.Teams[0]
545+
var exApp *ParsedSoftwarePackage
546+
for i := range ws.Software.Packages {
547+
if strings.Contains(ws.Software.Packages[i].RefPath, "example-app") {
548+
exApp = &ws.Software.Packages[i]
549+
break
550+
}
551+
}
552+
if exApp == nil {
553+
t.Fatal("example-app package not found")
554+
}
555+
556+
if len(exApp.SourceFiles) != 2 {
557+
t.Fatalf("expected 2 SourceFiles (install.ps1, uninstall.ps1), got %d: %v", len(exApp.SourceFiles), exApp.SourceFiles)
558+
}
559+
560+
hasInstall, hasUninstall := false, false
561+
for _, sf := range exApp.SourceFiles {
562+
if strings.HasSuffix(sf, "install.ps1") && !strings.Contains(sf, "uninstall") {
563+
hasInstall = true
564+
}
565+
if strings.HasSuffix(sf, "uninstall.ps1") {
566+
hasUninstall = true
567+
}
568+
}
569+
if !hasInstall {
570+
t.Errorf("expected install.ps1 in SourceFiles, got %v", exApp.SourceFiles)
571+
}
572+
if !hasUninstall {
573+
t.Errorf("expected uninstall.ps1 in SourceFiles, got %v", exApp.SourceFiles)
574+
}
575+
}
576+
526577
// TestParseRepoDuplicateSoftwareRefs verifies duplicate software ref detection.
527578
func TestParseRepoDuplicateSoftwareRefs(t *testing.T) {
528579
root := t.TempDir()
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
11
url: https://downloads.example.com/example-app-2.0.msi
22
hash_sha256: a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2
33
self_service: false
4+
install_script:
5+
path: install.ps1
6+
uninstall_script:
7+
path: uninstall.ps1
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
$installer = "$env:INSTALLER_PATH"
2+
Start-Process msiexec.exe -ArgumentList "/i `"$installer`" /qn /norestart" -Wait
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
$app = Get-WmiObject -Class Win32_Product | Where-Object { $_.Name -match "Example App" }
2+
if ($app) { $app.Uninstall() }

0 commit comments

Comments
 (0)