diff --git a/CHANGELOG.md b/CHANGELOG.md index 46ef0e888f..9ddb4aa156 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/cmd/operator-sdk/add/controller.go b/cmd/operator-sdk/add/controller.go index fde3646779..ddb2dc2ca9 100644 --- a/cmd/operator-sdk/add/controller.go +++ b/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 { diff --git a/doc/sdk-cli-reference.md b/doc/sdk-cli-reference.md index d8f4d0f2ac..65907c476c 100644 --- a/doc/sdk-cli-reference.md +++ b/doc/sdk-cli-reference.md @@ -358,6 +358,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` 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 e87ede0e6e..0f8f4160d0 100644 --- a/internal/pkg/scaffold/controller_kind.go +++ b/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, } diff --git a/internal/pkg/scaffold/controller_kind_test.go b/internal/pkg/scaffold/controller_kind_test.go index 8ba8570e41..cd342a2c98 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" @@ -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) + } + } +}