Repository: suse-edge/edge-image-builder (go)

Issue:
Add missing Kubernetes version validation
We must ensure Kubernetes version is set when either of the following options is configured:
- manifest URLs
- local manifests
- Helm charts

Pull request:
Kubernetes version validation
Closes #469 

Patch:
diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
index 7a6ffdb..b887575 100644
--- a/RELEASE_NOTES.md
+++ b/RELEASE_NOTES.md
@@ -4,6 +4,7 @@
 
 ## General
 
+* Improved Kubernetes definition validation
 * Allow RKE2 deployments with Calico, Cilium and Multus on aarch64 platforms
 
 ## API
diff --git a/pkg/image/validation/kubernetes.go b/pkg/image/validation/kubernetes.go
index 233427d..b06b09d 100644
--- a/pkg/image/validation/kubernetes.go
+++ b/pkg/image/validation/kubernetes.go
@@ -30,6 +30,7 @@ func validateKubernetes(ctx *image.Context) []FailedValidation {
 	var failures []FailedValidation
 
 	if !isKubernetesDefined(&def.Kubernetes) {
+		failures = append(failures, validateAdditionalArtifacts(ctx)...)
 		return failures
 	}
 
@@ -414,3 +415,34 @@ func validateHelmChartDuplicates(charts []image.HelmChart) string {
 
 	return ""
 }
+
+func validateAdditionalArtifacts(ctx *image.Context) []FailedValidation {
+	var failures []FailedValidation
+
+	dirEntries, err := os.ReadDir(combustion.KubernetesManifestsPath(ctx))
+	if err != nil && !os.IsNotExist(err) {
+		failures = append(failures, FailedValidation{
+			UserMessage: "Kubernetes manifests directory could not be read",
+			Error:       err,
+		})
+	}
+
+	if len(dirEntries) != 0 {
+		failures = append(failures, FailedValidation{
+			UserMessage: "Kubernetes version must be defined when local manifests are configured",
+		})
+	}
+
+	if len(ctx.ImageDefinition.Kubernetes.Helm.Charts) != 0 {
+		failures = append(failures, FailedValidation{
+			UserMessage: "Kubernetes version must be defined when Helm charts are specified",
+		})
+	}
+	if len(ctx.ImageDefinition.Kubernetes.Manifests.URLs) != 0 {
+		failures = append(failures, FailedValidation{
+			UserMessage: "Kubernetes version must be defined when manifest URLs are specified",
+		})
+	}
+
+	return failures
+}


Test patch:
diff --git a/pkg/image/validation/kubernetes_test.go b/pkg/image/validation/kubernetes_test.go
index e8be1d9..55c1620 100644
--- a/pkg/image/validation/kubernetes_test.go
+++ b/pkg/image/validation/kubernetes_test.go
@@ -2,9 +2,13 @@ package validation
 
 import (
 	"fmt"
+	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 
+	"github.com/stretchr/testify/require"
+
 	"github.com/stretchr/testify/assert"
 	"github.com/suse-edge/edge-image-builder/pkg/image"
 )
@@ -15,6 +19,19 @@ var validNetwork = image.Network{
 }
 
 func TestValidateKubernetes(t *testing.T) {
+	configDir, err := os.MkdirTemp("", "eib-config-")
+	require.NoError(t, err)
+
+	defer func() {
+		assert.NoError(t, os.RemoveAll(configDir))
+	}()
+
+	valuesDir := filepath.Join(configDir, "kubernetes", "helm", "values")
+	require.NoError(t, os.MkdirAll(valuesDir, os.ModePerm))
+
+	apacheValuesPath := filepath.Join(valuesDir, "apache-values.yaml")
+	require.NoError(t, os.WriteFile(apacheValuesPath, []byte(""), 0o600))
+
 	tests := map[string]struct {
 		K8s                    image.Kubernetes
 		ExpectedFailedMessages []string
@@ -24,6 +41,7 @@ func TestValidateKubernetes(t *testing.T) {
 		},
 		`all valid`: {
 			K8s: image.Kubernetes{
+				Version: "1.0",
 				Network: validNetwork,
 				Nodes: []image.Node{
 					{
@@ -105,6 +123,7 @@ func TestValidateKubernetes(t *testing.T) {
 	for name, test := range tests {
 		t.Run(name, func(t *testing.T) {
 			ctx := image.Context{
+				ImageConfigDir: configDir,
 				ImageDefinition: &image.Definition{
 					Kubernetes: test.K8s,
 				},
@@ -987,3 +1006,76 @@ func TestValidateHelmCharts(t *testing.T) {
 		})
 	}
 }
+
+func TestValidateAdditionalArtifacts(t *testing.T) {
+	configDir, err := os.MkdirTemp("", "eib-config-")
+	require.NoError(t, err)
+
+	defer func() {
+		assert.NoError(t, os.RemoveAll(configDir))
+	}()
+
+	manifestsDir := filepath.Join(configDir, "kubernetes", "manifests")
+	require.NoError(t, os.MkdirAll(manifestsDir, os.ModePerm))
+
+	testManifest := filepath.Join(manifestsDir, "manifest.yaml")
+	require.NoError(t, os.WriteFile(testManifest, []byte(""), 0o600))
+
+	tests := map[string]struct {
+		K8s                    image.Kubernetes
+		ExpectedFailedMessages []string
+	}{
+		`missing versions all sections`: {
+			K8s: image.Kubernetes{
+				Manifests: image.Manifests{
+					URLs: []string{
+						"example.com",
+					},
+				},
+				Helm: image.Helm{
+					Charts: []image.HelmChart{
+						{
+							Name:           "",
+							RepositoryName: "another-apache-repo",
+							Version:        "10.7.0",
+						},
+					},
+					Repositories: []image.HelmRepository{
+						{
+							Name: "apache-repo",
+							URL:  "oci://registry-1.docker.io/bitnamicharts",
+						},
+					},
+				},
+			},
+			ExpectedFailedMessages: []string{
+				"Kubernetes version must be defined when Helm charts are specified",
+				"Kubernetes version must be defined when manifest URLs are specified",
+				"Kubernetes version must be defined when local manifests are configured",
+			},
+		},
+	}
+
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
+			ctx := &image.Context{
+				ImageConfigDir: configDir,
+				ImageDefinition: &image.Definition{
+					Kubernetes: test.K8s,
+				},
+			}
+			failures := validateAdditionalArtifacts(ctx)
+			assert.Len(t, failures, len(test.ExpectedFailedMessages))
+
+			var foundMessages []string
+			for _, foundValidation := range failures {
+				foundMessages = append(foundMessages, foundValidation.UserMessage)
+			}
+
+			for _, expectedMessage := range test.ExpectedFailedMessages {
+				assert.Contains(t, foundMessages, expectedMessage)
+			}
+
+		})
+	}
+}