Refactored ValidateDefinition to return a map

cmd/eib/main.go

@@ -75,19 +75,9 @@ func processArgs() (*image.Context, error) {
 	if len(failedValidations) > 0 {
 		audit.Audit("Image definition validation found the following errors:")
 
-		failuresByComponent := map[string][]validation.FailedValidation{}
 		logMessageBuilder := strings.Builder{}
 
-		for _, fv := range failedValidations {
-			failuresByComponent[fv.Component] = append(failuresByComponent[fv.Component], fv)
-
-			logMessageBuilder.WriteString(fv.UserMessage)
-			if fv.Error != nil {
-				logMessageBuilder.WriteString(fv.Error.Error())
-			}
-		}
-
-		for componentName, failures := range failuresByComponent {
+		for componentName, failures := range failedValidations {
 			audit.Audit(fmt.Sprintf("  %s", componentName))
 			for _, cf := range failures {
 				audit.Audit(fmt.Sprintf("    %s", cf.UserMessage))

pkg/image/validation/os_test.go

@@ -94,6 +94,16 @@ func TestValidateOperatingSystem(t *testing.T) {
 			}
 			failures := validateOperatingSystem(&ctx)
 			assert.Len(t, failures, len(test.ExpectedFailedMessages))
+
+			var foundMessages []string
+			for _, foundValidation := range failures {
+				foundMessages = append(foundMessages, foundValidation.UserMessage)
+				assert.Equal(t, osComponent, foundValidation.Component)
+			}
+
+			for _, expectedMessage := range test.ExpectedFailedMessages {
+				assert.Contains(t, foundMessages, expectedMessage)
+			}
 		})
 	}
 }

pkg/image/validation/validation.go

@@ -12,8 +12,8 @@ type FailedValidation struct {
 
 type validateComponent func(ctx *image.Context) []FailedValidation
 
-func ValidateDefinition(ctx *image.Context) []FailedValidation {
-	var failures []FailedValidation
+func ValidateDefinition(ctx *image.Context) map[string][]FailedValidation {
+	failures := map[string][]FailedValidation{}
 
 	validations := []validateComponent{
 		validateImage,
@@ -23,7 +23,10 @@ func ValidateDefinition(ctx *image.Context) []FailedValidation {
 	}
 	for _, v := range validations {
 		componentFailures := v(ctx)
-		failures = append(failures, componentFailures...)
+
+		if len(componentFailures) > 0 {
+			failures[componentFailures[0].Component] = componentFailures
+		}
 	}
 
 	return failures

pkg/image/validation/validation_test.go

@@ -0,0 +1,117 @@
+package validation
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"github.com/suse-edge/edge-image-builder/pkg/image"
+)
+
+func TestValidateDefinition(t *testing.T) {
+	configDir, err := os.MkdirTemp("", "eib-config-")
+	require.NoError(t, err)
+	defer func() {
+		_ = os.RemoveAll(configDir)
+	}()
+
+	testImagesDir := filepath.Join(configDir, "images")
+	err = os.MkdirAll(testImagesDir, os.ModePerm)
+	require.NoError(t, err)
+
+	fakeBaseImageName := "fake-base.iso"
+	_, err = os.Create(filepath.Join(testImagesDir, fakeBaseImageName))
+	require.NoError(t, err)
+
+	tests := map[string]struct {
+		Definition image.Definition
+		Expected   map[string][]string
+	}{
+		`minimal valid`: {
+			Definition: image.Definition{
+				APIVersion: "1.0",
+				Image: image.Image{
+					ImageType:       "iso",
+					Arch:            image.ArchTypeX86,
+					BaseImage:       fakeBaseImageName,
+					OutputImageName: "output.iso",
+				},
+			},
+		},
+		`one error from each`: {
+			Definition: image.Definition{
+				APIVersion: "1.0",
+				Image: image.Image{
+					Arch:            image.ArchTypeX86,
+					BaseImage:       fakeBaseImageName,
+					OutputImageName: "output.iso",
+				},
+				OperatingSystem: image.OperatingSystem{
+					KernelArgs: []string{"foo="},
+				},
+				EmbeddedArtifactRegistry: image.EmbeddedArtifactRegistry{
+					ContainerImages: []image.ContainerImage{
+						{
+							Name: "", // trips the missing name validation
+						},
+					},
+				},
+				Kubernetes: image.Kubernetes{
+					Network: image.Network{},
+					Nodes: []image.Node{
+						{
+							Hostname: "host1",
+							Type:     image.KubernetesNodeTypeServer,
+						},
+						{
+							Hostname: "host2",
+							Type:     image.KubernetesNodeTypeAgent,
+						},
+					},
+				},
+			},
+			Expected: map[string][]string{
+				imageComponent: {
+					"The 'imageType' field is required in the 'image' section.",
+				},
+				osComponent: {
+					"Kernel arguments must be specified as 'key=value'.",
+				},
+				registryComponent: {
+					"The 'name' field is required for each entry in 'images'.",
+				},
+				k8sComponent: {
+					"The 'apiVIP' field is required in the 'network' section when defining entries under 'nodes'.",
+					"The 'apiHost' field is required in the 'network' section when defining entries under 'nodes'.",
+				},
+			},
+		},
+	}
+
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
+			def := test.Definition
+			ctx := image.Context{
+				ImageDefinition: &def,
+				ImageConfigDir:  configDir,
+			}
+			failures := ValidateDefinition(&ctx)
+
+			for foundComponent, foundComponentFailures := range failures {
+				assert.Contains(t, test.Expected, foundComponent)
+				assert.Len(t, foundComponentFailures, len(test.Expected[foundComponent]))
+
+				var foundMessages []string
+				for _, foundValidation := range foundComponentFailures {
+					foundMessages = append(foundMessages, foundValidation.UserMessage)
+				}
+
+				for _, expectedMessage := range test.Expected[foundComponent] {
+					assert.Contains(t, foundMessages, expectedMessage)
+				}
+			}
+		})
+	}
+}