efivar bug

pkg/bootloader/grub.go

@@ -339,7 +339,7 @@ func (g *Grub) clearEntry() error {
 // createBootEntry will create an entry in the efi vars for our shim and set it to boot first in the bootorder
 func (g *Grub) CreateEntry(shimName string, relativeTo string, efiVariables eleefi.Variables) error {
 	g.logger.Debugf("Creating boot entry for elemental pointing to shim %s/%s", constants.EntryEFIPath, shimName)
-	bm, err := eleefi.NewBootManagerForVariables(efiVariables)
+	bm, err := eleefi.NewBootManagerForVariables(g.logger, efiVariables)
 	if err != nil {
 		return err
 	}

pkg/bootloader/grub_test.go

@@ -25,6 +25,9 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 
+	"github.com/twpayne/go-vfs/v4"
+	"github.com/twpayne/go-vfs/v4/vfst"
+
 	"github.com/rancher/elemental-toolkit/v2/cmd"
 	"github.com/rancher/elemental-toolkit/v2/pkg/bootloader"
 	"github.com/rancher/elemental-toolkit/v2/pkg/config"
@@ -33,8 +36,6 @@ import (
 	"github.com/rancher/elemental-toolkit/v2/pkg/mocks"
 	"github.com/rancher/elemental-toolkit/v2/pkg/types"
 	"github.com/rancher/elemental-toolkit/v2/pkg/utils"
-	"github.com/twpayne/go-vfs/v4"
-	"github.com/twpayne/go-vfs/v4/vfst"
 )
 
 var _ = Describe("Booloader", Label("bootloader", "grub"), func() {
@@ -90,7 +91,7 @@ var _ = Describe("Booloader", Label("bootloader", "grub"), func() {
 		Expect(fs.WriteFile(filepath.Join(rootDir, constants.GrubCfgPath, constants.GrubCfg), grubCfg, constants.FilePerm)).To(Succeed())
 
 		// EFI vars to test bootmanager
-		efivars = &eleefi.MockEFIVariables{}
+		efivars = mocks.NewMockEFIVariables()
 		err := fs.Mkdir("/EFI", constants.DirPerm)
 		Expect(err).ToNot(HaveOccurred())
 		err = fs.WriteFile("/EFI/test.efi", []byte(""), constants.FilePerm)

pkg/efi/efi_suite_test.go

@@ -0,0 +1,29 @@
+/*
+Copyright © 2021 SUSE LLC
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package efi_test
+
+import (
+	"testing"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+func TestTypes(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "efi test suite")
+}

pkg/efi/manager.go

@@ -15,14 +15,16 @@ import (
 
 	efi "github.com/canonical/go-efilib"
 	efilinux "github.com/canonical/go-efilib/linux"
+
+	"github.com/rancher/elemental-toolkit/v2/pkg/types"
 )
 
 const (
 	maxBootEntries = 65535 // Maximum number of boot entries we can hold
 )
 
 // NewBootManagerForVariables returns a boot manager for the given EFIVariables manager
-func NewBootManagerForVariables(efivars Variables) (BootManager, error) {
+func NewBootManagerForVariables(logger types.Logger, efivars Variables) (BootManager, error) {
 	var err error
 	bm := BootManager{}
 	bm.efivars = efivars
@@ -57,9 +59,10 @@ func NewBootManagerForVariables(efivars Variables) (BootManager, error) {
 		if err != nil {
 			return BootManager{}, fmt.Errorf("cannot read %s: %v", name, err)
 		}
-		entry.LoadOption, err = efi.ReadLoadOption(bytes.NewReader(entry.Data))
+		entry.LoadOption, err = bm.efivars.ReadLoadOption(bytes.NewReader(entry.Data))
 		if err != nil {
-			return bm, err
+			logger.Warnf("Error reading efi load option for '%s': %s", name, err.Error())
+			continue
 		}
 
 		bm.entries[entry.BootNumber] = entry

pkg/efi/manager_test.go

@@ -0,0 +1,67 @@
+/*
+Copyright © 2021 - 2024 SUSE LLC
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package efi_test
+
+import (
+	"bytes"
+	"fmt"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	"github.com/sirupsen/logrus"
+
+	efilib "github.com/canonical/go-efilib"
+
+	"github.com/rancher/elemental-toolkit/v2/pkg/efi"
+	"github.com/rancher/elemental-toolkit/v2/pkg/mocks"
+	"github.com/rancher/elemental-toolkit/v2/pkg/types"
+)
+
+var _ = Describe("EFI Manager", Label("efi", "manager"), func() {
+	var memLog *bytes.Buffer
+	var logger types.Logger
+
+	BeforeEach(func() {
+		memLog = &bytes.Buffer{}
+		logger = types.NewBufferLogger(memLog)
+		logger.SetLevel(logrus.DebugLevel)
+	})
+
+	It("creates a BootManager without error", func() {
+		var vars efi.Variables = mocks.NewMockEFIVariables()
+
+		manager, err := efi.NewBootManagerForVariables(logger, vars)
+		Expect(err).To(BeNil())
+
+		Expect(manager).ToNot(BeNil())
+	})
+
+	It("creates a BootManager with ReadLoadOptions error", func() {
+		var vars efi.Variables = mocks.NewMockEFIVariables().WithLoadOptionError(fmt.Errorf("cannot read device path: cannot decode node: 1: invalid length 14 bytes (too large)"))
+
+		err := vars.SetVariable(efilib.GlobalVariable, "BootOrder", []byte("0001"), efilib.AttributeNonVolatile)
+		Expect(err).To(BeNil())
+
+		err = vars.SetVariable(efilib.GlobalVariable, "Boot0001", []byte("test.efi"), efilib.AttributeNonVolatile)
+		Expect(err).To(BeNil())
+
+		manager, err := efi.NewBootManagerForVariables(logger, vars)
+		Expect(err).To(BeNil())
+
+		Expect(manager).ToNot(BeNil())
+	})
+})

pkg/efi/types.go

@@ -7,14 +7,10 @@
 package efi
 
 import (
-	"encoding/base64"
-	"encoding/binary"
-	"encoding/json"
-	"strings"
+	"io"
 
 	efi "github.com/canonical/go-efilib"
 	efi_linux "github.com/canonical/go-efilib/linux"
-	"github.com/twpayne/go-vfs/v4"
 )
 
 // Variables abstracts away the host-specific bits of the efivars module
@@ -24,6 +20,7 @@ type Variables interface {
 	SetVariable(guid efi.GUID, name string, data []byte, attrs efi.VariableAttributes) error
 	NewFileDevicePath(filepath string, mode efi_linux.FilePathToDevicePathMode) (efi.DevicePath, error)
 	DelVariable(guid efi.GUID, name string) error
+	ReadLoadOption(r io.Reader) (out *efi.LoadOption, err error)
 }
 
 // RealEFIVariables provides the real implementation of efivars
@@ -56,85 +53,9 @@ func (RealEFIVariables) SetVariable(guid efi.GUID, name string, data []byte, att
 	return efi.WriteVariable(name, guid, attrs, data)
 }
 
-type mockEFIVariable struct {
-	data  []byte
-	attrs efi.VariableAttributes
-}
-
-// MockEFIVariables implements an in-memory variable store.
-type MockEFIVariables struct {
-	store map[efi.VariableDescriptor]mockEFIVariable
-}
-
-func (m MockEFIVariables) DelVariable(_ efi.GUID, _ string) error {
-	return nil
-}
-
-// ListVariables implements EFIVariables
-func (m MockEFIVariables) ListVariables() (out []efi.VariableDescriptor, err error) {
-	for k := range m.store {
-		out = append(out, k)
-	}
-	return out, nil
-}
-
-// GetVariable implements EFIVariables
-func (m MockEFIVariables) GetVariable(guid efi.GUID, name string) (data []byte, attrs efi.VariableAttributes, err error) {
-	out, ok := m.store[efi.VariableDescriptor{Name: name, GUID: guid}]
-	if !ok {
-		return nil, 0, efi.ErrVarNotExist
-	}
-	return out.data, out.attrs, nil
-}
-
-// SetVariable implements EFIVariables
-func (m *MockEFIVariables) SetVariable(guid efi.GUID, name string, data []byte, attrs efi.VariableAttributes) error {
-	if m.store == nil {
-		m.store = make(map[efi.VariableDescriptor]mockEFIVariable)
-	}
-	if len(data) == 0 {
-		delete(m.store, efi.VariableDescriptor{Name: name, GUID: guid})
-	} else {
-		m.store[efi.VariableDescriptor{Name: name, GUID: guid}] = mockEFIVariable{data, attrs}
-	}
-	return nil
-}
-
-// JSON renders the MockEFIVariables as an Azure JSON config
-func (m MockEFIVariables) JSON() ([]byte, error) {
-	payload := make(map[string]map[string]string)
-
-	var numBytes [2]byte
-	for key, entry := range m.store {
-		entryID := key.Name
-		entryBase64 := base64.StdEncoding.EncodeToString(entry.data)
-		guidBase64 := base64.StdEncoding.EncodeToString(key.GUID[0:])
-		binary.LittleEndian.PutUint16(numBytes[0:], uint16(entry.attrs))
-		entryAttrBase64 := base64.StdEncoding.EncodeToString(numBytes[0:])
-
-		payload[entryID] = map[string]string{
-			"guid":       guidBase64,
-			"attributes": entryAttrBase64,
-			"value":      entryBase64,
-		}
-	}
-
-	return json.MarshalIndent(payload, "", "  ")
-}
-
-func (m MockEFIVariables) NewFileDevicePath(fpath string, _ efi_linux.FilePathToDevicePathMode) (efi.DevicePath, error) {
-	file, err := vfs.OSFS.Open(fpath)
-	if err != nil {
-		return nil, err
-	}
-	file.Close()
-
-	const espLocation = "/boot/efi/"
-	fpath = strings.TrimPrefix(fpath, espLocation)
-
-	return efi.DevicePath{
-		efi.NewFilePathDevicePathNode(fpath),
-	}, nil
+// ReadLoadOptions proxy
+func (RealEFIVariables) ReadLoadOption(r io.Reader) (out *efi.LoadOption, err error) {
+	return efi.ReadLoadOption(r)
 }
 
 // BootManager manages the boot device selection menu entries (Boot0000...BootFFFF).