Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 11 commits into from
May 17, 2019
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
7 changes: 5 additions & 2 deletions cmd/operator-sdk/add/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ import (
"github.com/spf13/cobra"
)

var customAPIImport string

func newAddControllerCmd() *cobra.Command {
controllerCmd := &cobra.Command{
Use: "controller",
Expand Down Expand Up @@ -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
}
Expand All @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions doc/sdk-cli-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
124 changes: 105 additions & 19 deletions internal/pkg/scaffold/controller_kind.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,11 @@
package scaffold

import (
"fmt"
"path"
"path/filepath"
"strings"
"unicode"

"github.com/operator-framework/operator-sdk/internal/pkg/scaffold/input"
)
Expand All @@ -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

estroz marked this conversation as resolved.
Show resolved Hide resolved
// 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) {
Expand All @@ -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
estroz marked this conversation as resolved.
Show resolved Hide resolved
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 }}")
Expand Down Expand Up @@ -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
}
Expand All @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
}
Expand Down
37 changes: 36 additions & 1 deletion internal/pkg/scaffold/controller_kind_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
}
}