v0.8.3: admission.Handler is being injected with a nil admission.Decoder

[nesv]

Hi there!

Recently, I have been upgrading a code base from sigs.k8s.io/controller-runtime from v0.6.0 → v0.8.3. In v0.6.0, everything was working fine, but after updating to v0.8.3, our admission.Handlers are being injected with nil *admission.Decoders.

Nothing has changed with how we are registering any CRDs.

I am still doing some digging through the changesets between 0.6.0 and 0.8.3 to see if anything changed with regards to decoder injection, but I wanted to file this issue just to get the ball rolling. I will post a comment for anything I find along the way.

Dependencies

As a side note about dependencies, any dependencies in the k8s.io/ tree had already been updated to a v.20.x tag. The k8s.io/ packages are updated separately, and when updating to controller-runtime v0.8.3, no k8s.io/ packages were updated as a side-effect, since the k8s.io/ packages were already at compatible version.

I felt it necessary to mention this aspect, to preempt any discussion about k8s.io/ package versions, since the only change in dependencies that occurred before this issue started, was bumping sigs.k8s.io/controller-runtime from 0.6.0 → 0.8.3.

[nesv]

I'm officially stuck on where this issue could be happening.

With regards to the runtime.Scheme we pass when instantiating a manager.Manager, we are:

• Registering our CRDs with the scheme.Builder following the examples in the scheme package's godoc;
• Creating a runtime.Scheme, passing it to "k8s.io/client-go/kubernetes/scheme".AddToScheme, as well as the AddToScheme functions in the packages for our CRDs;
• Creating a new manager.Manager (via manager.New), and passing the runtime.Scheme in via the manager.Options.Scheme field.

So, all of the resource we want to handle in our handlers (both validating and mutating) are being added to the manager's scheme, yet the handlers are consistently getting nil decoders injected.

I have looked through the controller-runtime packages, and seen how the runtime.Scheme is passed around, but I'm still unable to figure out what could be causing a nil decoder to be injected.

[porridge]

Can you please post a stack trace where you see this nil?
Looking at the library code, I cannot see how a nil could get passed to InjectDecoder.

Maybe in your case it simply is not passed at all, and you're observing the nil somewhere else?

[nesv]

Hi @porridge!

@d-honeybadger and I were working on this, and she may have found the culprit. I say "may" because although this fix works, it could indicate the code we are working with is doing something incorrectly.

In https://github.com/kubernetes-sigs/controller-runtime/pull/1307/files#diff-517abdd2f47b744e11990598de732426f33aba68600eaa9600d2ee20bc0b25dcL227-L256, the order of injection functions changes slightly, resulting in webhook.InjectScheme being called after webhook.InjectFunc. This means that when inject.InjectorInto is called, the scheme hasn't been set when cm.cluster.SetFields is called.

The "fix" in this case, is to move the call to inject.InjectorInto after the call to cm.cluster.SetFields.

EDIT: I had previously missed (or dismissed) this error when I initially encountered it:

unable to inject fields into webhook during registration

This error message shows up in the logs after the admission.Handler's InjectDecoder method has been called.

[d-honeybadger]

@porridge Looking more closely, the "just-in-case" decoder injection that you pointed out indeed works for most webhooks. The one case where it doesn't work is a webhook with a multiValidating handler (the same would be true for multiMutating handler).

Since multiValidating handler does not implement DecoderInjector interface, doing if _, err := InjectDecoderInto(w.GetDecoder(), w.Handler) doesn't inject anything in the sub-handlers of the multiValidating handler.

[alvaroaleman]

FIxed in #1502