From b8a1f9d676a3d2aeb68482d96ce53739c3fc867e Mon Sep 17 00:00:00 2001 From: DangPeng Liu Date: Thu, 3 Mar 2022 00:06:28 +0800 Subject: [PATCH] set default in webhook (#205) Signed-off-by: ldpliu --- pkg/cmd/webhook/webhook.go | 4 ++ pkg/webhook/cluster/mutating_webhook.go | 56 ++++++++++++++- pkg/webhook/cluster/mutating_webhook_test.go | 68 +++++++++++++++++- test/e2e/e2e_suite_test.go | 24 ++++++- test/e2e/webhook_test.go | 72 ++++++++++++++++++-- 5 files changed, 214 insertions(+), 10 deletions(-) diff --git a/pkg/cmd/webhook/webhook.go b/pkg/cmd/webhook/webhook.go index fef9cffa3..64391c092 100644 --- a/pkg/cmd/webhook/webhook.go +++ b/pkg/cmd/webhook/webhook.go @@ -6,6 +6,7 @@ import ( admissionserver "github.com/openshift/generic-admission-server/pkg/cmd/server" "github.com/spf13/cobra" genericapiserver "k8s.io/apiserver/pkg/server" + "open-cluster-management.io/registration/pkg/features" clusterwebhook "open-cluster-management.io/registration/pkg/webhook/cluster" clustersetbindingwebhook "open-cluster-management.io/registration/pkg/webhook/clustersetbinding" ) @@ -39,5 +40,8 @@ func NewAdmissionHook() *cobra.Command { o.RecommendedOptions.AddFlags(cmd.Flags()) + flags := cmd.Flags() + features.DefaultHubMutableFeatureGate.AddFlag(flags) + return cmd } diff --git a/pkg/webhook/cluster/mutating_webhook.go b/pkg/webhook/cluster/mutating_webhook.go index eed74c512..85d43bfea 100644 --- a/pkg/webhook/cluster/mutating_webhook.go +++ b/pkg/webhook/cluster/mutating_webhook.go @@ -7,17 +7,19 @@ import ( "strings" "time" - clusterv1 "open-cluster-management.io/api/cluster/v1" - "open-cluster-management.io/registration/pkg/helpers" - + "github.com/openshift/library-go/pkg/operator/resource/resourcemerge" admissionv1beta1 "k8s.io/api/admission/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/rest" "k8s.io/klog/v2" + clusterv1 "open-cluster-management.io/api/cluster/v1" + "open-cluster-management.io/registration/pkg/features" + "open-cluster-management.io/registration/pkg/helpers" ) var nowFunc = time.Now +var defaultClusterSetName = "default" type jsonPatchOperation struct { Operation string `json:"op"` @@ -77,6 +79,14 @@ func (a *ManagedClusterMutatingAdmissionHook) Admit(req *admissionv1beta1.Admiss } jsonPatches = append(jsonPatches, taintJsonPatches...) + if features.DefaultHubMutableFeatureGate.Enabled(features.DefaultClusterSet) { + labelJsonPatches, status := a.addDefaultClusterSetLabel(managedCluster, req.Object.Raw) + if !status.Allowed { + return status + } + jsonPatches = append(jsonPatches, labelJsonPatches...) + } + if len(jsonPatches) == 0 { return status } @@ -97,6 +107,38 @@ func (a *ManagedClusterMutatingAdmissionHook) Admit(req *admissionv1beta1.Admiss return status } +//addDefaultClusterSetLabel add label "cluster.open-cluster-management.io/clusterset:default" for ManagedCluster if the managedCluster has no ManagedClusterSet label +func (a *ManagedClusterMutatingAdmissionHook) addDefaultClusterSetLabel(managedCluster *clusterv1.ManagedCluster, clusterObj []byte) ([]jsonPatchOperation, *admissionv1beta1.AdmissionResponse) { + cluster := managedCluster.DeepCopy() + modified := false + var jsonPatches []jsonPatchOperation + + status := &admissionv1beta1.AdmissionResponse{ + Allowed: true, + } + + if len(managedCluster.Labels) != 0 { + if _, ok := managedCluster.Labels[clusterSetLabel]; ok { + return nil, status + } + } + + clusterSetLabels := map[string]string{} + clusterSetLabels[clusterSetLabel] = defaultClusterSetName + // merge clusterSetLabel into ManagedCluster.Labels + resourcemerge.MergeMap(&modified, &cluster.Labels, clusterSetLabels) + + // no work if the cluster labels have no change + if !modified { + return nil, status + } + + labelPatch := newLabelJsonPatch() + + jsonPatches = append(jsonPatches, labelPatch) + return jsonPatches, status +} + // processTaints generates json patched for cluster taints func (a *ManagedClusterMutatingAdmissionHook) processTaints(managedCluster *clusterv1.ManagedCluster, oldManagedClusterRaw []byte) ([]jsonPatchOperation, *admissionv1beta1.AdmissionResponse) { status := &admissionv1beta1.AdmissionResponse{ @@ -174,3 +216,11 @@ func newTaintTimeAddedJsonPatch(index int, timeAdded time.Time) jsonPatchOperati Value: timeAdded.UTC().Format(time.RFC3339), } } + +func newLabelJsonPatch() jsonPatchOperation { + return jsonPatchOperation{ + Operation: "add", + Path: fmt.Sprintf("/metadata/labels"), + Value: map[string]string{clusterSetLabel: defaultClusterSetName}, + } +} diff --git a/pkg/webhook/cluster/mutating_webhook_test.go b/pkg/webhook/cluster/mutating_webhook_test.go index debe4fc2b..dbc2f1482 100644 --- a/pkg/webhook/cluster/mutating_webhook_test.go +++ b/pkg/webhook/cluster/mutating_webhook_test.go @@ -7,11 +7,13 @@ import ( "testing" "time" + "github.com/openshift/library-go/pkg/operator/resource/resourcemerge" clusterv1 "open-cluster-management.io/api/cluster/v1" admissionv1beta1 "k8s.io/api/admission/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "open-cluster-management.io/registration/pkg/features" testinghelpers "open-cluster-management.io/registration/pkg/helpers/testing" ) @@ -51,6 +53,7 @@ func TestManagedClusterMutate(t *testing.T) { withLeaseDurationSeconds(60). addTaint(newTaint("a", "b", clusterv1.TaintEffectNoSelect, nil)). addTaint(newTaint("c", "d", clusterv1.TaintEffectPreferNoSelect, nil)). + addLabels(map[string]string{clusterSetLabel: defaultClusterSetName}). build(), }, expectedResponse: newAdmissionResponse(true). @@ -67,6 +70,7 @@ func TestManagedClusterMutate(t *testing.T) { withLeaseDurationSeconds(60). addTaint(newTaint("a", "b", clusterv1.TaintEffectNoSelect, newTime(now, 0))). addTaint(newTaint("c", "d", clusterv1.TaintEffectPreferNoSelect, newTime(now, 0))). + addLabels(map[string]string{clusterSetLabel: defaultClusterSetName}). build(), }, expectedResponse: newAdmissionResponse(false). @@ -82,11 +86,13 @@ func TestManagedClusterMutate(t *testing.T) { withLeaseDurationSeconds(60). addTaint(newTaint("a", "b", clusterv1.TaintEffectNoSelect, newTime(now, -10*time.Second))). addTaint(newTaint("c", "d", clusterv1.TaintEffectNoSelect, newTime(now, -10*time.Second))). + addLabels(map[string]string{clusterSetLabel: defaultClusterSetName}). build(), Object: newManagedCluster(). withLeaseDurationSeconds(60). addTaint(newTaint("a", "b", clusterv1.TaintEffectNoSelect, newTime(now, -10*time.Second))). // no change addTaint(newTaint("c", "d", clusterv1.TaintEffectNoSelectIfNew, nil)). // effect modified + addLabels(map[string]string{clusterSetLabel: defaultClusterSetName}). build(), }, expectedResponse: newAdmissionResponse(true). @@ -122,20 +128,75 @@ func TestManagedClusterMutate(t *testing.T) { withLeaseDurationSeconds(60). addTaint(newTaint("a", "b", clusterv1.TaintEffectNoSelect, newTime(now, -10*time.Second))). addTaint(newTaint("c", "d", clusterv1.TaintEffectNoSelect, newTime(now, -10*time.Second))). + addLabels(map[string]string{clusterSetLabel: defaultClusterSetName}). build(), Object: newManagedCluster(). withLeaseDurationSeconds(60). addTaint(newTaint("a", "b", clusterv1.TaintEffectNoSelect, newTime(now, -10*time.Second))). + addLabels(map[string]string{clusterSetLabel: defaultClusterSetName}). build(), }, expectedResponse: newAdmissionResponse(true).build(), }, + { + name: "no label in cluster", + request: &admissionv1beta1.AdmissionRequest{ + Resource: managedclustersSchema, + Operation: admissionv1beta1.Create, + Object: newManagedCluster(). + withLeaseDurationSeconds(60). + build(), + }, + expectedResponse: newAdmissionResponse(true). + addJsonPatch(newLabelJsonPatch()). + build(), + }, + { + name: "has other clusterset label", + request: &admissionv1beta1.AdmissionRequest{ + Resource: managedclustersSchema, + Operation: admissionv1beta1.Create, + Object: newManagedCluster(). + withLeaseDurationSeconds(60). + addLabels(map[string]string{clusterSetLabel: "c1"}). + build(), + }, + expectedResponse: newAdmissionResponse(true). + build(), + }, + { + name: "has default clusterset label", + request: &admissionv1beta1.AdmissionRequest{ + Resource: managedclustersSchema, + Operation: admissionv1beta1.Create, + Object: newManagedCluster(). + withLeaseDurationSeconds(60). + addLabels(map[string]string{clusterSetLabel: defaultClusterSetName}). + build(), + }, + expectedResponse: newAdmissionResponse(true). + build(), + }, + { + name: "has other label in cluster", + request: &admissionv1beta1.AdmissionRequest{ + Resource: managedclustersSchema, + Operation: admissionv1beta1.Create, + Object: newManagedCluster(). + withLeaseDurationSeconds(60). + addLabels(map[string]string{"k": "v"}). + build(), + }, + expectedResponse: newAdmissionResponse(true). + addJsonPatch(newLabelJsonPatch()). + build(), + }, } nowFunc = func() time.Time { return now } - + features.DefaultHubMutableFeatureGate.Set("DefaultClusterSet=true") for _, c := range cases { t.Run(c.name, func(t *testing.T) { admissionHook := &ManagedClusterMutatingAdmissionHook{} @@ -204,6 +265,11 @@ func (b *managedClusterBuilder) addTaint(taint clusterv1.Taint) *managedClusterB b.cluster.Spec.Taints = append(b.cluster.Spec.Taints, taint) return b } +func (b *managedClusterBuilder) addLabels(labels map[string]string) *managedClusterBuilder { + var modified bool + resourcemerge.MergeMap(&modified, &b.cluster.Labels, labels) + return b +} func (b *managedClusterBuilder) build() runtime.RawExtension { clusterObj, _ := json.Marshal(b.cluster) diff --git a/test/e2e/e2e_suite_test.go b/test/e2e/e2e_suite_test.go index 3b856ba9e..58dccf914 100644 --- a/test/e2e/e2e_suite_test.go +++ b/test/e2e/e2e_suite_test.go @@ -1,6 +1,7 @@ package e2e import ( + "context" "fmt" "os" "testing" @@ -8,10 +9,13 @@ import ( ginkgo "github.com/onsi/ginkgo" gomega "github.com/onsi/gomega" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/dynamic" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" + "k8s.io/client-go/util/retry" apiregistrationclient "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset/typed/apiregistration/v1" logf "sigs.k8s.io/controller-runtime/pkg/log" @@ -21,6 +25,10 @@ import ( clusterclient "open-cluster-management.io/api/client/cluster/clientset/versioned" ) +var hubNamespace = "open-cluster-management-hub" +var mutatingWebhookName = "managedcluster-admission" +var mutatingWebhookContainerName = "managedcluster-admission" + func TestE2E(t *testing.T) { gomega.RegisterFailHandler(ginkgo.Fail) ginkgo.RunSpecs(t, "E2E suite") @@ -82,7 +90,21 @@ var _ = ginkgo.BeforeSuite(func() { } clusterClient, err = clusterclient.NewForConfig(clusterCfg) - + //Enable DefaultClusterSet feature gates in mutatingWebhook + mutatingWebhookDeployment, err := hubClient.AppsV1().Deployments(hubNamespace).Get(context.Background(), mutatingWebhookName, metav1.GetOptions{}) + webhookContainers := mutatingWebhookDeployment.Spec.Template.Spec.Containers + var updatedContainer []v1.Container + for _, webhookContainer := range webhookContainers { + if webhookContainer.Name == mutatingWebhookContainerName { + webhookContainer.Args = append(webhookContainer.Args, "--feature-gates=DefaultClusterSet=true") + } + updatedContainer = append(updatedContainer, webhookContainer) + } + mutatingWebhookDeployment.Spec.Template.Spec.Containers = updatedContainer + err = retry.RetryOnConflict(retry.DefaultRetry, func() error { + _, err := hubClient.AppsV1().Deployments(hubNamespace).Update(context.Background(), mutatingWebhookDeployment, metav1.UpdateOptions{}) + return err + }) return err }() gomega.Expect(err).ToNot(gomega.HaveOccurred()) diff --git a/test/e2e/webhook_test.go b/test/e2e/webhook_test.go index d4fadc049..ec14d2b68 100644 --- a/test/e2e/webhook_test.go +++ b/test/e2e/webhook_test.go @@ -12,6 +12,7 @@ import ( clusterv1client "open-cluster-management.io/api/client/cluster/clientset/versioned" clusterv1 "open-cluster-management.io/api/cluster/v1" clusterv1beta1 "open-cluster-management.io/api/cluster/v1beta1" + "open-cluster-management.io/registration/pkg/features" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" @@ -25,11 +26,12 @@ import ( ) const ( - apiserviceName = "v1.admission.cluster.open-cluster-management.io" - invalidURL = "127.0.0.1:8001" - validURL = "https://127.0.0.1:8443" - saNamespace = "default" - clusterSetLabel = "cluster.open-cluster-management.io/clusterset" + defaultClusterSetName = "default" + apiserviceName = "v1.admission.cluster.open-cluster-management.io" + invalidURL = "127.0.0.1:8001" + validURL = "https://127.0.0.1:8443" + saNamespace = "default" + clusterSetLabel = "cluster.open-cluster-management.io/clusterset" ) var _ = ginkgo.Describe("Admission webhook", func() { @@ -38,6 +40,7 @@ var _ = ginkgo.Describe("Admission webhook", func() { ginkgo.BeforeEach(func() { // make sure the api service v1.admission.cluster.open-cluster-management.io is available gomega.Eventually(func() bool { + features.DefaultHubMutableFeatureGate.Set("DefaultClusterSet=true") apiService, err := hubAPIServiceClient.APIServices().Get(context.TODO(), apiserviceName, metav1.GetOptions{}) if err != nil { return false @@ -81,7 +84,20 @@ var _ = ginkgo.Describe("Admission webhook", func() { gomega.Expect(deleteManageClusterAndRelatedNamespace(clusterName)).ToNot(gomega.HaveOccurred()) }) + ginkgo.It("Should have the default Clusterset Label", func() { + clusterName := fmt.Sprintf("webhook-spoke-%s", rand.String(6)) + ginkgo.By(fmt.Sprintf("create a managed cluster %q", clusterName)) + _, err := clusterClient.ClusterV1().ManagedClusters().Create(context.TODO(), newManagedCluster(clusterName, false, validURL), metav1.CreateOptions{}) + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + + managedCluster, err := clusterClient.ClusterV1().ManagedClusters().Get(context.TODO(), clusterName, metav1.GetOptions{}) + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + + gomega.Expect(managedCluster.Labels[clusterSetLabel]).To(gomega.Equal(string(defaultClusterSetName))) + + gomega.Expect(deleteManageClusterAndRelatedNamespace(clusterName)).ToNot(gomega.HaveOccurred()) + }) ginkgo.It("Should have the timeAdded for taints", func() { clusterName := fmt.Sprintf("webhook-spoke-%s", rand.String(6)) ginkgo.By(fmt.Sprintf("create a managed cluster %q with taint", clusterName)) @@ -202,6 +218,11 @@ var _ = ginkgo.Describe("Admission webhook", func() { Resources: []string{"managedclusters"}, Verbs: []string{"create", "get", "update"}, }, + { + APIGroups: []string{"cluster.open-cluster-management.io"}, + Resources: []string{"managedclustersets/join"}, + Verbs: []string{"create"}, + }, { APIGroups: []string{"register.open-cluster-management.io"}, Resources: []string{"managedclusters/accept"}, @@ -343,6 +364,41 @@ var _ = ginkgo.Describe("Admission webhook", func() { gomega.Expect(managedCluster.Spec.LeaseDurationSeconds).To(gomega.Equal(int32(60))) }) + ginkgo.It("Should not delete the default ClusterSet Label", func() { + ginkgo.By(fmt.Sprintf("try to update managed cluster %q ClusterSet label", clusterName)) + err := retry.RetryOnConflict(retry.DefaultRetry, func() error { + managedCluster, err := clusterClient.ClusterV1().ManagedClusters().Get(context.TODO(), clusterName, metav1.GetOptions{}) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + delete(managedCluster.Labels, clusterSetLabel) + _, err = clusterClient.ClusterV1().ManagedClusters().Update(context.TODO(), managedCluster, metav1.UpdateOptions{}) + return err + }) + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + + managedCluster, err := clusterClient.ClusterV1().ManagedClusters().Get(context.TODO(), clusterName, metav1.GetOptions{}) + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + gomega.Expect(managedCluster.Labels[clusterSetLabel]).To(gomega.Equal(string(defaultClusterSetName))) + }) + + ginkgo.It("Should not update the other ClusterSet Label", func() { + ginkgo.By(fmt.Sprintf("try to update managed cluster %q ClusterSet label", clusterName)) + err := retry.RetryOnConflict(retry.DefaultRetry, func() error { + managedCluster, err := clusterClient.ClusterV1().ManagedClusters().Get(context.TODO(), clusterName, metav1.GetOptions{}) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + managedCluster.Labels[clusterSetLabel] = "s1" + + _, err = clusterClient.ClusterV1().ManagedClusters().Update(context.TODO(), managedCluster, metav1.UpdateOptions{}) + return err + }) + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + + managedCluster, err := clusterClient.ClusterV1().ManagedClusters().Get(context.TODO(), clusterName, metav1.GetOptions{}) + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + gomega.Expect(managedCluster.Labels[clusterSetLabel]).To(gomega.Equal("s1")) + }) + ginkgo.It("Should respond bad request when updating a managed cluster with invalid external server URLs", func() { ginkgo.By(fmt.Sprintf("update managed cluster %q with an invalid external server URL %q", clusterName, invalidURL)) @@ -467,6 +523,12 @@ var _ = ginkgo.Describe("Admission webhook", func() { Resources: []string{"managedclusters"}, Verbs: []string{"create", "get", "update"}, }, + { + APIGroups: []string{"cluster.open-cluster-management.io"}, + Resources: []string{"managedclustersets/join"}, + ResourceNames: []string{"default"}, + Verbs: []string{"create"}, + }, }, nil) gomega.Expect(err).ToNot(gomega.HaveOccurred())