From c1664497601f5f42b48c2f636b80b8f231358e56 Mon Sep 17 00:00:00 2001 From: dbw7 Date: Thu, 9 Jan 2025 14:39:54 -0500 Subject: [PATCH] cleanup + feedback --- pkg/image/definition_test.go | 2 +- pkg/image/testdata/full-valid-example.yaml | 2 +- pkg/image/validation/kubernetes_test.go | 873 +++++++++------------ pkg/image/validation/version.go | 6 + pkg/kubernetes/cluster.go | 35 +- pkg/kubernetes/cluster_test.go | 12 +- pkg/version/version.go | 3 +- 7 files changed, 403 insertions(+), 530 deletions(-) diff --git a/pkg/image/definition_test.go b/pkg/image/definition_test.go index 61bd2ee6..7cb88cd2 100644 --- a/pkg/image/definition_test.go +++ b/pkg/image/definition_test.go @@ -21,7 +21,7 @@ func TestParse(t *testing.T) { require.NoError(t, err) // - Definition - assert.Equal(t, "1.1", definition.APIVersion) + assert.Equal(t, "1.2", definition.APIVersion) assert.EqualValues(t, "x86_64", definition.Image.Arch) assert.Equal(t, "iso", definition.Image.ImageType) diff --git a/pkg/image/testdata/full-valid-example.yaml b/pkg/image/testdata/full-valid-example.yaml index 311de199..3824ea21 100644 --- a/pkg/image/testdata/full-valid-example.yaml +++ b/pkg/image/testdata/full-valid-example.yaml @@ -1,4 +1,4 @@ -apiVersion: 1.1 +apiVersion: 1.2 image: imageType: iso arch: x86_64 diff --git a/pkg/image/validation/kubernetes_test.go b/pkg/image/validation/kubernetes_test.go index ab14f0c0..0e91eb9e 100644 --- a/pkg/image/validation/kubernetes_test.go +++ b/pkg/image/validation/kubernetes_test.go @@ -123,7 +123,7 @@ func TestValidateKubernetes(t *testing.T) { "Helm repository 'name' field for \"apache-repo\" must match the 'repositoryName' field in at least one defined Helm chart.", "Helm chart 'repositoryName' \"another-apache-repo\" for Helm chart \"\" does not match the name of any defined repository.", "Non-unicast cluster API address (127.0.0.1) for field 'apiVIP' is invalid.", - "Non-unicast cluster API address (127.0.0.1) for field 'apiVIP' is invalid.", + "Non-unicast cluster API address (ff02::1) for field 'apiVIP6' is invalid.", fmt.Sprintf("Kubernetes server config could not be found at '%s'; dual-stack configuration requires a valid cluster-cidr and service-cidr.", filepath.Join(configDir, "kubernetes", "config", "server.yaml")), }, }, @@ -1308,35 +1308,28 @@ func TestValidateNetwork(t *testing.T) { } } -func TestValidateConfigValidIPv6Prio(t *testing.T) { +func TestValidateConfigInvalidServerConfigNotConfigured(t *testing.T) { k8s := image.Kubernetes{Network: image.Network{ APIVIP4: "192.168.1.1", APIVIP6: "fd12:3456:789a::21", }} - configDir, err := os.MkdirTemp("", "eib-config-") - require.NoError(t, err) - - defer func() { - assert.NoError(t, os.RemoveAll(configDir)) - }() + failures := validateNetworkingConfig(&k8s, "fake-path") - serverConfigDir := filepath.Join(configDir, "kubernetes", "config") - require.NoError(t, os.MkdirAll(serverConfigDir, os.ModePerm)) + assert.Len(t, failures, 1) - serverConfig := map[string]any{ - "cluster-cidr": "fd12:3456:789b::/48,10.42.0.0/16", - "service-cidr": "fd12:3456:789c::/112,10.43.0.0/16", + var foundMessages []string + for _, foundValidation := range failures { + foundMessages = append(foundMessages, foundValidation.UserMessage) } - b, err := yaml.Marshal(serverConfig) - require.NoError(t, err) - - configFile := filepath.Join(serverConfigDir, "server.yaml") - require.NoError(t, os.WriteFile(configFile, []byte(b), 0o600)) + assert.Contains(t, foundMessages, "Kubernetes server config could not be found at 'fake-path'; dual-stack configuration requires a valid cluster-cidr and service-cidr.") +} - failures := validateNetworkingConfig(&k8s, configFile) +func TestValidateConfigValidAPIVIPNotConfigured(t *testing.T) { + k8s := image.Kubernetes{} + failures := validateNetworkingConfig(&k8s, "") assert.Len(t, failures, 0) } @@ -1372,514 +1365,396 @@ func TestValidateConfigValidIPv4Prio(t *testing.T) { assert.Len(t, failures, 0) } -func TestValidateConfigInvalidBothIPv4(t *testing.T) { - k8s := image.Kubernetes{Network: image.Network{ - APIVIP4: "192.168.1.1", - APIVIP6: "fd12:3456:789a::21", - }} - - configDir, err := os.MkdirTemp("", "eib-config-") - require.NoError(t, err) +func mockValidateConfig(k8s *image.Kubernetes, serverConfig map[string]any) []FailedValidation { + var failures []FailedValidation - defer func() { - assert.NoError(t, os.RemoveAll(configDir)) - }() + failures = append(failures, validateNodeIP(k8s, serverConfig)...) + failures = append(failures, validateCIDRConfig(k8s, serverConfig)...) - serverConfigDir := filepath.Join(configDir, "kubernetes", "config") - require.NoError(t, os.MkdirAll(serverConfigDir, os.ModePerm)) + return failures +} - serverConfig := map[string]any{ - "cluster-cidr": "10.42.0.0/16,10.44.0.0/16", - "service-cidr": "10.43.0.0/16,10.45.0.0/16", +func TestValidateConfig(t *testing.T) { + tests := map[string]struct { + K8s image.Kubernetes + ServerConfig map[string]any + ExpectedFailedMessages []string + }{ + `valid IPv6 prio`: { + K8s: image.Kubernetes{ + Network: image.Network{ + APIVIP4: "192.168.1.1", + APIVIP6: "fd12:3456:789a::21", + }, + }, + ServerConfig: map[string]any{ + "cluster-cidr": "fd12:3456:789b::/48,10.42.0.0/16", + "service-cidr": "fd12:3456:789c::/112,10.43.0.0/16", + }, + }, + `invalid both IPv4`: { + K8s: image.Kubernetes{ + Network: image.Network{ + APIVIP4: "192.168.1.1", + APIVIP6: "fd12:3456:789a::21", + }, + }, + ServerConfig: map[string]any{ + "cluster-cidr": "10.42.0.0/16,10.44.0.0/16", + "service-cidr": "10.43.0.0/16,10.45.0.0/16", + }, + ExpectedFailedMessages: []string{ + "Kubernetes server config cluster-cidr cannot contain addresses of the same IP address family; one must be IPv4, and the other IPv6", + "Kubernetes server config service-cidr cannot contain addresses of the same IP address family; one must be IPv4, and the other IPv6", + }, + }, + `invalid both IPv6`: { + K8s: image.Kubernetes{ + Network: image.Network{ + APIVIP4: "192.168.1.1", + APIVIP6: "fd12:3456:789a::21", + }, + }, + ServerConfig: map[string]any{ + "cluster-cidr": "fd12:3456:789d::/48,fd12:3456:789b::/48", + "service-cidr": "fd12:3456:789e::/112,fd12:3456:789c::/112", + }, + ExpectedFailedMessages: []string{ + "Kubernetes server config cluster-cidr cannot contain addresses of the same IP address family; one must be IPv4, and the other IPv6", + "Kubernetes server config service-cidr cannot contain addresses of the same IP address family; one must be IPv4, and the other IPv6", + }, + }, } - b, err := yaml.Marshal(serverConfig) - require.NoError(t, err) - - configFile := filepath.Join(serverConfigDir, "server.yaml") - require.NoError(t, os.WriteFile(configFile, []byte(b), 0o600)) + for name, test := range tests { + t.Run(name, func(t *testing.T) { + k := test.K8s + sc := test.ServerConfig + failures := mockValidateConfig(&k, sc) + assert.Len(t, failures, len(test.ExpectedFailedMessages)) - failures := validateNetworkingConfig(&k8s, configFile) + var foundMessages []string + for _, foundValidation := range failures { + foundMessages = append(foundMessages, foundValidation.UserMessage) + } - assert.Len(t, failures, 2) + for _, expectedMessage := range test.ExpectedFailedMessages { + assert.Contains(t, foundMessages, expectedMessage) + } - var foundMessages []string - for _, foundValidation := range failures { - foundMessages = append(foundMessages, foundValidation.UserMessage) + }) } - - assert.Contains(t, foundMessages, "Kubernetes server config cluster-cidr cannot contain addresses of the same IP address family; one must be IPv4, and the other IPv6") - assert.Contains(t, foundMessages, "Kubernetes server config service-cidr cannot contain addresses of the same IP address family; one must be IPv4, and the other IPv6") } -func TestValidateConfigInvalidBothIPv6(t *testing.T) { - k8s := image.Kubernetes{Network: image.Network{ - APIVIP4: "192.168.1.1", - APIVIP6: "fd12:3456:789a::21", - }} - - configDir, err := os.MkdirTemp("", "eib-config-") - require.NoError(t, err) - - defer func() { - assert.NoError(t, os.RemoveAll(configDir)) - }() - - serverConfigDir := filepath.Join(configDir, "kubernetes", "config") - require.NoError(t, os.MkdirAll(serverConfigDir, os.ModePerm)) - - serverConfig := map[string]any{ - "cluster-cidr": "fd12:3456:789d::/48,fd12:3456:789b::/48", - "service-cidr": "fd12:3456:789e::/112,fd12:3456:789c::/112", +func TestValidateCIDRConfig(t *testing.T) { + tests := map[string]struct { + K8s image.Kubernetes + ServerConfig map[string]any + ExpectedFailedMessages []string + }{ + `cluster cidr not configured`: { + K8s: image.Kubernetes{ + Network: image.Network{ + APIVIP4: "192.168.1.1", + APIVIP6: "fd12:3456:789a::21", + }, + }, + ServerConfig: map[string]any{ + "service-cidr": "10.43.0.0/16,fd12:3456:789c::/112", + }, + ExpectedFailedMessages: []string{ + "Kubernetes server config must contain a valid cluster-cidr when configuring dual-stack", + }, + }, + `service cidr not configured`: { + K8s: image.Kubernetes{ + Network: image.Network{ + APIVIP4: "192.168.1.1", + APIVIP6: "fd12:3456:789a::21", + }, + }, + ServerConfig: map[string]any{ + "cluster-cidr": "fd12:3456:789b::/48,10.42.0.0/16", + }, + ExpectedFailedMessages: []string{ + "Kubernetes server config must contain a valid service-cidr when configuring dual-stack", + }, + }, + `invalid IPv4`: { + K8s: image.Kubernetes{ + Network: image.Network{ + APIVIP4: "192.168.1.1", + APIVIP6: "fd12:3456:789a::21", + }, + }, + ServerConfig: map[string]any{ + "cluster-cidr": "500.42.0.0/16,fd12:3456:789b::/48", + "service-cidr": "500.43.0.0/16,fd12:3456:789c::/112", + }, + ExpectedFailedMessages: []string{ + "Kubernetes server config cluster-cidr value '500.42.0.0/16' could not be parsed", + "Kubernetes server config service-cidr value '500.43.0.0/16' could not be parsed", + }, + }, + `invalid IPv6`: { + K8s: image.Kubernetes{ + Network: image.Network{ + APIVIP4: "192.168.1.1", + APIVIP6: "fd12:3456:789a::21", + }, + }, + ServerConfig: map[string]any{ + "cluster-cidr": "10.42.0.0/16,xxxx:3456:789b::/48", + "service-cidr": "10.43.0.0/16,xxxx:3456:789c::/112", + }, + ExpectedFailedMessages: []string{ + "Kubernetes server config cluster-cidr value 'xxxx:3456:789b::/48' could not be parsed", + "Kubernetes server config service-cidr value 'xxxx:3456:789c::/112' could not be parsed", + }, + }, + `invalid IPv6 prefix`: { + K8s: image.Kubernetes{ + Network: image.Network{ + APIVIP4: "192.168.1.1", + APIVIP6: "fd12:3456:789a::21", + }, + }, + ServerConfig: map[string]any{ + "cluster-cidr": "10.42.0.0/16,fd12:3456:789a::/480", + "service-cidr": "10.43.0.0/16,fd12:3456:789a::/1122", + }, + ExpectedFailedMessages: []string{ + "Kubernetes server config cluster-cidr value 'fd12:3456:789a::/480' could not be parsed", + "Kubernetes server config service-cidr value 'fd12:3456:789a::/1122' could not be parsed", + }, + }, + `invalid IPv4 prefix`: { + K8s: image.Kubernetes{ + Network: image.Network{ + APIVIP4: "192.168.1.1", + APIVIP6: "fd12:3456:789a::21", + }, + }, + ServerConfig: map[string]any{ + "cluster-cidr": "10.42.0.0/50,fd12:3456:789a::/48", + "service-cidr": "10.43.0.0/50,fd12:3456:789a::/112", + }, + ExpectedFailedMessages: []string{ + "Kubernetes server config cluster-cidr value '10.42.0.0/50' could not be parsed", + "Kubernetes server config service-cidr value '10.43.0.0/50' could not be parsed", + }, + }, + `invalid IPv4 non-unicast`: { + K8s: image.Kubernetes{ + Network: image.Network{ + APIVIP4: "192.168.1.1", + APIVIP6: "fd12:3456:789a::21", + }, + }, + ServerConfig: map[string]any{ + "cluster-cidr": "127.0.0.1/16,fd12:3456:789a::/48", + "service-cidr": "127.0.0.1/16,fd12:3456:789a::/112", + }, + ExpectedFailedMessages: []string{ + "Kubernetes server config cluster-cidr value '127.0.0.1/16' must be a valid unicast address", + "Kubernetes server config service-cidr value '127.0.0.1/16' must be a valid unicast address", + }, + }, + `invalid IPv6 non-unicast`: { + K8s: image.Kubernetes{ + Network: image.Network{ + APIVIP4: "192.168.1.1", + APIVIP6: "fd12:3456:789a::21", + }, + }, + ServerConfig: map[string]any{ + "cluster-cidr": "10.42.0.0/16,FF01::/48", + "service-cidr": "10.43.0.0/16,FF01::/112", + }, + ExpectedFailedMessages: []string{ + "Kubernetes server config cluster-cidr value 'FF01::/48' must be a valid unicast address", + "Kubernetes server config service-cidr value 'FF01::/112' must be a valid unicast address", + }, + }, + `invalid node ip`: { + K8s: image.Kubernetes{ + Network: image.Network{ + APIVIP4: "192.168.1.1", + APIVIP6: "fd12:3456:789a::21", + }, + }, + ServerConfig: map[string]any{ + "cluster-cidr": "10.42.0.0/16,FF01::/48", + "service-cidr": "10.43.0.0/16,FF01::/112", + }, + ExpectedFailedMessages: []string{ + "Kubernetes server config cluster-cidr value 'FF01::/48' must be a valid unicast address", + "Kubernetes server config service-cidr value 'FF01::/112' must be a valid unicast address", + }, + }, + `mismatched prio`: { + K8s: image.Kubernetes{}, + ServerConfig: map[string]any{ + "cluster-cidr": "10.42.0.0/16,fd12:3456:789b::/48", + "service-cidr": "fd12:3456:789c::/112,10.43.0.0/16", + }, + ExpectedFailedMessages: []string{ + "Kubernetes server config cluster-cidr cannot prioritize one address family while service-cidr prioritizes another; both must have the same priority", + }, + }, + `single cidr IPv6`: { + K8s: image.Kubernetes{}, + ServerConfig: map[string]any{ + "cluster-cidr": "fd12:3456:789b::/48", + "service-cidr": "fd12:3456:789c::/112", + }, + }, + `single cidr IPv4`: { + K8s: image.Kubernetes{ + Network: image.Network{ + APIVIP4: "192.168.1.1", + }, + }, + ServerConfig: map[string]any{ + "cluster-cidr": "10.42.0.0/16", + "service-cidr": "10.43.0.0/16", + }, + }, } - b, err := yaml.Marshal(serverConfig) - require.NoError(t, err) - - configFile := filepath.Join(serverConfigDir, "server.yaml") - require.NoError(t, os.WriteFile(configFile, []byte(b), 0o600)) + for name, test := range tests { + t.Run(name, func(t *testing.T) { + k := test.K8s + sc := test.ServerConfig + failures := validateCIDRConfig(&k, sc) + assert.Len(t, failures, len(test.ExpectedFailedMessages)) - failures := validateNetworkingConfig(&k8s, configFile) + var foundMessages []string + for _, foundValidation := range failures { + foundMessages = append(foundMessages, foundValidation.UserMessage) + } - assert.Len(t, failures, 2) + for _, expectedMessage := range test.ExpectedFailedMessages { + assert.Contains(t, foundMessages, expectedMessage) + } - var foundMessages []string - for _, foundValidation := range failures { - foundMessages = append(foundMessages, foundValidation.UserMessage) + }) } - - assert.Contains(t, foundMessages, "Kubernetes server config cluster-cidr cannot contain addresses of the same IP address family; one must be IPv4, and the other IPv6") - assert.Contains(t, foundMessages, "Kubernetes server config service-cidr cannot contain addresses of the same IP address family; one must be IPv4, and the other IPv6") } -func TestValidateConfigInvalidServerConfigNotConfigured(t *testing.T) { - k8s := image.Kubernetes{Network: image.Network{ - APIVIP4: "192.168.1.1", - APIVIP6: "fd12:3456:789a::21", - }} - - failures := validateNetworkingConfig(&k8s, "fake-path") - - assert.Len(t, failures, 1) - - var foundMessages []string - for _, foundValidation := range failures { - foundMessages = append(foundMessages, foundValidation.UserMessage) - } - - assert.Contains(t, foundMessages, "Kubernetes server config could not be found at 'fake-path'; dual-stack configuration requires a valid cluster-cidr and service-cidr.") -} - -func TestValidateConfigValidAPIVIPNotConfigured(t *testing.T) { - k8s := image.Kubernetes{} - - failures := validateNetworkingConfig(&k8s, "") - assert.Len(t, failures, 0) -} - -func TestValidateConfigInvalidClusterCIDRNotConfigured(t *testing.T) { - k8s := image.Kubernetes{Network: image.Network{ - APIVIP4: "192.168.1.1", - APIVIP6: "fd12:3456:789a::21", - }} - - serverConfig := map[string]any{ - "service-cidr": "10.43.0.0/16,fd12:3456:789c::/112", - } - - failures := validateCIDRConfig(&k8s, serverConfig) - - assert.Len(t, failures, 1) - - var foundMessages []string - for _, foundValidation := range failures { - foundMessages = append(foundMessages, foundValidation.UserMessage) - } - - assert.Contains(t, foundMessages, "Kubernetes server config must contain a valid cluster-cidr when configuring dual-stack") -} - -func TestValidateConfigInvalidServiceCIDRNotConfigured(t *testing.T) { - k8s := image.Kubernetes{Network: image.Network{ - APIVIP4: "192.168.1.1", - APIVIP6: "fd12:3456:789a::21", - }} - - serverConfig := map[string]any{ - "cluster-cidr": "fd12:3456:789b::/48,10.42.0.0/16", - } - - failures := validateCIDRConfig(&k8s, serverConfig) - - assert.Len(t, failures, 1) - - var foundMessages []string - for _, foundValidation := range failures { - foundMessages = append(foundMessages, foundValidation.UserMessage) - } - - assert.Contains(t, foundMessages, "Kubernetes server config must contain a valid service-cidr when configuring dual-stack") -} - -func TestValidateConfigInvalidIPv4(t *testing.T) { - k8s := image.Kubernetes{Network: image.Network{ - APIVIP4: "192.168.1.1", - APIVIP6: "fd12:3456:789a::21", - }} - serverConfig := map[string]any{ - "cluster-cidr": "500.42.0.0/16,fd12:3456:789b::/48", - "service-cidr": "500.43.0.0/16,fd12:3456:789c::/112", - } - - failures := validateCIDRConfig(&k8s, serverConfig) - - assert.Len(t, failures, 2) - - var foundMessages []string - for _, foundValidation := range failures { - foundMessages = append(foundMessages, foundValidation.UserMessage) - } - - assert.Contains(t, foundMessages, "Kubernetes server config cluster-cidr value '500.42.0.0/16' could not be parsed") - assert.Contains(t, foundMessages, "Kubernetes server config service-cidr value '500.43.0.0/16' could not be parsed") -} - -func TestValidateConfigInvalidIPv6(t *testing.T) { - k8s := image.Kubernetes{Network: image.Network{ - APIVIP4: "192.168.1.1", - APIVIP6: "fd12:3456:789a::21", - }} - - serverConfig := map[string]any{ - "cluster-cidr": "10.42.0.0/16,xxxx:3456:789b::/48", - "service-cidr": "10.43.0.0/16,xxxx:3456:789c::/112", - } - - failures := validateCIDRConfig(&k8s, serverConfig) - - assert.Len(t, failures, 2) - - var foundMessages []string - for _, foundValidation := range failures { - foundMessages = append(foundMessages, foundValidation.UserMessage) - } - - assert.Contains(t, foundMessages, "Kubernetes server config cluster-cidr value 'xxxx:3456:789b::/48' could not be parsed") - assert.Contains(t, foundMessages, "Kubernetes server config service-cidr value 'xxxx:3456:789c::/112' could not be parsed") -} - -func TestValidateConfigInvalidIPv6Prefix(t *testing.T) { - k8s := image.Kubernetes{Network: image.Network{ - APIVIP4: "192.168.1.1", - APIVIP6: "fd12:3456:789a::21", - }} - - serverConfig := map[string]any{ - "cluster-cidr": "10.42.0.0/16,fd12:3456:789a::/480", - "service-cidr": "10.43.0.0/16,fd12:3456:789a::/1122", - } - - failures := validateCIDRConfig(&k8s, serverConfig) - - assert.Len(t, failures, 2) - - var foundMessages []string - for _, foundValidation := range failures { - foundMessages = append(foundMessages, foundValidation.UserMessage) - } - - assert.Contains(t, foundMessages, "Kubernetes server config cluster-cidr value 'fd12:3456:789a::/480' could not be parsed") - assert.Contains(t, foundMessages, "Kubernetes server config service-cidr value 'fd12:3456:789a::/1122' could not be parsed") -} - -func TestValidateConfigInvalidIPv4Prefix(t *testing.T) { - k8s := image.Kubernetes{Network: image.Network{ - APIVIP4: "192.168.1.1", - APIVIP6: "fd12:3456:789a::21", - }} - - serverConfig := map[string]any{ - "cluster-cidr": "10.42.0.0/50,fd12:3456:789a::/48", - "service-cidr": "10.43.0.0/50,fd12:3456:789a::/112", - } - - failures := validateCIDRConfig(&k8s, serverConfig) - - assert.Len(t, failures, 2) - - var foundMessages []string - for _, foundValidation := range failures { - foundMessages = append(foundMessages, foundValidation.UserMessage) - } - - assert.Contains(t, foundMessages, "Kubernetes server config cluster-cidr value '10.42.0.0/50' could not be parsed") - assert.Contains(t, foundMessages, "Kubernetes server config service-cidr value '10.43.0.0/50' could not be parsed") -} - -func TestValidateConfigInvalidIPv4NonUnicast(t *testing.T) { - k8s := image.Kubernetes{Network: image.Network{ - APIVIP4: "192.168.1.1", - APIVIP6: "fd12:3456:789a::21", - }} - - serverConfig := map[string]any{ - "cluster-cidr": "127.0.0.1/16,fd12:3456:789a::/48", - "service-cidr": "127.0.0.1/16,fd12:3456:789a::/112", - } - - failures := validateCIDRConfig(&k8s, serverConfig) - - assert.Len(t, failures, 2) - - var foundMessages []string - for _, foundValidation := range failures { - 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") - 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) { - k8s := image.Kubernetes{Network: image.Network{ - APIVIP4: "192.168.1.1", - APIVIP6: "fd12:3456:789a::21", - }} - - serverConfig := map[string]any{ - "cluster-cidr": "10.42.0.0/16,FF01::/48", - "service-cidr": "10.43.0.0/16,FF01::/112", - } - - failures := validateCIDRConfig(&k8s, serverConfig) - - assert.Len(t, failures, 2) - - var foundMessages []string - for _, foundValidation := range failures { - foundMessages = append(foundMessages, foundValidation.UserMessage) - } - - 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) { - k8s := image.Kubernetes{Network: image.Network{ - APIVIP4: "192.168.1.1", - APIVIP6: "fd12:3456:789a::21", - }} - - serverConfig := map[string]any{ - "cluster-cidr": "10.42.0.0/16,FF01::/48", - "service-cidr": "10.43.0.0/16,FF01::/112", - } - - failures := validateCIDRConfig(&k8s, serverConfig) - - assert.Len(t, failures, 2) - - var foundMessages []string - for _, foundValidation := range failures { - foundMessages = append(foundMessages, foundValidation.UserMessage) - } - - 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) { - k8s := image.Kubernetes{} - - serverConfig := map[string]any{ - "cluster-cidr": "10.42.0.0/16,fd12:3456:789b::/48", - "service-cidr": "fd12:3456:789c::/112,10.43.0.0/16", - } - - failures := validateCIDRConfig(&k8s, serverConfig) - - assert.Len(t, failures, 1) - - var foundMessages []string - for _, foundValidation := range failures { - foundMessages = append(foundMessages, foundValidation.UserMessage) - } - - assert.Contains(t, foundMessages, "Kubernetes server config cluster-cidr cannot prioritize one address family while service-cidr prioritizes another; both must have the same priority") -} - -func TestValidateConfigSingleCIDRIPv6(t *testing.T) { - k8s := image.Kubernetes{} - - serverConfig := map[string]any{ - "cluster-cidr": "fd12:3456:789b::/48", - "service-cidr": "fd12:3456:789c::/112", - } - - failures := validateCIDRConfig(&k8s, serverConfig) - - assert.Len(t, failures, 0) -} - -func TestValidateConfigSingleCIDRIPv4(t *testing.T) { - k8s := image.Kubernetes{Network: image.Network{ - APIVIP4: "192.168.1.1", - }} - - serverConfig := map[string]any{ - "cluster-cidr": "10.42.0.0/16", - "service-cidr": "10.43.0.0/16", - } - - failures := validateCIDRConfig(&k8s, serverConfig) - - assert.Len(t, failures, 0) -} - -func TestValidateNodeIPValidSingleIPv4(t *testing.T) { - k8s := image.Kubernetes{} - - serverConfig := map[string]any{ - "node-ip": "10.42.0.0", - } - - failures := validateNodeIP(&k8s, serverConfig) - - assert.Len(t, failures, 0) -} - -func TestValidateNodeIPValidSingleIPv6(t *testing.T) { - k8s := image.Kubernetes{Network: image.Network{}} - - serverConfig := map[string]any{ - "node-ip": "fd12:3456:789a::21", - } - - failures := validateNodeIP(&k8s, serverConfig) - - assert.Len(t, failures, 0) -} - -func TestValidateNodeIPMultipleNodesValid(t *testing.T) { - k8s := image.Kubernetes{Nodes: []image.Node{ - { - Hostname: "agent1", - Type: image.KubernetesNodeTypeAgent, +func TestValidateNodeIP(t *testing.T) { + tests := map[string]struct { + K8s image.Kubernetes + ServerConfig map[string]any + ExpectedFailedMessages []string + }{ + `single IPv4`: { + K8s: image.Kubernetes{}, + ServerConfig: map[string]any{ + "node-ip": "10.42.0.0", + }, }, - { - Hostname: "agent2", - Type: image.KubernetesNodeTypeAgent, + `single IPv6`: { + K8s: image.Kubernetes{}, + ServerConfig: map[string]any{ + "node-ip": "fd12:3456:789a::21", + }, }, - { - Hostname: "server", - Type: image.KubernetesNodeTypeServer, - Initialiser: true, + `multiple nodes valid`: { + K8s: image.Kubernetes{ + Nodes: []image.Node{ + { + Hostname: "server1", + Type: image.KubernetesNodeTypeServer, + }, + { + Hostname: "agent", + Type: image.KubernetesNodeTypeAgent, + Initialiser: true, + }, + { + Hostname: "agent2", + Type: image.KubernetesNodeTypeAgent, + Initialiser: true, + }, + }, + }, + ServerConfig: map[string]any{ + "node-ip": "10.42.0.0", + }, }, - }} - - serverConfig := map[string]any{ - "node-ip": "10.42.0.0", - } - - failures := validateNodeIP(&k8s, serverConfig) - - assert.Len(t, failures, 0) -} - -func TestValidateNodeIPMultipleServersInvalid(t *testing.T) { - k8s := image.Kubernetes{Nodes: []image.Node{ - { - Hostname: "server1", - Type: image.KubernetesNodeTypeServer, + `multiple servers invalid`: { + K8s: image.Kubernetes{ + Nodes: []image.Node{ + { + Hostname: "server1", + Type: image.KubernetesNodeTypeServer, + }, + { + Hostname: "server2", + Type: image.KubernetesNodeTypeServer, + Initialiser: true, + }, + }, + }, + ServerConfig: map[string]any{ + "node-ip": "10.42.0.0", + }, + ExpectedFailedMessages: []string{ + "Kubernetes server config node-ip can not be specified when there is more than one Kubernetes server node", + }, }, - { - Hostname: "server2", - Type: image.KubernetesNodeTypeServer, - Initialiser: true, + `node ip family both same invalid`: { + K8s: image.Kubernetes{}, + ServerConfig: map[string]any{ + "node-ip": "10.42.0.0,10.43.0.0", + }, + ExpectedFailedMessages: []string{ + "Kubernetes server config node-ip cannot contain addresses of the same IP address family; one must be IPv4, and the other IPv6", + }, + }, + `node ip dualstack valid`: { + K8s: image.Kubernetes{}, + ServerConfig: map[string]any{ + "node-ip": "10.42.0.0,fd12:3456:789a::21", + }, + }, + `node ip non-unicast IPv4 invalid`: { + K8s: image.Kubernetes{}, + ServerConfig: map[string]any{ + "node-ip": "127.0.0.1,fd12:3456:789a::21", + }, + ExpectedFailedMessages: []string{ + "Kubernetes server config node-ip value '127.0.0.1' must be a valid unicast address", + }, + }, + `node ip IPv6 invalid`: { + K8s: image.Kubernetes{}, + ServerConfig: map[string]any{ + "node-ip": "xxxx:3456:789a::21", + }, + ExpectedFailedMessages: []string{ + "Kubernetes server config node-ip value 'xxxx:3456:789a::21' could not be parsed", + }, }, - }} - - serverConfig := map[string]any{ - "node-ip": "10.42.0.0", - } - - failures := validateNodeIP(&k8s, serverConfig) - - assert.Len(t, failures, 1) - - var foundMessages []string - for _, foundValidation := range failures { - foundMessages = append(foundMessages, foundValidation.UserMessage) - } - - assert.Contains(t, foundMessages, "Kubernetes server config node-ip can not be specified when there is more than one Kubernetes server node") -} - -func TestValidateNodeIPBothSameFamilyInvalid(t *testing.T) { - k8s := image.Kubernetes{} - - serverConfig := map[string]any{ - "node-ip": "10.42.0.0,10.43.0.0", - } - - failures := validateNodeIP(&k8s, serverConfig) - - assert.Len(t, failures, 1) - - var foundMessages []string - for _, foundValidation := range failures { - foundMessages = append(foundMessages, foundValidation.UserMessage) - } - - assert.Contains(t, foundMessages, "Kubernetes server config node-ip cannot contain addresses of the same IP address family; one must be IPv4, and the other IPv6") -} - -func TestValidateNodeIPDualstackValid(t *testing.T) { - k8s := image.Kubernetes{} - - serverConfig := map[string]any{ - "node-ip": "10.42.0.0,fd12:3456:789a::21", - } - - failures := validateNodeIP(&k8s, serverConfig) - - assert.Len(t, failures, 0) -} - -func TestValidateNodeIPNonunicastIPv4Invalid(t *testing.T) { - k8s := image.Kubernetes{} - - serverConfig := map[string]any{ - "node-ip": "127.0.0.1,fd12:3456:789a::21", - } - - failures := validateNodeIP(&k8s, serverConfig) - - assert.Len(t, failures, 1) - - var foundMessages []string - for _, foundValidation := range failures { - foundMessages = append(foundMessages, foundValidation.UserMessage) } - 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) { - k8s := image.Kubernetes{} - - serverConfig := map[string]any{ - "node-ip": "xxxx:3456:789a::21", - } + for name, test := range tests { + t.Run(name, func(t *testing.T) { + k := test.K8s + sc := test.ServerConfig + failures := validateNodeIP(&k, sc) + assert.Len(t, failures, len(test.ExpectedFailedMessages)) - failures := validateNodeIP(&k8s, serverConfig) + var foundMessages []string + for _, foundValidation := range failures { + foundMessages = append(foundMessages, foundValidation.UserMessage) + } - assert.Len(t, failures, 1) + for _, expectedMessage := range test.ExpectedFailedMessages { + assert.Contains(t, foundMessages, expectedMessage) + } - var foundMessages []string - for _, foundValidation := range failures { - foundMessages = append(foundMessages, foundValidation.UserMessage) + }) } - - assert.Contains(t, foundMessages, "Kubernetes server config node-ip value 'xxxx:3456:789a::21' could not be parsed") } diff --git a/pkg/image/validation/version.go b/pkg/image/validation/version.go index 58265687..ea2a39f6 100644 --- a/pkg/image/validation/version.go +++ b/pkg/image/validation/version.go @@ -33,5 +33,11 @@ func validateVersion(ctx *image.Context) []FailedValidation { }) } + if definition.APIVersion != "1.2" && definition.Kubernetes.Network.APIVIP6 != "" { + failures = append(failures, FailedValidation{ + UserMessage: "IPv6 support for the Kubernetes API VIP is only available in EIB version >= 1.2", + }) + } + return failures } diff --git a/pkg/kubernetes/cluster.go b/pkg/kubernetes/cluster.go index f67befc1..41392e9a 100644 --- a/pkg/kubernetes/cluster.go +++ b/pkg/kubernetes/cluster.go @@ -60,22 +60,14 @@ func NewCluster(kubernetes *image.Kubernetes, configPath string) (*Cluster, erro return nil, fmt.Errorf("failed to determine cluster initialiser") } - var ip4 *netip.Addr + var ip4 netip.Addr if kubernetes.Network.APIVIP4 != "" { - ip4Holder, ipErr := netip.ParseAddr(kubernetes.Network.APIVIP4) - if ipErr != nil { - return nil, fmt.Errorf("parsing kubernetes ipv4 address: %w", ipErr) - } - ip4 = &ip4Holder + ip4, err = netip.ParseAddr(kubernetes.Network.APIVIP4) } - var ip6 *netip.Addr + var ip6 netip.Addr if kubernetes.Network.APIVIP6 != "" { - ip6Holder, ipErr := netip.ParseAddr(kubernetes.Network.APIVIP6) - if ipErr != nil { - return nil, fmt.Errorf("parsing kubernetes ipv6 address: %w", ipErr) - } - ip6 = &ip6Holder + ip6, err = netip.ParseAddr(kubernetes.Network.APIVIP6) } prioritizeIPv6 := IsIPv6Priority(serverConfig) @@ -179,7 +171,7 @@ func setSingleNodeConfigDefaults(kubernetes *image.Kubernetes, config map[string delete(config, serverKey) } -func setMultiNodeConfigDefaults(kubernetes *image.Kubernetes, config map[string]any, ip4 *netip.Addr, ip6 *netip.Addr, prioritizeIPv6 bool) { +func setMultiNodeConfigDefaults(kubernetes *image.Kubernetes, config map[string]any, ip4 netip.Addr, ip6 netip.Addr, prioritizeIPv6 bool) { const ( k3sServerPort = 6443 rke2ServerPort = 9345 @@ -232,19 +224,16 @@ func setClusterCNI(config map[string]any) { config[cniKey] = cniDefaultValue } -func setClusterAPIAddress(config map[string]any, ip4 *netip.Addr, ip6 *netip.Addr, port uint16, prioritizeIPv6 bool) { - if ip4 == nil && ip6 == nil { - panic("Attempted to set an empty cluster API address") +func setClusterAPIAddress(config map[string]any, ip4 netip.Addr, ip6 netip.Addr, port uint16, prioritizeIPv6 bool) { + if !ip4.IsValid() && !ip6.IsValid() { + panic("Attempted to set an empty invalid API address") } - switch { - case ip6 != nil && prioritizeIPv6: - config[serverKey] = fmt.Sprintf("https://%s", netip.AddrPortFrom(*ip6, port).String()) - case ip6 != nil && ip4 == nil: - config[serverKey] = fmt.Sprintf("https://%s", netip.AddrPortFrom(*ip6, port).String()) - default: - config[serverKey] = fmt.Sprintf("https://%s", netip.AddrPortFrom(*ip4, port).String()) + if ip6.IsValid() && (prioritizeIPv6 || !ip4.IsValid()) { + config[serverKey] = fmt.Sprintf("https://%s", netip.AddrPortFrom(ip6, port).String()) + return } + config[serverKey] = fmt.Sprintf("https://%s", netip.AddrPortFrom(ip4, port).String()) } func setSELinux(config map[string]any) { diff --git a/pkg/kubernetes/cluster_test.go b/pkg/kubernetes/cluster_test.go index d72291f0..764a76eb 100644 --- a/pkg/kubernetes/cluster_test.go +++ b/pkg/kubernetes/cluster_test.go @@ -480,19 +480,21 @@ func TestSetClusterAPIAddress(t *testing.T) { ip6, err := netip.ParseAddr("fd12:3456:789a::21") assert.NoError(t, err) - setClusterAPIAddress(config, &ip4, nil, 9345, false) + var emptyIP netip.Addr + + setClusterAPIAddress(config, ip4, emptyIP, 9345, false) assert.Equal(t, "https://192.168.122.50:9345", config["server"]) - setClusterAPIAddress(config, &ip4, &ip6, 9345, false) + setClusterAPIAddress(config, ip4, ip6, 9345, false) assert.Equal(t, "https://192.168.122.50:9345", config["server"]) - setClusterAPIAddress(config, &ip4, &ip6, 9345, true) + setClusterAPIAddress(config, ip4, ip6, 9345, true) assert.Equal(t, "https://[fd12:3456:789a::21]:9345", config["server"]) - setClusterAPIAddress(config, nil, &ip6, 9345, true) + setClusterAPIAddress(config, emptyIP, ip6, 9345, true) assert.Equal(t, "https://[fd12:3456:789a::21]:9345", config["server"]) - setClusterAPIAddress(config, &ip4, &ip6, 9345, true) + setClusterAPIAddress(config, ip4, ip6, 9345, true) assert.Equal(t, "https://[fd12:3456:789a::21]:9345", config["server"]) } diff --git a/pkg/version/version.go b/pkg/version/version.go index d360384e..30b3b020 100644 --- a/pkg/version/version.go +++ b/pkg/version/version.go @@ -9,9 +9,10 @@ import ( const ( version10 = "1.0" version11 = "1.1" + version12 = "1.2" ) -var SupportedSchemaVersions = []string{version10, version11} +var SupportedSchemaVersions = []string{version10, version11, version12} var version string