From 34c4a6e05767e6c6211b88ccc653f2b07021906a Mon Sep 17 00:00:00 2001 From: yue9944882 <291271447@qq.com> Date: Tue, 24 Jul 2018 12:24:59 +0800 Subject: [PATCH 1/4] Cherrypicking #66535 validate deletion admission object backward compatibility: add validation for direct storage delete calls apply nil validation to existing tests revert behavior changes in deleteCollection call fixes validation on wiring graceful deletion remove nil validation check continue admission check on not found error --- .../core/namespace/storage/storage.go | 8 +++- .../core/namespace/storage/storage_test.go | 4 +- pkg/registry/core/pod/storage/eviction.go | 4 +- .../core/pod/storage/eviction_test.go | 2 +- pkg/registry/core/pod/storage/storage_test.go | 4 +- pkg/registry/core/service/storage/rest.go | 6 +-- .../core/service/storage/rest_test.go | 10 ++-- pkg/registry/registrytest/endpoint.go | 4 +- .../scheduling/priorityclass/storage/BUILD | 1 + .../priorityclass/storage/storage.go | 4 +- .../priorityclass/storage/storage_test.go | 3 +- .../pkg/controller/finalizer/crd_finalizer.go | 2 +- .../registry/customresourcedefinition/etcd.go | 8 +++- .../apiserver/pkg/endpoints/apiserver_test.go | 21 ++++++-- .../pkg/endpoints/handlers/delete.go | 48 +++---------------- .../pkg/registry/generic/registry/store.go | 29 +++++++++-- .../registry/generic/registry/store_test.go | 22 ++++----- .../pkg/registry/generic/testing/BUILD | 1 + .../pkg/registry/generic/testing/tester.go | 5 +- .../apiserver/pkg/registry/rest/delete.go | 35 ++++++++++++++ .../apiserver/pkg/registry/rest/rest.go | 4 +- .../pkg/registry/rest/resttest/resttest.go | 28 +++++------ 22 files changed, 150 insertions(+), 103 deletions(-) diff --git a/pkg/registry/core/namespace/storage/storage.go b/pkg/registry/core/namespace/storage/storage.go index fecac7428c9..b76e48f01dd 100644 --- a/pkg/registry/core/namespace/storage/storage.go +++ b/pkg/registry/core/namespace/storage/storage.go @@ -125,7 +125,7 @@ func (r *REST) Export(ctx context.Context, name string, opts metav1.ExportOption } // Delete enforces life-cycle rules for namespace termination -func (r *REST) Delete(ctx context.Context, name string, options *metav1.DeleteOptions) (runtime.Object, bool, error) { +func (r *REST) Delete(ctx context.Context, name string, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions) (runtime.Object, bool, error) { nsObj, err := r.Get(ctx, name, &metav1.GetOptions{}) if err != nil { return nil, false, err @@ -162,6 +162,10 @@ func (r *REST) Delete(ctx context.Context, name string, options *metav1.DeleteOp // upon first request to delete, we switch the phase to start namespace termination // TODO: enhance graceful deletion's calls to DeleteStrategy to allow phase change and finalizer patterns if namespace.DeletionTimestamp.IsZero() { + if err := deleteValidation(nsObj); err != nil { + return nil, false, err + } + key, err := r.store.KeyFunc(ctx, name) if err != nil { return nil, false, err @@ -238,7 +242,7 @@ func (r *REST) Delete(ctx context.Context, name string, options *metav1.DeleteOp err = apierrors.NewConflict(api.Resource("namespaces"), namespace.Name, fmt.Errorf("The system is ensuring all content is removed from this namespace. Upon completion, this namespace will automatically be purged by the system.")) return nil, false, err } - return r.store.Delete(ctx, name, options) + return r.store.Delete(ctx, name, deleteValidation, options) } // ShouldDeleteNamespaceDuringUpdate adds namespace-specific spec.finalizer checks on top of the default generic ShouldDeleteDuringUpdate behavior diff --git a/pkg/registry/core/namespace/storage/storage_test.go b/pkg/registry/core/namespace/storage/storage_test.go index 68191b936b4..f36b645ce39 100644 --- a/pkg/registry/core/namespace/storage/storage_test.go +++ b/pkg/registry/core/namespace/storage/storage_test.go @@ -162,7 +162,7 @@ func TestDeleteNamespaceWithIncompleteFinalizers(t *testing.T) { if err := storage.store.Storage.Create(ctx, key, namespace, nil, 0, false); err != nil { t.Fatalf("unexpected error: %v", err) } - if _, _, err := storage.Delete(ctx, "foo", nil); err == nil { + if _, _, err := storage.Delete(ctx, "foo", rest.ValidateAllObjectFunc, nil); err == nil { t.Errorf("unexpected error: %v", err) } // should still exist @@ -375,7 +375,7 @@ func TestDeleteNamespaceWithCompleteFinalizers(t *testing.T) { if err := storage.store.Storage.Create(ctx, key, namespace, nil, 0, false); err != nil { t.Fatalf("unexpected error: %v", err) } - if _, _, err := storage.Delete(ctx, "foo", nil); err != nil { + if _, _, err := storage.Delete(ctx, "foo", rest.ValidateAllObjectFunc, nil); err != nil { t.Errorf("unexpected error: %v", err) } // should not exist diff --git a/pkg/registry/core/pod/storage/eviction.go b/pkg/registry/core/pod/storage/eviction.go index 61899427499..386244451b0 100644 --- a/pkg/registry/core/pod/storage/eviction.go +++ b/pkg/registry/core/pod/storage/eviction.go @@ -124,7 +124,7 @@ func (r *EvictionREST) Create(ctx context.Context, obj runtime.Object, createVal // Evicting a terminal pod should result in direct deletion of pod as it already caused disruption by the time we are evicting. // There is no need to check for pdb. if pod.Status.Phase == api.PodSucceeded || pod.Status.Phase == api.PodFailed { - _, _, err = r.store.Delete(ctx, eviction.Name, deletionOptions) + _, _, err = r.store.Delete(ctx, eviction.Name, rest.ValidateAllObjectFunc, deletionOptions) if err != nil { return nil, err } @@ -173,7 +173,7 @@ func (r *EvictionREST) Create(ctx context.Context, obj runtime.Object, createVal // At this point there was either no PDB or we succeeded in decrementing // Try the delete - _, _, err = r.store.Delete(ctx, eviction.Name, deletionOptions) + _, _, err = r.store.Delete(ctx, eviction.Name, rest.ValidateAllObjectFunc, deletionOptions) if err != nil { return nil, err } diff --git a/pkg/registry/core/pod/storage/eviction_test.go b/pkg/registry/core/pod/storage/eviction_test.go index 83590a22f95..e33a24f9d57 100644 --- a/pkg/registry/core/pod/storage/eviction_test.go +++ b/pkg/registry/core/pod/storage/eviction_test.go @@ -150,7 +150,7 @@ type FailDeleteUpdateStorage struct { storage.Interface } -func (f FailDeleteUpdateStorage) Delete(ctx context.Context, key string, out runtime.Object, precondition *storage.Preconditions) error { +func (f FailDeleteUpdateStorage) Delete(ctx context.Context, key string, out runtime.Object, precondition *storage.Preconditions, validateDeletion storage.ValidateObjectFunc) error { return storage.NewKeyNotFoundError(key, 0) } diff --git a/pkg/registry/core/pod/storage/storage_test.go b/pkg/registry/core/pod/storage/storage_test.go index 20b2b9443db..57d2804ffb5 100644 --- a/pkg/registry/core/pod/storage/storage_test.go +++ b/pkg/registry/core/pod/storage/storage_test.go @@ -183,7 +183,7 @@ func TestIgnoreDeleteNotFound(t *testing.T) { defer registry.Store.DestroyFunc() // should fail if pod A is not created yet. - _, _, err := registry.Delete(testContext, pod.Name, nil) + _, _, err := registry.Delete(testContext, pod.Name, rest.ValidateAllObjectFunc, nil) if !errors.IsNotFound(err) { t.Errorf("Unexpected error: %v", err) } @@ -198,7 +198,7 @@ func TestIgnoreDeleteNotFound(t *testing.T) { // registry shouldn't get any error since we ignore the NotFound error. zero := int64(0) opt := &metav1.DeleteOptions{GracePeriodSeconds: &zero} - obj, _, err := registry.Delete(testContext, pod.Name, opt) + obj, _, err := registry.Delete(testContext, pod.Name, rest.ValidateAllObjectFunc, opt) if err != nil { t.Fatalf("Unexpected error: %v", err) } diff --git a/pkg/registry/core/service/storage/rest.go b/pkg/registry/core/service/storage/rest.go index 4ed99d17917..b8d4471a0da 100644 --- a/pkg/registry/core/service/storage/rest.go +++ b/pkg/registry/core/service/storage/rest.go @@ -220,9 +220,9 @@ func (rs *REST) Create(ctx context.Context, obj runtime.Object, createValidation return out, err } -func (rs *REST) Delete(ctx context.Context, id string, options *metav1.DeleteOptions) (runtime.Object, bool, error) { +func (rs *REST) Delete(ctx context.Context, id string, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions) (runtime.Object, bool, error) { // TODO: handle graceful - obj, _, err := rs.services.Delete(ctx, id, options) + obj, _, err := rs.services.Delete(ctx, id, deleteValidation, options) if err != nil { return nil, false, err } @@ -233,7 +233,7 @@ func (rs *REST) Delete(ctx context.Context, id string, options *metav1.DeleteOpt if !dryrun.IsDryRun(options.DryRun) { // TODO: can leave dangling endpoints, and potentially return incorrect // endpoints if a new service is created with the same name - _, _, err = rs.endpoints.Delete(ctx, id, &metav1.DeleteOptions{}) + _, _, err = rs.endpoints.Delete(ctx, id, rest.ValidateAllObjectFunc, &metav1.DeleteOptions{}) if err != nil && !errors.IsNotFound(err) { return nil, false, err } diff --git a/pkg/registry/core/service/storage/rest_test.go b/pkg/registry/core/service/storage/rest_test.go index b226a87eac3..db7f90e4aab 100644 --- a/pkg/registry/core/service/storage/rest_test.go +++ b/pkg/registry/core/service/storage/rest_test.go @@ -136,14 +136,14 @@ func (s *serviceStorage) Update(ctx context.Context, name string, objInfo rest.U return obj, s.Created, s.Err } -func (s *serviceStorage) Delete(ctx context.Context, name string, options *metav1.DeleteOptions) (runtime.Object, bool, error) { +func (s *serviceStorage) Delete(ctx context.Context, name string, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions) (runtime.Object, bool, error) { if !dryrun.IsDryRun(options.DryRun) { s.DeletedID = name } return s.Service, s.DeletedImmediately, s.Err } -func (s *serviceStorage) DeleteCollection(ctx context.Context, options *metav1.DeleteOptions, listOptions *metainternalversion.ListOptions) (runtime.Object, error) { +func (s *serviceStorage) DeleteCollection(ctx context.Context, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions, listOptions *metainternalversion.ListOptions) (runtime.Object, error) { panic("not implemented") } @@ -949,7 +949,7 @@ func TestServiceRegistryDelete(t *testing.T) { }, } registry.Create(ctx, svc, rest.ValidateAllObjectFunc, &metav1.CreateOptions{}) - storage.Delete(ctx, svc.Name, &metav1.DeleteOptions{}) + storage.Delete(ctx, svc.Name, rest.ValidateAllObjectFunc, &metav1.DeleteOptions{}) if e, a := "foo", registry.DeletedID; e != a { t.Errorf("Expected %v, but got %v", e, a) } @@ -1038,7 +1038,7 @@ func TestServiceRegistryDeleteExternal(t *testing.T) { }, } registry.Create(ctx, svc, rest.ValidateAllObjectFunc, &metav1.CreateOptions{}) - storage.Delete(ctx, svc.Name, &metav1.DeleteOptions{}) + storage.Delete(ctx, svc.Name, rest.ValidateAllObjectFunc, &metav1.DeleteOptions{}) if e, a := "foo", registry.DeletedID; e != a { t.Errorf("Expected %v, but got %v", e, a) } @@ -1440,7 +1440,7 @@ func TestServiceRegistryIPReallocation(t *testing.T) { t.Errorf("Unexpected ClusterIP: %s", created_service_1.Spec.ClusterIP) } - _, _, err := storage.Delete(ctx, created_service_1.Name, &metav1.DeleteOptions{}) + _, _, err := storage.Delete(ctx, created_service_1.Name, rest.ValidateAllObjectFunc, &metav1.DeleteOptions{}) if err != nil { t.Errorf("Unexpected error deleting service: %v", err) } diff --git a/pkg/registry/registrytest/endpoint.go b/pkg/registry/registrytest/endpoint.go index c36a494282b..2dc31543bed 100644 --- a/pkg/registry/registrytest/endpoint.go +++ b/pkg/registry/registrytest/endpoint.go @@ -111,7 +111,7 @@ func (e *EndpointRegistry) Update(ctx context.Context, name string, objInfo rest return endpoints, false, nil } -func (e *EndpointRegistry) Delete(ctx context.Context, name string, options *metav1.DeleteOptions) (runtime.Object, bool, error) { +func (e *EndpointRegistry) Delete(ctx context.Context, name string, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions) (runtime.Object, bool, error) { // TODO: support namespaces in this mock e.lock.Lock() defer e.lock.Unlock() @@ -130,6 +130,6 @@ func (e *EndpointRegistry) Delete(ctx context.Context, name string, options *met return nil, true, nil } -func (e *EndpointRegistry) DeleteCollection(ctx context.Context, _ *metav1.DeleteOptions, _ *metainternalversion.ListOptions) (runtime.Object, error) { +func (e *EndpointRegistry) DeleteCollection(ctx context.Context, _ rest.ValidateObjectFunc, _ *metav1.DeleteOptions, _ *metainternalversion.ListOptions) (runtime.Object, error) { return nil, fmt.Errorf("unimplemented!") } diff --git a/pkg/registry/scheduling/priorityclass/storage/BUILD b/pkg/registry/scheduling/priorityclass/storage/BUILD index 002dbc8441a..1898120bee1 100644 --- a/pkg/registry/scheduling/priorityclass/storage/BUILD +++ b/pkg/registry/scheduling/priorityclass/storage/BUILD @@ -20,6 +20,7 @@ go_test( "//staging/src/k8s.io/apiserver/pkg/endpoints/request:go_default_library", "//staging/src/k8s.io/apiserver/pkg/registry/generic:go_default_library", "//staging/src/k8s.io/apiserver/pkg/registry/generic/testing:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/registry/rest:go_default_library", "//staging/src/k8s.io/apiserver/pkg/storage/etcd/testing:go_default_library", ], ) diff --git a/pkg/registry/scheduling/priorityclass/storage/storage.go b/pkg/registry/scheduling/priorityclass/storage/storage.go index dfdd67946bd..69e547116ce 100644 --- a/pkg/registry/scheduling/priorityclass/storage/storage.go +++ b/pkg/registry/scheduling/priorityclass/storage/storage.go @@ -68,12 +68,12 @@ func (r *REST) ShortNames() []string { } // Delete ensures that system priority classes are not deleted. -func (r *REST) Delete(ctx context.Context, name string, options *metav1.DeleteOptions) (runtime.Object, bool, error) { +func (r *REST) Delete(ctx context.Context, name string, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions) (runtime.Object, bool, error) { for _, spc := range scheduling.SystemPriorityClasses() { if name == spc.Name { return nil, false, apierrors.NewForbidden(scheduling.Resource("priorityclasses"), spc.Name, errors.New("this is a system priority class and cannot be deleted")) } } - return r.Store.Delete(ctx, name, options) + return r.Store.Delete(ctx, name, deleteValidation, options) } diff --git a/pkg/registry/scheduling/priorityclass/storage/storage_test.go b/pkg/registry/scheduling/priorityclass/storage/storage_test.go index a0963545864..d05e6da3f93 100644 --- a/pkg/registry/scheduling/priorityclass/storage/storage_test.go +++ b/pkg/registry/scheduling/priorityclass/storage/storage_test.go @@ -26,6 +26,7 @@ import ( genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/generic" genericregistrytest "k8s.io/apiserver/pkg/registry/generic/testing" + "k8s.io/apiserver/pkg/registry/rest" etcdtesting "k8s.io/apiserver/pkg/storage/etcd/testing" "k8s.io/kubernetes/pkg/apis/scheduling" "k8s.io/kubernetes/pkg/registry/registrytest" @@ -117,7 +118,7 @@ func TestDeleteSystemPriorityClass(t *testing.T) { if err := storage.Store.Storage.Create(ctx, key, pc, nil, 0, false); err != nil { t.Fatalf("unexpected error: %v", err) } - if _, _, err := storage.Delete(ctx, pc.Name, nil); err == nil { + if _, _, err := storage.Delete(ctx, pc.Name, rest.ValidateAllObjectFunc, nil); err == nil { t.Error("expected to receive an error") } } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/finalizer/crd_finalizer.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/finalizer/crd_finalizer.go index 04caac82905..2730f006c59 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/finalizer/crd_finalizer.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/finalizer/crd_finalizer.go @@ -204,7 +204,7 @@ func (c *CRDFinalizer) deleteInstances(crd *apiextensions.CustomResourceDefiniti // don't retry deleting the same namespace deletedNamespaces.Insert(metadata.GetNamespace()) nsCtx := genericapirequest.WithNamespace(ctx, metadata.GetNamespace()) - if _, err := crClient.DeleteCollection(nsCtx, nil, nil); err != nil { + if _, err := crClient.DeleteCollection(nsCtx, rest.ValidateAllObjectFunc, nil, nil); err != nil { deleteErrors = append(deleteErrors, err) continue } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/etcd.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/etcd.go index 0bc9f64ddc8..9d020cab437 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/etcd.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/etcd.go @@ -67,7 +67,7 @@ func (r *REST) ShortNames() []string { } // Delete adds the CRD finalizer to the list -func (r *REST) Delete(ctx context.Context, name string, options *metav1.DeleteOptions) (runtime.Object, bool, error) { +func (r *REST) Delete(ctx context.Context, name string, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions) (runtime.Object, bool, error) { obj, err := r.Get(ctx, name, &metav1.GetOptions{}) if err != nil { return nil, false, err @@ -103,6 +103,10 @@ func (r *REST) Delete(ctx context.Context, name string, options *metav1.DeleteOp // upon first request to delete, add our finalizer and then delegate if crd.DeletionTimestamp.IsZero() { + if err := deleteValidation(obj); err != nil { + return nil, false, err + } + key, err := r.Store.KeyFunc(ctx, name) if err != nil { return nil, false, err @@ -153,7 +157,7 @@ func (r *REST) Delete(ctx context.Context, name string, options *metav1.DeleteOp return out, false, nil } - return r.Store.Delete(ctx, name, options) + return r.Store.Delete(ctx, name, deleteValidation, options) } // NewStatusREST makes a RESTStorage for status that has more limited options. diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go index a40c41cc6e5..a0cd62fb435 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go @@ -440,13 +440,16 @@ func (storage *SimpleRESTStorage) checkContext(ctx context.Context) { storage.actualNamespace, storage.namespacePresent = request.NamespaceFrom(ctx) } -func (storage *SimpleRESTStorage) Delete(ctx context.Context, id string, options *metav1.DeleteOptions) (runtime.Object, bool, error) { +func (storage *SimpleRESTStorage) Delete(ctx context.Context, id string, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions) (runtime.Object, bool, error) { storage.checkContext(ctx) storage.deleted = id storage.deleteOptions = options if err := storage.errors["delete"]; err != nil { return nil, false, err } + if err := deleteValidation(nil); err != nil { + return nil, false, err + } var obj runtime.Object = &metav1.Status{Status: metav1.StatusSuccess} var err error if storage.injectedFunction != nil { @@ -569,6 +572,18 @@ func (s *ConnecterRESTStorage) NewConnectOptions() (runtime.Object, bool, string return s.emptyConnectOptions, false, "" } +<<<<<<< HEAD +======= +type LegacyRESTStorage struct { + *SimpleRESTStorage +} + +func (storage LegacyRESTStorage) Delete(ctx context.Context, id string) (runtime.Object, error) { + obj, _, err := storage.SimpleRESTStorage.Delete(ctx, id, nil, nil) + return obj, err +} + +>>>>>>> ab07482ecb... validate deletion admission object type MetadataRESTStorage struct { *SimpleRESTStorage types []string @@ -4192,7 +4207,7 @@ type SimpleRESTStorageWithDeleteCollection struct { } // Delete collection doesn't do much, but let us test this path. -func (storage *SimpleRESTStorageWithDeleteCollection) DeleteCollection(ctx context.Context, options *metav1.DeleteOptions, listOptions *metainternalversion.ListOptions) (runtime.Object, error) { +func (storage *SimpleRESTStorageWithDeleteCollection) DeleteCollection(ctx context.Context, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions, listOptions *metainternalversion.ListOptions) (runtime.Object, error) { storage.checkContext(ctx) return nil, nil } @@ -4263,7 +4278,7 @@ func (storage *SimpleXGSubresourceRESTStorage) Get(ctx context.Context, id strin return storage.item.DeepCopyObject(), nil } -func (storage *SimpleXGSubresourceRESTStorage) Delete(ctx context.Context, name string, options *metav1.DeleteOptions) (runtime.Object, bool, error) { +func (storage *SimpleXGSubresourceRESTStorage) Delete(ctx context.Context, name string, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions) (runtime.Object, bool, error) { return nil, true, nil } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go index 7375c575eba..eef4b1f62b1 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go @@ -115,28 +115,12 @@ func DeleteResource(r rest.GracefulDeleter, allowsOptions bool, scope *RequestSc } options.TypeMeta.SetGroupVersionKind(metav1.SchemeGroupVersion.WithKind("DeleteOptions")) - trace.Step("About to check admission control") - if admit != nil && admit.Handles(admission.Delete) { - userInfo, _ := request.UserFrom(ctx) - attrs := admission.NewAttributesRecord(nil, nil, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Delete, options, dryrun.IsDryRun(options.DryRun), userInfo) - if mutatingAdmission, ok := admit.(admission.MutationInterface); ok { - if err := mutatingAdmission.Admit(attrs, scope); err != nil { - scope.err(err, w, req) - return - } - } - if validatingAdmission, ok := admit.(admission.ValidationInterface); ok { - if err := validatingAdmission.Validate(attrs, scope); err != nil { - scope.err(err, w, req) - return - } - } - } - trace.Step("About to delete object from database") wasDeleted := true + userInfo, _ := request.UserFrom(ctx) + staticAdmissionAttrs := admission.NewAttributesRecord(nil, nil, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Delete, options, dryrun.IsDryRun(options.DryRun), userInfo) result, err := finishRequest(timeout, func() (runtime.Object, error) { - obj, deleted, err := r.Delete(ctx, name, options) + obj, deleted, err := r.Delete(ctx, name, rest.AdmissionToValidateObjectDeleteFunc(admit, staticAdmissionAttrs, scope), options) wasDeleted = deleted return obj, err }) @@ -196,6 +180,7 @@ func DeleteCollection(r rest.CollectionDeleter, checkBody bool, scope *RequestSc ctx := req.Context() ctx = request.WithNamespace(ctx, namespace) ae := request.AuditEventFrom(ctx) + admit = admission.WithAudit(admit, ae) outputMediaType, _, err := negotiation.NegotiateOutputMediaType(req, scope.Serializer, scope) if err != nil { @@ -267,29 +252,10 @@ func DeleteCollection(r rest.CollectionDeleter, checkBody bool, scope *RequestSc } options.TypeMeta.SetGroupVersionKind(metav1.SchemeGroupVersion.WithKind("DeleteOptions")) - admit = admission.WithAudit(admit, ae) - if admit != nil && admit.Handles(admission.Delete) { - userInfo, _ := request.UserFrom(ctx) - attrs := admission.NewAttributesRecord(nil, nil, scope.Kind, namespace, "", scope.Resource, scope.Subresource, admission.Delete, options, dryrun.IsDryRun(options.DryRun), userInfo) - if mutatingAdmission, ok := admit.(admission.MutationInterface); ok { - err = mutatingAdmission.Admit(attrs, scope) - if err != nil { - scope.err(err, w, req) - return - } - } - - if validatingAdmission, ok := admit.(admission.ValidationInterface); ok { - err = validatingAdmission.Validate(attrs, scope) - if err != nil { - scope.err(err, w, req) - return - } - } - } - + userInfo, _ := request.UserFrom(ctx) + staticAdmissionAttrs := admission.NewAttributesRecord(nil, nil, scope.Kind, namespace, "", scope.Resource, scope.Subresource, admission.Delete, options, dryrun.IsDryRun(options.DryRun), userInfo) result, err := finishRequest(timeout, func() (runtime.Object, error) { - return r.DeleteCollection(ctx, options, &listOptions) + return r.DeleteCollection(ctx, rest.AdmissionToValidateObjectDeleteFunc(admit, staticAdmissionAttrs, scope), options, &listOptions) }) if err != nil { scope.err(err, w, req) diff --git a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go index 7b8aad1449d..c8bbfe6316d 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go @@ -24,6 +24,7 @@ import ( "sync" "time" + "k8s.io/apimachinery/pkg/api/errors" kubeerr "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/api/validation/path" @@ -881,16 +882,28 @@ func (e *Store) updateForGracefulDeletionAndFinalizers(ctx context.Context, name } // Delete removes the item from storage. -func (e *Store) Delete(ctx context.Context, name string, options *metav1.DeleteOptions) (runtime.Object, bool, error) { +func (e *Store) Delete(ctx context.Context, name string, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions) (runtime.Object, bool, error) { key, err := e.KeyFunc(ctx, name) if err != nil { return nil, false, err } obj := e.NewFunc() qualifiedResource := e.qualifiedResourceFromContext(ctx) - if err := e.Storage.Get(ctx, key, "", obj, false); err != nil { - return nil, false, storeerr.InterpretDeleteError(err, qualifiedResource, name) + err = e.Storage.Get(ctx, key, "", obj, false) + if err != nil { + // Note that we continue the admission check on "Not Found" error is for the compatibility + // with the NodeRestriction admission controller. + if interpretedErr := storeerr.InterpretDeleteError(err, qualifiedResource, name); errors.IsNotFound(interpretedErr) { + if err := deleteValidation(obj); err != nil { + return nil, false, err + } + return nil, false, interpretedErr + } } + if err := deleteValidation(obj.DeepCopyObject()); err != nil { + return nil, false, err + } + // support older consumers of delete by treating "nil" as delete immediately if options == nil { options = metav1.NewDeleteOptions(0) @@ -972,13 +985,19 @@ func (e *Store) Delete(ctx context.Context, name string, options *metav1.DeleteO // are removing all objects of a given type) with the current API (it's technically // possibly with storage API, but watch is not delivered correctly then). // It will be possible to fix it with v3 etcd API. -func (e *Store) DeleteCollection(ctx context.Context, options *metav1.DeleteOptions, listOptions *metainternalversion.ListOptions) (runtime.Object, error) { +func (e *Store) DeleteCollection(ctx context.Context, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions, listOptions *metainternalversion.ListOptions) (runtime.Object, error) { if listOptions == nil { listOptions = &metainternalversion.ListOptions{} } else { listOptions = listOptions.DeepCopy() } + if deleteValidation != nil { + if err := deleteValidation(nil); err != nil { + return nil, err + } + } + listObj, err := e.List(ctx, listOptions) if err != nil { return nil, err @@ -1025,7 +1044,7 @@ func (e *Store) DeleteCollection(ctx context.Context, options *metav1.DeleteOpti errs <- err return } - if _, _, err := e.Delete(ctx, accessor.GetName(), options); err != nil && !kubeerr.IsNotFound(err) { + if _, _, err := e.Delete(ctx, accessor.GetName(), rest.ValidateAllObjectFunc, options); err != nil && !kubeerr.IsNotFound(err) { klog.V(4).Infof("Delete %s in DeleteCollection failed: %v", accessor.GetName(), err) errs <- err return diff --git a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store_test.go b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store_test.go index 7b16f614e5b..a919f193763 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store_test.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store_test.go @@ -357,7 +357,7 @@ func TestStoreCreate(t *testing.T) { // now delete pod with graceful period set delOpts := &metav1.DeleteOptions{GracePeriodSeconds: &gracefulPeriod} - _, _, err = registry.Delete(testContext, podA.Name, delOpts) + _, _, err = registry.Delete(testContext, podA.Name, rest.ValidateAllObjectFunc, delOpts) if err != nil { t.Fatalf("Failed to delete pod gracefully. Unexpected error: %v", err) } @@ -660,7 +660,7 @@ func TestStoreDelete(t *testing.T) { defer destroyFunc() // test failure condition - _, _, err := registry.Delete(testContext, podA.Name, nil) + _, _, err := registry.Delete(testContext, podA.Name, rest.ValidateAllObjectFunc, nil) if !errors.IsNotFound(err) { t.Errorf("Unexpected error: %v", err) } @@ -672,7 +672,7 @@ func TestStoreDelete(t *testing.T) { } // delete object - _, wasDeleted, err := registry.Delete(testContext, podA.Name, nil) + _, wasDeleted, err := registry.Delete(testContext, podA.Name, rest.ValidateAllObjectFunc, nil) if err != nil { t.Errorf("Unexpected error: %v", err) } @@ -749,7 +749,7 @@ func TestGracefulStoreHandleFinalizers(t *testing.T) { } // delete the pod with grace period=0, the pod should still exist because it has a finalizer - _, wasDeleted, err := registry.Delete(testContext, podWithFinalizer.Name, metav1.NewDeleteOptions(0)) + _, wasDeleted, err := registry.Delete(testContext, podWithFinalizer.Name, rest.ValidateAllObjectFunc, metav1.NewDeleteOptions(0)) if err != nil { t.Fatalf("Unexpected error: %v", err) } @@ -815,7 +815,7 @@ func TestNonGracefulStoreHandleFinalizers(t *testing.T) { } // delete object with nil delete options doesn't delete the object - _, wasDeleted, err := registry.Delete(testContext, podWithFinalizer.Name, nil) + _, wasDeleted, err := registry.Delete(testContext, podWithFinalizer.Name, rest.ValidateAllObjectFunc, nil) if err != nil { t.Errorf("Unexpected error: %v", err) } @@ -1115,7 +1115,7 @@ func TestStoreDeleteWithOrphanDependents(t *testing.T) { if err != nil { t.Fatalf("Unexpected error: %v", err) } - _, _, err = registry.Delete(testContext, tc.pod.Name, tc.options) + _, _, err = registry.Delete(testContext, tc.pod.Name, rest.ValidateAllObjectFunc, tc.options) if err != nil { t.Fatalf("Unexpected error: %v", err) } @@ -1334,7 +1334,7 @@ func TestStoreDeletionPropagation(t *testing.T) { if err != nil { t.Fatalf("Unexpected error: %v", err) } - _, _, err = registry.Delete(testContext, pod.Name, tc.options) + _, _, err = registry.Delete(testContext, pod.Name, rest.ValidateAllObjectFunc, tc.options) obj, err := registry.Get(testContext, pod.Name, &metav1.GetOptions{}) if tc.expectedNotFound { if err == nil || !errors.IsNotFound(err) { @@ -1382,7 +1382,7 @@ func TestStoreDeleteCollection(t *testing.T) { } // Delete all pods. - deleted, err := registry.DeleteCollection(testContext, nil, &metainternalversion.ListOptions{}) + deleted, err := registry.DeleteCollection(testContext, rest.ValidateAllObjectFunc, nil, &metainternalversion.ListOptions{}) if err != nil { t.Fatalf("Unexpected error: %v", err) } @@ -1423,7 +1423,7 @@ func TestStoreDeleteCollectionNotFound(t *testing.T) { wg.Add(1) go func() { defer wg.Done() - _, err := registry.DeleteCollection(testContext, nil, &metainternalversion.ListOptions{}) + _, err := registry.DeleteCollection(testContext, rest.ValidateAllObjectFunc, nil, &metainternalversion.ListOptions{}) if err != nil { t.Fatalf("Unexpected error: %v", err) } @@ -1461,7 +1461,7 @@ func TestStoreDeleteCollectionWithWatch(t *testing.T) { } defer watcher.Stop() - if _, err := registry.DeleteCollection(testContext, nil, &metainternalversion.ListOptions{}); err != nil { + if _, err := registry.DeleteCollection(testContext, rest.ValidateAllObjectFunc, nil, &metainternalversion.ListOptions{}); err != nil { t.Fatalf("Unexpected error: %v", err) } @@ -1683,7 +1683,7 @@ func TestQualifiedResource(t *testing.T) { } // delete a non-exist object - _, _, err = registry.Delete(testContext, podA.Name, nil) + _, _, err = registry.Delete(testContext, podA.Name, rest.ValidateAllObjectFunc, nil) if !errors.IsNotFound(err) { t.Fatalf("Unexpected error: %v", err) diff --git a/staging/src/k8s.io/apiserver/pkg/registry/generic/testing/BUILD b/staging/src/k8s.io/apiserver/pkg/registry/generic/testing/BUILD index bc4ad5fdf03..f9494438db0 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/generic/testing/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/registry/generic/testing/BUILD @@ -14,6 +14,7 @@ go_library( "//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", "//staging/src/k8s.io/apiserver/pkg/registry/generic/registry:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/registry/rest:go_default_library", "//staging/src/k8s.io/apiserver/pkg/registry/rest/resttest:go_default_library", "//staging/src/k8s.io/apiserver/pkg/storage/testing:go_default_library", ], diff --git a/staging/src/k8s.io/apiserver/pkg/registry/generic/testing/tester.go b/staging/src/k8s.io/apiserver/pkg/registry/generic/testing/tester.go index 5db5c49d3f0..78cc15bf151 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/generic/testing/tester.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/generic/testing/tester.go @@ -28,6 +28,7 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" genericregistry "k8s.io/apiserver/pkg/registry/generic/registry" + "k8s.io/apiserver/pkg/registry/rest" "k8s.io/apiserver/pkg/registry/rest/resttest" storagetesting "k8s.io/apiserver/pkg/storage/testing" ) @@ -168,7 +169,7 @@ func (t *Tester) createObject(ctx context.Context, obj runtime.Object) error { func (t *Tester) setObjectsForList(objects []runtime.Object) []runtime.Object { key := t.storage.KeyRootFunc(t.tester.TestContext()) - if _, err := t.storage.DeleteCollection(t.tester.TestContext(), nil, nil); err != nil { + if _, err := t.storage.DeleteCollection(t.tester.TestContext(), rest.ValidateAllObjectFunc, nil, nil); err != nil { t.tester.Errorf("unable to clear collection: %v", err) return nil } @@ -192,7 +193,7 @@ func (t *Tester) emitObject(obj runtime.Object, action string) error { if err != nil { return err } - _, _, err = t.storage.Delete(ctx, accessor.GetName(), nil) + _, _, err = t.storage.Delete(ctx, accessor.GetName(), rest.ValidateAllObjectFunc, nil) default: err = fmt.Errorf("unexpected action: %v", action) } diff --git a/staging/src/k8s.io/apiserver/pkg/registry/rest/delete.go b/staging/src/k8s.io/apiserver/pkg/registry/rest/delete.go index 16c9e1b40db..661db668bf8 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/rest/delete.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/rest/delete.go @@ -26,6 +26,7 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/validation" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apiserver/pkg/admission" ) // RESTDeleteStrategy defines deletion behavior on an object that follows Kubernetes @@ -140,3 +141,37 @@ func BeforeDelete(strategy RESTDeleteStrategy, ctx context.Context, obj runtime. } return true, false, nil } + +// AdmissionToValidateObjectDeleteFunc returns a admission validate func for object deletion +func AdmissionToValidateObjectDeleteFunc(admit admission.Interface, staticAttributes admission.Attributes, objInterfaces admission.ObjectInterfaces) ValidateObjectFunc { + mutatingAdmission, isMutatingAdmission := admit.(admission.MutationInterface) + validatingAdmission, isValidatingAdmission := admit.(admission.ValidationInterface) + + return func(old runtime.Object) error { + if !isMutatingAdmission && !isValidatingAdmission { + return nil + } + finalAttributes := admission.NewAttributesRecord( + nil, + old, + staticAttributes.GetKind(), + staticAttributes.GetNamespace(), + staticAttributes.GetName(), + staticAttributes.GetResource(), + staticAttributes.GetSubresource(), + staticAttributes.GetOperation(), + staticAttributes.GetUserInfo(), + ) + if isMutatingAdmission && mutatingAdmission.Handles(finalAttributes.GetOperation()) { + if err := mutatingAdmission.Admit(finalAttributes, objInterfaces); err != nil { + return err + } + } + if isValidatingAdmission && validatingAdmission.Handles(finalAttributes.GetOperation()) { + if err := validatingAdmission.Validate(finalAttributes, objInterfaces); err != nil { + return err + } + } + return nil + } +} diff --git a/staging/src/k8s.io/apiserver/pkg/registry/rest/rest.go b/staging/src/k8s.io/apiserver/pkg/registry/rest/rest.go index b16f7f677bc..e4336fb52d6 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/rest/rest.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/rest/rest.go @@ -156,7 +156,7 @@ type GracefulDeleter interface { // information about deletion. // It also returns a boolean which is set to true if the resource was instantly // deleted or false if it will be deleted asynchronously. - Delete(ctx context.Context, name string, options *metav1.DeleteOptions) (runtime.Object, bool, error) + Delete(ctx context.Context, name string, deleteValidation ValidateObjectFunc, options *metav1.DeleteOptions) (runtime.Object, bool, error) } // CollectionDeleter is an object that can delete a collection @@ -167,7 +167,7 @@ type CollectionDeleter interface { // them or return an invalid request error. // DeleteCollection may not be atomic - i.e. it may delete some objects and still // return an error after it. On success, returns a list of deleted objects. - DeleteCollection(ctx context.Context, options *metav1.DeleteOptions, listOptions *metainternalversion.ListOptions) (runtime.Object, error) + DeleteCollection(ctx context.Context, deleteValidation ValidateObjectFunc, options *metav1.DeleteOptions, listOptions *metainternalversion.ListOptions) (runtime.Object, error) } // Creater is an object that can create an instance of a RESTful object. diff --git a/staging/src/k8s.io/apiserver/pkg/registry/rest/resttest/resttest.go b/staging/src/k8s.io/apiserver/pkg/registry/rest/resttest/resttest.go index 6518968c418..7e8f9f2a20b 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/rest/resttest/resttest.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/rest/resttest/resttest.go @@ -257,7 +257,7 @@ func (t *Tester) delete(ctx context.Context, obj runtime.Object) error { if !ok { return fmt.Errorf("Expected deleting storage, got %v", t.storage) } - _, _, err = deleter.Delete(ctx, objectMeta.GetName(), nil) + _, _, err = deleter.Delete(ctx, objectMeta.GetName(), rest.ValidateAllObjectFunc, nil) return err } @@ -840,7 +840,7 @@ func (t *Tester) testDeleteNoGraceful(obj runtime.Object, createFn CreateFunc, g if dryRun { opts.DryRun = []string{metav1.DryRunAll} } - obj, wasDeleted, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.GetName(), opts) + obj, wasDeleted, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.GetName(), rest.ValidateAllObjectFunc, opts) if err != nil { t.Errorf("unexpected error: %v", err) } @@ -866,7 +866,7 @@ func (t *Tester) testDeleteNoGraceful(obj runtime.Object, createFn CreateFunc, g func (t *Tester) testDeleteNonExist(obj runtime.Object, opts metav1.DeleteOptions) { objectMeta := t.getObjectMetaOrFail(obj) - _, _, err := t.storage.(rest.GracefulDeleter).Delete(t.TestContext(), objectMeta.GetName(), &opts) + _, _, err := t.storage.(rest.GracefulDeleter).Delete(t.TestContext(), objectMeta.GetName(), rest.ValidateAllObjectFunc, &opts) if err == nil || !errors.IsNotFound(err) { t.Errorf("unexpected error: %v", err) } @@ -886,12 +886,12 @@ func (t *Tester) testDeleteWithUID(obj runtime.Object, createFn CreateFunc, getF t.Errorf("unexpected error: %v", err) } opts.Preconditions = metav1.NewPreconditionDeleteOptions("UID1111").Preconditions - obj, _, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.GetName(), &opts) + obj, _, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.GetName(), rest.ValidateAllObjectFunc, &opts) if err == nil || !errors.IsConflict(err) { t.Errorf("unexpected error: %v", err) } - obj, _, err = t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.GetName(), metav1.NewPreconditionDeleteOptions("UID0000")) + obj, _, err = t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.GetName(), rest.ValidateAllObjectFunc, metav1.NewPreconditionDeleteOptions("UID0000")) if err != nil { t.Errorf("unexpected error: %v", err) } @@ -989,7 +989,7 @@ func (t *Tester) testDeleteGracefulHasDefault(obj runtime.Object, createFn Creat } objectMeta := t.getObjectMetaOrFail(foo) generation := objectMeta.GetGeneration() - _, wasDeleted, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.GetName(), &metav1.DeleteOptions{}) + _, wasDeleted, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.GetName(), rest.ValidateAllObjectFunc, &metav1.DeleteOptions{}) if err != nil { t.Errorf("unexpected error: %v", err) } @@ -1023,7 +1023,7 @@ func (t *Tester) testDeleteGracefulWithValue(obj runtime.Object, createFn Create } objectMeta := t.getObjectMetaOrFail(foo) generation := objectMeta.GetGeneration() - _, wasDeleted, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.GetName(), metav1.NewDeleteOptions(expectedGrace+2)) + _, wasDeleted, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.GetName(), rest.ValidateAllObjectFunc, metav1.NewDeleteOptions(expectedGrace+2)) if err != nil { t.Errorf("unexpected error: %v", err) } @@ -1057,7 +1057,7 @@ func (t *Tester) testDeleteGracefulExtend(obj runtime.Object, createFn CreateFun } objectMeta := t.getObjectMetaOrFail(foo) generation := objectMeta.GetGeneration() - _, wasDeleted, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.GetName(), metav1.NewDeleteOptions(expectedGrace)) + _, wasDeleted, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.GetName(), rest.ValidateAllObjectFunc, metav1.NewDeleteOptions(expectedGrace)) if err != nil { t.Errorf("unexpected error: %v", err) } @@ -1069,7 +1069,7 @@ func (t *Tester) testDeleteGracefulExtend(obj runtime.Object, createFn CreateFun } // second delete duration is ignored - _, wasDeleted, err = t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.GetName(), metav1.NewDeleteOptions(expectedGrace+2)) + _, wasDeleted, err = t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.GetName(), rest.ValidateAllObjectFunc, metav1.NewDeleteOptions(expectedGrace+2)) if err != nil { t.Errorf("unexpected error: %v", err) } @@ -1099,7 +1099,7 @@ func (t *Tester) testDeleteGracefulImmediate(obj runtime.Object, createFn Create } objectMeta := t.getObjectMetaOrFail(foo) generation := objectMeta.GetGeneration() - _, wasDeleted, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.GetName(), metav1.NewDeleteOptions(expectedGrace)) + _, wasDeleted, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.GetName(), rest.ValidateAllObjectFunc, metav1.NewDeleteOptions(expectedGrace)) if err != nil { t.Errorf("unexpected error: %v", err) } @@ -1111,7 +1111,7 @@ func (t *Tester) testDeleteGracefulImmediate(obj runtime.Object, createFn Create } // second delete is immediate, resource is deleted - out, wasDeleted, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.GetName(), metav1.NewDeleteOptions(0)) + out, wasDeleted, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.GetName(), rest.ValidateAllObjectFunc, metav1.NewDeleteOptions(0)) if err != nil { t.Errorf("unexpected error: %v", err) } @@ -1141,7 +1141,7 @@ func (t *Tester) testDeleteGracefulUsesZeroOnNil(obj runtime.Object, createFn Cr t.Errorf("unexpected error: %v", err) } objectMeta := t.getObjectMetaOrFail(foo) - _, wasDeleted, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.GetName(), nil) + _, wasDeleted, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.GetName(), rest.ValidateAllObjectFunc, nil) if err != nil { t.Errorf("unexpected error: %v", err) } @@ -1167,7 +1167,7 @@ func (t *Tester) testDeleteGracefulShorten(obj runtime.Object, createFn CreateFu bigGrace = 2 * expectedGrace } objectMeta := t.getObjectMetaOrFail(foo) - _, wasDeleted, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.GetName(), metav1.NewDeleteOptions(bigGrace)) + _, wasDeleted, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.GetName(), rest.ValidateAllObjectFunc, metav1.NewDeleteOptions(bigGrace)) if err != nil { t.Errorf("unexpected error: %v", err) } @@ -1182,7 +1182,7 @@ func (t *Tester) testDeleteGracefulShorten(obj runtime.Object, createFn CreateFu deletionTimestamp := *objectMeta.GetDeletionTimestamp() // second delete duration is ignored - _, wasDeleted, err = t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.GetName(), metav1.NewDeleteOptions(expectedGrace)) + _, wasDeleted, err = t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.GetName(), rest.ValidateAllObjectFunc, metav1.NewDeleteOptions(expectedGrace)) if err != nil { t.Errorf("unexpected error: %v", err) } From 7bb4a3bace048cb9cd93d0221a7bf7c4accbf6be Mon Sep 17 00:00:00 2001 From: Chao Xu Date: Tue, 9 Apr 2019 13:49:16 -0700 Subject: [PATCH 2/4] Run deleteValidation at the storage layer so that it will be retried on conflict. Adding unit test verify that deleteValidation is retried. adding e2e test verifying the webhook can intercept configmap and custom resource deletion, and the existing object is sent via the admissionreview.OldObject. update the admission integration test to verify that the existing object is passed to the deletion admission webhook as oldObject, in case of an immediate deletion and in case of an update-on-delete. --- pkg/master/reconcilers/lease.go | 3 +- .../core/namespace/storage/storage.go | 7 +- .../core/namespace/storage/storage_test.go | 4 +- pkg/registry/core/pod/storage/storage_test.go | 2 +- .../core/service/storage/rest_test.go | 4 +- .../registry/customresourcedefinition/etcd.go | 7 +- .../apiserver/pkg/endpoints/apiserver_test.go | 14 +- .../pkg/endpoints/handlers/delete.go | 2 +- .../pkg/registry/generic/registry/dryrun.go | 9 +- .../registry/generic/registry/dryrun_test.go | 11 +- .../pkg/registry/generic/registry/store.go | 36 ++--- .../registry/generic/registry/store_test.go | 86 ++++++++++- .../apiserver/pkg/registry/rest/delete.go | 14 +- .../apiserver/pkg/registry/rest/rest.go | 6 +- .../pkg/registry/rest/resttest/resttest.go | 8 +- .../apiserver/pkg/storage/cacher/cacher.go | 4 +- .../storage/cacher/cacher_whitebox_test.go | 2 +- .../apiserver/pkg/storage/etcd3/store.go | 41 ++--- .../apiserver/pkg/storage/etcd3/store_test.go | 15 +- .../pkg/storage/etcd3/watcher_test.go | 4 +- .../apiserver/pkg/storage/interfaces.go | 12 +- .../pkg/storage/tests/cacher_test.go | 4 +- test/e2e/apimachinery/webhook.go | 141 ++++++++++++++++-- test/images/webhook/VERSION | 2 +- test/images/webhook/configmap.go | 16 +- test/images/webhook/customresource.go | 16 +- .../admissionwebhook/admission_test.go | 79 +++++++++- test/utils/image/manifest.go | 2 +- 28 files changed, 406 insertions(+), 145 deletions(-) diff --git a/pkg/master/reconcilers/lease.go b/pkg/master/reconcilers/lease.go index 7a0d54080c7..25afeb72262 100644 --- a/pkg/master/reconcilers/lease.go +++ b/pkg/master/reconcilers/lease.go @@ -35,6 +35,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" kruntime "k8s.io/apimachinery/pkg/runtime" apirequest "k8s.io/apiserver/pkg/endpoints/request" + "k8s.io/apiserver/pkg/registry/rest" "k8s.io/apiserver/pkg/storage" corev1client "k8s.io/client-go/kubernetes/typed/core/v1" endpointsv1 "k8s.io/kubernetes/pkg/api/v1/endpoints" @@ -106,7 +107,7 @@ func (s *storageLeases) UpdateLease(ip string) error { // RemoveLease removes the lease on a master IP in storage func (s *storageLeases) RemoveLease(ip string) error { - return s.storage.Delete(apirequest.NewDefaultContext(), s.baseKey+"/"+ip, &corev1.Endpoints{}, nil) + return s.storage.Delete(apirequest.NewDefaultContext(), s.baseKey+"/"+ip, &corev1.Endpoints{}, nil, rest.ValidateAllObjectFunc) } // NewLeases creates a new etcd-based Leases implementation. diff --git a/pkg/registry/core/namespace/storage/storage.go b/pkg/registry/core/namespace/storage/storage.go index b76e48f01dd..7a7b3a646fe 100644 --- a/pkg/registry/core/namespace/storage/storage.go +++ b/pkg/registry/core/namespace/storage/storage.go @@ -162,10 +162,6 @@ func (r *REST) Delete(ctx context.Context, name string, deleteValidation rest.Va // upon first request to delete, we switch the phase to start namespace termination // TODO: enhance graceful deletion's calls to DeleteStrategy to allow phase change and finalizer patterns if namespace.DeletionTimestamp.IsZero() { - if err := deleteValidation(nsObj); err != nil { - return nil, false, err - } - key, err := r.store.KeyFunc(ctx, name) if err != nil { return nil, false, err @@ -182,6 +178,9 @@ func (r *REST) Delete(ctx context.Context, name string, deleteValidation rest.Va // wrong type return nil, fmt.Errorf("expected *api.Namespace, got %v", existing) } + if err := deleteValidation(existingNamespace); err != nil { + return nil, err + } // Set the deletion timestamp if needed if existingNamespace.DeletionTimestamp.IsZero() { now := metav1.Now() diff --git a/pkg/registry/core/namespace/storage/storage_test.go b/pkg/registry/core/namespace/storage/storage_test.go index f36b645ce39..0f2560be473 100644 --- a/pkg/registry/core/namespace/storage/storage_test.go +++ b/pkg/registry/core/namespace/storage/storage_test.go @@ -163,7 +163,7 @@ func TestDeleteNamespaceWithIncompleteFinalizers(t *testing.T) { t.Fatalf("unexpected error: %v", err) } if _, _, err := storage.Delete(ctx, "foo", rest.ValidateAllObjectFunc, nil); err == nil { - t.Errorf("unexpected error: %v", err) + t.Errorf("unexpected no error") } // should still exist _, err := storage.Get(ctx, "foo", &metav1.GetOptions{}) @@ -578,7 +578,7 @@ func TestDeleteWithGCFinalizers(t *testing.T) { } var obj runtime.Object var err error - if obj, _, err = storage.Delete(ctx, test.name, test.deleteOptions); err != nil { + if obj, _, err = storage.Delete(ctx, test.name, rest.ValidateAllObjectFunc, test.deleteOptions); err != nil { t.Fatalf("unexpected error: %v", err) } ns, ok := obj.(*api.Namespace) diff --git a/pkg/registry/core/pod/storage/storage_test.go b/pkg/registry/core/pod/storage/storage_test.go index 57d2804ffb5..f0f03aa33af 100644 --- a/pkg/registry/core/pod/storage/storage_test.go +++ b/pkg/registry/core/pod/storage/storage_test.go @@ -156,7 +156,7 @@ type FailDeletionStorage struct { Called *bool } -func (f FailDeletionStorage) Delete(ctx context.Context, key string, out runtime.Object, precondition *storage.Preconditions) error { +func (f FailDeletionStorage) Delete(ctx context.Context, key string, out runtime.Object, precondition *storage.Preconditions, _ storage.ValidateObjectFunc) error { *f.Called = true return storage.NewKeyNotFoundError(key, 0) } diff --git a/pkg/registry/core/service/storage/rest_test.go b/pkg/registry/core/service/storage/rest_test.go index db7f90e4aab..b2a995d718e 100644 --- a/pkg/registry/core/service/storage/rest_test.go +++ b/pkg/registry/core/service/storage/rest_test.go @@ -979,7 +979,7 @@ func TestServiceRegistryDeleteDryRun(t *testing.T) { if err != nil { t.Fatalf("Expected no error: %v", err) } - _, _, err = storage.Delete(ctx, svc.Name, &metav1.DeleteOptions{DryRun: []string{metav1.DryRunAll}}) + _, _, err = storage.Delete(ctx, svc.Name, rest.ValidateAllObjectFunc, &metav1.DeleteOptions{DryRun: []string{metav1.DryRunAll}}) if err != nil { t.Fatalf("Expected no error: %v", err) } @@ -1009,7 +1009,7 @@ func TestServiceRegistryDeleteDryRun(t *testing.T) { if err != nil { t.Fatalf("Expected no error: %v", err) } - _, _, err = storage.Delete(ctx, svc.Name, &metav1.DeleteOptions{DryRun: []string{metav1.DryRunAll}}) + _, _, err = storage.Delete(ctx, svc.Name, rest.ValidateAllObjectFunc, &metav1.DeleteOptions{DryRun: []string{metav1.DryRunAll}}) if err != nil { t.Fatalf("Expected no error: %v", err) } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/etcd.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/etcd.go index 9d020cab437..6d36d37bddc 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/etcd.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/etcd.go @@ -103,10 +103,6 @@ func (r *REST) Delete(ctx context.Context, name string, deleteValidation rest.Va // upon first request to delete, add our finalizer and then delegate if crd.DeletionTimestamp.IsZero() { - if err := deleteValidation(obj); err != nil { - return nil, false, err - } - key, err := r.Store.KeyFunc(ctx, name) if err != nil { return nil, false, err @@ -123,6 +119,9 @@ func (r *REST) Delete(ctx context.Context, name string, deleteValidation rest.Va // wrong type return nil, fmt.Errorf("expected *apiextensions.CustomResourceDefinition, got %v", existing) } + if err := deleteValidation(existingCRD); err != nil { + return nil, err + } // Set the deletion timestamp if needed if existingCRD.DeletionTimestamp.IsZero() { diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go index a0cd62fb435..084987092ee 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go @@ -447,7 +447,7 @@ func (storage *SimpleRESTStorage) Delete(ctx context.Context, id string, deleteV if err := storage.errors["delete"]; err != nil { return nil, false, err } - if err := deleteValidation(nil); err != nil { + if err := deleteValidation(&storage.item); err != nil { return nil, false, err } var obj runtime.Object = &metav1.Status{Status: metav1.StatusSuccess} @@ -572,18 +572,6 @@ func (s *ConnecterRESTStorage) NewConnectOptions() (runtime.Object, bool, string return s.emptyConnectOptions, false, "" } -<<<<<<< HEAD -======= -type LegacyRESTStorage struct { - *SimpleRESTStorage -} - -func (storage LegacyRESTStorage) Delete(ctx context.Context, id string) (runtime.Object, error) { - obj, _, err := storage.SimpleRESTStorage.Delete(ctx, id, nil, nil) - return obj, err -} - ->>>>>>> ab07482ecb... validate deletion admission object type MetadataRESTStorage struct { *SimpleRESTStorage types []string diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go index eef4b1f62b1..2ac2cc3f131 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go @@ -180,7 +180,6 @@ func DeleteCollection(r rest.CollectionDeleter, checkBody bool, scope *RequestSc ctx := req.Context() ctx = request.WithNamespace(ctx, namespace) ae := request.AuditEventFrom(ctx) - admit = admission.WithAudit(admit, ae) outputMediaType, _, err := negotiation.NegotiateOutputMediaType(req, scope.Serializer, scope) if err != nil { @@ -252,6 +251,7 @@ func DeleteCollection(r rest.CollectionDeleter, checkBody bool, scope *RequestSc } options.TypeMeta.SetGroupVersionKind(metav1.SchemeGroupVersion.WithKind("DeleteOptions")) + admit = admission.WithAudit(admit, ae) userInfo, _ := request.UserFrom(ctx) staticAdmissionAttrs := admission.NewAttributesRecord(nil, nil, scope.Kind, namespace, "", scope.Resource, scope.Subresource, admission.Delete, options, dryrun.IsDryRun(options.DryRun), userInfo) result, err := finishRequest(timeout, func() (runtime.Object, error) { diff --git a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/dryrun.go b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/dryrun.go index 08046fa22cc..24a1b1ec288 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/dryrun.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/dryrun.go @@ -44,14 +44,17 @@ func (s *DryRunnableStorage) Create(ctx context.Context, key string, obj, out ru return s.Storage.Create(ctx, key, obj, out, ttl) } -func (s *DryRunnableStorage) Delete(ctx context.Context, key string, out runtime.Object, preconditions *storage.Preconditions, dryRun bool) error { +func (s *DryRunnableStorage) Delete(ctx context.Context, key string, out runtime.Object, preconditions *storage.Preconditions, deleteValidation storage.ValidateObjectFunc, dryRun bool) error { if dryRun { if err := s.Storage.Get(ctx, key, "", out, false); err != nil { return err } - return preconditions.Check(key, out) + if err := preconditions.Check(key, out); err != nil { + return err + } + return deleteValidation(out) } - return s.Storage.Delete(ctx, key, out, preconditions) + return s.Storage.Delete(ctx, key, out, preconditions, deleteValidation) } func (s *DryRunnableStorage) Watch(ctx context.Context, key string, resourceVersion string, p storage.SelectionPredicate) (watch.Interface, error) { diff --git a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/dryrun_test.go b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/dryrun_test.go index 8fbf219cfdb..9b1fbb18691 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/dryrun_test.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/dryrun_test.go @@ -29,6 +29,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" examplev1 "k8s.io/apiserver/pkg/apis/example/v1" + "k8s.io/apiserver/pkg/registry/rest" "k8s.io/apiserver/pkg/storage" etcdtesting "k8s.io/apiserver/pkg/storage/etcd/testing" "k8s.io/apiserver/pkg/storage/storagebackend/factory" @@ -233,7 +234,7 @@ func TestDryRunDeleteDoesntDelete(t *testing.T) { t.Fatalf("Failed to create new object: %v", err) } - err = s.Delete(context.Background(), "key", out, nil, true) + err = s.Delete(context.Background(), "key", out, nil, rest.ValidateAllObjectFunc, true) if err != nil { t.Fatalf("Failed to dry-run delete the object: %v", err) } @@ -249,7 +250,7 @@ func TestDryRunDeleteMissingObjectFails(t *testing.T) { defer destroy() out := UnstructuredOrDie(`{}`) - err := s.Delete(context.Background(), "key", out, nil, true) + err := s.Delete(context.Background(), "key", out, nil, rest.ValidateAllObjectFunc, true) if e, ok := err.(*storage.StorageError); !ok || e.Code != storage.ErrCodeKeyNotFound { t.Errorf("Expected key to be not found, error: %v", err) } @@ -269,7 +270,7 @@ func TestDryRunDeleteReturnsObject(t *testing.T) { out = UnstructuredOrDie(`{}`) expected := UnstructuredOrDie(`{"kind": "Pod", "metadata": {"resourceVersion": "2"}}`) - err = s.Delete(context.Background(), "key", out, nil, true) + err = s.Delete(context.Background(), "key", out, nil, rest.ValidateAllObjectFunc, true) if err != nil { t.Fatalf("Failed to delete with valid precondition: %v", err) } @@ -292,12 +293,12 @@ func TestDryRunDeletePreconditions(t *testing.T) { wrongID := types.UID("wrong-uid") myID := types.UID("my-uid") - err = s.Delete(context.Background(), "key", out, &storage.Preconditions{UID: &wrongID}, true) + err = s.Delete(context.Background(), "key", out, &storage.Preconditions{UID: &wrongID}, rest.ValidateAllObjectFunc, true) if e, ok := err.(*storage.StorageError); !ok || e.Code != storage.ErrCodeInvalidObj { t.Errorf("Expected invalid object, error: %v", err) } - err = s.Delete(context.Background(), "key", out, &storage.Preconditions{UID: &myID}, true) + err = s.Delete(context.Background(), "key", out, &storage.Preconditions{UID: &myID}, rest.ValidateAllObjectFunc, true) if err != nil { t.Fatalf("Failed to delete with valid precondition: %v", err) } diff --git a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go index c8bbfe6316d..ff44081a7d7 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go @@ -24,7 +24,6 @@ import ( "sync" "time" - "k8s.io/apimachinery/pkg/api/errors" kubeerr "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/api/validation/path" @@ -427,7 +426,8 @@ func ShouldDeleteDuringUpdate(ctx context.Context, key string, obj, existing run func (e *Store) deleteWithoutFinalizers(ctx context.Context, name, key string, obj runtime.Object, preconditions *storage.Preconditions, dryRun bool) (runtime.Object, bool, error) { out := e.NewFunc() klog.V(6).Infof("going to delete %s from registry, triggered by update", name) - if err := e.Storage.Delete(ctx, key, out, preconditions, dryRun); err != nil { + // Using the rest.ValidateAllObjectFunc because the request is an UPDATE request and has already passed the admission for the UPDATE verb. + if err := e.Storage.Delete(ctx, key, out, preconditions, rest.ValidateAllObjectFunc, dryRun); err != nil { // Deletion is racy, i.e., there could be multiple update // requests to remove all finalizers from the object, so we // ignore the NotFound error. @@ -801,7 +801,7 @@ func markAsDeleting(obj runtime.Object, now time.Time) (err error) { // should be deleted immediately // 4. a new output object with the state that was updated // 5. a copy of the last existing state of the object -func (e *Store) updateForGracefulDeletionAndFinalizers(ctx context.Context, name, key string, options *metav1.DeleteOptions, preconditions storage.Preconditions, in runtime.Object) (err error, ignoreNotFound, deleteImmediately bool, out, lastExisting runtime.Object) { +func (e *Store) updateForGracefulDeletionAndFinalizers(ctx context.Context, name, key string, options *metav1.DeleteOptions, preconditions storage.Preconditions, deleteValidation rest.ValidateObjectFunc, in runtime.Object) (err error, ignoreNotFound, deleteImmediately bool, out, lastExisting runtime.Object) { lastGraceful := int64(0) var pendingFinalizers bool out = e.NewFunc() @@ -812,6 +812,9 @@ func (e *Store) updateForGracefulDeletionAndFinalizers(ctx context.Context, name false, /* ignoreNotFound */ &preconditions, storage.SimpleUpdate(func(existing runtime.Object) (runtime.Object, error) { + if err := deleteValidation(existing); err != nil { + return nil, err + } graceful, pendingGraceful, err := rest.BeforeDelete(e.DeleteStrategy, ctx, existing, options) if err != nil { return nil, err @@ -889,19 +892,8 @@ func (e *Store) Delete(ctx context.Context, name string, deleteValidation rest.V } obj := e.NewFunc() qualifiedResource := e.qualifiedResourceFromContext(ctx) - err = e.Storage.Get(ctx, key, "", obj, false) - if err != nil { - // Note that we continue the admission check on "Not Found" error is for the compatibility - // with the NodeRestriction admission controller. - if interpretedErr := storeerr.InterpretDeleteError(err, qualifiedResource, name); errors.IsNotFound(interpretedErr) { - if err := deleteValidation(obj); err != nil { - return nil, false, err - } - return nil, false, interpretedErr - } - } - if err := deleteValidation(obj.DeepCopyObject()); err != nil { - return nil, false, err + if err = e.Storage.Get(ctx, key, "", obj, false); err != nil { + return nil, false, storeerr.InterpretDeleteError(err, qualifiedResource, name) } // support older consumers of delete by treating "nil" as delete immediately @@ -937,7 +929,7 @@ func (e *Store) Delete(ctx context.Context, name string, deleteValidation rest.V shouldUpdateFinalizers, _ := deletionFinalizersForGarbageCollection(ctx, e, accessor, options) // TODO: remove the check, because we support no-op updates now. if graceful || pendingFinalizers || shouldUpdateFinalizers { - err, ignoreNotFound, deleteImmediately, out, lastExisting = e.updateForGracefulDeletionAndFinalizers(ctx, name, key, options, preconditions, obj) + err, ignoreNotFound, deleteImmediately, out, lastExisting = e.updateForGracefulDeletionAndFinalizers(ctx, name, key, options, preconditions, deleteValidation, obj) } // !deleteImmediately covers all cases where err != nil. We keep both to be future-proof. @@ -960,7 +952,7 @@ func (e *Store) Delete(ctx context.Context, name string, deleteValidation rest.V // delete immediately, or no graceful deletion supported klog.V(6).Infof("going to delete %s from registry: ", name) out = e.NewFunc() - if err := e.Storage.Delete(ctx, key, out, &preconditions, dryrun.IsDryRun(options.DryRun)); err != nil { + if err := e.Storage.Delete(ctx, key, out, &preconditions, storage.ValidateObjectFunc(deleteValidation), dryrun.IsDryRun(options.DryRun)); err != nil { // Please refer to the place where we set ignoreNotFound for the reason // why we ignore the NotFound error . if storage.IsNotFound(err) && ignoreNotFound && lastExisting != nil { @@ -992,12 +984,6 @@ func (e *Store) DeleteCollection(ctx context.Context, deleteValidation rest.Vali listOptions = listOptions.DeepCopy() } - if deleteValidation != nil { - if err := deleteValidation(nil); err != nil { - return nil, err - } - } - listObj, err := e.List(ctx, listOptions) if err != nil { return nil, err @@ -1044,7 +1030,7 @@ func (e *Store) DeleteCollection(ctx context.Context, deleteValidation rest.Vali errs <- err return } - if _, _, err := e.Delete(ctx, accessor.GetName(), rest.ValidateAllObjectFunc, options); err != nil && !kubeerr.IsNotFound(err) { + if _, _, err := e.Delete(ctx, accessor.GetName(), deleteValidation, options); err != nil && !kubeerr.IsNotFound(err) { klog.V(4).Infof("Delete %s in DeleteCollection failed: %v", accessor.GetName(), err) errs <- err return diff --git a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store_test.go b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store_test.go index a919f193763..e8a46425e08 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store_test.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store_test.go @@ -1885,7 +1885,7 @@ func TestDeleteWithCachedObject(t *testing.T) { t.Fatal(err) } // The object shouldn't be deleted, because the persisted object has pending finalizers. - _, _, err = registry.Delete(ctx, podName, nil) + _, _, err = registry.Delete(ctx, podName, rest.ValidateAllObjectFunc, nil) if err != nil { t.Fatal(err) } @@ -1895,3 +1895,87 @@ func TestDeleteWithCachedObject(t *testing.T) { t.Fatal(err) } } + +// TestRetryDeleteValidation checks if the deleteValidation is called again if +// the GuaranteedUpdate in the Delete handler conflicts with a simultaneous +// Update. +func TestRetryDeleteValidation(t *testing.T) { + testContext := genericapirequest.WithNamespace(genericapirequest.NewContext(), "test") + destroyFunc, registry := NewTestGenericStoreRegistry(t) + defer destroyFunc() + + tests := []struct { + pod *example.Pod + deleted bool + }{ + { + pod: &example.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "test", Finalizers: []string{"pending"}}, + Spec: example.PodSpec{NodeName: "machine"}, + }, + deleted: false, + }, + + { + pod: &example.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "bar", Namespace: "test"}, + Spec: example.PodSpec{NodeName: "machine"}, + }, + deleted: true, + }, + } + + for _, test := range tests { + ready := make(chan struct{}) + updated := make(chan struct{}) + var readyOnce, updatedOnce sync.Once + var called int + deleteValidation := func(runtime.Object) error { + readyOnce.Do(func() { + close(ready) + }) + // wait for the update completes + <-updated + called++ + return nil + } + + if _, err := registry.Create(testContext, test.pod, rest.ValidateAllObjectFunc, &metav1.CreateOptions{}); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + transformer := func(ctx context.Context, newObj runtime.Object, oldObj runtime.Object) (transformedNewObj runtime.Object, err error) { + <-ready + pod, ok := newObj.(*example.Pod) + if !ok { + t.Fatalf("unexpected object %v", newObj) + } + pod.Labels = map[string]string{ + "modified": "true", + } + return pod, nil + } + + go func() { + // This update will cause the Delete to retry due to conflict. + _, _, err := registry.Update(testContext, test.pod.Name, rest.DefaultUpdatedObjectInfo(test.pod, transformer), rest.ValidateAllObjectFunc, rest.ValidateAllObjectUpdateFunc, false, &metav1.UpdateOptions{}) + if err != nil { + t.Fatal(err) + } + updatedOnce.Do(func() { + close(updated) + }) + }() + + _, deleted, err := registry.Delete(testContext, test.pod.Name, deleteValidation, &metav1.DeleteOptions{}) + if err != nil { + t.Fatal(err) + } + if a, e := deleted, test.deleted; a != e { + t.Fatalf("expected deleted to be %v, got %v", e, a) + } + if called != 2 { + t.Fatalf("expected deleteValidation to be called twice") + } + } +} diff --git a/staging/src/k8s.io/apiserver/pkg/registry/rest/delete.go b/staging/src/k8s.io/apiserver/pkg/registry/rest/delete.go index 661db668bf8..2b5038b210d 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/rest/delete.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/rest/delete.go @@ -147,27 +147,33 @@ func AdmissionToValidateObjectDeleteFunc(admit admission.Interface, staticAttrib mutatingAdmission, isMutatingAdmission := admit.(admission.MutationInterface) validatingAdmission, isValidatingAdmission := admit.(admission.ValidationInterface) + mutating := isMutatingAdmission && mutatingAdmission.Handles(staticAttributes.GetOperation()) + validating := isValidatingAdmission && validatingAdmission.Handles(staticAttributes.GetOperation()) + return func(old runtime.Object) error { - if !isMutatingAdmission && !isValidatingAdmission { + if !mutating && !validating { return nil } finalAttributes := admission.NewAttributesRecord( nil, - old, + // Deep copy the object to avoid accidentally changing the object. + old.DeepCopyObject(), staticAttributes.GetKind(), staticAttributes.GetNamespace(), staticAttributes.GetName(), staticAttributes.GetResource(), staticAttributes.GetSubresource(), staticAttributes.GetOperation(), + staticAttributes.GetOperationOptions(), + staticAttributes.IsDryRun(), staticAttributes.GetUserInfo(), ) - if isMutatingAdmission && mutatingAdmission.Handles(finalAttributes.GetOperation()) { + if mutating { if err := mutatingAdmission.Admit(finalAttributes, objInterfaces); err != nil { return err } } - if isValidatingAdmission && validatingAdmission.Handles(finalAttributes.GetOperation()) { + if validating { if err := validatingAdmission.Validate(finalAttributes, objInterfaces); err != nil { return err } diff --git a/staging/src/k8s.io/apiserver/pkg/registry/rest/rest.go b/staging/src/k8s.io/apiserver/pkg/registry/rest/rest.go index e4336fb52d6..08c7cafc6b7 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/rest/rest.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/rest/rest.go @@ -148,6 +148,7 @@ type TableConvertor interface { // RESTful object. type GracefulDeleter interface { // Delete finds a resource in the storage and deletes it. + // The delete attempt is validated by the deleteValidation first. // If options are provided, the resource will attempt to honor them or return an invalid // request error. // Although it can return an arbitrary error value, IsNotFound(err) is true for the @@ -163,8 +164,9 @@ type GracefulDeleter interface { // of RESTful resources. type CollectionDeleter interface { // DeleteCollection selects all resources in the storage matching given 'listOptions' - // and deletes them. If 'options' are provided, the resource will attempt to honor - // them or return an invalid request error. + // and deletes them. The delete attempt is validated by the deleteValidation first. + // If 'options' are provided, the resource will attempt to honor them or return an + // invalid request error. // DeleteCollection may not be atomic - i.e. it may delete some objects and still // return an error after it. On success, returns a list of deleted objects. DeleteCollection(ctx context.Context, deleteValidation ValidateObjectFunc, options *metav1.DeleteOptions, listOptions *metainternalversion.ListOptions) (runtime.Object, error) diff --git a/staging/src/k8s.io/apiserver/pkg/registry/rest/resttest/resttest.go b/staging/src/k8s.io/apiserver/pkg/registry/rest/resttest/resttest.go index 7e8f9f2a20b..ccf6c74b91d 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/rest/resttest/resttest.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/rest/resttest/resttest.go @@ -923,14 +923,14 @@ func (t *Tester) testDeleteWithResourceVersion(obj runtime.Object, createFn Crea t.Errorf("unexpected error: %v", err) } opts.Preconditions = metav1.NewRVDeletionPrecondition("RV1111").Preconditions - obj, wasDeleted, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.GetName(), &opts) + obj, wasDeleted, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.GetName(), rest.ValidateAllObjectFunc, &opts) if err == nil || !errors.IsConflict(err) { t.Errorf("unexpected error: %v", err) } if wasDeleted { t.Errorf("unexpected, object %s should not have been deleted immediately", objectMeta.GetName()) } - obj, _, err = t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.GetName(), metav1.NewRVDeletionPrecondition("RV0000")) + obj, _, err = t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.GetName(), rest.ValidateAllObjectFunc, metav1.NewRVDeletionPrecondition("RV0000")) if err != nil { t.Errorf("unexpected error: %v", err) } @@ -962,7 +962,7 @@ func (t *Tester) testDeleteDryRunGracefulHasdefault(obj runtime.Object, createFn t.Errorf("unexpected error: %v", err) } objectMeta := t.getObjectMetaOrFail(foo) - object, wasDeleted, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.GetName(), &metav1.DeleteOptions{DryRun: []string{metav1.DryRunAll}}) + object, wasDeleted, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.GetName(), rest.ValidateAllObjectFunc, &metav1.DeleteOptions{DryRun: []string{metav1.DryRunAll}}) if err != nil { t.Errorf("unexpected error: %v", err) } @@ -973,7 +973,7 @@ func (t *Tester) testDeleteDryRunGracefulHasdefault(obj runtime.Object, createFn if objectMeta.GetDeletionTimestamp() == nil || objectMeta.GetDeletionGracePeriodSeconds() == nil || *objectMeta.GetDeletionGracePeriodSeconds() != expectedGrace { t.Errorf("unexpected deleted meta: %#v", objectMeta) } - _, _, err = t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.GetName(), &metav1.DeleteOptions{}) + _, _, err = t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.GetName(), rest.ValidateAllObjectFunc, &metav1.DeleteOptions{}) if err != nil { t.Errorf("unexpected error: %v", err) } diff --git a/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher.go b/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher.go index 51c3d36ca7b..c51c4b86404 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher.go @@ -402,8 +402,8 @@ func (c *Cacher) Create(ctx context.Context, key string, obj, out runtime.Object } // Delete implements storage.Interface. -func (c *Cacher) Delete(ctx context.Context, key string, out runtime.Object, preconditions *storage.Preconditions) error { - return c.storage.Delete(ctx, key, out, preconditions) +func (c *Cacher) Delete(ctx context.Context, key string, out runtime.Object, preconditions *storage.Preconditions, validateDeletion storage.ValidateObjectFunc) error { + return c.storage.Delete(ctx, key, out, preconditions, validateDeletion) } // Watch implements storage.Interface. diff --git a/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher_whitebox_test.go b/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher_whitebox_test.go index f2a2326f3f6..b305c96920a 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher_whitebox_test.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher_whitebox_test.go @@ -299,7 +299,7 @@ func (d *dummyStorage) Versioner() storage.Versioner { return nil } func (d *dummyStorage) Create(_ context.Context, _ string, _, _ runtime.Object, _ uint64) error { return fmt.Errorf("unimplemented") } -func (d *dummyStorage) Delete(_ context.Context, _ string, _ runtime.Object, _ *storage.Preconditions) error { +func (d *dummyStorage) Delete(_ context.Context, _ string, _ runtime.Object, _ *storage.Preconditions, _ storage.ValidateObjectFunc) error { return fmt.Errorf("unimplemented") } func (d *dummyStorage) Watch(_ context.Context, _ string, _ string, _ storage.SelectionPredicate) (watch.Interface, error) { diff --git a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store.go b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store.go index 5a349c7a3eb..349c0a5e4dc 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store.go @@ -181,44 +181,16 @@ func (s *store) Create(ctx context.Context, key string, obj, out runtime.Object, } // Delete implements storage.Interface.Delete. -func (s *store) Delete(ctx context.Context, key string, out runtime.Object, preconditions *storage.Preconditions) error { +func (s *store) Delete(ctx context.Context, key string, out runtime.Object, preconditions *storage.Preconditions, validateDeletion storage.ValidateObjectFunc) error { v, err := conversion.EnforcePtr(out) if err != nil { panic("unable to convert output object to pointer") } key = path.Join(s.pathPrefix, key) - if preconditions == nil { - return s.unconditionalDelete(ctx, key, out) - } - return s.conditionalDelete(ctx, key, out, v, preconditions) + return s.conditionalDelete(ctx, key, out, v, preconditions, validateDeletion) } -func (s *store) unconditionalDelete(ctx context.Context, key string, out runtime.Object) error { - // We need to do get and delete in single transaction in order to - // know the value and revision before deleting it. - startTime := time.Now() - txnResp, err := s.client.KV.Txn(ctx).If().Then( - clientv3.OpGet(key), - clientv3.OpDelete(key), - ).Commit() - metrics.RecordEtcdRequestLatency("delete", getTypeName(out), startTime) - if err != nil { - return err - } - getResp := txnResp.Responses[0].GetResponseRange() - if len(getResp.Kvs) == 0 { - return storage.NewKeyNotFoundError(key, 0) - } - - kv := getResp.Kvs[0] - data, _, err := s.transformer.TransformFromStorage(kv.Value, authenticatedDataString(key)) - if err != nil { - return storage.NewInternalError(err.Error()) - } - return decode(s.codec, s.versioner, data, out, kv.ModRevision) -} - -func (s *store) conditionalDelete(ctx context.Context, key string, out runtime.Object, v reflect.Value, preconditions *storage.Preconditions) error { +func (s *store) conditionalDelete(ctx context.Context, key string, out runtime.Object, v reflect.Value, preconditions *storage.Preconditions, validateDeletion storage.ValidateObjectFunc) error { startTime := time.Now() getResp, err := s.client.KV.Get(ctx, key) metrics.RecordEtcdRequestLatency("get", getTypeName(out), startTime) @@ -230,7 +202,12 @@ func (s *store) conditionalDelete(ctx context.Context, key string, out runtime.O if err != nil { return err } - if err := preconditions.Check(key, origState.obj); err != nil { + if preconditions != nil { + if err := preconditions.Check(key, origState.obj); err != nil { + return err + } + } + if err := validateDeletion(origState.obj); err != nil { return err } startTime := time.Now() diff --git a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store_test.go b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store_test.go index 4d4c1e158ca..65627fbbe41 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store_test.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store_test.go @@ -74,7 +74,7 @@ func (p prefixTransformer) TransformFromStorage(b []byte, ctx value.Context) ([] panic("no context provided") } if !bytes.HasPrefix(b, p.prefix) { - return nil, false, fmt.Errorf("value does not have expected prefix: %s", string(b)) + return nil, false, fmt.Errorf("value does not have expected prefix %q: %s,", p.prefix, string(b)) } return bytes.TrimPrefix(b, p.prefix), p.stale, p.err } @@ -241,7 +241,7 @@ func TestUnconditionalDelete(t *testing.T) { for i, tt := range tests { out := &example.Pod{} // reset - err := store.Delete(ctx, tt.key, out, nil) + err := store.Delete(ctx, tt.key, out, nil, storage.ValidateAllObjectFunc) if tt.expectNotFoundErr { if err == nil || !storage.IsNotFound(err) { t.Errorf("#%d: expecting not found error, but get: %s", i, err) @@ -275,7 +275,7 @@ func TestConditionalDelete(t *testing.T) { for i, tt := range tests { out := &example.Pod{} - err := store.Delete(ctx, key, out, tt.precondition) + err := store.Delete(ctx, key, out, tt.precondition, storage.ValidateAllObjectFunc) if tt.expectInvalidObjErr { if err == nil || !storage.IsInvalidObj(err) { t.Errorf("#%d: expecting invalid UID error, but get: %s", i, err) @@ -740,12 +740,11 @@ func TestTransformationFailure(t *testing.T) { t.Errorf("Unexpected error: %v", err) } - // Delete succeeds but reports an error because we cannot access the body - if err := store.Delete(ctx, preset[1].key, &example.Pod{}, nil); !storage.IsInternalError(err) { + // Delete fails with internal error. + if err := store.Delete(ctx, preset[1].key, &example.Pod{}, nil, storage.ValidateAllObjectFunc); !storage.IsInternalError(err) { t.Errorf("Unexpected error: %v", err) } - - if err := store.Get(ctx, preset[1].key, "", &example.Pod{}, false); !storage.IsNotFound(err) { + if err := store.Get(ctx, preset[1].key, "", &example.Pod{}, false); !storage.IsInternalError(err) { t.Errorf("Unexpected error: %v", err) } } @@ -1349,7 +1348,7 @@ func testSetup(t *testing.T) (context.Context, *store, *integration.ClusterV3) { func testPropogateStore(ctx context.Context, t *testing.T, store *store, obj *example.Pod) (string, *example.Pod) { // Setup store with a key and grab the output for returning. key := "/testkey" - err := store.unconditionalDelete(ctx, key, &example.Pod{}) + err := store.conditionalDelete(ctx, key, &example.Pod{}, reflect.ValueOf(example.Pod{}), nil, storage.ValidateAllObjectFunc) if err != nil && !storage.IsNotFound(err) { t.Fatalf("Cleanup failed: %v", err) } diff --git a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/watcher_test.go b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/watcher_test.go index a8438038850..243eebc9b9a 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/watcher_test.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/watcher_test.go @@ -135,7 +135,7 @@ func TestDeleteTriggerWatch(t *testing.T) { if err != nil { t.Fatalf("Watch failed: %v", err) } - if err := store.Delete(ctx, key, &example.Pod{}, nil); err != nil { + if err := store.Delete(ctx, key, &example.Pod{}, nil, storage.ValidateAllObjectFunc); err != nil { t.Fatalf("Delete failed: %v", err) } testCheckEventType(t, watch.Deleted, w) @@ -295,7 +295,7 @@ func TestWatchDeleteEventObjectHaveLatestRV(t *testing.T) { } etcdW := cluster.RandClient().Watch(ctx, "/", clientv3.WithPrefix()) - if err := store.Delete(ctx, key, &example.Pod{}, &storage.Preconditions{}); err != nil { + if err := store.Delete(ctx, key, &example.Pod{}, &storage.Preconditions{}, storage.ValidateAllObjectFunc); err != nil { t.Fatalf("Delete failed: %v", err) } diff --git a/staging/src/k8s.io/apiserver/pkg/storage/interfaces.go b/staging/src/k8s.io/apiserver/pkg/storage/interfaces.go index f2a1f105373..1e78be8da5f 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/interfaces.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/interfaces.go @@ -95,6 +95,16 @@ var Everything = SelectionPredicate{ // See the comment for GuaranteedUpdate for more details. type UpdateFunc func(input runtime.Object, res ResponseMeta) (output runtime.Object, ttl *uint64, err error) +// ValidateObjectFunc is a function to act on a given object. An error may be returned +// if the hook cannot be completed. The function may NOT transform the provided +// object. +type ValidateObjectFunc func(obj runtime.Object) error + +// ValidateAllObjectFunc is a "admit everything" instance of ValidateObjectFunc. +func ValidateAllObjectFunc(obj runtime.Object) error { + return nil +} + // Preconditions must be fulfilled before an operation (update, delete, etc.) is carried out. type Preconditions struct { // Specifies the target UID. @@ -153,7 +163,7 @@ type Interface interface { // Delete removes the specified key and returns the value that existed at that spot. // If key didn't exist, it will return NotFound storage error. - Delete(ctx context.Context, key string, out runtime.Object, preconditions *Preconditions) error + Delete(ctx context.Context, key string, out runtime.Object, preconditions *Preconditions, validateDeletion ValidateObjectFunc) error // Watch begins watching the specified key. Events are decoded into API objects, // and any items selected by 'p' are sent down to returned watch.Interface. diff --git a/staging/src/k8s.io/apiserver/pkg/storage/tests/cacher_test.go b/staging/src/k8s.io/apiserver/pkg/storage/tests/cacher_test.go index 0f151688b68..f50def6315c 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/tests/cacher_test.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/tests/cacher_test.go @@ -259,7 +259,7 @@ func TestList(t *testing.T) { updatePod(t, etcdStorage, podFooNS2, nil) deleted := example.Pod{} - if err := etcdStorage.Delete(context.TODO(), "pods/ns/bar", &deleted, nil); err != nil { + if err := etcdStorage.Delete(context.TODO(), "pods/ns/bar", &deleted, nil, storage.ValidateAllObjectFunc); err != nil { t.Errorf("Unexpected error: %v", err) } @@ -521,7 +521,7 @@ func TestFiltering(t *testing.T) { _ = updatePod(t, etcdStorage, podFooPrime, fooUnfiltered) deleted := example.Pod{} - if err := etcdStorage.Delete(context.TODO(), "pods/ns/foo", &deleted, nil); err != nil { + if err := etcdStorage.Delete(context.TODO(), "pods/ns/foo", &deleted, nil, storage.ValidateAllObjectFunc); err != nil { t.Errorf("Unexpected error: %v", err) } diff --git a/test/e2e/apimachinery/webhook.go b/test/e2e/apimachinery/webhook.go index fa05c07cd01..ed25afe80f4 100644 --- a/test/e2e/apimachinery/webhook.go +++ b/test/e2e/apimachinery/webhook.go @@ -73,19 +73,20 @@ const ( crdWebhookConfigName = "e2e-test-webhook-config-crd" slowWebhookConfigName = "e2e-test-webhook-config-slow" - skipNamespaceLabelKey = "skip-webhook-admission" - skipNamespaceLabelValue = "yes" - skippedNamespaceName = "exempted-namesapce" - disallowedPodName = "disallowed-pod" - toBeAttachedPodName = "to-be-attached-pod" - hangingPodName = "hanging-pod" - disallowedConfigMapName = "disallowed-configmap" - allowedConfigMapName = "allowed-configmap" - failNamespaceLabelKey = "fail-closed-webhook" - failNamespaceLabelValue = "yes" - failNamespaceName = "fail-closed-namesapce" - addedLabelKey = "added-label" - addedLabelValue = "yes" + skipNamespaceLabelKey = "skip-webhook-admission" + skipNamespaceLabelValue = "yes" + skippedNamespaceName = "exempted-namesapce" + disallowedPodName = "disallowed-pod" + toBeAttachedPodName = "to-be-attached-pod" + hangingPodName = "hanging-pod" + disallowedConfigMapName = "disallowed-configmap" + nonDeletableConfigmapName = "nondeletable-configmap" + allowedConfigMapName = "allowed-configmap" + failNamespaceLabelKey = "fail-closed-webhook" + failNamespaceLabelValue = "yes" + failNamespaceName = "fail-closed-namesapce" + addedLabelKey = "added-label" + addedLabelValue = "yes" ) var serverWebhookVersion = utilversion.MustParseSemantic("v1.8.0") @@ -136,7 +137,7 @@ var _ = SIGDescribe("AdmissionWebhook", func() { testAttachingPodWebhook(f) }) - ginkgo.It("Should be able to deny custom resource creation", func() { + ginkgo.It("Should be able to deny custom resource creation and deletion", func() { testcrd, err := crd.CreateTestCRD(f) if err != nil { return @@ -145,6 +146,7 @@ var _ = SIGDescribe("AdmissionWebhook", func() { webhookCleanup := registerWebhookForCustomResource(f, context, testcrd) defer webhookCleanup() testCustomResourceWebhook(f, testcrd.Crd, testcrd.DynamicClients["v1"]) + testBlockingCustomResourceDeletion(f, testcrd.Crd, testcrd.DynamicClients["v1"]) }) ginkgo.It("Should unconditionally reject operations on fail closed webhook", func() { @@ -458,7 +460,7 @@ func registerWebhook(f *framework.Framework, context *certContext) func() { { Name: "deny-unwanted-configmap-data.k8s.io", Rules: []v1beta1.RuleWithOperations{{ - Operations: []v1beta1.OperationType{v1beta1.Create, v1beta1.Update}, + Operations: []v1beta1.OperationType{v1beta1.Create, v1beta1.Update, v1beta1.Delete}, Rule: v1beta1.Rule{ APIGroups: []string{""}, APIVersions: []string{"v1"}, @@ -788,6 +790,36 @@ func testWebhook(f *framework.Framework) { framework.ExpectNoError(err, "failed to create configmap %s in namespace: %s", configmap.Name, skippedNamespaceName) } +func testBlockingConfigmapDeletion(f *framework.Framework) { + ginkgo.By("create a configmap that should be denied by the webhook when deleting") + client := f.ClientSet + configmap := nonDeletableConfigmap(f) + _, err := client.CoreV1().ConfigMaps(f.Namespace.Name).Create(configmap) + gomega.Expect(err).NotTo(gomega.HaveOccurred(), "failed to create configmap %s in namespace: %s", configmap.Name, f.Namespace.Name) + + ginkgo.By("deleting the configmap should be denied by the webhook") + err = client.CoreV1().ConfigMaps(f.Namespace.Name).Delete(configmap.Name, &metav1.DeleteOptions{}) + gomega.Expect(err).To(gomega.HaveOccurred(), "deleting configmap %s in namespace: %s should be denied", configmap.Name, f.Namespace.Name) + expectedErrMsg1 := "the configmap cannot be deleted because it contains unwanted key and value" + if !strings.Contains(err.Error(), expectedErrMsg1) { + framework.Failf("expect error contains %q, got %q", expectedErrMsg1, err.Error()) + } + + ginkgo.By("remove the offending key and value from the configmap data") + toCompliantFn := func(cm *v1.ConfigMap) { + if cm.Data == nil { + cm.Data = map[string]string{} + } + cm.Data["webhook-e2e-test"] = "webhook-allow" + } + _, err = updateConfigMap(client, f.Namespace.Name, configmap.Name, toCompliantFn) + gomega.Expect(err).NotTo(gomega.HaveOccurred(), "failed to update configmap %s in namespace: %s", configmap.Name, f.Namespace.Name) + + ginkgo.By("deleting the updated configmap should be successful") + err = client.CoreV1().ConfigMaps(f.Namespace.Name).Delete(configmap.Name, &metav1.DeleteOptions{}) + gomega.Expect(err).NotTo(gomega.HaveOccurred(), "failed to delete configmap %s in namespace: %s", configmap.Name, f.Namespace.Name) +} + func testAttachingPodWebhook(f *framework.Framework) { ginkgo.By("create a pod") client := f.ClientSet @@ -1187,6 +1219,17 @@ func nonCompliantConfigMap(f *framework.Framework) *v1.ConfigMap { } } +func nonDeletableConfigmap(f *framework.Framework) *v1.ConfigMap { + return &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: nonDeletableConfigmapName, + }, + Data: map[string]string{ + "webhook-e2e-test": "webhook-nondeletable", + }, + } +} + func toBeMutatedConfigMap(f *framework.Framework) *v1.ConfigMap { return &v1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ @@ -1224,6 +1267,28 @@ func updateConfigMap(c clientset.Interface, ns, name string, update updateConfig return cm, pollErr } +type updateCustomResourceFn func(cm *unstructured.Unstructured) + +func updateCustomResource(c dynamic.ResourceInterface, ns, name string, update updateCustomResourceFn) (*unstructured.Unstructured, error) { + var cr *unstructured.Unstructured + pollErr := wait.PollImmediate(2*time.Second, 1*time.Minute, func() (bool, error) { + var err error + if cr, err = c.Get(name, metav1.GetOptions{}); err != nil { + return false, err + } + update(cr) + if cr, err = c.Update(cr, metav1.UpdateOptions{}); err == nil { + return true, nil + } + // Only retry update on conflict + if !errors.IsConflict(err) { + return false, err + } + return false, nil + }) + return cr, pollErr +} + func cleanWebhookTest(client clientset.Interface, namespaceName string) { _ = client.CoreV1().Services(namespaceName).Delete(serviceName, nil) _ = client.AppsV1().Deployments(namespaceName).Delete(deploymentName, nil) @@ -1245,7 +1310,7 @@ func registerWebhookForCustomResource(f *framework.Framework, context *certConte { Name: "deny-unwanted-custom-resource-data.k8s.io", Rules: []v1beta1.RuleWithOperations{{ - Operations: []v1beta1.OperationType{v1beta1.Create, v1beta1.Update}, + Operations: []v1beta1.OperationType{v1beta1.Create, v1beta1.Update, v1beta1.Delete}, Rule: v1beta1.Rule{ APIGroups: []string{testcrd.Crd.Spec.Group}, APIVersions: servedAPIVersions(testcrd.Crd), @@ -1358,6 +1423,50 @@ func testCustomResourceWebhook(f *framework.Framework, crd *apiextensionsv1beta1 } } +func testBlockingCustomResourceDeletion(f *framework.Framework, crd *apiextensionsv1beta1.CustomResourceDefinition, customResourceClient dynamic.ResourceInterface) { + ginkgo.By("Creating a custom resource whose deletion would be denied by the webhook") + crInstanceName := "cr-instance-2" + crInstance := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": crd.Spec.Names.Kind, + "apiVersion": crd.Spec.Group + "/" + crd.Spec.Version, + "metadata": map[string]interface{}{ + "name": crInstanceName, + "namespace": f.Namespace.Name, + }, + "data": map[string]interface{}{ + "webhook-e2e-test": "webhook-nondeletable", + }, + }, + } + _, err := customResourceClient.Create(crInstance, metav1.CreateOptions{}) + gomega.Expect(err).NotTo(gomega.HaveOccurred(), "failed to create custom resource %s in namespace: %s", crInstanceName, f.Namespace.Name) + + ginkgo.By("Deleting the custom resource should be denied") + err = customResourceClient.Delete(crInstanceName, &metav1.DeleteOptions{}) + gomega.Expect(err).To(gomega.HaveOccurred(), "deleting custom resource %s in namespace: %s should be denied", crInstanceName, f.Namespace.Name) + expectedErrMsg1 := "the custom resource cannot be deleted because it contains unwanted key and value" + if !strings.Contains(err.Error(), expectedErrMsg1) { + framework.Failf("expect error contains %q, got %q", expectedErrMsg1, err.Error()) + } + + ginkgo.By("Remove the offending key and value from the custom resource data") + toCompliantFn := func(cr *unstructured.Unstructured) { + if _, ok := cr.Object["data"]; !ok { + cr.Object["data"] = map[string]interface{}{} + } + data := cr.Object["data"].(map[string]interface{}) + data["webhook-e2e-test"] = "webhook-allow" + } + _, err = updateCustomResource(customResourceClient, f.Namespace.Name, crInstanceName, toCompliantFn) + gomega.Expect(err).NotTo(gomega.HaveOccurred(), "failed to update custom resource %s in namespace: %s", crInstanceName, f.Namespace.Name) + + ginkgo.By("Deleting the updated custom resource should be successful") + err = customResourceClient.Delete(crInstanceName, &metav1.DeleteOptions{}) + gomega.Expect(err).NotTo(gomega.HaveOccurred(), "failed to delete custom resource %s in namespace: %s", crInstanceName, f.Namespace.Name) + +} + func testMutatingCustomResourceWebhook(f *framework.Framework, crd *apiextensionsv1beta1.CustomResourceDefinition, customResourceClient dynamic.ResourceInterface, prune bool) { ginkgo.By("Creating a custom resource that should be mutated by the webhook") crName := "cr-instance-1" diff --git a/test/images/webhook/VERSION b/test/images/webhook/VERSION index 42e1b6f2255..951143a81a2 100644 --- a/test/images/webhook/VERSION +++ b/test/images/webhook/VERSION @@ -1 +1 @@ -1.14v1 +1.15v1 diff --git a/test/images/webhook/configmap.go b/test/images/webhook/configmap.go index 25ba7f4efa3..0a700446d5b 100644 --- a/test/images/webhook/configmap.go +++ b/test/images/webhook/configmap.go @@ -41,7 +41,12 @@ func admitConfigMaps(ar v1beta1.AdmissionReview) *v1beta1.AdmissionResponse { return nil } - raw := ar.Request.Object.Raw + var raw []byte + if ar.Request.Operation == v1beta1.Delete { + raw = ar.Request.OldObject.Raw + } else { + raw = ar.Request.Object.Raw + } configmap := corev1.ConfigMap{} deserializer := codecs.UniversalDeserializer() if _, _, err := deserializer.Decode(raw, nil, &configmap); err != nil { @@ -51,12 +56,19 @@ func admitConfigMaps(ar v1beta1.AdmissionReview) *v1beta1.AdmissionResponse { reviewResponse := v1beta1.AdmissionResponse{} reviewResponse.Allowed = true for k, v := range configmap.Data { - if k == "webhook-e2e-test" && v == "webhook-disallow" { + if k == "webhook-e2e-test" && v == "webhook-disallow" && + (ar.Request.Operation == v1beta1.Create || ar.Request.Operation == v1beta1.Update) { reviewResponse.Allowed = false reviewResponse.Result = &metav1.Status{ Reason: "the configmap contains unwanted key and value", } } + if k == "webhook-e2e-test" && v == "webhook-nondeletable" && ar.Request.Operation == v1beta1.Delete { + reviewResponse.Allowed = false + reviewResponse.Result = &metav1.Status{ + Reason: "the configmap cannot be deleted because it contains unwanted key and value", + } + } } return &reviewResponse } diff --git a/test/images/webhook/customresource.go b/test/images/webhook/customresource.go index 53e235f3fa1..d14b036a0f3 100644 --- a/test/images/webhook/customresource.go +++ b/test/images/webhook/customresource.go @@ -69,7 +69,12 @@ func admitCustomResource(ar v1beta1.AdmissionReview) *v1beta1.AdmissionResponse Data map[string]string }{} - raw := ar.Request.Object.Raw + var raw []byte + if ar.Request.Operation == v1beta1.Delete { + raw = ar.Request.OldObject.Raw + } else { + raw = ar.Request.Object.Raw + } err := json.Unmarshal(raw, &cr) if err != nil { klog.Error(err) @@ -79,12 +84,19 @@ func admitCustomResource(ar v1beta1.AdmissionReview) *v1beta1.AdmissionResponse reviewResponse := v1beta1.AdmissionResponse{} reviewResponse.Allowed = true for k, v := range cr.Data { - if k == "webhook-e2e-test" && v == "webhook-disallow" { + if k == "webhook-e2e-test" && v == "webhook-disallow" && + (ar.Request.Operation == v1beta1.Create || ar.Request.Operation == v1beta1.Update) { reviewResponse.Allowed = false reviewResponse.Result = &metav1.Status{ Reason: "the custom resource contains unwanted data", } } + if k == "webhook-e2e-test" && v == "webhook-nondeletable" && ar.Request.Operation == v1beta1.Delete { + reviewResponse.Allowed = false + reviewResponse.Result = &metav1.Status{ + Reason: "the custom resource cannot be deleted because it contains unwanted key and value", + } + } } return &reviewResponse } diff --git a/test/integration/apiserver/admissionwebhook/admission_test.go b/test/integration/apiserver/admissionwebhook/admission_test.go index f9069eef522..4d47adcad7b 100644 --- a/test/integration/apiserver/admissionwebhook/admission_test.go +++ b/test/integration/apiserver/admissionwebhook/admission_test.go @@ -527,6 +527,7 @@ func testResourcePatch(c *testContext) { } func testResourceDelete(c *testContext) { + // Verify that an immediate delete triggers the webhook and populates the admisssionRequest.oldObject. obj, err := createOrGetResource(c.client, c.gvr, c.resource) if err != nil { c.t.Error(err) @@ -534,12 +535,13 @@ func testResourceDelete(c *testContext) { } background := metav1.DeletePropagationBackground zero := int64(0) - c.admissionHolder.expect(c.gvr, gvk(c.resource.Group, c.resource.Version, c.resource.Kind), gvkDeleteOptions, v1beta1.Delete, obj.GetName(), obj.GetNamespace(), false, false, true) + c.admissionHolder.expect(c.gvr, gvk(c.resource.Group, c.resource.Version, c.resource.Kind), gvkDeleteOptions, v1beta1.Delete, obj.GetName(), obj.GetNamespace(), false, true, true) err = c.client.Resource(c.gvr).Namespace(obj.GetNamespace()).Delete(obj.GetName(), &metav1.DeleteOptions{GracePeriodSeconds: &zero, PropagationPolicy: &background}) if err != nil { c.t.Error(err) return } + c.admissionHolder.verify(c.t) // wait for the item to be gone err = wait.PollImmediate(100*time.Millisecond, 10*time.Second, func() (bool, error) { @@ -557,6 +559,77 @@ func testResourceDelete(c *testContext) { c.t.Error(err) return } + + // Verify that an update-on-delete triggers the webhook and populates the admisssionRequest.oldObject. + obj, err = createOrGetResource(c.client, c.gvr, c.resource) + if err != nil { + c.t.Error(err) + return + } + // Adding finalizer to the object, then deleting it. + // We don't add finalizers by setting DeleteOptions.PropagationPolicy + // because some resource (e.g., events) do not support garbage + // collector finalizers. + _, err = c.client.Resource(c.gvr).Namespace(obj.GetNamespace()).Patch( + obj.GetName(), + types.MergePatchType, + []byte(`{"metadata":{"finalizers":["test/k8s.io"]}}`), + metav1.PatchOptions{}) + if err != nil { + c.t.Error(err) + return + } + c.admissionHolder.expect(c.gvr, gvk(c.resource.Group, c.resource.Version, c.resource.Kind), gvkDeleteOptions, v1beta1.Delete, obj.GetName(), obj.GetNamespace(), false, true, true) + err = c.client.Resource(c.gvr).Namespace(obj.GetNamespace()).Delete(obj.GetName(), &metav1.DeleteOptions{GracePeriodSeconds: &zero, PropagationPolicy: &background}) + if err != nil { + c.t.Error(err) + return + } + c.admissionHolder.verify(c.t) + + // wait other finalizers (e.g., crd's customresourcecleanup finalizer) to be removed. + err = wait.PollImmediate(100*time.Millisecond, 10*time.Second, func() (bool, error) { + obj, err := c.client.Resource(c.gvr).Namespace(obj.GetNamespace()).Get(obj.GetName(), metav1.GetOptions{}) + if err != nil { + return false, err + } + finalizers := obj.GetFinalizers() + if len(finalizers) != 1 { + c.t.Logf("waiting for other finalizers on %#v %s to be removed, existing finalizers are %v", c.gvr, obj.GetName(), obj.GetFinalizers()) + return false, nil + } + if finalizers[0] != "test/k8s.io" { + return false, fmt.Errorf("expected the single finalizer on %#v %s to be test/k8s.io, got %v", c.gvr, obj.GetName(), obj.GetFinalizers()) + } + return true, nil + }) + + // remove the finalizer + _, err = c.client.Resource(c.gvr).Namespace(obj.GetNamespace()).Patch( + obj.GetName(), + types.MergePatchType, + []byte(`{"metadata":{"finalizers":[]}}`), + metav1.PatchOptions{}) + if err != nil { + c.t.Error(err) + return + } + // wait for the item to be gone + err = wait.PollImmediate(100*time.Millisecond, 10*time.Second, func() (bool, error) { + obj, err := c.client.Resource(c.gvr).Namespace(obj.GetNamespace()).Get(obj.GetName(), metav1.GetOptions{}) + if errors.IsNotFound(err) { + return true, nil + } + if err == nil { + c.t.Logf("waiting for %#v to be deleted (name: %s, finalizers: %v)...\n", c.gvr, obj.GetName(), obj.GetFinalizers()) + return false, nil + } + return false, err + }) + if err != nil { + c.t.Error(err) + return + } } func testResourceDeletecollection(c *testContext) { @@ -580,7 +653,7 @@ func testResourceDeletecollection(c *testContext) { } // set expectations - c.admissionHolder.expect(c.gvr, gvk(c.resource.Group, c.resource.Version, c.resource.Kind), gvkDeleteOptions, v1beta1.Delete, "", obj.GetNamespace(), false, false, true) + c.admissionHolder.expect(c.gvr, gvk(c.resource.Group, c.resource.Version, c.resource.Kind), gvkDeleteOptions, v1beta1.Delete, "", obj.GetNamespace(), false, true, true) // delete err = c.client.Resource(c.gvr).Namespace(obj.GetNamespace()).DeleteCollection(&metav1.DeleteOptions{GracePeriodSeconds: &zero, PropagationPolicy: &background}, metav1.ListOptions{LabelSelector: "webhooktest=true"}) @@ -708,7 +781,7 @@ func testNamespaceDelete(c *testContext) { background := metav1.DeletePropagationBackground zero := int64(0) - c.admissionHolder.expect(c.gvr, gvk(c.resource.Group, c.resource.Version, c.resource.Kind), gvkDeleteOptions, v1beta1.Delete, obj.GetName(), obj.GetNamespace(), false, false, true) + c.admissionHolder.expect(c.gvr, gvk(c.resource.Group, c.resource.Version, c.resource.Kind), gvkDeleteOptions, v1beta1.Delete, obj.GetName(), obj.GetNamespace(), false, true, true) err = c.client.Resource(c.gvr).Namespace(obj.GetNamespace()).Delete(obj.GetName(), &metav1.DeleteOptions{GracePeriodSeconds: &zero, PropagationPolicy: &background}) if err != nil { c.t.Error(err) diff --git a/test/utils/image/manifest.go b/test/utils/image/manifest.go index e1eea1a98b8..2e547833d87 100644 --- a/test/utils/image/manifest.go +++ b/test/utils/image/manifest.go @@ -196,7 +196,7 @@ const ( func initImageConfigs() map[int]Config { configs := map[int]Config{} configs[CRDConversionWebhook] = Config{e2eRegistry, "crd-conversion-webhook", "1.13rev2"} - configs[AdmissionWebhook] = Config{e2eRegistry, "webhook", "1.14v1"} + configs[AdmissionWebhook] = Config{e2eRegistry, "webhook", "1.15v1"} configs[Agnhost] = Config{e2eRegistry, "agnhost", "1.0"} configs[APIServer] = Config{e2eRegistry, "sample-apiserver", "1.10"} configs[AppArmorLoader] = Config{e2eRegistry, "apparmor-loader", "1.0"} From 0b88095a17c9863a8abae2374bd99912be1ce6c9 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Fri, 26 Apr 2019 01:10:00 -0400 Subject: [PATCH 3/4] Switch admission webhook test to work with shared etcd --- cmd/kube-apiserver/app/testing/testserver.go | 2 ++ .../admissionwebhook/admission_test.go | 28 ++++++++++--------- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/cmd/kube-apiserver/app/testing/testserver.go b/cmd/kube-apiserver/app/testing/testserver.go index d9cde3599de..cfa15352ae4 100644 --- a/cmd/kube-apiserver/app/testing/testserver.go +++ b/cmd/kube-apiserver/app/testing/testserver.go @@ -179,6 +179,8 @@ func StartTestServer(t Logger, instanceOptions *TestServerInstanceOptions, custo // from here the caller must call tearDown result.ClientConfig = server.LoopbackClientConfig + result.ClientConfig.QPS = 1000 + result.ClientConfig.Burst = 10000 result.ServerOpts = s result.TearDownFn = tearDown diff --git a/test/integration/apiserver/admissionwebhook/admission_test.go b/test/integration/apiserver/admissionwebhook/admission_test.go index 4d47adcad7b..26c8c906ecf 100644 --- a/test/integration/apiserver/admissionwebhook/admission_test.go +++ b/test/integration/apiserver/admissionwebhook/admission_test.go @@ -34,6 +34,7 @@ import ( admissionv1beta1 "k8s.io/api/admissionregistration/v1beta1" appsv1beta1 "k8s.io/api/apps/v1beta1" corev1 "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" extensionsv1beta1 "k8s.io/api/extensions/v1beta1" policyv1beta1 "k8s.io/api/policy/v1beta1" apiextensionsclientset "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" @@ -353,31 +354,32 @@ func TestWebhookV1beta1(t *testing.T) { defer webhookServer.Close() // start API server - s, err := kubeapiservertesting.StartTestServer(t, kubeapiservertesting.NewDefaultTestServerOptions(), []string{ + etcdConfig := framework.SharedEtcd() + server := kubeapiservertesting.StartTestServerOrDie(t, nil, []string{ + // turn off admission plugins that add finalizers "--disable-admission-plugins=ServiceAccount,StorageObjectInUseProtection", - "--runtime-config=extensions/v1beta1/deployments=true,extensions/v1beta1/daemonsets=true,extensions/v1beta1/replicasets=true,extensions/v1beta1/podsecuritypolicies=true,extensions/v1beta1/networkpolicies=true", - }, framework.SharedEtcd()) - if err != nil { - t.Fatal(err) - } - defer s.TearDownFn() - - // create CRDs so we can make sure that custom resources do not get lost - etcd.CreateTestCRDs(t, apiextensionsclientset.NewForConfigOrDie(s.ClientConfig), false, etcd.GetCustomResourceDefinitionData()...) + // force enable all resources so we can check storage. + // TODO: drop these once we stop allowing them to be served. + "--runtime-config=api/all=true,extensions/v1beta1/deployments=true,extensions/v1beta1/daemonsets=true,extensions/v1beta1/replicasets=true,extensions/v1beta1/podsecuritypolicies=true,extensions/v1beta1/networkpolicies=true", + }, etcdConfig) + defer server.TearDownFn() // Configure a client with a distinct user name so that it is easy to distinguish requests // made by the client from requests made by controllers. We use this to filter out requests // before recording them to ensure we don't accidentally mistake requests from controllers // as requests made by the client. - clientConfig := rest.CopyConfig(s.ClientConfig) + clientConfig := rest.CopyConfig(server.ClientConfig) clientConfig.Impersonate.UserName = testClientUsername clientConfig.Impersonate.Groups = []string{"system:masters", "system:authenticated"} client, err := clientset.NewForConfig(clientConfig) if err != nil { - t.Fatal(err) + t.Fatalf("unexpected error: %v", err) } - if _, err := client.CoreV1().Namespaces().Create(&corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: testNamespace}}); err != nil { + // create CRDs + etcd.CreateTestCRDs(t, apiextensionsclientset.NewForConfigOrDie(server.ClientConfig), false, etcd.GetCustomResourceDefinitionData()...) + + if _, err := client.CoreV1().Namespaces().Create(&v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: testNamespace}}); err != nil { t.Fatal(err) } if err := createV1beta1MutationWebhook(client, webhookServer.URL+"/"+mutation); err != nil { From 6d40e3c9e98ecade4a62d18f20163164d3e5e862 Mon Sep 17 00:00:00 2001 From: Chao Xu Date: Wed, 10 Apr 2019 14:11:11 -0700 Subject: [PATCH 4/4] generated --- pkg/master/reconcilers/BUILD | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/master/reconcilers/BUILD b/pkg/master/reconcilers/BUILD index 4ad16ac8b74..cc498604880 100644 --- a/pkg/master/reconcilers/BUILD +++ b/pkg/master/reconcilers/BUILD @@ -18,6 +18,7 @@ go_library( "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", "//staging/src/k8s.io/apiserver/pkg/endpoints/request:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/registry/rest:go_default_library", "//staging/src/k8s.io/apiserver/pkg/storage:go_default_library", "//staging/src/k8s.io/client-go/kubernetes/typed/core/v1:go_default_library", "//staging/src/k8s.io/client-go/util/retry:go_default_library",