Merge pull request #3779 from yike21/add-validation-for-mcs

add validation for MultiClusterService
2023-07-12 15:26:25 +08:00 · 2023-07-12 15:26:25 +08:00 · a07260980c
parent 0b5106fcc2 9079268ebb
commit a07260980c
8 changed files with 260 additions and 15 deletions
--- a/artifacts/deploy/webhook-configuration.yaml
+++ b/artifacts/deploy/webhook-configuration.yaml
@ -18,7 +18,7 @@ webhooks:
    failurePolicy: Fail
    sideEffects: None
    admissionReviewVersions: ["v1"]
-    timeoutSeconds: 10
+    timeoutSeconds: 3
  - name: clusterpropagationpolicy.karmada.io
    rules:
      - operations: ["CREATE", "UPDATE"]
@ -32,7 +32,7 @@ webhooks:
    failurePolicy: Fail
    sideEffects: None
    admissionReviewVersions: ["v1"]
-    timeoutSeconds: 10
+    timeoutSeconds: 3
  - name: overridepolicy.karmada.io
    rules:
      - operations: ["CREATE", "UPDATE"]
@ -46,7 +46,7 @@ webhooks:
    failurePolicy: Fail
    sideEffects: None
    admissionReviewVersions: ["v1"]
-    timeoutSeconds: 10
+    timeoutSeconds: 3
  - name: work.karmada.io
    rules:
      - operations: ["CREATE", "UPDATE"]
@ -60,7 +60,7 @@ webhooks:
    failurePolicy: Fail
    sideEffects: None
    admissionReviewVersions: ["v1"]
-    timeoutSeconds: 10
+    timeoutSeconds: 3
  - name: autoscaling.karmada.io
    rules:
      - operations: ["CREATE", "UPDATE"]
@ -74,7 +74,7 @@ webhooks:
    failurePolicy: Fail
    sideEffects: None
    admissionReviewVersions: [ "v1" ]
-    timeoutSeconds: 10
+    timeoutSeconds: 3
 ---
 apiVersion: admissionregistration.k8s.io/v1
 kind: ValidatingWebhookConfiguration
@ -96,7 +96,7 @@ webhooks:
    failurePolicy: Fail
    sideEffects: None
    admissionReviewVersions: ["v1"]
-    timeoutSeconds: 10
+    timeoutSeconds: 3
  - name: clusterpropagationpolicy.karmada.io
    rules:
      - operations: ["CREATE", "UPDATE"]
@ -110,7 +110,7 @@ webhooks:
    failurePolicy: Fail
    sideEffects: None
    admissionReviewVersions: ["v1"]
-    timeoutSeconds: 10
+    timeoutSeconds: 3
  - name: overridepolicy.karmada.io
    rules:
      - operations: ["CREATE", "UPDATE"]
@ -124,7 +124,7 @@ webhooks:
    failurePolicy: Fail
    sideEffects: None
    admissionReviewVersions: ["v1"]
-    timeoutSeconds: 10
+    timeoutSeconds: 3
  - name: clusteroverridepolicy.karmada.io
    rules:
      - operations: ["CREATE", "UPDATE"]
@ -138,7 +138,7 @@ webhooks:
    failurePolicy: Fail
    sideEffects: None
    admissionReviewVersions: ["v1"]
-    timeoutSeconds: 10
+    timeoutSeconds: 3
  - name: config.karmada.io
    rules:
      - operations: ["CREATE", "UPDATE"]
@ -152,7 +152,7 @@ webhooks:
    failurePolicy: Fail
    sideEffects: None
    admissionReviewVersions: ["v1"]
-    timeoutSeconds: 10
+    timeoutSeconds: 3
  - name: resourceinterpretercustomization.karmada.io
    rules:
      - operations: ["CREATE", "UPDATE"]
@ -166,7 +166,7 @@ webhooks:
    failurePolicy: Fail
    sideEffects: None
    admissionReviewVersions: ["v1"]
-    timeoutSeconds: 10
+    timeoutSeconds: 3
  - name: federatedresourcequota.karmada.io
    rules:
      - operations: ["CREATE", "UPDATE"]
@ -180,7 +180,7 @@ webhooks:
    failurePolicy: Fail
    sideEffects: None
    admissionReviewVersions: [ "v1" ]
