diff --git a/staging/src/k8s.io/kube-apiextensions-server/pkg/apiserver/BUILD b/staging/src/k8s.io/kube-apiextensions-server/pkg/apiserver/BUILD index 56a8d0a96af..7bcbf18de77 100644 --- a/staging/src/k8s.io/kube-apiextensions-server/pkg/apiserver/BUILD +++ b/staging/src/k8s.io/kube-apiextensions-server/pkg/apiserver/BUILD @@ -43,7 +43,6 @@ go_library( "//vendor/k8s.io/apiserver/pkg/server:go_default_library", "//vendor/k8s.io/apiserver/pkg/storage/storagebackend:go_default_library", "//vendor/k8s.io/client-go/discovery:go_default_library", - "//vendor/k8s.io/client-go/dynamic:go_default_library", "//vendor/k8s.io/client-go/tools/cache:go_default_library", "//vendor/k8s.io/client-go/util/workqueue:go_default_library", "//vendor/k8s.io/kube-apiextensions-server/pkg/apis/apiextensions:go_default_library", diff --git a/staging/src/k8s.io/kube-apiextensions-server/pkg/apiserver/apiserver.go b/staging/src/k8s.io/kube-apiextensions-server/pkg/apiserver/apiserver.go index 819e53dae58..dec369eb56f 100644 --- a/staging/src/k8s.io/kube-apiextensions-server/pkg/apiserver/apiserver.go +++ b/staging/src/k8s.io/kube-apiextensions-server/pkg/apiserver/apiserver.go @@ -31,7 +31,6 @@ import ( genericregistry "k8s.io/apiserver/pkg/registry/generic" "k8s.io/apiserver/pkg/registry/rest" genericapiserver "k8s.io/apiserver/pkg/server" - "k8s.io/client-go/dynamic" "k8s.io/kube-apiextensions-server/pkg/apis/apiextensions" "k8s.io/kube-apiextensions-server/pkg/apis/apiextensions/install" @@ -165,7 +164,8 @@ func (c completedConfig) New(delegationTarget genericapiserver.DelegationTarget) finalizingController := finalizer.NewCRDFinalizer( s.Informers.Apiextensions().InternalVersion().CustomResourceDefinitions(), crdClient, - dynamic.NewDynamicClientPool(s.GenericAPIServer.LoopbackClientConfig)) + crdHandler, + ) s.GenericAPIServer.AddPostStartHook("start-apiextensions-informers", func(context genericapiserver.PostStartHookContext) error { s.Informers.Start(context.StopCh) diff --git a/staging/src/k8s.io/kube-apiextensions-server/pkg/apiserver/customresource_handler.go b/staging/src/k8s.io/kube-apiextensions-server/pkg/apiserver/customresource_handler.go index 2f8f3e4151d..294ccb6c440 100644 --- a/staging/src/k8s.io/kube-apiextensions-server/pkg/apiserver/customresource_handler.go +++ b/staging/src/k8s.io/kube-apiextensions-server/pkg/apiserver/customresource_handler.go @@ -45,6 +45,7 @@ import ( "k8s.io/kube-apiextensions-server/pkg/apis/apiextensions" listers "k8s.io/kube-apiextensions-server/pkg/client/listers/apiextensions/internalversion" + "k8s.io/kube-apiextensions-server/pkg/controller/finalizer" "k8s.io/kube-apiextensions-server/pkg/registry/customresource" ) @@ -245,6 +246,17 @@ func (r *crdHandler) removeDeadStorage() { r.customStorage.Store(storageMap) } +// GetCustomResourceListerCollectionDeleter returns the ListerCollectionDeleter for +// the given uid, or nil if one does not exist. +func (r *crdHandler) GetCustomResourceListerCollectionDeleter(uid types.UID) finalizer.ListerCollectionDeleter { + storageMap := r.customStorage.Load().(crdStorageMap) + ret, ok := storageMap[uid] + if !ok { + return nil + } + return ret.storage +} + func (r *crdHandler) getServingInfoFor(crd *apiextensions.CustomResourceDefinition) *crdInfo { storageMap := r.customStorage.Load().(crdStorageMap) ret, ok := storageMap[crd.UID] diff --git a/staging/src/k8s.io/kube-apiextensions-server/pkg/controller/finalizer/BUILD b/staging/src/k8s.io/kube-apiextensions-server/pkg/controller/finalizer/BUILD index 00b7e0ce951..30442094d60 100644 --- a/staging/src/k8s.io/kube-apiextensions-server/pkg/controller/finalizer/BUILD +++ b/staging/src/k8s.io/kube-apiextensions-server/pkg/controller/finalizer/BUILD @@ -15,15 +15,15 @@ go_library( "//vendor/github.com/golang/glog:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/meta:go_default_library", - "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured:go_default_library", "//vendor/k8s.io/apimachinery/pkg/conversion:go_default_library", - "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/types:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/errors:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/runtime:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/wait:go_default_library", - "//vendor/k8s.io/client-go/dynamic:go_default_library", + "//vendor/k8s.io/apiserver/pkg/endpoints/request:go_default_library", + "//vendor/k8s.io/apiserver/pkg/registry/rest:go_default_library", "//vendor/k8s.io/client-go/tools/cache:go_default_library", "//vendor/k8s.io/client-go/util/workqueue:go_default_library", "//vendor/k8s.io/kube-apiextensions-server/pkg/apis/apiextensions:go_default_library", diff --git a/staging/src/k8s.io/kube-apiextensions-server/pkg/controller/finalizer/crd_finalizer.go b/staging/src/k8s.io/kube-apiextensions-server/pkg/controller/finalizer/crd_finalizer.go index 2f6516d1db1..ab4fef4f6de 100644 --- a/staging/src/k8s.io/kube-apiextensions-server/pkg/controller/finalizer/crd_finalizer.go +++ b/staging/src/k8s.io/kube-apiextensions-server/pkg/controller/finalizer/crd_finalizer.go @@ -25,15 +25,15 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/conversion" - "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" utilerrors "k8s.io/apimachinery/pkg/util/errors" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/wait" - "k8s.io/client-go/dynamic" + genericapirequest "k8s.io/apiserver/pkg/endpoints/request" + "k8s.io/apiserver/pkg/registry/rest" "k8s.io/client-go/tools/cache" "k8s.io/client-go/util/workqueue" @@ -45,11 +45,10 @@ import ( var cloner = conversion.NewCloner() -// This controller finalizes the CRD by deleting all the CRs associated with it. +// CRDFinalizer is a controller that finalizes the CRD by deleting all the CRs associated with it. type CRDFinalizer struct { - crdClient client.CustomResourceDefinitionsGetter - // clientPool is a dynamic client used to delete the individual instances - clientPool dynamic.ClientPool + crdClient client.CustomResourceDefinitionsGetter + crClientGetter CRClientGetter crdLister listers.CustomResourceDefinitionLister crdSynced cache.InformerSynced @@ -60,17 +59,31 @@ type CRDFinalizer struct { queue workqueue.RateLimitingInterface } +// ListerCollectionDeleter combines rest.Lister and rest.CollectionDeleter. +type ListerCollectionDeleter interface { + rest.Lister + rest.CollectionDeleter +} + +// CRClientGetter knows how to get a ListerCollectionDeleter for a given CRD UID. +type CRClientGetter interface { + // GetCustomResourceListerCollectionDeleter gets the ListerCollectionDeleter for the given CRD + // UID. + GetCustomResourceListerCollectionDeleter(uid types.UID) ListerCollectionDeleter +} + +// NewCRDFinalizer creates a new CRDFinalizer. func NewCRDFinalizer( crdInformer informers.CustomResourceDefinitionInformer, crdClient client.CustomResourceDefinitionsGetter, - clientPool dynamic.ClientPool, + crClientGetter CRClientGetter, ) *CRDFinalizer { c := &CRDFinalizer{ - crdClient: crdClient, - clientPool: clientPool, - crdLister: crdInformer.Lister(), - crdSynced: crdInformer.Informer().HasSynced, - queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "CustomResourceDefinition-CRDFinalizer"), + crdClient: crdClient, + crdLister: crdInformer.Lister(), + crdSynced: crdInformer.Informer().HasSynced, + crClientGetter: crClientGetter, + queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "CustomResourceDefinition-CRDFinalizer"), } crdInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ @@ -114,7 +127,7 @@ func (c *CRDFinalizer) sync(key string) error { return err } - // Its possible for a naming conflict to have removed this resource from the API after instances were created. + // It's possible for a naming conflict to have removed this resource from the API after instances were created. // For now we will cowardly stop finalizing. If we don't go through the REST API, weird things may happen: // no audit trail, no admission checks or side effects, finalization would probably still work but defaulting // would be missed. It would be a mess. @@ -135,22 +148,15 @@ func (c *CRDFinalizer) sync(key string) error { return fmt.Errorf("cannot proceed with deletion because of %v condition", apiextensions.NameConflict) } - // Now we can start deleting items. We should use the REST API to ensure that all normal admission runs. - // Since we control the endpoints, we know that delete collection works. - crClient, err := c.clientPool.ClientForGroupVersionResource(schema.GroupVersionResource{Group: crd.Spec.Group, Version: crd.Spec.Version, Resource: crd.Status.AcceptedNames.Plural}) - if err != nil { - return err + // Now we can start deleting items. While it would be ideal to use a REST API client, doing so + // could incorrectly delete a ThirdPartyResource with the same URL as the CustomResource, so we go + // directly to the storage instead. Since we control the storage, we know that delete collection works. + crClient := c.crClientGetter.GetCustomResourceListerCollectionDeleter(crd.UID) + if crClient == nil { + return fmt.Errorf("unable to find a custom resource client for %s.%s", crd.Status.AcceptedNames.Plural, crd.Spec.Group) } - crAPIResource := &metav1.APIResource{ - Name: crd.Status.AcceptedNames.Plural, - SingularName: crd.Status.AcceptedNames.Singular, - Namespaced: crd.Spec.Scope == apiextensions.NamespaceScoped, - Kind: crd.Status.AcceptedNames.Kind, - Verbs: metav1.Verbs([]string{"deletecollection", "list"}), - ShortNames: crd.Status.AcceptedNames.ShortNames, - } - crResourceClient := crClient.Resource(crAPIResource, "" /* namespace all */) - allResources, err := crResourceClient.List(metav1.ListOptions{}) + ctx := genericapirequest.NewContext() + allResources, err := crClient.List(ctx, nil) if err != nil { return err } @@ -168,7 +174,8 @@ func (c *CRDFinalizer) sync(key string) error { } // don't retry deleting the same namespace deletedNamespaces.Insert(metadata.GetNamespace()) - if err := crClient.Resource(crAPIResource, metadata.GetNamespace()).DeleteCollection(nil, metav1.ListOptions{}); err != nil { + nsCtx := genericapirequest.WithNamespace(ctx, metadata.GetNamespace()) + if _, err := crClient.DeleteCollection(nsCtx, nil, nil); err != nil { deleteErrors = append(deleteErrors, err) continue } @@ -191,7 +198,7 @@ func (c *CRDFinalizer) sync(key string) error { // TODO not all servers are synchronized on caches. It is possible for a stale one to still be creating things. // Once we have a mechanism for servers to indicate their states, we should check that for concurrence. listErr := wait.PollImmediate(5*time.Second, 1*time.Minute, func() (bool, error) { - listObj, err := crResourceClient.List(metav1.ListOptions{}) + listObj, err := crClient.List(ctx, nil) if err != nil { return false, err }