Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistent deserialization behaviour when using KubernetesObjectApi.list vs KubernetesObjectApi.get #2018

Open
LFrobeen opened this issue Nov 8, 2024 · 2 comments

Comments

@LFrobeen
Copy link

LFrobeen commented Nov 8, 2024

Describe the bug
The data returned by KubernetesObjectApi.list() does not deserialize timestamps in the items[].metadata field which makes the results inconsistent with KubernetesObjectApi.get(). As a result, the returned manifests cannot directly be used with the KubernetesObjectApi.patch() method because the string-based timestamps will be rejected.

This issue might be related to the changes in #1695.

Client Version
0.22.1

Server Version
v1.30.5

Steps to Reproduce & Example Code

The following code snippet reproduces the problem. The code tries to list VirtualServices and thus requires istio to be installed. However, the problem can be reproduced with any other resource kind that is not known to the kubernetes/client-node package.

import { KubeConfig, KubernetesObjectApi } from '@kubernetes/client-node';

const TEST_RESOURCES = {
  VirtualService: {
    apiVersion: 'networking.istio.io/v1',
    kind: 'VirtualService',
    namespace: 'default',
    name: 'my-service',
  },
  Pod: {
    apiVersion: 'v1',
    kind: 'Pod',
    namespace: 'default',
    name: 'my-pod',
  },
};

(async () => {
  const kc = new KubeConfig();
  kc.loadFromDefault();

  const client = KubernetesObjectApi.makeApiClient(kc);

  // const { apiVersion, kind, name, namespace } = TEST_RESOURCES.Pod; // known resource kinds work as expected
  const { apiVersion, kind, name, namespace } = TEST_RESOURCES.VirtualService; // "unknown" resource kinds have inconsistent timestamp types

  await client.list(apiVersion, kind, namespace);

  const pod1 = await client
    .list(apiVersion, kind, namespace)
    .then((response) => response.body.items.find((p) => p.metadata?.name === name)!);

  const pod2 = await client
    .read({ apiVersion, kind, metadata: { namespace, name } })
    .then((response) => response.body);

  // Types of timestamps will be different when listing VirtualServices
  console.log(pod1.metadata?.creationTimestamp, pod1.metadata?.creationTimestamp?.constructor.name);
  console.log(pod2.metadata?.creationTimestamp, pod2.metadata?.creationTimestamp?.constructor.name);

  console.log();
})();

Expected behavior
The above code should print the same line twice, e.g.:

2023-07-19T12:59:47.000Z Date
2023-07-19T12:59:47.000Z Date

However, when listing VirtualServices (or any other resource that isn't known to this package) the timestamp types in the metadata are different, e.g.:

2023-07-19T12:59:47Z String
2023-07-19T12:59:47.000Z Date

Environment:
Probably happens on any environment. For completeness, this is my test environment:

  • OS: MacOS
  • NodeJS Version: 18
  • Cloud runtime: GKE
@LFrobeen LFrobeen changed the title Inconsistent deserializing when using KubernetesObjectApi.list vs KubernetesObjectApi.get Inconsistent deserialization behaviour when using KubernetesObjectApi.list vs KubernetesObjectApi.get Nov 8, 2024
@brendandburns
Copy link
Contributor

I think that this is because we aren't passing type information down here:

https://github.com/kubernetes-client/javascript/blob/master/src/object.ts#L478

requestPromise takes an optional type parameter, but we're not passing it.

If you want to investigate further, we'd be happy to take PRs to fix this.

@LFrobeen
Copy link
Author

LFrobeen commented Dec 2, 2024

Hi @brendandburns I might try to tackle this in the coming days.

In my project, I've fixed the issue by checking whether the package's list implementation returns deserialized objects or just plain objects. In the latter case I'm running the KubernetesObject deserialization logic on the elements of the items array manually.

I think the problem is basically that the KubernetesObjectSerializer has a fallback for unknown response types which handles read responses just fine, but does not have special logic for list responses.

I have noticed that the implementation is aware of some list response types, e.g. there is V1PodList.

My approach would be to define a generic list type, e.g. V1KubernetesObjectList (like this KubernetesObject type) that specifies the type of items to be a KubernetesObject array.

If I understand correctly, passing an explicit / generic type parameter to requestPromise would override the deserialization behaviour for known resource types like V1Pod in a V1PodList. Would it make sense to add an additional parameter like typeFallback to requestPromise and potentially KubernetesObjectSerializer that would be used if there is no known type for a apiVersion/kind combination?

That way I'm hoping to keep backwards compatibility for the known list types, and ensure unknown response types are treated consistently in the list and get implementations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants