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

Drop implicit kind->resource conversion #284

Closed
4 tasks
nightkr opened this issue Jul 23, 2020 · 7 comments · Fixed by #556
Closed
4 tasks

Drop implicit kind->resource conversion #284

nightkr opened this issue Jul 23, 2020 · 7 comments · Fixed by #556
Labels
api Api abstraction related bug Something isn't working

Comments

@nightkr
Copy link
Member

nightkr commented Jul 23, 2020

Motivated by #14, but the current behaviour mostly works by accident anyway.

  • Add RESOURCE_NAME const to k8s_openapi::Resource (and add it to their generated object types)
  • Replace kube::Resource::kind with resource_name
  • Generate RESOURCE_NAME from the CustomResource deriver
  • Drop inflector runtime dependency
@nightkr nightkr added api Api abstraction related bug Something isn't working labels Jul 23, 2020
@clux
Copy link
Member

clux commented Jul 23, 2020

Totally! If it's possible to grab the actual values without relying on pluralization then that would be great. I just assumed the information was not available in the openapi spec by looking at the underlying types; e.g. Pod and introduced the dep in a bit of a hurry.

Wonder how k8s-openapi does this.

@nightkr
Copy link
Member Author

nightkr commented Jul 23, 2020

You can get the mapping at runtime from the APIResourceList endpoints, which seems to be what kubectl is doing.

image

Of course, that isn't great for our use-case (wouldn't want to need a running K8s cluster to be able to compile k8s_openapi.. :P). Haven't looked through the OpenAPI stuff yet to see whether anything similar is available there.

@nightkr
Copy link
Member Author

nightkr commented Jul 23, 2020

Looks like the OpenAPI does not include the resource name. On the other hand, I don't think it would be too bad to use a heurestic like "find the last path item of the list endpoint`.

@clux
Copy link
Member

clux commented Aug 10, 2020

Found another case of our implicit naming not working today when doing #305. CustomResource assumes PascalCase + singular kinds, but even kubernetes fails to adhere to this: APIService, CSINode, NodeMetrics, Endpoints (as well as numerous CRDs that do not have enforcement (details).

Ended up removing case/plural pedantry on the dynamic api to a mere debug check. Which is fine, dynamically created resources work if we aren't being strict. But THIS issue means that some of the objects noted in that PR might not work with Api. Thankfully, only found Endpoints (badly pluralised) and APIResource/APIService (bad pascal) as NATIVE failure cases when listing all available resource types (though all of istio's crds were also in violation..) with the new dynamic_api example.

Not even sure excess pluralisation in Endpoints is an actual failure case either. to_plural should be idempotent.

Can at least confirm that the required data is there on the api for all cases (albeit awkwardly split between two api calls). Hopefully there's a way we can get the data in the trait in k8s-openapi without using any api though. It's not like it's generated from a live cluster, and Arnav is creating the urls with plural somehow, the data must be there.

@clux
Copy link
Member

clux commented Dec 26, 2020

There's no easy way of getting the plural as it stands. It happens to be in the urls of the schemas as noted in Arnavion/k8s-openapi#73 (comment). If the k8s-openapi issue resolves, we can progress here, otherwise marking as awaiting upstream work.

@clux clux added the blocked awaiting upstream work label Dec 26, 2020
@kazk kazk mentioned this issue Mar 19, 2021
@clux
Copy link
Member

clux commented Mar 29, 2021

This is less of an issue now that we maintain a pluralisation fn that handles the few non-conformant cases.

All other entry ways no longer need to do this implicitly.

  • api-discovery will automatically deduce the plural
  • kube-derive optionally allow #[kube(plural = "fooz")]

@MikailBag
Copy link
Contributor

MikailBag commented Apr 7, 2021

I've sent Arnavion/k8s-openapi#88 which adds resource name to the k8s-openapi::Resource trait

@clux clux removed the blocked awaiting upstream work label Jun 16, 2021
@clux clux linked a pull request Jun 16, 2021 that will close this issue
@clux clux closed this as completed in #556 Jun 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Api abstraction related bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants