move impl. to cluster_config_validate

canonical · Sep 27, 2024 · 147a3a4 · 147a3a4
1 parent 27d2359
commit 147a3a4
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 125 deletions.
diff --git a/src/k8s/pkg/k8sd/app/hooks_bootstrap.go b/src/k8s/pkg/k8sd/app/hooks_bootstrap.go
@@ -302,18 +302,6 @@ func (a *App) onBootstrapControlPlane(ctx context.Context, s state.State, bootst
 		return fmt.Errorf("failed to get IP address(es) from ServiceCIDR %q: %w", cfg.Network.GetServiceCIDR(), err)
 	}
 
-	if cfg.Network.GetPodCIDR() != "" && cfg.Network.GetServiceCIDR() != "" {
-		if err = validateCIDROverlap(cfg.Network.GetPodCIDR(), cfg.Network.GetServiceCIDR()); err != nil {
-			return fmt.Errorf("failed to validate the CIDR configuration: %w", err)
-		}
-	}
-	// Can't be an else-if, because default values are already set.
-	if cfg.Network.GetServiceCIDR() != "" {
-		if err = validateIPv6CIDRSize(cfg.Network.GetServiceCIDR()); err != nil {
-			return fmt.Errorf("failed to validate the CIDR configuration: %w", err)
-		}
-	}
-
 	// NOTE: Set the notBefore certificate time to the current time.
 	notBefore := time.Now()
 
@@ -506,59 +494,3 @@ func (a *App) onBootstrapControlPlane(ctx context.Context, s state.State, bootst
 	a.NotifyUpdateNodeConfigController()
 	return nil
 }
-
-// validateCIDROverlapAndSize checks for overlap and size constraints between pod and service CIDRs.
-// It parses the provided podCIDR and serviceCIDR strings, checks for IPv4 and IPv6 overlaps.
-func validateCIDROverlap(podCIDR string, serviceCIDR string) error {
-	// Parse the CIDRs
-	podIPv4CIDR, podIPv6CIDR, err := utils.SplitCIDRStrings(podCIDR)
-	if err != nil {
-		return fmt.Errorf("failed to parse pod CIDR: %w", err)
-	}
-
-	svcIPv4CIDR, svcIPv6CIDR, err := utils.SplitCIDRStrings(serviceCIDR)
-	if err != nil {
-		return fmt.Errorf("failed to parse service CIDR: %w", err)
-	}
-
-	// Check for IPv4 overlap
-	if podIPv4CIDR != "" && svcIPv4CIDR != "" {
-		if overlap, err := utils.CIDRsOverlap(podIPv4CIDR, svcIPv4CIDR); err != nil {
-			return fmt.Errorf("failed to check for IPv4 overlap: %w", err)
-		} else if overlap {
-			return fmt.Errorf("pod CIDR %q and service CIDR %q overlap", podCIDR, serviceCIDR)
-		}
-	}
-
-	// Check for IPv6 overlap
-	if podIPv6CIDR != "" && svcIPv6CIDR != "" {
-		if overlap, err := utils.CIDRsOverlap(podIPv6CIDR, svcIPv6CIDR); err != nil {
-			return fmt.Errorf("failed to check for IPv6 overlap: %w", err)
-		} else if overlap {
-			return fmt.Errorf("pod CIDR %q and service CIDR %q overlap", podCIDR, serviceCIDR)
-		}
-	}
-
-	return nil
-}
-
-// Check CIDR size ensures that the service IPv6 CIDR is not larger than /108.
-// Ref: https://documentation.ubuntu.com/canonical-kubernetes/latest/snap/howto/networking/dualstack/#cidr-size-limitations
-func validateIPv6CIDRSize(serviceCIDR string) error {
-	_, svcIPv6CIDR, err := utils.SplitCIDRStrings(serviceCIDR)
-	if err != nil {
-		return fmt.Errorf("invalid CIDR: %w", err)
-	}
-
-	_, ipv6Net, err := net.ParseCIDR(svcIPv6CIDR)
-	if err != nil {
-		return fmt.Errorf("invalid CIDR: %w", err)
-	}
-
-	prefixLength, _ := ipv6Net.Mask.Size()
-	if prefixLength < 108 {
-		return fmt.Errorf("service CIDR %q cannot be larger than /108", serviceCIDR)
-	}
-
-	return nil
-}
diff --git a/src/k8s/pkg/k8sd/app/hooks_bootstrap_test.go b/src/k8s/pkg/k8sd/app/hooks_bootstrap_test.go
@@ -1,58 +1 @@
 package app