-    timeoutSeconds: 10
+    timeoutSeconds: 3
  - name: multiclusteringress.karmada.io
    rules:
      - operations: ["CREATE", "UPDATE"]
@ -194,7 +194,7 @@ webhooks:
    failurePolicy: Fail
    sideEffects: None
    admissionReviewVersions: [ "v1" ]
-    timeoutSeconds: 10
+    timeoutSeconds: 3
  - name: federatedhpa.karmada.io
    rules:
      - operations: ["CREATE", "UPDATE"]
@ -208,7 +208,7 @@ webhooks:
    failurePolicy: Fail
    sideEffects: None
    admissionReviewVersions: [ "v1" ]
-    timeoutSeconds: 10
+    timeoutSeconds: 3
  - name: cronfederatedhpa.karmada.io
    rules:
      - operations: ["CREATE", "UPDATE"]
@ -222,4 +222,18 @@ webhooks:
    failurePolicy: Fail
    sideEffects: None
    admissionReviewVersions: [ "v1" ]
-    timeoutSeconds: 10
+    timeoutSeconds: 3
+  - name: multiclusterservice.karmada.io
+    rules:
+      - operations: ["CREATE", "UPDATE"]
+        apiGroups: ["networking.karmada.io"]
+        apiVersions: ["*"]
+        resources: ["multiclusterservices"]
+        scope: "Namespaced"
+    clientConfig:
+      url: https://karmada-webhook.karmada-system.svc:443/validate-multiclusterservice
+      caBundle: {{caBundle}}
+    failurePolicy: Fail
+    sideEffects: None
+    admissionReviewVersions: [ "v1" ]
+    timeoutSeconds: 3
--- a/charts/karmada/templates/_karmada_webhook_configuration.tpl
+++ b/charts/karmada/templates/_karmada_webhook_configuration.tpl
@ -199,4 +199,18 @@ webhooks:
    sideEffects: None
    admissionReviewVersions: ["v1"]
    timeoutSeconds: 3
+  - name: multiclusterservice.karmada.io
+    rules:
+      - operations: ["CREATE", "UPDATE"]
+        apiGroups: ["networking.karmada.io"]
+        apiVersions: ["*"]
+        resources: ["multiclusterservices"]
+        scope: "Namespaced"
+    clientConfig:
+      url: https://{{ $name }}-webhook.{{ $namespace }}.svc:443/validate-multiclusterservice
+      {{- include "karmada.webhook.caBundle" . | nindent 6 }}
+    failurePolicy: Fail
+    sideEffects: None
+    admissionReviewVersions: [ "v1" ]
+    timeoutSeconds: 3
 {{- end -}}
