From e52ebc13ffb4eba2df1033b1c3ffaaa1d2f28b0e Mon Sep 17 00:00:00 2001 From: David Eads Date: Mon, 18 Jun 2018 09:16:07 -0400 Subject: [PATCH] MFOJTIK: make project request endpoint use dynamic client instead of kubectl based client --- .../origin/admission/plugin_initializer.go | 46 ++++------- pkg/cmd/server/origin/master_config.go | 20 ++++- pkg/cmd/server/origin/openshift_apiserver.go | 6 ++ pkg/project/apiserver/apiserver.go | 10 ++- .../projectrequest/delegated/delegated.go | 78 ++++++++++--------- 5 files changed, 90 insertions(+), 70 deletions(-) diff --git a/pkg/cmd/server/origin/admission/plugin_initializer.go b/pkg/cmd/server/origin/admission/plugin_initializer.go index ecb307e5eb28..9a43fe19042f 100644 --- a/pkg/cmd/server/origin/admission/plugin_initializer.go +++ b/pkg/cmd/server/origin/admission/plugin_initializer.go @@ -4,7 +4,6 @@ import ( "fmt" "io/ioutil" "os" - "time" authorizationclient "github.com/openshift/client-go/authorization/clientset/versioned" buildclient "github.com/openshift/client-go/build/clientset/versioned" @@ -27,15 +26,11 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/admission/initializer" webhookconfig "k8s.io/apiserver/pkg/admission/plugin/webhook/config" webhookinitializer "k8s.io/apiserver/pkg/admission/plugin/webhook/initializer" "k8s.io/apiserver/pkg/authorization/authorizer" - genericapiserver "k8s.io/apiserver/pkg/server" - "k8s.io/client-go/discovery" - cacheddiscovery "k8s.io/client-go/discovery/cached" kexternalinformers "k8s.io/client-go/informers" kubeclientgoclient "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" @@ -63,39 +58,40 @@ func NewPluginInitializer( informers InformerAccess, authorizer authorizer.Authorizer, projectCache *projectcache.ProjectCache, + restMapper meta.RESTMapper, clusterQuotaMappingController *clusterquotamapping.ClusterQuotaMappingController, -) (admission.PluginInitializer, genericapiserver.PostStartHookFunc, error) { +) (admission.PluginInitializer, error) { kubeInternalClient, err := kclientsetinternal.NewForConfig(privilegedLoopbackConfig) if err != nil { - return nil, nil, err + return nil, err } kubeClientGoClientSet, err := kubeclientgoclient.NewForConfig(privilegedLoopbackConfig) if err != nil { - return nil, nil, err + return nil, err } authorizationClient, err := authorizationclient.NewForConfig(privilegedLoopbackConfig) if err != nil { - return nil, nil, err + return nil, err } buildClient, err := buildclient.NewForConfig(privilegedLoopbackConfig) if err != nil { - return nil, nil, err + return nil, err } imageClient, err := imageclient.NewForConfig(privilegedLoopbackConfig) if err != nil { - return nil, nil, err + return nil, err } quotaClient, err := quotaclient.NewForConfig(privilegedLoopbackConfig) if err != nil { - return nil, nil, err + return nil, err } templateClient, err := templateclient.NewForConfig(privilegedLoopbackConfig) if err != nil { - return nil, nil, err + return nil, err } userClient, err := userclient.NewForConfig(privilegedLoopbackConfig) if err != nil { - return nil, nil, err + return nil, err } // TODO make a union registry @@ -112,18 +108,14 @@ func NewPluginInitializer( svcCache := service.NewServiceResolverCache(kubeInternalClient.Core().Services(metav1.NamespaceDefault).Get) defaultRegistryFunc, err := svcCache.Defer(defaultRegistry) if err != nil { - return nil, nil, fmt.Errorf("OPENSHIFT_DEFAULT_REGISTRY variable is invalid %q: %v", defaultRegistry, err) + return nil, fmt.Errorf("OPENSHIFT_DEFAULT_REGISTRY variable is invalid %q: %v", defaultRegistry, err) } - // Use a discovery client capable of being refreshed. - discoveryClient := cacheddiscovery.NewMemCacheClient(kubeInternalClient.Discovery()) - restMapper := discovery.NewDeferredDiscoveryRESTMapper(discoveryClient, meta.InterfacesForUnstructured) - // punch through layers to build this in order to get a string for a cloud provider file // TODO refactor us into a forward building flow with a side channel like this kubeOptions, err := kubernetes.BuildKubeAPIserverOptions(options) if err != nil { - return nil, nil, err + return nil, err } var cloudConfig []byte @@ -131,7 +123,7 @@ func NewPluginInitializer( var err error cloudConfig, err = ioutil.ReadFile(kubeOptions.CloudProvider.CloudConfigFile) if err != nil { - return nil, nil, fmt.Errorf("Error reading from cloud configuration file %s: %v", kubeOptions.CloudProvider.CloudConfigFile, err) + return nil, fmt.Errorf("Error reading from cloud configuration file %s: %v", kubeOptions.CloudProvider.CloudConfigFile, err) } } // note: we are passing a combined quota registry here... @@ -190,17 +182,7 @@ func NewPluginInitializer( UserInformers: informers.GetUserInformers(), } - return admission.PluginInitializers{genericInitializer, webhookInitializer, kubePluginInitializer, openshiftPluginInitializer}, - func(context genericapiserver.PostStartHookContext) error { - restMapper.Reset() - go func() { - wait.Until(func() { - restMapper.Reset() - }, 10*time.Second, context.StopCh) - }() - return nil - }, - nil + return admission.PluginInitializers{genericInitializer, webhookInitializer, kubePluginInitializer, openshiftPluginInitializer}, nil } // env returns an environment variable, or the defaultValue if it is not set. diff --git a/pkg/cmd/server/origin/master_config.go b/pkg/cmd/server/origin/master_config.go index aba78737fa25..c6931317b146 100644 --- a/pkg/cmd/server/origin/master_config.go +++ b/pkg/cmd/server/origin/master_config.go @@ -2,9 +2,13 @@ package origin import ( "fmt" + "time" "github.com/golang/glog" "github.com/openshift/origin/pkg/admission/namespaceconditions" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/client-go/restmapper" kapierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -71,6 +75,7 @@ type MasterConfig struct { ProjectCache *projectcache.ProjectCache ClusterQuotaMappingController *clusterquotamapping.ClusterQuotaMappingController LimitVerifier imageadmission.LimitVerifier + RESTMapper meta.RESTMapper // RegistryHostnameRetriever retrieves the name of the integrated registry, or false if no such registry // is available. @@ -162,7 +167,9 @@ func BuildMasterConfig( return nil, err } clusterQuotaMappingController := newClusterQuotaMappingController(informers) - admissionInitializer, admissionPostStartHook, err := originadmission.NewPluginInitializer(options, privilegedLoopbackConfig, informers, authorizer, projectCache, clusterQuotaMappingController) + discoveryClient := cacheddiscovery.NewMemCacheClient(kubeInternalClient.Discovery()) + restMapper := restmapper.NewDeferredDiscoveryRESTMapper(discoveryClient) + admissionInitializer, err := originadmission.NewPluginInitializer(options, privilegedLoopbackConfig, informers, authorizer, projectCache, restMapper, clusterQuotaMappingController) if err != nil { return nil, err } @@ -199,7 +206,15 @@ func BuildMasterConfig( kubeAPIServerConfig: kubeAPIServerConfig, additionalPostStartHooks: map[string]genericapiserver.PostStartHookFunc{ - "openshift.io-AdmissionInit": admissionPostStartHook, + "openshift.io-RESTMapper": func(context genericapiserver.PostStartHookContext) error { + restMapper.Reset() + go func() { + wait.Until(func() { + restMapper.Reset() + }, 10*time.Second, context.StopCh) + }() + return nil + }, "openshift.io-StartInformers": func(context genericapiserver.PostStartHookContext) error { informers.Start(context.StopCh) return nil @@ -218,6 +233,7 @@ func BuildMasterConfig( ), ProjectCache: projectCache, ClusterQuotaMappingController: clusterQuotaMappingController, + RESTMapper: restMapper, RegistryHostnameRetriever: imageapi.DefaultRegistryHostnameRetriever(defaultRegistryFunc, options.ImagePolicyConfig.ExternalRegistryHostname, options.ImagePolicyConfig.InternalRegistryHostname), diff --git a/pkg/cmd/server/origin/openshift_apiserver.go b/pkg/cmd/server/origin/openshift_apiserver.go index bc9279d6860a..3973fccec877 100644 --- a/pkg/cmd/server/origin/openshift_apiserver.go +++ b/pkg/cmd/server/origin/openshift_apiserver.go @@ -9,6 +9,7 @@ import ( restful "github.com/emicklei/go-restful" "github.com/golang/glog" + "k8s.io/apimachinery/pkg/api/meta" kapierror "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -101,6 +102,7 @@ type OpenshiftAPIExtraConfig struct { ProjectCache *projectcache.ProjectCache ProjectRequestTemplate string ProjectRequestMessage string + RESTMapper meta.RESTMapper // oauth API server ServiceAccountMethod configapi.GrantHandlerType @@ -155,6 +157,9 @@ func (c *OpenshiftAPIExtraConfig) Validate() error { if c.ClusterQuotaMappingController == nil { ret = append(ret, fmt.Errorf("ClusterQuotaMappingController is required")) } + if c.RESTMapper == nil { + ret = append(ret, fmt.Errorf("RESTMapper is required")) + } return utilerrors.NewAggregate(ret) } @@ -369,6 +374,7 @@ func (c *completedConfig) withProjectAPIServer(delegateAPIServer genericapiserve ProjectCache: c.ExtraConfig.ProjectCache, ProjectRequestTemplate: c.ExtraConfig.ProjectRequestTemplate, ProjectRequestMessage: c.ExtraConfig.ProjectRequestMessage, + RESTMapper: c.ExtraConfig.RESTMapper, Codecs: legacyscheme.Codecs, Scheme: legacyscheme.Scheme, }, diff --git a/pkg/project/apiserver/apiserver.go b/pkg/project/apiserver/apiserver.go index 4bec73f9423b..195115b46234 100644 --- a/pkg/project/apiserver/apiserver.go +++ b/pkg/project/apiserver/apiserver.go @@ -4,6 +4,8 @@ import ( "sync" "github.com/golang/glog" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/client-go/dynamic" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -32,6 +34,7 @@ type ExtraConfig struct { ProjectCache *projectcache.ProjectCache ProjectRequestTemplate string ProjectRequestMessage string + RESTMapper meta.RESTMapper // TODO these should all become local eventually Scheme *runtime.Scheme @@ -122,6 +125,10 @@ func (c *completedConfig) newV1RESTStorage() (map[string]rest.Storage, error) { if err != nil { return nil, err } + dynamicClient, err := dynamic.NewForConfig(c.ExtraConfig.KubeAPIServerClientConfig) + if err != nil { + return nil, err + } projectStorage := projectproxy.NewREST(kubeInternalClient.Core().Namespaces(), c.ExtraConfig.ProjectAuthorizationCache, c.ExtraConfig.ProjectAuthorizationCache, c.ExtraConfig.ProjectCache) @@ -137,7 +144,8 @@ func (c *completedConfig) newV1RESTStorage() (map[string]rest.Storage, error) { projectClient.Project(), templateClient, authorizationClient.SubjectAccessReviews(), - c.ExtraConfig.KubeAPIServerClientConfig, + dynamicClient, + c.ExtraConfig.RESTMapper, c.ExtraConfig.KubeInternalInformers.Rbac().InternalVersion().RoleBindings().Lister(), ) diff --git a/pkg/project/registry/projectrequest/delegated/delegated.go b/pkg/project/registry/projectrequest/delegated/delegated.go index f47c2c35374c..2cf3c548c051 100644 --- a/pkg/project/registry/projectrequest/delegated/delegated.go +++ b/pkg/project/registry/projectrequest/delegated/delegated.go @@ -7,10 +7,11 @@ import ( "strings" "github.com/golang/glog" - kapierror "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" metainternal "k8s.io/apimachinery/pkg/apis/meta/internalversion" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" utilerrors "k8s.io/apimachinery/pkg/util/errors" utilruntime "k8s.io/apimachinery/pkg/util/runtime" @@ -18,7 +19,7 @@ import ( "k8s.io/apimachinery/pkg/util/wait" apirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/rest" - restclient "k8s.io/client-go/rest" + "k8s.io/client-go/dynamic" "k8s.io/client-go/util/retry" "k8s.io/kubernetes/pkg/api/legacyscheme" authorizationapi "k8s.io/kubernetes/pkg/apis/authorization" @@ -26,11 +27,9 @@ import ( "k8s.io/kubernetes/pkg/apis/rbac" authorizationclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/authorization/internalversion" rbaclisters "k8s.io/kubernetes/pkg/client/listers/rbac/internalversion" - "k8s.io/kubernetes/pkg/kubectl/genericclioptions/resource" osauthorizationapi "github.com/openshift/origin/pkg/authorization/apis/authorization" authorizationutil "github.com/openshift/origin/pkg/authorization/util" - configcmd "github.com/openshift/origin/pkg/bulk" "github.com/openshift/origin/pkg/cmd/server/bootstrappolicy" projectapi "github.com/openshift/origin/pkg/project/apis/project" projectclientinternal "github.com/openshift/origin/pkg/project/generated/internalclientset/typed/project/internalversion" @@ -38,7 +37,6 @@ import ( templateapi "github.com/openshift/origin/pkg/template/apis/template" templateinternalclient "github.com/openshift/origin/pkg/template/client/internalversion" templateclient "github.com/openshift/origin/pkg/template/generated/internalclientset" - restutil "github.com/openshift/origin/pkg/util/rest" ) type REST struct { @@ -49,7 +47,8 @@ type REST struct { sarClient authorizationclient.SubjectAccessReviewInterface projectGetter projectclientinternal.ProjectsGetter templateClient templateclient.Interface - restConfig *restclient.Config + client dynamic.Interface + restMapper meta.RESTMapper // policyBindings is an auth cache that is shared with the authorizer for the API server. // we use this cache to detect when the authorizer has observed the change for the auth rules @@ -59,7 +58,13 @@ type REST struct { var _ rest.Lister = &REST{} var _ rest.Creater = &REST{} -func NewREST(message, templateNamespace, templateName string, projectClient projectclientinternal.ProjectsGetter, templateClient templateclient.Interface, sarClient authorizationclient.SubjectAccessReviewInterface, restConfig *restclient.Config, roleBindings rbaclisters.RoleBindingLister) *REST { +func NewREST(message, templateNamespace, templateName string, + projectClient projectclientinternal.ProjectsGetter, + templateClient templateclient.Interface, + sarClient authorizationclient.SubjectAccessReviewInterface, + client dynamic.Interface, + restMapper meta.RESTMapper, + roleBindings rbaclisters.RoleBindingLister) *REST { return &REST{ message: message, templateNamespace: templateNamespace, @@ -67,7 +72,8 @@ func NewREST(message, templateNamespace, templateName string, projectClient proj projectGetter: projectClient, templateClient: templateClient, sarClient: sarClient, - restConfig: restConfig, + client: client, + restMapper: restMapper, roleBindings: roleBindings, } } @@ -190,36 +196,38 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation return nil, err } - // Stop on the first error, since we have to delete the whole project if any item in the template fails - stopOnErr := configcmd.AfterFunc(func(info *resource.Info, err error) bool { - // if a default role binding already exists, we're probably racing the controller. Don't die - if gvk := info.Mapping.GroupVersionKind; kapierror.IsAlreadyExists(err) && - gvk.Kind == roleBindingKind && roleBindingGroups.Has(gvk.Group) && defaultRoleBindingNames.Has(info.Name) { - return false + // TODO, stop doing this crazy thing, but for now it's a very simple way to get the unstructured objects we need + jsonBytes, err := runtime.Encode(legacyscheme.Codecs.LegacyCodec(legacyscheme.Scheme.PrioritizedVersionsAllGroups()...), objectsToCreate) + if err != nil { + return nil, kapierror.NewInternalError(err) + } + uncastList, err := runtime.Decode(unstructured.UnstructuredJSONScheme, jsonBytes) + if err != nil { + return nil, kapierror.NewInternalError(err) + } + toCreateList := uncastList.(*unstructured.UnstructuredList) + + for _, toCreate := range toCreateList.Items { + restMapping, mappingErr := r.restMapper.RESTMapping(toCreate.GroupVersionKind().GroupKind(), toCreate.GroupVersionKind().Version) + if mappingErr != nil { + utilruntime.HandleError(fmt.Errorf("error creating items in requested project %q: %v", createdProject.Name, mappingErr)) + return nil, kapierror.NewInternalError(mappingErr) } - return err != nil - }) - bulk := configcmd.Bulk{ - Mapper: &resource.Mapper{ - RESTMapper: restutil.DefaultMultiRESTMapper(), - ObjectTyper: legacyscheme.Scheme, - ClientMapper: configcmd.ClientMapperFromConfig(r.restConfig), - }, - IgnoreError: func(err error) bool { - // it is safe to ignore all such errors since stopOnErr will only let these through for the default role bindings - return kapierror.IsAlreadyExists(err) - }, - After: stopOnErr, - Op: configcmd.Create, - } - if err := utilerrors.NewAggregate(bulk.Run(objectsToCreate, createdProject.Name)); err != nil { - utilruntime.HandleError(fmt.Errorf("error creating items in requested project %q: %v", createdProject.Name, err)) - // We have to clean up the project if any part of the project request template fails - if deleteErr := r.projectGetter.Projects().Delete(createdProject.Name, &metav1.DeleteOptions{}); deleteErr != nil { - utilruntime.HandleError(fmt.Errorf("error cleaning up requested project %q: %v", createdProject.Name, deleteErr)) + _, createErr := r.client.Resource(restMapping.Resource).Namespace(createdProject.Name).Create(&toCreate) + // if a default role binding already exists, we're probably racing the controller. Don't die + if gvk := restMapping.GroupVersionKind; kapierror.IsAlreadyExists(createErr) && + gvk.Kind == roleBindingKind && roleBindingGroups.Has(gvk.Group) && defaultRoleBindingNames.Has(toCreate.GetName()) { + continue + } + // it is safe to ignore all such errors since stopOnErr will only let these through for the default role bindings + if kapierror.IsAlreadyExists(createErr) { + continue + } + if createErr != nil { + utilruntime.HandleError(fmt.Errorf("error creating items in requested project %q: %v", createdProject.Name, createErr)) + return nil, kapierror.NewInternalError(createErr) } - return nil, kapierror.NewInternalError(err) } // wait for a rolebinding if we created one