From 5b0a69e220aae57cab61563e77fa64b51366eb15 Mon Sep 17 00:00:00 2001 From: Lan Luo Date: Thu, 21 Jul 2022 16:17:47 +0800 Subject: [PATCH] Update ClusterSet and ClusterClaim webhooks 1. Add a ClusterSet webhook to make Leader's ClusterID immutable, and allow only one ClusterSet in mc-controller's Namespace. 2. Update ClusterClaim webhook to make `Value` field (ClusterID/ClusterSetID) immutable 3. Remove unused mutate webhook of ClusterClaim Signed-off-by: Lan Luo --- .../v1alpha2/clusterclaim_webhook.go | 16 +- .../antrea-multicluster-leader-namespaced.yml | 30 +-- .../yamls/antrea-multicluster-member.yml | 28 +-- .../clusterset_webhook.go | 83 +++++++ .../clusterset_webhook_test.go | 213 ++++++++++++++++++ .../cmd/multicluster-controller/controller.go | 8 + .../cmd/multicluster-controller/main.go | 2 +- .../config/overlays/member/webhook_patch.yaml | 14 +- multicluster/config/webhook/manifests.yaml | 26 +-- 9 files changed, 363 insertions(+), 57 deletions(-) create mode 100644 multicluster/cmd/multicluster-controller/clusterset_webhook.go create mode 100644 multicluster/cmd/multicluster-controller/clusterset_webhook_test.go diff --git a/multicluster/apis/multicluster/v1alpha2/clusterclaim_webhook.go b/multicluster/apis/multicluster/v1alpha2/clusterclaim_webhook.go index 9633d2de2cd..0c655313d01 100644 --- a/multicluster/apis/multicluster/v1alpha2/clusterclaim_webhook.go +++ b/multicluster/apis/multicluster/v1alpha2/clusterclaim_webhook.go @@ -31,8 +31,6 @@ func (r *ClusterClaim) SetupWebhookWithManager(mgr ctrl.Manager) error { Complete() } -//+kubebuilder:webhook:path=/mutate-multicluster-crd-antrea-io-v1alpha2-clusterclaim,mutating=true,failurePolicy=fail,sideEffects=None,groups=multicluster.crd.antrea.io,resources=clusterclaims,verbs=create;update,versions=v1alpha2,name=mclusterclaim.kb.io,admissionReviewVersions={v1,v1beta1} - var _ webhook.Defaulter = &ClusterClaim{} // Default implements webhook.Defaulter so a webhook will be registered for the type @@ -49,9 +47,9 @@ var _ webhook.Validator = &ClusterClaim{} // ValidateCreate implements webhook.Validator so a webhook will be registered for the type func (r *ClusterClaim) ValidateCreate() error { - klog.InfoS("validate create", "name", r.Name) + klog.InfoS("Validate create", "name", r.Name) if r.Name != WellKnownClusterClaimClusterSet && r.Name != WellKnownClusterClaimID { - err := fmt.Errorf("The name %s is not valid, only 'id.k8s.io' and 'clusterset.k8s.io' are valid names for ClusterClaim", r.Name) + err := fmt.Errorf("the name %s is not valid, only 'id.k8s.io' and 'clusterset.k8s.io' are valid names for ClusterClaim", r.Name) return err } @@ -60,15 +58,19 @@ func (r *ClusterClaim) ValidateCreate() error { // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type func (r *ClusterClaim) ValidateUpdate(old runtime.Object) error { - klog.InfoS("validate update", "name", r.Name) + klog.InfoS("Validate update", "name", r.Name) - // TODO(user): fill in your validation logic upon object update. + oldClusterClaim := old.(*ClusterClaim) + if r.Value != oldClusterClaim.Value { + err := fmt.Errorf("the field 'value' is immutable") + return err + } return nil } // ValidateDelete implements webhook.Validator so a webhook will be registered for the type func (r *ClusterClaim) ValidateDelete() error { - klog.InfoS("validate delete", "name", r.Name) + klog.InfoS("Validate delete", "name", r.Name) // TODO(user): fill in your validation logic upon object deletion. return nil diff --git a/multicluster/build/yamls/antrea-multicluster-leader-namespaced.yml b/multicluster/build/yamls/antrea-multicluster-leader-namespaced.yml index 5ebb7c78cdc..22d84dda034 100644 --- a/multicluster/build/yamls/antrea-multicluster-leader-namespaced.yml +++ b/multicluster/build/yamls/antrea-multicluster-leader-namespaced.yml @@ -441,6 +441,15 @@ webhooks: resources: - resourceexports sideEffects: None +--- +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingWebhookConfiguration +metadata: + creationTimestamp: null + labels: + app: antrea + name: antrea-multicluster-antrea-mc-validating-webhook-configuration +webhooks: - admissionReviewVersions: - v1 - v1beta1 @@ -448,9 +457,9 @@ webhooks: service: name: antrea-mc-webhook-service namespace: antrea-multicluster - path: /mutate-multicluster-crd-antrea-io-v1alpha2-clusterclaim + path: /validate-multicluster-crd-antrea-io-v1alpha2-clusterclaim failurePolicy: Fail - name: mclusterclaim.kb.io + name: vclusterclaim.kb.io namespaceSelector: matchLabels: kubernetes.io/metadata.name: antrea-multicluster @@ -465,15 +474,6 @@ webhooks: resources: - clusterclaims sideEffects: None ---- -apiVersion: admissionregistration.k8s.io/v1 -kind: ValidatingWebhookConfiguration -metadata: - creationTimestamp: null - labels: - app: antrea - name: antrea-multicluster-antrea-mc-validating-webhook-configuration -webhooks: - admissionReviewVersions: - v1 - v1beta1 @@ -481,9 +481,9 @@ webhooks: service: name: antrea-mc-webhook-service namespace: antrea-multicluster - path: /validate-multicluster-crd-antrea-io-v1alpha2-clusterclaim + path: /validate-multicluster-crd-antrea-io-v1alpha1-clusterset failurePolicy: Fail - name: vclusterclaim.kb.io + name: vclusterset.kb.io namespaceSelector: matchLabels: kubernetes.io/metadata.name: antrea-multicluster @@ -491,12 +491,12 @@ webhooks: - apiGroups: - multicluster.crd.antrea.io apiVersions: - - v1alpha2 + - v1alpha1 operations: - CREATE - UPDATE resources: - - clusterclaims + - clustersets sideEffects: None - admissionReviewVersions: - v1 diff --git a/multicluster/build/yamls/antrea-multicluster-member.yml b/multicluster/build/yamls/antrea-multicluster-member.yml index 3fb72522510..45cf629373b 100644 --- a/multicluster/build/yamls/antrea-multicluster-member.yml +++ b/multicluster/build/yamls/antrea-multicluster-member.yml @@ -1041,6 +1041,14 @@ metadata: labels: app: antrea name: antrea-mc-mutating-webhook-configuration +webhooks: [] +--- +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingWebhookConfiguration +metadata: + labels: + app: antrea + name: antrea-mc-validating-webhook-configuration webhooks: - admissionReviewVersions: - v1 @@ -1049,9 +1057,9 @@ webhooks: service: name: antrea-mc-webhook-service namespace: kube-system - path: /mutate-multicluster-crd-antrea-io-v1alpha2-clusterclaim + path: /validate-multicluster-crd-antrea-io-v1alpha2-clusterclaim failurePolicy: Fail - name: mclusterclaim.kb.io + name: vclusterclaim.kb.io namespaceSelector: matchLabels: kubernetes.io/metadata.name: kube-system @@ -1066,14 +1074,6 @@ webhooks: resources: - clusterclaims sideEffects: None ---- -apiVersion: admissionregistration.k8s.io/v1 -kind: ValidatingWebhookConfiguration -metadata: - labels: - app: antrea - name: antrea-mc-validating-webhook-configuration -webhooks: - admissionReviewVersions: - v1 - v1beta1 @@ -1081,9 +1081,9 @@ webhooks: service: name: antrea-mc-webhook-service namespace: kube-system - path: /validate-multicluster-crd-antrea-io-v1alpha2-clusterclaim + path: /validate-multicluster-crd-antrea-io-v1alpha1-clusterset failurePolicy: Fail - name: vclusterclaim.kb.io + name: vclusterset.kb.io namespaceSelector: matchLabels: kubernetes.io/metadata.name: kube-system @@ -1091,10 +1091,10 @@ webhooks: - apiGroups: - multicluster.crd.antrea.io apiVersions: - - v1alpha2 + - v1alpha1 operations: - CREATE - UPDATE resources: - - clusterclaims + - clustersets sideEffects: None diff --git a/multicluster/cmd/multicluster-controller/clusterset_webhook.go b/multicluster/cmd/multicluster-controller/clusterset_webhook.go new file mode 100644 index 00000000000..22b1c0e5a50 --- /dev/null +++ b/multicluster/cmd/multicluster-controller/clusterset_webhook.go @@ -0,0 +1,83 @@ +/* +Copyright 2022 Antrea 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 main + +import ( + "context" + "fmt" + "net/http" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" + "k8s.io/klog/v2" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + mcv1alpha1 "antrea.io/antrea/multicluster/apis/multicluster/v1alpha1" +) + +//+kubebuilder:webhook:path=/validate-multicluster-crd-antrea-io-v1alpha1-clusterset,mutating=false,failurePolicy=fail,sideEffects=None,groups=multicluster.crd.antrea.io,resources=clustersets,verbs=create;update,versions=v1alpha1,name=vclusterset.kb.io,admissionReviewVersions={v1,v1beta1} + +// ClusterSet validator +type clusterSetValidator struct { + Client client.Client + decoder *admission.Decoder + namespace string +} + +// Handle handles admission requests. +func (v *clusterSetValidator) Handle(ctx context.Context, req admission.Request) admission.Response { + clusterSet := &mcv1alpha1.ClusterSet{} + err := v.decoder.Decode(req, clusterSet) + if err != nil { + klog.ErrorS(err, "Error while decoding") + return admission.Errored(http.StatusBadRequest, err) + } + + oldClusterSet := &mcv1alpha1.ClusterSet{} + err = v.Client.Get(ctx, types.NamespacedName{Namespace: v.namespace, Name: clusterSet.GetName()}, oldClusterSet) + if err != nil && !apierrors.IsNotFound(err) { + return admission.Errored(http.StatusPreconditionFailed, + fmt.Errorf("failed to get existing ClusterSet %s", clusterSet.GetName())) + } + + // Check if there is any existing ClusterSet. + clusterSetList := &mcv1alpha1.ClusterSetList{} + if err := v.Client.List(context.TODO(), clusterSetList, client.InNamespace(v.namespace)); err != nil { + klog.ErrorS(err, "Error reading ClusterSet", "namespace", v.namespace) + return admission.Errored(http.StatusPreconditionFailed, err) + } + + if apierrors.IsNotFound(err) { + if len(clusterSetList.Items) > 0 { + return admission.Errored(http.StatusPreconditionFailed, + fmt.Errorf("multiple ClusterSets in the Namespace %s are not allowed, please contact your administrator", v.namespace)) + } + return admission.Allowed("") + } + + if oldClusterSet.Spec.Leaders[0].ClusterID != clusterSet.Spec.Leaders[0].ClusterID { + return admission.Denied("the field 'clusterID' is immutable") + } + + return admission.Allowed("") +} + +func (v *clusterSetValidator) InjectDecoder(d *admission.Decoder) error { + v.decoder = d + return nil +} diff --git a/multicluster/cmd/multicluster-controller/clusterset_webhook_test.go b/multicluster/cmd/multicluster-controller/clusterset_webhook_test.go new file mode 100644 index 00000000000..98321503af8 --- /dev/null +++ b/multicluster/cmd/multicluster-controller/clusterset_webhook_test.go @@ -0,0 +1,213 @@ +/* +Copyright 2022 Antrea 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 main + +import ( + "context" + j "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + v1 "k8s.io/api/admission/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "k8s.io/klog/v2" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + k8smcsv1alpha1 "sigs.k8s.io/mcs-api/pkg/apis/v1alpha1" + + mcsv1alpha1 "antrea.io/antrea/multicluster/apis/multicluster/v1alpha1" +) + +var clusterSetWebhookUnderTest *clusterSetValidator + +func TestWebhookClusterSetEvents(t *testing.T) { + newClusterSet := &mcsv1alpha1.ClusterSet{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "mcs1", + Name: "clusterset1", + }, + Spec: mcsv1alpha1.ClusterSetSpec{ + Leaders: []mcsv1alpha1.MemberCluster{ + { + ClusterID: "leader1", + }}, + Members: []mcsv1alpha1.MemberCluster{ + { + ClusterID: "east", + ServiceAccount: "east-access-sa", + }, + { + ClusterID: "west", + ServiceAccount: "west-access-sa", + }, + }, + Namespace: "mcs-A", + }, + } + + existingClusterSet1 := &mcsv1alpha1.ClusterSet{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "mcs1", + Name: "clusterset1", + }, + Spec: mcsv1alpha1.ClusterSetSpec{ + Leaders: []mcsv1alpha1.MemberCluster{ + { + ClusterID: "leader1", + }}, + Members: []mcsv1alpha1.MemberCluster{ + { + ClusterID: "east", + ServiceAccount: "east-access-sa", + }, + { + ClusterID: "west", + ServiceAccount: "west-access-sa", + }, + }, + Namespace: "mcs-A", + }, + } + + existingClusterSet2 := existingClusterSet1.DeepCopy() + existingClusterSet2.Name = "clusterset2" + + updatedClusterSet := &mcsv1alpha1.ClusterSet{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "mcs1", + Name: "clusterset1", + }, + Spec: mcsv1alpha1.ClusterSetSpec{ + Leaders: []mcsv1alpha1.MemberCluster{ + { + ClusterID: "leader1-1", + }}, + Members: []mcsv1alpha1.MemberCluster{ + { + ClusterID: "east", + ServiceAccount: "east-access-sa", + }, + { + ClusterID: "west", + ServiceAccount: "west-access-sa", + }, + }, + Namespace: "mcs-A", + }, + } + newCS, _ := j.Marshal(newClusterSet) + updatedCS, _ := j.Marshal(updatedClusterSet) + + newReq := admission.Request{ + AdmissionRequest: v1.AdmissionRequest{ + UID: "07e52e8d-4513-11e9-a716-42010a800270", + Kind: metav1.GroupVersionKind{ + Group: "multicluster.crd.antrea.io", + Version: "v1alpha1", + Kind: "ClusterSet", + }, + Resource: metav1.GroupVersionResource{ + Group: "multicluster.crd.antrea.io", + Version: "v1alpha1", + Resource: "ClusterSets", + }, + Name: "clusterset1", + Namespace: "mcs1", + Operation: v1.Create, + Object: runtime.RawExtension{ + Raw: newCS, + }, + }, + } + + updatedReq := admission.Request{ + AdmissionRequest: v1.AdmissionRequest{ + UID: "07e52e8d-4513-11e9-a716-42010a800270", + Kind: metav1.GroupVersionKind{ + Group: "multicluster.crd.antrea.io", + Version: "v1alpha1", + Kind: "ClusterSet", + }, + Resource: metav1.GroupVersionResource{ + Group: "multicluster.crd.antrea.io", + Version: "v1alpha1", + Resource: "ClusterSets", + }, + Name: "clusterset1", + Namespace: "mcs1", + Operation: v1.Update, + Object: runtime.RawExtension{ + Raw: updatedCS, + }, + }, + } + + tests := []struct { + name string + req admission.Request + existingClusterSet *mcsv1alpha1.ClusterSet + newClusterSet *mcsv1alpha1.ClusterSet + isAllowed bool + }{ + { + name: "create a new ClusterSet", + req: newReq, + isAllowed: true, + }, + { + name: "create a new ClusterSet when there is an existing ClusterSet", + existingClusterSet: existingClusterSet2, + req: newReq, + isAllowed: false, + }, + { + name: "update a new ClusterSet's leader ClusterID when there is an existing ClusterSet", + existingClusterSet: existingClusterSet1, + newClusterSet: updatedClusterSet, + req: updatedReq, + isAllowed: false, + }, + } + + newScheme := runtime.NewScheme() + utilruntime.Must(clientgoscheme.AddToScheme(newScheme)) + utilruntime.Must(k8smcsv1alpha1.AddToScheme(newScheme)) + utilruntime.Must(mcsv1alpha1.AddToScheme(newScheme)) + decoder, err := admission.NewDecoder(newScheme) + if err != nil { + klog.ErrorS(err, "Error constructing a decoder") + } + + for _, tt := range tests { + fakeClient := fake.NewClientBuilder().WithScheme(newScheme).WithObjects().Build() + if tt.existingClusterSet != nil { + fakeClient = fake.NewClientBuilder().WithScheme(newScheme).WithObjects(tt.existingClusterSet).Build() + } + clusterSetWebhookUnderTest = &clusterSetValidator{ + Client: fakeClient, + namespace: "mcs1"} + clusterSetWebhookUnderTest.InjectDecoder(decoder) + + t.Run(tt.name, func(t *testing.T) { + response := clusterSetWebhookUnderTest.Handle(context.Background(), tt.req) + assert.Equal(t, tt.isAllowed, response.Allowed) + }) + } +} diff --git a/multicluster/cmd/multicluster-controller/controller.go b/multicluster/cmd/multicluster-controller/controller.go index 328e41075c8..14e8927c3f9 100644 --- a/multicluster/cmd/multicluster-controller/controller.go +++ b/multicluster/cmd/multicluster-controller/controller.go @@ -36,12 +36,14 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/healthz" "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/webhook" k8smcsv1alpha1 "sigs.k8s.io/mcs-api/pkg/apis/v1alpha1" multiclusterv1alpha1 "antrea.io/antrea/multicluster/apis/multicluster/v1alpha1" multiclusterv1alpha2 "antrea.io/antrea/multicluster/apis/multicluster/v1alpha2" antreacrd "antrea.io/antrea/pkg/apis/crd/v1alpha1" "antrea.io/antrea/pkg/apiserver/certificate" + "antrea.io/antrea/pkg/util/env" // +kubebuilder:scaffold:imports ) @@ -150,6 +152,12 @@ func setupManagerAndCertController(o *Options) (manager.Manager, error) { return nil, fmt.Errorf("error create ClusterClaim webhook: %v", err) } + hookServer := mgr.GetWebhookServer() + hookServer.Register("/validate-multicluster-crd-antrea-io-v1alpha1-clusterset", + &webhook.Admission{Handler: &clusterSetValidator{ + Client: mgr.GetClient(), + namespace: env.GetPodNamespace()}}) + //+kubebuilder:scaffold:builder if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil { diff --git a/multicluster/cmd/multicluster-controller/main.go b/multicluster/cmd/multicluster-controller/main.go index e2f20a8fd49..af585913826 100644 --- a/multicluster/cmd/multicluster-controller/main.go +++ b/multicluster/cmd/multicluster-controller/main.go @@ -48,7 +48,7 @@ func main() { func newControllerCommand() *cobra.Command { return &cobra.Command{ Use: "antrea-mc-controller", - Long: "The Antrea MultiCluster Controller.", + Long: "The Antrea Multi-cluster Controller.", Run: func(cmd *cobra.Command, args []string) { fmt.Println("Error: must be run in leader or member mode") }, diff --git a/multicluster/config/overlays/member/webhook_patch.yaml b/multicluster/config/overlays/member/webhook_patch.yaml index d61ccf02c55..d2f2c1ea3c7 100644 --- a/multicluster/config/overlays/member/webhook_patch.yaml +++ b/multicluster/config/overlays/member/webhook_patch.yaml @@ -24,13 +24,6 @@ webhooks: - admissionReviewVersions: name: mresourceimport.kb.io $patch: delete -- admissionReviewVersions: - - v1 - - v1beta1 - name: mclusterclaim.kb.io - namespaceSelector: - matchLabels: - kubernetes.io/metadata.name: kube-system --- apiVersion: admissionregistration.k8s.io/v1 kind: ValidatingWebhookConfiguration @@ -44,3 +37,10 @@ webhooks: namespaceSelector: matchLabels: kubernetes.io/metadata.name: kube-system +- admissionReviewVersions: + - v1 + - v1beta1 + name: vclusterset.kb.io + namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: kube-system diff --git a/multicluster/config/webhook/manifests.yaml b/multicluster/config/webhook/manifests.yaml index 1bd335a0d86..c3d94e305e4 100644 --- a/multicluster/config/webhook/manifests.yaml +++ b/multicluster/config/webhook/manifests.yaml @@ -26,6 +26,13 @@ webhooks: resources: - resourceexports sideEffects: None +--- +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingWebhookConfiguration +metadata: + creationTimestamp: null + name: validating-webhook-configuration +webhooks: - admissionReviewVersions: - v1 - v1beta1 @@ -33,9 +40,9 @@ webhooks: service: name: webhook-service namespace: system - path: /mutate-multicluster-crd-antrea-io-v1alpha2-clusterclaim + path: /validate-multicluster-crd-antrea-io-v1alpha2-clusterclaim failurePolicy: Fail - name: mclusterclaim.kb.io + name: vclusterclaim.kb.io rules: - apiGroups: - multicluster.crd.antrea.io @@ -47,13 +54,6 @@ webhooks: resources: - clusterclaims sideEffects: None ---- -apiVersion: admissionregistration.k8s.io/v1 -kind: ValidatingWebhookConfiguration -metadata: - creationTimestamp: null - name: validating-webhook-configuration -webhooks: - admissionReviewVersions: - v1 - v1beta1 @@ -61,19 +61,19 @@ webhooks: service: name: webhook-service namespace: system - path: /validate-multicluster-crd-antrea-io-v1alpha2-clusterclaim + path: /validate-multicluster-crd-antrea-io-v1alpha1-clusterset failurePolicy: Fail - name: vclusterclaim.kb.io + name: vclusterset.kb.io rules: - apiGroups: - multicluster.crd.antrea.io apiVersions: - - v1alpha2 + - v1alpha1 operations: - CREATE - UPDATE resources: - - clusterclaims + - clustersets sideEffects: None - admissionReviewVersions: - v1