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.

Sign up for GitHub

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

Jump to bottom

validation: Allow overlap of pod/node CIDR and service CIDR #16344

Merged
merged 1 commit into from Feb 10, 2024
Merged

validation: Allow overlap of pod/node CIDR and service CIDR #16344

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 5 additions & 3 deletions pkg/apis/kops/validation/validation.go
View file
Open in desktop
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
View file
Open in desktop
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