`karmada-webhook`: Fix panic when validating ResourceInterpreterWebhookConfiguration with unspecified service port #5960

XiShanYongYe-Chang · 2024-12-17T12:17:33Z

What type of PR is this?

/kind bug

What this PR does / why we need it:

When I apply a ResourceInterpreterWebhookConfiguration's .webhooks[*].clientConfig.service.port is nil, it report panic in the karmada-webhook component.

apiVersion: config.karmada.io/v1alpha1
kind: ResourceInterpreterWebhookConfiguration
metadata:
  name: examples
webhooks:
- clientConfig:
    caBundle: {{caBundle}}
    service:
      namespace: karmada-system
      name: karmada-interpreter-webhook-example
      path: /interpreter-workload
  interpreterContextVersions:
  - v1alpha1
  name: workloads.example.com
  rules:
  - apiGroups:
    - workload.example.io
    apiVersions:
    - v1alpha1
    kinds:
    - Workload
    operations:
    - InterpretReplica
    - ReviseReplica
    - Retain
    - AggregateStatus
    - InterpretHealth
    - InterpretStatus
    - InterpretDependency
  timeoutSeconds: 3

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

`karmada-webhook`: Fixed panic when validating ResourceInterpreterWebhookConfiguration with unspecified service port.

codecov-commenter · 2024-12-17T12:32:28Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 48.17%. Comparing base (8d89d77) to head (262fcb3).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #5960   +/-   ##
=======================================
  Coverage   48.17%   48.17%           
=======================================
  Files         664      664           
  Lines       54799    54811   +12     
=======================================
+ Hits        26399    26407    +8     
- Misses      26684    26688    +4     
  Partials     1716     1716

