Make Persistatant grub vars respect existing ones

We were basically overwriting the file if it existed which is bad.
Now it will read the file if it exists and ingest the existing ones,
then in will override witht he given vars if they match, warn the user
and then store the full processed vars.

This means it will never overwrite any vars again

Signed-off-by: Itxaka <itxaka@kairos.io>

internal/agent/hooks/gruboptions.go

@@ -56,7 +56,7 @@ func grubOptions(c config.Config, opts map[string]string) error {
 			return err
 		}
 	}
-	err = utils.SetPersistentVariables(filepath.Join(runtime.OEM.MountPoint, "grubenv"), opts, c.Fs)
+	err = utils.SetPersistentVariables(filepath.Join(runtime.OEM.MountPoint, "grubenv"), opts, &c)
 	if err != nil {
 		c.Logger.Logger.Error().Err(err).Msg("Failed to set grub options")
 	}

pkg/action/bootentries.go

@@ -60,7 +60,7 @@ func selectBootEntryGrub(cfg *config.Config, entry string) error {
 	vars := map[string]string{
 		"next_entry": entry,
 	}
-	err = utils.SetPersistentVariables("/oem/grubenv", vars, cfg.Fs)
+	err = utils.SetPersistentVariables("/oem/grubenv", vars, cfg)
 	if err != nil {
 		cfg.Logger.Errorf("could not set default boot entry: %s\n", err)
 		return err

pkg/action/bootentries_test.go

@@ -694,7 +694,7 @@ var _ = Describe("Bootentries tests", Label("bootentry"), func() {
 				err = SelectBootEntry(config, "kairos")
 				Expect(err).ToNot(HaveOccurred())
 				Expect(memLog.String()).To(ContainSubstring("Default boot entry set to kairos"))
-				variables, err := utils.ReadPersistentVariables("/oem/grubenv", fs)
+				variables, err := utils.ReadPersistentVariables("/oem/grubenv", config)
 				Expect(err).ToNot(HaveOccurred())
 				Expect(variables["next_entry"]).To(Equal("kairos"))
 			})

pkg/elemental/elemental.go

@@ -584,7 +584,7 @@ func (e Elemental) SetDefaultGrubEntry(partMountPoint string, imgMountPoint stri
 	return utils.SetPersistentVariables(
 		filepath.Join(partMountPoint, cnst.GrubOEMEnv),
 		map[string]string{"default_menu_entry": defaultEntry},
-		e.config.Fs,
+		e.config,
 	)
 }
 

pkg/elemental/elemental_test.go

@@ -848,7 +848,7 @@ var _ = Describe("Elemental", Label("elemental"), func() {
 			el := elemental.NewElemental(config)
 			Expect(config.Fs.Mkdir("/tmp", cnst.DirPerm)).To(BeNil())
 			Expect(el.SetDefaultGrubEntry("/tmp", "/imgMountpoint", "dio")).To(BeNil())
-			varsParsed, err := utils.ReadPersistentVariables(filepath.Join("/tmp", cnst.GrubOEMEnv), config.Fs)
+			varsParsed, err := utils.ReadPersistentVariables(filepath.Join("/tmp", cnst.GrubOEMEnv), config)
 			Expect(err).To(BeNil())
 			Expect(varsParsed["default_menu_entry"]).To(Equal("dio"))
 		})
@@ -856,7 +856,7 @@ var _ = Describe("Elemental", Label("elemental"), func() {
 			el := elemental.NewElemental(config)
 			Expect(config.Fs.Mkdir("/mountpoint", cnst.DirPerm)).To(BeNil())
 			Expect(el.SetDefaultGrubEntry("/mountpoint", "/imgMountPoint", "")).To(BeNil())
-			_, err := utils.ReadPersistentVariables(filepath.Join("/tmp", cnst.GrubOEMEnv), config.Fs)
+			_, err := utils.ReadPersistentVariables(filepath.Join("/tmp", cnst.GrubOEMEnv), config)
 			// Because it didnt do anything due to the entry being empty, the file should not be there
 			Expect(err).ToNot(BeNil())
 			_, err = config.Fs.Stat(filepath.Join("/tmp", cnst.GrubOEMEnv))
@@ -871,7 +871,7 @@ var _ = Describe("Elemental", Label("elemental"), func() {
 
 			el := elemental.NewElemental(config)
 			Expect(el.SetDefaultGrubEntry("/mountpoint", "/imgMountPoint", "")).To(BeNil())
-			varsParsed, err := utils.ReadPersistentVariables(filepath.Join("/mountpoint", cnst.GrubOEMEnv), config.Fs)
+			varsParsed, err := utils.ReadPersistentVariables(filepath.Join("/mountpoint", cnst.GrubOEMEnv), config)
 			Expect(err).To(BeNil())
 			Expect(varsParsed["default_menu_entry"]).To(Equal("test"))
 

pkg/utils/grub.go

@@ -24,6 +24,7 @@ import (
 	"github.com/kairos-io/kairos-agent/v2/pkg/utils/fs"
 	"github.com/kairos-io/kairos-sdk/utils"
 	"io/fs"
+	"os"
 	"path/filepath"
 	"sort"
 	"strings"
@@ -298,21 +299,38 @@ func findGrubDir(vfs v1.FS, dir string) string {
 
 }
 
-// SetPersistentVariables sets the given vars into the given grubEnvFile for grub to read them
+func SetPersistentVariables(grubEnvFile string, vars map[string]string, c *agentConfig.Config) error {
-// TODO: Ingest the existing values from the grubenv file and set them as well
-func SetPersistentVariables(grubEnvFile string, vars map[string]string, fs v1.FS) error {
 	var b bytes.Buffer
+	// Write header
 	b.WriteString("# GRUB Environment Block\n")
 
-	keys := make([]string, 0, len(vars))
+	// First we need to read the existing values from the grubenv file if they exist
-	for k := range vars {
+	finalVars, err := ReadPersistentVariables(grubEnvFile, c)
+	if err != nil {
+		if !os.IsNotExist(err) {
+			return fmt.Errorf("error reading existing grubenv file %s: %s", grubEnvFile, err)
+		}
+	}
+
+	// Merge the existing vars with the new ones
+	// existing vars will be overridden by the new ones from vars if they match
+	for key, newValue := range vars {
+		if oldValue, exists := finalVars[key]; exists {
+			c.Logger.Logger.Warn().Str("key", key).Str("oldValue", oldValue).Str("newValue", newValue).Msg("Overriding existing grubenv variable")
+		}
+		finalVars[key] = newValue
+	}
+
+	keys := make([]string, 0, len(finalVars))
+
+	for k := range finalVars {
 		keys = append(keys, k)
 	}
 	sort.Strings(keys)
 
 	for _, k := range keys {
-		if len(vars[k]) > 0 {
+		if len(finalVars[k]) > 0 {
-			b.WriteString(fmt.Sprintf("%s=%s\n", k, vars[k]))
+			b.WriteString(fmt.Sprintf("%s=%s\n", k, finalVars[k]))
 		}
 	}
 
@@ -322,7 +340,7 @@ func SetPersistentVariables(grubEnvFile string, vars map[string]string, fs v1.FS
 	for i := 0; i < toBeFilled; i++ {
 		b.WriteByte('#')
 	}
-	return fs.WriteFile(grubEnvFile, b.Bytes(), cnst.FilePerm)
+	return c.Fs.WriteFile(grubEnvFile, b.Bytes(), cnst.FilePerm)
 }
 
 // copyGrubFonts will try to finds and copy the needed grub fonts into the system
@@ -439,9 +457,9 @@ func (g Grub) copyGrub() error {
 }
 
 // ReadPersistentVariables will read a grub env file and parse the values
-func ReadPersistentVariables(grubEnvFile string, fs v1.FS) (map[string]string, error) {
+func ReadPersistentVariables(grubEnvFile string, c *agentConfig.Config) (map[string]string, error) {
 	vars := make(map[string]string)
-	f, err := fs.ReadFile(grubEnvFile)
+	f, err := c.Fs.ReadFile(grubEnvFile)
 	if err != nil {
 		return nil, err
 	}

pkg/utils/utils_test.go

@@ -804,9 +804,9 @@ var _ = Describe("Utils", Label("utils"), func() {
 				defer os.Remove(temp.Name())
 				Expect(utils.SetPersistentVariables(
 					temp.Name(), map[string]string{"key1": "value1", "key2": "value2"},
-					config.Fs,
+					config,
 				)).To(BeNil())
-				readVars, err := utils.ReadPersistentVariables(temp.Name(), config.Fs)
+				readVars, err := utils.ReadPersistentVariables(temp.Name(), config)
 				Expect(err).To(BeNil())
 				Expect(readVars["key1"]).To(Equal("value1"))
 				Expect(readVars["key2"]).To(Equal("value2"))
@@ -814,7 +814,7 @@ var _ = Describe("Utils", Label("utils"), func() {
 			It("Fails setting variables", func() {
 				e := utils.SetPersistentVariables(
 					"badfilenopath", map[string]string{"key1": "value1"},
-					config.Fs,
+					config,
 				)
 				Expect(e).NotTo(BeNil())
 			})