From ca9b8b23418e3f034f1b6e54172c8f6ef120ed93 Mon Sep 17 00:00:00 2001 From: Abu Kashem Date: Mon, 23 Sep 2024 12:22:53 -0400 Subject: [PATCH] api: add a new field to meta/v1 DeleteOptions - add a new boolean field IgnoreStoreReadErrorWithClusterBreakingPotential to meta/v1 DeleteOptions - add validation for the new delete option add validation for the new field in the delete options ignoreStoreReadErrorWithClusterBreakingPotential - prevent the pod eviction handler from issuing an unsafe pod delete prevent the pod eviction handler from enabling the 'ignoreStoreReadErrorWithClusterBreakingPotential' delete option Kubernetes-commit: b6773f15897dc31190b2be7cb49dd02015440465 --- pkg/apis/meta/v1/types.go | 15 +++ pkg/apis/meta/v1/validation/validation.go | 31 ++++++ .../meta/v1/validation/validation_test.go | 94 +++++++++++++++++++ 3 files changed, 140 insertions(+) diff --git a/pkg/apis/meta/v1/types.go b/pkg/apis/meta/v1/types.go index 4cea1c148..f4cc8e38b 100644 --- a/pkg/apis/meta/v1/types.go +++ b/pkg/apis/meta/v1/types.go @@ -560,6 +560,21 @@ type DeleteOptions struct { // +optional // +listType=atomic DryRun []string `json:"dryRun,omitempty" protobuf:"bytes,5,rep,name=dryRun"` + + // if set to true, it will trigger an unsafe deletion of the resource in + // case the normal deletion flow fails with a corrupt object error. + // A resource is considered corrupt if it can not be retrieved from + // the underlying storage successfully because of a) its data can + // not be transformed e.g. decryption failure, or b) it fails + // to decode into an object. + // NOTE: unsafe deletion ignores finalizer constraints, skips + // precondition checks, and removes the object from the storage. + // WARNING: This may potentially break the cluster if the workload + // associated with the resource being unsafe-deleted relies on normal + // deletion flow. Use only if you REALLY know what you are doing. + // The default value is false, and the user must opt in to enable it + // +optional + IgnoreStoreReadErrorWithClusterBreakingPotential *bool `json:"ignoreStoreReadErrorWithClusterBreakingPotential,omitempty" protobuf:"varint,6,opt,name=ignoreStoreReadErrorWithClusterBreakingPotential"` } const ( diff --git a/pkg/apis/meta/v1/validation/validation.go b/pkg/apis/meta/v1/validation/validation.go index ba61ddc8b..b1eb1bbfc 100644 --- a/pkg/apis/meta/v1/validation/validation.go +++ b/pkg/apis/meta/v1/validation/validation.go @@ -26,6 +26,8 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" + + "k8s.io/utils/ptr" ) // LabelSelectorValidationOptions is a struct that can be passed to ValidateLabelSelector to record the validate options @@ -165,6 +167,7 @@ func ValidateDeleteOptions(options *metav1.DeleteOptions) field.ErrorList { allErrs = append(allErrs, field.NotSupported(field.NewPath("propagationPolicy"), options.PropagationPolicy, []string{string(metav1.DeletePropagationForeground), string(metav1.DeletePropagationBackground), string(metav1.DeletePropagationOrphan), "nil"})) } allErrs = append(allErrs, ValidateDryRun(field.NewPath("dryRun"), options.DryRun)...) + allErrs = append(allErrs, ValidateIgnoreStoreReadError(field.NewPath("ignoreStoreReadErrorWithClusterBreakingPotential"), options)...) return allErrs } @@ -358,3 +361,31 @@ func isValidConditionReason(value string) []string { } return nil } + +// ValidateIgnoreStoreReadError validates that delete options are valid when +// ignoreStoreReadErrorWithClusterBreakingPotential is enabled +func ValidateIgnoreStoreReadError(fldPath *field.Path, options *metav1.DeleteOptions) field.ErrorList { + allErrs := field.ErrorList{} + if enabled := ptr.Deref[bool](options.IgnoreStoreReadErrorWithClusterBreakingPotential, false); !enabled { + return allErrs + } + + if len(options.DryRun) > 0 { + allErrs = append(allErrs, field.Invalid(fldPath, true, "cannot be set together with .dryRun")) + } + if options.PropagationPolicy != nil { + allErrs = append(allErrs, field.Invalid(fldPath, true, "cannot be set together with .propagationPolicy")) + } + //nolint:staticcheck // Keep validation for deprecated OrphanDependents option until it's being removed + if options.OrphanDependents != nil { + allErrs = append(allErrs, field.Invalid(fldPath, true, "cannot be set together with .orphanDependents")) + } + if options.GracePeriodSeconds != nil { + allErrs = append(allErrs, field.Invalid(fldPath, true, "cannot be set together with .gracePeriodSeconds")) + } + if options.Preconditions != nil { + allErrs = append(allErrs, field.Invalid(fldPath, true, "cannot be set together with .preconditions")) + } + + return allErrs +} diff --git a/pkg/apis/meta/v1/validation/validation_test.go b/pkg/apis/meta/v1/validation/validation_test.go index 7b16cd1f3..3b4b0040d 100644 --- a/pkg/apis/meta/v1/validation/validation_test.go +++ b/pkg/apis/meta/v1/validation/validation_test.go @@ -21,9 +21,13 @@ import ( "strings" "testing" + "github.com/google/go-cmp/cmp" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/validation/field" + + "k8s.io/utils/ptr" ) func TestValidateLabels(t *testing.T) { @@ -132,6 +136,96 @@ func boolPtr(b bool) *bool { return &b } +func TestValidateDeleteOptionsWithIgnoreStoreReadError(t *testing.T) { + fieldPath := field.NewPath("ignoreStoreReadErrorWithClusterBreakingPotential") + tests := []struct { + name string + opts metav1.DeleteOptions + expectedErrors field.ErrorList + }{ + { + name: "option is nil", + opts: metav1.DeleteOptions{ + IgnoreStoreReadErrorWithClusterBreakingPotential: nil, + DryRun: []string{"All"}, + }, + expectedErrors: field.ErrorList{}, + }, + { + name: "option is false, PropagationPolicy is set", + opts: metav1.DeleteOptions{ + IgnoreStoreReadErrorWithClusterBreakingPotential: ptr.To[bool](false), + DryRun: []string{"All"}, + PropagationPolicy: ptr.To[metav1.DeletionPropagation](metav1.DeletePropagationBackground), + GracePeriodSeconds: ptr.To[int64](0), + Preconditions: &metav1.Preconditions{}, + }, + expectedErrors: field.ErrorList{}, + }, + { + name: "option is false, OrphanDependents is set", + opts: metav1.DeleteOptions{ + IgnoreStoreReadErrorWithClusterBreakingPotential: ptr.To[bool](false), + DryRun: []string{"All"}, + //nolint:staticcheck // until it's being removed + OrphanDependents: ptr.To[bool](true), + GracePeriodSeconds: ptr.To[int64](0), + Preconditions: &metav1.Preconditions{}, + }, + expectedErrors: field.ErrorList{}, + }, + { + name: "option is true, PropagationPolicy is set", + opts: metav1.DeleteOptions{ + IgnoreStoreReadErrorWithClusterBreakingPotential: ptr.To[bool](true), + DryRun: []string{"All"}, + PropagationPolicy: ptr.To[metav1.DeletionPropagation](metav1.DeletePropagationBackground), + GracePeriodSeconds: ptr.To[int64](0), + Preconditions: &metav1.Preconditions{}, + }, + expectedErrors: field.ErrorList{ + field.Invalid(fieldPath, true, "cannot be set together with .dryRun"), + field.Invalid(fieldPath, true, "cannot be set together with .propagationPolicy"), + field.Invalid(fieldPath, true, "cannot be set together with .gracePeriodSeconds"), + field.Invalid(fieldPath, true, "cannot be set together with .preconditions"), + }, + }, + { + name: "option is true, OrphanDependents is set", + opts: metav1.DeleteOptions{ + IgnoreStoreReadErrorWithClusterBreakingPotential: ptr.To[bool](true), + DryRun: []string{"All"}, + //nolint:staticcheck // until it's being removed + OrphanDependents: ptr.To[bool](true), + GracePeriodSeconds: ptr.To[int64](0), + Preconditions: &metav1.Preconditions{}, + }, + expectedErrors: field.ErrorList{ + field.Invalid(fieldPath, true, "cannot be set together with .dryRun"), + field.Invalid(fieldPath, true, "cannot be set together with .orphanDependents"), + field.Invalid(fieldPath, true, "cannot be set together with .gracePeriodSeconds"), + field.Invalid(fieldPath, true, "cannot be set together with .preconditions"), + }, + }, + { + name: "option is true, no other option is set", + opts: metav1.DeleteOptions{ + IgnoreStoreReadErrorWithClusterBreakingPotential: ptr.To[bool](false), + }, + expectedErrors: field.ErrorList{}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + errGot := ValidateDeleteOptions(&test.opts) + if !cmp.Equal(test.expectedErrors, errGot) { + t.Errorf("expected error(s) to match, diff: %s", cmp.Diff(test.expectedErrors, errGot)) + } + }) + } +} + func TestValidPatchOptions(t *testing.T) { tests := []struct { opts metav1.PatchOptions