Skip to content

Commit

Permalink
Forbid certain rotation operations when Shoot is hibernated (garden…
Browse files Browse the repository at this point in the history 
…er#6148)

* Cleanup a few nits

* Forbid certain rotation operations when shoot is hibernated

The forbidden rotations require the shoot client since they need to interact with the kube-apiserver. However, when the shoot is hibernated the kube-apiserver is likely not available.

* Address PR review feedback
  • Loading branch information
@rfranzke
rfranzke authored and Kristiyan Gostev committed Jul 5, 2022
1 parent fc8bcfa commit 05f173c
Show file tree
Hide file tree
Showing 2 changed files with 96 additions and 35 deletions.
57 changes: 37 additions & 20 deletions pkg/apis/core/validation/shoot.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import (
"k8s.io/apimachinery/pkg/util/validation"
"k8s.io/apimachinery/pkg/util/validation/field"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/utils/pointer"
)

var (
Expand All @@ -64,22 +65,24 @@ var (
string(corev1.ServiceExternalTrafficPolicyTypeLocal),
)
availableShootOperations = sets.NewString(
string(v1beta1constants.ShootOperationMaintain),
string(v1beta1constants.ShootOperationRetry),
v1beta1constants.ShootOperationMaintain,
v1beta1constants.ShootOperationRetry,
).Union(availableShootMaintenanceOperations)
availableShootMaintenanceOperations = sets.NewString(
string(v1beta1constants.GardenerOperationReconcile),
string(v1beta1constants.ShootOperationRotateCAStart),
string(v1beta1constants.ShootOperationRotateCAComplete),
string(v1beta1constants.ShootOperationRotateCredentialsStart),
string(v1beta1constants.ShootOperationRotateCredentialsComplete),
string(v1beta1constants.ShootOperationRotateETCDEncryptionKeyStart),
string(v1beta1constants.ShootOperationRotateETCDEncryptionKeyComplete),
string(v1beta1constants.ShootOperationRotateKubeconfigCredentials),
string(v1beta1constants.ShootOperationRotateObservabilityCredentials),
string(v1beta1constants.ShootOperationRotateSSHKeypair),
string(v1beta1constants.ShootOperationRotateServiceAccountKeyStart),
string(v1beta1constants.ShootOperationRotateServiceAccountKeyComplete),
v1beta1constants.GardenerOperationReconcile,
v1beta1constants.ShootOperationRotateCAStart,
v1beta1constants.ShootOperationRotateCAComplete,
v1beta1constants.ShootOperationRotateKubeconfigCredentials,
v1beta1constants.ShootOperationRotateObservabilityCredentials,
v1beta1constants.ShootOperationRotateSSHKeypair,
).Union(forbiddenShootOperationsWhenHibernated)
forbiddenShootOperationsWhenHibernated = sets.NewString(
v1beta1constants.ShootOperationRotateCredentialsStart,
v1beta1constants.ShootOperationRotateCredentialsComplete,
v1beta1constants.ShootOperationRotateETCDEncryptionKeyStart,
v1beta1constants.ShootOperationRotateETCDEncryptionKeyComplete,
v1beta1constants.ShootOperationRotateServiceAccountKeyStart,
v1beta1constants.ShootOperationRotateServiceAccountKeyComplete,
)
availableShootPurposes = sets.NewString(
string(core.ShootPurposeEvaluation),
Expand Down Expand Up @@ -204,7 +207,7 @@ func ValidateShootSpec(meta metav1.ObjectMeta, spec *core.ShootSpec, fldPath *fi
allErrs = append(allErrs, validateNetworking(spec.Networking, fldPath.Child("networking"))...)
allErrs = append(allErrs, validateMaintenance(spec.Maintenance, fldPath.Child("maintenance"))...)
allErrs = append(allErrs, validateMonitoring(spec.Monitoring, fldPath.Child("monitoring"))...)
allErrs = append(allErrs, ValidateHibernation(spec.Hibernation, fldPath.Child("hibernation"))...)
allErrs = append(allErrs, ValidateHibernation(meta.Annotations, spec.Hibernation, fldPath.Child("hibernation"))...)
allErrs = append(allErrs, validateProvider(spec.Provider, spec.Kubernetes, fldPath.Child("provider"), inTemplate)...)

if len(spec.Region) == 0 {
Expand Down Expand Up @@ -1506,13 +1509,17 @@ func ValidateWorkers(workers []core.Worker, fldPath *field.Path) field.ErrorList
}

// ValidateHibernation validates a Hibernation object.
func ValidateHibernation(hibernation *core.Hibernation, fldPath *field.Path) field.ErrorList {
func ValidateHibernation(annotations map[string]string, hibernation *core.Hibernation, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}

if hibernation == nil {
return allErrs
}

if maintenanceOp := annotations[v1beta1constants.GardenerMaintenanceOperation]; forbiddenShootOperationsWhenHibernated.Has(maintenanceOp) && pointer.BoolDeref(hibernation.Enabled, false) {
allErrs = append(allErrs, field.Forbidden(fldPath.Child("enabled"), fmt.Sprintf("shoot cannot be hibernated when %s=%s annotation is set", v1beta1constants.GardenerMaintenanceOperation, maintenanceOp)))
}

allErrs = append(allErrs, ValidateHibernationSchedules(hibernation.Schedules, fldPath.Child("schedules"))...)

return allErrs
Expand Down Expand Up @@ -1737,12 +1744,22 @@ func validateShootOperation(operation, maintenanceOperation string, shoot *core.
allErrs = append(allErrs, field.Forbidden(fldPath, fmt.Sprintf("annotations %s and %s must not be equal", fldPathOp, fldPathMaintOp)))
}

if operation != "" && !availableShootOperations.Has(string(operation)) {
allErrs = append(allErrs, field.NotSupported(fldPathOp, operation, availableShootOperations.List()))
if operation != "" {
if !availableShootOperations.Has(operation) {
allErrs = append(allErrs, field.NotSupported(fldPathOp, operation, availableShootOperations.List()))
}
if helper.HibernationIsEnabled(shoot) && forbiddenShootOperationsWhenHibernated.Has(operation) {
allErrs = append(allErrs, field.Forbidden(fldPathOp, "operation is not permitted when shoot is hibernated"))
}
}

if maintenanceOperation != "" && !availableShootMaintenanceOperations.Has(string(maintenanceOperation)) {
allErrs = append(allErrs, field.NotSupported(fldPathMaintOp, maintenanceOperation, availableShootMaintenanceOperations.List()))
if maintenanceOperation != "" {
if !availableShootMaintenanceOperations.Has(maintenanceOperation) {
allErrs = append(allErrs, field.NotSupported(fldPathMaintOp, maintenanceOperation, availableShootMaintenanceOperations.List()))
}
if helper.HibernationIsEnabled(shoot) && forbiddenShootOperationsWhenHibernated.Has(maintenanceOperation) {
allErrs = append(allErrs, field.Forbidden(fldPathMaintOp, "operation is not permitted when shoot is hibernated"))
}
}

allErrs = append(allErrs, validateShootOperationContext(operation, shoot, fldPathOp)...)
Expand Down
74 changes: 59 additions & 15 deletions pkg/apis/core/validation/shoot_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3501,22 +3501,20 @@ var _ = Describe("Shoot Validation Tests", func() {
}),
)

It("should return an error if the reconciliation operation annotation is invalid", func() {
It("should return an error if the operation annotation is invalid", func() {
metav1.SetMetaDataAnnotation(&shoot.ObjectMeta, "gardener.cloud/operation", "foo-bar")
matcher := ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{
Expect(ValidateShoot(shoot)).To(ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{
"Type": Equal(field.ErrorTypeNotSupported),
"Field": Equal("metadata.annotations[gardener.cloud/operation]"),
})))
Expect(ValidateShoot(shoot)).To(matcher)
}))))
})

It("should return an error if the maintenance operation annotation is invalid", func() {
metav1.SetMetaDataAnnotation(&shoot.ObjectMeta, "maintenance.gardener.cloud/operation", "foo-bar")
matcher := ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{
Expect(ValidateShoot(shoot)).To(ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{
"Type": Equal(field.ErrorTypeNotSupported),
"Field": Equal("metadata.annotations[maintenance.gardener.cloud/operation]"),
})))
Expect(ValidateShoot(shoot)).To(matcher)
}))))
})

