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 @@ -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 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 @@ -339,6 +339,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` - External Kubernetes resource import path of the form "host.com/repo/path[=import_identifier]". import_identifier is optional
estroz marked this conversation as resolved.
Show resolved Hide resolved

#### Example

Expand Down
117 changes: 97 additions & 20 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,39 +30,112 @@ 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
// 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)
}
// Error if this file exists.
s.IfExistsAction = input.Error
s.TemplateBody = controllerKindTemplate

// Set imports.
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 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
}
}
s.ImportMap[importPath] = s.GoImportIdent
return s.Input, 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(`k8s import "%s" path is empty`, m)
estroz marked this conversation as resolved.
Show resolved Hide resolved
}
if id == "" {
if len(sm) == 2 {
return "", "", fmt.Errorf(`k8s import "%s" identifier is empty, remove "=" from passed string`, m)
estroz marked this conversation as resolved.
Show resolved Hide resolved
}
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 +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
}
Expand All @@ -97,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
Expand Down Expand Up @@ -128,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) {
Expand Down Expand Up @@ -171,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,
}
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 @@ -191,3 +190,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)
}
}
}