Skip to content

Commit

Permalink
[GEP-20] Allow adding HA alpha annotation where none existed (#6533)
Browse files Browse the repository at this point in the history 
* fix for Issue #6532
added validation for HA shoots in admission plugin
Apply suggestions from code review

Co-authored-by: Rafael Franzke <rafael.franzke@sap.com>

* Enable PeerTLS only for HA clusters

Co-authored-by: Rafael Franzke <rafael.franzke@sap.com>
Co-authored-by: Tim Usner <tim.usner@sap.com>
  • Loading branch information
@unmarshall @rfranzke @timuthy
3 people committed Sep 21, 2022
1 parent 80d8db9 commit cb5947f
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 48 deletions.
57 changes: 39 additions & 18 deletions pkg/apis/core/validation/shoot.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ func ValidateShootUpdate(newShoot, oldShoot *core.Shoot) field.ErrorList {
allErrs = append(allErrs, ValidateShootObjectMetaUpdate(newShoot.ObjectMeta, oldShoot.ObjectMeta, field.NewPath("metadata"))...)
allErrs = append(allErrs, ValidateShootSpecUpdate(&newShoot.Spec, &oldShoot.Spec, newShoot.ObjectMeta, field.NewPath("spec"))...)
allErrs = append(allErrs, ValidateShoot(newShoot)...)
allErrs = append(allErrs, ValidateShootHAControlPlaneUpdate(newShoot, oldShoot)...)
allErrs = append(allErrs, ValidateShootHAConfigUpdate(newShoot, oldShoot)...)

return allErrs
}
Expand Down Expand Up @@ -1936,20 +1936,20 @@ func ValidateShootHAConfig(shoot *core.Shoot) field.ErrorList {
return allErrs
}

// ValidateShootHAControlPlaneUpdate validates the HA shoot control plane configuration
func ValidateShootHAControlPlaneUpdate(newShoot, oldSnoot *core.Shoot) field.ErrorList {
// ValidateShootHAConfigUpdate validates the HA shoot control plane configuration
func ValidateShootHAConfigUpdate(newShoot, oldSnoot *core.Shoot) field.ErrorList {
allErrs := field.ErrorList{}

// check if there has been a switch from HA annotation to HA Spec failureTolerance and validate if the new value is semantically equivalent to the older value
// since we do not allow changing the failure tolerance once it is set.
hasSwitched, errs := validateSwitchedFromHAAnnotationToSpecFailureTolerance(newShoot, oldSnoot, field.NewPath("metadata"))
hasSwitched, errs := validateSwitchBetweenHAAnnotationToSpecFailureTolerance(newShoot, oldSnoot)
if hasSwitched {
return append(allErrs, errs...)
}
// validate HA annotation if one exists and collect errors if any
allErrs = append(allErrs, validateShootHAControlPlaneAnnotationUpdate(newShoot, oldSnoot, field.NewPath("metadata"))...)
// validate HA ControlPlane Spec and collect errors if any
allErrs = append(allErrs, validateShootHAControlPlaneUpdate(&newShoot.Spec, &oldSnoot.Spec, field.NewPath("spec.controlPlane"))...)
allErrs = append(allErrs, validateShootHAControlPlaneSpecUpdate(&newShoot.Spec, &oldSnoot.Spec, field.NewPath("spec.controlPlane"))...)

return allErrs
}
Expand Down Expand Up @@ -1977,31 +1977,45 @@ func bothHAAnnotationAndHASpecSet(shoot *core.Shoot) bool {

func validateShootHAControlPlaneAnnotationUpdate(newShoot, oldShoot *core.Shoot, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
oldVal := oldShoot.Annotations[v1beta1constants.ShootAlphaControlPlaneHighAvailability]
oldVal, oldExists := oldShoot.Annotations[v1beta1constants.ShootAlphaControlPlaneHighAvailability]
newVal := newShoot.Annotations[v1beta1constants.ShootAlphaControlPlaneHighAvailability]

// It is not yet allowed to upgrade from non-HA to HA or to upgrade from single-zone to multi-zone.
allErrs = append(allErrs, apivalidation.ValidateImmutableField(newVal, oldVal, fldPath.Child("annotations").Key(v1beta1constants.ShootAlphaControlPlaneHighAvailability))...)
// If there is no annotation set
if oldExists {
allErrs = append(allErrs, apivalidation.ValidateImmutableField(newVal, oldVal, fldPath.Child("annotations").Key(v1beta1constants.ShootAlphaControlPlaneHighAvailability))...)
}

return allErrs
}

func validateSwitchedFromHAAnnotationToSpecFailureTolerance(newShoot, oldShoot *core.Shoot, fldPath *field.Path) (bool, field.ErrorList) {
func validateSwitchBetweenHAAnnotationToSpecFailureTolerance(newShoot, oldShoot *core.Shoot) (bool, field.ErrorList) {
allErrs := field.ErrorList{}

oldVal, oldExists := oldShoot.Annotations[v1beta1constants.ShootAlphaControlPlaneHighAvailability]
_, newExists := newShoot.Annotations[v1beta1constants.ShootAlphaControlPlaneHighAvailability]
oldHASpecExists := oldShoot.Spec.ControlPlane != nil && oldShoot.Spec.ControlPlane.HighAvailability != nil
oldAnnotationVal, oldAnnotationExists := oldShoot.Annotations[v1beta1constants.ShootAlphaControlPlaneHighAvailability]
_, newAnnotationExists := newShoot.Annotations[v1beta1constants.ShootAlphaControlPlaneHighAvailability]

// switching from HA spec.ControlPlane.highAvailability.failureTolerance.type to HA alpha annotation is disallowed
if oldHASpecExists && newAnnotationExists {
allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "controlPlane", "highAvailability", "failureTolerance", "type"),
oldShoot.Spec.ControlPlane.HighAvailability.FailureTolerance.Type, "switching from spec.controlPlane.highAvailability.failureTolerance.type to HA annotation is not allowed"))
return true, allErrs
}

if oldExists && !newExists {
// switching
if oldAnnotationExists && !newAnnotationExists {
//Check if switch has been made from annotation to Spec for HA shoot control plane
if newShoot.Spec.ControlPlane != nil && newShoot.Spec.ControlPlane.HighAvailability != nil {
// Check if the new value in the spec is exactly the same as the old value in the annotation
failureTolerance := newShoot.Spec.ControlPlane.HighAvailability.FailureTolerance.Type
if !isHAAnnotationValSemanticallyEqualToFailureToleranceType(oldVal, failureTolerance) {
allErrs = append(allErrs, field.Invalid(fldPath.Child("annotations").Key(v1beta1constants.ShootAlphaControlPlaneHighAvailability), oldVal, "Invalid switch from HA annotation to HA spec.controlPlane.highAvailability.type, cannot change failure tolerance"))
if !isHAAnnotationValSemanticallyEqualToFailureToleranceType(oldAnnotationVal, failureTolerance) {
allErrs = append(allErrs, field.Forbidden(field.NewPath("metadata", "annotations").Key(v1beta1constants.ShootAlphaControlPlaneHighAvailability),
"forbidden switch from HA annotation to spec.controlPlane.highAvailability.failureTolerance.type, cannot change failure tolerance"))
}
return true, allErrs
}
}

return false, nil
}

Expand All @@ -2015,20 +2029,27 @@ func isHAAnnotationValSemanticallyEqualToFailureToleranceType(annotationValue st
return false
}

func validateShootHAControlPlaneUpdate(newSpec, oldSpec *core.ShootSpec, fldPath *field.Path) field.ErrorList {
func validateShootHAControlPlaneSpecUpdate(newSpec, oldSpec *core.ShootSpec, fldPath *field.Path) field.ErrorList {
var (
oldVal, newVal core.FailureToleranceType
oldValExists bool
)
allErrs := field.ErrorList{}
var oldVal, newVal core.FailureToleranceType

if oldSpec.ControlPlane != nil && oldSpec.ControlPlane.HighAvailability != nil {
oldVal = oldSpec.ControlPlane.HighAvailability.FailureTolerance.Type
oldValExists = true
}

if newSpec.ControlPlane != nil && newSpec.ControlPlane.HighAvailability != nil {
newVal = newSpec.ControlPlane.HighAvailability.FailureTolerance.Type
}

// If the HighAvailability field is already set for the shoot then enforce that it cannot be changed
allErrs = append(allErrs, apivalidation.ValidateImmutableField(newVal, oldVal, fldPath.Child("highAvailability", "failureTolerance", "type"))...)
if oldValExists {
// If the HighAvailability field is already set for the shoot then enforce that it cannot be changed
allErrs = append(allErrs, apivalidation.ValidateImmutableField(newVal, oldVal, fldPath.Child("highAvailability", "failureTolerance", "type"))...)
}

return allErrs
}

Expand Down
48 changes: 18 additions & 30 deletions pkg/apis/core/validation/shoot_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ var _ = Describe("Shoot Validation Tests", func() {
v1beta1constants.ShootAlphaControlPlaneHighAvailability: v1beta1constants.ShootAlphaControlPlaneHighAvailabilityMultiZone,
}
newShoot := prepareShootForUpdate(shoot)
errorList := ValidateShootHAControlPlaneUpdate(newShoot, shoot)
errorList := ValidateShootHAConfigUpdate(newShoot, shoot)
Expect(errorList).To(HaveLen(0))
})

Expand All @@ -375,7 +375,7 @@ var _ = Describe("Shoot Validation Tests", func() {
v1beta1constants.ShootAlphaControlPlaneHighAvailability: v1beta1constants.ShootAlphaControlPlaneHighAvailabilitySingleZone,
}

errorList := ValidateShootHAControlPlaneUpdate(newShoot, shoot)
errorList := ValidateShootHAConfigUpdate(newShoot, shoot)
Expect(errorList).To(ConsistOf(
PointTo(MatchFields(IgnoreExtras, Fields{
"Type": Equal(field.ErrorTypeInvalid),
Expand All @@ -394,7 +394,7 @@ var _ = Describe("Shoot Validation Tests", func() {
"foo": "bar",
}

errorList := ValidateShootHAControlPlaneUpdate(newShoot, shoot)
errorList := ValidateShootHAConfigUpdate(newShoot, shoot)

Expect(errorList).To(ConsistOf(
PointTo(MatchFields(IgnoreExtras, Fields{
Expand All @@ -407,13 +407,13 @@ var _ = Describe("Shoot Validation Tests", func() {
It("should pass as Shoot ControlPlane Spec with HA set to zone has not changed", func() {
shoot.Spec.ControlPlane = &core.ControlPlane{HighAvailability: &core.HighAvailability{FailureTolerance: core.FailureTolerance{Type: core.FailureToleranceTypeZone}}}
newShoot := prepareShootForUpdate(shoot)
errorList := ValidateShootHAControlPlaneUpdate(newShoot, shoot)
errorList := ValidateShootHAConfigUpdate(newShoot, shoot)
Expect(errorList).To(HaveLen(0))
})

It("should pass as non-HA Shoot ControlPlane Spec has not changed", func() {
newShoot := prepareShootForUpdate(shoot)
errorList := ValidateShootHAControlPlaneUpdate(newShoot, shoot)
errorList := ValidateShootHAConfigUpdate(newShoot, shoot)
Expect(errorList).To(HaveLen(0))
})

Expand All @@ -422,7 +422,7 @@ var _ = Describe("Shoot Validation Tests", func() {
newShoot := prepareShootForUpdate(shoot)
newShoot.Spec.ControlPlane = &core.ControlPlane{HighAvailability: &core.HighAvailability{FailureTolerance: core.FailureTolerance{Type: core.FailureToleranceTypeNode}}}

errorList := ValidateShootHAControlPlaneUpdate(newShoot, shoot)
errorList := ValidateShootHAConfigUpdate(newShoot, shoot)
Expect(errorList).To(ConsistOf(
PointTo(MatchFields(IgnoreExtras, Fields{
"Type": Equal(field.ErrorTypeInvalid),
Expand All @@ -437,7 +437,7 @@ var _ = Describe("Shoot Validation Tests", func() {
newShoot := prepareShootForUpdate(shoot)
newShoot.Spec.ControlPlane = nil

errorList := ValidateShootHAControlPlaneUpdate(newShoot, shoot)
errorList := ValidateShootHAConfigUpdate(newShoot, shoot)

Expect(errorList).To(ConsistOf(
PointTo(MatchFields(IgnoreExtras, Fields{
Expand All @@ -447,34 +447,22 @@ var _ = Describe("Shoot Validation Tests", func() {
))
})

It("should forbid upgrading from non-HA to HA Shoot using annotations", func() {
It("should allow upgrading from non-HA to HA Shoot using annotations", func() {
shoot.Spec.ControlPlane = &core.ControlPlane{}
newShoot := prepareShootForUpdate(shoot)
newShoot.Annotations = map[string]string{
v1beta1constants.ShootAlphaControlPlaneHighAvailability: v1beta1constants.ShootAlphaControlPlaneHighAvailabilityMultiZone,
}
errorList := ValidateShootHAControlPlaneUpdate(newShoot, shoot)
Expect(errorList).To(ConsistOf(
PointTo(MatchFields(IgnoreExtras, Fields{
"Type": Equal(field.ErrorTypeInvalid),
"Field": Equal("metadata.annotations[alpha.control-plane.shoot.gardener.cloud/high-availability]"),
"BadValue": Equal("multi-zone"),
})),
))
errorList := ValidateShootHAConfigUpdate(newShoot, shoot)
Expect(errorList).To(HaveLen(0))
})

It("should forbid upgrading from non-HA to HA Shoot ControlPlane.HighAvailability Spec", func() {
It("should allow upgrading from non-HA to HA Shoot ControlPlane.HighAvailability Spec", func() {
shoot.Spec.ControlPlane = &core.ControlPlane{}
newShoot := prepareShootForUpdate(shoot)
newShoot.Spec.ControlPlane = &core.ControlPlane{HighAvailability: &core.HighAvailability{FailureTolerance: core.FailureTolerance{Type: core.FailureToleranceTypeZone}}}
errorList := ValidateShootHAControlPlaneUpdate(newShoot, shoot)
Expect(errorList).To(ConsistOf(
PointTo(MatchFields(IgnoreExtras, Fields{
"Type": Equal(field.ErrorTypeInvalid),
"Field": Equal("spec.controlPlane.highAvailability.failureTolerance.type"),
"BadValue": Equal(core.FailureToleranceTypeZone),
})),
))
errorList := ValidateShootHAConfigUpdate(newShoot, shoot)
Expect(errorList).To(HaveLen(0))
})

It("should allow switching from HA annotation to semantically equivalent failureTolerance in HA Spec", func() {
Expand All @@ -483,7 +471,7 @@ var _ = Describe("Shoot Validation Tests", func() {
v1beta1constants.ShootAlphaControlPlaneHighAvailability: v1beta1constants.ShootAlphaControlPlaneHighAvailabilityMultiZone,
}
newShoot.Spec.ControlPlane = &core.ControlPlane{HighAvailability: &core.HighAvailability{FailureTolerance: core.FailureTolerance{Type: core.FailureToleranceTypeZone}}}
errorList := ValidateShootHAControlPlaneUpdate(newShoot, shoot)
errorList := ValidateShootHAConfigUpdate(newShoot, shoot)
Expect(errorList).To(HaveLen(0))
})

Expand All @@ -493,10 +481,10 @@ var _ = Describe("Shoot Validation Tests", func() {
v1beta1constants.ShootAlphaControlPlaneHighAvailability: v1beta1constants.ShootAlphaControlPlaneHighAvailabilitySingleZone,
}
newShoot.Spec.ControlPlane = &core.ControlPlane{HighAvailability: &core.HighAvailability{FailureTolerance: core.FailureTolerance{Type: core.FailureToleranceTypeZone}}}
errorList := ValidateShootHAControlPlaneUpdate(newShoot, shoot)
errorList := ValidateShootHAConfigUpdate(newShoot, shoot)
Expect(errorList).To(ConsistOf(
PointTo(MatchFields(IgnoreExtras, Fields{
"Type": Equal(field.ErrorTypeInvalid),
"Type": Equal(field.ErrorTypeForbidden),
"Field": Equal("metadata.annotations[alpha.control-plane.shoot.gardener.cloud/high-availability]"),
})),
))
Expand All @@ -508,8 +496,8 @@ var _ = Describe("Shoot Validation Tests", func() {
v1beta1constants.ShootAlphaControlPlaneHighAvailability: v1beta1constants.ShootAlphaControlPlaneHighAvailabilitySingleZone,
}
shoot.Spec.ControlPlane = &core.ControlPlane{HighAvailability: &core.HighAvailability{FailureTolerance: core.FailureTolerance{Type: core.FailureToleranceTypeZone}}}
errorList := ValidateShootHAControlPlaneUpdate(newShoot, shoot)
Expect(errorList).To(HaveLen(2))
errorList := ValidateShootHAConfigUpdate(newShoot, shoot)
Expect(errorList).To(HaveLen(1))
})
})

Expand Down
2 changes: 2 additions & 0 deletions pkg/operation/botanist/component/etcd/etcd.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,7 @@ func (e *etcd) Deploy(ctx context.Context) error {
Quota: &quota,
}

// TODO(timuthy): Once https://github.com/gardener/etcd-backup-restore/issues/538 is resolved we can enable PeerUrlTLS for all remaining clusters as well.
if e.hasHAControlPlane() {
e.etcd.Spec.Etcd.PeerUrlTLS = &druidv1alpha1.TLSConfig{
TLSCASecretRef: druidv1alpha1.SecretReference{
Expand Down Expand Up @@ -982,6 +983,7 @@ func (e *etcd) computeFullSnapshotSchedule(existingEtcd *druidv1alpha1.Etcd) *st
}

func (e *etcd) handlePeerCertificates(ctx context.Context) (caSecretName, peerSecretName string, err error) {
// TODO(timuthy): Remove this once https://github.com/gardener/etcd-backup-restore/issues/538 is resolved.
if !e.hasHAControlPlane() {
return
}
Expand Down

0 comments on commit cb5947f

Please sign in to comment.