From f2d70f96221ec4722a338f8a6b36a0c6d0906968 Mon Sep 17 00:00:00 2001 From: Eric Stroczynski Date: Tue, 23 Apr 2019 14:49:01 -0700 Subject: [PATCH 1/9] cmd/operator-sdk/add/controller.go: --k8s-api and --k8s-import-path for built-in Kubernetes API controllers internal/pkg/scaffold/controller_kind*.go: set imports at runtime as to respect a Kubernetes API controller import --- cmd/operator-sdk/add/controller.go | 15 +++++- internal/pkg/scaffold/controller_kind.go | 52 +++++++++++++------ internal/pkg/scaffold/controller_kind_test.go | 1 - 3 files changed, 50 insertions(+), 18 deletions(-) diff --git a/cmd/operator-sdk/add/controller.go b/cmd/operator-sdk/add/controller.go index fde3646779..6d2a4c6777 100644 --- a/cmd/operator-sdk/add/controller.go +++ b/cmd/operator-sdk/add/controller.go @@ -25,6 +25,11 @@ import ( "github.com/spf13/cobra" ) +var ( + k8sAPI bool + k8sImportPath string +) + func newAddControllerCmd() *cobra.Command { controllerCmd := &cobra.Command{ Use: "controller", @@ -57,6 +62,8 @@ Example: if err := controllerCmd.MarkFlagRequired("kind"); err != nil { log.Fatalf("Failed to mark `kind` flag for `add controller` subcommand as required") } + controllerCmd.Flags().StringVar(&k8sImportPath, "k8s-import-path", "k8s.io/api", "Kubernetes resource import path. Only valid if --k8s-api is set") + controllerCmd.Flags().BoolVar(&k8sAPI, "k8s-api", false, "The provided kind for api-version is a Kubernetes resource API") return controllerCmd } @@ -81,10 +88,14 @@ func controllerRun(cmd *cobra.Command, args []string) error { Repo: projutil.CheckAndGetProjectGoPkg(), AbsProjectPath: projutil.MustGetwd(), } - s := &scaffold.Scaffold{} + + ck := &scaffold.ControllerKind{Resource: r} + if k8sAPI { + ck.K8sImportPath = k8sImportPath + } err = s.Execute(cfg, - &scaffold.ControllerKind{Resource: r}, + ck, &scaffold.AddController{Resource: r}, ) if err != nil { diff --git a/internal/pkg/scaffold/controller_kind.go b/internal/pkg/scaffold/controller_kind.go index 38529fa9b5..1ee9bd7bc0 100644 --- a/internal/pkg/scaffold/controller_kind.go +++ b/internal/pkg/scaffold/controller_kind.go @@ -15,6 +15,7 @@ package scaffold import ( + "path" "path/filepath" "github.com/operator-framework/operator-sdk/internal/pkg/scaffold/input" @@ -26,6 +27,12 @@ type ControllerKind struct { // Resource defines the inputs for the controller's primary resource Resource *Resource + // K8sImportPath holds the import path for a built-in Kubernetes API that + // this controller reconciles if specified by the scaffold invoker. + K8sImportPath string + // ImportMap maps all imports destined for the scaffold to their import + // identifier, if any. + ImportMap map[string]string } func (s *ControllerKind) GetInput() (input.Input, error) { @@ -36,29 +43,44 @@ func (s *ControllerKind) GetInput() (input.Input, error) { // Error if this file exists. s.IfExistsAction = input.Error s.TemplateBody = controllerKindTemplate + + // Set imports. + if len(s.ImportMap) == 0 { + s.ImportMap = controllerKindImports + } + importPrefix := path.Join(s.Repo, "pkg", "apis") + if s.K8sImportPath != "" { + importPrefix = s.K8sImportPath + } + importPath := path.Join(importPrefix, s.Resource.GoImportGroup, s.Resource.Version) + s.ImportMap[importPath] = s.Resource.GoImportGroup + s.Resource.Version return s.Input, nil } +var controllerKindImports = map[string]string{ + "k8s.io/api/core/v1": "corev1", + "k8s.io/apimachinery/pkg/api/errors": "", + "k8s.io/apimachinery/pkg/apis/meta/v1": "metav1", + "k8s.io/apimachinery/pkg/runtime": "", + "k8s.io/apimachinery/pkg/types": "", + "sigs.k8s.io/controller-runtime/pkg/client": "", + "sigs.k8s.io/controller-runtime/pkg/controller": "", + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil": "", + "sigs.k8s.io/controller-runtime/pkg/handler": "", + "sigs.k8s.io/controller-runtime/pkg/manager": "", + "sigs.k8s.io/controller-runtime/pkg/reconcile": "", + "sigs.k8s.io/controller-runtime/pkg/runtime/log": "logf", + "sigs.k8s.io/controller-runtime/pkg/source": "", +} + const controllerKindTemplate = `package {{ .Resource.LowerKind }} import ( "context" - {{ .Resource.GoImportGroup}}{{ .Resource.Version }} "{{ .Repo }}/pkg/apis/{{ .Resource.GoImportGroup}}/{{ .Resource.Version }}" - - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/types" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/controller" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - "sigs.k8s.io/controller-runtime/pkg/handler" - "sigs.k8s.io/controller-runtime/pkg/manager" - "sigs.k8s.io/controller-runtime/pkg/reconcile" - logf "sigs.k8s.io/controller-runtime/pkg/runtime/log" - "sigs.k8s.io/controller-runtime/pkg/source" + {{range $p, $i := .ImportMap -}} + {{$i}} "{{$p}}" + {{end}} ) var log = logf.Log.WithName("controller_{{ .Resource.LowerKind }}") diff --git a/internal/pkg/scaffold/controller_kind_test.go b/internal/pkg/scaffold/controller_kind_test.go index bae8254539..41baa8a22a 100644 --- a/internal/pkg/scaffold/controller_kind_test.go +++ b/internal/pkg/scaffold/controller_kind_test.go @@ -43,7 +43,6 @@ import ( "context" appv1alpha1 "github.com/example-inc/app-operator/pkg/apis/app/v1alpha1" - corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" From 4e1aded74ab4de6adf1dcf746feb28c4f76886c7 Mon Sep 17 00:00:00 2001 From: Eric Stroczynski Date: Tue, 23 Apr 2019 15:56:45 -0700 Subject: [PATCH 2/9] doc/sdk-cli-reference.md: add --k8s-api and --k8s-import-path to 'add controller' --- doc/sdk-cli-reference.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/sdk-cli-reference.md b/doc/sdk-cli-reference.md index 691263c229..645e02938b 100644 --- a/doc/sdk-cli-reference.md +++ b/doc/sdk-cli-reference.md @@ -339,6 +339,8 @@ Adds a new controller under `pkg/controller//...` that, by default, reconc * `--api-version` string - CRD APIVersion in the format `$GROUP_NAME/$VERSION` (e.g app.example.com/v1alpha1) * `--kind` string - CRD Kind. (e.g AppService) +* `--k8s-api` - The provided kind for api-version is a Kubernetes resource API +* `--k8s-import-path` - Kubernetes resource import path. Only valid if --k8s-api is set (default `k8s.io/api`) #### Example From 10750fc4820467e7ef86351746e24b1a14efc769 Mon Sep 17 00:00:00 2001 From: Eric Stroczynski Date: Tue, 23 Apr 2019 15:57:22 -0700 Subject: [PATCH 3/9] CHANGELOG.md: additions --k8s-api and --k8s-import-path for 'add controller' --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c195c21e7..9cbcc95d9a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - New option for [`operator-sdk build --image-builder`](https://github.com/operator-framework/operator-sdk/blob/master/doc/sdk-cli-reference.md#build), which can be used to specify which image builder to use. Adds support for [buildah](https://github.com/containers/buildah/). ([#1311](https://github.com/operator-framework/operator-sdk/pull/1311)) - Manager is now configured with a new `DynamicRESTMapper`, which accounts for the fact that the default `RESTMapper`, which only checks resource types at startup, can't handle the case of first creating a CRD and then an instance of that CRD. ([#1329](https://github.com/operator-framework/operator-sdk/pull/1329)) +- New optional flags `--k8s-api` and `--k8s-import-path` for [`operator-sdk add controller`](https://github.com/operator-framework/operator-sdk/blob/master/doc/sdk-cli-reference.md#controller) to specify that the new controller reconciles a built-in Kubernetes API. ([#1344](https://github.com/operator-framework/operator-sdk/pull/1344)) ### Changed From 03d0aa30abac75274ffa8caae5c72880c81174e3 Mon Sep 17 00:00:00 2001 From: Eric Stroczynski Date: Wed, 24 Apr 2019 11:37:49 -0700 Subject: [PATCH 4/9] overwrite ImportMap --- internal/pkg/scaffold/controller_kind.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/internal/pkg/scaffold/controller_kind.go b/internal/pkg/scaffold/controller_kind.go index 1ee9bd7bc0..90e037b120 100644 --- a/internal/pkg/scaffold/controller_kind.go +++ b/internal/pkg/scaffold/controller_kind.go @@ -45,9 +45,7 @@ func (s *ControllerKind) GetInput() (input.Input, error) { s.TemplateBody = controllerKindTemplate // Set imports. - if len(s.ImportMap) == 0 { - s.ImportMap = controllerKindImports - } + s.ImportMap = controllerKindImports importPrefix := path.Join(s.Repo, "pkg", "apis") if s.K8sImportPath != "" { importPrefix = s.K8sImportPath From a832f2856c6234a496015f01f21f66692450ac2d Mon Sep 17 00:00:00 2001 From: Eric Stroczynski Date: Thu, 25 Apr 2019 14:56:28 -0700 Subject: [PATCH 5/9] cmd/operator-sdk/add/controller.go: --k8s-api-import takes a path and optionally an import identifier for an expernal k8s API internal/pkg/scaffold/controller_kind*.go: parse and set K8sImport in template --- cmd/operator-sdk/add/controller.go | 14 +--- internal/pkg/scaffold/controller_kind.go | 83 ++++++++++++++++--- internal/pkg/scaffold/controller_kind_test.go | 36 ++++++++ 3 files changed, 109 insertions(+), 24 deletions(-) diff --git a/cmd/operator-sdk/add/controller.go b/cmd/operator-sdk/add/controller.go index 6d2a4c6777..c0cbd5b8cc 100644 --- a/cmd/operator-sdk/add/controller.go +++ b/cmd/operator-sdk/add/controller.go @@ -25,10 +25,7 @@ import ( "github.com/spf13/cobra" ) -var ( - k8sAPI bool - k8sImportPath string -) +var k8sAPIImport string func newAddControllerCmd() *cobra.Command { controllerCmd := &cobra.Command{ @@ -62,8 +59,7 @@ Example: if err := controllerCmd.MarkFlagRequired("kind"); err != nil { log.Fatalf("Failed to mark `kind` flag for `add controller` subcommand as required") } - controllerCmd.Flags().StringVar(&k8sImportPath, "k8s-import-path", "k8s.io/api", "Kubernetes resource import path. Only valid if --k8s-api is set") - controllerCmd.Flags().BoolVar(&k8sAPI, "k8s-api", false, "The provided kind for api-version is a Kubernetes resource API") + controllerCmd.Flags().StringVar(&k8sAPIImport, "k8s-api-import", "", `External Kubernetes resource import path of the form "host.com/repo/path[=import_identifier]". import_identifier is optional`) return controllerCmd } @@ -90,12 +86,8 @@ func controllerRun(cmd *cobra.Command, args []string) error { } s := &scaffold.Scaffold{} - ck := &scaffold.ControllerKind{Resource: r} - if k8sAPI { - ck.K8sImportPath = k8sImportPath - } err = s.Execute(cfg, - ck, + &scaffold.ControllerKind{Resource: r, K8sImport: k8sAPIImport}, &scaffold.AddController{Resource: r}, ) if err != nil { diff --git a/internal/pkg/scaffold/controller_kind.go b/internal/pkg/scaffold/controller_kind.go index 90e037b120..13b7b62854 100644 --- a/internal/pkg/scaffold/controller_kind.go +++ b/internal/pkg/scaffold/controller_kind.go @@ -15,8 +15,11 @@ package scaffold import ( + "fmt" "path" "path/filepath" + "strings" + "unicode" "github.com/operator-framework/operator-sdk/internal/pkg/scaffold/input" ) @@ -27,15 +30,19 @@ type ControllerKind struct { // Resource defines the inputs for the controller's primary resource Resource *Resource - // K8sImportPath holds the import path for a built-in Kubernetes API that - // this controller reconciles if specified by the scaffold invoker. - K8sImportPath string + // K8sImport holds the import path for a built-in or custom Kubernetes + // API that this controller reconciles, if specified by the scaffold invoker. + K8sImport string + // ImportMap maps all imports destined for the scaffold to their import // identifier, if any. ImportMap map[string]string + // GoImportIdent is the import identifier for the API reconciled by this + // controller. + GoImportIdent string } -func (s *ControllerKind) GetInput() (input.Input, error) { +func (s *ControllerKind) GetInput() (_ input.Input, err error) { if s.Path == "" { fileName := s.Resource.LowerKind + "_controller.go" s.Path = filepath.Join(ControllerDir, s.Resource.LowerKind, fileName) @@ -46,15 +53,65 @@ func (s *ControllerKind) GetInput() (input.Input, error) { // Set imports. s.ImportMap = controllerKindImports - importPrefix := path.Join(s.Repo, "pkg", "apis") - if s.K8sImportPath != "" { - importPrefix = s.K8sImportPath + importPath := "" + if s.K8sImport != "" { + importPath, s.GoImportIdent, err = getK8sAPIImportPathAndIdent(s.K8sImport) + if err != nil { + return input.Input{}, err + } + } else { + importPath = path.Join(s.Repo, "pkg", "apis", s.Resource.GoImportGroup, s.Resource.Version) + s.GoImportIdent = s.Resource.GoImportGroup + s.Resource.Version + } + // Import identifiers must be unique within a file. + for p, id := range s.ImportMap { + if s.GoImportIdent == id && importPath != p { + // Append "api" to the conflicting import identifier. + s.GoImportIdent = s.GoImportIdent + "api" + break + } } - importPath := path.Join(importPrefix, s.Resource.GoImportGroup, s.Resource.Version) - s.ImportMap[importPath] = s.Resource.GoImportGroup + s.Resource.Version + s.ImportMap[importPath] = s.GoImportIdent return s.Input, nil } +func getK8sAPIImportPathAndIdent(m string) (p string, id string, err error) { + sm := strings.Split(m, "=") + for i, e := range sm { + if i == 0 { + p = strings.TrimSpace(e) + } else if i == 1 { + id = strings.TrimSpace(e) + } + } + if p == "" { + return "", "", fmt.Errorf(`k8s import "%s" path is empty`, m) + } + if id == "" { + if len(sm) == 2 { + return "", "", fmt.Errorf(`k8s import "%s" identifier is empty, remove "=" from passed string`, m) + } + sp := strings.Split(p, "/") + if len(sp) > 1 { + id = sp[len(sp)-2] + sp[len(sp)-1] + } else { + id = sp[0] + } + id = strings.ToLower(id) + } + idb := &strings.Builder{} + // By definition, all package identifiers must be comprised of "_", unicode + // digits, and/or letters. + for _, r := range id { + if unicode.IsDigit(r) || unicode.IsLetter(r) || r == '_' { + if _, err := idb.WriteRune(r); err != nil { + return "", "", err + } + } + } + return p, idb.String(), nil +} + var controllerKindImports = map[string]string{ "k8s.io/api/core/v1": "corev1", "k8s.io/apimachinery/pkg/api/errors": "", @@ -108,7 +165,7 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error { } // Watch for changes to primary resource {{ .Resource.Kind }} - err = c.Watch(&source.Kind{Type: &{{ .Resource.GoImportGroup}}{{ .Resource.Version }}.{{ .Resource.Kind }}{}}, &handler.EnqueueRequestForObject{}) + err = c.Watch(&source.Kind{Type: &{{ .GoImportIdent }}.{{ .Resource.Kind }}{}}, &handler.EnqueueRequestForObject{}) if err != nil { return err } @@ -117,7 +174,7 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error { // Watch for changes to secondary resource Pods and requeue the owner {{ .Resource.Kind }} err = c.Watch(&source.Kind{Type: &corev1.Pod{}}, &handler.EnqueueRequestForOwner{ IsController: true, - OwnerType: &{{ .Resource.GoImportGroup}}{{ .Resource.Version }}.{{ .Resource.Kind }}{}, + OwnerType: &{{ .GoImportIdent }}.{{ .Resource.Kind }}{}, }) if err != nil { return err @@ -148,7 +205,7 @@ func (r *Reconcile{{ .Resource.Kind }}) Reconcile(request reconcile.Request) (re reqLogger.Info("Reconciling {{ .Resource.Kind }}") // Fetch the {{ .Resource.Kind }} instance - instance := &{{ .Resource.GoImportGroup}}{{ .Resource.Version }}.{{ .Resource.Kind }}{} + instance := &{{ .GoImportIdent }}.{{ .Resource.Kind }}{} err := r.client.Get(context.TODO(), request.NamespacedName, instance) if err != nil { if errors.IsNotFound(err) { @@ -191,7 +248,7 @@ func (r *Reconcile{{ .Resource.Kind }}) Reconcile(request reconcile.Request) (re } // newPodForCR returns a busybox pod with the same name/namespace as the cr -func newPodForCR(cr *{{ .Resource.GoImportGroup}}{{ .Resource.Version }}.{{ .Resource.Kind }}) *corev1.Pod { +func newPodForCR(cr *{{ .GoImportIdent }}.{{ .Resource.Kind }}) *corev1.Pod { labels := map[string]string{ "app": cr.Name, } diff --git a/internal/pkg/scaffold/controller_kind_test.go b/internal/pkg/scaffold/controller_kind_test.go index 41baa8a22a..9af72c9806 100644 --- a/internal/pkg/scaffold/controller_kind_test.go +++ b/internal/pkg/scaffold/controller_kind_test.go @@ -190,3 +190,39 @@ func newPodForCR(cr *appv1alpha1.AppService) *corev1.Pod { } } ` + +func TestGetK8sAPIImportPathAndIdentifier(t *testing.T) { + cases := []struct { + inputImport, wantImportPath, wantImportIdent string + wantErr bool + }{ + {"", "", "", true}, + {"=rbacv1", "", "", true}, + {"k8s.io/api/rbac-2/v1", "k8s.io/api/rbac-2/v1", "rbac2v1", false}, + {"k8s.io/api/rbac/v1=rbacv1", "k8s.io/api/rbac/v1", "rbacv1", false}, + {"k8s.io/api/rbac/v1=rbac-v1", "k8s.io/api/rbac/v1", "rbacv1", false}, + {"k8s.io/api/rb_ac/v1", "k8s.io/api/rb_ac/v1", "rb_acv1", false}, + {"k8s.io/api/rbac/v1=rbaC_v1", "k8s.io/api/rbac/v1", "rbaC_v1", false}, + {"k8s.io/api/rbac/v1=", "", "", true}, + {"k8s.io/api/rbac/v1=rbacv1=", "k8s.io/api/rbac/v1", "rbacv1", false}, + {"k8s.io/api/rbac/v1=Rbacv1", "k8s.io/api/rbac/v1", "Rbacv1", false}, + } + + for _, c := range cases { + gotPath, gotIdent, err := getK8sAPIImportPathAndIdent(c.inputImport) + if err != nil && !c.wantErr { + t.Errorf(`wanted import path "%s" and identifier "%s" from "%s", got error %v`, c.wantImportPath, c.wantImportIdent, c.inputImport, err) + continue + } + if err == nil && c.wantErr { + t.Errorf(`wanted error from "%s", got import path "%s" and identifier "%s"`, c.inputImport, c.wantImportPath, c.wantImportIdent) + continue + } + if gotPath != c.wantImportPath { + t.Errorf(`wanted import path "%s" from "%s", got "%s"`, c.wantImportPath, c.inputImport, gotPath) + } + if gotIdent != c.wantImportIdent { + t.Errorf(`wanted import identifier "%s" from "%s", got "%s"`, c.wantImportIdent, c.inputImport, gotIdent) + } + } +} From ff140ab2b3a64a3e6f7f5d85e3d1c4852626fe12 Mon Sep 17 00:00:00 2001 From: Eric Stroczynski Date: Thu, 25 Apr 2019 15:44:18 -0700 Subject: [PATCH 6/9] update CHANGELOG and sdk cli doc --- CHANGELOG.md | 2 +- doc/sdk-cli-reference.md | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9cbcc95d9a..853e456654 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ - New option for [`operator-sdk build --image-builder`](https://github.com/operator-framework/operator-sdk/blob/master/doc/sdk-cli-reference.md#build), which can be used to specify which image builder to use. Adds support for [buildah](https://github.com/containers/buildah/). ([#1311](https://github.com/operator-framework/operator-sdk/pull/1311)) - Manager is now configured with a new `DynamicRESTMapper`, which accounts for the fact that the default `RESTMapper`, which only checks resource types at startup, can't handle the case of first creating a CRD and then an instance of that CRD. ([#1329](https://github.com/operator-framework/operator-sdk/pull/1329)) -- New optional flags `--k8s-api` and `--k8s-import-path` for [`operator-sdk add controller`](https://github.com/operator-framework/operator-sdk/blob/master/doc/sdk-cli-reference.md#controller) to specify that the new controller reconciles a built-in Kubernetes API. ([#1344](https://github.com/operator-framework/operator-sdk/pull/1344)) +- New optional flag `--k8s-api-import` for [`operator-sdk add controller`](https://github.com/operator-framework/operator-sdk/blob/master/doc/sdk-cli-reference.md#controller) to specify that the new controller reconciles a built-in or external Kubernetes API, and what import path and identifier it should have. ([#1344](https://github.com/operator-framework/operator-sdk/pull/1344)) ### Changed diff --git a/doc/sdk-cli-reference.md b/doc/sdk-cli-reference.md index 645e02938b..8b0ada1447 100644 --- a/doc/sdk-cli-reference.md +++ b/doc/sdk-cli-reference.md @@ -339,8 +339,7 @@ Adds a new controller under `pkg/controller//...` that, by default, reconc * `--api-version` string - CRD APIVersion in the format `$GROUP_NAME/$VERSION` (e.g app.example.com/v1alpha1) * `--kind` string - CRD Kind. (e.g AppService) -* `--k8s-api` - The provided kind for api-version is a Kubernetes resource API -* `--k8s-import-path` - Kubernetes resource import path. Only valid if --k8s-api is set (default `k8s.io/api`) +* `--k8s-api-import` - External Kubernetes resource import path of the form "host.com/repo/path[=import_identifier]". import_identifier is optional #### Example From 4195646cc6a646050b482f2948bff87324cac891 Mon Sep 17 00:00:00 2001 From: Eric Stroczynski Date: Fri, 26 Apr 2019 12:58:34 -0700 Subject: [PATCH 7/9] rename flag, variables, fields: k8s -> custom --- CHANGELOG.md | 2 +- cmd/operator-sdk/add/controller.go | 6 +++--- doc/sdk-cli-reference.md | 2 +- internal/pkg/scaffold/controller_kind.go | 10 +++++----- internal/pkg/scaffold/controller_kind_test.go | 4 ++-- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 853e456654..ec8e3c9957 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ - New option for [`operator-sdk build --image-builder`](https://github.com/operator-framework/operator-sdk/blob/master/doc/sdk-cli-reference.md#build), which can be used to specify which image builder to use. Adds support for [buildah](https://github.com/containers/buildah/). ([#1311](https://github.com/operator-framework/operator-sdk/pull/1311)) - Manager is now configured with a new `DynamicRESTMapper`, which accounts for the fact that the default `RESTMapper`, which only checks resource types at startup, can't handle the case of first creating a CRD and then an instance of that CRD. ([#1329](https://github.com/operator-framework/operator-sdk/pull/1329)) -- New optional flag `--k8s-api-import` for [`operator-sdk add controller`](https://github.com/operator-framework/operator-sdk/blob/master/doc/sdk-cli-reference.md#controller) to specify that the new controller reconciles a built-in or external Kubernetes API, and what import path and identifier it should have. ([#1344](https://github.com/operator-framework/operator-sdk/pull/1344)) +- New optional flag `--custom-api-import` for [`operator-sdk add controller`](https://github.com/operator-framework/operator-sdk/blob/master/doc/sdk-cli-reference.md#controller) to specify that the new controller reconciles a built-in or external Kubernetes API, and what import path and identifier it should have. ([#1344](https://github.com/operator-framework/operator-sdk/pull/1344)) ### Changed diff --git a/cmd/operator-sdk/add/controller.go b/cmd/operator-sdk/add/controller.go index c0cbd5b8cc..ddb2dc2ca9 100644 --- a/cmd/operator-sdk/add/controller.go +++ b/cmd/operator-sdk/add/controller.go @@ -25,7 +25,7 @@ import ( "github.com/spf13/cobra" ) -var k8sAPIImport string +var customAPIImport string func newAddControllerCmd() *cobra.Command { controllerCmd := &cobra.Command{ @@ -59,7 +59,7 @@ Example: if err := controllerCmd.MarkFlagRequired("kind"); err != nil { log.Fatalf("Failed to mark `kind` flag for `add controller` subcommand as required") } - controllerCmd.Flags().StringVar(&k8sAPIImport, "k8s-api-import", "", `External Kubernetes resource import path of the form "host.com/repo/path[=import_identifier]". import_identifier is optional`) + controllerCmd.Flags().StringVar(&customAPIImport, "custom-api-import", "", `External Kubernetes resource import path of the form "host.com/repo/path[=import_identifier]". import_identifier is optional`) return controllerCmd } @@ -87,7 +87,7 @@ func controllerRun(cmd *cobra.Command, args []string) error { s := &scaffold.Scaffold{} err = s.Execute(cfg, - &scaffold.ControllerKind{Resource: r, K8sImport: k8sAPIImport}, + &scaffold.ControllerKind{Resource: r, CustomImport: customAPIImport}, &scaffold.AddController{Resource: r}, ) if err != nil { diff --git a/doc/sdk-cli-reference.md b/doc/sdk-cli-reference.md index 8b0ada1447..8c84449efb 100644 --- a/doc/sdk-cli-reference.md +++ b/doc/sdk-cli-reference.md @@ -339,7 +339,7 @@ Adds a new controller under `pkg/controller//...` that, by default, reconc * `--api-version` string - CRD APIVersion in the format `$GROUP_NAME/$VERSION` (e.g app.example.com/v1alpha1) * `--kind` string - CRD Kind. (e.g AppService) -* `--k8s-api-import` - External Kubernetes resource import path of the form "host.com/repo/path[=import_identifier]". import_identifier is optional +* `--custom-api-import` - External Kubernetes resource import path of the form "host.com/repo/path[=import_identifier]". import_identifier is optional #### Example diff --git a/internal/pkg/scaffold/controller_kind.go b/internal/pkg/scaffold/controller_kind.go index 13b7b62854..58023cdbff 100644 --- a/internal/pkg/scaffold/controller_kind.go +++ b/internal/pkg/scaffold/controller_kind.go @@ -30,9 +30,9 @@ type ControllerKind struct { // Resource defines the inputs for the controller's primary resource Resource *Resource - // K8sImport holds the import path for a built-in or custom Kubernetes + // CustomImport holds the import path for a built-in or custom Kubernetes // API that this controller reconciles, if specified by the scaffold invoker. - K8sImport string + CustomImport string // ImportMap maps all imports destined for the scaffold to their import // identifier, if any. @@ -54,8 +54,8 @@ func (s *ControllerKind) GetInput() (_ input.Input, err error) { // Set imports. s.ImportMap = controllerKindImports importPath := "" - if s.K8sImport != "" { - importPath, s.GoImportIdent, err = getK8sAPIImportPathAndIdent(s.K8sImport) + if s.CustomImport != "" { + importPath, s.GoImportIdent, err = getCustomAPIImportPathAndIdent(s.CustomImport) if err != nil { return input.Input{}, err } @@ -75,7 +75,7 @@ func (s *ControllerKind) GetInput() (_ input.Input, err error) { return s.Input, nil } -func getK8sAPIImportPathAndIdent(m string) (p string, id string, err error) { +func getCustomAPIImportPathAndIdent(m string) (p string, id string, err error) { sm := strings.Split(m, "=") for i, e := range sm { if i == 0 { diff --git a/internal/pkg/scaffold/controller_kind_test.go b/internal/pkg/scaffold/controller_kind_test.go index 9af72c9806..c8f62f26e7 100644 --- a/internal/pkg/scaffold/controller_kind_test.go +++ b/internal/pkg/scaffold/controller_kind_test.go @@ -191,7 +191,7 @@ func newPodForCR(cr *appv1alpha1.AppService) *corev1.Pod { } ` -func TestGetK8sAPIImportPathAndIdentifier(t *testing.T) { +func TestGetCustomAPIImportPathAndIdentifier(t *testing.T) { cases := []struct { inputImport, wantImportPath, wantImportIdent string wantErr bool @@ -209,7 +209,7 @@ func TestGetK8sAPIImportPathAndIdentifier(t *testing.T) { } for _, c := range cases { - gotPath, gotIdent, err := getK8sAPIImportPathAndIdent(c.inputImport) + gotPath, gotIdent, err := getCustomAPIImportPathAndIdent(c.inputImport) if err != nil && !c.wantErr { t.Errorf(`wanted import path "%s" and identifier "%s" from "%s", got error %v`, c.wantImportPath, c.wantImportIdent, c.inputImport, err) continue From 8cf98cd2413f0a97640ae9e5f7e86de16a2d20c1 Mon Sep 17 00:00:00 2001 From: Eric Stroczynski Date: Tue, 7 May 2019 09:39:17 -0700 Subject: [PATCH 8/9] changes based on PR comments --- doc/sdk-cli-reference.md | 2 +- internal/pkg/scaffold/controller_kind.go | 15 ++++++++++++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/doc/sdk-cli-reference.md b/doc/sdk-cli-reference.md index 284225ceb2..a3b68c5576 100644 --- a/doc/sdk-cli-reference.md +++ b/doc/sdk-cli-reference.md @@ -348,7 +348,7 @@ Adds a new controller under `pkg/controller//...` that, by default, reconc * `--api-version` string - CRD APIVersion in the format `$GROUP_NAME/$VERSION` (e.g app.example.com/v1alpha1) * `--kind` string - CRD Kind. (e.g AppService) -* `--custom-api-import` - External Kubernetes resource import path of the form "host.com/repo/path[=import_identifier]". import_identifier is optional +* `--custom-api-import` string - External Kubernetes resource import path of the form "host.com/repo/path[=import_identifier]". import_identifier is optional #### Example diff --git a/internal/pkg/scaffold/controller_kind.go b/internal/pkg/scaffold/controller_kind.go index 58023cdbff..dd722703e1 100644 --- a/internal/pkg/scaffold/controller_kind.go +++ b/internal/pkg/scaffold/controller_kind.go @@ -34,6 +34,8 @@ type ControllerKind struct { // API that this controller reconciles, if specified by the scaffold invoker. CustomImport string + // The following fields will be overwritten by GetInput(). + // // ImportMap maps all imports destined for the scaffold to their import // identifier, if any. ImportMap map[string]string @@ -42,7 +44,7 @@ type ControllerKind struct { GoImportIdent string } -func (s *ControllerKind) GetInput() (_ input.Input, err error) { +func (s *ControllerKind) GetInput() (input.Input, error) { if s.Path == "" { fileName := s.Resource.LowerKind + "_controller.go" s.Path = filepath.Join(ControllerDir, s.Resource.LowerKind, fileName) @@ -52,12 +54,19 @@ func (s *ControllerKind) GetInput() (_ input.Input, err error) { s.TemplateBody = controllerKindTemplate // Set imports. + if err := s.setImports(); err != nil { + return input.Input{}, err + } + return s.Input, nil +} + +func (s *ControllerKind) setImports() (err error) { s.ImportMap = controllerKindImports importPath := "" if s.CustomImport != "" { importPath, s.GoImportIdent, err = getCustomAPIImportPathAndIdent(s.CustomImport) if err != nil { - return input.Input{}, err + return err } } else { importPath = path.Join(s.Repo, "pkg", "apis", s.Resource.GoImportGroup, s.Resource.Version) @@ -72,7 +81,7 @@ func (s *ControllerKind) GetInput() (_ input.Input, err error) { } } s.ImportMap[importPath] = s.GoImportIdent - return s.Input, nil + return nil } func getCustomAPIImportPathAndIdent(m string) (p string, id string, err error) { From 70ff02bb4d19346276a35201599151892ec06fff Mon Sep 17 00:00:00 2001 From: Eric Stroczynski Date: Thu, 16 May 2019 13:43:10 -0700 Subject: [PATCH 9/9] Apply suggestions from code review Co-Authored-By: Joe Lanford --- internal/pkg/scaffold/controller_kind.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/pkg/scaffold/controller_kind.go b/internal/pkg/scaffold/controller_kind.go index dd722703e1..996f54ec58 100644 --- a/internal/pkg/scaffold/controller_kind.go +++ b/internal/pkg/scaffold/controller_kind.go @@ -94,11 +94,11 @@ func getCustomAPIImportPathAndIdent(m string) (p string, id string, err error) { } } if p == "" { - return "", "", fmt.Errorf(`k8s import "%s" path is empty`, m) + return "", "", fmt.Errorf(`custom import "%s" path is empty`, m) } if id == "" { if len(sm) == 2 { - return "", "", fmt.Errorf(`k8s import "%s" identifier is empty, remove "=" from passed string`, m) + return "", "", fmt.Errorf(`custom import "%s" identifier is empty, remove "=" from passed string`, m) } sp := strings.Split(p, "/") if len(sp) > 1 {