Flag	Coverage Δ
unittests	`48.17% <100.00%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

pkg/webhook/configuration/validating.go

XiShanYongYe-Chang · 2024-12-19T08:22:44Z

/cc @zhzhuang-zju @RainbowMango

pkg/webhook/configuration/validating.go

zhzhuang-zju · 2024-12-19T08:52:31Z

thanks
/lgtm

RainbowMango

/assign

RainbowMango · 2024-12-20T02:24:54Z

Please share the panic logs, I want to know where the panic is.

RainbowMango · 2024-12-20T02:27:18Z

I reproduced it on my side:

-bash-5.0# kubectl apply -f webhook-configuration.yaml (with the configuration on PR description)
Error from server: error when creating "webhook-configuration.yaml": admission webhook "config.karmada.io" denied the request: panic: runtime error: invalid memory address or nil pointer dereference [recovered]

I1220 02:25:46.272745       1 validating.go:52] Validating ResourceInterpreterWebhookConfiguration(examples) for request: CREATE
E1220 02:25:46.275364       1 signal_unix.go:881] "Observed a panic" panic="runtime error: invalid memory address or nil pointer dereference" panicGoValue="\"invalid memory address or nil pointer dereference\"" stacktrace=<
	goroutine 1202366 [running]:
	k8s.io/apimachinery/pkg/util/runtime.logPanic({0x2245d10, 0xc00067bdd0}, {0x1c05020, 0x32454a0})
		/root/go/src/github.com/karmada-io/karmada/vendor/k8s.io/apimachinery/pkg/util/runtime/runtime.go:107 +0xbc
	sigs.k8s.io/controller-runtime/pkg/webhook/admission.(*Webhook).Handle.func1()
		/root/go/src/github.com/karmada-io/karmada/vendor/sigs.k8s.io/controller-runtime/pkg/webhook/admission/webhook.go:163 +0x103
	panic({0x1c05020?, 0x32454a0?})
		/root/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.22.9.linux-amd64/src/runtime/panic.go:770 +0x132
	github.com/karmada-io/karmada/pkg/webhook/configuration.validateWebhook(0xc0003aee00, 0xc0006465d0)
		/root/go/src/github.com/karmada-io/karmada/pkg/webhook/configuration/validating.go:106 +0x953
	github.com/karmada-io/karmada/pkg/webhook/configuration.(*ValidatingAdmission).Handle(_, {_, _}, {{{0xc0005ba690, 0x24}, {{0xc0000284e0, 0x11}, {0xc000013c10, 0x8}, {0xc0005ba6f0, ...}}, ...}})
		/root/go/src/github.com/karmada-io/karmada/pkg/webhook/configuration/validating.go:57 +0x4b3
	sigs.k8s.io/controller-runtime/pkg/webhook/admission.(*Webhook).Handle(_, {_, _}, {{{0xc0005ba690, 0x24}, {{0xc0000284e0, 0x11}, {0xc000013c10, 0x8}, {0xc0005ba6f0, ...}}, ...}})
		/root/go/src/github.com/karmada-io/karmada/vendor/sigs.k8s.io/controller-runtime/pkg/webhook/admission/webhook.go:181 +0x244
	sigs.k8s.io/controller-runtime/pkg/webhook/admission.(*Webhook).ServeHTTP(0xc00064c280, {0x7f65541369a0, 0xc0000a55e0}, 0xc00043a120)
		/root/go/src/github.com/karmada-io/karmada/vendor/sigs.k8s.io/controller-runtime/pkg/webhook/admission/http.go:119 +0xb12
	sigs.k8s.io/controller-runtime/pkg/webhook/internal/metrics.InstrumentedHook.InstrumentHandlerInFlight.func1({0x7f65541369a0, 0xc0000a55e0}, 0xc00043a120)
		/root/go/src/github.com/karmada-io/karmada/vendor/github.com/prometheus/client_golang/prometheus/promhttp/instrument_server.go:60 +0xcb
	net/http.HandlerFunc.ServeHTTP(0x324eb80?, {0x7f65541369a0?, 0xc0000a55e0?}, 0x86?)
		/root/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.22.9.linux-amd64/src/net/http/server.go:2171 +0x29
	github.com/prometheus/client_golang/prometheus/promhttp.InstrumentHandlerCounter.func1({0x22374a0?, 0xc00018d0a0?}, 0xc00043a120)
		/root/go/src/github.com/karmada-io/karmada/vendor/github.com/prometheus/client_golang/prometheus/promhttp/instrument_server.go:147 +0xc3
	net/http.HandlerFunc.ServeHTTP(0x862448?, {0x22374a0?, 0xc00018d0a0?}, 0x412125?)
		/root/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.22.9.linux-amd64/src/net/http/server.go:2171 +0x29
	github.com/prometheus/client_golang/prometheus/promhttp.InstrumentHandlerDuration.func2({0x22374a0, 0xc00018d0a0}, 0xc00043a120)
		/root/go/src/github.com/karmada-io/karmada/vendor/github.com/prometheus/client_golang/prometheus/promhttp/instrument_server.go:109 +0xc2
	net/http.HandlerFunc.ServeHTTP(0xc0005fa410?, {0x22374a0?, 0xc00018d0a0?}, 0x10?)
		/root/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.22.9.linux-amd64/src/net/http/server.go:2171 +0x29
	net/http.(*ServeMux).ServeHTTP(0x412125?, {0x22374a0, 0xc00018d0a0}, 0xc00043a120)
		/root/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.22.9.linux-amd64/src/net/http/server.go:2688 +0x1ad
	net/http.serverHandler.ServeHTTP({0x222efb8?}, {0x22374a0?, 0xc00018d0a0?}, 0x6?)
		/root/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.22.9.linux-amd64/src/net/http/server.go:3142 +0x8e
	net/http.(*conn).serve(0xc000632e10, {0x2245d10, 0xc00067a4b0})
		/root/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.22.9.linux-amd64/src/net/http/server.go:2044 +0x5e8
	created by net/http.(*Server).Serve in goroutine 125
		/root/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.22.9.linux-amd64/src/net/http/server.go:3290 +0x4b4
 >

RainbowMango · 2024-12-20T04:03:12Z

@lxtywypc, According to the discussion at #2998, you are using service instead of URL to discover the webhook.
Can you help confirm whether you specified the service port? Appreciated if you can share your configuration here.

lxtywypc · 2024-12-20T05:10:37Z

Yes, we quite specified the port.

Like this:

apiVersion: config.karmada.io/v1alpha1
kind: ResourceInterpreterWebhookConfiguration
metadata:
  name: workload-interpretreplica-retain
webhooks:
- name: workload-interpreter-webhook
  rules:
  - apiGroups: 
    - workload.example.io
    apiVersions: 
    - v1
    kinds: 
    - Workload
    operations: 
    - InterpretReplica
    - Retain
  clientConfig:
    service:
      name: workload-interpreter-webhook
      namespace: karmada-system
      path: /interpret-workload
      port: 443
  interpreterContextVersions: 
  - v1alpha1
  timeoutSeconds: 3

RainbowMango · 2024-12-20T06:26:21Z

Thank you @lxtywypc for the clarification. So, this patch will not affect your use case.

RainbowMango · 2024-12-20T06:31:23Z

/retile karmada-webhook: Fix panic when validating ResourceInterpreterWebhookConfiguration with unspecified service port

RainbowMango · 2024-12-20T06:31:51Z

/retitle karmada-webhook: Fix panic when validating ResourceInterpreterWebhookConfiguration with unspecified service port

RainbowMango

Let's fix this with this temporary solution and backport all affected releases.
In the long term, we might need to introduce a mutating webhook for ResourceInterpreterWebhook.

pkg/webhook/configuration/validating.go

pkg/webhook/configuration/validating_test.go

…ooks[*].clientConfig.service.port is nil Signed-off-by: changzhen <changzhen5@huawei.com>

RainbowMango

/lgtm
/approve

karmada-bot · 2024-12-20T09:41:13Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: RainbowMango

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [RainbowMango]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

karmada-bot added the kind/bug Categorizes issue or PR as related to a bug. label Dec 17, 2024

karmada-bot requested review from Poor12 and RainbowMango December 17, 2024 12:17

karmada-bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Dec 17, 2024

zhzhuang-zju reviewed Dec 18, 2024

View reviewed changes

pkg/webhook/configuration/validating.go Show resolved Hide resolved

RainbowMango mentioned this pull request Dec 18, 2024

[bugfix] workqueue metrics are mising #5954

Open

6 tasks

XiShanYongYe-Chang force-pushed the fix-validate-err-ResourceInterpreterWebhookConfiguration branch from e34b026 to 342a7b6 Compare December 18, 2024 09:55

karmada-bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Dec 18, 2024

XiShanYongYe-Chang force-pushed the fix-validate-err-ResourceInterpreterWebhookConfiguration branch 2 times, most recently from ec4e596 to c3fedf0 Compare December 19, 2024 06:16

karmada-bot requested a review from zhzhuang-zju December 19, 2024 08:22

zhzhuang-zju reviewed Dec 19, 2024

View reviewed changes

pkg/webhook/configuration/validating.go Show resolved Hide resolved

karmada-bot assigned zhzhuang-zju Dec 19, 2024

karmada-bot added the lgtm Indicates that a PR is ready to be merged. label Dec 19, 2024

RainbowMango reviewed Dec 20, 2024

View reviewed changes

karmada-bot assigned RainbowMango Dec 20, 2024

XiShanYongYe-Chang force-pushed the fix-validate-err-ResourceInterpreterWebhookConfiguration branch from c3fedf0 to 0e22d2e Compare December 20, 2024 03:44

karmada-bot removed the lgtm Indicates that a PR is ready to be merged. label Dec 20, 2024

RainbowMango added this to the v1.13 milestone Dec 20, 2024

karmada-bot changed the title ~~fix validate panic when ResourceInterpreterWebhookConfiguration's .webhooks[*].clientConfig.service.port is nil~~ karmada-webhook: Fix panic when validating ResourceInterpreterWebhookConfiguration with unspecified service port Dec 20, 2024

RainbowMango reviewed Dec 20, 2024

View reviewed changes

pkg/webhook/configuration/validating.go Outdated Show resolved Hide resolved

pkg/webhook/configuration/validating_test.go Outdated Show resolved Hide resolved

fix validate panic when ResourceInterpreterWebhookConfiguration .webh…

262fcb3

…ooks[*].clientConfig.service.port is nil Signed-off-by: changzhen <changzhen5@huawei.com>

XiShanYongYe-Chang force-pushed the fix-validate-err-ResourceInterpreterWebhookConfiguration branch from 0e22d2e to 262fcb3 Compare December 20, 2024 07:16

RainbowMango approved these changes Dec 20, 2024

View reviewed changes

karmada-bot added the lgtm Indicates that a PR is ready to be merged. label Dec 20, 2024

karmada-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 20, 2024

karmada-bot merged commit 93ffe35 into karmada-io:master Dec 20, 2024
21 checks passed

XiShanYongYe-Chang mentioned this pull request Dec 20, 2024

Cherrypick #5960: Fix panic when validating ResourceInterpreterWebhookConfiguration with unspecified service port #5964

Open

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`karmada-webhook`: Fix panic when validating ResourceInterpreterWebhookConfiguration with unspecified service port #5960

`karmada-webhook`: Fix panic when validating ResourceInterpreterWebhookConfiguration with unspecified service port #5960

XiShanYongYe-Chang commented Dec 17, 2024 •

edited by RainbowMango

Loading

codecov-commenter commented Dec 17, 2024 •

edited

Loading

XiShanYongYe-Chang commented Dec 19, 2024

zhzhuang-zju commented Dec 19, 2024

RainbowMango left a comment

RainbowMango commented Dec 20, 2024

RainbowMango commented Dec 20, 2024 •

edited

Loading

RainbowMango commented Dec 20, 2024

lxtywypc commented Dec 20, 2024

RainbowMango commented Dec 20, 2024

RainbowMango commented Dec 20, 2024

RainbowMango commented Dec 20, 2024

RainbowMango left a comment

RainbowMango left a comment

karmada-bot commented Dec 20, 2024

karmada-webhook: Fix panic when validating ResourceInterpreterWebhookConfiguration with unspecified service port #5960

karmada-webhook: Fix panic when validating ResourceInterpreterWebhookConfiguration with unspecified service port #5960

Conversation

XiShanYongYe-Chang commented Dec 17, 2024 • edited by RainbowMango Loading

codecov-commenter commented Dec 17, 2024 • edited Loading

Codecov Report

XiShanYongYe-Chang commented Dec 19, 2024

zhzhuang-zju commented Dec 19, 2024

RainbowMango left a comment

Choose a reason for hiding this comment

RainbowMango commented Dec 20, 2024

RainbowMango commented Dec 20, 2024 • edited Loading

RainbowMango commented Dec 20, 2024

lxtywypc commented Dec 20, 2024

RainbowMango commented Dec 20, 2024

RainbowMango commented Dec 20, 2024

RainbowMango commented Dec 20, 2024

RainbowMango left a comment

Choose a reason for hiding this comment

RainbowMango left a comment

Choose a reason for hiding this comment

karmada-bot commented Dec 20, 2024

`karmada-webhook`: Fix panic when validating ResourceInterpreterWebhookConfiguration with unspecified service port #5960

`karmada-webhook`: Fix panic when validating ResourceInterpreterWebhookConfiguration with unspecified service port #5960

XiShanYongYe-Chang commented Dec 17, 2024 •

edited by RainbowMango

Loading

codecov-commenter commented Dec 17, 2024 •

edited

Loading

RainbowMango commented Dec 20, 2024 •

edited

Loading