search

containers / podman-py

Python bindings for Podman's RESTful API
Apache License 2.0
246 stars 88 forks source link

client.images.push missing support for format parameter #399

Closed AlexanderNull closed 1 month ago

AlexanderNull commented 2 months ago

Some repositories (AWS lambdas in ECR) have issues with the oci manifest format and require the manifest be pushed in the docker/v2s2 format. If using the podman cli adding the --format docker option to podman push command pushes the manifest files in the correct format and everything works well. Current implementation for this python client however only passes on the destination and tlsVerify parameters to the podman API.

I checked in the podman API docs to see if this is supported and while it's not in the docs, format parameter is implemented in both the compat and libpod versions of the /images/{name}/push implementations. I have submitted a bug for the missing podman documentation, here: https://github.com/containers/podman/issues/23040, but it looks like this was just missed when the functionality was added a few years back.

I tried modifying the podman client locally to add the format parameter onto the params object that is passed along and it worked without issue. Would be nice to add this as an official parameter option for other users as well.

Are there any objections to adding this as a supported kwarg to images_manager.push? Does there need to be a verification of the format values passed in within the python client or would the return error if someone provides an unsupported format string suffice? Currently the supported format values in the podman api are (None, "oci", "v2s2", "docker", "v2s1") found at: https://github.com/containers/podman/blob/main/pkg/domain/infra/abi/images.go#L310

jwhonce commented 1 month ago

@AlexanderNull Adding to kwargs is a great solution. We usually document the values in the docstring but let the server return errors vs validating. Unless there is some magic required on the client to map docker expected values to podman expected values.

AlexanderNull commented 1 month ago

No special magic required in the python client. The podman REST api already handles converting those 5 acceptable values above (including None) into the correct internal types. Those 5 values seem pretty stable also, they haven't changed in 4 years. Is this an open for outside help issue?

inknos commented 1 month ago

@AlexanderNull this is great. Would you like to take care of it, opening a PR? I can help with reviewing it :)

inknos commented 1 month ago

resolved here https://github.com/containers/podman-py/pull/415