Use controller.Options from crossplane-runtime

See crossplane/crossplane-runtime#293

This allows us to plumb more options down from our CLI flags to each controller
without adding more and more arguments to our Setup functions. This commit also
removes most of the internal/feature package, which has moved to
crossplane-runtime.

Signed-off-by: Nic Cope <negz@rk0n.org>

cmd/crossplane/core/core.go

@@ -25,13 +25,15 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
 
+	"github.com/crossplane/crossplane-runtime/pkg/controller"
 	"github.com/crossplane/crossplane-runtime/pkg/errors"
-
 	"github.com/crossplane/crossplane-runtime/pkg/logging"
+	"github.com/crossplane/crossplane-runtime/pkg/ratelimiter"
 
 	"github.com/crossplane/crossplane/internal/controller/apiextensions"
 	"github.com/crossplane/crossplane/internal/controller/pkg"
-	"github.com/crossplane/crossplane/internal/feature"
+	pkgcontroller "github.com/crossplane/crossplane/internal/controller/pkg/controller"
+	"github.com/crossplane/crossplane/internal/features"
 	"github.com/crossplane/crossplane/internal/xpkg"
 )
 
@@ -83,19 +85,38 @@ func (c *startCommand) Run(s *runtime.Scheme, log logging.Logger) error {
 		return errors.Wrap(err, "Cannot create manager")
 	}
 
-	f := &feature.Flags{}
+	o := controller.Options{
+		Logger:                  log,
+		MaxConcurrentReconciles: 1,
+		PollInterval:            1 * time.Minute,
+		GlobalRateLimiter:       ratelimiter.NewGlobal(ratelimiter.DefaultGlobalRPS),
+	}
+
 	if c.EnableCompositionRevisions {
-		f.Enable(feature.FlagEnableAlphaCompositionRevisions)
-		log.Info("Alpha feature enabled", "flag", feature.FlagEnableAlphaCompositionRevisions.String())
+		o.Features.Enable(features.EnableAlphaCompositionRevisions)
+		log.Info("Alpha feature enabled", "flag", features.EnableAlphaCompositionRevisions)
 	}
 
