-
Notifications
You must be signed in to change notification settings - Fork 40.6k
API reference has incorrect response schema for many delete calls #30537
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
Comments
Delete returns inconsistently across types. Too late to do anything about this? cc @bgrant0607 @kubernetes/api-review-team |
@bgrant0607 it would be nice to sum up that quirk in a note somewhere, since it's true of all the API calls. However, this issue is about calls where the documentation specifies an incorrect response schema for the successful responses, and that successful delete response's schemas are inconsistent from resource type to resource type. |
From my experimenting:
|
I think REST dictates that for a synchronous delete, the body for a 200 response should be the entity deleted. So the Pod API is an example of good RESTful behavior, and the CertificateSigningRequest API is incorrect. EDIT: Either way the documentation does not reflect the real behavior. |
@randalloveson Yes, unfortunately there are many quirks. Please add this to #25716. If you're interested in helping, check out SIG API machinery. |
I added this to the umbrella issue. Sounds like pod is doing this correctly, all resources should be going through the same code so it's a bit weird that it's different for other resources. |
Actually the swagger spec is wrong in this regard and that is IMHO an even worse situation than broken documentation. See https://github.com/kubernetes/kubernetes/blob/master/api/swagger-spec/v1.json#L7861 for the Pod example of wrong return type. What should I expect when this is going to be resolved? The client binding I am using is completely generated from swagger and hence it is wrong. |
Issues go stale after 30d of inactivity. Prevent issues from auto-closing with an If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or |
Stale issues rot after 30d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or |
@mbohlool is this still an issue? |
Rotten issues close after 30d of inactivity. Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
@bgrant0607 The problem still exists because our openapi v2 spec cannot represent |
The API reference seems to assume a response schema of unversioned.Status for all delete calls, individual or collection. In reality, the response schema differers from resource to resource, and for some it is correct, but for many the response schema is actually an individual or list of the given resource. For example, the delete call for the Pod resource returns a Pod and a PodList for the individual and collection delete calls, respectively.
Short term, we should update the docs to reflect reality. Long term, wouldn't it make sense to unify the API so all the resources behave the same? Currently it is left up to the resource implemenation:
https://github.com/kubernetes/kubernetes/blob/master/pkg/apiserver/resthandler.go#L807-L827
The text was updated successfully, but these errors were encountered: