review improvements

Signed-off-by: lsviben <[email protected]>

Makefile

@@ -130,6 +130,11 @@ generate.run: kustomize.gen
 
 generate.done: kustomize.clean
 
+# This hack is needed because we want to inject the conversion webhook
+# configuration into the Object CRD. This is not possible with the CRD
+# generation through controller-gen, and actually kubebuilder does
+# something similar, so we are following suit. Can be removed once we
+# drop support for v1alpha1.
 kustomize.gen: $(KUBECTL)
 	@$(INFO) Generating CRDs with kustomize
 	@$(KUBECTL) kustomize cluster/kustomize/ > cluster/kustomize/kubernetes.crossplane.io_objects.yaml

cmd/provider/main.go

@@ -102,6 +102,10 @@ func main() {
 		log.Info("Beta feature enabled", "flag", feature.EnableBetaManagementPolicies)
 	}
 
+	// NOTE(lsviben): We are registering the conversion webhook with v1alpha1
+	// Object. As far as I can see and based on some tests, it doesn't matter
+	// which version we use here. Leaving it as v1alpha1 as it will be easy to
+	// notice and remove when we drop support for v1alpha1.
 	kingpin.FatalIfError(ctrl.NewWebhookManagedBy(mgr).For(&v1alpha1.Object{}).Complete(), "Cannot create Object webhook")
 
 	kingpin.FatalIfError(object.Setup(mgr, o), "Cannot setup controller")

internal/controller/object/object.go

@@ -91,47 +91,33 @@ func Setup(mgr ctrl.Manager, o controller.Options) error {
 
 	cps := []managed.ConnectionPublisher{managed.NewAPISecretPublisher(mgr.GetClient(), mgr.GetScheme())}
 
-	// TODO(lsviben) there should be a better way to do this
-	var r *managed.Reconciler
+	reconcilerOptions := []managed.ReconcilerOption{
+		managed.WithExternalConnecter(&connector{
+			logger:          o.Logger,
+			kube:            mgr.GetClient(),
+			usage:           resource.NewProviderConfigUsageTracker(mgr.GetClient(), &apisv1alpha1.ProviderConfigUsage{}),
+			kcfgExtractorFn: resource.CommonCredentialExtractor,
+			gcpExtractorFn:  resource.CommonCredentialExtractor,
+			gcpInjectorFn:   gke.WrapRESTConfig,
+			newRESTConfigFn: clients.NewRESTConfig,
+			newKubeClientFn: clients.NewKubeClient,
+		}),
+		managed.WithFinalizer(&objFinalizer{client: mgr.GetClient()}),
+		managed.WithPollInterval(o.PollInterval),
+		managed.WithLogger(o.Logger.WithValues("controller", name)),
+		managed.WithRecorder(event.NewAPIRecorder(mgr.GetEventRecorderFor(name))),
+		managed.WithConnectionPublishers(cps...),
+	}
+
 	if o.Features.Enabled(feature.EnableBetaManagementPolicies) {
-		r = managed.NewReconciler(mgr,
-			resource.ManagedKind(v1beta1.ObjectGroupVersionKind),
-			managed.WithExternalConnecter(&connector{
-				logger:          o.Logger,
-				kube:            mgr.GetClient(),
-				usage:           resource.NewProviderConfigUsageTracker(mgr.GetClient(), &apisv1alpha1.ProviderConfigUsage{}),
-				kcfgExtractorFn: resource.CommonCredentialExtractor,
-				gcpExtractorFn:  resource.CommonCredentialExtractor,
-				gcpInjectorFn:   gke.WrapRESTConfig,
-				newRESTConfigFn: clients.NewRESTConfig,
-				newKubeClientFn: clients.NewKubeClient,
-			}),
-			managed.WithFinalizer(&objFinalizer{client: mgr.GetClient()}),
-			managed.WithPollInterval(o.PollInterval),
-			managed.WithLogger(o.Logger.WithValues("controller", name)),
-			managed.WithRecorder(event.NewAPIRecorder(mgr.GetEventRecorderFor(name))),
-			managed.WithConnectionPublishers(cps...),
-			managed.WithManagementPolicies())
-	} else {
-		r = managed.NewReconciler(mgr,
-			resource.ManagedKind(v1beta1.ObjectGroupVersionKind),
-			managed.WithExternalConnecter(&connector{
-				logger:          o.Logger,
-				kube:            mgr.GetClient(),
-				usage:           resource.NewProviderConfigUsageTracker(mgr.GetClient(), &apisv1alpha1.ProviderConfigUsage{}),
-				kcfgExtractorFn: resource.CommonCredentialExtractor,
-				gcpExtractorFn:  resource.CommonCredentialExtractor,
-				gcpInjectorFn:   gke.WrapRESTConfig,
-				newRESTConfigFn: clients.NewRESTConfig,
-				newKubeClientFn: clients.NewKubeClient,
-			}),
-			managed.WithFinalizer(&objFinalizer{client: mgr.GetClient()}),
-			managed.WithPollInterval(o.PollInterval),
-			managed.WithLogger(o.Logger.WithValues("controller", name)),
-			managed.WithRecorder(event.NewAPIRecorder(mgr.GetEventRecorderFor(name))),
-			managed.WithConnectionPublishers(cps...))
+		reconcilerOptions = append(reconcilerOptions, managed.WithManagementPolicies())
 	}
 
+	r := managed.NewReconciler(mgr,
+		resource.ManagedKind(v1beta1.ObjectGroupVersionKind),
+		reconcilerOptions...,
+	)
+
 	return ctrl.NewControllerManagedBy(mgr).
 		Named(name).
 		WithOptions(o.ForControllerRuntime()).