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

Convert str to json format before evaluating length in find_image() and inspect_image(). #574

Merged
merged 2 commits into from
Apr 6, 2023

Conversation

nishipy
Copy link
Contributor

@nishipy nishipy commented Apr 5, 2023

Background
During working on #563, I found some strange behaviors of podman_image.find_image() and podman_image.inspect_image().
I guess this implementation is not as expected.

Details
These functions will execute podman commands such as podman image ls <IMG_NAME> --format json and podman image inspect <IMG_NAME> --format json.
The following snippet is from find_image(), and it looks images is expected to receive a list from rc, images, err = self._run(args, ignore_errors=True) . Then its length will be evaluated.
However, I believe that currently images is str type and it is []\n if the target image doesn't exist on a server. So len(image) will be not zero, but 3 or more.

args = ['image', 'ls', image_name, '--format', 'json']
rc, images, err = self._run(args, ignore_errors=True)
if len(images) > 0:
return json.loads(images)

I think that the expectation here is len(image) will be zero if the target image is absent.
Therefore, this PR fixes that and converts str to json format before evaluating its length in find_image() and inspect_image().

Copy link
Member

@sshnaidm sshnaidm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to add test to it.

@sshnaidm
Copy link
Member

sshnaidm commented Apr 6, 2023

Thanks for fix!

@sshnaidm sshnaidm merged commit e8c2703 into containers:master Apr 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants