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 all 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
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
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)
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
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) {
eaudetcobello marked this conversation as resolved.
Show resolved Hide resolved
_, 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