Skip to content
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

Fix assertation error in manifest.py #320

Merged
merged 1 commit into from
Sep 2, 2023
Merged

Conversation

umohnani8
Copy link
Member

No description provided.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 31, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: umohnani8

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jwhonce
Copy link
Member

jwhonce commented Aug 31, 2023

LGTM, did api change?

@umohnani8
Copy link
Member Author

@jwhonce I don't think so, I don't see any changes to manifest remove digest in the recent podman history. I wonder if the functionality of assertIsNone changed? Any idea where I can confirm that?

@umohnani8
Copy link
Member Author

Hmm, nvm assertIsNone is a part of unittest, so don't think that changed. I am not sure where the api might have possibly changed to have manifests be an empty list instead of None

@andryyy
Copy link
Contributor

andryyy commented Sep 1, 2023

Strange.

I just tried Podman 4.3.something on Debian 12 and testing with podman-py's current stable as well as the code from the main branch. It always returned None. Same when I upgraded Podman to 4.5.1 from the Kubic repository.

I compiled Podman 4.7.0-dev on the same system and just then it returned an empty list.

Didn't try 4.6 though.

I mirrored the socket with socat and compared the request and response:

# socat -t100 -x -v UNIX-LISTEN:/home/user/podman-mirror.sock,mode=777,reuseaddr,fork UNIX-CONNECT:/run/user/1000/podman/podman.sock

# Result dev:

GET /v4.7.0/libpod/manifests/localhost%2Funittest%2Falpine/json HTTP/1.1\r
Host: %2fhome%2fuser%2fpodman-mirror.sock\r
User-Agent: PodmanPy/4.7.0-dev (API v4.7.0; Compatible v1.40)\r
Accept-Encoding: gzip, deflate\r
Accept: */*\r
Connection: keep-alive\r
\r
< 2023/09/01 08:51:33.000905957  length=330 from=1781 to=2110
HTTP/1.1 200 OK\r
Api-Version: 1.41\r
Content-Type: application/json\r
Libpod-Api-Version: 4.7.0-dev\r
Server: Libpod/4.7.0-dev (linux)\r
X-Reference-Id: 0xc000123888\r
Date: Fri, 01 Sep 2023 06:51:33 GMT\r
Content-Length: 107\r
\r
{"schemaVersion":2,"mediaType":"application/vnd.docker.distribution.manifest.list.v2+json","manifests":[]}

# Result v4.5.1:

GET /v4.7.0/libpod/manifests/localhost%2Funittest%2Falpine/json HTTP/1.1\r
Host: %2fhome%2fuser%2fpodman-mirror.sock\r
User-Agent: PodmanPy/4.7.0-dev (API v4.7.0; Compatible v1.40)\r
Accept-Encoding: gzip, deflate, zstd\r
Accept: */*\r
Connection: keep-alive\r
\r
< 2023/09/01 08:56:39.000681039  length=324 from=374 to=697
HTTP/1.1 200 OK\r
Api-Version: 1.41\r
Content-Type: application/json\r
Libpod-Api-Version: 4.5.1\r
Server: Libpod/4.5.1 (linux)\r
X-Reference-Id: 0xc000030710\r
Date: Fri, 01 Sep 2023 06:56:39 GMT\r
Content-Length: 109\r
\r
{"schemaVersion":2,"mediaType":"application/vnd.docker.distribution.manifest.list.v2+json","manifests":null}

I think this happened somewhere in this commit by changing the decoder in https://github.com/containers/podman/blame/main/pkg/api/handlers/decoder.go

Since it’s a list by media type, it seems to be an actual fix for the previous behavior. 👍

@rhatdan
Copy link
Member

rhatdan commented Sep 2, 2023

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Sep 2, 2023
@rhatdan rhatdan merged commit c8abef1 into containers:main Sep 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants