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
ExtractItems for unstructured apply configurations #103564
Conversation
/triage accepted |
dd58658
to
d90110a
Compare
3f95f10
to
d45679c
Compare
@apelisse or @lavalamp do either of you have thoughts on where to move Currently it is internal to This does not work though, because |
Actually, I'm thinking |
I think it makes sense in apimachinery, I like |
/test pull-kubernetes-integration |
@@ -5,6 +5,7 @@ rules: | |||
- selectorRegexp: k8s[.]io/kube-openapi/ | |||
allowedPrefixes: | |||
- k8s.io/kube-openapi/pkg/util/proto | |||
- k8s.io/kube-openapi/pkg/schemaconv |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure why I have to do this.
The only change I made to imports was to make client-go
import kube-openapi
, but if I don't add this verify import-boss
will fail with this error for various packages of kubeadm
:
errors in package "k8s.io/kubernetes/cmd/kubeadm/app/util/etcd":
the following imports did not match any allowed prefix:
k8s.io/kube-openapi/pkg/schemaconv
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kubeadm imports client-go, so probably the import-restriction tooling is finding it as an indirect dependency.
but kubeadm does not use k8s.io/kube-openapi directly:
https://github.com/kubernetes/kubernetes/search?q=path%3A%2Fcmd%2Fkubeadm+%22kube-openapi%22
/approve
for kubeadm, but haven't reviewed the other changes.
@@ -67,6 +67,7 @@ | |||
- k8s.io/apimachinery | |||
- k8s.io/client-go | |||
- k8s.io/klog | |||
- k8s.io/kube-openapi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sttts, can you take a look at this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
Looks good to me, thanks. I think apimachinery
is a good place to put this, but I'm not sure about the dependency thing.
Ya, it makes sense to me that we need to add I was more confused about why I had to do it in |
I'm surprised this is the first use of kube-openapi from within client-go. /approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kevindelgado, lavalamp, neolit123 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/kind feature |
What type of PR is this?
/kind feature
What this PR does / why we need it:
Enable running fieldmanager "ExtractItems" on unstructured objects. Currently
ExtractItems
can only be called by typed clients, and extracted into typed apply configurations. This lets you extract any arbitrary object into an unstructured apply configuration (i.e. an `unstructured.Unstructured) which enables extracting CRDs.In order to call
ExtractItems
for a given gvk we need thetyped.ParseableType
for that gvk. For builtins this exists in a large static file. For CRDs we need to dynamically generate thetyped.ParseableType
We do this by dynamically downloading the OpenAPISpec, converting it into a GVKParser, and resolving the typed.ParseableType from it for the given gvk.
To prevent having to download the OpenAPISpec and and generate the GVKParser every time we want to extract a CRD, we've implemented a caching mechanism to only download the open api spec and generate the parser at most every minute.
Does this PR introduce a user-facing change?