-
Notifications
You must be signed in to change notification settings - Fork 40.9k
Introduce OpenAPI format support for k8s-short-name and k8s-long-name #132504
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
Conversation
This issue is currently awaiting triage. If a SIG or subproject determines this is a relevant issue, they will accept it by applying the The Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
/assign @liggitt @dims /assign @lalitc375 cc @thockin |
@jpbetz: GitHub didn't allow me to assign the following users: lalitc375. Note that only kubernetes members with read permissions, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
@@ -50,7 +50,7 @@ import ( | |||
|
|||
var ( | |||
printerColumnDatatypes = sets.NewString("integer", "number", "string", "boolean", "date") | |||
customResourceColumnDefinitionFormats = sets.NewString("int32", "int64", "float", "double", "byte", "date", "date-time", "password") | |||
customResourceColumnDefinitionFormats = sets.NewString("int32", "int64", "float", "double", "byte", "date", "date-time", "password", "k8s-short-name", "k8s-long-name") |
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.
What is the format on column definitions used for?
The API doc isn't very clear and also references a name
format that doesn't appear to be allowed here? Are these columns appended to the standard name
column? If so, it's pretty weird for these new name formats to not be usable on the name column (and I don't think we can change name
on the name column since clients use it to recognize the name), and for the name
printer column format to have no openapi format meaning :-/
// format is an optional OpenAPI type definition for this column. The 'name' format is applied
// to the primary identifier column to assist in clients identifying column is the resource name.
// See https://github.com/OAI/OpenAPI-Specification/blob/master/versions/2.0.md#data-types for details.
// +optional
Format string `json:"format,omitempty" protobuf:"bytes,3,opt,name=format"`
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.
What is the format on column definitions used for?
I think I misunderstood this. It looks like customResourceColumnDefinitionFormats
enumerates the formats a printer column can be explicitly declared as.
I has mistaken this to be a filter on what fields are allowed to used as printer columns.
To fix, I've dropped the k8s names from this list and added tests showing that fields using these formats can be used as string printer columns.
I believe this is consistent with the existing behavior and doesn't add any new wrinkles. I do find the name
format odd here since this format field otherwise makes consistent use of OpenAPI names. I'll think about that more.
This PR may require API review. If so, when the changes are ready, complete the pre-review checklist and request an API review. Status of requested reviews is tracked in the API Review project. |
@jpbetz: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
lgtm, will let others tagged take a look |
I looked at the kube-openapi changes and have no comments. /lgtm |
LGTM label has been added. Git tree hash: d0483d037b29cfe50920461b01d919b999f3709b
|
/approve |
Changelog nit -Introduces OpenAPI format support for `k8s-short-name` and `k8s-long-name`.
+Introduced OpenAPI format support for `k8s-short-name` and `k8s-long-name` in CustomResourceDefinition schemas. (is that right?) |
Yes, that's a good improvement. I've updated the release note to match. Thx! |
What type of PR is this?
/kind feature
What this PR does / why we need it:
Kubernetes name formats are used throughout the Kubernetes native types (Pod, Deployment, ...). This enables CRDs to use the same formats in OpenAPI validations.
For example,
Special notes for your reviewer:
Does this PR introduce a user-facing change?