Allow empty body in POST /commit again
#45550
Merged
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
The internal Client request methods which accept an object as a body use nil to signal that the request should not have a body. But it is easy to accidentally pass a typed-nil value as the object, e.g. if the object comes from a function argument or struct field of a concrete type. The result is that these requests will, surprisingly, have a JSON body of `null`. Treat typed-nil pointers the same as untyped nils for the purposes of determining whether or not the request should include a body. Stop assuming that POST requests should always have a body. POST /commit does not require a body, for example. Signed-off-by: Cory Snider <csnider@mirantis.com>
The error returned by DecodeConfig was changed in b6d58d7 and caused this to regress. Allow empty request bodies for this endpoint once again. Signed-off-by: Cory Snider <csnider@mirantis.com>
🙈😂 |
thaJeztah
approved these changes
May 18, 2023
neersighted
approved these changes
May 18, 2023
This was referenced May 18, 2023
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.
- What I did
Fixed a regression in the
POST /commitAPI endpoint which made it an error to not include a JSON body in the request. This regression went unnoticed because a bug in the client caused it to always send a request body ofnullwhen
options.Config == nil.- How I did it
I changed
runconfig.loadJSONto return an error which wraps the error from the JSON decoder so that the caller can once again test whether the error isio.EOF.I modified the client to treat typed-nil request objects the same as untyped nil: a signal that the request should not have a body. This may be a controversial change as it will break client compatibility with buggy daemons, though I feel it is necessary for the long-term health of the project. The regression snuck in because the quirks of the client let it go unnoticed; making the client less quirky forces us to not become dependent on those quirks. (I also want to see what else a full CI run with this client change reveals.)
- How to verify it
Existing integration tests fail after the first commit, and pass after the second.
- Description for the changelog
POST /commitwhich made it an error to make a request with no body.- A picture of a cute animal (not mandatory but encouraged)