admission.Decoder.Decode is panicking after version bump

[mahmud2011]

I updated controller-runtime from v0.14.4 to v0.15.3 along with k8s.io/* from v0.26.2 to v0.27.7.

type PodMut struct {
    decoder        *admission.Decoder
    ...
}


func (pm *PodMut) Handle(ctx context.Context, req admission.Request) admission.Response {
	pod := &corev1.Pod{}
	err := pm.decoder.Decode(req, pod)
        ...
}

runtime error: invalid memory address or nil pointer dereference

/usr/local/go/src/runtime/panic.go:920 +0x270
sigs.k8s.io/controller-runtime/pkg/webhook/admission.(*Decoder).DecodeRaw(0x0, {{0xc000018b00, 0x2049, 0x2529}, {0x0, 0x0}}, {0x1a42b72, 0xc000229690})

I don't see any breaking changes for this in release log. I also inspected the dump using spew couldn't figure out why it's panicking.

[mahmud2011]

/kind support

[troy0820]

https://github.com/kubernetes-sigs/controller-runtime/releases/tag/v0.15.0 shows that

Add constructors for webhook/conversion, remove webhook/admission.GetDecoder() (https://github.com/kubernetes-sigs/controller-runtime/pull/2144)
There are two set of changes related to the webhooks and how they're exposed. For users using the Manager, there should be minimal to no required changes.
webhook/admission/Webhook.GetDecoder() method has been removed, it was unused before and relied on runtime dependency injection.
webhook/conversion.Webhook struct has been un-exported. Users should use webhook/conversion.NewWebhookHandler instead.
pkg/webhook/admission:
Use Result.Message instead of Result.Reason (https://github.com/kubernetes-sigs/controller-runtime/pull/1539)
Validator and CustomValidator interfaces have been modified to allow returning warnings (https://github.com/kubernetes-sigs/controller-runtime/pull/2014)

Not sure if you are running into issues with this or something involved with the admission.NewDecoder. Can you provide a little more clarity with more information beyond the snippet?

[sbueringer]

@mahmud2011 Please provide more of the stacktrace (at least from: pm.decoder.Decode(req, pod)). You can censor file paths if necessary

[antoninbas]

I ran into the same issue, and I believe the relevant change is actually #2134
The decoder is no longer automatically injected into the webhook handler. You need to provide it yourself when you instantiate PodMut with admission.NewDecoder(mgr.GetScheme()).

[antoninbas]

Providing a full stack trace in case it is still needed, but my analysis above should be correct hopefully:

2024/03/27 01:28:44 http: panic serving 172.18.0.2:21191: runtime error: invalid memory address or nil pointer dereference
goroutine 373 [running]:
net/http.(*conn).serve.func1()
	/usr/local/go/src/net/http/server.go:1868 +0xb9
panic({0x1ffad40?, 0x3a1e740?})
	/usr/local/go/src/runtime/panic.go:920 +0x270
sigs.k8s.io/controller-runtime/pkg/webhook/admission.(*Decoder).DecodeRaw(0x0, {{0xc0007f2900, 0x466, 0x480}, {0x0, 0x0}}, {0x26e9da8, 0xc0002a5200})
	/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/webhook/admission/decode.go:77 +0x140
sigs.k8s.io/controller-runtime/pkg/webhook/admission.(*Decoder).Decode(_, {{{0xc000b26270, 0x24}, {{0xc000588ae0, 0x1a}, {0xc000add768, 0x8}, {0xc000add780, 0xa}}, {{0xc000588b00, ...}, ...}, ...}}, ...)
	/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/webhook/admission/decode.go:49 +0x85
main.(*clusterSetValidator).Handle(_, {_, _}, {{{0xc000b26270, 0x24}, {{0xc000588ae0, 0x1a}, {0xc000add768, 0x8}, {0xc000add780, ...}}, ...}})
	/antrea/multicluster/cmd/multicluster-controller/clusterset_webhook.go:60 +0x50f
sigs.k8s.io/controller-runtime/pkg/webhook/admission.(*Webhook).Handle(_, {_, _}, {{{0xc000b26270, 0x24}, {{0xc000588ae0, 0x1a}, {0xc000add768, 0x8}, {0xc000add780, ...}}, ...}})
	/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/webhook/admission/webhook.go:169 +0x1ed
sigs.k8s.io/controller-runtime/pkg/webhook/admission.(*Webhook).ServeHTTP(0xc0002744b0, {0x7f76c13826a0?, 0xc0003bceb0}, 0xc000b2f000)
	/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/webhook/admission/http.go:98 +0xc10
sigs.k8s.io/controller-runtime/pkg/webhook/internal/metrics.InstrumentedHook.InstrumentHandlerInFlight.func1({0x7f76c13826a0, 0xc0003bceb0}, 0x26f3300?)
	/go/pkg/mod/github.com/prometheus/[email protected]/prometheus/promhttp/instrument_server.go:60 +0xcb
net/http.HandlerFunc.ServeHTTP(0x26f33d0?, {0x7f76c13826a0?, 0xc0003bceb0?}, 0xc0008e18a0?)
	/usr/local/go/src/net/http/server.go:2136 +0x29
github.com/prometheus/client_golang/prometheus/promhttp.InstrumentHandlerCounter.func1({0x26f33d0?, 0xc00043aa80?}, 0xc000b2f000)
	/go/pkg/mod/github.com/prometheus/[email protected]/prometheus/promhttp/instrument_server.go:147 +0xb6
net/http.HandlerFunc.ServeHTTP(0x6fe8c6?, {0x26f33d0?, 0xc00043aa80?}, 0xc0003c8600?)
	/usr/local/go/src/net/http/server.go:2136 +0x29
github.com/prometheus/client_golang/prometheus/promhttp.InstrumentHandlerDuration.func2({0x26f33d0, 0xc00043aa80}, 0xc000b2f000)
	/go/pkg/mod/github.com/prometheus/[email protected]/prometheus/promhttp/instrument_server.go:109 +0xc2
net/http.HandlerFunc.ServeHTTP(0x10?, {0x26f33d0?, 0xc00043aa80?}, 0xc000aa506c?)
	/usr/local/go/src/net/http/server.go:2136 +0x29
net/http.(*ServeMux).ServeHTTP(0x410965?, {0x26f33d0, 0xc00043aa80}, 0xc000b2f000)
	/usr/local/go/src/net/http/server.go:2514 +0x142
net/http.serverHandler.ServeHTTP({0x26e9ec0?}, {0x26f33d0?, 0xc00043aa80?}, 0x6?)
	/usr/local/go/src/net/http/server.go:2938 +0x8e
net/http.(*conn).serve(0xc000ad95f0, {0x2706c68, 0xc0007eb500})
	/usr/local/go/src/net/http/server.go:2009 +0x5f4
created by net/http.(*Server).Serve in goroutine 186
	/usr/local/go/src/net/http/server.go:3086 +0x5cb

[sbueringer]

Thx, agree. This is the most likely cause

[vincepri]

/assign

[vincepri]

/kind bug