diff --git a/pkg/endpoints/installer.go b/pkg/endpoints/installer.go index f39f4619b..db4a72155 100644 --- a/pkg/endpoints/installer.go +++ b/pkg/endpoints/installer.go @@ -1057,6 +1057,8 @@ func typeToJSON(typeName string) string { return "string" case "byte", "*byte": return "string" + case "v1.DeletionPropagation", "*v1.DeletionPropagation": + return "string" // TODO: Fix these when go-restful supports a way to specify an array query param: // https://github.com/emicklei/go-restful/issues/225 diff --git a/pkg/registry/generic/registry/store.go b/pkg/registry/generic/registry/store.go index 5977613ed..3f8f8d566 100644 --- a/pkg/registry/generic/registry/store.go +++ b/pkg/registry/generic/registry/store.go @@ -531,52 +531,121 @@ var ( errEmptiedFinalizers = fmt.Errorf("emptied finalizers") ) -// shouldUpdateFinalizers returns if we need to update the finalizers of the -// object, and the desired list of finalizers. When deciding whether to add -// the OrphanDependent finalizer, factors in the order of highest to lowest -// priority are: -// -// - options.OrphanDependents, -// - existing finalizers of the object -// - e.DeleteStrategy.DefaultGarbageCollectionPolicy -func shouldUpdateFinalizers(e *Store, accessor metav1.Object, options *metav1.DeleteOptions) (shouldUpdate bool, newFinalizers []string) { - shouldOrphan := false +// shouldUpdateFinalizerOrphanDependents returns if the finalizers need to be +// updated for FinalizerOrphanDependents. In the order of highest to lowest +// priority, there are three factors affect whether to add/remove the +// FinalizerOrphanDependents: options, existing finalizers of the object, +// and e.DeleteStrategy.DefaultGarbageCollectionPolicy. +func shouldUpdateFinalizerOrphanDependents(e *Store, accessor metav1.Object, options *metav1.DeleteOptions) (shouldUpdate bool, shouldOrphan bool) { + shouldOrphan = false // Get default orphan policy from this REST object type if gcStrategy, ok := e.DeleteStrategy.(rest.GarbageCollectionDeleteStrategy); ok { if gcStrategy.DefaultGarbageCollectionPolicy() == rest.OrphanDependents { shouldOrphan = true } } + // If a finalizer is set in the object, it overrides the default hasOrphanFinalizer := false finalizers := accessor.GetFinalizers() for _, f := range finalizers { - if f == metav1.FinalizerOrphan { + // validation should make sure the two cases won't be true at the same + // time. + switch f { + case metav1.FinalizerOrphanDependents: shouldOrphan = true hasOrphanFinalizer = true break + case metav1.FinalizerDeleteDependents: + shouldOrphan = false + break } - // TODO: update this when we add a finalizer indicating a preference for the other behavior } + // If an explicit policy was set at deletion time, that overrides both if options != nil && options.OrphanDependents != nil { shouldOrphan = *options.OrphanDependents } - if shouldOrphan && !hasOrphanFinalizer { - finalizers = append(finalizers, metav1.FinalizerOrphan) - return true, finalizers - } - if !shouldOrphan && hasOrphanFinalizer { - var newFinalizers []string - for _, f := range finalizers { - if f == metav1.FinalizerOrphan { - continue - } - newFinalizers = append(newFinalizers, f) + if options != nil && options.PropagationPolicy != nil { + switch *options.PropagationPolicy { + case metav1.DeletePropagationOrphan: + shouldOrphan = true + case metav1.DeletePropagationBackground, metav1.DeletePropagationForeground: + shouldOrphan = false } - return true, newFinalizers } - return false, finalizers + + shouldUpdate = shouldOrphan != hasOrphanFinalizer + return shouldUpdate, shouldOrphan +} + +// shouldUpdateFinalizerDeleteDependents returns if the finalizers need to be +// updated for FinalizerDeleteDependents. In the order of highest to lowest +// priority, there are three factors affect whether to add/remove the +// FinalizerDeleteDependents: options, existing finalizers of the object, and +// e.DeleteStrategy.DefaultGarbageCollectionPolicy. +func shouldUpdateFinalizerDeleteDependents(e *Store, accessor metav1.Object, options *metav1.DeleteOptions) (shouldUpdate bool, shouldDeleteDependentInForeground bool) { + // default to false + shouldDeleteDependentInForeground = false + + // If a finalizer is set in the object, it overrides the default + hasFinalizerDeleteDependents := false + finalizers := accessor.GetFinalizers() + for _, f := range finalizers { + // validation has made sure the two cases won't be true at the same + // time. + switch f { + case metav1.FinalizerDeleteDependents: + shouldDeleteDependentInForeground = true + hasFinalizerDeleteDependents = true + break + case metav1.FinalizerOrphanDependents: + shouldDeleteDependentInForeground = false + break + } + } + + // If an explicit policy was set at deletion time, that overrides both + if options != nil && options.OrphanDependents != nil { + shouldDeleteDependentInForeground = false + } + if options != nil && options.PropagationPolicy != nil { + switch *options.PropagationPolicy { + case metav1.DeletePropagationForeground: + shouldDeleteDependentInForeground = true + case metav1.DeletePropagationBackground, metav1.DeletePropagationOrphan: + shouldDeleteDependentInForeground = false + } + } + + shouldUpdate = shouldDeleteDependentInForeground != hasFinalizerDeleteDependents + return shouldUpdate, shouldDeleteDependentInForeground +} + +// shouldUpdateFinalizers returns if we need to update the finalizers of the +// object, and the desired list of finalizers. +func shouldUpdateFinalizers(e *Store, accessor metav1.Object, options *metav1.DeleteOptions) (shouldUpdate bool, newFinalizers []string) { + shouldUpdate1, shouldOrphan := shouldUpdateFinalizerOrphanDependents(e, accessor, options) + shouldUpdate2, shouldDeleteDependentInForeground := shouldUpdateFinalizerDeleteDependents(e, accessor, options) + oldFinalizers := accessor.GetFinalizers() + if !shouldUpdate1 && !shouldUpdate2 { + return false, oldFinalizers + } + + // first remove both finalizers, add them back if needed. + for _, f := range oldFinalizers { + if f == metav1.FinalizerOrphanDependents || f == metav1.FinalizerDeleteDependents { + continue + } + newFinalizers = append(newFinalizers, f) + } + if shouldOrphan { + newFinalizers = append(newFinalizers, metav1.FinalizerOrphanDependents) + } + if shouldDeleteDependentInForeground { + newFinalizers = append(newFinalizers, metav1.FinalizerDeleteDependents) + } + return true, newFinalizers } // markAsDeleting sets the obj's DeletionGracePeriodSeconds to 0, and sets the diff --git a/pkg/registry/generic/registry/store_test.go b/pkg/registry/generic/registry/store_test.go index 5fa68c677..5f92a4a4b 100644 --- a/pkg/registry/generic/registry/store_test.go +++ b/pkg/registry/generic/registry/store_test.go @@ -818,7 +818,7 @@ func TestStoreDeleteWithOrphanDependents(t *testing.T) { initialGeneration := int64(1) podWithOrphanFinalizer := func(name string) *example.Pod { return &example.Pod{ - ObjectMeta: metav1.ObjectMeta{Name: name, Finalizers: []string{"foo.com/x", metav1.FinalizerOrphan, "bar.com/y"}, Generation: initialGeneration}, + ObjectMeta: metav1.ObjectMeta{Name: name, Finalizers: []string{"foo.com/x", metav1.FinalizerOrphanDependents, "bar.com/y"}, Generation: initialGeneration}, Spec: example.PodSpec{NodeName: "machine"}, } } @@ -836,7 +836,7 @@ func TestStoreDeleteWithOrphanDependents(t *testing.T) { } podWithOnlyOrphanFinalizer := func(name string) *example.Pod { return &example.Pod{ - ObjectMeta: metav1.ObjectMeta{Name: name, Finalizers: []string{metav1.FinalizerOrphan}, Generation: initialGeneration}, + ObjectMeta: metav1.ObjectMeta{Name: name, Finalizers: []string{metav1.FinalizerOrphanDependents}, Generation: initialGeneration}, Spec: example.PodSpec{NodeName: "machine"}, } } @@ -864,28 +864,28 @@ func TestStoreDeleteWithOrphanDependents(t *testing.T) { orphanOptions, defaultDeleteStrategy, false, - []string{"foo.com/x", metav1.FinalizerOrphan, "bar.com/y"}, + []string{"foo.com/x", metav1.FinalizerOrphanDependents, "bar.com/y"}, }, { podWithOtherFinalizers("pod2"), orphanOptions, defaultDeleteStrategy, false, - []string{"foo.com/x", "bar.com/y", metav1.FinalizerOrphan}, + []string{"foo.com/x", "bar.com/y", metav1.FinalizerOrphanDependents}, }, { podWithNoFinalizer("pod3"), orphanOptions, defaultDeleteStrategy, false, - []string{metav1.FinalizerOrphan}, + []string{metav1.FinalizerOrphanDependents}, }, { podWithOnlyOrphanFinalizer("pod4"), orphanOptions, defaultDeleteStrategy, false, - []string{metav1.FinalizerOrphan}, + []string{metav1.FinalizerOrphanDependents}, }, // cases run with DeleteOptions.OrphanDedependents=false // these cases all have oprhanDeleteStrategy, which should be ignored @@ -927,14 +927,14 @@ func TestStoreDeleteWithOrphanDependents(t *testing.T) { nilOrphanOptions, defaultDeleteStrategy, false, - []string{"foo.com/x", metav1.FinalizerOrphan, "bar.com/y"}, + []string{"foo.com/x", metav1.FinalizerOrphanDependents, "bar.com/y"}, }, { podWithOrphanFinalizer("pod10"), nilOrphanOptions, orphanDeleteStrategy, false, - []string{"foo.com/x", metav1.FinalizerOrphan, "bar.com/y"}, + []string{"foo.com/x", metav1.FinalizerOrphanDependents, "bar.com/y"}, }, { podWithOtherFinalizers("pod11"), @@ -948,7 +948,7 @@ func TestStoreDeleteWithOrphanDependents(t *testing.T) { nilOrphanOptions, orphanDeleteStrategy, false, - []string{"foo.com/x", "bar.com/y", metav1.FinalizerOrphan}, + []string{"foo.com/x", "bar.com/y", metav1.FinalizerOrphanDependents}, }, { podWithNoFinalizer("pod13"), @@ -962,21 +962,21 @@ func TestStoreDeleteWithOrphanDependents(t *testing.T) { nilOrphanOptions, orphanDeleteStrategy, false, - []string{metav1.FinalizerOrphan}, + []string{metav1.FinalizerOrphanDependents}, }, { podWithOnlyOrphanFinalizer("pod15"), nilOrphanOptions, defaultDeleteStrategy, false, - []string{metav1.FinalizerOrphan}, + []string{metav1.FinalizerOrphanDependents}, }, { podWithOnlyOrphanFinalizer("pod16"), nilOrphanOptions, orphanDeleteStrategy, false, - []string{metav1.FinalizerOrphan}, + []string{metav1.FinalizerOrphanDependents}, }, // cases run with nil DeleteOptions should have exact same behavior. @@ -987,14 +987,14 @@ func TestStoreDeleteWithOrphanDependents(t *testing.T) { nil, defaultDeleteStrategy, false, - []string{"foo.com/x", metav1.FinalizerOrphan, "bar.com/y"}, + []string{"foo.com/x", metav1.FinalizerOrphanDependents, "bar.com/y"}, }, { podWithOrphanFinalizer("pod18"), nil, orphanDeleteStrategy, false, - []string{"foo.com/x", metav1.FinalizerOrphan, "bar.com/y"}, + []string{"foo.com/x", metav1.FinalizerOrphanDependents, "bar.com/y"}, }, { podWithOtherFinalizers("pod19"), @@ -1008,7 +1008,7 @@ func TestStoreDeleteWithOrphanDependents(t *testing.T) { nil, orphanDeleteStrategy, false, - []string{"foo.com/x", "bar.com/y", metav1.FinalizerOrphan}, + []string{"foo.com/x", "bar.com/y", metav1.FinalizerOrphanDependents}, }, { podWithNoFinalizer("pod21"), @@ -1022,21 +1022,21 @@ func TestStoreDeleteWithOrphanDependents(t *testing.T) { nil, orphanDeleteStrategy, false, - []string{metav1.FinalizerOrphan}, + []string{metav1.FinalizerOrphanDependents}, }, { podWithOnlyOrphanFinalizer("pod23"), nil, defaultDeleteStrategy, false, - []string{metav1.FinalizerOrphan}, + []string{metav1.FinalizerOrphanDependents}, }, { podWithOnlyOrphanFinalizer("pod24"), nil, orphanDeleteStrategy, false, - []string{metav1.FinalizerOrphan}, + []string{metav1.FinalizerOrphanDependents}, }, } @@ -1084,6 +1084,225 @@ func TestStoreDeleteWithOrphanDependents(t *testing.T) { } } +// Test the DeleteOptions.PropagationPolicy is handled correctly +func TestStoreDeletionPropagation(t *testing.T) { + initialGeneration := int64(1) + + // defaultDeleteStrategy doesn't implement rest.GarbageCollectionDeleteStrategy. + defaultDeleteStrategy := &testRESTStrategy{scheme, names.SimpleNameGenerator, true, false, true} + // orphanDeleteStrategy indicates the default garbage collection policy is + // to orphan dependentes. + orphanDeleteStrategy := &testOrphanDeleteStrategy{defaultDeleteStrategy} + + foregroundPolicy := metav1.DeletePropagationForeground + backgroundPolicy := metav1.DeletePropagationBackground + orphanPolicy := metav1.DeletePropagationOrphan + + testcases := map[string]struct { + options *metav1.DeleteOptions + strategy rest.RESTDeleteStrategy + // finalizers that are already set in the object + existingFinalizers []string + expectedNotFound bool + expectedFinalizers []string + }{ + "no existing finalizers, PropagationPolicy=Foreground, defaultDeleteStrategy": { + options: &metav1.DeleteOptions{PropagationPolicy: &foregroundPolicy}, + strategy: defaultDeleteStrategy, + expectedFinalizers: []string{metav1.FinalizerDeleteDependents}, + }, + "no existing finalizers, PropagationPolicy=Foreground, orphanDeleteStrategy": { + options: &metav1.DeleteOptions{PropagationPolicy: &foregroundPolicy}, + strategy: orphanDeleteStrategy, + expectedFinalizers: []string{metav1.FinalizerDeleteDependents}, + }, + "no existing finalizers, PropagationPolicy=Background, defaultDeleteStrategy": { + options: &metav1.DeleteOptions{PropagationPolicy: &backgroundPolicy}, + strategy: defaultDeleteStrategy, + expectedNotFound: true, + }, + "no existing finalizers, PropagationPolicy=Background, orphanDeleteStrategy": { + options: &metav1.DeleteOptions{PropagationPolicy: &backgroundPolicy}, + strategy: orphanDeleteStrategy, + expectedNotFound: true, + }, + "no existing finalizers, PropagationPolicy=OrphanDependents, defaultDeleteStrategy": { + options: &metav1.DeleteOptions{PropagationPolicy: &orphanPolicy}, + strategy: defaultDeleteStrategy, + expectedFinalizers: []string{metav1.FinalizerOrphanDependents}, + }, + "no existing finalizers, PropagationPolicy=OrphanDependents, orphanDeleteStrategy": { + options: &metav1.DeleteOptions{PropagationPolicy: &orphanPolicy}, + strategy: orphanDeleteStrategy, + expectedFinalizers: []string{metav1.FinalizerOrphanDependents}, + }, + "no existing finalizers, PropagationPolicy=Default, defaultDeleteStrategy": { + options: &metav1.DeleteOptions{PropagationPolicy: nil}, + strategy: defaultDeleteStrategy, + expectedNotFound: true, + }, + "no existing finalizers, PropagationPolicy=Default, orphanDeleteStrategy": { + options: &metav1.DeleteOptions{PropagationPolicy: nil}, + strategy: orphanDeleteStrategy, + expectedFinalizers: []string{metav1.FinalizerOrphanDependents}, + }, + + // all cases in the following block have "existing orphan finalizer" + "existing orphan finalizer, PropagationPolicy=Foreground, defaultDeleteStrategy": { + options: &metav1.DeleteOptions{PropagationPolicy: &foregroundPolicy}, + strategy: defaultDeleteStrategy, + existingFinalizers: []string{metav1.FinalizerOrphanDependents}, + expectedFinalizers: []string{metav1.FinalizerDeleteDependents}, + }, + "existing orphan finalizer, PropagationPolicy=Foreground, orphanDeleteStrategy": { + options: &metav1.DeleteOptions{PropagationPolicy: &foregroundPolicy}, + strategy: orphanDeleteStrategy, + existingFinalizers: []string{metav1.FinalizerOrphanDependents}, + expectedFinalizers: []string{metav1.FinalizerDeleteDependents}, + }, + "existing orphan finalizer, PropagationPolicy=Background, defaultDeleteStrategy": { + options: &metav1.DeleteOptions{PropagationPolicy: &backgroundPolicy}, + strategy: defaultDeleteStrategy, + existingFinalizers: []string{metav1.FinalizerOrphanDependents}, + expectedNotFound: true, + }, + "existing orphan finalizer, PropagationPolicy=Background, orphanDeleteStrategy": { + options: &metav1.DeleteOptions{PropagationPolicy: &backgroundPolicy}, + strategy: orphanDeleteStrategy, + existingFinalizers: []string{metav1.FinalizerOrphanDependents}, + expectedNotFound: true, + }, + "existing orphan finalizer, PropagationPolicy=OrphanDependents, defaultDeleteStrategy": { + options: &metav1.DeleteOptions{PropagationPolicy: &orphanPolicy}, + strategy: defaultDeleteStrategy, + existingFinalizers: []string{metav1.FinalizerOrphanDependents}, + expectedFinalizers: []string{metav1.FinalizerOrphanDependents}, + }, + "existing orphan finalizer, PropagationPolicy=OrphanDependents, orphanDeleteStrategy": { + options: &metav1.DeleteOptions{PropagationPolicy: &orphanPolicy}, + strategy: orphanDeleteStrategy, + existingFinalizers: []string{metav1.FinalizerOrphanDependents}, + expectedFinalizers: []string{metav1.FinalizerOrphanDependents}, + }, + "existing orphan finalizer, PropagationPolicy=Default, defaultDeleteStrategy": { + options: &metav1.DeleteOptions{PropagationPolicy: nil}, + strategy: defaultDeleteStrategy, + existingFinalizers: []string{metav1.FinalizerOrphanDependents}, + expectedFinalizers: []string{metav1.FinalizerOrphanDependents}, + }, + "existing orphan finalizer, PropagationPolicy=Default, orphanDeleteStrategy": { + options: &metav1.DeleteOptions{PropagationPolicy: nil}, + strategy: orphanDeleteStrategy, + existingFinalizers: []string{metav1.FinalizerOrphanDependents}, + expectedFinalizers: []string{metav1.FinalizerOrphanDependents}, + }, + + // all cases in the following block have "existing deleteDependents finalizer" + "existing deleteDependents finalizer, PropagationPolicy=Foreground, defaultDeleteStrategy": { + options: &metav1.DeleteOptions{PropagationPolicy: &foregroundPolicy}, + strategy: defaultDeleteStrategy, + existingFinalizers: []string{metav1.FinalizerDeleteDependents}, + expectedFinalizers: []string{metav1.FinalizerDeleteDependents}, + }, + "existing deleteDependents finalizer, PropagationPolicy=Foreground, orphanDeleteStrategy": { + options: &metav1.DeleteOptions{PropagationPolicy: &foregroundPolicy}, + strategy: orphanDeleteStrategy, + existingFinalizers: []string{metav1.FinalizerDeleteDependents}, + expectedFinalizers: []string{metav1.FinalizerDeleteDependents}, + }, + "existing deleteDependents finalizer, PropagationPolicy=Background, defaultDeleteStrategy": { + options: &metav1.DeleteOptions{PropagationPolicy: &backgroundPolicy}, + strategy: defaultDeleteStrategy, + existingFinalizers: []string{metav1.FinalizerDeleteDependents}, + expectedNotFound: true, + }, + "existing deleteDependents finalizer, PropagationPolicy=Background, orphanDeleteStrategy": { + options: &metav1.DeleteOptions{PropagationPolicy: &backgroundPolicy}, + strategy: orphanDeleteStrategy, + existingFinalizers: []string{metav1.FinalizerDeleteDependents}, + expectedNotFound: true, + }, + "existing deleteDependents finalizer, PropagationPolicy=OrphanDependents, defaultDeleteStrategy": { + options: &metav1.DeleteOptions{PropagationPolicy: &orphanPolicy}, + strategy: defaultDeleteStrategy, + existingFinalizers: []string{metav1.FinalizerDeleteDependents}, + expectedFinalizers: []string{metav1.FinalizerOrphanDependents}, + }, + "existing deleteDependents finalizer, PropagationPolicy=OrphanDependents, orphanDeleteStrategy": { + options: &metav1.DeleteOptions{PropagationPolicy: &orphanPolicy}, + strategy: orphanDeleteStrategy, + existingFinalizers: []string{metav1.FinalizerDeleteDependents}, + expectedFinalizers: []string{metav1.FinalizerOrphanDependents}, + }, + "existing deleteDependents finalizer, PropagationPolicy=Default, defaultDeleteStrategy": { + options: &metav1.DeleteOptions{PropagationPolicy: nil}, + strategy: defaultDeleteStrategy, + existingFinalizers: []string{metav1.FinalizerDeleteDependents}, + expectedFinalizers: []string{metav1.FinalizerDeleteDependents}, + }, + "existing deleteDependents finalizer, PropagationPolicy=Default, orphanDeleteStrategy": { + options: &metav1.DeleteOptions{PropagationPolicy: nil}, + strategy: orphanDeleteStrategy, + existingFinalizers: []string{metav1.FinalizerDeleteDependents}, + expectedFinalizers: []string{metav1.FinalizerDeleteDependents}, + }, + } + + testContext := genericapirequest.WithNamespace(genericapirequest.NewContext(), "test") + destroyFunc, registry := NewTestGenericStoreRegistry(t) + registry.EnableGarbageCollection = true + defer destroyFunc() + + createPod := func(i int, finalizers []string) *example.Pod { + return &example.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: fmt.Sprintf("pod-%d", i), Finalizers: finalizers, Generation: initialGeneration}, + Spec: example.PodSpec{NodeName: "machine"}, + } + } + + i := 0 + for title, tc := range testcases { + t.Logf("case title: %s", title) + registry.DeleteStrategy = tc.strategy + i++ + pod := createPod(i, tc.existingFinalizers) + // create pod + _, err := registry.Create(testContext, pod) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + _, _, err = registry.Delete(testContext, pod.Name, tc.options) + obj, err := registry.Get(testContext, pod.Name, &metav1.GetOptions{}) + if tc.expectedNotFound { + if err == nil || !errors.IsNotFound(err) { + t.Fatalf("Unexpected error: %v", err) + } + continue + } + if !tc.expectedNotFound && err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if !tc.expectedNotFound { + pod, ok := obj.(*example.Pod) + if !ok { + t.Fatalf("Expect the object to be a pod, but got %#v", obj) + } + if e, a := tc.expectedFinalizers, pod.ObjectMeta.Finalizers; !reflect.DeepEqual(e, a) { + t.Errorf("%v: Expect object %s to have finalizers %v, got %v", pod.Name, pod.ObjectMeta.Name, e, a) + } + if pod.ObjectMeta.DeletionTimestamp == nil { + t.Errorf("%v: Expect the object to have DeletionTimestamp set, but got %#v", pod.Name, pod.ObjectMeta) + } + if pod.ObjectMeta.DeletionGracePeriodSeconds == nil || *pod.ObjectMeta.DeletionGracePeriodSeconds != 0 { + t.Errorf("%v: Expect the object to have 0 DeletionGracePeriodSecond, but got %#v", pod.Name, pod.ObjectMeta) + } + if pod.Generation <= initialGeneration { + t.Errorf("%v: Deletion didn't increase Generation.", pod.Name) + } + } + } +} + func TestStoreDeleteCollection(t *testing.T) { podA := &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}} podB := &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "bar"}} diff --git a/pkg/registry/rest/delete.go b/pkg/registry/rest/delete.go index 572114f99..ace4bab59 100644 --- a/pkg/registry/rest/delete.go +++ b/pkg/registry/rest/delete.go @@ -22,6 +22,7 @@ import ( "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/validation" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" @@ -66,6 +67,9 @@ func BeforeDelete(strategy RESTDeleteStrategy, ctx genericapirequest.Context, ob if kerr != nil { return false, false, kerr } + if errs := validation.ValidateDeleteOptions(options); len(errs) > 0 { + return false, false, errors.NewInvalid(schema.GroupKind{}, "", errs) + } // Checking the Preconditions here to fail early. They'll be enforced later on when we actually do the deletion, too. if options.Preconditions != nil && options.Preconditions.UID != nil && *options.Preconditions.UID != objectMeta.UID { return false, false, errors.NewConflict(schema.GroupResource{Group: gvk.Group, Resource: gvk.Kind}, objectMeta.Name, fmt.Errorf("the UID in the precondition (%s) does not match the UID in record (%s). The object might have been deleted and then recreated", *options.Preconditions.UID, objectMeta.UID)) diff --git a/pkg/registry/rest/update.go b/pkg/registry/rest/update.go index 51845bf4c..f5c48de12 100644 --- a/pkg/registry/rest/update.go +++ b/pkg/registry/rest/update.go @@ -22,6 +22,7 @@ import ( "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" genericvalidation "k8s.io/apimachinery/pkg/api/validation" + "k8s.io/apimachinery/pkg/api/validation/path" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/validation/field" @@ -60,7 +61,7 @@ type RESTUpdateStrategy interface { } // TODO: add other common fields that require global validation. -func validateCommonFields(obj, old runtime.Object) (field.ErrorList, error) { +func validateCommonFields(obj, old runtime.Object, strategy RESTUpdateStrategy) (field.ErrorList, error) { allErrs := field.ErrorList{} objectMeta, err := metav1.ObjectMetaFor(obj) if err != nil { @@ -70,6 +71,7 @@ func validateCommonFields(obj, old runtime.Object) (field.ErrorList, error) { if err != nil { return nil, fmt.Errorf("failed to get old object metadata: %v", err) } + allErrs = append(allErrs, genericvalidation.ValidateObjectMeta(objectMeta, strategy.NamespaceScoped(), path.ValidatePathSegmentName, field.NewPath("metadata"))...) allErrs = append(allErrs, genericvalidation.ValidateObjectMetaUpdate(objectMeta, oldObjectMeta, field.NewPath("metadata"))...) return allErrs, nil @@ -103,7 +105,7 @@ func BeforeUpdate(strategy RESTUpdateStrategy, ctx genericapirequest.Context, ob objectMeta.ClusterName = "" // Ensure some common fields, like UID, are validated for all resources. - errs, err := validateCommonFields(obj, old) + errs, err := validateCommonFields(obj, old, strategy) if err != nil { return errors.NewInternalError(err) }