From 262fcb328a4c1901133bcfe2ffd83dec16aa9d78 Mon Sep 17 00:00:00 2001 From: changzhen Date: Tue, 17 Dec 2024 19:43:38 +0800 Subject: [PATCH] fix validate panic when ResourceInterpreterWebhookConfiguration .webhooks[*].clientConfig.service.port is nil Signed-off-by: changzhen --- pkg/webhook/configuration/validating.go | 15 ++++++++++++++- pkg/webhook/configuration/validating_test.go | 14 ++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/pkg/webhook/configuration/validating.go b/pkg/webhook/configuration/validating.go index 7c86aef866c2..e5055c1dea4b 100644 --- a/pkg/webhook/configuration/validating.go +++ b/pkg/webhook/configuration/validating.go @@ -27,6 +27,7 @@ import ( "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apiserver/pkg/util/webhook" "k8s.io/klog/v2" + "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" configv1alpha1 "github.com/karmada-io/karmada/pkg/apis/config/v1alpha1" @@ -103,7 +104,19 @@ func validateWebhook(hook *configv1alpha1.ResourceInterpreterWebhook, fldPath *f case cc.URL != nil: allErrors = append(allErrors, webhook.ValidateWebhookURL(fldPath.Child("clientConfig").Child("url"), *cc.URL, true)...) case cc.Service != nil: - allErrors = append(allErrors, webhook.ValidateWebhookService(fldPath.Child("clientConfig").Child("service"), cc.Service.Name, cc.Service.Namespace, cc.Service.Path, *cc.Service.Port)...) + // Temporary fix: If the service port is not specified, set a default value of 443. + // This is a workaround to prevent a panic when validating ResourceInterpreterWebhookConfiguration + // with an unspecified service port. Ideally, this logic should be handled by a MutatingWebhook, + // but introducing a MutatingWebhook at this stage would require significant changes and is not + // convenient for cherry-picking to release branches. Therefore, we are temporarily setting the + // default port here. Once a MutatingWebhook is implemented, this logic can be moved there. + // + // Note: The Interpreter framework also sets the same default value (443) when processing Service, + // so the backend will not encounter issues due to missing port information. + if cc.Service.Port == nil { + cc.Service.Port = ptr.To[int32](443) + } + allErrors = append(allErrors, webhook.ValidateWebhookService(fldPath.Child("clientConfig").Child("service"), cc.Service.Namespace, cc.Service.Name, cc.Service.Path, *cc.Service.Port)...) } allErrors = append(allErrors, validateInterpreterContextVersions(hook.InterpreterContextVersions, fldPath.Child("interpreterContextVersions"))...) diff --git a/pkg/webhook/configuration/validating_test.go b/pkg/webhook/configuration/validating_test.go index 7097fcf33ade..eb59192949f2 100644 --- a/pkg/webhook/configuration/validating_test.go +++ b/pkg/webhook/configuration/validating_test.go @@ -271,6 +271,20 @@ func TestValidateWebhook(t *testing.T) { }, expectedError: fmt.Sprintf("must include at least one of %v", strings.Join(acceptedInterpreterContextVersions, ", ")), }, + { + name: "valid webhook configuration: use Service in ClientConfig but with port unspecified", + hook: &configv1alpha1.ResourceInterpreterWebhook{ + Name: "workloads.karmada.io", + ClientConfig: admissionregistrationv1.WebhookClientConfig{ + Service: &admissionregistrationv1.ServiceReference{ + Namespace: "default", + Name: "svc", + Path: strPtr("/interpreter"), + }, + }, + InterpreterContextVersions: []string{"v1alpha1"}, + }, + }, } for _, test := range tests {