From c284c6e0d16c77d045e151dd146a5e9277aa1214 Mon Sep 17 00:00:00 2001 From: dbw7 Date: Thu, 12 Dec 2024 21:08:53 -0500 Subject: [PATCH] more feedback updates --- pkg/combustion/kubernetes.go | 68 +++--- pkg/combustion/kubernetes_test.go | 64 ++++-- pkg/combustion/registry_test.go | 2 +- .../templates/k3s-multi-node-installer.sh.tpl | 2 +- .../k3s-single-node-installer.sh.tpl | 2 +- .../rke2-multi-node-installer.sh.tpl | 2 +- .../rke2-single-node-installer.sh.tpl | 2 +- pkg/combustion/templates/set-node-ip.sh.tpl | 2 +- pkg/image/validation/kubernetes.go | 195 ++++++++---------- pkg/image/validation/kubernetes_test.go | 28 +-- 10 files changed, 187 insertions(+), 180 deletions(-) diff --git a/pkg/combustion/kubernetes.go b/pkg/combustion/kubernetes.go index 5723e095..67e54998 100644 --- a/pkg/combustion/kubernetes.go +++ b/pkg/combustion/kubernetes.go @@ -19,8 +19,8 @@ import ( const ( k8sComponentName = "kubernetes" - K8sDir = "kubernetes" - K8sConfigDir = "config" + k8sDir = "kubernetes" + k8sConfigDir = "config" k8sInstallDir = "install" k8sImagesDir = "images" k8sManifestsDir = "manifests" @@ -80,8 +80,8 @@ func (c *Combustion) configureKubernetes(ctx *image.Context) ([]string, error) { zap.S().Warn("Kubernetes cluster of two server nodes has been requested") } - configDir := generateComponentPath(ctx, K8sDir) - configPath := filepath.Join(configDir, K8sConfigDir) + configDir := generateComponentPath(ctx, k8sDir) + configPath := filepath.Join(configDir, k8sConfigDir) cluster, err := kubernetes.NewCluster(&ctx.ImageDefinition.Kubernetes, configPath) if err != nil { @@ -89,10 +89,6 @@ func (c *Combustion) configureKubernetes(ctx *image.Context) ([]string, error) { return nil, fmt.Errorf("initialising cluster config: %w", err) } - if err = createNodeIPScript(ctx, cluster.ServerConfig); err != nil { - return nil, fmt.Errorf("creating set node IP script: %w", err) - } - artefactsPath := kubernetesArtefactsPath(ctx) if err = os.MkdirAll(artefactsPath, os.ModePerm); err != nil { return nil, fmt.Errorf("creating kubernetes artefacts path: %w", err) @@ -132,7 +128,7 @@ func (c *Combustion) downloadKubernetesInstallScript(ctx *image.Context, distrib return "", fmt.Errorf("downloading install script: %w", err) } - return prependArtefactPath(filepath.Join(K8sDir, installScript)), nil + return prependArtefactPath(filepath.Join(k8sDir, installScript)), nil } func (c *Combustion) configureK3S(ctx *image.Context, cluster *kubernetes.Cluster) (string, error) { @@ -153,6 +149,11 @@ func (c *Combustion) configureK3S(ctx *image.Context, cluster *kubernetes.Cluste return "", fmt.Errorf("configuring kubernetes manifests: %w", err) } + nodeIPScript, err := createNodeIPScript(ctx, cluster.ServerConfig) + if err != nil { + return "", fmt.Errorf("creating set node IP script: %w", err) + } + templateValues := map[string]any{ "installScript": installScript, "apiVIP4": ctx.ImageDefinition.Kubernetes.Network.APIVIP4, @@ -161,10 +162,9 @@ func (c *Combustion) configureK3S(ctx *image.Context, cluster *kubernetes.Cluste "binaryPath": binaryPath, "imagesPath": imagesPath, "manifestsPath": manifestsPath, - "configFilePath": prependArtefactPath(K8sDir), - "registryMirrors": prependArtefactPath(filepath.Join(K8sDir, registryMirrorsFileName)), - "setNodeIPScript": setNodeIPScript, - "getNodeIP": kubernetes.GetNodeIP(cluster.ServerConfig), + "configFilePath": prependArtefactPath(k8sDir), + "registryMirrors": prependArtefactPath(filepath.Join(k8sDir, registryMirrorsFileName)), + "setNodeIPScript": nodeIPScript, } singleNode := len(ctx.ImageDefinition.Kubernetes.Nodes) < 2 @@ -193,13 +193,13 @@ func (c *Combustion) configureK3S(ctx *image.Context, cluster *kubernetes.Cluste } func (c *Combustion) downloadK3sArtefacts(ctx *image.Context) (binaryPath, imagesPath string, err error) { - imagesPath = filepath.Join(K8sDir, k8sImagesDir) + imagesPath = filepath.Join(k8sDir, k8sImagesDir) imagesDestination := filepath.Join(ctx.ArtefactsDir, imagesPath) if err = os.MkdirAll(imagesDestination, os.ModePerm); err != nil { return "", "", fmt.Errorf("creating kubernetes images dir: %w", err) } - installPath := filepath.Join(K8sDir, k8sInstallDir) + installPath := filepath.Join(k8sDir, k8sInstallDir) installDestination := filepath.Join(ctx.ArtefactsDir, installPath) if err = os.MkdirAll(installDestination, os.ModePerm); err != nil { return "", "", fmt.Errorf("creating kubernetes install dir: %w", err) @@ -251,6 +251,11 @@ func (c *Combustion) configureRKE2(ctx *image.Context, cluster *kubernetes.Clust return "", fmt.Errorf("configuring kubernetes manifests: %w", err) } + nodeIPScript, err := createNodeIPScript(ctx, cluster.ServerConfig) + if err != nil { + return "", fmt.Errorf("creating set node IP script: %w", err) + } + templateValues := map[string]any{ "installScript": installScript, "apiVIP4": ctx.ImageDefinition.Kubernetes.Network.APIVIP4, @@ -259,10 +264,9 @@ func (c *Combustion) configureRKE2(ctx *image.Context, cluster *kubernetes.Clust "installPath": installPath, "imagesPath": imagesPath, "manifestsPath": manifestsPath, - "configFilePath": prependArtefactPath(K8sDir), - "registryMirrors": prependArtefactPath(filepath.Join(K8sDir, registryMirrorsFileName)), - "setNodeIPScript": setNodeIPScript, - "getNodeIP": kubernetes.GetNodeIP(cluster.ServerConfig), + "configFilePath": prependArtefactPath(k8sDir), + "registryMirrors": prependArtefactPath(filepath.Join(k8sDir, registryMirrorsFileName)), + "setNodeIPScript": nodeIPScript, } singleNode := len(ctx.ImageDefinition.Kubernetes.Nodes) < 2 @@ -303,13 +307,13 @@ func (c *Combustion) downloadRKE2Artefacts(ctx *image.Context, cluster *kubernet return "", "", fmt.Errorf("extracting CNI from cluster config: %w", err) } - imagesPath = filepath.Join(K8sDir, k8sImagesDir) + imagesPath = filepath.Join(k8sDir, k8sImagesDir) imagesDestination := filepath.Join(ctx.ArtefactsDir, imagesPath) if err = os.MkdirAll(imagesDestination, os.ModePerm); err != nil { return "", "", fmt.Errorf("creating kubernetes images dir: %w", err) } - installPath = filepath.Join(K8sDir, k8sInstallDir) + installPath = filepath.Join(k8sDir, k8sInstallDir) installDestination := filepath.Join(ctx.ArtefactsDir, installPath) if err = os.MkdirAll(installDestination, os.ModePerm); err != nil { return "", "", fmt.Errorf("creating kubernetes install dir: %w", err) @@ -343,10 +347,10 @@ func kubernetesVIPManifest(k *image.Kubernetes) (string, error) { return template.Parse("k8s-vip", k8sVIPManifest, &manifest) } -func createNodeIPScript(ctx *image.Context, serverConfig map[string]any) error { +func createNodeIPScript(ctx *image.Context, serverConfig map[string]any) (string, error) { // Setting the Node IP only matters if we're doing dual-stack or single-stack IPv6 - if ctx.ImageDefinition.Kubernetes.Network.APIVIP6 == "" { - return nil + if ctx.ImageDefinition.Kubernetes.Network.APIVIP6 == "" || !kubernetes.GetNodeIP(serverConfig) { + return "", nil } var isIPv4Enabled bool @@ -366,15 +370,15 @@ func createNodeIPScript(ctx *image.Context, serverConfig map[string]any) error { data, err := template.Parse("set-node-ip", nodeIPScriptTemplate, &manifest) if err != nil { - return fmt.Errorf("parsing '%s' template: %w", setNodeIPScript, err) + return "", fmt.Errorf("parsing '%s' template: %w", setNodeIPScript, err) } nodeIPScript := filepath.Join(ctx.CombustionDir, setNodeIPScript) if err = os.WriteFile(nodeIPScript, []byte(data), fileio.ExecutablePerms); err != nil { - return fmt.Errorf("writing set node IP script: %w", err) + return "", fmt.Errorf("writing set node IP script: %w", err) } - return nil + return setNodeIPScript, nil } func storeKubernetesClusterConfig(cluster *kubernetes.Cluster, destPath string) error { @@ -478,11 +482,11 @@ func (c *Combustion) configureManifests(ctx *image.Context) (string, error) { } func KubernetesConfigPath(ctx *image.Context) string { - return filepath.Join(ctx.ImageConfigDir, K8sDir, K8sConfigDir, k8sServerConfigFile) + return filepath.Join(ctx.ImageConfigDir, k8sDir, k8sConfigDir, k8sServerConfigFile) } func localKubernetesManifestsPath() string { - return filepath.Join(K8sDir, k8sManifestsDir) + return filepath.Join(k8sDir, k8sManifestsDir) } func KubernetesManifestsPath(ctx *image.Context) string { @@ -490,13 +494,13 @@ func KubernetesManifestsPath(ctx *image.Context) string { } func HelmValuesPath(ctx *image.Context) string { - return filepath.Join(ctx.ImageConfigDir, K8sDir, helmDir, helmValuesDir) + return filepath.Join(ctx.ImageConfigDir, k8sDir, helmDir, helmValuesDir) } func HelmCertsPath(ctx *image.Context) string { - return filepath.Join(ctx.ImageConfigDir, K8sDir, helmDir, helmCertsDir) + return filepath.Join(ctx.ImageConfigDir, k8sDir, helmDir, helmCertsDir) } func kubernetesArtefactsPath(ctx *image.Context) string { - return filepath.Join(ctx.ArtefactsDir, K8sDir) + return filepath.Join(ctx.ArtefactsDir, k8sDir) } diff --git a/pkg/combustion/kubernetes_test.go b/pkg/combustion/kubernetes_test.go index efd5b913..ba7d7ee4 100644 --- a/pkg/combustion/kubernetes_test.go +++ b/pkg/combustion/kubernetes_test.go @@ -479,7 +479,7 @@ func TestConfigureKubernetes_SuccessfulMultiNodeK3sClusterIPv4(t *testing.T) { b, err := yaml.Marshal(serverConfig) require.NoError(t, err) - configDir := filepath.Join(ctx.ImageConfigDir, K8sDir, K8sConfigDir) + configDir := filepath.Join(ctx.ImageConfigDir, k8sDir, k8sConfigDir) require.NoError(t, os.MkdirAll(configDir, os.ModePerm)) require.NoError(t, os.WriteFile(filepath.Join(configDir, "server.yaml"), b, os.ModePerm)) @@ -611,7 +611,7 @@ func TestConfigureKubernetes_SuccessfulMultiNodeK3sClusterIPv6(t *testing.T) { b, err := yaml.Marshal(serverConfig) require.NoError(t, err) - configDir := filepath.Join(ctx.ImageConfigDir, K8sDir, K8sConfigDir) + configDir := filepath.Join(ctx.ImageConfigDir, k8sDir, k8sConfigDir) require.NoError(t, os.MkdirAll(configDir, os.ModePerm)) require.NoError(t, os.WriteFile(filepath.Join(configDir, "server.yaml"), b, os.ModePerm)) @@ -746,7 +746,7 @@ func TestConfigureKubernetes_SuccessfulMultiNodeK3sClusterDualstackPrioIPv4(t *t b, err := yaml.Marshal(serverConfig) require.NoError(t, err) - configDir := filepath.Join(ctx.ImageConfigDir, K8sDir, K8sConfigDir) + configDir := filepath.Join(ctx.ImageConfigDir, k8sDir, k8sConfigDir) require.NoError(t, os.MkdirAll(configDir, os.ModePerm)) require.NoError(t, os.WriteFile(filepath.Join(configDir, "server.yaml"), b, os.ModePerm)) @@ -882,7 +882,7 @@ func TestConfigureKubernetes_SuccessfulMultiNodeK3sClusterDualstackPrioIPv6(t *t b, err := yaml.Marshal(serverConfig) require.NoError(t, err) - configDir := filepath.Join(ctx.ImageConfigDir, K8sDir, K8sConfigDir) + configDir := filepath.Join(ctx.ImageConfigDir, k8sDir, k8sConfigDir) require.NoError(t, os.MkdirAll(configDir, os.ModePerm)) require.NoError(t, os.WriteFile(filepath.Join(configDir, "server.yaml"), b, os.ModePerm)) @@ -1088,7 +1088,7 @@ func TestConfigureKubernetes_SuccessfulMultiNodeRKE2ClusterDualstackPrioIPv6With b, err := yaml.Marshal(serverConfig) require.NoError(t, err) - configDir := filepath.Join(ctx.ImageConfigDir, K8sDir, K8sConfigDir) + configDir := filepath.Join(ctx.ImageConfigDir, k8sDir, k8sConfigDir) require.NoError(t, os.MkdirAll(configDir, os.ModePerm)) require.NoError(t, os.WriteFile(filepath.Join(configDir, "server.yaml"), b, os.ModePerm)) @@ -1218,7 +1218,7 @@ func TestConfigureKubernetes_SuccessfulMultiNodeRKE2ClusterDualstackPrioIPv6With b, err := yaml.Marshal(serverConfig) require.NoError(t, err) - configDir := filepath.Join(ctx.ImageConfigDir, K8sDir, K8sConfigDir) + configDir := filepath.Join(ctx.ImageConfigDir, k8sDir, k8sConfigDir) require.NoError(t, os.MkdirAll(configDir, os.ModePerm)) require.NoError(t, os.WriteFile(filepath.Join(configDir, "server.yaml"), b, os.ModePerm)) @@ -1382,7 +1382,7 @@ values: content`, "oci://registry-1.docker.io/bitnamicharts"), assert.Equal(t, "$ARTEFACTS_DIR/kubernetes/manifests", manifestsPath) - manifestPath := filepath.Join(ctx.ArtefactsDir, K8sDir, k8sManifestsDir, "sample-crd.yaml") + manifestPath := filepath.Join(ctx.ArtefactsDir, k8sDir, k8sManifestsDir, "sample-crd.yaml") b, err := os.ReadFile(manifestPath) require.NoError(t, err) @@ -1393,7 +1393,7 @@ values: content`, "oci://registry-1.docker.io/bitnamicharts"), assert.Contains(t, contents, "name: my-nginx") assert.Contains(t, contents, "image: nginx:1.14.2") - chartPath := filepath.Join(ctx.ArtefactsDir, K8sDir, k8sManifestsDir, "apache.yaml") + chartPath := filepath.Join(ctx.ArtefactsDir, k8sDir, k8sManifestsDir, "apache.yaml") chartContent := `apiVersion: helm.cattle.io/v1 kind: HelmChart metadata: @@ -1497,7 +1497,7 @@ func TestConfigureKubernetes_SuccessfulRKE2ServerWithManifests(t *testing.T) { assert.Equal(t, []any{"192.168.122.100", "api.cluster01.hosted.on.edge.suse.com"}, configContents["tls-san"]) // Manifest assertions - manifest := filepath.Join(ctx.ArtefactsDir, K8sDir, k8sManifestsDir, "sample-crd.yaml") + manifest := filepath.Join(ctx.ArtefactsDir, k8sDir, k8sManifestsDir, "sample-crd.yaml") info, err = os.Stat(manifest) require.NoError(t, err) assert.Equal(t, fileio.NonExecutablePerms, info.Mode()) @@ -1583,11 +1583,12 @@ func TestCreateNodeIPScriptDualstackIPv6PrioK3s(t *testing.T) { "service-cidr": "fd12:3456:789c::/112,10.43.0.0/16", } - err := createNodeIPScript(ctx, serverConfig) + nodeIPScript, err := createNodeIPScript(ctx, serverConfig) require.NoError(t, err) + assert.Contains(t, nodeIPScript, "set-node-ip.sh") - nodeIPScript := filepath.Join(ctx.CombustionDir, setNodeIPScript) - b, err := os.ReadFile(nodeIPScript) + nodeIPScriptPath := filepath.Join(ctx.CombustionDir, setNodeIPScript) + b, err := os.ReadFile(nodeIPScriptPath) require.NoError(t, err) contents := string(b) @@ -1615,11 +1616,12 @@ func TestCreateNodeIPScriptDualstackIPv4PrioRke2(t *testing.T) { "service-cidr": "10.43.0.0/16,fd12:3456:789c::/112", } - err := createNodeIPScript(ctx, serverConfig) + nodeIPScript, err := createNodeIPScript(ctx, serverConfig) require.NoError(t, err) + assert.Contains(t, nodeIPScript, "set-node-ip.sh") - nodeIPScript := filepath.Join(ctx.CombustionDir, setNodeIPScript) - b, err := os.ReadFile(nodeIPScript) + nodeIPScriptPath := filepath.Join(ctx.CombustionDir, setNodeIPScript) + b, err := os.ReadFile(nodeIPScriptPath) require.NoError(t, err) contents := string(b) @@ -1643,8 +1645,9 @@ func TestCreateNodeIPScriptIPv4Only(t *testing.T) { serverConfig := map[string]any{} - err := createNodeIPScript(ctx, serverConfig) - require.Nil(t, err) + nodeIPScript, err := createNodeIPScript(ctx, serverConfig) + require.NoError(t, err) + assert.Contains(t, nodeIPScript, "") } func TestCreateNodeIPScriptIPv6Only(t *testing.T) { @@ -1663,11 +1666,12 @@ func TestCreateNodeIPScriptIPv6Only(t *testing.T) { "service-cidr": "fd12:3456:789c::/112", } - err := createNodeIPScript(ctx, serverConfig) + nodeIPScript, err := createNodeIPScript(ctx, serverConfig) require.NoError(t, err) + assert.Contains(t, nodeIPScript, "set-node-ip.sh") - nodeIPScript := filepath.Join(ctx.CombustionDir, setNodeIPScript) - b, err := os.ReadFile(nodeIPScript) + nodeIPScriptPath := filepath.Join(ctx.CombustionDir, setNodeIPScript) + b, err := os.ReadFile(nodeIPScriptPath) require.NoError(t, err) contents := string(b) @@ -1675,3 +1679,23 @@ func TestCreateNodeIPScriptIPv6Only(t *testing.T) { assert.Contains(t, contents, "IPv6=true") assert.Contains(t, contents, "CONFIG_FILE=\"/etc/rancher/rke2/config.yaml\"") } + +func TestCreateNodeIPScriptNodeIPSpecified(t *testing.T) { + ctx, teardown := setupContext(t) + defer teardown() + + ctx.ImageDefinition.Kubernetes = image.Kubernetes{ + Version: "v1.30.3+rke2r1", + Network: image.Network{ + APIVIP4: "192.168.1.1", + }, + } + + serverConfig := map[string]any{ + "node-ip": "192.168.100.100", + } + + nodeIPScript, err := createNodeIPScript(ctx, serverConfig) + require.NoError(t, err) + assert.Contains(t, nodeIPScript, "") +} diff --git a/pkg/combustion/registry_test.go b/pkg/combustion/registry_test.go index e16d1856..b8a27d56 100644 --- a/pkg/combustion/registry_test.go +++ b/pkg/combustion/registry_test.go @@ -153,7 +153,7 @@ func TestWriteRegistryMirrorsValid(t *testing.T) { // Verify require.NoError(t, err) - manifestFileName := filepath.Join(ctx.ArtefactsDir, K8sDir, registryMirrorsFileName) + manifestFileName := filepath.Join(ctx.ArtefactsDir, k8sDir, registryMirrorsFileName) foundBytes, err := os.ReadFile(manifestFileName) require.NoError(t, err) diff --git a/pkg/combustion/templates/k3s-multi-node-installer.sh.tpl b/pkg/combustion/templates/k3s-multi-node-installer.sh.tpl index bb12e816..91ab8503 100644 --- a/pkg/combustion/templates/k3s-multi-node-installer.sh.tpl +++ b/pkg/combustion/templates/k3s-multi-node-installer.sh.tpl @@ -101,7 +101,7 @@ mkdir -p /etc/rancher/k3s/ cp $CONFIGFILE /etc/rancher/k3s/config.yaml if [ "$NODETYPE" = "server" ]; then -{{- if and .apiVIP6 .getNodeIP}} +{{- if .setNodeIPScript }} sh {{ .setNodeIPScript }} {{- end }} fi diff --git a/pkg/combustion/templates/k3s-single-node-installer.sh.tpl b/pkg/combustion/templates/k3s-single-node-installer.sh.tpl index a47688d9..304d0a8a 100644 --- a/pkg/combustion/templates/k3s-single-node-installer.sh.tpl +++ b/pkg/combustion/templates/k3s-single-node-installer.sh.tpl @@ -73,7 +73,7 @@ echo "{{ .apiVIP6 }} {{ .apiHost }}" >> /etc/hosts mkdir -p /etc/rancher/k3s/ cp {{ .configFilePath }}/{{ .configFile }} /etc/rancher/k3s/config.yaml -{{- if and .apiVIP6 .getNodeIP}} +{{- if .setNodeIPScript }} sh {{ .setNodeIPScript }} {{- end }} diff --git a/pkg/combustion/templates/rke2-multi-node-installer.sh.tpl b/pkg/combustion/templates/rke2-multi-node-installer.sh.tpl index 5e9de80d..1242b11a 100644 --- a/pkg/combustion/templates/rke2-multi-node-installer.sh.tpl +++ b/pkg/combustion/templates/rke2-multi-node-installer.sh.tpl @@ -103,7 +103,7 @@ mkdir -p /etc/rancher/rke2/ cp $CONFIGFILE /etc/rancher/rke2/config.yaml if [ "$NODETYPE" = "server" ]; then -{{- if and .apiVIP6 .getNodeIP}} +{{- if .setNodeIPScript }} sh {{ .setNodeIPScript }} {{- end }} fi diff --git a/pkg/combustion/templates/rke2-single-node-installer.sh.tpl b/pkg/combustion/templates/rke2-single-node-installer.sh.tpl index 91189a0c..16bcc84c 100644 --- a/pkg/combustion/templates/rke2-single-node-installer.sh.tpl +++ b/pkg/combustion/templates/rke2-single-node-installer.sh.tpl @@ -75,7 +75,7 @@ echo "{{ .apiVIP6 }} {{ .apiHost }}" >> /etc/hosts mkdir -p /etc/rancher/rke2/ cp {{ .configFilePath }}/{{ .configFile }} /etc/rancher/rke2/config.yaml -{{- if and .apiVIP6 .getNodeIP}} +{{- if .setNodeIPScript }} sh {{ .setNodeIPScript }} {{- end }} diff --git a/pkg/combustion/templates/set-node-ip.sh.tpl b/pkg/combustion/templates/set-node-ip.sh.tpl index 8b19526e..2abb1fce 100644 --- a/pkg/combustion/templates/set-node-ip.sh.tpl +++ b/pkg/combustion/templates/set-node-ip.sh.tpl @@ -106,7 +106,7 @@ update_config() { if [ -n "${IPv4_ADDRESS}" ] && [ -n "${IPv6_ADDRESS}" ]; then if [ "$prioritizeIPv6" = "false" ]; then echo "node-ip: ${IPv4_ADDRESS},${IPv6_ADDRESS}" >> "$CONFIG_FILE" - echo "Added IPv4 and IPv6 addresses to config (IPv4 prioritized)" + echo "Added IPv4 and IPv6 addresses ${IPv4_ADDRESS},${IPv6_ADDRESS} to config (IPv4 prioritized)" else echo "node-ip: ${IPv6_ADDRESS},${IPv4_ADDRESS}" >> "$CONFIG_FILE" echo "Added IPv6 and IPv4 addresses ${IPv6_ADDRESS},${IPv4_ADDRESS} to config (IPv6 prioritized)" diff --git a/pkg/image/validation/kubernetes.go b/pkg/image/validation/kubernetes.go index 9eb18fb7..4dc28ce7 100644 --- a/pkg/image/validation/kubernetes.go +++ b/pkg/image/validation/kubernetes.go @@ -10,6 +10,8 @@ import ( "slices" "strings" + "github.com/suse-edge/edge-image-builder/pkg/kubernetes" + "gopkg.in/yaml.v3" "github.com/suse-edge/edge-image-builder/pkg/combustion" @@ -35,7 +37,7 @@ func validateKubernetes(ctx *image.Context) []FailedValidation { return failures } - failures = append(failures, validateDualStackConfig(&def.Kubernetes, combustion.KubernetesConfigPath(ctx))...) + failures = append(failures, validateNetworkingConfig(&def.Kubernetes, combustion.KubernetesConfigPath(ctx))...) failures = append(failures, validateNetwork(&def.Kubernetes)...) failures = append(failures, validateNodes(&def.Kubernetes)...) failures = append(failures, validateManifestURLs(&def.Kubernetes)...) @@ -167,7 +169,7 @@ func validateNetwork(k8s *image.Kubernetes) []FailedValidation { if !ip6.Is6() { failures = append(failures, FailedValidation{ - UserMessage: "Only a IPv6 addresses are valid for field 'apiVIP6'.", + UserMessage: "Only IPv6 addresses are valid for field 'apiVIP6'.", }) } @@ -182,24 +184,25 @@ func validateNetwork(k8s *image.Kubernetes) []FailedValidation { return failures } -func validateDualStackConfig(k8s *image.Kubernetes, kubernetesConfigPath string) []FailedValidation { +func validateNetworkingConfig(k8s *image.Kubernetes, kubernetesConfigPath string) []FailedValidation { var failures []FailedValidation configFile, err := os.ReadFile(kubernetesConfigPath) if err != nil { - if errors.Is(err, os.ErrNotExist) && (k8s.Network.APIVIP4 != "" && k8s.Network.APIVIP6 != "") { - failures = append(failures, FailedValidation{ - UserMessage: fmt.Sprintf("Kubernetes server config could not be found at '%s,' dual-stack configuration requires a valid cluster-cidr and service-cidr.", kubernetesConfigPath), - }) - + if errors.Is(err, os.ErrNotExist) { + if isDualstackConfigured(k8s) { + failures = append(failures, FailedValidation{ + UserMessage: fmt.Sprintf("Kubernetes server config could not be found at '%s,' dual-stack configuration requires a valid cluster-cidr and service-cidr.", kubernetesConfigPath), + }) + } return failures - } else if k8s.Network.APIVIP4 != "" && k8s.Network.APIVIP6 != "" { - failures = append(failures, FailedValidation{ - UserMessage: "Kubernetes server config could not be read", - Error: err, - }) } + failures = append(failures, FailedValidation{ + UserMessage: "Kubernetes server config could not be read", + Error: err, + }) + return failures } @@ -222,34 +225,24 @@ func validateDualStackConfig(k8s *image.Kubernetes, kubernetesConfigPath string) func validateCIDRConfig(k8s *image.Kubernetes, serverConfig map[string]any) []FailedValidation { var failures []FailedValidation - parsedClusterCIDRs, cidrFailures := parseCIDRs(k8s, serverConfig, "cluster-cidr") - if len(cidrFailures) != 0 { - failures = append(failures, cidrFailures...) - return failures - } - - parsedServiceCIDRs, cidrFailures := parseCIDRs(k8s, serverConfig, "service-cidr") - if len(cidrFailures) != 0 { - failures = append(failures, cidrFailures...) - return failures - } - - if len(parsedClusterCIDRs) == 0 && len(parsedServiceCIDRs) == 0 { - return failures + parsedClusterCIDRs := parseCIDRs(serverConfig, "cluster-cidr") + if len(parsedClusterCIDRs) < 2 { + if isDualstackConfigured(k8s) { + failures = append(failures, FailedValidation{ + UserMessage: "Kubernetes server config must contain a valid cluster-cidr when configuring dual-stack", + }) + return failures + } } - if len(parsedServiceCIDRs) != 0 && len(parsedClusterCIDRs) == 0 { - failures = append(failures, FailedValidation{ - UserMessage: "Kubernetes server config cluster-cidr must be defined when service-cidr is defined", - }) - - return failures - } else if len(parsedServiceCIDRs) == 0 && len(parsedClusterCIDRs) != 0 { - failures = append(failures, FailedValidation{ - UserMessage: "Kubernetes server config service-cidr must be defined when cluster-cidr is defined", - }) - - return failures + parsedServiceCIDRs := parseCIDRs(serverConfig, "service-cidr") + if len(parsedServiceCIDRs) < 2 { + if isDualstackConfigured(k8s) { + failures = append(failures, FailedValidation{ + UserMessage: "Kubernetes server config must contain a valid service-cidr when configuring dual-stack", + }) + return failures + } } clusterCIDRIPv6Priority, clusterCIDRFailures := validateCIDRs(parsedClusterCIDRs, "cluster-cidr") @@ -262,38 +255,23 @@ func validateCIDRConfig(k8s *image.Kubernetes, serverConfig map[string]any) []Fa failures = append(failures, serviceCIDRFailures...) } - if clusterCIDRIPv6Priority && !serviceCIDRIPv6Priority { - failures = append(failures, FailedValidation{ - UserMessage: "Kubernetes server config cluster-cidr cannot prioritize IPv6 while service-cidr prioritizes IPv4, both must have the same priority", - }) - } else if !clusterCIDRIPv6Priority && serviceCIDRIPv6Priority { + if clusterCIDRIPv6Priority && !serviceCIDRIPv6Priority || !clusterCIDRIPv6Priority && serviceCIDRIPv6Priority { failures = append(failures, FailedValidation{ - UserMessage: "Kubernetes server config cluster-cidr cannot prioritize IPv4 while service-cidr prioritizes IPv6, both must have the same priority", + UserMessage: "Kubernetes server config cluster-cidr cannot prioritize one address family while service-cidr prioritizes another address family, both must have the same priority", }) } return failures } -func parseCIDRs(k8s *image.Kubernetes, serverConfig map[string]any, cidrField string) ([]string, []FailedValidation) { - var failures []FailedValidation +func parseCIDRs(serverConfig map[string]any, cidrField string) []string { var parsedCIDRs []string if cidr, ok := serverConfig[cidrField].(string); ok { parsedCIDRs = strings.Split(cidr, ",") - if len(parsedCIDRs) != 2 && k8s.Network.APIVIP4 != "" && k8s.Network.APIVIP6 != "" { - failures = append(failures, FailedValidation{ - UserMessage: fmt.Sprintf("Kubernetes server config %s required for a dual-stack cluster", cidrField), - }) - return nil, failures - } - } else if k8s.Network.APIVIP4 != "" && k8s.Network.APIVIP6 != "" { - failures = append(failures, FailedValidation{ - UserMessage: fmt.Sprintf("Kubernetes server config must contain a valid %s when configuring dual-stack", cidrField), - }) - return nil, failures + return parsedCIDRs } - return parsedCIDRs, failures + return nil } func validateCIDRs(parsedCIDRs []string, cidrType string) (bool, []FailedValidation) { @@ -337,58 +315,56 @@ func validateCIDRs(parsedCIDRs []string, cidrType string) (bool, []FailedValidat func validateNodeIP(k8s *image.Kubernetes, serverConfig map[string]any) []FailedValidation { var failures []FailedValidation + var parsedNodeIPs []string - if nodeIPs, ok := serverConfig["node-ip"].(string); ok { - if len(k8s.Nodes) > 1 { - var serverCount int - for _, nodes := range k8s.Nodes { - if nodes.Type == "server" { - serverCount++ - } - } + var nodeIPs string + var ok bool + if nodeIPs, ok = serverConfig["node-ip"].(string); !ok { + return failures + } + parsedNodeIPs = strings.Split(nodeIPs, ",") - if serverCount > 1 { - failures = append(failures, FailedValidation{ - UserMessage: "Kubernetes server config node-ip can not be specified when there is more than one Kubernetes server node", - }) - return failures - } + if len(k8s.Nodes) > 1 { + if kubernetes.ServersCount(k8s.Nodes) > 1 { + failures = append(failures, FailedValidation{ + UserMessage: "Kubernetes server config node-ip can not be specified when there is more than one Kubernetes server node", + }) + return failures } + } - parsedNodeIPs := strings.Split(nodeIPs, ",") - switch len(parsedNodeIPs) { - case 1: - _, ipFailures := validateIP(parsedNodeIPs[0], "node-ip", false) - if len(ipFailures) != 0 { - failures = append(failures, ipFailures...) - return failures - } - case 2: - ip1, ipFailures := validateIP(parsedNodeIPs[0], "node-ip", false) - if len(ipFailures) != 0 { - failures = append(failures, ipFailures...) - return failures - } + switch len(parsedNodeIPs) { + case 1: + _, ipFailures := validateIP(parsedNodeIPs[0], "node-ip", false) + if len(ipFailures) != 0 { + failures = append(failures, ipFailures...) + return failures + } + case 2: + ip1, ipFailures := validateIP(parsedNodeIPs[0], "node-ip", false) + if len(ipFailures) != 0 { + failures = append(failures, ipFailures...) + return failures + } - ip2, ipFailures := validateIP(parsedNodeIPs[1], "node-ip", false) - if len(ipFailures) != 0 { - failures = append(failures, ipFailures...) - return failures - } + ip2, ipFailures := validateIP(parsedNodeIPs[1], "node-ip", false) + if len(ipFailures) != 0 { + failures = append(failures, ipFailures...) + return failures + } - if (ip1.Is4() && ip2.Is4()) || (ip1.Is6() && ip2.Is6()) { - failures = append(failures, FailedValidation{ - UserMessage: fmt.Sprintf("Kubernetes server config node-ip %s and %s cannot both be of the same IP address family, one must be IPv4, and the other IPv6", parsedNodeIPs[0], parsedNodeIPs[1]), - }) - return failures - } - default: + if (ip1.Is4() && ip2.Is4()) || (ip1.Is6() && ip2.Is6()) { failures = append(failures, FailedValidation{ - UserMessage: "Kubernetes server config node-ip cannot contain more than one IPv4 IP and/or one IPv6 IP", + UserMessage: fmt.Sprintf("Kubernetes server config node-ip %s and %s cannot both be of the same IP address family, one must be IPv4, and the other IPv6", parsedNodeIPs[0], parsedNodeIPs[1]), }) return failures - } + default: + failures = append(failures, FailedValidation{ + UserMessage: "Kubernetes server config node-ip cannot contain more than one IPv4 IP and/or one IPv6 IP", + }) + return failures + } return failures @@ -421,14 +397,9 @@ func validateIP(ip string, configField string, hasPrefix bool) (*netip.Addr, []F extractedIP = parsedIP } - if !extractedIP.IsGlobalUnicast() && !hasPrefix { + if !extractedIP.IsGlobalUnicast() { failures = append(failures, FailedValidation{ - UserMessage: fmt.Sprintf("Kubernetes server config node-ip %s must be a valid unicast IP", ip), - }) - return nil, failures - } else if !extractedIP.IsGlobalUnicast() && hasPrefix { - failures = append(failures, FailedValidation{ - UserMessage: fmt.Sprintf("Kubernetes server config %s value '%s' must be a valid unicast address with a prefix", configField, ip), + UserMessage: fmt.Sprintf("Kubernetes server config %s value '%s' must be a valid unicast address", configField, ip), }) return nil, failures } @@ -771,3 +742,11 @@ func validateAdditionalArtifacts(ctx *image.Context) []FailedValidation { return failures } + +func isDualstackConfigured(k8s *image.Kubernetes) bool { + if k8s.Network.APIVIP4 != "" && k8s.Network.APIVIP6 != "" { + return true + } + + return false +} diff --git a/pkg/image/validation/kubernetes_test.go b/pkg/image/validation/kubernetes_test.go index 64043db3..e9ca608c 100644 --- a/pkg/image/validation/kubernetes_test.go +++ b/pkg/image/validation/kubernetes_test.go @@ -1335,7 +1335,7 @@ func TestValidateConfigValidIPv6Prio(t *testing.T) { configFile := filepath.Join(serverConfigDir, "server.yaml") require.NoError(t, os.WriteFile(configFile, []byte(b), 0o600)) - failures := validateDualStackConfig(&k8s, configFile) + failures := validateNetworkingConfig(&k8s, configFile) assert.Len(t, failures, 0) } @@ -1367,7 +1367,7 @@ func TestValidateConfigValidIPv4Prio(t *testing.T) { configFile := filepath.Join(serverConfigDir, "server.yaml") require.NoError(t, os.WriteFile(configFile, []byte(b), 0o600)) - failures := validateDualStackConfig(&k8s, configFile) + failures := validateNetworkingConfig(&k8s, configFile) assert.Len(t, failures, 0) } @@ -1399,7 +1399,7 @@ func TestValidateConfigInvalidBothIPv4(t *testing.T) { configFile := filepath.Join(serverConfigDir, "server.yaml") require.NoError(t, os.WriteFile(configFile, []byte(b), 0o600)) - failures := validateDualStackConfig(&k8s, configFile) + failures := validateNetworkingConfig(&k8s, configFile) assert.Len(t, failures, 2) @@ -1439,7 +1439,7 @@ func TestValidateConfigInvalidBothIPv6(t *testing.T) { configFile := filepath.Join(serverConfigDir, "server.yaml") require.NoError(t, os.WriteFile(configFile, []byte(b), 0o600)) - failures := validateDualStackConfig(&k8s, configFile) + failures := validateNetworkingConfig(&k8s, configFile) assert.Len(t, failures, 2) @@ -1458,7 +1458,7 @@ func TestValidateConfigInvalidServerConfigNotConfigured(t *testing.T) { APIVIP6: "fd12:3456:789a::21", }} - failures := validateDualStackConfig(&k8s, "fake-path") + failures := validateNetworkingConfig(&k8s, "fake-path") assert.Len(t, failures, 1) @@ -1473,7 +1473,7 @@ func TestValidateConfigInvalidServerConfigNotConfigured(t *testing.T) { func TestValidateConfigValidAPIVIPNotConfigured(t *testing.T) { k8s := image.Kubernetes{} - failures := validateDualStackConfig(&k8s, "") + failures := validateNetworkingConfig(&k8s, "") assert.Len(t, failures, 0) } @@ -1636,8 +1636,8 @@ func TestValidateConfigInvalidIPv4NonUnicast(t *testing.T) { foundMessages = append(foundMessages, foundValidation.UserMessage) } - assert.Contains(t, foundMessages, "Kubernetes server config cluster-cidr value '127.0.0.1/16' must be a valid unicast address with a prefix") - assert.Contains(t, foundMessages, "Kubernetes server config service-cidr value '127.0.0.1/16' must be a valid unicast address with a prefix") + assert.Contains(t, foundMessages, "Kubernetes server config cluster-cidr value '127.0.0.1/16' must be a valid unicast address") + assert.Contains(t, foundMessages, "Kubernetes server config service-cidr value '127.0.0.1/16' must be a valid unicast address") } func TestValidateConfigInvalidIPv6NonUnicast(t *testing.T) { @@ -1660,8 +1660,8 @@ func TestValidateConfigInvalidIPv6NonUnicast(t *testing.T) { foundMessages = append(foundMessages, foundValidation.UserMessage) } - assert.Contains(t, foundMessages, "Kubernetes server config cluster-cidr value 'FF01::/48' must be a valid unicast address with a prefix") - assert.Contains(t, foundMessages, "Kubernetes server config service-cidr value 'FF01::/112' must be a valid unicast address with a prefix") + assert.Contains(t, foundMessages, "Kubernetes server config cluster-cidr value 'FF01::/48' must be a valid unicast address") + assert.Contains(t, foundMessages, "Kubernetes server config service-cidr value 'FF01::/112' must be a valid unicast address") } func TestValidateNodeIPValid(t *testing.T) { @@ -1684,8 +1684,8 @@ func TestValidateNodeIPValid(t *testing.T) { foundMessages = append(foundMessages, foundValidation.UserMessage) } - assert.Contains(t, foundMessages, "Kubernetes server config cluster-cidr value 'FF01::/48' must be a valid unicast address with a prefix") - assert.Contains(t, foundMessages, "Kubernetes server config service-cidr value 'FF01::/112' must be a valid unicast address with a prefix") + assert.Contains(t, foundMessages, "Kubernetes server config cluster-cidr value 'FF01::/48' must be a valid unicast address") + assert.Contains(t, foundMessages, "Kubernetes server config service-cidr value 'FF01::/112' must be a valid unicast address") } func TestValidateConfigMismatchedPrio(t *testing.T) { @@ -1705,7 +1705,7 @@ func TestValidateConfigMismatchedPrio(t *testing.T) { foundMessages = append(foundMessages, foundValidation.UserMessage) } - assert.Contains(t, foundMessages, "Kubernetes server config cluster-cidr cannot prioritize IPv4 while service-cidr prioritizes IPv6, both must have the same priority") + assert.Contains(t, foundMessages, "Kubernetes server config cluster-cidr cannot prioritize one address family while service-cidr prioritizes another address family, both must have the same priority") } func TestValidateConfigSingleCIDRIPv6(t *testing.T) { @@ -1862,7 +1862,7 @@ func TestValidateNodeIPNonunicastIPv4Invalid(t *testing.T) { foundMessages = append(foundMessages, foundValidation.UserMessage) } - assert.Contains(t, foundMessages, "Kubernetes server config node-ip 127.0.0.1 must be a valid unicast IP") + assert.Contains(t, foundMessages, "Kubernetes server config node-ip value '127.0.0.1' must be a valid unicast address") } func TestValidateNodeIPIPv6Invalid(t *testing.T) {