OSModifier: Add services, modules, and kernel cmdline in EMU API

toolkit/tools/imagecustomizerapi/config_test.go

@@ -336,8 +336,11 @@ func TestConfigIsValidKernelCLI(t *testing.T) {
 		OS: &OS{
 			ResetBootLoaderType: "hard-reset",
 			Hostname:            "test",
+
 			KernelCommandLine: KernelCommandLine{
-				ExtraCommandLine: "console=ttyS0",
+				ExtraCommandLine: KernelExtraArguments{
+					"console=ttyS0",
+				},
 			},
 		},
 	}

toolkit/tools/imagecustomizerapi/iso_test.go

@@ -12,7 +12,7 @@ import (
 func TestIsoIsValid(t *testing.T) {
 	iso := Iso{
 		KernelCommandLine: KernelCommandLine{
-			ExtraCommandLine: "'",
+			ExtraCommandLine: KernelExtraArguments{"'"},
 		},
 	}
 

toolkit/tools/imagecustomizerapi/kernelextraarguments.go

@@ -7,14 +7,15 @@ import (
 )
 
 // KernelExtraArguments defines one or more extra kernel arguments.
-type KernelExtraArguments string
+type KernelExtraArguments []string
 
 func (e KernelExtraArguments) IsValid() error {
-	err := validateKernelArgumentsFormat(string(e))
-	if err != nil {
-		return err
+	for _, arg := range e {
+		err := validateKernelArgumentsFormat(arg)
+		if err != nil {
+			return err
+		}
 	}
-
 	return nil
 }
 

toolkit/tools/imagecustomizerapi/kernelextraarguments_test.go

@@ -53,50 +53,66 @@ func TestKernelExtraArgumentsIsValid(t *testing.T) {
 	missingClosingDoubleQuotes := "invalid double-quoted string: missing closing double-quotes"
 	missingClosingSingleQuote := "invalid single-quoted string: missing closing single-quote"
 
-	configsToTest := map[KernelExtraArguments]*string{
-		// very simple cases (no quoting)
-		"":        nil,
-		"a":       nil,
-		"a=b":     nil,
-		"a=b x=y": nil,
-		// enlosed in double quotes (4)
-		"\"a=b\"": nil,
-		// enclosed in single quotes (2)
-		"'a=b'": nil,
-		// single quote embedded within double quotes and vice versa (2)
-		"\"a='b\" 'x=\"y'": nil,
-		// single-quoted string embedded within a double quoted value (4)
-		"\"'a=b' x=y\"": nil,
-		// double-quoted string embedded within a double quoted value (4)(6)(7)
-		"\"a=b \\\"x=y\\\"\"": nil,
-		// \n embedded within a double quoted value (4)(6)
-		"\"a=b x=y\\n\"": nil,
-		// \ embedded within a double quoted value (4)(6)
-		"\"a=b x=y\\\\\"": nil,
-		// unmatched open double-quote - beginning of string (4)
-		"\"a=b x=y": &missingClosingDoubleQuotes,
-		// unmatched open double-quote - middle of string (4)
-		"a=b \"x=y": &missingClosingDoubleQuotes,
-		// unmatched open double-quote - end of string (4)
-		"a=b x=y\"": &missingClosingDoubleQuotes,
-		// unmatched open single-quote - beginning of string (2)
-		"'a=b x=y": &missingClosingSingleQuote,
-		// unmatched open single-quote - middle of string (2)
-		"a=b 'x=y": &missingClosingSingleQuote,
-		// unmatched open single-quote - end of string (2)
-		"a=b x='y": &missingClosingSingleQuote,
-		// attempt to use \ to escape single quotes (3)
-		"'a=\\'b'": &missingClosingSingleQuote,
+	configsToTest := []struct {
+		config      KernelExtraArguments
+		expectedErr *string
+	}{
+		// Simple cases (no quoting)
+		{KernelExtraArguments{""}, nil},
+		{KernelExtraArguments{"a"}, nil},
+		{KernelExtraArguments{"a=b"}, nil},
+		{KernelExtraArguments{"a=b", "x=y"}, nil},
+
+		// Enclosed in double quotes
+		{KernelExtraArguments{"\"a=b\""}, nil},
+
+		// Enclosed in single quotes
+		{KernelExtraArguments{"'a=b'"}, nil},
+
+		// Single quote embedded within double quotes and vice versa
+		{KernelExtraArguments{"\"a='b\" 'x=\"y'"}, nil},
+
+		// Single-quoted string embedded within a double-quoted value
+		{KernelExtraArguments{"\"'a=b'", "x=y\""}, nil},
+
+		// Double-quoted string embedded within a double-quoted value
+		{KernelExtraArguments{"\"a=b", "\\\"x=y\\\"\""}, nil},
+
+		// \n embedded within a double-quoted value
+		{KernelExtraArguments{"\"a=b", "x=y\\n\""}, nil},
+
+		// \ embedded within a double-quoted value
+		{KernelExtraArguments{"\"a=b", "x=y\\\\\""}, nil},
+
+		// Unmatched open double-quote - beginning of string
+		{KernelExtraArguments{"\"a=b", "x=y"}, &missingClosingDoubleQuotes},
+
+		// Unmatched open double-quote - middle of string
+		{KernelExtraArguments{"a=b", "\"x=y"}, &missingClosingDoubleQuotes},
+
+		// Unmatched open double-quote - end of string
+		{KernelExtraArguments{"a=b", "x=y\""}, &missingClosingDoubleQuotes},
+
+		// Unmatched open single-quote - beginning of string
+		{KernelExtraArguments{"'a=b", "x=y"}, &missingClosingSingleQuote},
+
+		// Unmatched open single-quote - middle of string
+		{KernelExtraArguments{"a=b", "'x=y"}, &missingClosingSingleQuote},
+
+		// Unmatched open single-quote - end of string
+		{KernelExtraArguments{"a=b", "x='y"}, &missingClosingSingleQuote},
+
+		// Attempt to use \ to escape single quotes
+		{KernelExtraArguments{"'a=\\'b'"}, &missingClosingSingleQuote},
 	}
 
-	// testsOk := true
-	for config, expectedErr := range configsToTest {
-		err := config.IsValid()
-		if expectedErr == nil {
+	for _, test := range configsToTest {
+		err := test.config.IsValid()
+		if test.expectedErr == nil {
 			assert.NoError(t, err)
 		} else {
 			assert.Error(t, err)
-			assert.ErrorContains(t, err, *expectedErr)
+			assert.ErrorContains(t, err, *test.expectedErr)
 		}
 	}
 }

toolkit/tools/imagecustomizerapi/os_test.go

@@ -219,7 +219,7 @@ func TestOSIsValidOverlayDuplicateWorkDir(t *testing.T) {
 func TestOSIsValidInvalidKernelCommandLine(t *testing.T) {
 	os := OS{
 		KernelCommandLine: KernelCommandLine{
-			ExtraCommandLine: "\"",
+			ExtraCommandLine: KernelExtraArguments{"\""},
 		},
 	}
 

toolkit/tools/osmodifierapi/os.go

@@ -13,10 +13,13 @@ import (
 
 // OS defines how each system present on the image is supposed to be configured.
 type OS struct {
-	Hostname string                     `yaml:"hostname"`
-	SELinux  imagecustomizerapi.SELinux `yaml:"selinux"`
-	Users    []imagecustomizerapi.User  `yaml:"users"`
-	Overlays *[]Overlay                 `yaml:"overlays"`
+	Hostname          string                               `yaml:"hostname"`
+	SELinux           imagecustomizerapi.SELinux           `yaml:"selinux"`
+	Users             []imagecustomizerapi.User            `yaml:"users"`
+	Overlays          *[]Overlay                           `yaml:"overlays"`
+	Services          imagecustomizerapi.Services          `yaml:"services"`
+	Modules           []imagecustomizerapi.Module          `yaml:"modules"`
+	KernelCommandLine imagecustomizerapi.KernelCommandLine `yaml:"kernelCommandLine"`
 }
 
 func (s *OS) IsValid() error {
@@ -65,5 +68,22 @@ func (s *OS) IsValid() error {
 		}
 	}
 
+	if err := s.Services.IsValid(); err != nil {
+		return err
+	}
+
+	moduleMap := make(map[string]int)
+	for i, module := range s.Modules {
+		// Check if module is duplicated to avoid conflicts with modules potentially having different LoadMode
+		if _, exists := moduleMap[module.Name]; exists {
+			return fmt.Errorf("duplicate module found: %s at index %d", module.Name, i)
+		}
+		moduleMap[module.Name] = i
+		err = module.IsValid()
+		if err != nil {
+			return fmt.Errorf("invalid modules item at index %d:\n%w", i, err)
+		}
+	}
+
 	return nil
 }

toolkit/tools/pkg/imagecustomizerlib/customizebootloader.go

@@ -24,7 +24,7 @@ func handleBootLoader(baseConfigPath string, config *imagecustomizerapi.Config,
 
 	default:
 		// Append the kernel command-line args to the existing grub config.
-		err := addKernelCommandLine(config.OS.KernelCommandLine.ExtraCommandLine, imageConnection.Chroot())
+		err := AddKernelCommandLine(config.OS.KernelCommandLine.ExtraCommandLine, imageConnection.Chroot())
 		if err != nil {
 			return fmt.Errorf("failed to add extra kernel command line:\n%w", err)
 		}
@@ -86,12 +86,12 @@ func hardResetBootLoader(baseConfigPath string, config *imagecustomizerapi.Confi
 }
 
 // Inserts new kernel command-line args into the grub config file.
-func addKernelCommandLine(kernelExtraArguments imagecustomizerapi.KernelExtraArguments,
-	imageChroot *safechroot.Chroot,
+func AddKernelCommandLine(kernelExtraArguments imagecustomizerapi.KernelExtraArguments,
+	imageChroot safechroot.ChrootInterface,
 ) error {
 	var err error
 
-	if kernelExtraArguments == "" {
+	if len(kernelExtraArguments) == 0 {
 		// Nothing to do.
 		return nil
 	}
@@ -103,9 +103,11 @@ func addKernelCommandLine(kernelExtraArguments imagecustomizerapi.KernelExtraArg
 		return err
 	}
 
-	err = bootCustomizer.AddKernelCommandLine(string(kernelExtraArguments))
-	if err != nil {
-		return err
+	for _, arg := range kernelExtraArguments {
+		err = bootCustomizer.AddKernelCommandLine(arg)
+		if err != nil {
+			return err
+		}
 	}
 
 	err = bootCustomizer.WriteToFile(imageChroot)

toolkit/tools/pkg/imagecustomizerlib/customizeos.go

@@ -49,12 +49,12 @@ func doOsCustomizations(buildDir string, baseConfigPath string, config *imagecus
 		return err
 	}
 
-	err = enableOrDisableServices(config.OS.Services, imageChroot)
+	err = EnableOrDisableServices(config.OS.Services, imageChroot)
 	if err != nil {
 		return err
 	}
 
-	err = loadOrDisableModules(config.OS.Modules, imageChroot.RootDir())
+	err = LoadOrDisableModules(config.OS.Modules, imageChroot.RootDir())
 	if err != nil {
 		return err
 	}

toolkit/tools/pkg/imagecustomizerlib/customizeservices.go

@@ -13,7 +13,7 @@ import (
 	"github.com/microsoft/azurelinux/toolkit/tools/internal/systemd"
 )
 
-func enableOrDisableServices(services imagecustomizerapi.Services, imageChroot *safechroot.Chroot) error {
+func EnableOrDisableServices(services imagecustomizerapi.Services, imageChroot safechroot.ChrootInterface) error {
 	var err error
 
 	// Handle enabling services

toolkit/tools/pkg/imagecustomizerlib/imagecustomizer_test.go

@@ -194,7 +194,7 @@ func TestCustomizeImageKernelCommandLineAdd(t *testing.T) {
 	config := &imagecustomizerapi.Config{
 		OS: &imagecustomizerapi.OS{
 			KernelCommandLine: imagecustomizerapi.KernelCommandLine{
-				ExtraCommandLine: "console=tty0 console=ttyS0",
+				ExtraCommandLine: []string{"console=tty0", "console=ttyS0"},
 			},
 		},
 	}

toolkit/tools/pkg/imagecustomizerlib/kernelmoduleutils.go

@@ -27,7 +27,7 @@ const (
 	moduleOptionsPath  = modprobeConfigDir + "/" + moduleOptionsFileName
 )
 
-func loadOrDisableModules(modules []imagecustomizerapi.Module, rootDir string) error {
+func LoadOrDisableModules(modules []imagecustomizerapi.Module, rootDir string) error {
 	var err error
 	var modulesToLoad []string
 	var modulesToDisable []string

toolkit/tools/pkg/imagecustomizerlib/kernelmoduleutils_test.go

@@ -33,7 +33,7 @@ func TestLoadOrDisableModules(t *testing.T) {
 		},
 	}
 
-	err := loadOrDisableModules(modules, rootDir)
+	err := LoadOrDisableModules(modules, rootDir)
 	assert.NoError(t, err)
 
 	moduleLoadFilePath := filepath.Join(rootDir, moduleLoadPath)
@@ -68,7 +68,7 @@ func TestLoadOrDisableModules(t *testing.T) {
 		},
 	}
 
-	err = loadOrDisableModules(modules, rootDir)
+	err = LoadOrDisableModules(modules, rootDir)
 	assert.Contains(t, err.Error(), "cannot add options for disabled module (module2)")
 
 	// Test updating module2's loadmode and module3's option
@@ -83,7 +83,7 @@ func TestLoadOrDisableModules(t *testing.T) {
 			Options: map[string]string{"option3_1": "new_value3_1", "option3_3": "new_value3_3"},
 		},
 	}
-	err = loadOrDisableModules(modules, rootDir)
+	err = LoadOrDisableModules(modules, rootDir)
 	assert.NoError(t, err)
 
 	moduleDisableContent, _ = os.ReadFile(moduleDisableFilePath)
@@ -104,7 +104,7 @@ func TestLoadOrDisableModules(t *testing.T) {
 		},
 	}
 
-	err = loadOrDisableModules(modules, rootDir)
+	err = LoadOrDisableModules(modules, rootDir)
 	assert.NoError(t, err)
 
 	moduleLoadContent, _ = os.ReadFile(moduleLoadFilePath)

toolkit/tools/pkg/imagecustomizerlib/liveosisobuilder.go

@@ -312,11 +312,14 @@ func updateSavedConfigs(savedConfigsFilePath string, newKernelArgs imagecustomiz
 
 	if savedConfigs != nil {
 		// do we have kernel arguments from a previous run?
-		if savedConfigs.Iso.KernelCommandLine.ExtraCommandLine != "" {
+		if len(savedConfigs.Iso.KernelCommandLine.ExtraCommandLine) > 0 {
 			// If yes, add them before the new kernel arguments.
-			savedArgs := strings.TrimSpace(string(savedConfigs.Iso.KernelCommandLine.ExtraCommandLine))
-			newArgs := strings.TrimSpace(string(newKernelArgs))
-			updatedSavedConfigs.Iso.KernelCommandLine.ExtraCommandLine = imagecustomizerapi.KernelExtraArguments(savedArgs + " " + newArgs)
+			savedArgs := savedConfigs.Iso.KernelCommandLine.ExtraCommandLine
+			newArgs := newKernelArgs
+
+			// Combine saved arguments with new ones
+			combinedArgs := append(savedArgs, newArgs...)
+			updatedSavedConfigs.Iso.KernelCommandLine.ExtraCommandLine = imagecustomizerapi.KernelExtraArguments(combinedArgs)
 		}
 
 		// if the PXE iso image url is not set, set it to the value from the previous run.
@@ -420,7 +423,8 @@ func (b *LiveOSIsoBuilder) updateGrubCfg(isoGrubCfgFileName string, pxeGrubCfgFi
 	}
 
 	liveosKernelArgs := fmt.Sprintf(kernelArgsLiveOSTemplate, liveOSDir, liveOSImage)
-	additionalKernelCommandline := liveosKernelArgs + " " + string(savedConfigs.Iso.KernelCommandLine.ExtraCommandLine)
+	savedArgs := strings.Join(savedConfigs.Iso.KernelCommandLine.ExtraCommandLine, " ")
+	additionalKernelCommandline := liveosKernelArgs + " " + savedArgs
 
 	inputContentString, err = appendKernelCommandLineArgsAll(inputContentString, additionalKernelCommandline,
 		true /*allowMultiple*/, false /*requireKernelOpts*/)

toolkit/tools/pkg/imagecustomizerlib/liveosisobuilder_test.go

@@ -81,7 +81,8 @@ func TestCustomizeImageLiveCd1(t *testing.T) {
 	savedConfigs := &SavedConfigs{}
 	err = imagecustomizerapi.UnmarshalYamlFile(savedConfigsFilePath, savedConfigs)
 	assert.NoErrorf(t, err, "read (%s) file", savedConfigsFilePath)
-	assert.Equal(t, "rd.info", string(savedConfigs.Iso.KernelCommandLine.ExtraCommandLine))
+	expectedKernelArgs := []string{"rd.info"}
+	assert.Equal(t, expectedKernelArgs, savedConfigs.Iso.KernelCommandLine.ExtraCommandLine)
 
 	VerifyPXEArtifacts(t, savedConfigs.OS.DracutPackageInfo, isoMountDir, pxeKernelIpArg, pxeKernelRootArgV1,
 		pxeArtifactsPathVhdxToIso)
@@ -111,7 +112,7 @@ func TestCustomizeImageLiveCd1(t *testing.T) {
 		},
 		Iso: &imagecustomizerapi.Iso{
 			KernelCommandLine: imagecustomizerapi.KernelCommandLine{
-				ExtraCommandLine: "rd.debug",
+				ExtraCommandLine: []string{"rd.debug"},
 			},
 			AdditionalFiles: imagecustomizerapi.AdditionalFileList{
 				{
@@ -165,7 +166,7 @@ func TestCustomizeImageLiveCd1(t *testing.T) {
 	savedConfigs = &SavedConfigs{}
 	err = imagecustomizerapi.UnmarshalYamlFile(savedConfigsFilePath, savedConfigs)
 	assert.NoErrorf(t, err, "read (%s) file", savedConfigsFilePath)
-	assert.Equal(t, "rd.info rd.debug", string(savedConfigs.Iso.KernelCommandLine.ExtraCommandLine))
+	assert.Equal(t, "rd.info rd.debug", strings.Join(savedConfigs.Iso.KernelCommandLine.ExtraCommandLine, " "))
 
 	VerifyPXEArtifacts(t, savedConfigs.OS.DracutPackageInfo, isoMountDir, pxeKernelIpArg, pxeKernelRootArgV2,
 		pxeArtifactsPathIsoToIso)

toolkit/tools/pkg/imagecustomizerlib/typeConversion.go

@@ -5,6 +5,7 @@ package imagecustomizerlib
 
 import (
 	"fmt"
+	"strings"
 
 	"github.com/microsoft/azurelinux/toolkit/tools/imagecustomizerapi"
 	"github.com/microsoft/azurelinux/toolkit/tools/imagegen/configuration"
@@ -192,7 +193,7 @@ func kernelCommandLineToImager(kernelCommandLine imagecustomizerapi.KernelComman
 	}
 
 	imagerKernelCommandLine := configuration.KernelCommandLine{
-		ExtraCommandLine: string(kernelCommandLine.ExtraCommandLine),
+		ExtraCommandLine: strings.Join(kernelCommandLine.ExtraCommandLine, " "),
 		SELinux:          imagerSELinuxMode,
 		SELinuxPolicy:    "",
 	}