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

[release-1.x] getPreferredMediaType doesn't handle empty arrays resulting in None of the given media types are supported: #1075

Closed
per2plex opened this issue May 4, 2023 · 6 comments

Comments

@per2plex
Copy link

per2plex commented May 4, 2023

Describe the bug

getPreferredMediaType only checks if mediaTypes is falsy and not if it's an empty array:
https://github.com/kubernetes-client/javascript/blob/release-1.x/src/gen/models/ObjectSerializer.ts#L1745-L1749

The API provides an empty array, e.g:
https://github.com/kubernetes-client/javascript/blob/release-1.x/src/gen/apis/CoreV1Api.ts#L2779

If the TypeScript types can be trusted, mediaTypes should never be undefined anyway.

I didn't know where to provide a fix, because those files are generated?

Might be related to #893

Client Version
1.0.0-rc2

Server Version
1.24.10

To Reproduce
Steps to reproduce the behavior:

  • Try creating a namespace via API
  • Throws None of the given media types are supported:

Expected behavior
Not throw None of the given media types are supported:

Example Code

const kc = new k8s.KubeConfig()

kc.loadFromDefault()

const k8sApi = kc.makeApiClient(k8s.CoreV1Api)

await k8sApi.createNamespace({
  body: {
    metadata: {
      name: 'my-namespace',
    },
  },
})
@felix-gohla
Copy link

Maybe this is rather a problem with either a) the OpenAPI specs over at the Kubernetes repo or b) in the OpenAPI generator.
The place to fix for the latter one would be here (I did not find any issue regarding that in the OpenAPI generator repo):

https://github.com/OpenAPITools/openapi-generator/blob/cc620d8ba22feedff4a2a4e4ef7c3a69a98eacff/modules/openapi-generator/src/main/resources/typescript/model/ObjectSerializer.mustache#L198-L201

Another potential fix was in the kubernetes-client/gen repo by preprocessing the swagger documentation replacing all "consumes": ["*/*"] with "consumes": ["application/json"]. But I think fixing the generator would be better.

@davidgamero
Copy link
Contributor

after taking a look, I think you're correct. The current generated code doesn't respect the */* media type as a pattern.
This is most likely best addressed with an upstream PR to add support.

I can take a shot at it this week. I don't know if it makes more sense to check explicitly for an allow all pattern or to use the existing default of an empty array (this could end up hiding error depending on the implementation`

@clintonmedbery
Copy link
Contributor

Any movement on this @davidgamero? I was going to give it a shot tomorrow.

@davidgamero
Copy link
Contributor

I've tried to find a simple solution in the upstream MIME type code, but it got tricky in the Java/outside typescript generator level. It may be worth adding a fix substituting the full list of MIME values in the python we use to pull the swagger to get around this, but Id love your thoughts when you take a look tomorrow!

@brendandburns
Copy link
Contributor

Closed by #1127, I will try to cut a new release today.

@felix-gohla
Copy link

@brendandburns Could you please release another version when you get a chance, so that we can test the changes? 😊

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

5 participants