From f39b00d4e735e2dfd1ad34be5f60f7a1e53e8d60 Mon Sep 17 00:00:00 2001 From: Jian Qiu Date: Thu, 16 Mar 2023 09:43:13 +0800 Subject: [PATCH] Validate metadata.name of managed cluster (#304) Signed-off-by: Jian Qiu --- pkg/webhook/cluster/mutating_webhook.go | 254 ------------- pkg/webhook/cluster/mutating_webhook_test.go | 341 ------------------ pkg/webhook/cluster/validating_webhook.go | 291 --------------- .../cluster/validating_webhook_test.go | 326 ----------------- .../clustersetbinding/validating_webhook.go | 130 ------- .../validating_webhook_test.go | 171 --------- pkg/webhook/v1/managedcluster_validating.go | 7 +- .../v1/managedcluster_validating_test.go | 14 + test/e2e/webhook_test.go | 12 + 9 files changed, 32 insertions(+), 1514 deletions(-) delete mode 100644 pkg/webhook/cluster/mutating_webhook.go delete mode 100644 pkg/webhook/cluster/mutating_webhook_test.go delete mode 100644 pkg/webhook/cluster/validating_webhook.go delete mode 100644 pkg/webhook/cluster/validating_webhook_test.go delete mode 100644 pkg/webhook/clustersetbinding/validating_webhook.go delete mode 100644 pkg/webhook/clustersetbinding/validating_webhook_test.go diff --git a/pkg/webhook/cluster/mutating_webhook.go b/pkg/webhook/cluster/mutating_webhook.go deleted file mode 100644 index f227620be..000000000 --- a/pkg/webhook/cluster/mutating_webhook.go +++ /dev/null @@ -1,254 +0,0 @@ -package cluster - -import ( - "encoding/json" - "fmt" - "net/http" - "strings" - "time" - - ocmfeature "open-cluster-management.io/api/feature" - - admissionv1beta1 "k8s.io/api/admission/v1beta1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime/schema" - utilfeature "k8s.io/apiserver/pkg/util/feature" - "k8s.io/client-go/rest" - "k8s.io/klog/v2" - clusterv1 "open-cluster-management.io/api/cluster/v1" - clusterv1beta2 "open-cluster-management.io/api/cluster/v1beta1" - "open-cluster-management.io/registration/pkg/helpers" -) - -var nowFunc = time.Now -var defaultClusterSetName = "default" - -type jsonPatchOperation struct { - Operation string `json:"op"` - Path string `json:"path"` - Value interface{} `json:"value,omitempty"` -} - -// ManagedClusterMutatingAdmissionHook will mutate the creating/updating managedcluster request. -type ManagedClusterMutatingAdmissionHook struct{} - -// MutatingResource is called by generic-admission-server on startup to register the returned REST resource through which the -// webhook is accessed by the kube apiserver. -func (a *ManagedClusterMutatingAdmissionHook) MutatingResource() (schema.GroupVersionResource, string) { - return schema.GroupVersionResource{ - Group: "admission.cluster.open-cluster-management.io", - Version: "v1", - Resource: "managedclustermutators", - }, - "managedclustermutators" -} - -// Admit is called by generic-admission-server when the registered REST resource above is called with an admission request. -func (a *ManagedClusterMutatingAdmissionHook) Admit(req *admissionv1beta1.AdmissionRequest) *admissionv1beta1.AdmissionResponse { - klog.V(4).Infof("mutate %q operation for object %q", req.Operation, req.Object) - - status := &admissionv1beta1.AdmissionResponse{ - Allowed: true, - } - - // only mutate the request for managedcluster - if req.Resource.Group != "cluster.open-cluster-management.io" || - req.Resource.Resource != "managedclusters" { - return status - } - - // only mutate create and update operation - if req.Operation != admissionv1beta1.Create && req.Operation != admissionv1beta1.Update { - return status - } - - managedCluster := &clusterv1.ManagedCluster{} - if err := json.Unmarshal(req.Object.Raw, managedCluster); err != nil { - status.Allowed = false - status.Result = &metav1.Status{ - Status: metav1.StatusFailure, Code: http.StatusBadRequest, Reason: metav1.StatusReasonBadRequest, - Message: err.Error(), - } - return status - } - - var jsonPatches []jsonPatchOperation - - // set timeAdded of taint if it is nil and reset it if it is modified - taintJsonPatches, status := a.processTaints(managedCluster, req.OldObject.Raw) - if !status.Allowed { - return status - } - jsonPatches = append(jsonPatches, taintJsonPatches...) - - if utilfeature.DefaultMutableFeatureGate.Enabled(ocmfeature.DefaultClusterSet) { - labelJsonPatches, status := a.addDefaultClusterSetLabel(managedCluster, req.Object.Raw) - if !status.Allowed { - return status - } - jsonPatches = append(jsonPatches, labelJsonPatches...) - } - - if len(jsonPatches) == 0 { - return status - } - - patch, err := json.Marshal(jsonPatches) - if err != nil { - status.Allowed = false - status.Result = &metav1.Status{ - Status: metav1.StatusFailure, Code: http.StatusInternalServerError, Reason: metav1.StatusReasonInternalError, - Message: err.Error(), - } - return status - } - - status.Patch = patch - pt := admissionv1beta1.PatchTypeJSONPatch - status.PatchType = &pt - 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) { - var jsonPatches []jsonPatchOperation - - status := &admissionv1beta1.AdmissionResponse{ - Allowed: true, - } - - if len(managedCluster.Labels) == 0 { - jsonPatches = []jsonPatchOperation{ - { - Operation: "add", - Path: "/metadata/labels", - Value: map[string]string{ - clusterv1beta2.ClusterSetLabel: defaultClusterSetName, - }, - }, - } - return jsonPatches, status - } - - clusterSetName, ok := managedCluster.Labels[clusterv1beta2.ClusterSetLabel] - // Clusterset label do not exist - if !ok { - jsonPatches = []jsonPatchOperation{ - { - Operation: "add", - // there is a "/" in clusterset label. So need to transfer the "/" to "~1". - Path: "/metadata/labels/cluster.open-cluster-management.io~1clusterset", - Value: defaultClusterSetName, - }, - } - return jsonPatches, status - } - - // The clusterset label's value is "", update it to "default" - if len(clusterSetName) == 0 { - jsonPatches = []jsonPatchOperation{ - { - Operation: "replace", - // there is a "/" in clusterset label. So need to transfer the "/" to "~1". - Path: "/metadata/labels/cluster.open-cluster-management.io~1clusterset", - Value: defaultClusterSetName, - }, - } - return jsonPatches, status - } - - return nil, status -} - -// processTaints generates json patched for cluster taints -func (a *ManagedClusterMutatingAdmissionHook) processTaints(managedCluster *clusterv1.ManagedCluster, oldManagedClusterRaw []byte) ([]jsonPatchOperation, *admissionv1beta1.AdmissionResponse) { - status := &admissionv1beta1.AdmissionResponse{ - Allowed: true, - } - - if len(managedCluster.Spec.Taints) == 0 { - return nil, status - } - - var oldManagedCluster *clusterv1.ManagedCluster - if len(oldManagedClusterRaw) > 0 { - cluster := &clusterv1.ManagedCluster{} - if err := json.Unmarshal(oldManagedClusterRaw, cluster); err != nil { - status.Allowed = false - status.Result = &metav1.Status{ - Status: metav1.StatusFailure, Code: http.StatusInternalServerError, Reason: metav1.StatusReasonInternalError, - Message: err.Error(), - } - return nil, status - } - oldManagedCluster = cluster - } - - var invalidTaints []string - var jsonPatches []jsonPatchOperation - now := metav1.NewTime(nowFunc()) - for index, taint := range managedCluster.Spec.Taints { - originalTaint := helpers.FindTaintByKey(oldManagedCluster, taint.Key) - switch { - case oldManagedCluster == nil: - // handle CREATE operation. - // The request will not be denied if it has taints with timeAdded specified, - // while the specified values will be ignored. - if !taint.TimeAdded.IsZero() { - status.Warnings = append(status.Warnings, - fmt.Sprintf("The specified TimeAdded value of Taint %q is ignored: %s.", taint.Key, taint.TimeAdded.UTC().Format(time.RFC3339))) - } - jsonPatches = append(jsonPatches, newTaintTimeAddedJsonPatch(index, now.Time)) - case originalTaint == nil: - // handle UPDATE operation. - // new taint - // The request will be denied if it has any taint with timeAdded specified. - if !taint.TimeAdded.IsZero() { - invalidTaints = append(invalidTaints, taint.Key) - continue - } - jsonPatches = append(jsonPatches, newTaintTimeAddedJsonPatch(index, now.Time)) - case originalTaint.Value == taint.Value && originalTaint.Effect == taint.Effect: - // handle UPDATE operation. - // no change - // The request will be denied if it has any taint with different timeAdded specified. - if !originalTaint.TimeAdded.Equal(&taint.TimeAdded) { - invalidTaints = append(invalidTaints, taint.Key) - } - default: - // handle UPDATE operation. - // taint's value/effect has changed - // The request will be denied if it has any taint with timeAdded specified. - if !taint.TimeAdded.IsZero() { - invalidTaints = append(invalidTaints, taint.Key) - continue - } - jsonPatches = append(jsonPatches, newTaintTimeAddedJsonPatch(index, now.Time)) - } - } - - if len(invalidTaints) == 0 { - return jsonPatches, status - } - - status.Allowed = false - status.Result = &metav1.Status{ - Status: metav1.StatusFailure, Code: http.StatusBadRequest, Reason: metav1.StatusReasonBadRequest, - Message: fmt.Sprintf("It is not allowed to set TimeAdded of Taint %q.", strings.Join(invalidTaints, ",")), - } - return nil, status -} - -// Initialize is called by generic-admission-server on startup to setup initialization that managedclusters webhook needs. -func (a *ManagedClusterMutatingAdmissionHook) Initialize(kubeClientConfig *rest.Config, stopCh <-chan struct{}) error { - // do nothing - return nil -} - -func newTaintTimeAddedJsonPatch(index int, timeAdded time.Time) jsonPatchOperation { - return jsonPatchOperation{ - Operation: "replace", - Path: fmt.Sprintf("/spec/taints/%d/timeAdded", index), - Value: timeAdded.UTC().Format(time.RFC3339), - } -} diff --git a/pkg/webhook/cluster/mutating_webhook_test.go b/pkg/webhook/cluster/mutating_webhook_test.go deleted file mode 100644 index 23bd4899f..000000000 --- a/pkg/webhook/cluster/mutating_webhook_test.go +++ /dev/null @@ -1,341 +0,0 @@ -package cluster - -import ( - "encoding/json" - "fmt" - "net/http" - "reflect" - "testing" - "time" - - ocmfeature "open-cluster-management.io/api/feature" - - "github.com/openshift/library-go/pkg/operator/resource/resourcemerge" - clusterv1 "open-cluster-management.io/api/cluster/v1" - clusterv1beta2 "open-cluster-management.io/api/cluster/v1beta2" - - admissionv1beta1 "k8s.io/api/admission/v1beta1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - utilruntime "k8s.io/apimachinery/pkg/util/runtime" - utilfeature "k8s.io/apiserver/pkg/util/feature" - testinghelpers "open-cluster-management.io/registration/pkg/helpers/testing" -) - -func TestManagedClusterMutate(t *testing.T) { - now := time.Now() - cases := []struct { - name string - request *admissionv1beta1.AdmissionRequest - expectedResponse *admissionv1beta1.AdmissionResponse - allowUpdateAcceptField bool - }{ - { - name: "mutate non-managedclusters request", - request: &admissionv1beta1.AdmissionRequest{ - Resource: metav1.GroupVersionResource{ - Group: "test.open-cluster-management.io", - Version: "v1", - Resource: "tests", - }, - }, - expectedResponse: newAdmissionResponse(true).build(), - }, - { - name: "mutate deleting operation", - request: &admissionv1beta1.AdmissionRequest{ - Resource: managedclustersSchema, - Operation: admissionv1beta1.Delete, - }, - expectedResponse: newAdmissionResponse(true).build(), - }, - { - name: "new taints", - request: &admissionv1beta1.AdmissionRequest{ - Resource: managedclustersSchema, - Operation: admissionv1beta1.Create, - Object: newManagedCluster(). - withLeaseDurationSeconds(60). - addTaint(newTaint("a", "b", clusterv1.TaintEffectNoSelect, nil)). - addTaint(newTaint("c", "d", clusterv1.TaintEffectPreferNoSelect, nil)). - addLabels(map[string]string{clusterv1beta2.ClusterSetLabel: defaultClusterSetName}). - build(), - }, - expectedResponse: newAdmissionResponse(true). - addJsonPatch(newTaintTimeAddedJsonPatch(0, now)). - addJsonPatch(newTaintTimeAddedJsonPatch(1, now)). - build(), - }, - { - name: "new taint with timeAdded specified", - request: &admissionv1beta1.AdmissionRequest{ - Resource: managedclustersSchema, - Operation: admissionv1beta1.Create, - Object: newManagedCluster(). - withLeaseDurationSeconds(60). - addTaint(newTaint("a", "b", clusterv1.TaintEffectNoSelect, nil)). - addTaint(newTaint("c", "d", clusterv1.TaintEffectPreferNoSelect, newTime(now, 0))). - addLabels(map[string]string{clusterv1beta2.ClusterSetLabel: defaultClusterSetName}). - build(), - }, - expectedResponse: newAdmissionResponse(true). - addJsonPatch(newTaintTimeAddedJsonPatch(0, now)). - addJsonPatch(newTaintTimeAddedJsonPatch(1, now)). - addWarning(fmt.Sprintf("The specified TimeAdded value of Taint %q is ignored: %s.", "c", newTime(now, 0).UTC().Format(time.RFC3339))). - build(), - }, - { - name: "update taint", - request: &admissionv1beta1.AdmissionRequest{ - Resource: managedclustersSchema, - Operation: admissionv1beta1.Create, - OldObject: newManagedCluster(). - 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{clusterv1beta2.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{clusterv1beta2.ClusterSetLabel: defaultClusterSetName}). - build(), - }, - expectedResponse: newAdmissionResponse(true). - addJsonPatch(newTaintTimeAddedJsonPatch(1, now)). - build(), - }, - { - name: "taint update request denied", - request: &admissionv1beta1.AdmissionRequest{ - Resource: managedclustersSchema, - Operation: admissionv1beta1.Create, - OldObject: newManagedCluster(). - withLeaseDurationSeconds(60). - addTaint(newTaint("a", "b", clusterv1.TaintEffectNoSelect, newTime(now, -10*time.Second))). - addTaint(newTaint("c", "d", clusterv1.TaintEffectNoSelect, newTime(now, -10*time.Second))). - build(), - Object: newManagedCluster(). - withLeaseDurationSeconds(60). - addTaint(newTaint("a", "b", clusterv1.TaintEffectNoSelect, newTime(now, -20*time.Second))). // timeAdded modified - addTaint(newTaint("c", "d", clusterv1.TaintEffectNoSelectIfNew, newTime(now, -10*time.Second))). // effect modified with timeAdded - build(), - }, - expectedResponse: newAdmissionResponse(false). - withResult(metav1.StatusFailure, http.StatusBadRequest, metav1.StatusReasonBadRequest, "It is not allowed to set TimeAdded of Taint \"a,c\"."). - build(), - }, - { - name: "delete taint", - request: &admissionv1beta1.AdmissionRequest{ - Resource: managedclustersSchema, - Operation: admissionv1beta1.Create, - OldObject: newManagedCluster(). - 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{clusterv1beta2.ClusterSetLabel: defaultClusterSetName}). - build(), - Object: newManagedCluster(). - withLeaseDurationSeconds(60). - addTaint(newTaint("a", "b", clusterv1.TaintEffectNoSelect, newTime(now, -10*time.Second))). - addLabels(map[string]string{clusterv1beta2.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(jsonPatchOperation{ - Operation: "add", - Path: "/metadata/labels", - Value: map[string]string{ - clusterv1beta2.ClusterSetLabel: defaultClusterSetName, - }, - }). - build(), - }, - { - name: "has other clusterset label", - request: &admissionv1beta1.AdmissionRequest{ - Resource: managedclustersSchema, - Operation: admissionv1beta1.Create, - Object: newManagedCluster(). - withLeaseDurationSeconds(60). - addLabels(map[string]string{clusterv1beta2.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{clusterv1beta2.ClusterSetLabel: defaultClusterSetName}). - build(), - }, - expectedResponse: newAdmissionResponse(true). - build(), - }, - { - name: "has null clusterset label", - request: &admissionv1beta1.AdmissionRequest{ - Resource: managedclustersSchema, - Operation: admissionv1beta1.Create, - Object: newManagedCluster(). - withLeaseDurationSeconds(60). - addLabels(map[string]string{clusterv1beta2.ClusterSetLabel: ""}). - build(), - }, - expectedResponse: newAdmissionResponse(true). - addJsonPatch(jsonPatchOperation{ - Operation: "replace", - Path: "/metadata/labels/cluster.open-cluster-management.io~1clusterset", - Value: defaultClusterSetName, - }). - 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(jsonPatchOperation{ - Operation: "add", - Path: "/metadata/labels/cluster.open-cluster-management.io~1clusterset", - Value: defaultClusterSetName, - }). - build(), - }, - } - - nowFunc = func() time.Time { - return now - } - utilruntime.Must(utilfeature.DefaultMutableFeatureGate.Add(ocmfeature.DefaultHubRegistrationFeatureGates)) - if err := utilfeature.DefaultMutableFeatureGate.Set(fmt.Sprintf("%s=true", string(ocmfeature.DefaultClusterSet))); err != nil { - t.Fatal(err) - } - for _, c := range cases { - t.Run(c.name, func(t *testing.T) { - admissionHook := &ManagedClusterMutatingAdmissionHook{} - actualResponse := admissionHook.Admit(c.request) - if !reflect.DeepEqual(actualResponse, c.expectedResponse) { - t.Errorf("expected \n%#v but got: \n%#v", c.expectedResponse, actualResponse) - } - }) - } -} - -type admissionResponseBuilder struct { - jsonPatchOperations []jsonPatchOperation - response admissionv1beta1.AdmissionResponse -} - -func newAdmissionResponse(allowed bool) *admissionResponseBuilder { - return &admissionResponseBuilder{ - response: admissionv1beta1.AdmissionResponse{ - Allowed: allowed, - }, - } -} - -func (b *admissionResponseBuilder) addJsonPatch(jsonPatch jsonPatchOperation) *admissionResponseBuilder { - b.jsonPatchOperations = append(b.jsonPatchOperations, jsonPatch) - pt := admissionv1beta1.PatchTypeJSONPatch - b.response.PatchType = &pt - return b -} - -func (b *admissionResponseBuilder) withResult(status string, code int32, reason metav1.StatusReason, message string) *admissionResponseBuilder { - b.response.Result = &metav1.Status{ - Status: status, - Code: code, - Reason: reason, - Message: message, - } - return b -} - -func (b *admissionResponseBuilder) addWarning(warning string) *admissionResponseBuilder { - b.response.Warnings = append(b.response.Warnings, warning) - return b -} - -func (b *admissionResponseBuilder) build() *admissionv1beta1.AdmissionResponse { - if len(b.jsonPatchOperations) > 0 { - patch, _ := json.Marshal(b.jsonPatchOperations) - b.response.Patch = patch - } - return &b.response -} - -type managedClusterBuilder struct { - cluster clusterv1.ManagedCluster -} - -func newManagedCluster() *managedClusterBuilder { - return &managedClusterBuilder{ - cluster: *testinghelpers.NewManagedCluster(), - } -} - -func (b *managedClusterBuilder) withLeaseDurationSeconds(leaseDurationSeconds int32) *managedClusterBuilder { - b.cluster.Spec.LeaseDurationSeconds = leaseDurationSeconds - return b -} - -func (b *managedClusterBuilder) addTaint(taint clusterv1.Taint) *managedClusterBuilder { - 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) - return runtime.RawExtension{ - Raw: clusterObj, - } -} - -func newTaint(key, value string, effect clusterv1.TaintEffect, timeAdded *metav1.Time) clusterv1.Taint { - taint := clusterv1.Taint{ - Key: key, - Value: value, - Effect: effect, - } - - if timeAdded != nil { - taint.TimeAdded = *timeAdded - } - - return taint -} - -func newTime(time time.Time, offset time.Duration) *metav1.Time { - mt := metav1.NewTime(time.Add(offset)) - return &mt -} diff --git a/pkg/webhook/cluster/validating_webhook.go b/pkg/webhook/cluster/validating_webhook.go deleted file mode 100644 index 453518c88..000000000 --- a/pkg/webhook/cluster/validating_webhook.go +++ /dev/null @@ -1,291 +0,0 @@ -package cluster - -import ( - "context" - "encoding/json" - "fmt" - "net/http" - - clusterv1 "open-cluster-management.io/api/cluster/v1" - "open-cluster-management.io/registration/pkg/helpers" - - operatorhelpers "github.com/openshift/library-go/pkg/operator/v1helpers" - - admissionv1beta1 "k8s.io/api/admission/v1beta1" - authenticationv1 "k8s.io/api/authentication/v1" - authorizationv1 "k8s.io/api/authorization/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/client-go/kubernetes" - "k8s.io/client-go/rest" - "k8s.io/klog/v2" - clusterv1beta2 "open-cluster-management.io/api/cluster/v1beta1" -) - -// ManagedClusterValidatingAdmissionHook will validate the creating/updating managedcluster request. -type ManagedClusterValidatingAdmissionHook struct { - kubeClient kubernetes.Interface -} - -// ValidatingResource is called by generic-admission-server on startup to register the returned REST resource through which the -// webhook is accessed by the kube apiserver. -func (a *ManagedClusterValidatingAdmissionHook) ValidatingResource() (plural schema.GroupVersionResource, singular string) { - return schema.GroupVersionResource{ - Group: "admission.cluster.open-cluster-management.io", - Version: "v1", - Resource: "managedclustervalidators", - }, - "managedclustervalidators" -} - -// Validate is called by generic-admission-server when the registered REST resource above is called with an admission request. -func (a *ManagedClusterValidatingAdmissionHook) Validate(admissionSpec *admissionv1beta1.AdmissionRequest) *admissionv1beta1.AdmissionResponse { - klog.V(4).Infof("validate %q operation for object %q", admissionSpec.Operation, admissionSpec.Object) - - status := &admissionv1beta1.AdmissionResponse{} - - // only validate the request for managedcluster - if admissionSpec.Resource.Group != "cluster.open-cluster-management.io" || - admissionSpec.Resource.Resource != "managedclusters" { - status.Allowed = true - return status - } - - switch admissionSpec.Operation { - case admissionv1beta1.Create: - return a.validateCreateRequest(admissionSpec) - case admissionv1beta1.Update: - return a.validateUpdateRequest(admissionSpec) - default: - status.Allowed = true - return status - } -} - -// Initialize is called by generic-admission-server on startup to setup initialization that managedclusters webhook needs. -func (a *ManagedClusterValidatingAdmissionHook) Initialize(kubeClientConfig *rest.Config, stopCh <-chan struct{}) error { - var err error - a.kubeClient, err = kubernetes.NewForConfig(kubeClientConfig) - return err -} - -// validateCreateRequest validates create managed cluster operation -func (a *ManagedClusterValidatingAdmissionHook) validateCreateRequest(request *admissionv1beta1.AdmissionRequest) *admissionv1beta1.AdmissionResponse { - status := &admissionv1beta1.AdmissionResponse{} - - // validate ManagedCluster object firstly - managedCluster, err := a.validateManagedClusterObj(request.Object) - if err != nil { - status.Allowed = false - status.Result = &metav1.Status{ - Status: metav1.StatusFailure, Code: http.StatusBadRequest, Reason: metav1.StatusReasonBadRequest, - Message: err.Error(), - } - return status - } - - if managedCluster.Spec.HubAcceptsClient { - // the HubAcceptsClient field is changed, we need to check the request user whether - // has been allowed to change the HubAcceptsClient field with SubjectAccessReview api - if status := a.allowUpdateAcceptField(managedCluster.Name, request.UserInfo); !status.Allowed { - return status - } - } - - // check whether the request user has been allowed to set clusterset label - var clusterSetName string - if len(managedCluster.Labels) > 0 { - clusterSetName = managedCluster.Labels[clusterv1beta2.ClusterSetLabel] - } - - return a.allowSetClusterSetLabel(request.UserInfo, "", clusterSetName) -} - -// validateUpdateRequest validates update managed cluster operation. -func (a *ManagedClusterValidatingAdmissionHook) validateUpdateRequest(request *admissionv1beta1.AdmissionRequest) *admissionv1beta1.AdmissionResponse { - status := &admissionv1beta1.AdmissionResponse{} - - oldManagedCluster := &clusterv1.ManagedCluster{} - if err := json.Unmarshal(request.OldObject.Raw, oldManagedCluster); err != nil { - status.Allowed = false - status.Result = &metav1.Status{ - Status: metav1.StatusFailure, Code: http.StatusBadRequest, Reason: metav1.StatusReasonBadRequest, - Message: err.Error(), - } - return status - } - - // validate the updating ManagedCluster object firstly - newManagedCluster, err := a.validateManagedClusterObj(request.Object) - if err != nil { - status.Allowed = false - status.Result = &metav1.Status{ - Status: metav1.StatusFailure, Code: http.StatusBadRequest, Reason: metav1.StatusReasonBadRequest, - Message: err.Error(), - } - return status - } - - if newManagedCluster.Spec.HubAcceptsClient != oldManagedCluster.Spec.HubAcceptsClient { - // the HubAcceptsClient field is changed, we need to check the request user whether - // has been allowed to update the HubAcceptsClient field with SubjectAccessReview api - if status := a.allowUpdateAcceptField(newManagedCluster.Name, request.UserInfo); !status.Allowed { - return status - } - } - - // check whether the request user has been allowed to set clusterset label - var originalClusterSetName, currentClusterSetName string - if len(oldManagedCluster.Labels) > 0 { - originalClusterSetName = oldManagedCluster.Labels[clusterv1beta2.ClusterSetLabel] - } - if len(newManagedCluster.Labels) > 0 { - currentClusterSetName = newManagedCluster.Labels[clusterv1beta2.ClusterSetLabel] - } - - return a.allowSetClusterSetLabel(request.UserInfo, originalClusterSetName, currentClusterSetName) -} - -// validateManagedClusterObj validates the fileds of ManagedCluster object -func (a *ManagedClusterValidatingAdmissionHook) validateManagedClusterObj(requestObj runtime.RawExtension) (*clusterv1.ManagedCluster, error) { - errs := []error{} - - managedCluster := &clusterv1.ManagedCluster{} - if err := json.Unmarshal(requestObj.Raw, managedCluster); err != nil { - errs = append(errs, err) - } - - // there are no spoke client configs, finish the validation process - if len(managedCluster.Spec.ManagedClusterClientConfigs) == 0 { - return managedCluster, operatorhelpers.NewMultiLineAggregate(errs) - } - - // validate the url in spoke client configs - for _, clientConfig := range managedCluster.Spec.ManagedClusterClientConfigs { - if !helpers.IsValidHTTPSURL(clientConfig.URL) { - errs = append(errs, fmt.Errorf("url %q is invalid in client configs", clientConfig.URL)) - } - } - - return managedCluster, operatorhelpers.NewMultiLineAggregate(errs) -} - -// allowUpdateHubAcceptsClientField using SubjectAccessReview API to check whether a request user has been authorized to update -// HubAcceptsClient field -func (a *ManagedClusterValidatingAdmissionHook) allowUpdateAcceptField(clusterName string, userInfo authenticationv1.UserInfo) *admissionv1beta1.AdmissionResponse { - status := &admissionv1beta1.AdmissionResponse{} - - extra := make(map[string]authorizationv1.ExtraValue) - for k, v := range userInfo.Extra { - extra[k] = authorizationv1.ExtraValue(v) - } - - sar := &authorizationv1.SubjectAccessReview{ - Spec: authorizationv1.SubjectAccessReviewSpec{ - User: userInfo.Username, - UID: userInfo.UID, - Groups: userInfo.Groups, - Extra: extra, - ResourceAttributes: &authorizationv1.ResourceAttributes{ - Group: "register.open-cluster-management.io", - Resource: "managedclusters", - Verb: "update", - Subresource: "accept", - Name: clusterName, - }, - }, - } - sar, err := a.kubeClient.AuthorizationV1().SubjectAccessReviews().Create(context.TODO(), sar, metav1.CreateOptions{}) - if err != nil { - status.Allowed = false - status.Result = &metav1.Status{ - Status: metav1.StatusFailure, Code: http.StatusForbidden, Reason: metav1.StatusReasonForbidden, - Message: err.Error(), - } - return status - } - - if !sar.Status.Allowed { - status.Allowed = false - status.Result = &metav1.Status{ - Status: metav1.StatusFailure, Code: http.StatusForbidden, Reason: metav1.StatusReasonForbidden, - Message: fmt.Sprintf("user %q cannot update the HubAcceptsClient field", userInfo.Username), - } - return status - } - - status.Allowed = true - return status -} - -// allowSetClusterSetLabel checks whether a request user has been authorized to set clusterset label -func (a *ManagedClusterValidatingAdmissionHook) allowSetClusterSetLabel(userInfo authenticationv1.UserInfo, originalClusterSet, newClusterSet string) *admissionv1beta1.AdmissionResponse { - if originalClusterSet == newClusterSet { - return &admissionv1beta1.AdmissionResponse{Allowed: true} - } - - if len(originalClusterSet) > 0 { - if status := a.allowUpdateClusterSet(userInfo, originalClusterSet); !status.Allowed { - return status - } - } - - if len(newClusterSet) > 0 { - if status := a.allowUpdateClusterSet(userInfo, newClusterSet); !status.Allowed { - return status - } - } - - return &admissionv1beta1.AdmissionResponse{ - Allowed: true, - } -} - -// allowUpdateClusterSet checks whether a request user has been authorized to add/remove a ManagedCluster -// to/from the ManagedClusterSet -func (a *ManagedClusterValidatingAdmissionHook) allowUpdateClusterSet(userInfo authenticationv1.UserInfo, clusterSetName string) *admissionv1beta1.AdmissionResponse { - status := &admissionv1beta1.AdmissionResponse{} - - extra := make(map[string]authorizationv1.ExtraValue) - for k, v := range userInfo.Extra { - extra[k] = authorizationv1.ExtraValue(v) - } - - sar := &authorizationv1.SubjectAccessReview{ - Spec: authorizationv1.SubjectAccessReviewSpec{ - User: userInfo.Username, - UID: userInfo.UID, - Groups: userInfo.Groups, - Extra: extra, - ResourceAttributes: &authorizationv1.ResourceAttributes{ - Group: "cluster.open-cluster-management.io", - Resource: "managedclustersets", - Subresource: "join", - Name: clusterSetName, - Verb: "create", - }, - }, - } - sar, err := a.kubeClient.AuthorizationV1().SubjectAccessReviews().Create(context.TODO(), sar, metav1.CreateOptions{}) - if err != nil { - status.Allowed = false - status.Result = &metav1.Status{ - Status: metav1.StatusFailure, Code: http.StatusForbidden, Reason: metav1.StatusReasonForbidden, - Message: err.Error(), - } - return status - } - - if !sar.Status.Allowed { - status.Allowed = false - status.Result = &metav1.Status{ - Status: metav1.StatusFailure, Code: http.StatusForbidden, Reason: metav1.StatusReasonForbidden, - Message: fmt.Sprintf("user %q cannot add/remove a ManagedCluster to/from ManagedClusterSet %q", userInfo.Username, clusterSetName), - } - return status - } - - status.Allowed = true - return status -} diff --git a/pkg/webhook/cluster/validating_webhook_test.go b/pkg/webhook/cluster/validating_webhook_test.go deleted file mode 100644 index c0e9829e7..000000000 --- a/pkg/webhook/cluster/validating_webhook_test.go +++ /dev/null @@ -1,326 +0,0 @@ -package cluster - -import ( - "encoding/json" - "net/http" - "reflect" - "testing" - - clusterv1 "open-cluster-management.io/api/cluster/v1" - clusterv1beta2 "open-cluster-management.io/api/cluster/v1beta2" - testinghelpers "open-cluster-management.io/registration/pkg/helpers/testing" - - admissionv1beta1 "k8s.io/api/admission/v1beta1" - authenticationv1 "k8s.io/api/authentication/v1" - authorizationv1 "k8s.io/api/authorization/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - kubefake "k8s.io/client-go/kubernetes/fake" - clienttesting "k8s.io/client-go/testing" -) - -var managedclustersSchema = metav1.GroupVersionResource{ - Group: "cluster.open-cluster-management.io", - Version: "v1", - Resource: "managedclusters", -} - -func TestManagedClusterValidate(t *testing.T) { - cases := []struct { - name string - request *admissionv1beta1.AdmissionRequest - expectedResponse *admissionv1beta1.AdmissionResponse - allowUpdateAcceptField bool - allowUpdateClusterSets map[string]bool - }{ - { - name: "validate non-managedclusters request", - request: &admissionv1beta1.AdmissionRequest{ - Resource: metav1.GroupVersionResource{ - Group: "test.open-cluster-management.io", - Version: "v1", - Resource: "tests", - }, - }, - expectedResponse: &admissionv1beta1.AdmissionResponse{ - Allowed: true, - }, - }, - { - name: "validate deleting operation", - request: &admissionv1beta1.AdmissionRequest{ - Resource: managedclustersSchema, - Operation: admissionv1beta1.Delete, - }, - expectedResponse: &admissionv1beta1.AdmissionResponse{ - Allowed: true, - }, - }, - { - name: "validate creating ManagedCluster", - request: &admissionv1beta1.AdmissionRequest{ - Resource: managedclustersSchema, - Operation: admissionv1beta1.Create, - Object: newManagedClusterObj(), - }, - expectedResponse: &admissionv1beta1.AdmissionResponse{ - Allowed: true, - }, - }, - { - name: "validate creating ManagedCluster with invalid fields", - request: &admissionv1beta1.AdmissionRequest{ - Resource: managedclustersSchema, - Operation: admissionv1beta1.Create, - Object: newManagedClusterObjWithClientConfigs(clusterv1.ClientConfig{URL: "http://127.0.0.1:8001"}), - }, - expectedResponse: &admissionv1beta1.AdmissionResponse{ - Allowed: false, - Result: &metav1.Status{ - Status: metav1.StatusFailure, Code: http.StatusBadRequest, Reason: metav1.StatusReasonBadRequest, - Message: "url \"http://127.0.0.1:8001\" is invalid in client configs", - }, - }, - }, - { - name: "validate creating an accepted ManagedCluster without update acceptance permission", - request: &admissionv1beta1.AdmissionRequest{ - Resource: managedclustersSchema, - Operation: admissionv1beta1.Create, - Object: newManagedClusterObjWithHubAcceptsClient(true), - UserInfo: authenticationv1.UserInfo{Username: "tester"}, - }, - expectedResponse: &admissionv1beta1.AdmissionResponse{ - Allowed: false, - Result: &metav1.Status{ - Status: metav1.StatusFailure, Code: http.StatusForbidden, Reason: metav1.StatusReasonForbidden, - Message: "user \"tester\" cannot update the HubAcceptsClient field", - }, - }, - }, - { - name: "validate creating an accepted ManagedCluster", - request: &admissionv1beta1.AdmissionRequest{ - Resource: managedclustersSchema, - Operation: admissionv1beta1.Create, - Object: newManagedClusterObjWithHubAcceptsClient(true), - }, - expectedResponse: &admissionv1beta1.AdmissionResponse{ - Allowed: true, - }, - allowUpdateAcceptField: true, - }, - { - name: "validate update ManagedCluster without HubAcceptsClient field changed", - request: &admissionv1beta1.AdmissionRequest{ - Resource: managedclustersSchema, - Operation: admissionv1beta1.Update, - OldObject: newManagedClusterObjWithClientConfigs(clusterv1.ClientConfig{URL: "https://127.0.0.1:6443"}), - Object: newManagedClusterObjWithClientConfigs(clusterv1.ClientConfig{URL: "https://127.0.0.1:8443"}), - }, - expectedResponse: &admissionv1beta1.AdmissionResponse{ - Allowed: true, - }, - }, - { - name: "validate updating HubAcceptsClient field without update acceptance permission", - request: &admissionv1beta1.AdmissionRequest{ - Resource: managedclustersSchema, - Operation: admissionv1beta1.Update, - OldObject: newManagedClusterObjWithHubAcceptsClient(false), - Object: newManagedClusterObjWithHubAcceptsClient(true), - UserInfo: authenticationv1.UserInfo{Username: "tester"}, - }, - expectedResponse: &admissionv1beta1.AdmissionResponse{ - Allowed: false, - Result: &metav1.Status{ - Status: metav1.StatusFailure, Code: http.StatusForbidden, Reason: metav1.StatusReasonForbidden, - Message: "user \"tester\" cannot update the HubAcceptsClient field", - }, - }, - }, - { - name: "validate updating HubAcceptsClient field", - request: &admissionv1beta1.AdmissionRequest{ - Resource: managedclustersSchema, - Operation: admissionv1beta1.Update, - OldObject: newManagedClusterObjWithHubAcceptsClient(false), - Object: newManagedClusterObjWithHubAcceptsClient(true), - }, - expectedResponse: &admissionv1beta1.AdmissionResponse{ - Allowed: true, - }, - allowUpdateAcceptField: true, - }, - { - name: "validate setting clusterset label", - request: &admissionv1beta1.AdmissionRequest{ - Resource: managedclustersSchema, - Operation: admissionv1beta1.Create, - Object: newManagedClusterObjWithClientSet("clusterset1"), - }, - expectedResponse: &admissionv1beta1.AdmissionResponse{ - Allowed: true, - }, - allowUpdateClusterSets: map[string]bool{ - "clusterset1": true, - }, - }, - { - name: "validate setting clusterset label without permission", - request: &admissionv1beta1.AdmissionRequest{ - Resource: managedclustersSchema, - Operation: admissionv1beta1.Create, - Object: newManagedClusterObjWithClientSet("clusterset1"), - }, - expectedResponse: &admissionv1beta1.AdmissionResponse{ - Allowed: false, - Result: &metav1.Status{ - Status: metav1.StatusFailure, Code: http.StatusForbidden, Reason: metav1.StatusReasonForbidden, - Message: "user \"\" cannot add/remove a ManagedCluster to/from ManagedClusterSet \"clusterset1\"", - }, - }, - allowUpdateClusterSets: map[string]bool{ - "clusterset1": false, - }, - }, - { - name: "validate updating clusterset label", - request: &admissionv1beta1.AdmissionRequest{ - Resource: managedclustersSchema, - Operation: admissionv1beta1.Update, - OldObject: newManagedClusterObjWithClientSet("clusterset1"), - Object: newManagedClusterObjWithClientSet("clusterset2"), - }, - expectedResponse: &admissionv1beta1.AdmissionResponse{ - Allowed: true, - }, - allowUpdateClusterSets: map[string]bool{ - "clusterset1": true, - "clusterset2": true, - }, - }, - { - name: "validate updating clusterset label without permission", - request: &admissionv1beta1.AdmissionRequest{ - Resource: managedclustersSchema, - Operation: admissionv1beta1.Update, - OldObject: newManagedClusterObjWithClientSet("clusterset1"), - Object: newManagedClusterObjWithClientSet("clusterset2"), - }, - expectedResponse: &admissionv1beta1.AdmissionResponse{ - Allowed: false, - Result: &metav1.Status{ - Status: metav1.StatusFailure, Code: http.StatusForbidden, Reason: metav1.StatusReasonForbidden, - Message: "user \"\" cannot add/remove a ManagedCluster to/from ManagedClusterSet \"clusterset1\"", - }, - }, - }, - { - name: "validate updating clusterset label with partial permission", - request: &admissionv1beta1.AdmissionRequest{ - Resource: managedclustersSchema, - Operation: admissionv1beta1.Update, - OldObject: newManagedClusterObjWithClientSet("clusterset1"), - Object: newManagedClusterObjWithClientSet("clusterset2"), - }, - expectedResponse: &admissionv1beta1.AdmissionResponse{ - Allowed: false, - Result: &metav1.Status{ - Status: metav1.StatusFailure, Code: http.StatusForbidden, Reason: metav1.StatusReasonForbidden, - Message: "user \"\" cannot add/remove a ManagedCluster to/from ManagedClusterSet \"clusterset2\"", - }, - }, - allowUpdateClusterSets: map[string]bool{ - "clusterset1": true, - }, - }, - { - name: "validate resetting clusterset label", - request: &admissionv1beta1.AdmissionRequest{ - Resource: managedclustersSchema, - Operation: admissionv1beta1.Update, - OldObject: newManagedClusterObjWithClientSet("clusterset1"), - Object: newManagedClusterObjWithClientSet(""), - }, - expectedResponse: &admissionv1beta1.AdmissionResponse{ - Allowed: true, - }, - allowUpdateClusterSets: map[string]bool{ - "clusterset1": true, - }, - }, - } - - for _, c := range cases { - t.Run(c.name, func(t *testing.T) { - kubeClient := kubefake.NewSimpleClientset() - kubeClient.PrependReactor( - "create", - "subjectaccessreviews", - func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { - allowed := false - - sar := action.(clienttesting.CreateAction).GetObject().(*authorizationv1.SubjectAccessReview) - switch sar.Spec.ResourceAttributes.Resource { - case "managedclusters": - allowed = c.allowUpdateAcceptField - case "managedclustersets": - allowed = c.allowUpdateClusterSets[sar.Spec.ResourceAttributes.Name] - } - - return true, &authorizationv1.SubjectAccessReview{ - Status: authorizationv1.SubjectAccessReviewStatus{ - Allowed: allowed, - }, - }, nil - }, - ) - - admissionHook := &ManagedClusterValidatingAdmissionHook{kubeClient: kubeClient} - - actualResponse := admissionHook.Validate(c.request) - - if !reflect.DeepEqual(actualResponse, c.expectedResponse) { - t.Errorf("expected %#v but got: %#v", c.expectedResponse.Result, actualResponse.Result) - } - }) - } -} - -func newManagedClusterObj() runtime.RawExtension { - managedCluster := testinghelpers.NewManagedCluster() - clusterObj, _ := json.Marshal(managedCluster) - return runtime.RawExtension{ - Raw: clusterObj, - } -} - -func newManagedClusterObjWithHubAcceptsClient(hubAcceptsClient bool) runtime.RawExtension { - managedCluster := testinghelpers.NewManagedCluster() - managedCluster.Spec.HubAcceptsClient = hubAcceptsClient - clusterObj, _ := json.Marshal(managedCluster) - return runtime.RawExtension{ - Raw: clusterObj, - } -} - -func newManagedClusterObjWithClientConfigs(clientConfig clusterv1.ClientConfig) runtime.RawExtension { - managedCluster := testinghelpers.NewManagedCluster() - managedCluster.Spec.ManagedClusterClientConfigs = []clusterv1.ClientConfig{clientConfig} - clusterObj, _ := json.Marshal(managedCluster) - return runtime.RawExtension{ - Raw: clusterObj, - } -} - -func newManagedClusterObjWithClientSet(clusterSetName string) runtime.RawExtension { - managedCluster := testinghelpers.NewManagedCluster() - managedCluster.Labels = map[string]string{ - clusterv1beta2.ClusterSetLabel: clusterSetName, - } - clusterObj, _ := json.Marshal(managedCluster) - return runtime.RawExtension{ - Raw: clusterObj, - } -} diff --git a/pkg/webhook/clustersetbinding/validating_webhook.go b/pkg/webhook/clustersetbinding/validating_webhook.go deleted file mode 100644 index 7082013d9..000000000 --- a/pkg/webhook/clustersetbinding/validating_webhook.go +++ /dev/null @@ -1,130 +0,0 @@ -package clustersetbinding - -import ( - "context" - "encoding/json" - "fmt" - "net/http" - - admissionv1beta1 "k8s.io/api/admission/v1beta1" - authenticationv1 "k8s.io/api/authentication/v1" - authorizationv1 "k8s.io/api/authorization/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/client-go/kubernetes" - "k8s.io/client-go/rest" - "k8s.io/klog/v2" - clusterv1beta2 "open-cluster-management.io/api/cluster/v1beta2" -) - -// ManagedClusterSetBindingValidatingAdmissionHook will validate the creating/updating ManagedClusterSetBinding request. -type ManagedClusterSetBindingValidatingAdmissionHook struct { - kubeClient kubernetes.Interface -} - -// ValidatingResource is called by generic-admission-server on startup to register the returned REST resource through which the -// webhook is accessed by the kube apiserver. -func (a *ManagedClusterSetBindingValidatingAdmissionHook) ValidatingResource() (plural schema.GroupVersionResource, singular string) { - return schema.GroupVersionResource{ - Group: "admission.cluster.open-cluster-management.io", - Version: "v1", - Resource: "managedclustersetbindingvalidators", - }, - "managedclustersetbindingvalidators" -} - -// Validate is called by generic-admission-server when the registered REST resource above is called with an admission request. -func (a *ManagedClusterSetBindingValidatingAdmissionHook) Validate(admissionSpec *admissionv1beta1.AdmissionRequest) *admissionv1beta1.AdmissionResponse { - klog.V(4).Infof("validate %q operation for object %q", admissionSpec.Operation, admissionSpec.Object) - - // only validate the request for ManagedClusterSetBinding - if admissionSpec.Resource.Group != "cluster.open-cluster-management.io" || - admissionSpec.Resource.Resource != "managedclustersetbindings" { - return acceptRequest() - } - - // only handle Create/Update Operation - if admissionSpec.Operation != admissionv1beta1.Create && admissionSpec.Operation != admissionv1beta1.Update { - return acceptRequest() - } - - binding := &clusterv1beta2.ManagedClusterSetBinding{} - if err := json.Unmarshal(admissionSpec.Object.Raw, binding); err != nil { - return denyRequest(http.StatusBadRequest, metav1.StatusReasonBadRequest, - fmt.Sprintf("Unable to unmarshal the ManagedClusterSetBinding object: %v", err)) - } - - // force the instance name to match the target cluster set name - if binding.Name != binding.Spec.ClusterSet { - return denyRequest(http.StatusBadRequest, metav1.StatusReasonBadRequest, - "The ManagedClusterSetBinding must have the same name as the target ManagedClusterSet") - } - - // check if the request user has permission to bind the target cluster set - if admissionSpec.Operation == admissionv1beta1.Create { - return a.allowBindingToClusterSet(binding.Spec.ClusterSet, admissionSpec.UserInfo) - } - - return acceptRequest() -} - -// Initialize is called by generic-admission-server on startup to setup initialization that ManagedClusterSetBinding webhook needs. -func (a *ManagedClusterSetBindingValidatingAdmissionHook) Initialize(kubeClientConfig *rest.Config, stopCh <-chan struct{}) error { - var err error - a.kubeClient, err = kubernetes.NewForConfig(kubeClientConfig) - if err != nil { - return err - } - - return nil -} - -// allowBindingToClusterSet checks if the user has permission to bind a particular cluster set -func (a *ManagedClusterSetBindingValidatingAdmissionHook) allowBindingToClusterSet(clusterSetName string, userInfo authenticationv1.UserInfo) *admissionv1beta1.AdmissionResponse { - extra := make(map[string]authorizationv1.ExtraValue) - for k, v := range userInfo.Extra { - extra[k] = authorizationv1.ExtraValue(v) - } - - sar := &authorizationv1.SubjectAccessReview{ - Spec: authorizationv1.SubjectAccessReviewSpec{ - User: userInfo.Username, - UID: userInfo.UID, - Groups: userInfo.Groups, - Extra: extra, - ResourceAttributes: &authorizationv1.ResourceAttributes{ - Group: "cluster.open-cluster-management.io", - Resource: "managedclustersets", - Subresource: "bind", - Verb: "create", - Name: clusterSetName, - }, - }, - } - sar, err := a.kubeClient.AuthorizationV1().SubjectAccessReviews().Create(context.TODO(), sar, metav1.CreateOptions{}) - if err != nil { - return denyRequest(http.StatusForbidden, metav1.StatusReasonForbidden, err.Error()) - } - if !sar.Status.Allowed { - return denyRequest(http.StatusForbidden, metav1.StatusReasonForbidden, fmt.Sprintf("user %q is not allowed to bind cluster set %q", userInfo.Username, clusterSetName)) - } - return acceptRequest() -} - -func acceptRequest() *admissionv1beta1.AdmissionResponse { - return &admissionv1beta1.AdmissionResponse{ - Allowed: true, - } -} - -func denyRequest(code int32, reason metav1.StatusReason, message string) *admissionv1beta1.AdmissionResponse { - return &admissionv1beta1.AdmissionResponse{ - Allowed: false, - Result: &metav1.Status{ - Status: metav1.StatusFailure, - Code: code, - Reason: reason, - Message: message, - }, - } -} diff --git a/pkg/webhook/clustersetbinding/validating_webhook_test.go b/pkg/webhook/clustersetbinding/validating_webhook_test.go deleted file mode 100644 index b5869b1f8..000000000 --- a/pkg/webhook/clustersetbinding/validating_webhook_test.go +++ /dev/null @@ -1,171 +0,0 @@ -package clustersetbinding - -import ( - "encoding/json" - "net/http" - "reflect" - "testing" - - admissionv1beta1 "k8s.io/api/admission/v1beta1" - authorizationv1 "k8s.io/api/authorization/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - kubefake "k8s.io/client-go/kubernetes/fake" - clienttesting "k8s.io/client-go/testing" - clusterv1beta2 "open-cluster-management.io/api/cluster/v1beta2" -) - -var managedclustersetbindingSchema = metav1.GroupVersionResource{ - Group: "cluster.open-cluster-management.io", - Version: "v1alpha1", - Resource: "managedclustersetbindings", -} - -func TestManagedClusterValidate(t *testing.T) { - cases := []struct { - name string - request *admissionv1beta1.AdmissionRequest - expectedResponse *admissionv1beta1.AdmissionResponse - allowBindingToClusterSet bool - }{ - { - name: "validate non-managedclustersetbindings request", - request: &admissionv1beta1.AdmissionRequest{ - Resource: metav1.GroupVersionResource{ - Group: "test.open-cluster-management.io", - Version: "v1", - Resource: "tests", - }, - }, - expectedResponse: &admissionv1beta1.AdmissionResponse{ - Allowed: true, - }, - }, - { - name: "validate deleting operation", - request: &admissionv1beta1.AdmissionRequest{ - Resource: managedclustersetbindingSchema, - Operation: admissionv1beta1.Delete, - }, - expectedResponse: &admissionv1beta1.AdmissionResponse{ - Allowed: true, - }, - }, - { - name: "validate creating cluster set binding", - request: &admissionv1beta1.AdmissionRequest{ - Resource: managedclustersetbindingSchema, - Operation: admissionv1beta1.Create, - Object: newManagedClusterSetBindingObj("ns1", "cs1", "cs1", nil), - }, - allowBindingToClusterSet: true, - expectedResponse: &admissionv1beta1.AdmissionResponse{ - Allowed: true, - }, - }, - { - name: "validate creating cluster set binding with unmatched name", - request: &admissionv1beta1.AdmissionRequest{ - Resource: managedclustersetbindingSchema, - Operation: admissionv1beta1.Create, - Object: newManagedClusterSetBindingObj("ns1", "csb1", "cs1", nil), - }, - expectedResponse: &admissionv1beta1.AdmissionResponse{ - Allowed: false, - Result: &metav1.Status{ - Status: metav1.StatusFailure, Code: http.StatusBadRequest, Reason: metav1.StatusReasonBadRequest, - Message: "The ManagedClusterSetBinding must have the same name as the target ManagedClusterSet", - }, - }, - }, - { - name: "validate creating cluster set binding without permission", - request: &admissionv1beta1.AdmissionRequest{ - Resource: managedclustersetbindingSchema, - Operation: admissionv1beta1.Create, - Object: newManagedClusterSetBindingObj("ns1", "cs1", "cs1", nil), - }, - expectedResponse: &admissionv1beta1.AdmissionResponse{ - Allowed: false, - Result: &metav1.Status{ - Status: metav1.StatusFailure, Code: http.StatusForbidden, Reason: metav1.StatusReasonForbidden, - Message: "user \"\" is not allowed to bind cluster set \"cs1\"", - }, - }, - }, - { - name: "validate updating cluster set binding", - request: &admissionv1beta1.AdmissionRequest{ - Resource: managedclustersetbindingSchema, - Operation: admissionv1beta1.Update, - Object: newManagedClusterSetBindingObj("ns1", "cs1", "cs1", nil), - OldObject: newManagedClusterSetBindingObj("ns1", "cs1", "cs2", map[string]string{ - "team": "team1", - }), - }, - allowBindingToClusterSet: true, - expectedResponse: &admissionv1beta1.AdmissionResponse{ - Allowed: true, - }, - }, - { - name: "validate updating cluster set binding with different cluster set", - request: &admissionv1beta1.AdmissionRequest{ - Resource: managedclustersetbindingSchema, - Operation: admissionv1beta1.Update, - Object: newManagedClusterSetBindingObj("ns1", "cs1", "cs2", nil), - OldObject: newManagedClusterSetBindingObj("ns1", "cs1", "cs1", nil), - }, - expectedResponse: &admissionv1beta1.AdmissionResponse{ - Allowed: false, - Result: &metav1.Status{ - Status: metav1.StatusFailure, Code: http.StatusBadRequest, Reason: metav1.StatusReasonBadRequest, - Message: "The ManagedClusterSetBinding must have the same name as the target ManagedClusterSet", - }, - }, - }, - } - - for _, c := range cases { - t.Run(c.name, func(t *testing.T) { - kubeClient := kubefake.NewSimpleClientset() - kubeClient.PrependReactor( - "create", - "subjectaccessreviews", - func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { - return true, &authorizationv1.SubjectAccessReview{ - Status: authorizationv1.SubjectAccessReviewStatus{ - Allowed: c.allowBindingToClusterSet, - }, - }, nil - }, - ) - - admissionHook := &ManagedClusterSetBindingValidatingAdmissionHook{ - kubeClient: kubeClient, - } - - actualResponse := admissionHook.Validate(c.request) - if !reflect.DeepEqual(actualResponse, c.expectedResponse) { - t.Errorf("expected %#v but got: %#v", c.expectedResponse.Result, actualResponse.Result) - } - }) - } -} - -func newManagedClusterSetBindingObj(namespace, name, clusterSetName string, labels map[string]string) runtime.RawExtension { - managedClusterSetBinding := &clusterv1beta2.ManagedClusterSetBinding{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: namespace, - Name: name, - Labels: labels, - }, - Spec: clusterv1beta2.ManagedClusterSetBindingSpec{ - ClusterSet: clusterSetName, - }, - } - bindingObj, _ := json.Marshal(managedClusterSetBinding) - return runtime.RawExtension{ - Raw: bindingObj, - } -} diff --git a/pkg/webhook/v1/managedcluster_validating.go b/pkg/webhook/v1/managedcluster_validating.go index 798b2e37b..5f69b90de 100644 --- a/pkg/webhook/v1/managedcluster_validating.go +++ b/pkg/webhook/v1/managedcluster_validating.go @@ -3,11 +3,13 @@ package v1 import ( "context" "fmt" + "strings" operatorhelpers "github.com/openshift/library-go/pkg/operator/v1helpers" authenticationv1 "k8s.io/api/authentication/v1" authorizationv1 "k8s.io/api/authorization/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" + apimachineryvalidation "k8s.io/apimachinery/pkg/api/validation" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" clusterv1beta2 "open-cluster-management.io/api/cluster/v1beta2" @@ -107,7 +109,10 @@ func (r *ManagedClusterWebhook) ValidateDelete(_ context.Context, obj runtime.Ob // validateManagedClusterObj validates the fileds of ManagedCluster object func (r *ManagedClusterWebhook) validateManagedClusterObj(cluster v1.ManagedCluster) error { errs := []error{} - + // The cluster name must be the same format of namespace name. + if errMsgs := apimachineryvalidation.ValidateNamespaceName(cluster.Name, false); len(errMsgs) > 0 { + errs = append(errs, fmt.Errorf("metadata.name format is not correct: %s", strings.Join(errMsgs, ","))) + } // there are no spoke client configs, finish the validation process if len(cluster.Spec.ManagedClusterClientConfigs) == 0 { return nil diff --git a/pkg/webhook/v1/managedcluster_validating_test.go b/pkg/webhook/v1/managedcluster_validating_test.go index 73a6f4f1d..3c94171ae 100644 --- a/pkg/webhook/v1/managedcluster_validating_test.go +++ b/pkg/webhook/v1/managedcluster_validating_test.go @@ -119,6 +119,20 @@ func TestValidateCreate(t *testing.T) { }, }, }, + { + name: "validate cluster name", + expectedError: true, + cluster: &v1.ManagedCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "01.set", + }, + Spec: v1.ManagedClusterSpec{ + ManagedClusterClientConfigs: []v1.ClientConfig{ + {URL: "https://127.0.0.1:8001"}, + }, + }, + }, + }, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { diff --git a/test/e2e/webhook_test.go b/test/e2e/webhook_test.go index 2e93e01a6..b37ec0cd1 100644 --- a/test/e2e/webhook_test.go +++ b/test/e2e/webhook_test.go @@ -201,6 +201,18 @@ var _ = ginkgo.Describe("Admission webhook", func() { gomega.Expect(u.deleteManageClusterAndRelatedNamespace(clusterName)).ToNot(gomega.HaveOccurred()) }) + ginkgo.It("Should respond bad request when cluster name is invalid", func() { + clusterName := fmt.Sprintf("webhook.spoke-%s", rand.String(6)) + ginkgo.By(fmt.Sprintf("create a managed cluster %q with an invalid name", clusterName)) + + managedCluster := newManagedCluster(clusterName, false, validURL) + + _, err := clusterClient.ClusterV1().ManagedClusters().Create(context.TODO(), managedCluster, metav1.CreateOptions{}) + gomega.Expect(err).To(gomega.HaveOccurred()) + gomega.Expect(errors.IsBadRequest(err)).Should(gomega.BeTrue()) + gomega.Expect(u.deleteManageClusterAndRelatedNamespace(clusterName)).ToNot(gomega.HaveOccurred()) + }) + ginkgo.It("Should forbid the request when creating an accepted managed cluster by unauthorized user", func() { sa := fmt.Sprintf("webhook-sa-%s", rand.String(6)) clusterName := fmt.Sprintf("webhook-spoke-%s", rand.String(6))