Unable to deserialize duplicated resource names

[schrodit]

Describe the bug

The generic client is unable to correctly deserialize resources correctly that overlap the kind name. E.g. querying a custom resource of kind Probe with group monitoring.coreos.com/v1 results in an empty class of type V1Probe. This is because the serializer does not take the group into account while deserializing.

Client Version e.g. 1.1.2

Server Version e.g. 1.30.1

To Reproduce

1. Create a custom resource with an overlapping name (e.g. install the prometheus operator).
2. create an instance
3. try to query that instance: see code snippet

Expected behavior

The client should return the plane K8s object when the type is unknown.

Example Code

const kc = new KubeConfig();
kc.loadFromDefault();
const client = new KubernetesObjectApi(kc);
const p = await client.read({
    apiVersion: 'monitoring.coreos.com/v1',
    kind: 'Probe',
    metadata: {
        name: 'my-probe',
        namespace: 'default',
    },
});

Environment (please complete the following information):

• OS: Linux
• Node.js version: 20

Additional context

I think to correctly fix this, the ObjectSerializer has to use the whole unique name of a resource (group version and kind) to identify an object. The group information is currently not available so I guess the converter has to be adapted to provide that info.

[brendandburns]

Happy to take PRs to address this.

[schrodit]

@brendandburns do you know if there is a similar info like the register.go files we have in the go client?

[schrodit]

I started working in a fix for this. But it requires a change in the generator to add the group and version information to every type.

See the related PR: https://github.com/kubernetes-client/gen/pull/279

[brendandburns]

fwiw, I think instead of this generator changes it might be useful to do something similar to what we did in Java:

https://github.com/kubernetes-client/java/blob/043f86810ce7c7a532ea46c5fe43d67f21dbadc2/util/src/main/java/io/kubernetes/client/util/ModelMapper.java#L52

We can use the discovery endpoint on the server to dynamically discover and register GVK+Type into the ModelMapper.

[schrodit]

@brendandburns

https://github.com/kubernetes-client/java/blob/043f86810ce7c7a532ea46c5fe43d67f21dbadc2/util/src/main/java/io/kubernetes/client/util/ModelMapper.java#L52

I'm not sure if that works as expected. If I read the code correctly, it does read the classes from io.kubernetes.client.openapi.models (e.g. V1DeamoSet). Then the apiGroup is parsed from the class name in getApiGroup. But the classnames do not include the groupname (e.g. V1DeamonSet) so we always end up in the else case where the group is set to null. This leads to all classes being in the core group. The class should be named AppsV1DaemonSet to properly work.

sidenote: some classes like CoreV1Endpoint include the group. But the majority does not include the group name.

Also the java implementation uses some hardcoded mapping (in initApiGroupMap) This mapping has to be manually maintained and is therefore error prone. Although it might be ok as the core groups do not change that often.

We can use the discovery endpoint on the server to dynamically discover and register GVK+Type into the ModelMapper.

With that issue above, I don't think that the discovery solves that issue fully. In case there is a resource (either from a CRD or a apiextension) with a duplicated kind we end up with a potential wrong assignment? E.g. if there is a custom resource with kind Deployment but of apiVersion my-company.com/v1.

Also the discovery can be skipped (https://github.com/kubernetes-client/java/blob/master/extended/src/main/java/io/kubernetes/client/extended/kubectl/Kubectl.java#L232) and I guess we should try to solve the issue for both cases?

It was some time ago that I was working with java so I might have missed something here?

[brendandburns]

@schrodit ah, you are right, the code generator we used used to place the group in the class name (e.g. https://github.com/kubernetes-client/java/tree/release-11/kubernetes/src/main/java/io/kubernetes/client/openapi/models) but we switched generators and it looks like it doesn't do that anymore :(

[brendandburns]

@schrodit while we work out the potential generator fixes for this, why don't we add an equivalent of the ModelMapper to the code so that you can at least statically register mappings to solve this particular issue. We're going to need it one way or the other anyway since Typescript doesn't really have reflection.

[mstruebing]

The PR says it's partly fixes this issue - should we reopen it? cc @brendandburns @schrodit

[schrodit]

Yes we should reopen it until it's completely fixed

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle stale
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle rotten
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Reopen this issue with /reopen
• Mark this issue as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close not-planned

[k8s-ci-robot]

@k8s-triage-robot: Closing this issue, marking it as "Not Planned".

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Reopen this issue with /reopen
• Mark this issue as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close not-planned

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[mstruebing]

/reopen

This is not fixed yet.

[k8s-ci-robot]

@mstruebing: Reopened this issue.

In response to this:

/reopen

This is not fixed yet.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[brendandburns]

fwiw, I think that #2688 is a partial fix for this.