From 26b4bc2c63fcfb44504ef1817efc2fcf3789ec94 Mon Sep 17 00:00:00 2001 From: huchen2021 <85480625+huchen2021@users.noreply.github.com> Date: Thu, 13 Jan 2022 10:45:25 +0800 Subject: [PATCH] Define strategy to provide value for bundleLookupTag in Cluster Template (#287) * Define strategy to provide value for bundleLookupTag in Cluster Template Signed-off-by: Hui Chen --- PROJECT | 4 + .../v1beta1/byocluster_webhook.go | 67 +++++++++++ .../v1beta1/byocluster_webhook_test.go | 111 ++++++++++++++++++ .../v1beta1/webhook_suite_test.go | 5 + config/crd/kustomization.yaml | 4 +- config/default/webhookcainjection_patch.yaml | 6 + config/webhook/manifests.yaml | 48 ++++++++ main.go | 4 + 8 files changed, 247 insertions(+), 2 deletions(-) create mode 100644 apis/infrastructure/v1beta1/byocluster_webhook.go create mode 100644 apis/infrastructure/v1beta1/byocluster_webhook_test.go diff --git a/PROJECT b/PROJECT index a217fbfcf..5f2bb397c 100644 --- a/PROJECT +++ b/PROJECT @@ -32,6 +32,10 @@ resources: kind: ByoCluster path: github.com/vmware-tanzu/cluster-api-provider-bringyourownhost/apis/infrastructure/v1beta1 version: v1beta1 + webhooks: + defaulting: true + validation: true + webhookVersion: v1 - api: crdVersion: v1 namespaced: true diff --git a/apis/infrastructure/v1beta1/byocluster_webhook.go b/apis/infrastructure/v1beta1/byocluster_webhook.go new file mode 100644 index 000000000..994403c19 --- /dev/null +++ b/apis/infrastructure/v1beta1/byocluster_webhook.go @@ -0,0 +1,67 @@ +// Copyright 2021 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package v1beta1 + +import ( + "errors" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/validation/field" + ctrl "sigs.k8s.io/controller-runtime" + logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/webhook" +) + +// log is for logging in this package. +var byoclusterlog = logf.Log.WithName("byocluster-resource") + +func (r *ByoCluster) SetupWebhookWithManager(mgr ctrl.Manager) error { + return ctrl.NewWebhookManagedBy(mgr). + For(r). + Complete() +} + +//+kubebuilder:webhook:path=/mutate-infrastructure-cluster-x-k8s-io-v1beta1-byocluster,mutating=true,failurePolicy=fail,sideEffects=None,groups=infrastructure.cluster.x-k8s.io,resources=byoclusters,verbs=create;update,versions=v1beta1,name=mbyocluster.kb.io,admissionReviewVersions=v1 + +var _ webhook.Defaulter = &ByoCluster{} + +// Default implements webhook.Defaulter so a webhook will be registered for the type +// nolint: stylecheck +func (r *ByoCluster) Default() { +} + +// TODO(user): change verbs to "verbs=create;update;delete" if you want to enable deletion validation. +//+kubebuilder:webhook:path=/validate-infrastructure-cluster-x-k8s-io-v1beta1-byocluster,mutating=false,failurePolicy=fail,sideEffects=None,groups=infrastructure.cluster.x-k8s.io,resources=byoclusters,verbs=create;update,versions=v1beta1,name=vbyocluster.kb.io,admissionReviewVersions=v1 + +var _ webhook.Validator = &ByoCluster{} + +// ValidateCreate implements webhook.Validator so a webhook will be registered for the type +func (r *ByoCluster) ValidateCreate() error { + byoclusterlog.Info("validate create", "name", r.Name) + if r.Spec.BundleLookupTag == "" { + return apierrors.NewInvalid(r.GroupVersionKind().GroupKind(), r.Name, field.ErrorList{ + field.InternalError(nil, errors.New("cannot create ByoCluster without Spec.BundleLookupTag")), + }) + } + + return nil +} + +// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type +func (r *ByoCluster) ValidateUpdate(old runtime.Object) error { + byoclusterlog.Info("validate update", "name", r.Name) + if r.Spec.BundleLookupTag == "" { + return apierrors.NewInvalid(r.GroupVersionKind().GroupKind(), r.Name, field.ErrorList{ + field.InternalError(nil, errors.New("cannot update ByoCluster with empty Spec.BundleLookupTag")), + }) + } + return nil +} + +// ValidateDelete implements webhook.Validator so a webhook will be registered for the type +func (r *ByoCluster) ValidateDelete() error { + // TODO(user): fill in your validation logic upon object deletion. + return nil +} diff --git a/apis/infrastructure/v1beta1/byocluster_webhook_test.go b/apis/infrastructure/v1beta1/byocluster_webhook_test.go new file mode 100644 index 000000000..c25db3ad3 --- /dev/null +++ b/apis/infrastructure/v1beta1/byocluster_webhook_test.go @@ -0,0 +1,111 @@ +// Copyright 2021 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package v1beta1 + +import ( + "context" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/kubectl/pkg/scheme" + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +var _ = Describe("ByoclusterWebhook", func() { + Context("When ByoCluster gets a create request", func() { + var ( + byoCluster *ByoCluster + ctx context.Context + k8sClientUncached client.Client + ) + BeforeEach(func() { + ctx = context.Background() + var clientErr error + k8sClientUncached, clientErr = client.New(cfg, client.Options{Scheme: scheme.Scheme}) + Expect(clientErr).NotTo(HaveOccurred()) + + byoCluster = &ByoCluster{ + TypeMeta: metav1.TypeMeta{ + Kind: "ByoCluster", + APIVersion: clusterv1.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "byocluster-create", + Namespace: "default", + }, + Spec: ByoClusterSpec{}, + } + }) + + It("should reject the request when BundleLookupTag is empty", func() { + err := k8sClientUncached.Create(ctx, byoCluster) + Expect(err).To(HaveOccurred()) + Expect(err).To(MatchError("admission webhook \"vbyocluster.kb.io\" denied the request: ByoCluster.infrastructure.cluster.x-k8s.io \"" + byoCluster.Name + "\" is invalid: : Internal error: cannot create ByoCluster without Spec.BundleLookupTag")) + }) + + It("should success when BundleLookupTag is not empty", func() { + byoCluster.Spec.BundleLookupTag = "v0.1.0_alpha.2" + err := k8sClientUncached.Create(ctx, byoCluster) + Expect(err).NotTo(HaveOccurred()) + }) + + }) + + Context("When ByoCluster gets an update request", func() { + var ( + byoCluster *ByoCluster + ctx context.Context + k8sClientUncached client.Client + ) + BeforeEach(func() { + ctx = context.Background() + var clientErr error + k8sClientUncached, clientErr = client.New(cfg, client.Options{Scheme: scheme.Scheme}) + Expect(clientErr).NotTo(HaveOccurred()) + + byoCluster = &ByoCluster{ + TypeMeta: metav1.TypeMeta{ + Kind: "ByoCluster", + APIVersion: clusterv1.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "byocluster-update", + Namespace: "default", + }, + Spec: ByoClusterSpec{ + BundleLookupTag: "v0.1.0_alpha.2", + }, + } + + Expect(k8sClientUncached.Create(ctx, byoCluster)).Should(Succeed()) + }) + + AfterEach(func() { + Expect(k8sClientUncached.Delete(ctx, byoCluster)).Should(Succeed()) + }) + + It("should reject the request when BundleLookupTag is empty", func() { + byoCluster.Spec.BundleLookupTag = "" + err := k8sClientUncached.Update(ctx, byoCluster) + Expect(err).To(HaveOccurred()) + Expect(err).To(MatchError("admission webhook \"vbyocluster.kb.io\" denied the request: ByoCluster.infrastructure.cluster.x-k8s.io \"" + byoCluster.Name + "\" is invalid: : Internal error: cannot update ByoCluster with empty Spec.BundleLookupTag")) + }) + + It("should update the cluster with new BundleLookupTag value", func() { + newBundleLookupTag := "new_tag" + + byoCluster.Spec.BundleLookupTag = newBundleLookupTag + err := k8sClientUncached.Update(ctx, byoCluster) + Expect(err).NotTo(HaveOccurred()) + + updatedByoCluster := &ByoCluster{} + byoCLusterLookupKey := types.NamespacedName{Name: byoCluster.Name, Namespace: byoCluster.Namespace} + Expect(k8sClientUncached.Get(ctx, byoCLusterLookupKey, updatedByoCluster)).Should(Not(HaveOccurred())) + Expect(updatedByoCluster.Spec.BundleLookupTag).To(Equal(newBundleLookupTag)) + }) + }) +}) diff --git a/apis/infrastructure/v1beta1/webhook_suite_test.go b/apis/infrastructure/v1beta1/webhook_suite_test.go index 64eab5c50..09b9623cd 100644 --- a/apis/infrastructure/v1beta1/webhook_suite_test.go +++ b/apis/infrastructure/v1beta1/webhook_suite_test.go @@ -68,12 +68,14 @@ var _ = BeforeSuite(func() { Expect(cfg).NotTo(BeNil()) err = AddToScheme(scheme.Scheme) + Expect(err).NotTo(HaveOccurred()) err = admissionv1beta1.AddToScheme(scheme.Scheme) Expect(err).NotTo(HaveOccurred()) err = AddToScheme(scheme.Scheme) + Expect(err).NotTo(HaveOccurred()) //+kubebuilder:scaffold:scheme @@ -97,6 +99,9 @@ var _ = BeforeSuite(func() { err = (&ByoHost{}).SetupWebhookWithManager(mgr) Expect(err).NotTo(HaveOccurred()) + err = (&ByoCluster{}).SetupWebhookWithManager(mgr) + Expect(err).NotTo(HaveOccurred()) + //+kubebuilder:scaffold:webhook go func() { diff --git a/config/crd/kustomization.yaml b/config/crd/kustomization.yaml index 2cfbbdeb9..b800a1bc0 100644 --- a/config/crd/kustomization.yaml +++ b/config/crd/kustomization.yaml @@ -18,7 +18,7 @@ patchesStrategicMerge: #- patches/webhook_in_byomachines.yaml - patches/webhook_in_byohosts.yaml #- patches/webhook_in_byomachinetemplates.yaml -#- patches/webhook_in_byoclusters.yaml +- patches/webhook_in_byoclusters.yaml #- patches/webhook_in_byohosts.yaml #+kubebuilder:scaffold:crdkustomizewebhookpatch @@ -27,7 +27,7 @@ patchesStrategicMerge: #- patches/cainjection_in_byomachines.yaml - patches/cainjection_in_byohosts.yaml #- patches/cainjection_in_byomachinetemplates.yaml -#- patches/cainjection_in_byoclusters.yaml +- patches/cainjection_in_byoclusters.yaml #- patches/cainjection_in_byohosts.yaml #+kubebuilder:scaffold:crdkustomizecainjectionpatch diff --git a/config/default/webhookcainjection_patch.yaml b/config/default/webhookcainjection_patch.yaml index f5e23673e..02ab515d4 100644 --- a/config/default/webhookcainjection_patch.yaml +++ b/config/default/webhookcainjection_patch.yaml @@ -1,5 +1,11 @@ # This patch add annotation to admission webhook config and # the variables $(CERTIFICATE_NAMESPACE) and $(CERTIFICATE_NAME) will be substituted by kustomize. +apiVersion: admissionregistration.k8s.io/v1 +kind: MutatingWebhookConfiguration +metadata: + name: mutating-webhook-configuration + annotations: + cert-manager.io/inject-ca-from: $(CERTIFICATE_NAMESPACE)/$(CERTIFICATE_NAME) --- apiVersion: admissionregistration.k8s.io/v1 kind: ValidatingWebhookConfiguration diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index c39ccf70b..3acb6f693 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -1,4 +1,32 @@ +--- +apiVersion: admissionregistration.k8s.io/v1 +kind: MutatingWebhookConfiguration +metadata: + creationTimestamp: null + name: mutating-webhook-configuration +webhooks: +- admissionReviewVersions: + - v1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /mutate-infrastructure-cluster-x-k8s-io-v1beta1-byocluster + failurePolicy: Fail + name: mbyocluster.kb.io + rules: + - apiGroups: + - infrastructure.cluster.x-k8s.io + apiVersions: + - v1beta1 + operations: + - CREATE + - UPDATE + resources: + - byoclusters + sideEffects: None + --- apiVersion: admissionregistration.k8s.io/v1 kind: ValidatingWebhookConfiguration @@ -6,6 +34,26 @@ metadata: creationTimestamp: null name: validating-webhook-configuration webhooks: +- admissionReviewVersions: + - v1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /validate-infrastructure-cluster-x-k8s-io-v1beta1-byocluster + failurePolicy: Fail + name: vbyocluster.kb.io + rules: + - apiGroups: + - infrastructure.cluster.x-k8s.io + apiVersions: + - v1beta1 + operations: + - CREATE + - UPDATE + resources: + - byoclusters + sideEffects: None - admissionReviewVersions: - v1 - v1beta1 diff --git a/main.go b/main.go index 56a89415c..d4407740b 100644 --- a/main.go +++ b/main.go @@ -121,6 +121,10 @@ func main() { setupLog.Error(err, "unable to create webhook", "webhook", "ByoHost") os.Exit(1) } + if err = (&infrastructurev1beta1.ByoCluster{}).SetupWebhookWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create webhook", "webhook", "ByoCluster") + os.Exit(1) + } //+kubebuilder:scaffold:builder if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil {