Kubernetes 1.26/1.27 support

[wlynch]

Expected Behavior

I should be able to use knative.dev/pkg with Kubernetes 1.26 and 1.27 libraries.

Actual Behavior

Cannot build because the injection interface is missing the new AdmissionregistrationV1alpha1 client added in 1.26:

vendor/knative.dev/pkg/client/injection/kube/client/client.go:215:30: cannot use (*wrapClient)(nil) (value of type *wrapClient) as type kubernetes.Interface in variable declaration:
	*wrapClient does not implement kubernetes.Interface (missing AdmissionregistrationV1alpha1 method)

Steps to Reproduce the Problem

1. Use latest knative.dev/pkg + k8s.io/client-go v0.26 or v0.27:

   package main
   
   import (
       _ "k8s.io/client-go/kubernetes"
       _ "knative.dev/pkg/client/injection/kube/client"
   )
   
   func main() {}

2. go.mod

   require (                                  
           k8s.io/client-go v0.27.0                                                            
           knative.dev/pkg v0.0.0-20230412013349-d3d7625d8e1e              
   )  

3. $ go build .
   # knative.dev/pkg/client/injection/kube/client
   /Users/wlynch/go/pkg/mod/knative.dev/pkg@v0.0.0-20230412013349-d3d7625d8e1e/client/injection/kube/client/client.go:215:30: cannot use (*wrapClient)(nil) (value of type *wrapClient) as kubernetes.Interface value in variable declaration: *wrapClient does not implement kubernetes.Interface (missing method AdmissionregistrationV1alpha1)

Additional Info

I took a stab at updating k8s.io/* packages to v0.27.0, but I got stuck because./hack/update-codegen wasn't picking up the AdmissionregistrationV1alpha1 client for the injection clients (despite stdout saying it was being recognized). If anyone has ideas lmk!

[dprotaso]

Hey - we're still targeting v1.24 as per our release schedule. We'll be moving to v1.25 in the next few weeks.

Cannot build because the injection interface is missing the new

So these dynamic wrapped clients were introduced here - #2210 But they're a bit of a headache as you encountered because the codegen/interface need to be updated.

Since Knative isn't actually using this functionality I'm thinking we just purge this from knative.dev/pkg and downstream repos can just generate these clients themselves.

cc @pierDipi
cc @imjasonh since I think this was added because of tekton chains

[wlynch]

Hey - we're still targeting v1.24 as per our release schedule. We'll be moving to v1.25 in the next few weeks.

Anything that relies on knative.dev/pkg and has a transitive dependency using k8s 1.26+ will break because we aren't including the new k8s client interfaces (regardless if they're being used). Because of Go's minimum version selection this is getting harder to avoid as more dependencies update. 😢

Not looking to bump the minimum supported server version - I'm looking to add support for the new k8s client interface definition so the injection client can be forwards compatible.

So these dynamic wrapped clients were introduced here - #2210 But they're a bit of a headache as you encountered because the codegen/interface need to be updated.

I'm thinking since Knative isn't actually using this functionality I'm thinking we just purge this from knative.dev/pkg and downstream repos can just generate these clients themselves.

This is for the typed k8s client, which looks like has usage across knative. I don't think the dynamic client is affected by this.

[wlynch]

I see what you're saying now about the dynamic client - it's because that's what the wrapClient is used for.

An idea - what if we modify the codegen to do this:

type wrapClient struct {
+	kubernetes.Interface
	dyn dynamic.Interface
}

- var _ kubernetes.Interface = (*wrapClient)(nil)

This would let the client satisfy newer interface versions, even if Go mod used a newer client version. This has a downside of if you tried to access a newer client API then what knative supports it will panic, but we're already allowing panics for Discovery endpoints and as long as you're using existing API versions you shouldn't see a difference.

wdyt? 🤔

[pierDipi]

The additional methods called _expansions are also panic-ing with NYI (not yet implemented): https://github.com/knative/pkg/blob/main/client/injection/kube/client/client_expansion.go, so the panic issue is currently already present for new methods that we don't use

[pierDipi]

The downside is that instead of having a NYI panic, we will get a nil panic? which would make the error message a lot more cryptic?

[wlynch]

Played around with this a bit - the codegen wasn't working for me because the repo wasn't on GOPATH.

The interface embedding isn't going to work because it will still fail for interfaces that were removed in newer k8s client versions (I ran into issues with the deprecated PodSecurityPolicy interface since it was removed in newer versions).

[dprotaso]

@wlynch we removed the dynamic.Client backed kubernetes.Interface wrapper that was causing problems.

PR #2742

You should be able to go get knative.dev/pkg@latest and then bump your k8s libs to the version you want.