dhall-openapi: make metadata optional for "inner" k8s types
#2128
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
muff1nman
approved these changes
Jan 6, 2021
|
Thanks for reviewing @muff1nman. I'm conscious that this will lead to a breaking change in the dhall generated in dhall-kubernetes. Do you see this being a problem given release branches are not being used in that repository? |
|
@WillSewell: We don't have release branches in Either way, I think this is totally fine to merge |
Gabriella439
approved these changes
Jan 8, 2021
There was a problem hiding this comment.
I just cut a 5.0.0 release for dhall-kubernetes so this should be good to go
The `metadata` field is required for all top level resources according to https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#metadata. However for "inner" types this is not the case. For example `io.k8s.api.batch.v1beta1.JobTemplateSpec` has a `metadata` field, but it is not required. The example in the official k8s docs does not include this field: https://kubernetes.io/docs/concepts/workloads/controllers/cron-jobs/#example. I searched the kubernetes 1.17 OpenAPI spec for "inner" types that include a `metadata` field with: ``` curl -s https://raw.githubusercontent.com/kubernetes/kubernetes/release-1.17/api/openapi-spec/swagger.json | jq '.definitions | with_entries(select(.value.properties.apiVersion == null and .value.properties.metadata != null))' ``` It came up with: * io.k8s.api.batch.v1beta1.JobTemplateSpec * io.k8s.api.batch.v2alpha1.JobTemplateSpec * io.k8s.api.core.v1.PodTemplateSpec The pod spec template example in https://kubernetes.io/docs/concepts/workloads/pods/ also does not include a `metadata` field. This PR adds special cases to these types to make these fields optional.
WillSewell
force-pushed
the
openapi-optional-metadata
branch
from
84c06c2 to
10fc1ce
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
metadatafield is required for all top level resourcesaccording to
https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#metadata.
However for "inner" types this is not the case. For example
io.k8s.api.batch.v1beta1.JobTemplateSpechas ametadatafield,but it is not required. The example in the official k8s docs does
not include this field:
https://kubernetes.io/docs/concepts/workloads/controllers/cron-jobs/#example.
I searched the kubernetes 1.17 OpenAPI spec for "inner" types
that include a
metadatafield with:It came up with:
The pod spec template example in
https://kubernetes.io/docs/concepts/workloads/pods/
also does not include a
metadatafield.This PR adds special cases to these types to make these fields
optional.
In general I think a nicer solution would be
dhall-lang/dhall-kubernetes#8 (comment).