kubernetes
/
kops
mirror of https://github.com/kubernetes/kops.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #15036 from johngmyers/addlcidr-subnet 

Improve support for AdditionalNetworkCIDRs
This commit is contained in:
Kubernetes Prow Robot 2023-02-24 06:33:34 -08:00 committed by GitHub
parent 43cc54b2b1 c905c4f827
commit e8f704a855
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 123 additions and 84 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

56
pkg/apis/kops/validation/aws.go
View File

@ -58,18 +58,6 @@ func awsValidateCluster(c *kops.Cluster) field.ErrorList {
allErrs = append(allErrs, awsValidateIAMAuthenticator(field.NewPath("spec", "authentication", "aws"), c.Spec.Authentication.AWS)...) allErrs = append(allErrs, awsValidateIAMAuthenticator(field.NewPath("spec", "authentication", "aws"), c.Spec.Authentication.AWS)...)
} }
for i, subnet := range c.Spec.Networking.Subnets {
f := field.NewPath("spec", "networking", "subnets").Index(i)
if subnet.AdditionalRoutes != nil {
if len(subnet.ID) > 0 {
allErrs = append(allErrs, field.Invalid(f, subnet, "additional routes cannot be added if the subnet is shared"))
} else if subnet.Type != kops.SubnetTypePrivate {
allErrs = append(allErrs, field.Invalid(f, subnet, "additional routes can only be added on private subnets"))
}
allErrs = append(allErrs, awsValidateAdditionalRoutes(f.Child("additionalRoutes"), subnet.AdditionalRoutes, c.Spec.Networking.NetworkCIDR)...)
}
}
return allErrs return allErrs
} }
@ -416,31 +404,29 @@ func hasAWSEBSCSIDriver(c kops.ClusterSpec) bool {
return *c.CloudProvider.AWS.EBSCSIDriver.Enabled return *c.CloudProvider.AWS.EBSCSIDriver.Enabled
} }
func awsValidateAdditionalRoutes(fieldPath *field.Path, routes []kops.RouteSpec, cidr string) field.ErrorList { func awsValidateAdditionalRoutes(fieldPath *field.Path, routes []kops.RouteSpec, networkCIDRs []*net.IPNet) field.ErrorList {
allErrs := field.ErrorList{} allErrs := field.ErrorList{}
_, clusterNet, errClusterNet := net.ParseCIDR(cidr) for i, r := range routes {
if errClusterNet != nil { f := fieldPath.Index(i)
allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "networking", "networkCIDR"), cidr, "Invalid cluster cidr"))
} else {
for i, r := range routes {
f := fieldPath.Index(i)
// Check if target is a known type // Check if target is a known type
if !strings.HasPrefix(r.Target, "pcx-") && if !strings.HasPrefix(r.Target, "pcx-") &&
!strings.HasPrefix(r.Target, "i-") && !strings.HasPrefix(r.Target, "i-") &&
!strings.HasPrefix(r.Target, "nat-") && !strings.HasPrefix(r.Target, "nat-") &&
!strings.HasPrefix(r.Target, "tgw-") && !strings.HasPrefix(r.Target, "tgw-") &&
!strings.HasPrefix(r.Target, "igw-") && !strings.HasPrefix(r.Target, "igw-") &&
!strings.HasPrefix(r.Target, "eigw-") { !strings.HasPrefix(r.Target, "eigw-") {
allErrs = append(allErrs, field.Invalid(f.Child("target"), r, "unknown target type for route")) allErrs = append(allErrs, field.Invalid(f.Child("target"), r, "unknown target type for route"))
} }
ipRoute, _, e := net.ParseCIDR(r.CIDR) var routeCIDRs []*net.IPNet
if e != nil { allErrs = append(allErrs, validateCIDR(r.CIDR, f.Child("cidr"), &routeCIDRs)...)
allErrs = append(allErrs, field.Invalid(f.Child("cidr"), r, "invalid cidr")) if len(routeCIDRs) > 0 {
} else if clusterNet.Contains(ipRoute) && strings.HasPrefix(r.Target, "pcx-") { for _, clusterNet := range networkCIDRs {
allErrs = append(allErrs, field.Forbidden(f.Child("target"), "target is more specific than cluster CIDR block. This route can target only an interface or an instance.")) if clusterNet.Contains(routeCIDRs[0].IP) && strings.HasPrefix(r.Target, "pcx-") {
allErrs = append(allErrs, field.Forbidden(f.Child("target"), "target is more specific than a network CIDR block. This route can target only an interface or an instance."))
}
} }
} }
} }
@ -448,7 +434,9 @@ func awsValidateAdditionalRoutes(fieldPath *field.Path, routes []kops.RouteSpec,
// Check for duplicated CIDR // Check for duplicated CIDR
{ {
cidrs := sets.NewString() cidrs := sets.NewString()
cidrs.Insert(cidr) for _, cidr := range networkCIDRs {
cidrs.Insert(cidr.String())
}
for i := range routes { for i := range routes {
rCidr := routes[i].CIDR rCidr := routes[i].CIDR
if cidrs.Has(rCidr) { if cidrs.Has(rCidr) {

128
pkg/apis/kops/validation/aws_test.go
View File

@ -22,6 +22,7 @@ import (
"github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2" "github.com/aws/aws-sdk-go/service/ec2"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/kops/cloudmock/aws/mockec2" "k8s.io/kops/cloudmock/aws/mockec2"
"k8s.io/kops/upup/pkg/fi" "k8s.io/kops/upup/pkg/fi"
@ -676,14 +677,17 @@ func TestAWSAuthentication(t *testing.T) {
func TestAWSAdditionalRoutes(t *testing.T) { func TestAWSAdditionalRoutes(t *testing.T) {
tests := []struct { tests := []struct {
clusterCidr string name string
providerId string clusterCIDR string
subnetType kops.SubnetType additionalClusterCIDRs []string
route []kops.RouteSpec providerId string
expected []string subnetType kops.SubnetType
route []kops.RouteSpec
expected []string
}{ }{
{ // valid pcx {
clusterCidr: "100.64.0.0/10", name: "valid pcx",
clusterCIDR: "100.64.0.0/10",
subnetType: kops.SubnetTypePrivate, subnetType: kops.SubnetTypePrivate,
route: []kops.RouteSpec{ route: []kops.RouteSpec{
{ {
@ -692,8 +696,9 @@ func TestAWSAdditionalRoutes(t *testing.T) {
}, },
}, },
}, },
{ // valid instance {
clusterCidr: "100.64.0.0/10", name: "valid instance",
clusterCIDR: "100.64.0.0/10",
subnetType: kops.SubnetTypePrivate, subnetType: kops.SubnetTypePrivate,
route: []kops.RouteSpec{ route: []kops.RouteSpec{
{ {
@ -702,8 +707,9 @@ func TestAWSAdditionalRoutes(t *testing.T) {
}, },
}, },
}, },
{ // valid nat {
clusterCidr: "100.64.0.0/10", name: "valid nat",
clusterCIDR: "100.64.0.0/10",
subnetType: kops.SubnetTypePrivate, subnetType: kops.SubnetTypePrivate,
route: []kops.RouteSpec{ route: []kops.RouteSpec{
{ {
@ -712,8 +718,9 @@ func TestAWSAdditionalRoutes(t *testing.T) {
}, },
}, },
}, },
{ // valid transit gateway {
clusterCidr: "100.64.0.0/10", name: "valid transit gateway",
clusterCIDR: "100.64.0.0/10",
subnetType: kops.SubnetTypePrivate, subnetType: kops.SubnetTypePrivate,
route: []kops.RouteSpec{ route: []kops.RouteSpec{
{ {
@ -722,8 +729,9 @@ func TestAWSAdditionalRoutes(t *testing.T) {
}, },
}, },
}, },
{ // valid internet gateway {
clusterCidr: "100.64.0.0/10", name: "valid internet gateway",
clusterCIDR: "100.64.0.0/10",
subnetType: kops.SubnetTypePrivate, subnetType: kops.SubnetTypePrivate,
route: []kops.RouteSpec{ route: []kops.RouteSpec{
{ {
@ -732,8 +740,9 @@ func TestAWSAdditionalRoutes(t *testing.T) {
}, },
}, },
}, },
{ // valid egress only internet gateway {
clusterCidr: "100.64.0.0/10", name: "valid egress only internet gateway",
clusterCIDR: "100.64.0.0/10",
subnetType: kops.SubnetTypePrivate, subnetType: kops.SubnetTypePrivate,
route: []kops.RouteSpec{ route: []kops.RouteSpec{
{ {
@ -742,8 +751,9 @@ func TestAWSAdditionalRoutes(t *testing.T) {
}, },
}, },
}, },
{ // bad cluster cidr {
clusterCidr: "not cidr", name: "bad cluster cidr",
clusterCIDR: "not cidr",
subnetType: kops.SubnetTypePrivate, subnetType: kops.SubnetTypePrivate,
route: []kops.RouteSpec{ route: []kops.RouteSpec{
{ {
@ -753,8 +763,9 @@ func TestAWSAdditionalRoutes(t *testing.T) {
}, },
expected: []string{"Invalid value::spec.networking.networkCIDR"}, expected: []string{"Invalid value::spec.networking.networkCIDR"},
}, },
{ // bad cidr {
clusterCidr: "100.64.0.0/10", name: "bad cidr",
clusterCIDR: "100.64.0.0/10",
subnetType: kops.SubnetTypePrivate, subnetType: kops.SubnetTypePrivate,
route: []kops.RouteSpec{ route: []kops.RouteSpec{
{ {
@ -764,8 +775,9 @@ func TestAWSAdditionalRoutes(t *testing.T) {
}, },
expected: []string{"Invalid value::spec.networking.subnets[0].additionalRoutes[0].cidr"}, expected: []string{"Invalid value::spec.networking.subnets[0].additionalRoutes[0].cidr"},
}, },
{ // bad target {
clusterCidr: "100.64.0.0/10", name: "bad target",
clusterCIDR: "100.64.0.0/10",
subnetType: kops.SubnetTypePrivate, subnetType: kops.SubnetTypePrivate,
route: []kops.RouteSpec{ route: []kops.RouteSpec{
{ {
@ -775,8 +787,9 @@ func TestAWSAdditionalRoutes(t *testing.T) {
}, },
expected: []string{"Invalid value::spec.networking.subnets[0].additionalRoutes[0].target"}, expected: []string{"Invalid value::spec.networking.subnets[0].additionalRoutes[0].target"},
}, },
{ // target more specific {
clusterCidr: "100.64.0.0/10", name: "target more specific",
clusterCIDR: "100.64.0.0/10",
subnetType: kops.SubnetTypePrivate, subnetType: kops.SubnetTypePrivate,
route: []kops.RouteSpec{ route: []kops.RouteSpec{
{ {
@ -786,8 +799,22 @@ func TestAWSAdditionalRoutes(t *testing.T) {
}, },
expected: []string{"Forbidden::spec.networking.subnets[0].additionalRoutes[0].target"}, expected: []string{"Forbidden::spec.networking.subnets[0].additionalRoutes[0].target"},
}, },
{ // duplicates cidr {
clusterCidr: "100.64.0.0/10", name: "target more specific additionalCIDR",
clusterCIDR: "100.64.0.0/16",
additionalClusterCIDRs: []string{"100.66.0.0/16", "100.67.0.0/16"},
subnetType: kops.SubnetTypePrivate,
route: []kops.RouteSpec{
{
CIDR: "100.66.2.0/24",
Target: "pcx-abcdef",
},
},
expected: []string{"Forbidden::spec.networking.subnets[0].additionalRoutes[0].target"},
},
{
name: "duplicates cidr",
clusterCIDR: "100.64.0.0/10",
subnetType: kops.SubnetTypePrivate, subnetType: kops.SubnetTypePrivate,
route: []kops.RouteSpec{ route: []kops.RouteSpec{
{ {
@ -801,8 +828,9 @@ func TestAWSAdditionalRoutes(t *testing.T) {
}, },
expected: []string{"Duplicate value::spec.networking.subnets[0].additionalRoutes[1].cidr"}, expected: []string{"Duplicate value::spec.networking.subnets[0].additionalRoutes[1].cidr"},
}, },
{ // shared subnet {
clusterCidr: "100.64.0.0/10", name: "shared subnet",
clusterCIDR: "100.64.0.0/10",
subnetType: kops.SubnetTypePrivate, subnetType: kops.SubnetTypePrivate,
providerId: "123456", providerId: "123456",
route: []kops.RouteSpec{ route: []kops.RouteSpec{
@ -811,10 +839,11 @@ func TestAWSAdditionalRoutes(t *testing.T) {
Target: "pcx-abcdef", Target: "pcx-abcdef",
}, },
}, },
expected: []string{"Invalid value::spec.networking.subnets[0]"}, expected: []string{"Forbidden::spec.networking.subnets[0].additionalRoutes"},
}, },
{ // not a private subnet {
clusterCidr: "100.64.0.0/10", name: "not a private subnet",
clusterCIDR: "100.64.0.0/10",
subnetType: kops.SubnetTypePublic, subnetType: kops.SubnetTypePublic,
route: []kops.RouteSpec{ route: []kops.RouteSpec{
{ {
@ -822,26 +851,33 @@ func TestAWSAdditionalRoutes(t *testing.T) {
Target: "pcx-abcdef", Target: "pcx-abcdef",
}, },
}, },
expected: []string{"Invalid value::spec.networking.subnets[0]"}, expected: []string{"Forbidden::spec.networking.subnets[0].additionalRoutes"},
}, },
} }
for _, test := range tests { for _, test := range tests {
cluster := kops.Cluster{ t.Run(test.name, func(t *testing.T) {
Spec: kops.ClusterSpec{ cluster := kops.Cluster{
Networking: kops.NetworkingSpec{ Spec: kops.ClusterSpec{
NetworkCIDR: test.clusterCidr, CloudProvider: kops.CloudProviderSpec{
Subnets: []kops.ClusterSubnetSpec{ AWS: &kops.AWSSpec{},
{ },
ID: test.providerId, Networking: kops.NetworkingSpec{
Type: test.subnetType, NetworkCIDR: test.clusterCIDR,
AdditionalRoutes: test.route, AdditionalNetworkCIDRs: test.additionalClusterCIDRs,
Subnets: []kops.ClusterSubnetSpec{
{
Name: "us-east-1a",
ID: test.providerId,
Type: test.subnetType,
AdditionalRoutes: test.route,
},
}, },
}, },
}, },
}, }
} errs := validateNetworking(&cluster, &cluster.Spec.Networking, field.NewPath("spec", "networking"), false, &cloudProviderConstraints{})
errs := awsValidateCluster(&cluster) testErrors(t, test, errs, test.expected)
testErrors(t, test, errs, test.expected) })
} }
} }

9
pkg/apis/kops/validation/validation.go
View File

@ -624,6 +624,15 @@ func validateSubnet(subnet *kops.ClusterSubnetSpec, c *kops.ClusterSpec, fieldPa
} }
} }
if c.CloudProvider.AWS != nil && subnet.AdditionalRoutes != nil {
if len(subnet.ID) > 0 {
allErrs = append(allErrs, field.Forbidden(fieldPath.Child("additionalRoutes"), "additional routes cannot be added if the subnet is shared"))
} else if subnet.Type != kops.SubnetTypePrivate {
allErrs = append(allErrs, field.Forbidden(fieldPath.Child("additionalRoutes"), "additional routes can only be added on private subnets"))
}
allErrs = append(allErrs, awsValidateAdditionalRoutes(fieldPath.Child("additionalRoutes"), subnet.AdditionalRoutes, networkCIDRs)...)
}
return allErrs return allErrs
} }

2
tests/integration/update_cluster/complex/data/aws_s3_object_cluster-completed.spec_content
View File

@ -248,7 +248,7 @@ spec:
- additionalRoutes: - additionalRoutes:
- cidr: 192.168.1.10/32 - cidr: 192.168.1.10/32
target: tgw-0123456 target: tgw-0123456
cidr: 172.20.64.0/19 cidr: 10.1.64.0/19
egress: tgw-123456 egress: tgw-123456
name: us-east-1a-private name: us-east-1a-private
type: Private type: Private

2
tests/integration/update_cluster/complex/in-legacy-v1alpha2.yaml
View File

@ -84,7 +84,7 @@ spec:
name: us-test-1a name: us-test-1a
type: Public type: Public
zone: us-test-1a zone: us-test-1a
- cidr: 172.20.64.0/19 - cidr: 10.1.64.0/19
name: us-east-1a-private name: us-east-1a-private
type: Private type: Private
zone: us-test-1a zone: us-test-1a

2
tests/integration/update_cluster/complex/in-v1alpha2.yaml
View File

@ -85,7 +85,7 @@ spec:
name: us-test-1a name: us-test-1a
type: Public type: Public
zone: us-test-1a zone: us-test-1a
- cidr: 172.20.64.0/19 - cidr: 10.1.64.0/19
name: us-east-1a-private name: us-east-1a-private
type: Private type: Private
zone: us-test-1a zone: us-test-1a

2
tests/integration/update_cluster/complex/kubernetes.tf
View File

@ -1135,7 +1135,7 @@ resource "aws_security_group_rule" "tcp-api-pl-44444444" {
resource "aws_subnet" "us-east-1a-private-complex-example-com" { resource "aws_subnet" "us-east-1a-private-complex-example-com" {
availability_zone = "us-test-1a" availability_zone = "us-test-1a"
cidr_block = "172.20.64.0/19" cidr_block = "10.1.64.0/19"
enable_resource_name_dns_a_record_on_launch = true enable_resource_name_dns_a_record_on_launch = true
private_dns_hostname_type_on_launch = "resource-name" private_dns_hostname_type_on_launch = "resource-name"
tags = { tags = {

6
upup/pkg/fi/cloudup/defaults.go
View File

@ -238,6 +238,12 @@ func assignProxy(cluster *kops.Cluster) (*kops.EgressProxySpec, error) {
klog.Warningf("No NetworkCIDR defined (yet), not adding to egressProxy.excludes") klog.Warningf("No NetworkCIDR defined (yet), not adding to egressProxy.excludes")
} }
for _, cidr := range cluster.Spec.Networking.AdditionalNetworkCIDRs {
if !strings.Contains(cluster.Spec.Networking.EgressProxy.ProxyExcludes, cidr) {
egressSlice = append(egressSlice, cidr)
}
}
egressProxy.ProxyExcludes = strings.Join(egressSlice, ",") egressProxy.ProxyExcludes = strings.Join(egressSlice, ",")
klog.V(8).Infof("Completed setting up Proxy excludes as follows: %q", egressProxy.ProxyExcludes) klog.V(8).Infof("Completed setting up Proxy excludes as follows: %q", egressProxy.ProxyExcludes)
} else { } else {