Skip to content

Commit 0a269a3

Browse files
magerstammagerstam
committed
Added go-tests, and fixed BL config parsing
1 parent 88455fe commit 0a269a3

8 files changed

Lines changed: 444 additions & 38 deletions

File tree

‎internal/image/imageinspect/bootloader_config.go‎

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -58,15 +58,6 @@ func parseGrubConfigContent(content string) BootloaderConfig {
5858
cfg.ConfigRaw["grub.cfg"] = content
5959
}
6060

61-
// Extract UUID-like tokens from config content
62-
uuids := extractUUIDsFromString(content)
63-
for _, uuid := range uuids {
64-
cfg.UUIDReferences = append(cfg.UUIDReferences, UUIDReference{
65-
UUID: uuid,
66-
Context: "grub_config",
67-
})
68-
}
69-
7061
// Extract critical metadata from the config
7162
lines := strings.Split(content, "\n")
7263
var configfilePath string
@@ -299,9 +290,12 @@ func parseGrubMenuEntry(menuLine string) *BootEntry {
299290
}
300291
}
301292

302-
// Fallback: extract whatever is between menuentry and {
303-
if idx := strings.Index(menuLine, "{"); idx > 0 {
304-
entry.Name = strings.TrimSpace(menuLine[9:idx])
293+
// Fallback: extract whatever is between "menuentry" and "{", if safely available.
294+
prefix := "menuentry"
295+
if strings.HasPrefix(menuLine, prefix) {
296+
if idx := strings.Index(menuLine, "{"); idx > len(prefix) {
297+
entry.Name = strings.TrimSpace(menuLine[len(prefix):idx])
298+
}
305299
}
306300

307301
return entry

‎internal/image/imageinspect/bootloader_config_test.go‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,20 @@ func TestCompareBootloaderConfigs(t *testing.T) {
151151
t.Errorf("Expected 2 boot entry changes (1 modified, 1 added), got %d", len(diff.BootEntryChanges))
152152
}
153153

154+
modifiedFound := false
155+
for _, change := range diff.BootEntryChanges {
156+
if change.Status != "modified" || change.Name != "Linux (old)" {
157+
continue
158+
}
159+
modifiedFound = true
160+
if change.InitrdFrom != "/initrd-5.14" || change.InitrdTo != "/initrd-5.15" {
161+
t.Errorf("Expected initrd change /initrd-5.14 -> /initrd-5.15, got %q -> %q", change.InitrdFrom, change.InitrdTo)
162+
}
163+
}
164+
if !modifiedFound {
165+
t.Errorf("Expected modified boot entry change for Linux (old)")
166+
}
167+
154168
// Should detect kernel reference changes
155169
// Old vmlinuz-5.14 removed, vmlinuz-5.15 modified, vmlinuz-5.16 added = 3 changes
156170
if len(diff.KernelRefChanges) != 3 {

‎internal/image/imageinspect/bootloader_efi.go‎

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -270,27 +270,3 @@ func parseOSRelease(raw string) (map[string]string, []KeyValue) {
270270

271271
return m, sorted
272272
}
273-
274-
// BootloaderConfigPaths returns the filesystem paths to check for bootloader config files
275-
// based on the bootloader kind.
276-
func BootloaderConfigPaths(kind BootloaderKind) []string {
277-
switch kind {
278-
case BootloaderGrub:
279-
return []string{
280-
"/EFI/grub/grub.cfg",
281-
"/efi/grub/grub.cfg",
282-
"/boot/grub/grub.cfg",
283-
"/boot/grub2/grub.cfg",
284-
"/grub/grub.cfg",
285-
}
286-
case BootloaderSystemdBoot:
287-
return []string{
288-
"/loader/loader.conf",
289-
"/loader/entries/",
290-
"/EFI/systemd/loader.conf",
291-
"/efi/systemd/loader.conf",
292-
}
293-
default:
294-
return nil
295-
}
296-
}

‎internal/image/imageinspect/compare.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,8 @@ type BootEntryChange struct {
226226
Status string `json:"status" yaml:"status"` // "added", "removed", "modified"
227227
KernelFrom string `json:"kernelFrom,omitempty" yaml:"kernelFrom,omitempty"`
228228
KernelTo string `json:"kernelTo,omitempty" yaml:"kernelTo,omitempty"`
229+
InitrdFrom string `json:"initrdFrom,omitempty" yaml:"initrdFrom,omitempty"`
230+
InitrdTo string `json:"initrdTo,omitempty" yaml:"initrdTo,omitempty"`
229231
CmdlineFrom string `json:"cmdlineFrom,omitempty" yaml:"cmdlineFrom,omitempty"`
230232
CmdlineTo string `json:"cmdlineTo,omitempty" yaml:"cmdlineTo,omitempty"`
231233
}
@@ -1088,6 +1090,8 @@ func tallyBootloaderConfigDiff(t *diffTally, diff *BootloaderConfigDiff, efiKey
10881090
// vs just the display name changed
10891091
if be.KernelFrom != be.KernelTo {
10901092
t.addMeaningful(1, "BootConfig["+efiKey+"] boot entry kernel changed: "+be.Name)
1093+
} else if be.InitrdFrom != be.InitrdTo {
1094+
t.addMeaningful(1, "BootConfig["+efiKey+"] boot entry initrd changed: "+be.Name)
10911095
} else if normalizeKernelCmdline(be.CmdlineFrom) != normalizeKernelCmdline(be.CmdlineTo) {
10921096
t.addMeaningful(1, "BootConfig["+efiKey+"] boot entry cmdline changed: "+be.Name)
10931097
} else {

‎internal/image/imageinspect/compare_efi.go‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -364,23 +364,27 @@ func compareBootEntries(a, b []BootEntry) []BootEntryChange {
364364
Name: name,
365365
Status: "removed",
366366
KernelFrom: entryA.Kernel,
367+
InitrdFrom: entryA.Initrd,
367368
CmdlineFrom: entryA.Cmdline,
368369
})
369370
case entryA == nil && entryB != nil:
370371
changes = append(changes, BootEntryChange{
371372
Name: name,
372373
Status: "added",
373374
KernelTo: entryB.Kernel,
375+
InitrdTo: entryB.Initrd,
374376
CmdlineTo: entryB.Cmdline,
375377
})
376378
case entryA != nil && entryB != nil:
377379
// Check for changes within the entry
378-
if entryA.Kernel != entryB.Kernel || entryA.Cmdline != entryB.Cmdline {
380+
if entryA.Kernel != entryB.Kernel || entryA.Initrd != entryB.Initrd || entryA.Cmdline != entryB.Cmdline {
379381
changes = append(changes, BootEntryChange{
380382
Name: name,
381383
Status: "modified",
382384
KernelFrom: entryA.Kernel,
383385
KernelTo: entryB.Kernel,
386+
InitrdFrom: entryA.Initrd,
387+
InitrdTo: entryB.Initrd,
384388
CmdlineFrom: entryA.Cmdline,
385389
CmdlineTo: entryB.Cmdline,
386390
})
@@ -471,6 +475,16 @@ func compareUUIDReferences(a, b []UUIDReference) []UUIDRefChange {
471475
MismatchTo: refB.Mismatch,
472476
})
473477
}
478+
if refA.Context != refB.Context {
479+
if refA.Mismatch == refB.Mismatch {
480+
changes = append(changes, UUIDRefChange{
481+
UUID: uuid,
482+
Status: "modified",
483+
ContextFrom: refA.Context,
484+
ContextTo: refB.Context,
485+
})
486+
}
487+
}
474488
}
475489
}
476490

‎internal/image/imageinspect/compare_test.go‎

Lines changed: 199 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package imageinspect
22

33
import (
4+
"strings"
45
"testing"
56
)
67

@@ -975,3 +976,201 @@ func TestCompareImages_EFISigningStatusChanges(t *testing.T) {
975976
t.Fatalf("expected signed false->true, got %v->%v", res.Diff.EFIBinaries.Modified[0].From.Signed, res.Diff.EFIBinaries.Modified[0].To.Signed)
976977
}
977978
}
979+
func TestCompareUUIDReferences_Branches(t *testing.T) {
980+
from := []UUIDReference{
981+
{UUID: "00000000-0000-0000-0000-000000000001", Context: "kernel_cmdline", Mismatch: false}, // removed
982+
{UUID: "00000000-0000-0000-0000-000000000002", Context: "kernel_cmdline", Mismatch: false}, // mismatch changed
983+
{UUID: "00000000-0000-0000-0000-000000000003", Context: "grub_search", Mismatch: false}, // context changed only
984+
{UUID: "00000000-0000-0000-0000-000000000004", Context: "root_device", Mismatch: true}, // unchanged
985+
}
986+
to := []UUIDReference{
987+
{UUID: "00000000-0000-0000-0000-000000000002", Context: "root_device", Mismatch: true},
988+
{UUID: "00000000-0000-0000-0000-000000000003", Context: "kernel_cmdline", Mismatch: false},
989+
{UUID: "00000000-0000-0000-0000-000000000004", Context: "root_device", Mismatch: true},
990+
{UUID: "00000000-0000-0000-0000-000000000005", Context: "kernel_cmdline", Mismatch: false}, // added
991+
}
992+
993+
changes := compareUUIDReferences(from, to)
994+
if len(changes) != 4 {
995+
t.Fatalf("expected 4 UUID reference changes, got %d: %+v", len(changes), changes)
996+
}
997+
998+
byUUID := map[string]UUIDRefChange{}
999+
for _, ch := range changes {
1000+
byUUID[ch.UUID] = ch
1001+
}
1002+
1003+
removed, ok := byUUID["00000000-0000-0000-0000-000000000001"]
1004+
if !ok || removed.Status != "removed" || removed.ContextFrom != "kernel_cmdline" || removed.MismatchFrom {
1005+
t.Fatalf("unexpected removed UUID change: %+v", removed)
1006+
}
1007+
1008+
added, ok := byUUID["00000000-0000-0000-0000-000000000005"]
1009+
if !ok || added.Status != "added" || added.ContextTo != "kernel_cmdline" || added.MismatchTo {
1010+
t.Fatalf("unexpected added UUID change: %+v", added)
1011+
}
1012+
1013+
mismatchChanged, ok := byUUID["00000000-0000-0000-0000-000000000002"]
1014+
if !ok || mismatchChanged.Status != "modified" || mismatchChanged.MismatchFrom != false || mismatchChanged.MismatchTo != true {
1015+
t.Fatalf("unexpected mismatch-changed UUID entry: %+v", mismatchChanged)
1016+
}
1017+
1018+
contextChanged, ok := byUUID["00000000-0000-0000-0000-000000000003"]
1019+
if !ok || contextChanged.Status != "modified" || contextChanged.ContextFrom != "grub_search" || contextChanged.ContextTo != "kernel_cmdline" {
1020+
t.Fatalf("unexpected context-changed UUID entry: %+v", contextChanged)
1021+
}
1022+
}
1023+
1024+
func TestUkiOnlyVolatile_Branches(t *testing.T) {
1025+
tests := []struct {
1026+
name string
1027+
mod ModifiedEFIBinaryEvidence
1028+
want bool
1029+
}{
1030+
{
1031+
name: "kernel hash changed is meaningful",
1032+
mod: ModifiedEFIBinaryEvidence{
1033+
UKI: &UKIDiff{KernelSHA256: &ValueDiff[string]{From: "k1", To: "k2"}, Changed: true},
1034+
},
1035+
want: false,
1036+
},
1037+
{
1038+
name: "uname hash changed is meaningful",
1039+
mod: ModifiedEFIBinaryEvidence{
1040+
UKI: &UKIDiff{UnameSHA256: &ValueDiff[string]{From: "u1", To: "u2"}, Changed: true},
1041+
},
1042+
want: false,
1043+
},
1044+
{
1045+
name: "cmdline changed only by UUID is volatile",
1046+
mod: ModifiedEFIBinaryEvidence{
1047+
From: EFIBinaryEvidence{Cmdline: "root=UUID=11111111-1111-1111-1111-111111111111 ro quiet"},
1048+
To: EFIBinaryEvidence{Cmdline: "root=UUID=22222222-2222-2222-2222-222222222222 ro quiet"},
1049+
UKI: &UKIDiff{CmdlineSHA256: &ValueDiff[string]{From: "c1", To: "c2"}, Changed: true},
1050+
},
1051+
want: true,
1052+
},
1053+
{
1054+
name: "cmdline semantic change is meaningful",
1055+
mod: ModifiedEFIBinaryEvidence{
1056+
From: EFIBinaryEvidence{Cmdline: "root=UUID=11111111-1111-1111-1111-111111111111 ro quiet"},
1057+
To: EFIBinaryEvidence{Cmdline: "root=UUID=22222222-2222-2222-2222-222222222222 rw quiet"},
1058+
UKI: &UKIDiff{CmdlineSHA256: &ValueDiff[string]{From: "c1", To: "c2"}, Changed: true},
1059+
},
1060+
want: false,
1061+
},
1062+
{
1063+
name: "no uki details defaults volatile",
1064+
mod: ModifiedEFIBinaryEvidence{},
1065+
want: true,
1066+
},
1067+
}
1068+
1069+
for _, tc := range tests {
1070+
t.Run(tc.name, func(t *testing.T) {
1071+
got := ukiOnlyVolatile(tc.mod)
1072+
if got != tc.want {
1073+
t.Fatalf("expected %v, got %v", tc.want, got)
1074+
}
1075+
})
1076+
}
1077+
}
1078+
func TestCompareVerity_NilCasesAndFieldChanges(t *testing.T) {
1079+
if got := compareVerity(nil, nil); got != nil {
1080+
t.Fatalf("expected nil diff when both verity values are nil")
1081+
}
1082+
1083+
added := compareVerity(nil, &VerityInfo{Enabled: true, Method: "systemd-verity", RootDevice: "/dev/vda2", HashPartition: 3})
1084+
if added == nil || !added.Changed || added.Added == nil {
1085+
t.Fatalf("expected added verity diff")
1086+
}
1087+
1088+
removed := compareVerity(&VerityInfo{Enabled: true, Method: "systemd-verity"}, nil)
1089+
if removed == nil || !removed.Changed || removed.Removed == nil {
1090+
t.Fatalf("expected removed verity diff")
1091+
}
1092+
1093+
from := &VerityInfo{
1094+
Enabled: true,
1095+
Method: "systemd-verity",
1096+
RootDevice: "/dev/vda2",
1097+
HashPartition: 3,
1098+
}
1099+
to := &VerityInfo{
1100+
Enabled: false,
1101+
Method: "custom-initramfs",
1102+
RootDevice: "/dev/vda3",
1103+
HashPartition: 4,
1104+
}
1105+
1106+
changed := compareVerity(from, to)
1107+
if changed == nil || !changed.Changed {
1108+
t.Fatalf("expected changed verity diff")
1109+
}
1110+
if changed.Enabled == nil || changed.Enabled.From != true || changed.Enabled.To != false {
1111+
t.Fatalf("expected enabled diff to be set")
1112+
}
1113+
if changed.Method == nil || changed.Method.From != "systemd-verity" || changed.Method.To != "custom-initramfs" {
1114+
t.Fatalf("expected method diff to be set")
1115+
}
1116+
if changed.RootDevice == nil || changed.RootDevice.From != "/dev/vda2" || changed.RootDevice.To != "/dev/vda3" {
1117+
t.Fatalf("expected root device diff to be set")
1118+
}
1119+
if changed.HashPartition == nil || changed.HashPartition.From != 3 || changed.HashPartition.To != 4 {
1120+
t.Fatalf("expected hash partition diff to be set")
1121+
}
1122+
}
1123+
1124+
func TestTallyBootloaderConfigDiff_ClassificationCoverage(t *testing.T) {
1125+
tally := &diffTally{}
1126+
d := &BootloaderConfigDiff{
1127+
ConfigFileChanges: []ConfigFileChange{
1128+
{Path: "/boot/grub/grub.cfg", Status: "added"},
1129+
{Path: "/loader/entries/old.conf", Status: "removed"},
1130+
{Path: "/loader/entries/default.conf", Status: "modified"},
1131+
},
1132+
BootEntryChanges: []BootEntryChange{
1133+
{Name: "entry-added", Status: "added"},
1134+
{Name: "entry-removed", Status: "removed"},
1135+
{Name: "entry-kernel", Status: "modified", KernelFrom: "/vmlinuz-a", KernelTo: "/vmlinuz-b"},
1136+
{Name: "entry-initrd", Status: "modified", KernelFrom: "/vmlinuz", KernelTo: "/vmlinuz", InitrdFrom: "/initrd-a", InitrdTo: "/initrd-b"},
1137+
{Name: "entry-cmdline", Status: "modified", KernelFrom: "/vmlinuz", KernelTo: "/vmlinuz", InitrdFrom: "/initrd", InitrdTo: "/initrd", CmdlineFrom: "root=UUID=11111111-1111-1111-1111-111111111111 ro", CmdlineTo: "root=UUID=22222222-2222-2222-2222-222222222222 rw"},
1138+
{Name: "entry-meta", Status: "modified", KernelFrom: "/vmlinuz", KernelTo: "/vmlinuz", InitrdFrom: "/initrd", InitrdTo: "/initrd", CmdlineFrom: " root=UUID=aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa ro ", CmdlineTo: "root=UUID=bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb ro"},
1139+
},
1140+
KernelRefChanges: []KernelRefChange{
1141+
{Path: "EFI/Linux/new.efi", Status: "added"},
1142+
{Path: "EFI/Linux/old.efi", Status: "removed"},
1143+
{Path: "EFI/Linux/uuid.efi", Status: "modified", UUIDFrom: "u1", UUIDTo: "u2"},
1144+
{Path: "EFI/Linux/meta.efi", Status: "modified", UUIDFrom: "same", UUIDTo: "same"},
1145+
},
1146+
UUIDReferenceChanges: []UUIDRefChange{
1147+
{UUID: "a", Status: "added", MismatchTo: true},
1148+
{UUID: "b", Status: "added", MismatchTo: false},
1149+
{UUID: "c", Status: "removed", MismatchFrom: true},
1150+
{UUID: "d", Status: "removed", MismatchFrom: false},
1151+
{UUID: "e", Status: "modified", MismatchFrom: false, MismatchTo: true},
1152+
{UUID: "f", Status: "modified", MismatchFrom: false, MismatchTo: false},
1153+
},
1154+
NotesAdded: []string{"note-a", "note-b"},
1155+
NotesRemoved: []string{"note-c"},
1156+
}
1157+
1158+
tallyBootloaderConfigDiff(tally, d, "EFI/BOOT/BOOTX64.EFI")
1159+
1160+
if tally.meaningful != 14 {
1161+
t.Fatalf("expected meaningful=14, got %d (reasons=%v)", tally.meaningful, tally.mReasons)
1162+
}
1163+
if tally.volatile != 8 {
1164+
t.Fatalf("expected volatile=8, got %d (reasons=%v)", tally.volatile, tally.vReasons)
1165+
}
1166+
1167+
meaningfulJoined := strings.Join(tally.mReasons, "\n")
1168+
volatileJoined := strings.Join(tally.vReasons, "\n")
1169+
1170+
if !strings.Contains(meaningfulJoined, "boot entry initrd changed: entry-initrd") {
1171+
t.Fatalf("expected initrd change to be classified meaningful, reasons=%v", tally.mReasons)
1172+
}
1173+
if !strings.Contains(volatileJoined, "boot entry metadata changed: entry-meta") {
1174+
t.Fatalf("expected metadata-only boot entry change to be classified volatile, reasons=%v", tally.vReasons)
1175+
}
1176+
}