-
-import "testing"
-
-func TestValidateCIDROverlap(t *testing.T) {
-	tests := []struct {
-		name        string
-		podCIDR     string
-		serviceCIDR string
-		expectErr   bool
-	}{
-		{"Empty", "", "", true},
-		{"EmptyServiceCIDR", "192.168.1.0/24", "", true},
-		{"EmptyPodCIDR", "", "192.168.1.0/24", true},
-		//IPv4
-		{"SameIPv4CIDRs", "192.168.100.0/24", "192.168.100.0/24", true},
-		{"OverlappingIPv4CIDRs", "10.2.0.13/24", "10.2.0.0/24", true},
-		{"IPv4CIDRMinimumSize", "192.168.1.1/32", "10.0.0.0/24", false},
-		{"InvalidIPv4CIDRFormat", "192.168.1.1/33", "10.0.0.0/24", true},
-		{"MaxSizeIPv4CIDRs", "0.0.0.0/0", "0.0.0.0/0", true},
-		//IPv6
-		{"SameIPv6CIDRs", "fe80::1/32", "fe80::1/32", true},
-		{"OverlappingIPv6CIDRs", "fe80::/48", "fe80::dead/48", true},
-		{"IPv6CIDRMinimumSize", "2001:db8::1/128", "fc00::/32", false},
-		{"InvalidIPv6CIDRFormat", "2001:db8::1/129", "fc00::/64", true},
-		{"MaxSizeIPv6CIDRs", "::/0", "::/0", true},
-		//Mixed
-		{"IPv4AndIPv6MixedCIDRs", "192.168.0.0/16", "2001:db8::/32", false},
-		{"OnlyInvalidIPv6CIDR", "", "2001:db8::/65", true},
-	}
-	for _, tc := range tests {
-		t.Run(tc.name, func(t *testing.T) {
-			if err := validateCIDROverlap(tc.podCIDR, tc.serviceCIDR); (err != nil) != tc.expectErr {
-				t.Errorf("validateCIDROverlap() error = %v, expectErr %v", err, tc.expectErr)
-			}
-		})
-	}
-}
-
-func TestValidateCIDRSize(t *testing.T) {
-	tests := []struct {
-		name      string
-		cidr      string
-		expectErr bool
-	}{
-		{"Empty", "", true},
-		{"DualstackCIDRValid", "192.168.2.0/24,fe80::/128", false},
-		{"DualstackCIDRPrefixAtLimit", "192.168.2.0/24,fe80::/108", true},
-		{"IPv6CIDRPrefixBiggerThanLimit", "fe80::/64", true},
-	}
-	for _, tc := range tests {
-		t.Run(tc.name, func(t *testing.T) {
-			if err := validateIPv6CIDRSize(tc.cidr); (err != nil) != tc.expectErr {
-				t.Errorf("validateCIDRSize() error = %v, expectErr %v", err, tc.expectErr)
-			}
-		})
-	}
-}
diff --git a/src/k8s/pkg/k8sd/types/cluster_config_validate.go b/src/k8s/pkg/k8sd/types/cluster_config_validate.go
@@ -6,6 +6,8 @@ import (
 	"net/netip"
 	"net/url"
 	"strings"
+
+	"github.com/canonical/k8s/pkg/utils"
 )
 
 func validateCIDRs(cidrString string) error {
@@ -21,6 +23,62 @@ func validateCIDRs(cidrString string) error {
 	return nil
 }
 
+// validateCIDROverlapAndSize checks for overlap and size constraints between pod and service CIDRs.
+// It parses the provided podCIDR and serviceCIDR strings, checks for IPv4 and IPv6 overlaps.
+func validateCIDROverlap(podCIDR string, serviceCIDR string) error {
+	// Parse the CIDRs
+	podIPv4CIDR, podIPv6CIDR, err := utils.SplitCIDRStrings(podCIDR)
+	if err != nil {
+		return fmt.Errorf("failed to parse pod CIDR: %w", err)
+	}
+
+	svcIPv4CIDR, svcIPv6CIDR, err := utils.SplitCIDRStrings(serviceCIDR)
+	if err != nil {
+		return fmt.Errorf("failed to parse service CIDR: %w", err)
+	}
+
+	// Check for IPv4 overlap
+	if podIPv4CIDR != "" && svcIPv4CIDR != "" {
+		if overlap, err := utils.CIDRsOverlap(podIPv4CIDR, svcIPv4CIDR); err != nil {
+			return fmt.Errorf("failed to check for IPv4 overlap: %w", err)
+		} else if overlap {
+			return fmt.Errorf("pod CIDR %q and service CIDR %q overlap", podCIDR, serviceCIDR)
+		}
+	}
+
+	// Check for IPv6 overlap
+	if podIPv6CIDR != "" && svcIPv6CIDR != "" {
+		if overlap, err := utils.CIDRsOverlap(podIPv6CIDR, svcIPv6CIDR); err != nil {
+			return fmt.Errorf("failed to check for IPv6 overlap: %w", err)
+		} else if overlap {
+			return fmt.Errorf("pod CIDR %q and service CIDR %q overlap", podCIDR, serviceCIDR)
+		}
+	}
+
+	return nil
+}
+
+// Check CIDR size ensures that the service IPv6 CIDR is not larger than /108.
+// Ref: https://documentation.ubuntu.com/canonical-kubernetes/latest/snap/howto/networking/dualstack/#cidr-size-limitations
+func validateIPv6CIDRSize(serviceCIDR string) error {
+	_, svcIPv6CIDR, err := utils.SplitCIDRStrings(serviceCIDR)
+	if err != nil {
+		return fmt.Errorf("invalid CIDR: %w", err)
+	}
+
+	_, ipv6Net, err := net.ParseCIDR(svcIPv6CIDR)
+	if err != nil {
+		return fmt.Errorf("invalid CIDR: %w", err)
+	}
+
+	prefixLength, _ := ipv6Net.Mask.Size()
+	if prefixLength < 108 {
+		return fmt.Errorf("service CIDR %q cannot be larger than /108", serviceCIDR)
+	}
+
+	return nil
+}
+
 // Validate that a ClusterConfig does not have conflicting or incompatible options.
 func (c *ClusterConfig) Validate() error {
 	// check: validate that PodCIDR and ServiceCIDR are configured
@@ -31,6 +89,14 @@ func (c *ClusterConfig) Validate() error {
 		return fmt.Errorf("invalid service CIDR: %w", err)
 	}
 
+	if err := validateCIDROverlap(c.Network.GetPodCIDR(), c.Network.GetServiceCIDR()); err != nil {
+		return fmt.Errorf("invalid cidr configuration: %w", err)
+	}
+	// Can't be an else-if, because default values could already be set.
+	if err := validateIPv6CIDRSize(c.Network.GetServiceCIDR()); err != nil {
+		return fmt.Errorf("invalid service CIDR: %w", err)
+	}
+
 	// check: ensure network is enabled if any of ingress, gateway, load-balancer are enabled
 	if !c.Network.GetEnabled() {
 		if c.Gateway.GetEnabled() {