Skip to content

Commit

Permalink
Merge pull request #210 from dkistner/validate-vnet
Browse files Browse the repository at this point in the history 
Validation: Allow to resize the vnet cidr
  • Loading branch information
@dkistner
dkistner authored Dec 7, 2020
2 parents d15aeec + ee8ceea commit 5541dfb
Show file tree
Hide file tree
Showing 3 changed files with 134 additions and 41 deletions.
99 changes: 73 additions & 26 deletions pkg/apis/azure/validation/infrastructure.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,35 +57,13 @@ func ValidateInfrastructureConfig(infra *apisazure.InfrastructureConfig, nodesCI

networksPath := fldPath.Child("networks")

// Validate workers subnet cidr
workerCIDR := cidrvalidation.NewCIDR(infra.Networks.Workers, networksPath.Child("workers"))

allErrs = append(allErrs, cidrvalidation.ValidateCIDRParse(workerCIDR)...)
allErrs = append(allErrs, cidrvalidation.ValidateCIDRIsCanonical(networksPath.Child("workers"), infra.Networks.Workers)...)

if (infra.Networks.VNet.Name != nil && infra.Networks.VNet.ResourceGroup == nil) || (infra.Networks.VNet.Name == nil && infra.Networks.VNet.ResourceGroup != nil) {
allErrs = append(allErrs, field.Invalid(networksPath.Child("vnet"), infra.Networks.VNet, "specifying an existing vnet name require a vnet name and vnet resource group"))
} else if infra.Networks.VNet.Name != nil && infra.Networks.VNet.ResourceGroup != nil {
if infra.Networks.VNet.CIDR != nil {
allErrs = append(allErrs, field.Invalid(networksPath.Child("vnet", "cidr"), *infra.Networks.VNet.ResourceGroup, "specifying a cidr for an existing vnet is not possible"))
}
if infra.ResourceGroup != nil && *infra.Networks.VNet.ResourceGroup == infra.ResourceGroup.Name {
allErrs = append(allErrs, field.Invalid(networksPath.Child("vnet", "resourceGroup"), *infra.Networks.VNet.ResourceGroup, "the vnet resource group must not be the same as the cluster resource group"))
}
} else {
cidrPath := networksPath.Child("vnet", "cidr")
if infra.Networks.VNet.CIDR == nil {
// Use worker/subnet cidr as cidr for the vnet.
allErrs = append(allErrs, workerCIDR.ValidateSubset(nodes)...)
allErrs = append(allErrs, workerCIDR.ValidateNotSubset(pods, services)...)
} else {
vpcCIDR := cidrvalidation.NewCIDR(*(infra.Networks.VNet.CIDR), cidrPath)
allErrs = append(allErrs, vpcCIDR.ValidateParse()...)
allErrs = append(allErrs, vpcCIDR.ValidateSubset(nodes)...)
allErrs = append(allErrs, vpcCIDR.ValidateSubset(workerCIDR)...)
allErrs = append(allErrs, vpcCIDR.ValidateNotSubset(pods, services)...)
allErrs = append(allErrs, cidrvalidation.ValidateCIDRIsCanonical(cidrPath, *infra.Networks.VNet.CIDR)...)
}
}
// Validate vnet config
allErrs = append(allErrs, validateVnetConfig(infra.Networks.VNet, infra.ResourceGroup, workerCIDR, nodes, pods, services, networksPath.Child("vnet"))...)

// TODO(dkistner) Remove once we proceed with multiple AvailabilitySet support.
// Currently we will not offer Nat Gateway for non zoned/AvailabilitySet based
Expand Down Expand Up @@ -114,14 +92,83 @@ func ValidateInfrastructureConfig(infra *apisazure.InfrastructureConfig, nodesCI
return allErrs
}

func validateVnetConfig(vnetConfig apisazure.VNet, resourceGroupConfig *apisazure.ResourceGroup, workers, nodes, pods, services cidrvalidation.CIDR, vnetConfigPath *field.Path) field.ErrorList {
var allErrs = field.ErrorList{}

// Validate that just vnet name or vnet resource group is specified.
if (vnetConfig.Name != nil && vnetConfig.ResourceGroup == nil) || (vnetConfig.Name == nil && vnetConfig.ResourceGroup != nil) {
return append(allErrs, field.Invalid(vnetConfigPath, vnetConfig, "a vnet cidr or vnet name and resource group need to be specified"))
}

if isExternalVnetUsed(&vnetConfig) {
if vnetConfig.CIDR != nil {
allErrs = append(allErrs, field.Invalid(vnetConfigPath.Child("cidr"), vnetConfig, "specifying a cidr for an existing vnet is not possible"))
}

if resourceGroupConfig != nil && *vnetConfig.ResourceGroup == resourceGroupConfig.Name {
allErrs = append(allErrs, field.Invalid(vnetConfigPath.Child("resourceGroup"), *vnetConfig.ResourceGroup, "the vnet resource group must not be the same as the cluster resource group"))
}
return allErrs
}

// Validate no cidr config is specified at all.
if isDefaultVnetConfig(&vnetConfig) {
allErrs = append(allErrs, workers.ValidateSubset(nodes)...)
allErrs = append(allErrs, workers.ValidateNotSubset(pods, services)...)
return allErrs
}

vnetCIDR := cidrvalidation.NewCIDR(*vnetConfig.CIDR, vnetConfigPath.Child("cidr"))
allErrs = append(allErrs, vnetCIDR.ValidateParse()...)
allErrs = append(allErrs, vnetCIDR.ValidateSubset(nodes)...)
allErrs = append(allErrs, vnetCIDR.ValidateSubset(workers)...)
allErrs = append(allErrs, vnetCIDR.ValidateNotSubset(pods, services)...)
allErrs = append(allErrs, cidrvalidation.ValidateCIDRIsCanonical(vnetConfigPath.Child("cidr"), *vnetConfig.CIDR)...)

return allErrs
}

// ValidateInfrastructureConfigUpdate validates a InfrastructureConfig object.
func ValidateInfrastructureConfigUpdate(oldConfig, newConfig *apisazure.InfrastructureConfig, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}

allErrs = append(allErrs, apivalidation.ValidateImmutableField(newConfig.ResourceGroup, oldConfig.ResourceGroup, fldPath.Child("resourceGroup"))...)
allErrs = append(allErrs, apivalidation.ValidateImmutableField(newConfig.Networks.VNet, oldConfig.Networks.VNet, fldPath.Child("networks").Child("vnet"))...)
allErrs = append(allErrs, apivalidation.ValidateImmutableField(newConfig.Networks.Workers, oldConfig.Networks.Workers, fldPath.Child("networks").Child("workers"))...)
allErrs = append(allErrs, apivalidation.ValidateImmutableField(oldConfig.Zoned, newConfig.Zoned, fldPath.Child("zoned"))...)
allErrs = append(allErrs, validateVnetConfigUpdate(&oldConfig.Networks, &newConfig.Networks, fldPath.Child("networks"))...)

return allErrs
}