‎internal/image/imageinspect/renderer_text.go‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -741,8 +741,17 @@ func renderBootloaderConfigDetails(w io.Writer, efiPath string, cfg *BootloaderC
741741

742742
// Config file hashes and content preview
743743
if len(cfg.ConfigFiles) > 0 {
744+
fmt.Fprintf(w, "EFI Path: %s\n", efiPath)
744745
fmt.Fprintln(w, "Config files:")
745-
for path, hash := range cfg.ConfigFiles {
746+
747+
// Collect and sort paths for deterministic output
748+
paths := make([]string, 0, len(cfg.ConfigFiles))
749+
for path := range cfg.ConfigFiles {
750+
paths = append(paths, path)
751+
}
752+
sort.Strings(paths)
753+
for _, path := range paths {
754+
hash := cfg.ConfigFiles[path]
746755
fmt.Fprintf(w, " %s: %s\n", path, shortHash(hash))
747756

748757
// Show raw config content (first 30 lines or up to 2KB)
@@ -880,6 +889,9 @@ func renderBootloaderConfigDiffText(w io.Writer, diff *BootloaderConfigDiff, ind
880889
if change.KernelFrom != change.KernelTo {
881890
fmt.Fprintf(w, "%s kernel: %s -> %s\n", indent, change.KernelFrom, change.KernelTo)
882891
}
892+
if change.InitrdFrom != change.InitrdTo {
893+
fmt.Fprintf(w, "%s initrd: %s -> %s\n", indent, change.InitrdFrom, change.InitrdTo)
894+
}
883895
if change.CmdlineFrom != change.CmdlineTo {
884896
if len(change.CmdlineFrom) > 80 {
885897
fmt.Fprintf(w, "%s cmdline: %s... -> %s...\n", indent, change.CmdlineFrom[:77], change.CmdlineTo[:77])

0 commit comments

Comments
 (0)