Skip to content

Adjust for structural schema KEP #216

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

Closed
DirectXMan12 opened this issue May 30, 2019 · 18 comments
Closed

Adjust for structural schema KEP #216

DirectXMan12 opened this issue May 30, 2019 · 18 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@DirectXMan12
Copy link
Contributor

A new kep was introduces to restrict how we generate OpenAPI validation for CRDs. It has a number of special extensions that we'll want to support, plus restrictions on validating objectmeta. We'll need some specially handling here, since people might want a full metadata openapi spec for other reasons.

See also https://github.com/kubernetes/enhancements/blob/master/keps/sig-api-machinery/20190425-structural-openapi.md

@DirectXMan12
Copy link
Contributor Author

/kind feature

@k8s-ci-robot k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label May 30, 2019
@DirectXMan12
Copy link
Contributor Author

cc @sttts

@sttts
Copy link
Contributor

sttts commented May 31, 2019

since people might want a full metadata openapi spec for other reasons.

Which are those reasons?

Note that the spec is most probably incomplete when defined by the developer because ObjectMeta is defined by the cluster at hand. In contrast to anything else in a CRD, the ObjectMeta life cycle is not the one the operator builder controls. When the cluster adds a meta feature (server side apply, more powerful GC, etc.), every resource will inherit it.

@DirectXMan12
Copy link
Contributor Author

We have a contributor who initially wanted to use this code for generating openapi specs to be consumed by editor tooling.

@jimmidyson
Copy link
Member

Any thoughts on how to tackle this @DirectXMan12?

@pires
Copy link

pires commented Jun 28, 2019

@vincepri @ncdc can you describe better what was the problem embedded CRD ObjectMeta was raising? Seems to be related to this.

@sttts
Copy link
Contributor

sttts commented Jun 28, 2019

@pires With structural schema we have x-kubernetes-embedded-resource: true which does everything around ObjectMeta validation.

And the published spec will have a $ref to the ObjectMeta schema such that editors or other tooling have full spec knowledge.

@jimmidyson
Copy link
Member

I've worked around this with a kustomize patch of the crd manifest to only include name and generateName in ObjectMeta properties, but still with ObjectMeta in original struct (nothing gets generated if this isn't present). We could just do that on generation of validation spec, just restrict to name and generateName properties in metadata.

@sttts
Copy link
Contributor

sttts commented Jun 28, 2019

@jimmidyson you will be able to have x-kubernetes-embedded-resource: true and in parallel your custom restrictions on name and generateName.

@pires
Copy link

pires commented Jun 28, 2019

@jimmidyson @sttts IIUC the approach Cluster API folks picked up was to remove ObjectMeta and explicitly specify the attributes Jimmi mentioned. As I was reading the updates for CRD in 1.15, including x-kubernetes-embedded-resource: true, I figured it could be a better solution. Hopefully, I'm not wrong 🤞

@sttts
Copy link
Contributor

sttts commented Jun 28, 2019

@pires that will verify TypeMeta and ObjectMeta automatically yes. Plus whatever you add to that for additional constraints.

@jimmidyson
Copy link
Member

@sttts I thought that was for types embedded in eg spec, not for the top level crd? Do you have an example to hand?

@sttts
Copy link
Contributor

sttts commented Jun 28, 2019

@jimmidyson right. For top-level it is automatic. And you are able to (only) constrain name and generateName.

I guess it is more of a kubebuilder question how to overlay arbitrary custom constraints.

@DirectXMan12
Copy link
Contributor Author

So, one option is to use an override, which will handle explicit uses of objectmeta. We'll override it to put x-kubernetes-embedded-resource in place.

@sttts presumably we need to strip that for the top-level objectmeta? We can do that with a special case.

@DirectXMan12
Copy link
Contributor Author

as for overlaying custom constraints, that'll take a bit more thought. One option would be to have a special marker for roughly the equivalent of ref, and then we'd include that in an AllOf. That seems like the easiest way without significantly changing things or forcing you to write an entire embedded jsonschema spec.

I'm open to alternatives, though.

@sttts
Copy link
Contributor

sttts commented Jun 29, 2019

presumably we need to strip that for the top-level objectmeta? We can do that with a special case.

Yes, top-level does not have it.

and then we'd include that in an AllOf.

That's what I would have expected. Even if we don't force anybody into the full OpenAPI spec (using some syntax), I would appreciate it very much if there were at least a generic, raw OpenAPI marker as well. Sometimes it will be easier to just write down what you want than to find a way with markers on types.

@DirectXMan12
Copy link
Contributor Author

See also #266

@DirectXMan12
Copy link
Contributor Author

Still need a marker for embedded-resource, but I'm calling this done for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

No branches or pull requests

5 participants