func validateVnetConfigUpdate(oldNeworkConfig, newNetworkConfig *apisazure.NetworkConfig, networkConfigPath *field.Path) field.ErrorList {
var allErrs = field.ErrorList{}

if isExternalVnetUsed(&oldNeworkConfig.VNet) || isDefaultVnetConfig(&oldNeworkConfig.VNet) {
allErrs = append(allErrs, apivalidation.ValidateImmutableField(newNetworkConfig.VNet.Name, oldNeworkConfig.VNet.Name, networkConfigPath.Child("vnet", "name"))...)
allErrs = append(allErrs, apivalidation.ValidateImmutableField(newNetworkConfig.VNet.ResourceGroup, oldNeworkConfig.VNet.ResourceGroup, networkConfigPath.Child("vnet", "resourceGroup"))...)
return allErrs
}

if oldNeworkConfig.VNet.CIDR != nil && newNetworkConfig.VNet.CIDR == nil {
return append(allErrs, field.Invalid(networkConfigPath.Child("vnet", "cidr"), newNetworkConfig.VNet.CIDR, "vnet cidr need to be specified"))
}

return allErrs
}

func isExternalVnetUsed(vnetConfig *apisazure.VNet) bool {
if vnetConfig == nil {
return false
}
if vnetConfig.Name != nil && vnetConfig.ResourceGroup != nil {
return true
}
return false
}

func isDefaultVnetConfig(vnetConfig *apisazure.VNet) bool {
if vnetConfig.CIDR == nil && vnetConfig.Name == nil && vnetConfig.ResourceGroup == nil {
return true
}
return false
}
60 changes: 49 additions & 11 deletions pkg/apis/azure/validation/infrastructure_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
. "github.com/onsi/gomega"
. "github.com/onsi/gomega/gstruct"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/utils/pointer"
)

var _ = Describe("InfrastructureConfig validation", func() {
Expand Down Expand Up @@ -76,7 +77,7 @@ var _ = Describe("InfrastructureConfig validation", func() {
Fields{
"Type": Equal(field.ErrorTypeInvalid),
"Field": Equal("networks.vnet"),
"Detail": Equal("specifying an existing vnet name require a vnet name and vnet resource group"),
"Detail": Equal("a vnet cidr or vnet name and resource group need to be specified"),
}))
})

Expand All @@ -91,7 +92,7 @@ var _ = Describe("InfrastructureConfig validation", func() {
Fields{
"Type": Equal(field.ErrorTypeInvalid),
"Field": Equal("networks.vnet"),
"Detail": Equal("specifying an existing vnet name require a vnet name and vnet resource group"),
"Detail": Equal("a vnet cidr or vnet name and resource group need to be specified"),
}))
})

