From 147a3a428e27bc9cb6853a6337bc7c43ffc60799 Mon Sep 17 00:00:00 2001 From: Etienne Audet-Cobello Date: Fri, 27 Sep 2024 18:50:07 -0400 Subject: [PATCH] move impl. to cluster_config_validate --- src/k8s/pkg/k8sd/app/hooks_bootstrap.go | 68 ------------------- src/k8s/pkg/k8sd/app/hooks_bootstrap_test.go | 57 ---------------- .../pkg/k8sd/types/cluster_config_validate.go | 66 ++++++++++++++++++ 3 files changed, 66 insertions(+), 125 deletions(-) diff --git a/src/k8s/pkg/k8sd/app/hooks_bootstrap.go b/src/k8s/pkg/k8sd/app/hooks_bootstrap.go index 79b9063d2..ccd828d1d 100644 --- 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 index ef398e9f1..4879f7a48 100644 --- 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 index c673e3173..5a0997d86 100644 --- 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() {