Image Customizer: Fix modules API. (#32)

PR #27 accidentally added a duplicate `modules` field under `modules`. This change fixes this.
microsoft · Dec 13, 2024 · 74cde3f · 74cde3f
1 parent 652697f
commit 74cde3f
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 59 deletions.
diff --git a/toolkit/tools/imagecustomizerapi/module.go b/toolkit/tools/imagecustomizerapi/module.go
@@ -14,13 +14,11 @@ type Module struct {
 	Options  map[string]string `yaml:"options"`
 }
 
-type ModuleList struct {
-	Modules []Module `yaml:"modules"`
-}
+type ModuleList []Module
 
-func (m *ModuleList) IsValid() error {
+func (m ModuleList) IsValid() error {
 	moduleMap := make(map[string]int)
-	for i, module := range m.Modules {
+	for i, module := range m {
 		// 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)

diff --git a/toolkit/tools/imagecustomizerapi/os_test.go b/toolkit/tools/imagecustomizerapi/os_test.go
@@ -133,11 +133,9 @@ func TestOSIsValidInvalidServices(t *testing.T) {
 
 func TestOSIsValidInvalidModule(t *testing.T) {
 	os := OS{
-		Modules: ModuleList{
-			Modules: []Module{
-				{
-					Name: "",
-				},
+		Modules: []Module{
+			{
+				Name: "",
 			},
 		},
 	}
@@ -149,14 +147,12 @@ func TestOSIsValidInvalidModule(t *testing.T) {
 
 func TestOSIsValidModuleDuplicateName(t *testing.T) {
 	os := OS{
-		Modules: ModuleList{
-			Modules: []Module{
-				{
-					Name: "nbd",
-				},
-				{
-					Name: "nbd",
-				},
+		Modules: []Module{
+			{
+				Name: "nbd",
+			},
+			{
+				Name: "nbd",
 			},
 		},
 	}

diff --git a/toolkit/tools/pkg/imagecustomizerlib/kernelmoduleutils.go b/toolkit/tools/pkg/imagecustomizerlib/kernelmoduleutils.go
@@ -36,7 +36,7 @@ func LoadOrDisableModules(modules imagecustomizerapi.ModuleList, rootDir string)
 	moduleLoadFilePath := filepath.Join(rootDir, moduleLoadPath)
 	moduleOptionsFilePath := filepath.Join(rootDir, moduleOptionsPath)
 
-	for i, module := range modules.Modules {
+	for i, module := range modules {
 		switch module.LoadMode {
 		case imagecustomizerapi.ModuleLoadModeAlways:
 			// If a module is disabled, remove it. Add the module to modules-load.d/. Write options if provided.

diff --git a/toolkit/tools/pkg/imagecustomizerlib/kernelmoduleutils_test.go b/toolkit/tools/pkg/imagecustomizerlib/kernelmoduleutils_test.go
@@ -16,22 +16,20 @@ import (
 
 func TestLoadOrDisableModules(t *testing.T) {
 	rootDir := filepath.Join(tmpDir, "TestLoadOrDisableModules")
-	modules := imagecustomizerapi.ModuleList{
-		Modules: []imagecustomizerapi.Module{
-			{
-				Name:     "module1",
-				LoadMode: imagecustomizerapi.ModuleLoadModeAlways,
-				Options:  map[string]string{"option1": "value1"},
-			},
-			{
-				Name:     "module2",
-				LoadMode: imagecustomizerapi.ModuleLoadModeDisable,
-			},
-			{
-				Name:     "module3",
-				LoadMode: imagecustomizerapi.ModuleLoadModeAuto,
-				Options:  map[string]string{"option3_1": "value3_1", "option3_2": "value3_2"},
-			},
+	modules := []imagecustomizerapi.Module{
+		{
+			Name:     "module1",
+			LoadMode: imagecustomizerapi.ModuleLoadModeAlways,
+			Options:  map[string]string{"option1": "value1"},
+		},
+		{
+			Name:     "module2",
+			LoadMode: imagecustomizerapi.ModuleLoadModeDisable,
+		},
+		{
+			Name:     "module3",
+			LoadMode: imagecustomizerapi.ModuleLoadModeAuto,
+			Options:  map[string]string{"option3_1": "value3_1", "option3_2": "value3_2"},
 		},
 	}
 
@@ -63,30 +61,26 @@ func TestLoadOrDisableModules(t *testing.T) {
 	assert.Contains(t, string(moduleOptionsContent), "option3_2=value3_2")
 
 	// Test add options for module2 which was disabled
-	modules = imagecustomizerapi.ModuleList{
-		Modules: []imagecustomizerapi.Module{
-			{
-				Name:    "module2",
-				Options: map[string]string{"option2": "value2"},
-			},
+	modules = []imagecustomizerapi.Module{
+		{
+			Name:    "module2",
+			Options: map[string]string{"option2": "value2"},
 		},
 	}
 
 	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
-	modules = imagecustomizerapi.ModuleList{
-		Modules: []imagecustomizerapi.Module{
-			{
-				Name:     "module2",
-				LoadMode: imagecustomizerapi.ModuleLoadModeAuto,
-				Options:  map[string]string{"option2": "value2"},
-			},
-			{
-				Name:    "module3",
-				Options: map[string]string{"option3_1": "new_value3_1", "option3_3": "new_value3_3"},
-			},
+	modules = []imagecustomizerapi.Module{
+		{
+			Name:     "module2",
+			LoadMode: imagecustomizerapi.ModuleLoadModeAuto,
+			Options:  map[string]string{"option2": "value2"},
+		},
+		{
+			Name:    "module3",
+			Options: map[string]string{"option3_1": "new_value3_1", "option3_3": "new_value3_3"},
 		},
 	}
 	err = LoadOrDisableModules(modules, rootDir)
@@ -102,13 +96,11 @@ func TestLoadOrDisableModules(t *testing.T) {
 	assert.Contains(t, string(moduleOptionsContent), "option3_3=new_value3_3")
 
 	// Test case where a module was already set to load at boot
-	modules = imagecustomizerapi.ModuleList{
-		Modules: []imagecustomizerapi.Module{
-			{
-				Name:     "module1",
-				LoadMode: imagecustomizerapi.ModuleLoadModeAlways,
-				Options:  map[string]string{"option1": "value1"},
-			},
+	modules = []imagecustomizerapi.Module{
+		{
+			Name:     "module1",
+			LoadMode: imagecustomizerapi.ModuleLoadModeAlways,
+			Options:  map[string]string{"option1": "value1"},
 		},
 	}