From c69ebb805a284ac3ee2180a9dca602214f86091a Mon Sep 17 00:00:00 2001 From: lsviben Date: Mon, 11 Dec 2023 12:53:07 +0100 Subject: [PATCH] review improvements Signed-off-by: lsviben --- Makefile | 5 ++ cmd/provider/main.go | 4 ++ .../{migration => deprecated}/default.yaml | 0 .../observe-create-update.yaml | 0 .../observe-delete.yaml | 0 .../{migration => deprecated}/observe.yaml | 0 internal/controller/object/object.go | 62 +++++++------------ 7 files changed, 33 insertions(+), 38 deletions(-) rename examples/object/{migration => deprecated}/default.yaml (100%) rename examples/object/{migration => deprecated}/observe-create-update.yaml (100%) rename examples/object/{migration => deprecated}/observe-delete.yaml (100%) rename examples/object/{migration => deprecated}/observe.yaml (100%) diff --git a/Makefile b/Makefile index 45d920b3..2c8578e9 100644 --- a/Makefile +++ b/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 diff --git a/cmd/provider/main.go b/cmd/provider/main.go index f4d50c63..5979bba8 100644 --- a/cmd/provider/main.go +++ b/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") diff --git a/examples/object/migration/default.yaml b/examples/object/deprecated/default.yaml similarity index 100% rename from examples/object/migration/default.yaml rename to examples/object/deprecated/default.yaml diff --git a/examples/object/migration/observe-create-update.yaml b/examples/object/deprecated/observe-create-update.yaml similarity index 100% rename from examples/object/migration/observe-create-update.yaml rename to examples/object/deprecated/observe-create-update.yaml diff --git a/examples/object/migration/observe-delete.yaml b/examples/object/deprecated/observe-delete.yaml similarity index 100% rename from examples/object/migration/observe-delete.yaml rename to examples/object/deprecated/observe-delete.yaml diff --git a/examples/object/migration/observe.yaml b/examples/object/deprecated/observe.yaml similarity index 100% rename from examples/object/migration/observe.yaml rename to examples/object/deprecated/observe.yaml diff --git a/internal/controller/object/object.go b/internal/controller/object/object.go index a0ca5696..931e45b9 100644 --- a/internal/controller/object/object.go +++ b/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()).