-
-
Notifications
You must be signed in to change notification settings - Fork 319
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
Feature: Allow to pass selectableFields for CRD definition #1605
Conversation
Signed-off-by: Danil-Grigorev <[email protected]>
Signed-off-by: Danil-Grigorev <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1605 +/- ##
=======================================
+ Coverage 75.2% 75.2% +0.1%
=======================================
Files 82 82
Lines 7335 7342 +7
=======================================
+ Hits 5514 5521 +7
Misses 1821 1821
|
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.
Looks great. Nice KEP also, had missed this! Maybe add a quick markdown header for it in the lib.rs
file so people can discover it?
Signed-off-by: Danil-Grigorev <[email protected]>
Signed-off-by: Danil Grigorev <[email protected]>
Signed-off-by: Danil-Grigorev <[email protected]>
705e04c
to
9ed7f88
Compare
Looks like CI failures are unrelated to the PR. |
indeed. dunno what's up with the rustls build atm. C library type errors :| |
I think this breaks the Kubernetes version guarantee - requires openapi to upgraded to 1.31 to compile kube-rs as a dependency. @clux |
oops, good spot. we need to version gate it! |
can we rollback this until it's version gate? can't build main as dependency :( |
It should not break CRD generation for pre 1.31 clusters if the selectable is not specified @aviramha |
I think there's some ambiguity whether the MK8SV is runtime or compilation, in my case, if we use "earliest" on k8s open api we can't compile this commit. |
Ah, I suppose I see where your problem is https://github.com/kube-rs/kube/pull/1605/files#diff-6750cfa2314830da21402810282fd1a9a9f270c217c393e03a2c04e1b2735d10R450, makes sense. |
i think you can use a https://docs.rs/k8s-openapi/latest/k8s_openapi/macro.k8s_if_ge_1_30.html around an if that factors out the @aviramha: a bit off-topic, but is there any reason you don't use latest? you generally don't lose anything by going newer. |
It's pretty much the first dependency we added, and earliest felt safest. I think it still makes sense, so devs won't try to use API that isn't compatible with older clusters by accident. |
Thanks for the pointer @clux, was about to ask how the version gating done elsewhere, will prepare a PR in a moment :) |
also had a quick look, i think this will do it: #1609 |
Motivation
KEP 4358 allows to define new field -
selectableFields
within the CRD schema, enabled by default (beta) in 1.31.Solution
Expose the
selectable
key inkube()
macro, allowing to provide a set of values into generatedCRD
.Example: