Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate pod CIDR and service CIDR #695

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/k8s/pkg/k8sd/app/hooks_bootstrap_test.go
eaudetcobello marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
package app
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, cfg types.Network, annota
}

podIpPools := []map[string]any{}
ipv4PodCIDR, ipv6PodCIDR, err := utils.ParseCIDRs(cfg.GetPodCIDR())
ipv4PodCIDR, ipv6PodCIDR, err := utils.SplitCIDRStrings(cfg.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, cfg types.Network, annota
}

serviceCIDRs := []string{}
ipv4ServiceCIDR, ipv6ServiceCIDR, err := utils.ParseCIDRs(cfg.GetServiceCIDR())
ipv4ServiceCIDR, ipv6ServiceCIDR, err := utils.SplitCIDRStrings(cfg.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 @@ -192,10 +192,10 @@ func TestEnabled(t *testing.T) {
func validateValues(t *testing.T, values map[string]any, cfg types.Network) {
g := NewWithT(t)

podIPv4CIDR, podIPv6CIDR, err := utils.ParseCIDRs(cfg.GetPodCIDR())
podIPv4CIDR, podIPv6CIDR, err := utils.SplitCIDRStrings(cfg.GetPodCIDR())
g.Expect(err).ToNot(HaveOccurred())

svcIPv4CIDR, svcIPv6CIDR, err := utils.ParseCIDRs(cfg.GetServiceCIDR())
svcIPv4CIDR, svcIPv6CIDR, err := utils.SplitCIDRStrings(cfg.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, cfg types.Network, _ type
}, nil
}

ipv4CIDR, ipv6CIDR, err := utils.ParseCIDRs(cfg.GetPodCIDR())
ipv4CIDR, ipv6CIDR, err := utils.SplitCIDRStrings(cfg.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 @@ -163,7 +163,7 @@ func validateNetworkValues(t *testing.T, values map[string]any, cfg types.Networ
t.Helper()
g := NewWithT(t)

ipv4CIDR, ipv6CIDR, err := utils.ParseCIDRs(cfg.GetPodCIDR())
ipv4CIDR, ipv6CIDR, err := utils.SplitCIDRStrings(cfg.GetPodCIDR())
g.Expect(err).ToNot(HaveOccurred())

bpfMount, err := utils.GetMountPath("bpf")
Expand Down
66 changes: 66 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,62 @@ func validateCIDRs(cidrString string) error {
return nil
}

// validateCIDROverlapAndSize checks for overlap and size constraints between pod and service CIDRs.
eaudetcobello marked this conversation as resolved.
Show resolved Hide resolved
// 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.
eaudetcobello marked this conversation as resolved.
Show resolved Hide resolved
eaudetcobello marked this conversation as resolved.
Show resolved Hide resolved
// 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 {
eaudetcobello marked this conversation as resolved.
Show resolved Hide resolved
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 +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)
HomayoonAlimohammadi marked this conversation as resolved.
Show resolved Hide resolved
}
// 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
46 changes: 45 additions & 1 deletion src/k8s/pkg/k8sd/types/cluster_config_validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,13 @@ func TestValidateCIDR(t *testing.T) {
cidr string
expectErr bool
}{
{cidr: "10.1.0.0/16"},
{cidr: "192.168.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: "10.1.0.0/32", expectErr: true},
} {
t.Run(tc.cidr, func(t *testing.T) {
t.Run("Pod", func(t *testing.T) {
Expand Down Expand Up @@ -128,3 +129,46 @@ func TestValidateExternalServers(t *testing.T) {
})
}
}

func TestValidateCIDROverlap(t *testing.T) {
eaudetcobello marked this conversation as resolved.
Show resolved Hide resolved
for _, tc := range []struct {
name string
podCIDR string
serviceCIDR string
expectErr bool
}{
{"RandomName", "nogood", "nogood", true},
{"BothValid", "10.1.0.0/16", "2001:0db8::/108", false},
//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.0.0/32", "10.0.0.0/24", true},
{"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", true},
{"InvalidIPv6CIDRFormat", "2001:db8::1/129", "fc00::/64", true},
{"MaxSizeIPv6CIDRs", "::/0", "::/0", true},
//Mixed
{"IPv4AndIPv6MixedCIDRs", "192.168.0.0/16", "2001:db8::/32", true},
{"OnlyInvalidIPv6CIDR", "", "2001:db8::/65", true},
} {
t.Run(tc.name, func(t *testing.T) {
g := NewWithT(t)
config := types.ClusterConfig{
Network: types.Network{
PodCIDR: utils.Pointer(tc.podCIDR),
ServiceCIDR: utils.Pointer(tc.serviceCIDR),
},
}
err := config.Validate()
if tc.expectErr {
g.Expect(err).To(HaveOccurred())
} else {
g.Expect(err).To(BeNil())
}
})
}
}
22 changes: 20 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,21 @@ 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) {
eaudetcobello marked this conversation as resolved.
Show resolved Hide resolved
_, ipNet1, err1 := net.ParseCIDR(cidr1)
_, ipNet2, err2 := net.ParseCIDR(cidr2)

if err1 != nil || err2 != nil {
return false, fmt.Errorf("invalid CIDR blocks, %v, %v", err1, err2)
eaudetcobello marked this conversation as resolved.
Show resolved Hide resolved
}

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