Skip to content

Commit

Permalink
tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@kon-angelo
kon-angelo committed Aug 20, 2022
1 parent aab8fdd commit 6117caf
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 52 deletions.
21 changes: 10 additions & 11 deletions pkg/admission/validator/shoot.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,6 @@ import (
"fmt"
"reflect"

"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/utils/pointer"

"github.com/gardener/gardener-extension-provider-gcp/pkg/admission"
apisgcp "github.com/gardener/gardener-extension-provider-gcp/pkg/apis/gcp"
gcpvalidation "github.com/gardener/gardener-extension-provider-gcp/pkg/apis/gcp/validation"
Expand Down Expand Up @@ -73,7 +70,9 @@ func (s *shoot) Validate(ctx context.Context, new, old client.Object) error {
if !ok {
return fmt.Errorf("wrong object type %T for old object", old)
}
return s.validateUpdate(ctx, oldShoot, shoot)
if err := s.validateUpdate(ctx, oldShoot, shoot); err != nil {
return err
}
}

return s.validateCreate(ctx, shoot)
Expand Down Expand Up @@ -174,11 +173,11 @@ func (s *shoot) validateUpdate(ctx context.Context, oldShoot, currentShoot *core
allErrors = field.ErrorList{}
)

var seedServices *string
if currentValContext.seed != nil {
seedServices = pointer.String(currentValContext.seed.Spec.Networks.Services)
}
allErrors = append(allErrors, gcpvalidation.ValidateInfrastructureConfigUpdate(oldInfrastructureConfig, currentInfrastructureConfig, seedServices, infrastructureConfigPath)...)
// var seedServices *string
// if currentValContext.seed != nil {
// seedServices = pointer.String(currentValContext.seed.Spec.Networks.Services)
// }
allErrors = append(allErrors, gcpvalidation.ValidateInfrastructureConfigUpdate(oldInfrastructureConfig, currentInfrastructureConfig, infrastructureConfigPath)...)

if !reflect.DeepEqual(oldControlPlaneConfig, currentControlPlaneConfig) {
allErrors = append(allErrors, gcpvalidation.ValidateControlPlaneConfigUpdate(oldControlPlaneConfig, currentControlPlaneConfig, controlPlaneConfigPath)...)
Expand Down Expand Up @@ -226,8 +225,8 @@ func newValidationContext(ctx context.Context, decoder runtime.Decoder, c client
if shoot.Spec.SeedName != nil && len(*shoot.Spec.SeedName) > 0 {
seed = &gardencorev1beta1.Seed{}
err = c.Get(ctx, kutil.Key(*shoot.Spec.SeedName), seed)
if err != nil && !errors.IsNotFound(err) {
return nil, fmt.Errorf("an error occured while reading seed information: %v", err)
if err != nil {
return nil, fmt.Errorf("failed to fetch seed information: %v", err)
}
}

Expand Down
33 changes: 14 additions & 19 deletions pkg/apis/gcp/validation/infrastructure.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ func ValidateInfrastructureConfig(infra *apisgcp.InfrastructureConfig, nodesCIDR

if infra.Networks.PrivateServiceConnect != nil {
ranges := []cidrvalidation.CIDR{
pods, services, nodes, workerCIDR,
pods, services, nodes, workerCIDR, cidrvalidation.NewCIDR(reverseVpnCidr, nil),
}
if infra.Networks.Internal != nil {
ranges = append(ranges, internalCIDR)
Expand All @@ -149,7 +149,7 @@ func ValidateInfrastructureConfig(infra *apisgcp.InfrastructureConfig, nodesCIDR
}

// ValidateInfrastructureConfigUpdate validates a InfrastructureConfig object.
func ValidateInfrastructureConfigUpdate(oldConfig, newConfig *apisgcp.InfrastructureConfig, seedServices *string, fldPath *field.Path) field.ErrorList {
func ValidateInfrastructureConfigUpdate(oldConfig, newConfig *apisgcp.InfrastructureConfig, fldPath *field.Path) field.ErrorList {
var (
allErrs = field.ErrorList{}
networksPath = fldPath.Child("networks")
Expand All @@ -171,14 +171,14 @@ func ValidateInfrastructureConfigUpdate(oldConfig, newConfig *apisgcp.Infrastruc
allErrs = append(allErrs, apivalidation.ValidateImmutableField(newConfig.Networks.Worker, oldConfig.Networks.Worker, networksPath.Child("worker"))...)
}

if newConfig.Networks.PrivateServiceConnect != nil {
var ranges []cidrvalidation.CIDR
if seedServices != nil {
ranges = append(ranges, cidrvalidation.NewCIDR(*seedServices, field.NewPath("seed service range")))
}

allErrs = append(allErrs, ValidatePrivateServiceConnect(networksPath.Child("privateServiceConnect"), newConfig.Networks.PrivateServiceConnect, ranges)...)
}
// if newConfig.Networks.PrivateServiceConnect != nil {
//
// if seedServicesCIDR != nil {
// ranges = append(ranges, cidrvalidation.NewCIDR(*seedServicesCIDR, field.NewPath("seed service range")))
// }
//
// allErrs = append(allErrs, ValidatePrivateServiceConnect(networksPath.Child("privateServiceConnect"), newConfig.Networks.PrivateServiceConnect, ranges)...)
// }
return allErrs
}

Expand All @@ -195,24 +195,19 @@ func ValidatePrivateServiceConnect(fldPath *field.Path, psc *apisgcp.PrivateServ
}

if len(psc.EndpointIP) == 0 {
allErrs = append(allErrs, field.Required(fldPath, "endpointIP must be specified"))
allErrs = append(allErrs, field.Required(fldPath.Child("endpointIP"), "endpointIP must be specified"))
return allErrs
}

if net.ParseIP(psc.EndpointIP) == nil {
allErrs = append(allErrs, field.Invalid(fldPath, psc.EndpointIP, "endpointIP must be a valid IP address"))
allErrs = append(allErrs, field.Invalid(fldPath.Child("endpointIP"), psc.EndpointIP, "endpointIP must be a valid IP address"))
return allErrs
}

endpointCIDR := cidrvalidation.NewCIDR(fmt.Sprintf("%s/32", psc.EndpointIP), fldPath)
endpointCIDR := cidrvalidation.NewCIDR(fmt.Sprintf("%s/32", psc.EndpointIP), fldPath.Child("endpointIP"))
allErrs = append(allErrs, cidrvalidation.ValidateCIDRParse(endpointCIDR)...)

var rangesToVerify = []cidrvalidation.CIDR{
cidrvalidation.NewCIDR(reverseVpnCidr, nil),
}
rangesToVerify = append(rangesToVerify, ranges...)

allErrs = append(allErrs, endpointCIDR.ValidateNotOverlap(rangesToVerify...)...)
allErrs = append(allErrs, endpointCIDR.ValidateNotOverlap(ranges...)...)
return allErrs
}

Expand Down
103 changes: 81 additions & 22 deletions pkg/apis/gcp/validation/infrastructure_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ var _ = Describe("InfrastructureConfig validation", func() {
It("should forbid invalid worker CIDRs", func() {
infrastructureConfig.Networks.Workers = invalidCIDR

errorList := ValidateInfrastructureConfig(infrastructureConfig, &nodes, &pods, &services, nil, nil, fldPath)
errorList := ValidateInfrastructureConfig(infrastructureConfig, &nodes, &pods, &services, nil, fldPath)

Expect(errorList).To(ConsistOfFields(Fields{
"Type": Equal(field.ErrorTypeInvalid),
Expand All @@ -85,7 +85,7 @@ var _ = Describe("InfrastructureConfig validation", func() {
invalidCIDR = "invalid-cidr"
infrastructureConfig.Networks.Internal = &invalidCIDR

errorList := ValidateInfrastructureConfig(infrastructureConfig, &nodes, &pods, &services, nil, nil, fldPath)
errorList := ValidateInfrastructureConfig(infrastructureConfig, &nodes, &pods, &services, nil, fldPath)

Expect(errorList).To(ConsistOfFields(Fields{
"Type": Equal(field.ErrorTypeInvalid),
Expand All @@ -97,7 +97,7 @@ var _ = Describe("InfrastructureConfig validation", func() {
It("should forbid workers CIDR which are not in Nodes CIDR", func() {
infrastructureConfig.Networks.Workers = "1.1.1.1/32"

errorList := ValidateInfrastructureConfig(infrastructureConfig, &nodes, &pods, &services, nil, nil, fldPath)
errorList := ValidateInfrastructureConfig(infrastructureConfig, &nodes, &pods, &services, nil, fldPath)

Expect(errorList).To(ConsistOfFields(Fields{
"Type": Equal(field.ErrorTypeInvalid),
Expand All @@ -111,7 +111,7 @@ var _ = Describe("InfrastructureConfig validation", func() {
infrastructureConfig.Networks.Internal = &overlappingCIDR
infrastructureConfig.Networks.Workers = overlappingCIDR

errorList := ValidateInfrastructureConfig(infrastructureConfig, &overlappingCIDR, &pods, &services, nil, nil, fldPath)
errorList := ValidateInfrastructureConfig(infrastructureConfig, &overlappingCIDR, &pods, &services, nil, fldPath)

Expect(errorList).To(ConsistOfFields(Fields{
"Type": Equal(field.ErrorTypeInvalid),
Expand All @@ -132,7 +132,7 @@ var _ = Describe("InfrastructureConfig validation", func() {
infrastructureConfig.Networks.Internal = &internal
infrastructureConfig.Networks.Workers = "10.250.3.8/24"

errorList := ValidateInfrastructureConfig(infrastructureConfig, &nodeCIDR, &podCIDR, &serviceCIDR, nil, nil, fldPath)
errorList := ValidateInfrastructureConfig(infrastructureConfig, &nodeCIDR, &podCIDR, &serviceCIDR, nil, fldPath)

Expect(errorList).To(HaveLen(2))
Expect(errorList).To(ConsistOfFields(Fields{
Expand All @@ -147,12 +147,12 @@ var _ = Describe("InfrastructureConfig validation", func() {
})

It("should allow specifying valid config", func() {
errorList := ValidateInfrastructureConfig(infrastructureConfig, &nodes, &pods, &services, nil, nil, fldPath)
errorList := ValidateInfrastructureConfig(infrastructureConfig, &nodes, &pods, &services, nil, fldPath)
Expect(errorList).To(BeEmpty())
})

It("should allow specifying valid config with podsCIDR=nil and servicesCIDR=nil", func() {
errorList := ValidateInfrastructureConfig(infrastructureConfig, &nodes, nil, nil, nil, nil, fldPath)
errorList := ValidateInfrastructureConfig(infrastructureConfig, &nodes, nil, nil, nil, fldPath)
Expect(errorList).To(BeEmpty())
})
})
Expand All @@ -167,7 +167,7 @@ var _ = Describe("InfrastructureConfig validation", func() {
testInfrastructureConfig.Networks.VPC = &apisgcp.VPC{}
testInfrastructureConfig.Networks.VPC.CloudRouter = &apisgcp.CloudRouter{}

errorList := ValidateInfrastructureConfig(testInfrastructureConfig, &nodes, &pods, &services, nil, nil, fldPath)
errorList := ValidateInfrastructureConfig(testInfrastructureConfig, &nodes, &pods, &services, nil, fldPath)
Expect(errorList).To(ConsistOfFields(Fields{
"Type": Equal(field.ErrorTypeInvalid),
"Field": Equal("networks.vpc.cloudRouter"),
Expand All @@ -181,7 +181,7 @@ var _ = Describe("InfrastructureConfig validation", func() {
It("should forbid empty VPC flow log config", func() {
infrastructureConfig.Networks.FlowLogs = &apisgcp.FlowLogs{}

errorList := ValidateInfrastructureConfig(infrastructureConfig, &nodes, &pods, &services, nil, nil, fldPath)
errorList := ValidateInfrastructureConfig(infrastructureConfig, &nodes, &pods, &services, nil, fldPath)
Expect(errorList).To(ConsistOfFields(Fields{
"Type": Equal(field.ErrorTypeRequired),
"Field": Equal("networks.flowLogs"),
Expand All @@ -194,7 +194,7 @@ var _ = Describe("InfrastructureConfig validation", func() {
metadata := "foo"
infrastructureConfig.Networks.FlowLogs = &apisgcp.FlowLogs{AggregationInterval: &aggregationInterval, FlowSampling: &flowSampling, Metadata: &metadata}

errorList := ValidateInfrastructureConfig(infrastructureConfig, &nodes, &pods, &services, nil, nil, fldPath)
errorList := ValidateInfrastructureConfig(infrastructureConfig, &nodes, &pods, &services, nil, fldPath)
Expect(errorList).To(ConsistOfFields(Fields{
"Type": Equal(field.ErrorTypeNotSupported),
"Field": Equal("networks.flowLogs.aggregationInterval"),
Expand All @@ -214,7 +214,7 @@ var _ = Describe("InfrastructureConfig validation", func() {
Name: "test-vpc",
}

errorList := ValidateInfrastructureConfig(testInfrastructureConfig, &nodes, &pods, &services, nil, nil, fldPath)
errorList := ValidateInfrastructureConfig(testInfrastructureConfig, &nodes, &pods, &services, nil, fldPath)
Expect(errorList).To(ConsistOfFields(Fields{
"Type": Equal(field.ErrorTypeInvalid),
"Field": Equal("networks.vpc.cloudRouter"),
Expand All @@ -227,7 +227,7 @@ var _ = Describe("InfrastructureConfig validation", func() {
CloudRouter: &apisgcp.CloudRouter{},
}

errorList := ValidateInfrastructureConfig(testInfrastructureConfig, &nodes, &pods, &services, nil, nil, fldPath)
errorList := ValidateInfrastructureConfig(testInfrastructureConfig, &nodes, &pods, &services, nil, fldPath)
Expect(errorList).To(ConsistOfFields(Fields{
"Type": Equal(field.ErrorTypeInvalid),
"Field": Equal("networks.vpc.cloudRouter.name"),
Expand All @@ -239,7 +239,7 @@ var _ = Describe("InfrastructureConfig validation", func() {
Name: "test-vpc",
}

errorList := ValidateInfrastructureConfig(testInfrastructureConfig, &nodes, &pods, &services, nil, nil, fldPath)
errorList := ValidateInfrastructureConfig(testInfrastructureConfig, &nodes, &pods, &services, nil, fldPath)
Expect(errorList).To(ConsistOfFields(Fields{
"Type": Equal(field.ErrorTypeInvalid),
"Field": Equal("networks.vpc.cloudRouter"),
Expand All @@ -252,7 +252,7 @@ var _ = Describe("InfrastructureConfig validation", func() {
CloudRouter: &apisgcp.CloudRouter{},
}

errorList := ValidateInfrastructureConfig(testInfrastructureConfig, &nodes, &pods, &services, nil, nil, fldPath)
errorList := ValidateInfrastructureConfig(testInfrastructureConfig, &nodes, &pods, &services, nil, fldPath)
Expect(errorList).To(ConsistOfFields(Fields{
"Type": Equal(field.ErrorTypeInvalid),
"Field": Equal("networks.vpc.cloudRouter.name"),
Expand All @@ -263,7 +263,7 @@ var _ = Describe("InfrastructureConfig validation", func() {
It("should forbid empty VPC flow log config", func() {
infrastructureConfig.Networks.FlowLogs = &apisgcp.FlowLogs{}

errorList := ValidateInfrastructureConfig(infrastructureConfig, &nodes, &pods, &services, nil, nil, fldPath)
errorList := ValidateInfrastructureConfig(infrastructureConfig, &nodes, &pods, &services, nil, fldPath)
Expect(errorList).To(ConsistOfFields(Fields{
"Type": Equal(field.ErrorTypeRequired),
"Field": Equal("networks.flowLogs"),
Expand All @@ -276,7 +276,7 @@ var _ = Describe("InfrastructureConfig validation", func() {
metadata := "foo"
infrastructureConfig.Networks.FlowLogs = &apisgcp.FlowLogs{AggregationInterval: &aggregationInterval, FlowSampling: &flowSampling, Metadata: &metadata}

errorList := ValidateInfrastructureConfig(infrastructureConfig, &nodes, &pods, &services, nil, nil, fldPath)
errorList := ValidateInfrastructureConfig(infrastructureConfig, &nodes, &pods, &services, nil, fldPath)
Expect(errorList).To(ConsistOfFields(Fields{
"Type": Equal(field.ErrorTypeNotSupported),
"Field": Equal("networks.flowLogs.aggregationInterval"),
Expand All @@ -297,7 +297,7 @@ var _ = Describe("InfrastructureConfig validation", func() {
metadata := "INCLUDE_ALL_METADATA"
infrastructureConfig.Networks.FlowLogs = &apisgcp.FlowLogs{AggregationInterval: &aggregationInterval, FlowSampling: &flowSampling, Metadata: &metadata}

errorList := ValidateInfrastructureConfig(infrastructureConfig, &nodes, &pods, &services, nil, nil, fldPath)
errorList := ValidateInfrastructureConfig(infrastructureConfig, &nodes, &pods, &services, nil, fldPath)
Expect(errorList).To(BeEmpty())
})
})
Expand All @@ -310,15 +310,74 @@ var _ = Describe("InfrastructureConfig validation", func() {
FlowSampling: pointer.Float32Ptr(0.5),
}

errorList := ValidateInfrastructureConfig(newInfrastructureConfig, &nodes, &pods, &services, nil, nil, fldPath)
errorList := ValidateInfrastructureConfig(newInfrastructureConfig, &nodes, &pods, &services, nil, fldPath)
Expect(errorList).To(BeEmpty())
})
})
Context("Private Service Connect", func() {
It("should allow correct PSC configuration", func() {
newInfrastructureConfig := infrastructureConfig.DeepCopy()
newInfrastructureConfig.Networks.PrivateServiceConnect = &apisgcp.PrivateServiceConnectConfig{
EndpointIP: "10.15.0.0",
}
errorList := ValidateInfrastructureConfig(newInfrastructureConfig, &nodes, &pods, &services, nil, fldPath)
Expect(errorList).To(BeEmpty())
})
It("should return an error if endpointIP is not specified", func() {
newInfrastructureConfig := infrastructureConfig.DeepCopy()
newInfrastructureConfig.Networks.PrivateServiceConnect = &apisgcp.PrivateServiceConnectConfig{
EndpointIP: "",
}
errorList := ValidateInfrastructureConfig(newInfrastructureConfig, &nodes, &pods, &services, nil, fldPath)
Expect(errorList).To(ConsistOfFields(Fields{
"Type": Equal(field.ErrorTypeRequired),
"Field": Equal("networks.privateServiceConnect.endpointIP"),
"Detail": Equal("endpointIP must be specified"),
}))
})
It("should return an error when PSC endpoint collides with shoot CIDRs", func() {
newInfrastructureConfig := infrastructureConfig.DeepCopy()
newInfrastructureConfig.Networks.PrivateServiceConnect = &apisgcp.PrivateServiceConnectConfig{
EndpointIP: "100.96.0.0",
}
errorList := ValidateInfrastructureConfig(newInfrastructureConfig, &nodes, &pods, &services, nil, fldPath)
Expect(errorList).To(ConsistOfFields(Fields{
"Type": Equal(field.ErrorTypeInvalid),
"Detail": Equal(`must not overlap with "networks.privateServiceConnect.endpointIP" ("100.96.0.0/32")`),
}))
})
It("should return an error when PSC endpoint collides with seed CIDRs", func() {
newInfrastructureConfig := infrastructureConfig.DeepCopy()
seedServiceCIDR := "10.100.0.0/16"
newInfrastructureConfig.Networks.PrivateServiceConnect = &apisgcp.PrivateServiceConnectConfig{
EndpointIP: "10.100.100.0",
}
errorList := ValidateInfrastructureConfig(newInfrastructureConfig, &nodes, &pods, &services, &seedServiceCIDR, fldPath)
Expect(errorList).To(ConsistOfFields(Fields{
"Type": Equal(field.ErrorTypeInvalid),
"Field": Equal("seed service range"),
"Detail": Equal(`must not overlap with "networks.privateServiceConnect.endpointIP" ("10.100.100.0/32")`),
}))
})

It("should return an error when PSC endpoint colliced with reverse VPN CIDRs", func() {
newInfrastructureConfig := infrastructureConfig.DeepCopy()
newInfrastructureConfig.Networks.PrivateServiceConnect = &apisgcp.PrivateServiceConnectConfig{
EndpointIP: "192.168.123.0",
}
errorList := ValidateInfrastructureConfig(newInfrastructureConfig, &nodes, &pods, &services, nil, fldPath)
Expect(errorList).To(ConsistOfFields(Fields{
"Type": Equal(field.ErrorTypeInvalid),
"Field": Equal("<nil>"),
"Detail": Equal(`must not overlap with "networks.privateServiceConnect.endpointIP" ("192.168.123.0/32")`),
}))
})
})
})

Describe("#ValidateInfrastructureConfigUpdate", func() {
It("should return no errors for an unchanged config", func() {
Expect(ValidateInfrastructureConfigUpdate(infrastructureConfig, infrastructureConfig, nil, nil, fldPath)).To(BeEmpty())
Expect(ValidateInfrastructureConfigUpdate(infrastructureConfig, infrastructureConfig, fldPath)).To(BeEmpty())
})

It("should allow changing cloudNAT AND Flow-logs details", func() {
Expand All @@ -333,7 +392,7 @@ var _ = Describe("InfrastructureConfig validation", func() {
AggregationInterval: pointer.StringPtr("INTERVAL_30_SEC"),
}

errorList := ValidateInfrastructureConfigUpdate(infrastructureConfig, newInfrastructureConfig, nil, nil, fldPath)
errorList := ValidateInfrastructureConfigUpdate(infrastructureConfig, newInfrastructureConfig, fldPath)
Expect(errorList).To(BeEmpty())
})

Expand All @@ -349,7 +408,7 @@ var _ = Describe("InfrastructureConfig validation", func() {
newInfrastructureConfig.Networks.Worker = "10.96.0.0/16"
newInfrastructureConfig.Networks.Internal = pointer.StringPtr("10.96.0.0/16")

errorList := ValidateInfrastructureConfigUpdate(infrastructureConfig, newInfrastructureConfig, nil, nil, fldPath)
errorList := ValidateInfrastructureConfigUpdate(infrastructureConfig, newInfrastructureConfig, fldPath)
Expect(errorList).To(ConsistOfFields(Fields{
"Type": Equal(field.ErrorTypeInvalid),
"Field": Equal("networks.vpc.name"),
Expand All @@ -372,7 +431,7 @@ var _ = Describe("InfrastructureConfig validation", func() {
newInfrastructureConfig := infrastructureConfig.DeepCopy()
newInfrastructureConfig.Networks.VPC = nil

errorList := ValidateInfrastructureConfigUpdate(infrastructureConfig, newInfrastructureConfig, nil, nil, fldPath)
errorList := ValidateInfrastructureConfigUpdate(infrastructureConfig, newInfrastructureConfig, fldPath)
Expect(errorList).To(ConsistOfFields(Fields{
"Type": Equal(field.ErrorTypeInvalid),
"Field": Equal("networks.vpc"),
Expand Down

0 comments on commit 6117caf

Please sign in to comment.