Expand Down Expand Up @@ -345,17 +346,54 @@ var _ = Describe("InfrastructureConfig validation", func() {
}))))
})

It("should forbid changing the network section", func() {
newInfrastructureConfig := infrastructureConfig.DeepCopy()
newCIDR := "1.2.3.4/5"
newInfrastructureConfig.Networks.VNet.CIDR = &newCIDR
Context("vnet config update", func() {
It("should allow to resize the vnet cidr", func() {
newInfrastructureConfig := infrastructureConfig.DeepCopy()
newInfrastructureConfig.Networks.VNet.CIDR = pointer.StringPtr("10.250.3.0/22")

errorList := ValidateInfrastructureConfigUpdate(infrastructureConfig, newInfrastructureConfig, fldPath)
errorList := ValidateInfrastructureConfigUpdate(infrastructureConfig, newInfrastructureConfig, fldPath)
Expect(errorList).Should(HaveLen(0))
})

It("should forbid to modify the external vnet config", func() {
infrastructureConfig.Networks.VNet.Name = pointer.StringPtr("external-vnet-name")
infrastructureConfig.Networks.VNet.ResourceGroup = pointer.StringPtr("external-vnet-rg")

newInfrastructureConfig := infrastructureConfig.DeepCopy()
newInfrastructureConfig.Networks.VNet.Name = pointer.StringPtr("modified")
newInfrastructureConfig.Networks.VNet.ResourceGroup = pointer.StringPtr("modified")

errorList := ValidateInfrastructureConfigUpdate(infrastructureConfig, newInfrastructureConfig, fldPath)
Expect(errorList).To(ConsistOfFields(Fields{
"Type": Equal(field.ErrorTypeInvalid),
"Field": Equal("networks.vnet.name"),
"Detail": Equal("field is immutable"),
}, Fields{
"Type": Equal(field.ErrorTypeInvalid),
"Field": Equal("networks.vnet.resourceGroup"),
"Detail": Equal("field is immutable"),
}))
})

It("should forbid to add external vnet config", func() {
infrastructureConfig.Networks.VNet = apisazure.VNet{}

newInfrastructureConfig := infrastructureConfig.DeepCopy()
newInfrastructureConfig.Networks.VNet.Name = pointer.StringPtr("modified")
newInfrastructureConfig.Networks.VNet.ResourceGroup = pointer.StringPtr("modified")

errorList := ValidateInfrastructureConfigUpdate(infrastructureConfig, newInfrastructureConfig, fldPath)
Expect(errorList).To(ConsistOfFields(Fields{
"Type": Equal(field.ErrorTypeInvalid),
"Field": Equal("networks.vnet.name"),
"Detail": Equal("field is immutable"),
}, Fields{
"Type": Equal(field.ErrorTypeInvalid),
"Field": Equal("networks.vnet.resourceGroup"),
"Detail": Equal("field is immutable"),
}))
})

Expect(errorList).To(ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{
"Type": Equal(field.ErrorTypeInvalid),
"Field": Equal("networks.vnet"),
}))))
})

DescribeTable("Zoned",
Expand Down
16 changes: 12 additions & 4 deletions pkg/validator/shoot_validator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package validator

import (
"errors"
"reflect"

"github.com/gardener/gardener-extension-provider-azure/pkg/apis/azure"
Expand Down Expand Up @@ -55,19 +56,26 @@ func (v *Shoot) validateShoot(shoot *core.Shoot, infraConfig *azure.Infrastructu
}

func (v *Shoot) validateShootUpdate(oldShoot, shoot *core.Shoot) error {
// InfrastructureConfig update

// Decode the new infrastructure config.
if shoot.Spec.Provider.InfrastructureConfig == nil {
return field.Required(infraConfigPath, "InfrastructureConfig must be set for Azure shoots")
}
infraConfig, err := checkAndDecodeInfrastructureConfig(v.decoder, shoot.Spec.Provider.InfrastructureConfig, infraConfigPath)
if err != nil {
return err
}

oldInfraConfig, err := checkAndDecodeInfrastructureConfig(v.decoder, shoot.Spec.Provider.InfrastructureConfig, infraConfigPath)
// Decode the old infrastructure config.
if oldShoot.Spec.Provider.InfrastructureConfig == nil {
return field.InternalError(infraConfigPath, errors.New("InfrastructureConfig is not available on old shoot"))
}
oldInfraConfig, err := checkAndDecodeInfrastructureConfig(v.decoder, oldShoot.Spec.Provider.InfrastructureConfig, infraConfigPath)
if err != nil {
return err
}

allErrs := field.ErrorList{}

var allErrs = field.ErrorList{}
if !reflect.DeepEqual(oldInfraConfig, infraConfig) {
allErrs = append(allErrs, azurevalidation.ValidateInfrastructureConfigUpdate(oldInfraConfig, infraConfig, infraConfigPath)...)
}
Expand Down

0 comments on commit 5541dfb

Please sign in to comment.