API: container wait endpoint regression in 3.0.0

[edigaryev]

In #9048, which was included in the 3.0.0, the condition parameter was changed to be a slice, however, the assumptions behind the change explained in #9048 (comment) does not seem to be valid. Two examples to demonstrate this:

Here's a Podman 3.0.0 with the fix from #9347 applied:

$ curl --unix-socket /tmp/podman.sock -d '' 'http://d/v1.0.0/libpod/containers/84138634fb08/wait?condition=running'
{"cause":"schema: error converting value for index 0 of \"condition\"","message":"failed to parse parameters for /v1.0.0/libpod/containers/84138634fb08/wait?condition=running: schema: error converting value for index 0 of \"condition\"","response":400}

However, in 2.2.1 everything works fine:

$ curl --unix-socket /tmp/podman.sock -d '' 'http://d/v1.0.0/libpod/containers/84138634fb08/wait?condition=running'                                                                          
-1

[mheon]

We were OK with this as 3.0 is a major version bump, including a major API version bump (to v3.0.0). It's obviously not ideal and we would like to keep breaking changes to a minimum, but 3.0 offered an opportunity to batch necessary ones together in one release - this was one of those.

[mheon]

Though it looks like the v1.0.0 in the URL should be causing errors... @jwhonce Minimum API version is set to 3.0.0, but we're happily accepting v1.0.0 URLs still.

[edigaryev]

this was one of those

But note that the #9048 (comment) implies that this wasn't intentional.

Moreover, current 3.0 API docs show that the parameter is named condition and it's a string, whereas currently to get it working you have to use the name condition[].

This prevents the proper API bindings to be generated from the Swagger spec, which is currently the only realistic way to get things working for Golang consumers (aside from writing everything by hand), because the Podman package cannot be used directly due to multiple dependency issues.

[mheon]

Alright, that definitely sounds like a bug - the expectation was to change parameter type but keep the name, from my understanding.

[edigaryev]

Looks like this:

whereas currently to get it working you have to use the name condition[]

...actually "works" only in terms of API simply ignoring the parameter by the virtue of:

podman/pkg/api/handlers/decoder.go

Line 19 in a5e37ad

d.IgnoreUnknownKeys(true)

So the actual parameter name remained the same (condition), but the error converting value for index 0 happens due to type change from string in 2.2.1:

podman/pkg/api/handlers/utils/containers.go

Line 25 in a0d478e

Condition string `schema:"condition"`

...to an int alias in 3.0.0:

podman/pkg/api/handlers/utils/containers.go

Line 29 in 5b2585f

Condition []define.ContainerStatus `schema:"condition"`

[rhatdan]

@edigaryev Can you open a PR to fix this?

[edigaryev]

@edigaryev Can you open a PR to fix this?

See #9414.

[matejvasek]

I believe the incompatibility is not related to the "slice thing".

I suspect it's caused by preceding PR: https://github.com/containers/podman/pull/9051/files#diff-58b335c77eb3163bb2dec31c4e20ba5a62bafa143a1fbafe1a0e02dc1372fa31R327-R330