-	if err := apiextensions.Setup(mgr, log, f); err != nil {
+	if err := apiextensions.Setup(mgr, o); err != nil {
 		return errors.Wrap(err, "Cannot setup API extension controllers")
 	}
 
-	pkgCache := xpkg.NewImageCache(c.CacheDir, afero.NewOsFs())
+	po := pkgcontroller.Options{
+		Options:         o,
+		Cache:           xpkg.NewImageCache(c.CacheDir, afero.NewOsFs()),
+		Namespace:       c.Namespace,
+		DefaultRegistry: c.Registry,
+	}
+
+	if c.CABundlePath != "" {
+		rootCAs, err := xpkg.ParseCertificatesFromPath(c.CABundlePath)
+		if err != nil {
+			return errors.Wrap(err, "Cannot parse CA bundle")
+		}
+		po.FetcherOptions = []xpkg.FetcherOpt{xpkg.WithCustomCA(rootCAs)}
+	}
 
-	if err := pkg.Setup(mgr, log, pkgCache, c.Namespace, c.Registry, c.CABundlePath); err != nil {
+	if err := pkg.Setup(mgr, po); err != nil {
 		return errors.Wrap(err, "Cannot add packages controllers to manager")
 	}
 

cmd/crossplane/rbac/rbac.go

@@ -24,17 +24,19 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
 
+	"github.com/crossplane/crossplane-runtime/pkg/controller"
 	"github.com/crossplane/crossplane-runtime/pkg/errors"
-
 	"github.com/crossplane/crossplane-runtime/pkg/logging"
+	"github.com/crossplane/crossplane-runtime/pkg/ratelimiter"
 
 	"github.com/crossplane/crossplane/internal/controller/rbac"
+	rbaccontroller "github.com/crossplane/crossplane/internal/controller/rbac/controller"
 )
 
 // Available RBAC management policies.
 const (
-	ManagementPolicyAll   = string(rbac.ManagementPolicyAll)
-	ManagementPolicyBasic = string(rbac.ManagementPolicyBasic)
+	ManagementPolicyAll   = string(rbaccontroller.ManagementPolicyAll)
+	ManagementPolicyBasic = string(rbaccontroller.ManagementPolicyBasic)
 )
 
 // KongVars represent the kong variables associated with the CLI parser
@@ -87,7 +89,18 @@ func (c *startCommand) Run(s *runtime.Scheme, log logging.Logger) error {
 		return errors.Wrap(err, "cannot create manager")
 	}
 
-	if err := rbac.Setup(mgr, log, rbac.ManagementPolicy(c.ManagementPolicy), c.ProviderClusterRole); err != nil {
+	o := rbaccontroller.Options{
+		Options: controller.Options{
+			Logger:                  log,
+			MaxConcurrentReconciles: 1,
+			PollInterval:            1 * time.Minute,
+			GlobalRateLimiter:       ratelimiter.NewGlobal(ratelimiter.DefaultGlobalRPS),
+		},
+		AllowClusterRole: c.ProviderClusterRole,
+		ManagementPolicy: rbaccontroller.ManagementPolicy(c.ManagementPolicy),
+	}
+
+	if err := rbac.Setup(mgr, o); err != nil {
 		return errors.Wrap(err, "cannot add RBAC controllers to manager")
 	}
 

internal/controller/apiextensions/apiextensions.go

@@ -19,29 +19,28 @@ package apiextensions
 import (
 	ctrl "sigs.k8s.io/controller-runtime"
 
-	"github.com/crossplane/crossplane-runtime/pkg/logging"
-
+	"github.com/crossplane/crossplane-runtime/pkg/controller"
 	"github.com/crossplane/crossplane/internal/controller/apiextensions/composition"
 	"github.com/crossplane/crossplane/internal/controller/apiextensions/definition"
 	"github.com/crossplane/crossplane/internal/controller/apiextensions/offered"
-	"github.com/crossplane/crossplane/internal/feature"
+	"github.com/crossplane/crossplane/internal/features"
 )
 
 // Setup API extensions controllers.
-func Setup(mgr ctrl.Manager, l logging.Logger, f *feature.Flags) error {
+func Setup(mgr ctrl.Manager, o controller.Options) error {
 
 	// The Composition controller only deals in the management of
 	// CompositionRevisions, so we don't need it at all unless the
 	// CompositionRevision feature flag is enabled.
-	if f.Enabled(feature.FlagEnableAlphaCompositionRevisions) {
-		if err := composition.Setup(mgr, l); err != nil {
+	if o.Features.Enabled(features.EnableAlphaCompositionRevisions) {
+		if err := composition.Setup(mgr, o); err != nil {
 			return err
 		}
 	}
 
-	if err := definition.Setup(mgr, l, f); err != nil {
+	if err := definition.Setup(mgr, o); err != nil {
 		return err
 	}
 
-	return offered.Setup(mgr, l)
+	return offered.Setup(mgr, o)
 }

internal/controller/apiextensions/composition/reconciler.go

@@ -28,6 +28,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
+	"github.com/crossplane/crossplane-runtime/pkg/controller"
 	"github.com/crossplane/crossplane-runtime/pkg/errors"
 	"github.com/crossplane/crossplane-runtime/pkg/event"
 	"github.com/crossplane/crossplane-runtime/pkg/logging"
@@ -59,15 +60,15 @@ const (
 
 // Setup adds a controller that reconciles Compositions by creating new
 // CompositionRevisions for each revision of the Composition's spec.
-func Setup(mgr ctrl.Manager, log logging.Logger) error {
+func Setup(mgr ctrl.Manager, o controller.Options) error {
 	name := "revisions/" + strings.ToLower(v1.CompositionGroupKind)
 
 	return ctrl.NewControllerManagedBy(mgr).
 		Named(name).
 		For(&v1.Composition{}).
 		Owns(&v1alpha1.CompositionRevision{}).
 		Complete(NewReconciler(mgr,
-			WithLogger(log.WithValues("controller", name)),
+			WithLogger(o.Logger.WithValues("controller", name)),
 			WithRecorder(event.NewAPIRecorder(mgr.GetEventRecorderFor(name)))))
 }
 

internal/controller/apiextensions/definition/reconciler.go

@@ -36,14 +36,15 @@ import (
 	"github.com/crossplane/crossplane-runtime/pkg/controller"
 	"github.com/crossplane/crossplane-runtime/pkg/errors"
 	"github.com/crossplane/crossplane-runtime/pkg/event"
+	"github.com/crossplane/crossplane-runtime/pkg/feature"
 	"github.com/crossplane/crossplane-runtime/pkg/logging"
 	"github.com/crossplane/crossplane-runtime/pkg/meta"
 	"github.com/crossplane/crossplane-runtime/pkg/resource"
 	"github.com/crossplane/crossplane-runtime/pkg/resource/unstructured"
 
 	v1 "github.com/crossplane/crossplane/apis/apiextensions/v1"
 	"github.com/crossplane/crossplane/internal/controller/apiextensions/composite"
-	"github.com/crossplane/crossplane/internal/feature"
+	"github.com/crossplane/crossplane/internal/features"
 	"github.com/crossplane/crossplane/internal/xcrd"
 )
 
@@ -107,7 +108,7 @@ func (fn CRDRenderFn) Render(d *v1.CompositeResourceDefinition) (*extv1.CustomRe
 
 // Setup adds a controller that reconciles CompositeResourceDefinitions by
 // defining a composite resource and starting a controller to reconcile it.
-func Setup(mgr ctrl.Manager, log logging.Logger, f *feature.Flags) error {
+func Setup(mgr ctrl.Manager, o controller.Options) error {
 	name := "defined/" + strings.ToLower(v1.CompositeResourceDefinitionGroupKind)
 
 	return ctrl.NewControllerManagedBy(mgr).
@@ -116,9 +117,9 @@ func Setup(mgr ctrl.Manager, log logging.Logger, f *feature.Flags) error {
 		Owns(&extv1.CustomResourceDefinition{}).
 		WithOptions(kcontroller.Options{MaxConcurrentReconciles: maxConcurrency}).
 		Complete(NewReconciler(mgr,
-			WithLogger(log.WithValues("controller", name)),
+			WithLogger(o.Logger.WithValues("controller", name)),
 			WithRecorder(event.NewAPIRecorder(mgr.GetEventRecorderFor(name))),
-			WithFeatureFlags(f)))
+			WithFeatureFlags(o.Features)))
 }
 
 // ReconcilerOption is used to configure the Reconciler.
@@ -401,7 +402,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco
 	// We only want to enable CompositionRevision support if the relevant
 	// feature flag is enabled. Otherwise we start the XR Reconciler with
 	// its default CompositionFetcher.
-	if r.flags.Enabled(feature.FlagEnableAlphaCompositionRevisions) {
+	if r.flags.Enabled(features.EnableAlphaCompositionRevisions) {
 		a := resource.ClientApplicator{Client: r.client, Applicator: resource.NewAPIPatchingApplicator(r.client)}
 		o = append(o, composite.WithCompositionFetcher(composite.NewAPIRevisionFetcher(a)))
 	}

internal/controller/apiextensions/offered/reconciler.go

@@ -110,7 +110,7 @@ func (fn CRDRenderFn) Render(d *v1.CompositeResourceDefinition) (*extv1.CustomRe
 // Setup adds a controller that reconciles CompositeResourceDefinitions by
 // defining a composite resource claim and starting a controller to reconcile
 // it.
-func Setup(mgr ctrl.Manager, log logging.Logger) error {
+func Setup(mgr ctrl.Manager, o controller.Options) error {
 	name := "offered/" + strings.ToLower(v1.CompositeResourceDefinitionGroupKind)
 
 	return ctrl.NewControllerManagedBy(mgr).
@@ -120,7 +120,7 @@ func Setup(mgr ctrl.Manager, log logging.Logger) error {
 		WithEventFilter(resource.NewPredicates(OffersClaim())).
 		WithOptions(kcontroller.Options{MaxConcurrentReconciles: maxConcurrency}).
 		Complete(NewReconciler(mgr,
-			WithLogger(log.WithValues("controller", name)),
+			WithLogger(o.Logger.WithValues("controller", name)),
 			WithRecorder(event.NewAPIRecorder(mgr.GetEventRecorderFor(name)))))
 }
 

internal/controller/pkg/controller/options.go

@@ -0,0 +1,42 @@
+/*
+Copyright 2021 The Crossplane Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+// Package controller contains options specific to pkg controllers.
+package controller
+
+import (
+	"github.com/crossplane/crossplane/internal/xpkg"
+
+	"github.com/crossplane/crossplane-runtime/pkg/controller"
+)
+
+// Options specific to pkg controllers.
+type Options struct {
+	controller.Options
+
+	// Cache for package OCI images.
+	Cache xpkg.Cache
+
+	// Namespace used to unpack and run packages.
+	Namespace string
+
+	// DefaultRegistry used to pull packages.
+	DefaultRegistry string
+
+	// FetcherOptions can be used to add optional parameters to
+	// NewK8sFetcher.
+	FetcherOptions []xpkg.FetcherOpt
+}

internal/controller/pkg/manager/reconciler.go

@@ -36,6 +36,7 @@ import (
 	"github.com/crossplane/crossplane-runtime/pkg/resource"
 
 	v1 "github.com/crossplane/crossplane/apis/pkg/v1"
+	"github.com/crossplane/crossplane/internal/controller/pkg/controller"
 	"github.com/crossplane/crossplane/internal/xpkg"
 )
 
@@ -140,17 +141,17 @@ type Reconciler struct {
 }
 
 // SetupProvider adds a controller that reconciles Providers.
-func SetupProvider(mgr ctrl.Manager, l logging.Logger, namespace, registry string, fetcherOpts ...xpkg.FetcherOpt) error {
+func SetupProvider(mgr ctrl.Manager, o controller.Options) error {
 	name := "packages/" + strings.ToLower(v1.ProviderGroupKind)
 	np := func() v1.Package { return &v1.Provider{} }
 	nr := func() v1.PackageRevision { return &v1.ProviderRevision{} }
 	nrl := func() v1.PackageRevisionList { return &v1.ProviderRevisionList{} }
 
-	clientset, err := kubernetes.NewForConfig(mgr.GetConfig())
+	cs, err := kubernetes.NewForConfig(mgr.GetConfig())
 	if err != nil {
 		return errors.Wrap(err, errCreateK8sClient)
 	}
-	fetcher, err := xpkg.NewK8sFetcher(clientset, namespace, fetcherOpts...)
+	f, err := xpkg.NewK8sFetcher(cs, o.Namespace, o.FetcherOptions...)
 	if err != nil {
 		return errors.Wrap(err, errBuildFetcher)
 	}
@@ -159,8 +160,8 @@ func SetupProvider(mgr ctrl.Manager, l logging.Logger, namespace, registry strin
 		WithNewPackageFn(np),
 		WithNewPackageRevisionFn(nr),
 		WithNewPackageRevisionListFn(nrl),
-		WithRevisioner(NewPackageRevisioner(fetcher, WithDefaultRegistry(registry))),
-		WithLogger(l.WithValues("controller", name)),
+		WithRevisioner(NewPackageRevisioner(f, WithDefaultRegistry(o.DefaultRegistry))),
+		WithLogger(o.Logger.WithValues("controller", name)),
 		WithRecorder(event.NewAPIRecorder(mgr.GetEventRecorderFor(name))),
 	)
 
@@ -172,7 +173,7 @@ func SetupProvider(mgr ctrl.Manager, l logging.Logger, namespace, registry strin
 }
 
 // SetupConfiguration adds a controller that reconciles Configurations.
-func SetupConfiguration(mgr ctrl.Manager, l logging.Logger, namespace, registry string, fetcherOpts ...xpkg.FetcherOpt) error {
+func SetupConfiguration(mgr ctrl.Manager, o controller.Options) error {
 	name := "packages/" + strings.ToLower(v1.ConfigurationGroupKind)
 	np := func() v1.Package { return &v1.Configuration{} }
 	nr := func() v1.PackageRevision { return &v1.ConfigurationRevision{} }
@@ -182,7 +183,7 @@ func SetupConfiguration(mgr ctrl.Manager, l logging.Logger, namespace, registry
 	if err != nil {
 		return errors.Wrap(err, "failed to initialize clientset")
 	}
-	fetcher, err := xpkg.NewK8sFetcher(clientset, namespace, fetcherOpts...)
+	fetcher, err := xpkg.NewK8sFetcher(clientset, o.Namespace, o.FetcherOptions...)
 	if err != nil {
 		return errors.Wrap(err, "cannot build fetcher")
 	}
@@ -191,8 +192,8 @@ func SetupConfiguration(mgr ctrl.Manager, l logging.Logger, namespace, registry
 		WithNewPackageFn(np),
 		WithNewPackageRevisionFn(nr),
 		WithNewPackageRevisionListFn(nrl),
-		WithRevisioner(NewPackageRevisioner(fetcher, WithDefaultRegistry(registry))),
-		WithLogger(l.WithValues("controller", name)),
+		WithRevisioner(NewPackageRevisioner(fetcher, WithDefaultRegistry(o.DefaultRegistry))),
+		WithLogger(o.Logger.WithValues("controller", name)),
 		WithRecorder(event.NewAPIRecorder(mgr.GetEventRecorderFor(name))),
 	)
 

internal/controller/pkg/pkg.go

@@ -19,40 +19,22 @@ package pkg
 import (
 	ctrl "sigs.k8s.io/controller-runtime"
 
-	"github.com/crossplane/crossplane-runtime/pkg/logging"
+	"github.com/crossplane/crossplane/internal/controller/pkg/controller"
 	"github.com/crossplane/crossplane/internal/controller/pkg/manager"
 	"github.com/crossplane/crossplane/internal/controller/pkg/resolver"
 	"github.com/crossplane/crossplane/internal/controller/pkg/revision"
-	"github.com/crossplane/crossplane/internal/xpkg"
 )
 
 // Setup package controllers.
-func Setup(mgr ctrl.Manager, l logging.Logger, c xpkg.Cache, namespace, registry, caBundlePath string) error {
-	fetcherOpts := []xpkg.FetcherOpt{}
-	if caBundlePath != "" {
-		rootCAs, err := xpkg.ParseCertificatesFromPath(caBundlePath)
-		if err != nil {
-			return err
-		}
-		fetcherOpts = append(fetcherOpts, xpkg.WithCustomCA(rootCAs))
-	}
-
-	for _, setup := range []func(ctrl.Manager, logging.Logger, string, string, ...xpkg.FetcherOpt) error{
+func Setup(mgr ctrl.Manager, o controller.Options) error {
+	for _, setup := range []func(ctrl.Manager, controller.Options) error{
 		manager.SetupConfiguration,
 		manager.SetupProvider,
-	} {
-		if err := setup(mgr, l, namespace, registry, fetcherOpts...); err != nil {
-			return err
-		}
-	}
-	if err := resolver.Setup(mgr, l, namespace, fetcherOpts...); err != nil {
-		return err
-	}
-	for _, setup := range []func(ctrl.Manager, logging.Logger, xpkg.Cache, string, string, ...xpkg.FetcherOpt) error{
+		resolver.Setup,
 		revision.SetupConfigurationRevision,
 		revision.SetupProviderRevision,
 	} {
-		if err := setup(mgr, l, c, namespace, registry, fetcherOpts...); err != nil {
+		if err := setup(mgr, o); err != nil {
 			return err
 		}
 	}