api: swagger: Adjust ContainerWaitResponse error as optional

[fussybeaver]

- What I did
Removed the required API swagger flag on the ContainerWaitResponse type Error property

- How I did it
Amended API 1.39, 1.40, 1.41 and the future swagger for 1.42 YAML description to remove the required flag

- How to verify it
Generated types for the Bollard docker client and ran integration tests on the new types.

- Description for the changelog
Removed the required field in the swagger documentation for the ContainerWaitResponse Error property.

fixes #43655

- A picture of a cute animal (not mandatory but encouraged)

[thaJeztah]

hm, interesting, so I recall I added the required to make the generated type match the existing one. In this commit, the type that was moved, already had "Required: true" in the generated Go code; e4c6ca3

Wondering if that was a quirk in the generator that's used in this repository 🤔

[thaJeztah]

Looks indeed that the generated type differs with this patch (perhaps it's ok to change though);

 The result of hack/generate-swagger-api.sh differs

 diff --git a/api/types/container/wait_response.go b/api/types/container/wait_response.go
 index d2fb63aac5..84fc6afddc 100644
 --- a/api/types/container/wait_response.go
 +++ b/api/types/container/wait_response.go
 @@ -10,8 +10,7 @@ package container
  type WaitResponse struct {
  
  	// error
 -	// Required: true
 -	Error *WaitExitError `json:"Error"`
 +	Error *WaitExitError `json:"Error,omitempty"`
  
  	// Exit code of the container
  	// Required: true

 Please update api/swagger.yaml with any API changes, then
 run hack/generate-swagger-api.sh.

[fussybeaver]

To be clear, I see the field exists but is set to null in JSON on the wire. So far the Bollard client API assumes required properties to be set to a value.

There is perhaps a case to look into using the x-nullable vendor extension, but I haven't tested thoroughly whether this works in Bollard across the whole API.

[thaJeztah]

Yes, to be honest; at times it's a bit tricky to pick the most suitable option in swagger, as not all of them are a "perfect fit" for the go types (combinations of "nullable" fields, "omit empty", etc). I think the change shown in the diff above (at least at a glance) looks still OK, and I don't expect that change to be causing issues from our side (or at least, i'd expect the client to unmarshal both the "before" and "after" identical), so to unblock this PR, I'd suggest to;

• re-generate the type (or manually update it with the diff as shown above)
• if it's not too much trouble for you; split the changes in the api/types/container/wait_response.go and api/swagger.yaml (i.e. "current version") to a separate commit

Last one gives us a bit more flexibility to decide if we want to backport only the "documentation" changes or both the documentation and code changes to the "20.10" release branch (which is the branch that's also used for the documentation at docs.docker.com)

[fussybeaver]

I've regenerated the types with the provided make command and split the commits. Let me know if there's anything outstanding..

[fussybeaver]

I've regenerated the types with the provided make command and split the commits. Let me know if there's anything outstanding..

[thaJeztah]

LGTM, thanks!

[thaJeztah]

CI failure is unrelated; I kicked the Windows CI once more, but this should be ready to merge 👍

=== Failed
=== FAIL: cmd/docker-proxy TestSCTP4Proxy (0.26s)
2022/05/30 16:55:48 Can't forward traffic to backend sctp/127.0.0.1:35806: transport endpoint is already connected
    network_proxy_test.go:161: EOF

[thaJeztah]

alright; good to go; thanks!