cmd/operator-sdk/add/controller.go: support built-in k8s API controllers (#1344)

* cmd/operator-sdk/add/controller.go: --custom-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

* update CHANGELOG and sdk cli doc

Author: Eric Stroczynski

CHANGELOG.md

@@ -6,6 +6,7 @@
 - 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))
 - Unify CLI debug logging under a global `--verbose` flag ([#1361](https://github.com/operator-framework/operator-sdk/pull/1361))
 - [Go module](https://github.com/golang/go/wiki/Modules) support by default for new Go operators and during Ansible and Helm operator migration. The dependency manager used for a new operator can be explicitly specified for new operators through the `--dep-manager` flag, available in [`operator-sdk new`](https://github.com/operator-framework/operator-sdk/blob/master/doc/sdk-cli-reference.md#new) and [`operator-sdk migrate`](https://github.com/operator-framework/operator-sdk/blob/master/doc/sdk-cli-reference.md#migrate). `dep` is still available through `--dep-manager=dep`. ([#1001](https://github.com/operator-framework/operator-sdk/pull/1001))
+- 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
 

cmd/operator-sdk/add/controller.go

@@ -25,6 +25,8 @@ import (
 	"github.com/spf13/cobra"
 )
 
+var customAPIImport string
+
 func newAddControllerCmd() *cobra.Command {
 	controllerCmd := &cobra.Command{
 		Use:   "controller",
@@ -57,6 +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(&customAPIImport, "custom-api-import", "", `External Kubernetes resource import path of the form "host.com/repo/path[=import_identifier]". import_identifier is optional`)
 
 	return controllerCmd
 }
@@ -81,10 +84,10 @@ func controllerRun(cmd *cobra.Command, args []string) error {
 		Repo:           projutil.CheckAndGetProjectGoPkg(),
 		AbsProjectPath: projutil.MustGetwd(),
 	}
-
 	s := &scaffold.Scaffold{}
+
 	err = s.Execute(cfg,
-		&scaffold.ControllerKind{Resource: r},
+		&scaffold.ControllerKind{Resource: r, CustomImport: customAPIImport},
 		&scaffold.AddController{Resource: r},
 	)
 	if err != nil {

doc/sdk-cli-reference.md

@@ -358,6 +358,7 @@ Adds a new controller under `pkg/controller/<kind>/...` 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` string - External Kubernetes resource import path of the form "host.com/repo/path[=import_identifier]". import_identifier is optional
 
 #### Example
 

internal/pkg/scaffold/controller_kind.go

@@ -15,7 +15,11 @@
 package scaffold
 
 import (
+	"fmt"
+	"path"
 	"path/filepath"
+	"strings"
+	"unicode"
 
 	"github.com/operator-framework/operator-sdk/internal/pkg/scaffold/input"
 )
@@ -26,6 +30,18 @@ type ControllerKind struct {
 
 	// Resource defines the inputs for the controller's primary resource
 	Resource *Resource
+	// CustomImport holds the import path for a built-in or custom Kubernetes
+	// 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
+	// GoImportIdent is the import identifier for the API reconciled by this
+	// controller.
+	GoImportIdent string
 }
 
 func (s *ControllerKind) GetInput() (input.Input, error) {
@@ -36,29 +52,99 @@ func (s *ControllerKind) GetInput() (input.Input, error) {
 	// Error if this file exists.
 	s.IfExistsAction = input.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 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
+		}
+	}
+	s.ImportMap[importPath] = s.GoImportIdent
+	return nil
+}
+
+func getCustomAPIImportPathAndIdent(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(`custom import "%s" path is empty`, m)
+	}
+	if id == "" {
+		if len(sm) == 2 {
+			return "", "", fmt.Errorf(`custom 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":                           "",
+	"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 }}")
@@ -88,7 +174,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
 	}
@@ -97,7 +183,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
@@ -129,7 +215,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) {
@@ -172,7 +258,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,
 	}

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"
@@ -192,3 +191,39 @@ func newPodForCR(cr *appv1alpha1.AppService) *corev1.Pod {
 	}
 }
 `
+
+func TestGetCustomAPIImportPathAndIdentifier(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 := 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
+		}
+		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)
+		}
+	}
+}