--- a/cmd/webhook/app/webhook.go
+++ b/cmd/webhook/app/webhook.go
@ -29,6 +29,7 @@ import (
 	"github.com/karmada-io/karmada/pkg/webhook/federatedhpa"
 	"github.com/karmada-io/karmada/pkg/webhook/federatedresourcequota"
 	"github.com/karmada-io/karmada/pkg/webhook/multiclusteringress"
+	"github.com/karmada-io/karmada/pkg/webhook/multiclusterservice"
 	"github.com/karmada-io/karmada/pkg/webhook/overridepolicy"
 	"github.com/karmada-io/karmada/pkg/webhook/propagationpolicy"
 	"github.com/karmada-io/karmada/pkg/webhook/resourceinterpretercustomization"
@ -133,6 +134,7 @@ func Run(ctx context.Context, opts *options.Options) error {
 	hookServer.Register("/validate-cronfederatedhpa", &webhook.Admission{Handler: &cronfederatedhpa.ValidatingAdmission{}})
 	hookServer.Register("/validate-resourceinterpretercustomization", &webhook.Admission{Handler: &resourceinterpretercustomization.ValidatingAdmission{Client: hookManager.GetClient()}})
 	hookServer.Register("/validate-multiclusteringress", &webhook.Admission{Handler: &multiclusteringress.ValidatingAdmission{}})
+	hookServer.Register("/validate-multiclusterservice", &webhook.Admission{Handler: &multiclusterservice.ValidatingAdmission{}})
 	hookServer.Register("/mutate-federatedhpa", &webhook.Admission{Handler: &federatedhpa.MutatingAdmission{}})
 	hookServer.WebhookMux.Handle("/readyz/", http.StripPrefix("/readyz/", &healthz.Handler{}))

--- a/operator/pkg/karmadaresource/webhookconfiguration/manifests.go
+++ b/operator/pkg/karmadaresource/webhookconfiguration/manifests.go
@ -231,5 +231,19 @@ webhooks:
    sideEffects: None
    admissionReviewVersions: [ "v1" ]
    timeoutSeconds: 3
+  - name: multiclusterservice.karmada.io
+    rules:
+      - operations: ["CREATE", "UPDATE"]
+        apiGroups: ["networking.karmada.io"]
+        apiVersions: ["*"]
+        resources: ["multiclusterservices"]
+        scope: "Namespaced"
+    clientConfig:
+      url: https://{{ .Service }}.{{ .Namespace }}.svc:443/validate-multiclusterservice
+      caBundle: {{ .CaBundle }}
+    failurePolicy: Fail
+    sideEffects: None
+    admissionReviewVersions: [ "v1" ]
+    timeoutSeconds: 3
 `
 )
--- a/pkg/karmadactl/cmdinit/karmada/webhook_configuration.go
+++ b/pkg/karmadactl/cmdinit/karmada/webhook_configuration.go
@ -242,6 +242,20 @@ webhooks:
    sideEffects: None
    admissionReviewVersions: [ "v1" ]
    timeoutSeconds: 3
+  - name: multiclusterservice.karmada.io
+    rules:
+      - operations: ["CREATE", "UPDATE"]
+        apiGroups: ["networking.karmada.io"]
+        apiVersions: ["*"]
+        resources: ["multiclusterservices"]
+        scope: "Namespaced"
+    clientConfig:
+      url: https://karmada-webhook.%[1]s.svc:443/validate-multiclusterservice
+      caBundle: %[2]s
+    failurePolicy: Fail
+    sideEffects: None
+    admissionReviewVersions: [ "v1" ]
+    timeoutSeconds: 3
 `, systemNamespace, caBundle)
 }

--- a/pkg/util/lifted/doc.go
+++ b/pkg/util/lifted/doc.go
@ -91,6 +91,7 @@ package lifted
 | validatingmci_test.go    | https://github.com/kubernetes/kubernetes/blob/release-1.27/pkg/apis/networking/validation/validation_test.go                             | func TestValidateIngressTLS                       | N       |
 | validatingmci_test.go    | https://github.com/kubernetes/kubernetes/blob/release-1.27/pkg/apis/networking/validation/validation_test.go                             | func TestValidateEmptyIngressTLS                  | N       |
 | validatingmci_test.go    | https://github.com/kubernetes/kubernetes/blob/release-1.27/pkg/apis/networking/validation/validation_test.go                             | func TestValidateIngressStatusUpdate              | Y       |
+| validatingmcs.go         | https://github.com/kubernetes/kubernetes/blob/release-1.27/pkg/apis/core/validation/validation.go#L6826-L6846                            | func ValidateLoadBalancerStatus                   | N       |
 | visitpod.go              | https://github.com/kubernetes/kubernetes/blob/release-1.23/pkg/api/v1/pod/util.go#L53-L63                                                | type ContainerType                                | N       |
 | visitpod.go              | https://github.com/kubernetes/kubernetes/blob/release-1.23/pkg/api/v1/pod/util.go#L65-L66                                                | const AllContainers                               | N       |
 | visitpod.go              | https://github.com/kubernetes/kubernetes/blob/release-1.23/pkg/api/v1/pod/util.go#L78-L80                                                | type ContainerVisitor                             | N       |
