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

Replace the unsafe gvk to gvr conversion with restmapper. #3775

Merged
merged 1 commit into from
Nov 18, 2021

Conversation

absoludity
Copy link
Contributor

Signed-off-by: Michael Nelson minelson@vmware.com

Description of the change

Who would have thought converting from the GroupVersionKind of a yaml resource to the GroupVersionResource required by the API server would be so arduous?!

This PR replaces the unsafe conversion that was used temporarily, instead initializing a RESTMapper from the APIserver (using the service account) at startup and switching to use that at run-time for the conversion.

Follows on from #3767

Benefits

We don't depend on known-to-be-unsafe code.

Possible drawbacks

Applicable issues

Additional information

Comment on lines +67 to +70
// createRESTMapper returns a rest mapper configured with the APIs of the
// local k8s API server. This is used to convert between the GroupVersionKinds
// of the resource references to the GroupVersionResource used by the API server.
func createRESTMapper() (meta.RESTMapper, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is nuts... what an extra amount of coding for performing just a conversion...I guess I lack some K8s internal knowledge here, but I don't know why we need this kind of runtime information.
Thanks a lot for coming up with this solution (I don't know if the restmapper might be useful for additional things as well).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It comes down to the fact that a client (kubectl or in our case our client-go client) cannot safely make assumptions about the mapping between an object (the yaml manifest with the GroupVersionKind) and the resource endpoint in the API. The only safe way to determine the mapping is to query the api server for the API information, which is what this now does.

// For testing, define a kindToResource converter that doesn't require
// a rest mapper.
kindToResource: func(mapper meta.RESTMapper, gvk schema.GroupVersionKind) (schema.GroupVersionResource, error) {
gvr, _ := meta.UnsafeGuessKindToResource(gvk)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, at least the unsafe thing is still useful :P

Base automatically changed from 3403-GetResources-3 to master November 17, 2021 22:32
Signed-off-by: Michael Nelson <minelson@vmware.com>
@absoludity absoludity merged commit 1833ef9 into master Nov 18, 2021
@absoludity absoludity deleted the 3403-GetResources-4 branch November 18, 2021 00:59
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

Successfully merging this pull request may close these issues.

2 participants