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

Validate additionalRoutes against additionalNetworkCIDRs

This commit is contained in:
John Gardiner Myers 2023-01-21 18:42:58 -08:00
parent 045647f9bb
commit 7d3c20d036
3 changed files with 113 additions and 80 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
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,13 +404,9 @@ 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)
if errClusterNet != nil {
allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "networking", "networkCIDR"), cidr, "Invalid cluster cidr"))
} else {
for i, r := range routes { for i, r := range routes {
f := fieldPath.Index(i) f := fieldPath.Index(i)
@ -436,11 +420,13 @@ func awsValidateAdditionalRoutes(fieldPath *field.Path, routes []kops.RouteSpec,
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) {

98
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
clusterCIDR string
additionalClusterCIDRs []string
providerId string providerId string
subnetType kops.SubnetType subnetType kops.SubnetType
route []kops.RouteSpec route []kops.RouteSpec
expected []string 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,17 +851,23 @@ 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 {
t.Run(test.name, func(t *testing.T) {
cluster := kops.Cluster{ cluster := kops.Cluster{
Spec: kops.ClusterSpec{ Spec: kops.ClusterSpec{
CloudProvider: kops.CloudProviderSpec{
AWS: &kops.AWSSpec{},
},
Networking: kops.NetworkingSpec{ Networking: kops.NetworkingSpec{
NetworkCIDR: test.clusterCidr, NetworkCIDR: test.clusterCIDR,
AdditionalNetworkCIDRs: test.additionalClusterCIDRs,
Subnets: []kops.ClusterSubnetSpec{ Subnets: []kops.ClusterSubnetSpec{
{ {
Name: "us-east-1a",
ID: test.providerId, ID: test.providerId,
Type: test.subnetType, Type: test.subnetType,
AdditionalRoutes: test.route, AdditionalRoutes: test.route,
@ -841,7 +876,8 @@ func TestAWSAdditionalRoutes(t *testing.T) {
}, },
}, },
} }
errs := awsValidateCluster(&cluster) errs := validateNetworking(&cluster, &cluster.Spec.Networking, field.NewPath("spec", "networking"), false, &cloudProviderConstraints{})
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
} }