Skip to content

Commit eadbdda

Browse files
frezbofrezbo
committed
fix: uefi boot order setting
Add missing test cases, which identified a bug in which entry was selected for removal. Signed-off-by: Noel Georgi <git@frezbo.dev>
1 parent cd9fb27 commit eadbdda

File tree

2 files changed

+121
-23
lines changed

2 files changed

+121
-23
lines changed

‎internal/app/machined/pkg/runtime/v1alpha1/bootloader/sdboot/efivars.go‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,10 @@ func CreateBootEntry(rw efivarfs.ReadWriter, blkidInfo *blkid.Info, printf func(
147147
}
148148
}
149149

150+
// we sort the indexes to make sure we always keep the lowest index
151+
// when removing duplicate Talos Linux UKI boot entries
152+
slices.Sort(existingTalosBootEntryIndexes)
153+
150154
printf("Found existing Talos Linux UKI boot entries: %v", existingTalosBootEntryIndexes)
151155

152156
// Remove any existing Talos Linux UKI boot entries from the BootOrder.
@@ -217,19 +221,19 @@ func CreateBootEntry(rw efivarfs.ReadWriter, blkidInfo *blkid.Info, printf func(
217221

218222
printf("created Talos Linux UKI boot entry at index %d", nextMinimalIndex)
219223

224+
if len(bootOrder) > 0 && bootOrder[0] == uint16(nextMinimalIndex) && !slices.Contains(bootOrder[1:], uint16(nextMinimalIndex)) {
225+
// Talos Linux UKI boot entry is already first in the BootOrder
226+
printf("Talos Linux UKI boot entry at index %d is already first in BootOrder: %v", nextMinimalIndex, bootOrder)
227+
228+
return nil
229+
}
230+
220231
// if we have the new Talos Linux UKI boot entry index in the boot order already, we need to remove it first
221232
// to avoid having it twice in the boot order
222233
bootOrderUnique = slices.DeleteFunc(bootOrderUnique, func(i uint16) bool {
223234
return i == uint16(nextMinimalIndex)
224235
})
225236

226-
if len(bootOrderUnique) > 0 && bootOrderUnique[0] == uint16(nextMinimalIndex) {
227-
// Talos Linux UKI boot entry is already first in the BootOrder
228-
printf("Talos Linux UKI boot entry at index %d is already first in BootOrder: %v", nextMinimalIndex, bootOrderUnique)
229-
230-
return nil
231-
}
232-
233237
bootOrderUnique = slices.Insert(bootOrderUnique, 0, uint16(nextMinimalIndex))
234238

235239
printf("setting Talos Linux UKI boot entry at index %d as first in BootOrder: %v", nextMinimalIndex, bootOrderUnique)

‎internal/app/machined/pkg/runtime/v1alpha1/bootloader/sdboot/efivars_test.go‎

Lines changed: 110 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
package sdboot_test
66

77
import (
8+
"fmt"
9+
"strings"
810
"testing"
911

1012
"github.com/google/uuid"
@@ -17,6 +19,14 @@ import (
1719
"github.com/siderolabs/talos/pkg/machinery/constants"
1820
)
1921

22+
type mockLogger struct {
23+
strings.Builder
24+
}
25+
26+
func (m *mockLogger) Printf(format string, v ...any) {
27+
m.WriteString(fmt.Sprintf(format, v...) + "\n")
28+
}
29+
2030
func TestSetBootEntry(t *testing.T) {
2131
t.Parallel()
2232

@@ -30,6 +40,16 @@ func TestSetBootEntry(t *testing.T) {
3040
defaultBootEntry, err := loadOption.Marshal()
3141
require.NoError(t, err)
3242

43+
talosLoadOption := &efivarfs.LoadOption{
44+
Description: sdboot.TalosBootEntryDescription,
45+
FilePath: efivarfs.DevicePath{
46+
efivarfs.FilePath("/EFI/TALOS/UKI.efi"),
47+
},
48+
}
49+
50+
talosBootEntry, err := talosLoadOption.Marshal()
51+
require.NoError(t, err)
52+
3353
blkidInfo := &blkid.Info{
3454
ProbeResult: blkid.ProbeResult{
3555
Name: "loop0",
@@ -53,14 +73,16 @@ func TestSetBootEntry(t *testing.T) {
5373
name string
5474
efivarfsMock *efivarfs.Mock
5575

56-
expectedBootOrder efivarfs.BootOrder
57-
expectedEntries map[int]string
76+
expectedMessageContains string
77+
expectedBootOrder efivarfs.BootOrder
78+
expectedEntries map[int]string
5879
}{
5980
{
6081
name: "empty efivarfs", // both BootOrder and BootEntries are initially empty
6182
efivarfsMock: &efivarfs.Mock{},
6283

63-
expectedBootOrder: efivarfs.BootOrder{0},
84+
expectedMessageContains: "setting Talos Linux UKI boot entry at index 0 as first in BootOrder: [0]",
85+
expectedBootOrder: efivarfs.BootOrder{0},
6486
expectedEntries: map[int]string{
6587
0: sdboot.TalosBootEntryDescription,
6688
},
@@ -78,7 +100,8 @@ func TestSetBootEntry(t *testing.T) {
78100
},
79101
},
80102

81-
expectedBootOrder: efivarfs.BootOrder{1},
103+
expectedMessageContains: "setting Talos Linux UKI boot entry at index 1 as first in BootOrder: [1]",
104+
expectedBootOrder: efivarfs.BootOrder{1},
82105
expectedEntries: map[int]string{
83106
0: "Default Boot Entry",
84107
1: sdboot.TalosBootEntryDescription,
@@ -97,7 +120,8 @@ func TestSetBootEntry(t *testing.T) {
97120
},
98121
},
99122

100-
expectedBootOrder: efivarfs.BootOrder{0},
123+
expectedMessageContains: "Talos Linux UKI boot entry at index 0 is already first in BootOrder: [0]",
124+
expectedBootOrder: efivarfs.BootOrder{0},
101125
expectedEntries: map[int]string{
102126
0: sdboot.TalosBootEntryDescription,
103127
},
@@ -119,7 +143,8 @@ func TestSetBootEntry(t *testing.T) {
119143
},
120144
},
121145

122-
expectedBootOrder: efivarfs.BootOrder{1, 0},
146+
expectedMessageContains: "setting Talos Linux UKI boot entry at index 1 as first in BootOrder: [1 0]",
147+
expectedBootOrder: efivarfs.BootOrder{1, 0},
123148
expectedEntries: map[int]string{
124149
0: "Default Boot Entry",
125150
1: sdboot.TalosBootEntryDescription,
@@ -142,7 +167,8 @@ func TestSetBootEntry(t *testing.T) {
142167
},
143168
},
144169

145-
expectedBootOrder: efivarfs.BootOrder{1},
170+
expectedMessageContains: "Talos Linux UKI boot entry at index 1 is already first in BootOrder: [1]",
171+
expectedBootOrder: efivarfs.BootOrder{1},
146172
expectedEntries: map[int]string{
147173
0: "Default Boot Entry",
148174
1: sdboot.TalosBootEntryDescription,
@@ -169,8 +195,8 @@ func TestSetBootEntry(t *testing.T) {
169195
},
170196
},
171197

172-
// in this case we cleanup the existing BootOrder since it points to a non-existing entry
173-
expectedBootOrder: efivarfs.BootOrder{1, 0, 3, 2},
198+
expectedMessageContains: "Talos Linux UKI boot entry at index 1 is already first in BootOrder: [1 0 3 2]",
199+
expectedBootOrder: efivarfs.BootOrder{1, 0, 3, 2},
174200
expectedEntries: map[int]string{
175201
0: "Default Boot Entry",
176202
1: sdboot.TalosBootEntryDescription,
@@ -198,8 +224,8 @@ func TestSetBootEntry(t *testing.T) {
198224
},
199225
},
200226

201-
// in this case we cleanup the existing BootOrder since it points to a non-existing entry
202-
expectedBootOrder: efivarfs.BootOrder{1, 5, 0, 3, 2},
227+
expectedMessageContains: "setting Talos Linux UKI boot entry at index 1 as first in BootOrder: [1 5 0 3 2]",
228+
expectedBootOrder: efivarfs.BootOrder{1, 5, 0, 3, 2},
203229
expectedEntries: map[int]string{
204230
0: "Default Boot Entry",
205231
1: sdboot.TalosBootEntryDescription,
@@ -219,12 +245,76 @@ func TestSetBootEntry(t *testing.T) {
219245
},
220246
},
221247

222-
// in this case we cleanup the existing BootOrder since it points to a non-existing entry
223-
expectedBootOrder: efivarfs.BootOrder{0, 1, 3, 2},
248+
expectedMessageContains: "setting Talos Linux UKI boot entry at index 0 as first in BootOrder: [0 1 3 2]",
249+
expectedBootOrder: efivarfs.BootOrder{0, 1, 3, 2},
224250
expectedEntries: map[int]string{
225251
0: sdboot.TalosBootEntryDescription,
226252
},
227253
},
254+
{
255+
name: "duplicate Talos entries in BootEntries", // BootOrder has unique entries but there are multiple Talos BootEntries
256+
efivarfsMock: &efivarfs.Mock{
257+
Variables: map[uuid.UUID]map[string]efivarfs.MockVariable{
258+
efivarfs.ScopeGlobal: {
259+
"BootOrder": {
260+
Attrs: 0,
261+
Data: []byte{0x01, 0x00, 0x02, 0x00}, // BootOrder: [1, 2]
262+
},
263+
"Boot0000": {
264+
Attrs: 0,
265+
Data: defaultBootEntry,
266+
},
267+
"Boot0001": {
268+
Attrs: 0,
269+
Data: talosBootEntry,
270+
},
271+
"Boot0002": {
272+
Attrs: 0,
273+
Data: talosBootEntry,
274+
},
275+
},
276+
},
277+
},
278+
279+
expectedMessageContains: "Removing existing Talos Linux UKI boot entry at index 2",
280+
expectedBootOrder: efivarfs.BootOrder{1},
281+
expectedEntries: map[int]string{
282+
0: "Default Boot Entry",
283+
1: sdboot.TalosBootEntryDescription,
284+
},
285+
},
286+
{
287+
name: "duplicate Talos entries in BootEntries and duplicate BootOrder", // BootOrder has duplicate entries and there are multiple Talos BootEntries
288+
efivarfsMock: &efivarfs.Mock{
289+
Variables: map[uuid.UUID]map[string]efivarfs.MockVariable{
290+
efivarfs.ScopeGlobal: {
291+
"BootOrder": {
292+
Attrs: 0,
293+
Data: []byte{0x01, 0x00, 0x02, 0x00, 0x02, 0x00, 0x00, 0x00}, // BootOrder: [1, 2, 2, 0]
294+
},
295+
"Boot0000": {
296+
Attrs: 0,
297+
Data: defaultBootEntry,
298+
},
299+
"Boot0001": {
300+
Attrs: 0,
301+
Data: talosBootEntry,
302+
},
303+
"Boot0002": {
304+
Attrs: 0,
305+
Data: talosBootEntry,
306+
},
307+
},
308+
},
309+
},
310+
311+
expectedMessageContains: "Removing existing Talos Linux UKI boot entry at index 2",
312+
expectedBootOrder: efivarfs.BootOrder{1, 0},
313+
expectedEntries: map[int]string{
314+
0: "Default Boot Entry",
315+
1: sdboot.TalosBootEntryDescription,
316+
},
317+
},
228318
{
229319
name: "duplicate entries in BootOrder and BootEntries", // BootOrder has duplicate entries and has multiple BootEntries
230320
efivarfsMock: &efivarfs.Mock{
@@ -254,8 +344,8 @@ func TestSetBootEntry(t *testing.T) {
254344
},
255345
},
256346

257-
// in this case we cleanup the existing BootOrder since it points to a non-existing entry
258-
expectedBootOrder: efivarfs.BootOrder{2, 1, 0, 3},
347+
expectedMessageContains: "setting Talos Linux UKI boot entry at index 2 as first in BootOrder: [2 1 0 3]",
348+
expectedBootOrder: efivarfs.BootOrder{2, 1, 0, 3},
259349
expectedEntries: map[int]string{
260350
0: "Default Boot Entry",
261351
1: "Default Boot Entry",
@@ -272,11 +362,15 @@ func TestSetBootEntry(t *testing.T) {
272362
t.Fatal("efivarfsMock must be set")
273363
}
274364

275-
require.NoError(t, sdboot.CreateBootEntry(testData.efivarfsMock, blkidInfo, t.Logf, "test-entry"))
365+
logger := &mockLogger{}
366+
367+
require.NoError(t, sdboot.CreateBootEntry(testData.efivarfsMock, blkidInfo, logger.Printf, "test-entry"))
276368

277369
bootOrder, err := efivarfs.GetBootOrder(testData.efivarfsMock)
278370
require.NoError(t, err)
279371

372+
require.Contains(t, logger.String(), testData.expectedMessageContains, "expected log message not found")
373+
280374
require.Equal(t, testData.expectedBootOrder, bootOrder, "BootOrder does not match expected value")
281375

282376
bootEntries, err := efivarfs.ListBootEntries(testData.efivarfsMock)

0 commit comments

Comments
 (0)