--- a/pkg/util/lifted/validatingmcs.go
+++ b/pkg/util/lifted/validatingmcs.go
@ -0,0 +1,52 @@
+/*
+Copyright 2017 The Kubernetes 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.
+*/
+
+// This code is directly lifted from the Kubernetes codebase.
+// For reference:
+// https://github.com/kubernetes/kubernetes/blob/release-1.27/pkg/apis/core/validation/validation.go
+
+package lifted
+
+import (
+	corev1 "k8s.io/api/core/v1"
+	validation "k8s.io/apimachinery/pkg/util/validation"
+	"k8s.io/apimachinery/pkg/util/validation/field"
+	netutils "k8s.io/utils/net"
+)
+
+// +lifted:source=https://github.com/kubernetes/kubernetes/blob/release-1.27/pkg/apis/core/validation/validation.go#L6826-L6846
+
+// ValidateLoadBalancerStatus validates required fields on a LoadBalancerStatus
+func ValidateLoadBalancerStatus(status *corev1.LoadBalancerStatus, fldPath *field.Path) field.ErrorList {
+	allErrs := field.ErrorList{}
+	for i, ingress := range status.Ingress {
+		idxPath := fldPath.Child("ingress").Index(i)
+		if len(ingress.IP) > 0 {
+			if isIP := (netutils.ParseIPSloppy(ingress.IP) != nil); !isIP {
+				allErrs = append(allErrs, field.Invalid(idxPath.Child("ip"), ingress.IP, "must be a valid IP address"))
+			}
+		}
+		if len(ingress.Hostname) > 0 {
+			for _, msg := range validation.IsDNS1123Subdomain(ingress.Hostname) {
+				allErrs = append(allErrs, field.Invalid(idxPath.Child("hostname"), ingress.Hostname, msg))
+			}
+			if isIP := (netutils.ParseIPSloppy(ingress.Hostname) != nil); isIP {
+				allErrs = append(allErrs, field.Invalid(idxPath.Child("hostname"), ingress.Hostname, "must be a DNS name, not an IP address"))
+			}
+		}
+	}
+	return allErrs
+}
--- a/pkg/webhook/multiclusterservice/validating.go
+++ b/pkg/webhook/multiclusterservice/validating.go
@ -0,0 +1,134 @@
+package multiclusterservice
+
+import (
+	"context"
+	"net/http"
+	"strings"
+
+	admissionv1 "k8s.io/api/admission/v1"
+	apimachineryvalidation "k8s.io/apimachinery/pkg/api/validation"
+	"k8s.io/apimachinery/pkg/util/sets"
+	validation "k8s.io/apimachinery/pkg/util/validation"
+	"k8s.io/apimachinery/pkg/util/validation/field"
+	"k8s.io/klog/v2"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+
+	clustervalidation "github.com/karmada-io/karmada/pkg/apis/cluster/validation"
+	networkingv1alpha1 "github.com/karmada-io/karmada/pkg/apis/networking/v1alpha1"
+	"github.com/karmada-io/karmada/pkg/util/lifted"
+)
+
+// ValidatingAdmission validates MultiClusterService object when creating/updating.
+type ValidatingAdmission struct {
+	decoder *admission.Decoder
+}
+
+// Check if our ValidatingAdmission implements necessary interface
+var _ admission.Handler = &ValidatingAdmission{}
+var _ admission.DecoderInjector = &ValidatingAdmission{}
+
+// Handle implements admission.Handler interface.
+// It yields a response to an AdmissionRequest.
+func (v *ValidatingAdmission) Handle(ctx context.Context, req admission.Request) admission.Response {
+	mcs := &networkingv1alpha1.MultiClusterService{}
+	err := v.decoder.Decode(req, mcs)
+	if err != nil {
+		return admission.Errored(http.StatusBadRequest, err)
+	}
+	klog.Infof("Validating MultiClusterService(%s/%s) for request: %s", mcs.Namespace, mcs.Name, req.Operation)
+
+	if req.Operation == admissionv1.Update {
+		oldMcs := &networkingv1alpha1.MultiClusterService{}
+		err = v.decoder.DecodeRaw(req.OldObject, oldMcs)
+		if err != nil {
+			return admission.Errored(http.StatusBadRequest, err)
+		}
+		if errs := v.validateMCSUpdate(oldMcs, mcs); len(errs) != 0 {
+			klog.Errorf("Validating MultiClusterServiceUpdate failed: %v", errs)
+			return admission.Denied(errs.ToAggregate().Error())
+		}
+	} else {
+		if errs := v.validateMCS(mcs); len(errs) != 0 {
+			klog.Errorf("Validating MultiClusterService failed: %v", errs)
+			return admission.Denied(errs.ToAggregate().Error())
+		}
+	}
+	return admission.Allowed("")
+}
+
+// InjectDecoder implements admission.DecoderInjector interface.
+// A decoder will be automatically injected.
+func (v *ValidatingAdmission) InjectDecoder(d *admission.Decoder) error {
+	v.decoder = d
+	return nil
+}
+
+func (v *ValidatingAdmission) validateMCSUpdate(oldMcs, newMcs *networkingv1alpha1.MultiClusterService) field.ErrorList {
+	allErrs := apimachineryvalidation.ValidateObjectMetaUpdate(&newMcs.ObjectMeta, &oldMcs.ObjectMeta, field.NewPath("metadata"))
+	allErrs = append(allErrs, v.validateMCS(newMcs)...)
+	allErrs = append(allErrs, lifted.ValidateLoadBalancerStatus(&newMcs.Status.LoadBalancer, field.NewPath("status", "loadBalancer"))...)
+	return allErrs
+}
+
+func (v *ValidatingAdmission) validateMCS(mcs *networkingv1alpha1.MultiClusterService) field.ErrorList {
+	allErrs := apimachineryvalidation.ValidateObjectMeta(&mcs.ObjectMeta, true,
+		apimachineryvalidation.NameIsDNS1035Label, field.NewPath("metadata"))
+	allErrs = append(allErrs, v.validateMultiClusterServiceSpec(mcs)...)
+	return allErrs
+}
+
+// validateMultiClusterServiceSpec validates MultiClusterService spec.
+func (v *ValidatingAdmission) validateMultiClusterServiceSpec(mcs *networkingv1alpha1.MultiClusterService) field.ErrorList {
+	allErrs := field.ErrorList{}
+	specPath := field.NewPath("spec")
+	allPortNames := sets.New[string]()
+
+	portsPath := specPath.Child("ports")
+	for i := range mcs.Spec.Ports {
+		portPath := portsPath.Index(i)
+		port := mcs.Spec.Ports[i]
+		allErrs = append(allErrs, v.validateExposurePort(&port, allPortNames, portPath)...)
+	}
+	typesPath := specPath.Child("types")
+	for i := range mcs.Spec.Ports {
+		typePath := typesPath.Index(i)
+		exposureType := mcs.Spec.Types[i]
+		allErrs = append(allErrs, v.validateExposureType(&exposureType, typePath)...)
+	}
+	clusterNamesPath := specPath.Child("range").Child("clusterNames")
+	for i := range mcs.Spec.Range.ClusterNames {
+		clusterNamePath := clusterNamesPath.Index(i)
+		clusterName := mcs.Spec.Range.ClusterNames[i]
+		if errMegs := clustervalidation.ValidateClusterName(clusterName); len(errMegs) > 0 {
+			allErrs = append(allErrs, field.Invalid(clusterNamePath, clusterName, strings.Join(errMegs, ",")))
+		}
+	}
+	return allErrs
+}
+
+// validateExposurePort validates MultiClusterService ExposurePort.
+func (v *ValidatingAdmission) validateExposurePort(ep *networkingv1alpha1.ExposurePort, allNames sets.Set[string], fldPath *field.Path) field.ErrorList {
+	allErrs := field.ErrorList{}
+	if len(ep.Name) != 0 {
+		allErrs = append(allErrs, lifted.ValidateDNS1123Label(ep.Name, fldPath.Child("name"))...)
+		if allNames.Has(ep.Name) {
+			allErrs = append(allErrs, field.Duplicate(fldPath.Child("name"), ep.Name))
+		} else {
+			allNames.Insert(ep.Name)
+		}
+	}
+	for _, msg := range validation.IsValidPortNum(int(ep.Port)) {
+		allErrs = append(allErrs, field.Invalid(fldPath.Child("port"), ep.Port, msg))
+	}
+	return allErrs
+}
+
+// validateExposureType validates MultiClusterService ExposureType.
+func (v *ValidatingAdmission) validateExposureType(et *networkingv1alpha1.ExposureType, fldPath *field.Path) field.ErrorList {
+	allErrs := field.ErrorList{}
+	if *et != networkingv1alpha1.ExposureTypeCrossCluster && *et != networkingv1alpha1.ExposureTypeLoadBalancer {
+		msg := "ExposureType Error"
+		allErrs = append(allErrs, field.Invalid(fldPath, *et, msg))
+	}
+	return allErrs
+}