From 9079268ebbff96be79dbec014a3e0db64db60264 Mon Sep 17 00:00:00 2001 From: yike21 Date: Sun, 9 Jul 2023 22:02:02 +0800 Subject: [PATCH] add validation for mcs Signed-off-by: yike21 --- artifacts/deploy/webhook-configuration.yaml | 44 ++++-- .../_karmada_webhook_configuration.tpl | 14 ++ cmd/webhook/app/webhook.go | 2 + .../webhookconfiguration/manifests.go | 14 ++ .../cmdinit/karmada/webhook_configuration.go | 14 ++ pkg/util/lifted/doc.go | 1 + pkg/util/lifted/validatingmcs.go | 52 +++++++ pkg/webhook/multiclusterservice/validating.go | 134 ++++++++++++++++++ 8 files changed, 260 insertions(+), 15 deletions(-) create mode 100644 pkg/util/lifted/validatingmcs.go create mode 100644 pkg/webhook/multiclusterservice/validating.go diff --git a/artifacts/deploy/webhook-configuration.yaml b/artifacts/deploy/webhook-configuration.yaml index b0459fb5b..bd997094e 100644 --- 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 diff --git a/charts/karmada/templates/_karmada_webhook_configuration.tpl b/charts/karmada/templates/_karmada_webhook_configuration.tpl index e33d21325..79bba82aa 100644 --- 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 -}} diff --git a/cmd/webhook/app/webhook.go b/cmd/webhook/app/webhook.go index 4f4041c62..558ab69b6 100644 --- 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{})) diff --git a/operator/pkg/karmadaresource/webhookconfiguration/manifests.go b/operator/pkg/karmadaresource/webhookconfiguration/manifests.go index b3595d91b..0a23b32f0 100644 --- 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 ` ) diff --git a/pkg/karmadactl/cmdinit/karmada/webhook_configuration.go b/pkg/karmadactl/cmdinit/karmada/webhook_configuration.go index 677fb1ee2..0ccfba31e 100644 --- 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) } diff --git a/pkg/util/lifted/doc.go b/pkg/util/lifted/doc.go index d3f79c2ef..05e8d3da7 100644 --- 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 | diff --git a/pkg/util/lifted/validatingmcs.go b/pkg/util/lifted/validatingmcs.go new file mode 100644 index 000000000..6e61c7bb3 --- /dev/null +++ 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 +} diff --git a/pkg/webhook/multiclusterservice/validating.go b/pkg/webhook/multiclusterservice/validating.go new file mode 100644 index 000000000..87b269037 --- /dev/null +++ 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 +}