Use OpenAPI to validate ImageVolumeSource.Reference #4308
Conversation
This version adds Title markers and accepts "optionalOldSelf" of Kubernetes 1.30 on XValidation markers. See: https://github.com/kubernetes-sigs/controller-tools/releases/v0.18.0
| k8s.io/utils v0.0.0-20241104100929-3ea5e8cea738 // indirect | ||
| sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.31.2 // indirect | ||
| sigs.k8s.io/controller-tools v0.17.3 // indirect | ||
| sigs.k8s.io/controller-tools v0.18.0 // indirect |
There was a problem hiding this comment.
🤔 v0.18 requires k8s.io/.. v0.33, but it wasn't bundled in the k8s.io updates last month: #4248
I don't know why!
| def merge(stream): reduce stream as $i ({}; . * $i); | ||
|
|
||
| # https://pkg.go.dev/k8s.io/api/core/v1#ImageVolumeSource | ||
| reduce paths(try .title == "$corev1.ImageVolumeSource") as $path (.; |
There was a problem hiding this comment.
📝 This may very well be "too clever." I'm curious what others think.
There was a problem hiding this comment.
Setting a title field on our CRD to add some validation rules and then removing the title field -- is that the part you're thinking might be too clever?
Seems reasonable to me, with the caveat that having the CRD be assembled from kubebuilder annotations and then post-process functions might make it a little harder to know easily where some element is coming from. Maybe that's a documentation issue, e.g., when we add the title in the struct, we could link to or reference this jq script at this point?
There was a problem hiding this comment.
Setting a
titlefield on our CRD to add some validation rules and then removing thetitlefield -- is that the part you're thinking might be too clever?
Yeah. Smuggling information through a marker that does something else.
Seems reasonable to me, with the caveat that having the CRD be assembled from kubebuilder annotations and then post-process functions might make it a little harder to know easily where some element is coming from. Maybe that's a documentation issue, e.g., when we add the title in the struct, we could link to or reference this jq script at this point?
A comment on the marker is better than none. I'll add one.
There was a problem hiding this comment.
A comment on the marker is better than none. I'll add one.
Done.
cbandy
force-pushed
the
openapi-image-source
branch
from
909426e to
c71315f
Compare
cbandy
changed the title
| @@ -1,9 +1,8 @@ | |||
| --- | |||
| # controller-gen.kubebuilder.io/version: v0.18.0 | |||
There was a problem hiding this comment.
I'm curious to hear your thinking behind this change
There was a problem hiding this comment.
This information is minimally interesting during development, but AFAIK has no use after that. As an annotation, it is deployed with the CRD but serves no purpose.
My impression has been that the generator is using metadata.annotations to push "stuff" through code that can only handle K8s objects.
|
|
||
| // Turn top-level strings that start with octothorpe U+0023 into YAML comments by removing their quotes. | ||
| yamlData := need(yaml.Marshal(v)) | ||
| yamlData = regexp.MustCompile(`(?m)^'(#[^']*)'(.*)$`).ReplaceAll(yamlData, []byte("$1$2")) |
There was a problem hiding this comment.
And we can't just capture the whole line as a single capture group because you want to exclude the quotes, yeah?
There was a problem hiding this comment.
🤔 It doesn't seem like I need the second capture... The line should end just after the quoted string, right? 🦆
I'll check this tomorrow.
There was a problem hiding this comment.
And we can't just capture the whole line as a single capture group because you want to exclude the quotes, yeah?
Right. This finds lines starting with '# and removes a pair of '. This is the transformation:
-'# controller-gen.kubebuilder.io/version': v0.18.0
+# controller-gen.kubebuilder.io/version: v0.18.0🤔 It doesn't seem like I need the second capture...
I forgot that the quotes are around only the key of the flow mapping. The second capture is for the remainder of the line, if any.
OpenAPI validation does not count toward the CRD XValidation CEL budget.
cbandy
force-pushed
the
openapi-image-source
branch
from
c71315f to
9b7ac1b
Compare
| // More info: https://kubernetes.io/docs/concepts/storage/volumes#image | ||
| // --- | ||
| // https://docs.k8s.io/concepts/storage/volumes#image | ||
| // Use "title" to add more validation in [internal/crd/post-process.jq]. |
Checklist:
Type of Changes:
What is the current behavior (link to any open issues here)?
We cannot add validation markers inside upstream types, so we have to use CEL validation from outside the type.
What is the new behavior (if this is a feature change)?
We can use the
+kubebuilder:titlemarker to add identifiers to CRD schemas, and manipulate those aftercontroller-genruns.