It("should return an error if maintenance annotation is not allowed in this context", func() {
Expand All @@ -3527,14 +3525,13 @@ var _ = Describe("Shoot Validation Tests", func() {
State: core.LastOperationStateSucceeded,
},
}
matcher := ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{
Expect(ValidateShoot(shoot)).To(ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{
"Type": Equal(field.ErrorTypeForbidden),
"Field": Equal("metadata.annotations[maintenance.gardener.cloud/operation]"),
})))
Expect(ValidateShoot(shoot)).To(matcher)
}))))
})

It("should return an error if both annotation have the same value", func() {
It("should return an error if both operation annotations have the same value", func() {
metav1.SetMetaDataAnnotation(&shoot.ObjectMeta, "gardener.cloud/operation", "rotate-etcd-encryption-key-start")
metav1.SetMetaDataAnnotation(&shoot.ObjectMeta, "maintenance.gardener.cloud/operation", "rotate-etcd-encryption-key-start")
shoot.Status = core.ShootStatus{
Expand All @@ -3543,19 +3540,18 @@ var _ = Describe("Shoot Validation Tests", func() {
State: core.LastOperationStateSucceeded,
},
}
matcher := ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{
Expect(ValidateShoot(shoot)).To(ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{
"Type": Equal(field.ErrorTypeForbidden),
"Field": Equal("metadata.annotations"),
})))
Expect(ValidateShoot(shoot)).To(matcher)
}))))
})

It("should return nothing if maintenance annotation is valid", func() {
metav1.SetMetaDataAnnotation(&shoot.ObjectMeta, "maintenance.gardener.cloud/operation", "reconcile")
Expect(ValidateShoot(shoot)).To(BeEmpty())
})

It("should return nothing if both annotations are valid and do not have the same value", func() {
It("should return nothing if both operation annotations are valid and do not have the same value", func() {
metav1.SetMetaDataAnnotation(&shoot.ObjectMeta, "gardener.cloud/operation", "rotate-serviceaccount-key-start")
metav1.SetMetaDataAnnotation(&shoot.ObjectMeta, "maintenance.gardener.cloud/operation", "rotate-etcd-encryption-key-start")
shoot.Status = core.ShootStatus{
Expand All @@ -3567,6 +3563,54 @@ var _ = Describe("Shoot Validation Tests", func() {
Expect(ValidateShoot(shoot)).To(BeEmpty())
})

DescribeTable("forbid certain rotation operations when shoot is hibernated",
func(operation string) {
shoot.Spec.Hibernation = &core.Hibernation{Enabled: pointer.Bool(true)}

metav1.SetMetaDataAnnotation(&shoot.ObjectMeta, "gardener.cloud/operation", operation)
Expect(ValidateShoot(shoot)).To(ContainElement(PointTo(MatchFields(IgnoreExtras, Fields{
"Type": Equal(field.ErrorTypeForbidden),
"Field": Equal("metadata.annotations[gardener.cloud/operation]"),
"Detail": ContainSubstring("operation is not permitted when shoot is hibernated"),
}))))
delete(shoot.Annotations, "gardener.cloud/operation")

metav1.SetMetaDataAnnotation(&shoot.ObjectMeta, "maintenance.gardener.cloud/operation", operation)
Expect(ValidateShoot(shoot)).To(ContainElement(PointTo(MatchFields(IgnoreExtras, Fields{
"Type": Equal(field.ErrorTypeForbidden),
"Field": Equal("metadata.annotations[maintenance.gardener.cloud/operation]"),
"Detail": ContainSubstring("operation is not permitted when shoot is hibernated"),
}))))
delete(shoot.Annotations, "maintenance.gardener.cloud/operation")
},

Entry("rotate-credentials-start", "rotate-credentials-start"),
Entry("rotate-credentials-complete", "rotate-credentials-complete"),
Entry("rotate-etcd-encryption-key-start", "rotate-etcd-encryption-key-start"),
Entry("rotate-etcd-encryption-key-complete", "rotate-etcd-encryption-key-complete"),
Entry("rotate-serviceaccount-key-start", "rotate-serviceaccount-key-start"),
Entry("rotate-serviceaccount-key-complete", "rotate-serviceaccount-key-complete"),
)

DescribeTable("forbid hibernating the shoot when certain rotation maintenance operations are set",
func(operation string) {
metav1.SetMetaDataAnnotation(&shoot.ObjectMeta, "maintenance.gardener.cloud/operation", operation)
shoot.Spec.Hibernation = &core.Hibernation{Enabled: pointer.Bool(true)}

Expect(ValidateShoot(shoot)).To(ContainElement(PointTo(MatchFields(IgnoreExtras, Fields{
"Type": Equal(field.ErrorTypeForbidden),
"Field": Equal("spec.hibernation.enabled"),
"Detail": ContainSubstring("shoot cannot be hibernated when maintenance.gardener.cloud/operation=" + operation + " annotation is set"),
}))))
},

Entry("rotate-credentials-start", "rotate-credentials-start"),
Entry("rotate-credentials-complete", "rotate-credentials-complete"),
Entry("rotate-etcd-encryption-key-start", "rotate-etcd-encryption-key-start"),
Entry("rotate-etcd-encryption-key-complete", "rotate-etcd-encryption-key-complete"),
Entry("rotate-serviceaccount-key-start", "rotate-serviceaccount-key-start"),
Entry("rotate-serviceaccount-key-complete", "rotate-serviceaccount-key-complete"),
)
})
})

Expand Down

0 comments on commit 05f173c

Please sign in to comment.