diff --git a/src/k8s/pkg/k8sd/features/calico/network.go b/src/k8s/pkg/k8sd/features/calico/network.go index f89651df2..414ea2938 100644 --- a/src/k8s/pkg/k8sd/features/calico/network.go +++ b/src/k8s/pkg/k8sd/features/calico/network.go @@ -54,7 +54,7 @@ func ApplyNetwork(ctx context.Context, snap snap.Snap, apiserver types.APIServer } podIpPools := []map[string]any{} - ipv4PodCIDR, ipv6PodCIDR, err := utils.ParseCIDRs(network.GetPodCIDR()) + ipv4PodCIDR, ipv6PodCIDR, err := utils.SplitCIDRStrings(network.GetPodCIDR()) if err != nil { err = fmt.Errorf("invalid pod cidr: %w", err) return types.FeatureStatus{ @@ -79,7 +79,7 @@ func ApplyNetwork(ctx context.Context, snap snap.Snap, apiserver types.APIServer } serviceCIDRs := []string{} - ipv4ServiceCIDR, ipv6ServiceCIDR, err := utils.ParseCIDRs(network.GetServiceCIDR()) + ipv4ServiceCIDR, ipv6ServiceCIDR, err := utils.SplitCIDRStrings(network.GetServiceCIDR()) if err != nil { err = fmt.Errorf("invalid service cidr: %v", err) return types.FeatureStatus{ diff --git a/src/k8s/pkg/k8sd/features/calico/network_test.go b/src/k8s/pkg/k8sd/features/calico/network_test.go index 75d31e0af..96d602b37 100644 --- a/src/k8s/pkg/k8sd/features/calico/network_test.go +++ b/src/k8s/pkg/k8sd/features/calico/network_test.go @@ -210,10 +210,10 @@ func TestEnabled(t *testing.T) { func validateValues(t *testing.T, values map[string]any, network types.Network) { g := NewWithT(t) - podIPv4CIDR, podIPv6CIDR, err := utils.ParseCIDRs(network.GetPodCIDR()) + podIPv4CIDR, podIPv6CIDR, err := utils.SplitCIDRStrings(network.GetPodCIDR()) g.Expect(err).ToNot(HaveOccurred()) - svcIPv4CIDR, svcIPv6CIDR, err := utils.ParseCIDRs(network.GetServiceCIDR()) + svcIPv4CIDR, svcIPv6CIDR, err := utils.SplitCIDRStrings(network.GetServiceCIDR()) g.Expect(err).ToNot(HaveOccurred()) // calico network diff --git a/src/k8s/pkg/k8sd/features/cilium/network.go b/src/k8s/pkg/k8sd/features/cilium/network.go index e0573d72b..ba9720dad 100644 --- a/src/k8s/pkg/k8sd/features/cilium/network.go +++ b/src/k8s/pkg/k8sd/features/cilium/network.go @@ -44,7 +44,7 @@ func ApplyNetwork(ctx context.Context, snap snap.Snap, apiserver types.APIServer }, nil } - ipv4CIDR, ipv6CIDR, err := utils.ParseCIDRs(network.GetPodCIDR()) + ipv4CIDR, ipv6CIDR, err := utils.SplitCIDRStrings(network.GetPodCIDR()) if err != nil { err = fmt.Errorf("invalid kube-proxy --cluster-cidr value: %v", err) return types.FeatureStatus{ diff --git a/src/k8s/pkg/k8sd/features/cilium/network_test.go b/src/k8s/pkg/k8sd/features/cilium/network_test.go index 978bebc0e..7b9c2e36b 100644 --- a/src/k8s/pkg/k8sd/features/cilium/network_test.go +++ b/src/k8s/pkg/k8sd/features/cilium/network_test.go @@ -180,7 +180,7 @@ func validateNetworkValues(t *testing.T, values map[string]any, network types.Ne t.Helper() g := NewWithT(t) - ipv4CIDR, ipv6CIDR, err := utils.ParseCIDRs(network.GetPodCIDR()) + ipv4CIDR, ipv6CIDR, err := utils.SplitCIDRStrings(network.GetPodCIDR()) g.Expect(err).ToNot(HaveOccurred()) bpfMount, err := utils.GetMountPath("bpf") diff --git a/src/k8s/pkg/k8sd/types/cluster_config_validate.go b/src/k8s/pkg/k8sd/types/cluster_config_validate.go index c673e3173..8ec1b8d50 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,65 @@ func validateCIDRs(cidrString string) error { return nil } +// validateCIDROverlap 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 +} + +// validateIPv6CIDRSize 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) + } + + if svcIPv6CIDR == "" { + return nil + } + + _, ipv6Net, err := net.ParseCIDR(svcIPv6CIDR) + if err != nil { + return fmt.Errorf("invalid CIDR: %w", err) + } + + if prefixLength, _ := ipv6Net.Mask.Size(); 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 +92,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() { diff --git a/src/k8s/pkg/k8sd/types/cluster_config_validate_test.go b/src/k8s/pkg/k8sd/types/cluster_config_validate_test.go index db58934ed..b2ef53af5 100644 --- a/src/k8s/pkg/k8sd/types/cluster_config_validate_test.go +++ b/src/k8s/pkg/k8sd/types/cluster_config_validate_test.go @@ -10,15 +10,18 @@ import ( func TestValidateCIDR(t *testing.T) { for _, tc := range []struct { - cidr string - expectErr bool + cidr string + expectPodErr bool + expectSvcErr bool }{ - {cidr: "10.1.0.0/16"}, - {cidr: "2001:0db8::/32"}, - {cidr: "10.1.0.0/16,2001:0db8::/32"}, - {cidr: "", expectErr: true}, - {cidr: "bananas", expectErr: true}, - {cidr: "fd01::/64,fd02::/64,fd03::/64", expectErr: true}, + {cidr: "192.168.0.0/16"}, + {cidr: "2001:0db8::/108"}, + {cidr: "10.2.0.0/16,2001:0db8::/108"}, + {cidr: "", expectPodErr: true, expectSvcErr: true}, + {cidr: "bananas", expectPodErr: true, expectSvcErr: true}, + {cidr: "fd01::/108,fd02::/108,fd03::/108", expectPodErr: true, expectSvcErr: true}, + {cidr: "10.1.0.0/32", expectPodErr: true, expectSvcErr: true}, + {cidr: "2001:0db8::/32", expectSvcErr: true}, } { t.Run(tc.cidr, func(t *testing.T) { t.Run("Pod", func(t *testing.T) { @@ -30,7 +33,7 @@ func TestValidateCIDR(t *testing.T) { }, } err := config.Validate() - if tc.expectErr { + if tc.expectPodErr { g.Expect(err).To(HaveOccurred()) } else { g.Expect(err).To(BeNil()) @@ -45,7 +48,7 @@ func TestValidateCIDR(t *testing.T) { }, } err := config.Validate() - if tc.expectErr { + if tc.expectSvcErr { g.Expect(err).To(HaveOccurred()) } else { g.Expect(err).To(BeNil()) diff --git a/src/k8s/pkg/utils/cidr.go b/src/k8s/pkg/utils/cidr.go index e5fe01bba..9b129b68f 100644 --- a/src/k8s/pkg/utils/cidr.go +++ b/src/k8s/pkg/utils/cidr.go @@ -101,8 +101,8 @@ func ParseAddressString(address string, port int64) (string, error) { return util.CanonicalNetworkAddress(address, port), nil } -// ParseCIDRs parses the given CIDR string and returns the respective IPv4 and IPv6 CIDRs. -func ParseCIDRs(CIDRstring string) (string, string, error) { +// SplitCIDRStrings parses the given CIDR string and returns the respective IPv4 and IPv6 CIDRs. +func SplitCIDRStrings(CIDRstring string) (string, string, error) { clusterCIDRs := strings.Split(CIDRstring, ",") if v := len(clusterCIDRs); v != 1 && v != 2 { return "", "", fmt.Errorf("invalid CIDR list: %v", clusterCIDRs) @@ -142,3 +142,25 @@ func ToIPString(ip net.IP) string { } return "[" + ip.String() + "]" } + +// CIDRsOverlap checks if two given CIDR blocks overlap. +// It takes two strings representing the CIDR blocks as input and returns a boolean indicating +// whether they overlap and an error if any of the CIDR blocks are invalid. +func CIDRsOverlap(cidr1, cidr2 string) (bool, error) { + _, ipNet1, err1 := net.ParseCIDR(cidr1) + _, ipNet2, err2 := net.ParseCIDR(cidr2) + + if err1 != nil { + return false, fmt.Errorf("couldn't parse CIDR block %q: %w", cidr1, err1) + } + + if err2 != nil { + return false, fmt.Errorf("couldn't parse CIDR block %q: %w", cidr2, err2) + } + + if ipNet1.Contains(ipNet2.IP) || ipNet2.Contains(ipNet1.IP) { + return true, nil + } + + return false, nil +} diff --git a/src/k8s/pkg/utils/cidr_test.go b/src/k8s/pkg/utils/cidr_test.go index 1d0e9da96..5bb1aef23 100644 --- a/src/k8s/pkg/utils/cidr_test.go +++ b/src/k8s/pkg/utils/cidr_test.go @@ -153,7 +153,7 @@ func TestParseCIDRs(t *testing.T) { for _, tc := range testCases { t.Run(tc.input, func(t *testing.T) { - ipv4CIDR, ipv6CIDR, err := utils.ParseCIDRs(tc.input) + ipv4CIDR, ipv6CIDR, err := utils.SplitCIDRStrings(tc.input) if tc.expectedErr { Expect(err).To(HaveOccurred()) } else {