From 2aabf5de02c47c39e6ac6427db08c3841306ae4d Mon Sep 17 00:00:00 2001 From: Jim Minter Date: Wed, 21 Jun 2017 14:49:28 +0100 Subject: [PATCH] allow templateinstance controller to instantiate non-v1 objects --- .../jenkinsbootstrapper/admission.go | 37 +-------- pkg/cmd/server/origin/controller/template.go | 6 ++ pkg/cmd/server/origin/storage.go | 2 +- pkg/config/cmd/cmd.go | 42 ++++++++++ .../projectrequest/delegated/delegated.go | 21 ++--- .../controller/templateinstance_controller.go | 17 ++-- .../templates/templateinstance_objectkinds.go | 64 +++++++++++++++ test/extended/testdata/bindata.go | 80 +++++++++++++++++++ .../templateinstance_objectkinds.yaml | 58 ++++++++++++++ 9 files changed, 268 insertions(+), 59 deletions(-) create mode 100644 test/extended/templates/templateinstance_objectkinds.go create mode 100644 test/extended/testdata/templates/templateinstance_objectkinds.yaml diff --git a/pkg/build/admission/jenkinsbootstrapper/admission.go b/pkg/build/admission/jenkinsbootstrapper/admission.go index 63c5413beadd..e5969d0d7d2d 100644 --- a/pkg/build/admission/jenkinsbootstrapper/admission.go +++ b/pkg/build/admission/jenkinsbootstrapper/admission.go @@ -6,7 +6,6 @@ import ( "github.com/golang/glog" kapierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" kutilerrors "k8s.io/apimachinery/pkg/util/errors" @@ -15,11 +14,9 @@ import ( kapi "k8s.io/kubernetes/pkg/api" kclientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset" coreclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/core/internalversion" - kclient "k8s.io/kubernetes/pkg/client/unversioned" kadmission "k8s.io/kubernetes/pkg/kubeapiserver/admission" "k8s.io/kubernetes/pkg/kubectl/resource" - "github.com/openshift/origin/pkg/api/latest" authenticationclient "github.com/openshift/origin/pkg/auth/client" buildapi "github.com/openshift/origin/pkg/build/api" jenkinscontroller "github.com/openshift/origin/pkg/build/controller/jenkins" @@ -101,37 +98,9 @@ func (a *jenkinsBootstrapper) Admit(attributes admission.Attributes) error { bulk := &cmd.Bulk{ Mapper: &resource.Mapper{ - RESTMapper: kapi.Registry.RESTMapper(), - ObjectTyper: kapi.Scheme, - ClientMapper: resource.ClientMapperFunc(func(mapping *meta.RESTMapping) (resource.RESTClient, error) { - // TODO this is a nasty copy&paste from pkg/cmd/util/clientcmd/factory_object_mapping.go#ClientForMapping - if latest.OriginKind(mapping.GroupVersionKind) { - if err := client.SetOpenShiftDefaults(&impersonatingConfig); err != nil { - return nil, err - } - impersonatingConfig.APIPath = "/apis" - if mapping.GroupVersionKind.Group == kapi.GroupName { - impersonatingConfig.APIPath = "/oapi" - } - gv := mapping.GroupVersionKind.GroupVersion() - impersonatingConfig.GroupVersion = &gv - return restclient.RESTClientFor(&impersonatingConfig) - } - // TODO and this from vendor/k8s.io/kubernetes/pkg/kubectl/cmd/util/factory_object_mapping.go#ClientForMapping - if err := kclient.SetKubernetesDefaults(&impersonatingConfig); err != nil { - return nil, err - } - gvk := mapping.GroupVersionKind - switch gvk.Group { - case kapi.GroupName: - impersonatingConfig.APIPath = "/api" - default: - impersonatingConfig.APIPath = "/apis" - } - gv := gvk.GroupVersion() - impersonatingConfig.GroupVersion = &gv - return restclient.RESTClientFor(&impersonatingConfig) - }), + RESTMapper: kapi.Registry.RESTMapper(), + ObjectTyper: kapi.Scheme, + ClientMapper: cmd.ClientMapperFromConfig(&impersonatingConfig), }, Op: cmd.Create, After: func(info *resource.Info, err error) bool { diff --git a/pkg/cmd/server/origin/controller/template.go b/pkg/cmd/server/origin/controller/template.go index 906cd9164e59..a1f5f73b2dd0 100644 --- a/pkg/cmd/server/origin/controller/template.go +++ b/pkg/cmd/server/origin/controller/template.go @@ -11,6 +11,11 @@ type TemplateInstanceControllerConfig struct { func (c *TemplateInstanceControllerConfig) RunController(ctx ControllerContext) (bool, error) { saName := bootstrappolicy.InfraTemplateInstanceControllerServiceAccountName + restConfig, err := ctx.ClientBuilder.Config(saName) + if err != nil { + return true, err + } + internalKubeClient, err := ctx.ClientBuilder.KubeInternalClient(saName) if err != nil { return true, err @@ -27,6 +32,7 @@ func (c *TemplateInstanceControllerConfig) RunController(ctx ControllerContext) } go templatecontroller.NewTemplateInstanceController( + restConfig, deprecatedOcClient, internalKubeClient, templateClient.Template(), diff --git a/pkg/cmd/server/origin/storage.go b/pkg/cmd/server/origin/storage.go index 65e1f14f37fd..ed892fb19edb 100644 --- a/pkg/cmd/server/origin/storage.go +++ b/pkg/cmd/server/origin/storage.go @@ -350,7 +350,7 @@ func (c OpenshiftAPIConfig) GetRestStorage() (map[schema.GroupVersion]map[string PolicyBindingLister: c.AuthorizationInformers.Authorization().InternalVersion().PolicyBindings().Lister(), versioner: c.AuthorizationInformers.Authorization().InternalVersion().PolicyBindings().Informer(), } - projectRequestStorage := projectrequeststorage.NewREST(c.ProjectRequestMessage, namespace, templateName, c.DeprecatedOpenshiftClient, c.KubeClientInternal, policyBindings) + projectRequestStorage := projectrequeststorage.NewREST(c.ProjectRequestMessage, namespace, templateName, c.DeprecatedOpenshiftClient, c.GenericConfig.LoopbackClientConfig, policyBindings) buildConfigWebHooks := buildconfigregistry.NewWebHookREST( buildConfigRegistry, diff --git a/pkg/config/cmd/cmd.go b/pkg/config/cmd/cmd.go index cea11e78faef..7144c147db34 100644 --- a/pkg/config/cmd/cmd.go +++ b/pkg/config/cmd/cmd.go @@ -10,9 +10,14 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/rest" kapi "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/client/unversioned" cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" "k8s.io/kubernetes/pkg/kubectl/resource" + + "github.com/openshift/origin/pkg/api/latest" + "github.com/openshift/origin/pkg/client" ) type Runner interface { @@ -99,6 +104,43 @@ func (b *Bulk) Run(list *kapi.List, namespace string) []error { return errs } +// ClientMapperFromConfig returns a ClientMapper suitable for Bulk operations. +// TODO: copied from +// pkg/cmd/util/clientcmd/factory_object_mapping.go#ClientForMapping and +// vendor/k8s.io/kubernetes/pkg/kubectl/cmd/util/factory_object_mapping.go#ClientForMapping +func ClientMapperFromConfig(config *rest.Config) resource.ClientMapperFunc { + return resource.ClientMapperFunc(func(mapping *meta.RESTMapping) (resource.RESTClient, error) { + configCopy := *config + + if latest.OriginKind(mapping.GroupVersionKind) { + if err := client.SetOpenShiftDefaults(&configCopy); err != nil { + return nil, err + } + configCopy.APIPath = "/apis" + if mapping.GroupVersionKind.Group == kapi.GroupName { + configCopy.APIPath = "/oapi" + } + gv := mapping.GroupVersionKind.GroupVersion() + configCopy.GroupVersion = &gv + return rest.RESTClientFor(&configCopy) + } + + if err := unversioned.SetKubernetesDefaults(&configCopy); err != nil { + return nil, err + } + gvk := mapping.GroupVersionKind + switch gvk.Group { + case kapi.GroupName: + configCopy.APIPath = "/api" + default: + configCopy.APIPath = "/apis" + } + gv := gvk.GroupVersion() + configCopy.GroupVersion = &gv + return rest.RESTClientFor(&configCopy) + }) +} + func NewPrintNameOrErrorAfterIndent(mapper meta.RESTMapper, short bool, operation string, out, errs io.Writer, dryRun bool, indent string, prefixForError PrefixForError) AfterFunc { return func(info *resource.Info, err error) bool { if err == nil { diff --git a/pkg/project/registry/projectrequest/delegated/delegated.go b/pkg/project/registry/projectrequest/delegated/delegated.go index 43fb10229c08..28d82115181a 100644 --- a/pkg/project/registry/projectrequest/delegated/delegated.go +++ b/pkg/project/registry/projectrequest/delegated/delegated.go @@ -8,7 +8,6 @@ import ( "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/labels" @@ -18,12 +17,11 @@ 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" kapi "k8s.io/kubernetes/pkg/api" - kclientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset" "k8s.io/kubernetes/pkg/client/retry" "k8s.io/kubernetes/pkg/kubectl/resource" - "github.com/openshift/origin/pkg/api/latest" authorizationapi "github.com/openshift/origin/pkg/authorization/api" authorizationlister "github.com/openshift/origin/pkg/authorization/generated/listers/authorization/internalversion" "github.com/openshift/origin/pkg/client" @@ -39,20 +37,20 @@ type REST struct { templateName string openshiftClient *client.Client - kubeClient kclientset.Interface + restConfig *restclient.Config // 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 policyBindings authorizationlister.PolicyBindingLister } -func NewREST(message, templateNamespace, templateName string, openshiftClient *client.Client, kubeClient kclientset.Interface, policyBindingCache authorizationlister.PolicyBindingLister) *REST { +func NewREST(message, templateNamespace, templateName string, openshiftClient *client.Client, restConfig *restclient.Config, policyBindingCache authorizationlister.PolicyBindingLister) *REST { return &REST{ message: message, templateNamespace: templateNamespace, templateName: templateName, openshiftClient: openshiftClient, - kubeClient: kubeClient, + restConfig: restConfig, policyBindings: policyBindingCache, } } @@ -169,14 +167,9 @@ func (r *REST) Create(ctx apirequest.Context, obj runtime.Object) (runtime.Objec bulk := configcmd.Bulk{ Mapper: &resource.Mapper{ - RESTMapper: client.DefaultMultiRESTMapper(), - ObjectTyper: kapi.Scheme, - ClientMapper: resource.ClientMapperFunc(func(mapping *meta.RESTMapping) (resource.RESTClient, error) { - if latest.OriginKind(mapping.GroupVersionKind) { - return r.openshiftClient, nil - } - return r.kubeClient.Core().RESTClient(), nil - }), + RESTMapper: client.DefaultMultiRESTMapper(), + ObjectTyper: kapi.Scheme, + ClientMapper: configcmd.ClientMapperFromConfig(r.restConfig), }, After: stopOnErr, Op: configcmd.Create, diff --git a/pkg/template/controller/templateinstance_controller.go b/pkg/template/controller/templateinstance_controller.go index 6b0d42012daa..7f87427e5b3d 100644 --- a/pkg/template/controller/templateinstance_controller.go +++ b/pkg/template/controller/templateinstance_controller.go @@ -14,6 +14,7 @@ import ( utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apiserver/pkg/authentication/user" + "k8s.io/client-go/rest" "k8s.io/client-go/tools/cache" "k8s.io/client-go/util/workqueue" kapi "k8s.io/kubernetes/pkg/api" @@ -23,7 +24,6 @@ import ( "github.com/golang/glog" - "github.com/openshift/origin/pkg/api/latest" "github.com/openshift/origin/pkg/authorization/util" "github.com/openshift/origin/pkg/client" "github.com/openshift/origin/pkg/config/cmd" @@ -40,6 +40,7 @@ import ( // using its own service account, first verifying that the requester also has // permissions to instantiate. type TemplateInstanceController struct { + config *rest.Config oc client.Interface kc kclientsetinternal.Interface templateclient internalversiontemplate.TemplateInterface @@ -51,8 +52,9 @@ type TemplateInstanceController struct { } // NewTemplateInstanceController returns a new TemplateInstanceController. -func NewTemplateInstanceController(oc client.Interface, kc kclientsetinternal.Interface, templateclient internalversiontemplate.TemplateInterface, informer internalversion.TemplateInstanceInformer) *TemplateInstanceController { +func NewTemplateInstanceController(config *rest.Config, oc client.Interface, kc kclientsetinternal.Interface, templateclient internalversiontemplate.TemplateInterface, informer internalversion.TemplateInstanceInformer) *TemplateInstanceController { c := &TemplateInstanceController{ + config: config, oc: oc, kc: kc, templateclient: templateclient, @@ -304,14 +306,9 @@ func (c *TemplateInstanceController) instantiate(templateInstance *templateapi.T bulk := cmd.Bulk{ Mapper: &resource.Mapper{ - RESTMapper: client.DefaultMultiRESTMapper(), - ObjectTyper: kapi.Scheme, - ClientMapper: resource.ClientMapperFunc(func(mapping *meta.RESTMapping) (resource.RESTClient, error) { - if latest.OriginKind(mapping.GroupVersionKind) { - return c.oc.(*client.Client), nil - } - return c.kc.Core().RESTClient(), nil - }), + RESTMapper: client.DefaultMultiRESTMapper(), + ObjectTyper: kapi.Scheme, + ClientMapper: cmd.ClientMapperFromConfig(c.config), }, Op: func(info *resource.Info, namespace string, obj runtime.Object) (runtime.Object, error) { if len(info.Namespace) > 0 { diff --git a/test/extended/templates/templateinstance_objectkinds.go b/test/extended/templates/templateinstance_objectkinds.go new file mode 100644 index 000000000000..7d1d0d46210f --- /dev/null +++ b/test/extended/templates/templateinstance_objectkinds.go @@ -0,0 +1,64 @@ +package templates + +import ( + "fmt" + "time" + + g "github.com/onsi/ginkgo" + o "github.com/onsi/gomega" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/wait" + kapi "k8s.io/kubernetes/pkg/api" + + exutil "github.com/openshift/origin/test/extended/util" +) + +// ensure that we can instantiate Kubernetes and OpenShift objects, legacy and +// non-legacy, from a range of API groups. +var _ = g.Describe("[templates] templateinstance object kinds test", func() { + defer g.GinkgoRecover() + + var ( + fixture = exutil.FixturePath("testdata", "templates", "templateinstance_objectkinds.yaml") + cli = exutil.NewCLI("templates", exutil.KubeConfigPath()) + ) + + g.It("should create objects from varying API groups", func() { + err := cli.Run("create").Args("-f", fixture).Execute() + o.Expect(err).NotTo(o.HaveOccurred()) + + // wait for templateinstance controller to do its thing + err = wait.Poll(time.Second, time.Minute, func() (bool, error) { + templateinstance, err := cli.TemplateClient().Template().TemplateInstances(cli.Namespace()).Get("templateinstance", metav1.GetOptions{}) + if err != nil { + return false, err + } + + for _, c := range templateinstance.Status.Conditions { + if c.Reason == "Failed" && c.Status == kapi.ConditionTrue { + return false, fmt.Errorf("failed condition: %s", c.Message) + } + if c.Reason == "Created" && c.Status == kapi.ConditionTrue { + return true, nil + } + } + + return false, nil + }) + o.Expect(err).NotTo(o.HaveOccurred()) + + // check everything was created as expected + _, err = cli.KubeClient().CoreV1().Secrets(cli.Namespace()).Get("secret", metav1.GetOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + _, err = cli.KubeClient().AppsV1beta1().Deployments(cli.Namespace()).Get("deployment", metav1.GetOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + _, err = cli.Client().Routes(cli.Namespace()).Get("route", metav1.GetOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + _, err = cli.Client().Routes(cli.Namespace()).Get("newroute", metav1.GetOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + }) +}) diff --git a/test/extended/testdata/bindata.go b/test/extended/testdata/bindata.go index 78635216b028..3c580a5c8778 100644 --- a/test/extended/testdata/bindata.go +++ b/test/extended/testdata/bindata.go @@ -139,6 +139,7 @@ // test/extended/testdata/sti-environment-build-app/.sti/environment // test/extended/testdata/sti-environment-build-app/Gemfile // test/extended/testdata/sti-environment-build-app/config.ru +// test/extended/testdata/templates/templateinstance_objectkinds.yaml // test/extended/testdata/test-auth-build.yaml // test/extended/testdata/test-bc-with-pr-ref.yaml // test/extended/testdata/test-build-app/Dockerfile @@ -7293,6 +7294,81 @@ func testExtendedTestdataStiEnvironmentBuildAppConfigRu() (*asset, error) { return a, nil } +var _testExtendedTestdataTemplatesTemplateinstance_objectkindsYaml = []byte(`kind: List +apiVersion: v1 +items: +- kind: Secret + apiVersion: v1 + metadata: + name: configsecret + +- kind: TemplateInstance + apiVersion: template.openshift.io/v1 + metadata: + name: templateinstance + spec: + template: + kind: Template + apiVersion: v1 + metadata: + name: template + namespace: default + objects: + - kind: Secret + apiVersion: v1 + metadata: + name: secret + - kind: Deployment + apiVersion: apps/v1beta1 + metadata: + name: deployment + spec: + replicas: 0 + selector: + matchLabels: + key: value + template: + metadata: + labels: + key: value + spec: + containers: + - name: hello-openshift + image: openshift/hello-openshift + - kind: Route + apiVersion: v1 + metadata: + name: route + spec: + to: + name: foo + - kind: Route + apiVersion: route.openshift.io/v1 + metadata: + name: newroute + spec: + to: + name: foo + + secret: + name: configsecret +`) + +func testExtendedTestdataTemplatesTemplateinstance_objectkindsYamlBytes() ([]byte, error) { + return _testExtendedTestdataTemplatesTemplateinstance_objectkindsYaml, nil +} + +func testExtendedTestdataTemplatesTemplateinstance_objectkindsYaml() (*asset, error) { + bytes, err := testExtendedTestdataTemplatesTemplateinstance_objectkindsYamlBytes() + if err != nil { + return nil, err + } + + info := bindataFileInfo{name: "test/extended/testdata/templates/templateinstance_objectkinds.yaml", size: 0, mode: os.FileMode(0), modTime: time.Unix(0, 0)} + a := &asset{bytes: bytes, info: info} + return a, nil +} + var _testExtendedTestdataTestAuthBuildYaml = []byte(`apiVersion: v1 kind: Template labels: @@ -20303,6 +20379,7 @@ var _bindata = map[string]func() (*asset, error){ "test/extended/testdata/sti-environment-build-app/.sti/environment": testExtendedTestdataStiEnvironmentBuildAppStiEnvironment, "test/extended/testdata/sti-environment-build-app/Gemfile": testExtendedTestdataStiEnvironmentBuildAppGemfile, "test/extended/testdata/sti-environment-build-app/config.ru": testExtendedTestdataStiEnvironmentBuildAppConfigRu, + "test/extended/testdata/templates/templateinstance_objectkinds.yaml": testExtendedTestdataTemplatesTemplateinstance_objectkindsYaml, "test/extended/testdata/test-auth-build.yaml": testExtendedTestdataTestAuthBuildYaml, "test/extended/testdata/test-bc-with-pr-ref.yaml": testExtendedTestdataTestBcWithPrRefYaml, "test/extended/testdata/test-build-app/Dockerfile": testExtendedTestdataTestBuildAppDockerfile, @@ -20701,6 +20778,9 @@ var _bintree = &bintree{nil, map[string]*bintree{ "Gemfile": &bintree{testExtendedTestdataStiEnvironmentBuildAppGemfile, map[string]*bintree{}}, "config.ru": &bintree{testExtendedTestdataStiEnvironmentBuildAppConfigRu, map[string]*bintree{}}, }}, + "templates": &bintree{nil, map[string]*bintree{ + "templateinstance_objectkinds.yaml": &bintree{testExtendedTestdataTemplatesTemplateinstance_objectkindsYaml, map[string]*bintree{}}, + }}, "test-auth-build.yaml": &bintree{testExtendedTestdataTestAuthBuildYaml, map[string]*bintree{}}, "test-bc-with-pr-ref.yaml": &bintree{testExtendedTestdataTestBcWithPrRefYaml, map[string]*bintree{}}, "test-build-app": &bintree{nil, map[string]*bintree{ diff --git a/test/extended/testdata/templates/templateinstance_objectkinds.yaml b/test/extended/testdata/templates/templateinstance_objectkinds.yaml new file mode 100644 index 000000000000..e07c86a7c507 --- /dev/null +++ b/test/extended/testdata/templates/templateinstance_objectkinds.yaml @@ -0,0 +1,58 @@ +kind: List +apiVersion: v1 +items: +- kind: Secret + apiVersion: v1 + metadata: + name: configsecret + +- kind: TemplateInstance + apiVersion: template.openshift.io/v1 + metadata: + name: templateinstance + spec: + template: + kind: Template + apiVersion: v1 + metadata: + name: template + namespace: default + objects: + - kind: Secret + apiVersion: v1 + metadata: + name: secret + - kind: Deployment + apiVersion: apps/v1beta1 + metadata: + name: deployment + spec: + replicas: 0 + selector: + matchLabels: + key: value + template: + metadata: + labels: + key: value + spec: + containers: + - name: hello-openshift + image: openshift/hello-openshift + - kind: Route + apiVersion: v1 + metadata: + name: route + spec: + to: + name: foo + - kind: Route + apiVersion: route.openshift.io/v1 + metadata: + name: newroute + spec: + to: + name: foo + + secret: + name: configsecret