Skip to content

Commit

Permalink
Validate pod CIDR and service CIDR (canonical#695)
Browse files Browse the repository at this point in the history
  • Loading branch information
eaudetcobello authored and evilnick committed Nov 14, 2024
1 parent 5c68695 commit 5755f01
Show file tree
Hide file tree
Showing 8 changed files with 113 additions and 19 deletions.
4 changes: 2 additions & 2 deletions src/k8s/pkg/k8sd/features/calico/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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{
Expand Down
4 changes: 2 additions & 2 deletions src/k8s/pkg/k8sd/features/calico/network_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/k8s/pkg/k8sd/features/cilium/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
2 changes: 1 addition & 1 deletion src/k8s/pkg/k8sd/features/cilium/network_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
69 changes: 69 additions & 0 deletions src/k8s/pkg/k8sd/types/cluster_config_validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"net/netip"
"net/url"
"strings"

"github.com/canonical/k8s/pkg/utils"
)

func validateCIDRs(cidrString string) error {
Expand All @@ -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
Expand All @@ -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() {
Expand Down
23 changes: 13 additions & 10 deletions src/k8s/pkg/k8sd/types/cluster_config_validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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())
Expand All @@ -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())
Expand Down
26 changes: 24 additions & 2 deletions src/k8s/pkg/utils/cidr.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
}
2 changes: 1 addition & 1 deletion src/k8s/pkg/utils/cidr_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 5755f01

Please sign in to comment.