Skip to content

Commit

Permalink
validation: Allow overlap of pod/node CIDR and service CIDR
Browse files Browse the repository at this point in the history
We allowed this previously, so this is a regression for existing clusters.

These clusters are not obviously broken, and the
kube-controller-manager (for example) will exclude the service range
when issuing node CIDRs.  As such, remove validation until we can
determine if anything is actually broken by an overlap (and a path
forwards if so).

Issue kubernetes#16340
  • Loading branch information
justinsb committed Feb 10, 2024
1 parent baae57a commit 3719027
Show file tree
Hide file tree
Showing 2 changed files with 114 additions and 3 deletions.
8 changes: 5 additions & 3 deletions pkg/apis/kops/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -1047,9 +1047,11 @@ func validateNetworking(cluster *kops.Cluster, v *kops.NetworkingSpec, fldPath *
serviceClusterIPRange, errs = parseCIDR(fldPath.Child("serviceClusterIPRange"), v.ServiceClusterIPRange)
allErrs = append(allErrs, errs...)

if subnet.Overlap(podCIDR, serviceClusterIPRange) {
allErrs = append(allErrs, field.Forbidden(fldPath.Child("serviceClusterIPRange"), fmt.Sprintf("serviceClusterIPRange %q must not overlap podCIDR %q", serviceClusterIPRange, podCIDR)))
}
// Removed as part of #16340; we previously supported this and it seems to work fine.
// We may add back if we find problems and have a path for migrating existing clusters.
// if subnet.Overlap(podCIDR, serviceClusterIPRange) {
// allErrs = append(allErrs, field.Forbidden(fldPath.Child("serviceClusterIPRange"), fmt.Sprintf("serviceClusterIPRange %q must not overlap podCIDR %q", serviceClusterIPRange, podCIDR)))
// }
}
}

Expand Down
109 changes: 109 additions & 0 deletions pkg/apis/kops/validation/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,115 @@ func Test_Validate_Networking_Flannel(t *testing.T) {
}
}

func Test_Validate_Networking_OverlappingCIDR(t *testing.T) {
grid := []struct {
Name string
Networking kops.NetworkingSpec
ExpectedErrors []*field.Error
}{
{
Name: "no-overlap",
Networking: kops.NetworkingSpec{
NetworkCIDR: "10.0.0.0/8",
NonMasqueradeCIDR: "100.64.0.0/10",
PodCIDR: "100.64.10.0/24",
ServiceClusterIPRange: "100.64.20.0/24",
Subnets: []kops.ClusterSubnetSpec{
{
Name: "subnet-test",
CIDR: "10.10.0.0/16",
Type: "Public",
},
},
},
},
{
Name: "overlap-podcidr-and-servicecidr",
Networking: kops.NetworkingSpec{
NetworkCIDR: "10.0.0.0/8",
NonMasqueradeCIDR: "100.64.0.0/10",
PodCIDR: "100.64.0.0/10",
ServiceClusterIPRange: "100.64.0.0/13",
Subnets: []kops.ClusterSubnetSpec{
{
Name: "subnet-test",
CIDR: "10.10.0.0/16",
Type: "Public",
},
},
},
},
{
Name: "overlap-servicecidr-and-subnetcidr",
Networking: kops.NetworkingSpec{
NetworkCIDR: "10.0.0.0/8",
NonMasqueradeCIDR: "100.64.0.0/10",
PodCIDR: "100.64.10.0/24",
ServiceClusterIPRange: "100.64.20.0/24",
Subnets: []kops.ClusterSubnetSpec{
{
Name: "subnet-test",
CIDR: "100.64.20.0/28",
Type: "Public",
},
},
},
ExpectedErrors: []*field.Error{
{
Type: field.ErrorTypeForbidden,
Detail: `subnet "subnet-test" cidr "100.64.20.0/28" is not a subnet of the networkCIDR "10.0.0.0/8"`,
Field: "networking.subnets[0].cidr",
},
{
Type: field.ErrorTypeForbidden,
Detail: `subnet "subnet-test" cidr "100.64.20.0/28" must not overlap serviceClusterIPRange "100.64.20.0/24"`,
Field: "networking.subnets[0].cidr",
},
},
},
}
for _, g := range grid {
t.Run(g.Name, func(t *testing.T) {
cluster := &kops.Cluster{
Spec: kops.ClusterSpec{
KubernetesVersion: "1.27.0",
},
}
cluster.Spec.Networking = g.Networking

errs := validateNetworking(cluster, &cluster.Spec.Networking, field.NewPath("networking"), true, &cloudProviderConstraints{})
testFieldErrors(t, errs, g.ExpectedErrors)
})
}
}

func testFieldErrors(t *testing.T, actual field.ErrorList, expectedErrors []*field.Error) {
t.Helper()

if len(actual) > len(expectedErrors) {
t.Errorf("found unexpected errors: %+v", actual)
}

for _, expected := range expectedErrors {
found := false
for _, err := range actual {
if expected.Type != "" && expected.Type != err.Type {
continue
}
if expected.Detail != "" && expected.Detail != err.Detail {
continue
}
if expected.Field != "" && expected.Field != err.Field {
continue
}
found = true
}
if !found {
t.Errorf("expected error %+v, was not found in errors: %+v", expected, actual)
}
}
}

func Test_Validate_AdditionalPolicies(t *testing.T) {
grid := []struct {
Input map[string]string
Expand Down

0 comments on commit 3719027

Please sign in to comment.