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

podman auto-update: reject short image names on container creation #15879

Closed
tmds opened this issue Sep 21, 2022 · 12 comments · Fixed by #15933
Closed

podman auto-update: reject short image names on container creation #15879

tmds opened this issue Sep 21, 2022 · 12 comments · Fixed by #15933
Labels
kind/bug Categorizes issue or PR as related to a bug. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.

Comments

@tmds
Copy link
Contributor

tmds commented Sep 21, 2022

ExecStart has:

ExecStart=/usr/bin/podman run \
	--cidfile=%t/%n.ctr-id \
	--cgroups=no-conmon \
	--rm \
	--sdnotify=container \
	-d \
	--replace \
	--name awesome-worker-app \
	--label io.containers.autoupdate=registry awesome-worker-app:latest

This container is running:

$ podman ps
CONTAINER ID  IMAGE                                COMMAND     CREATED        STATUS            PORTS                   NAMES
2fbeb5c35c56  localhost/awesome-worker-app:latest              7 minutes ago  Up 7 minutes ago                          awesome-worker-app

auto-update is checking docker.io:

$ podman auto-update 
UNIT                        CONTAINER                          IMAGE                      POLICY      UPDATED
awesome-worker-app.service  2fbeb5c35c56 (awesome-worker-app)  awesome-worker-app:latest  registry    failed
Error: registry auto-updating container "2fbeb5c35c5604996c22518047f33181282473f6f2f8077b8b47cfe3be9d4542": image check for "awesome-worker-app:latest" failed: reading manifest latest in docker.io/library/awesome-worker-app: errors:
denied: requested access to the resource is denied
unauthorized: authentication required

I know I can fix this using io.containers.autoupdate=local.

Does it make sense to check docker.io for autoupdate=registry when the image is referenced as awesome-worker-app:latest (and localhost/awesome-worker-app:latest)?

cc @mheon @rhatdan

@rhatdan
Copy link
Member

rhatdan commented Sep 21, 2022

@vrothberg PTAL
Is this happening on a podman-machine?

@Luap99
Copy link
Member

Luap99 commented Sep 21, 2022

I think this is expected and why we have both io.containers.autoupdate=registry and io.containers.autoupdate=local so podman does not have to guess what it is.

@tmds
Copy link
Contributor Author

tmds commented Sep 21, 2022

This is on Fedora 36 with Podman 4.2.0.

I think local vs registry is about whether to try to pull the image.

What is unexpected for me is that for podman run awesome-worker-app:latest means localhost/awesome-worker-app:latest.
While for podman auto-update it means docker.io/library/awesome-worker-app:latest.

@vrothberg
Copy link
Member

vrothberg commented Sep 21, 2022 via email

@vrothberg
Copy link
Member

OK, now I see it. awesome-worker-app:latest is not a valid input as auto updates require a full image reference.

It looks like a regression as Podman should error out on container creation.

@vrothberg vrothberg added the kind/bug Categorizes issue or PR as related to a bug. label Sep 26, 2022
@vrothberg vrothberg changed the title podman auto-update checks docker.io instead of localhost for autoupdate=registry podman auto-update: reject short image names on container creation Sep 26, 2022
@tmds
Copy link
Contributor Author

tmds commented Sep 26, 2022

It looks like a regression as Podman should error out on container creation.

You mean: dotnet run awesome-worker-app:latest should error out?

@vrothberg
Copy link
Member

It looks like a regression as Podman should error out on container creation.

You mean: dotnet run awesome-worker-app:latest should error out?

Yes. See the following quote from the docs:

The registry policy requires a fully-qualified image reference (e.g., quay.io/podman/stable:latest) to be used to create the container. This enforcement is necessary to know which image to actually check and pull. If an image ID was used, Podman would not know which image to check/pull anymore

vrothberg added a commit to vrothberg/libpod that referenced this issue Sep 26, 2022
Auto updates require container to be created with a fully-qualified
image reference.  Short names are not supported due the ambiguity of
their source registry.  Initially, container creation errored out for
non FQN images but it seems that Podman has regressed.

Fixes: containers#15879
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
@vrothberg
Copy link
Member

I opened #15933 to fix the underlying issue.

vrothberg added a commit to vrothberg/libpod that referenced this issue Sep 26, 2022
Auto updates require container to be created with a fully-qualified
image reference.  Short names are not supported due the ambiguity of
their source registry.  Initially, container creation errored out for
non FQN images but it seems that Podman has regressed.

Fixes: containers#15879
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
vrothberg added a commit to vrothberg/libpod that referenced this issue Sep 26, 2022
Auto updates require container to be created with a fully-qualified
image reference.  Short names are not supported due the ambiguity of
their source registry.  Initially, container creation errored out for
non FQN images but it seems that Podman has regressed.

Fixes: containers#15879
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
@tmds
Copy link
Contributor Author

tmds commented Sep 26, 2022

podman will now error out when creating a container with a short name if it has the auto-update label.

Another approach would be for auto-update to use the same image resolution for short-names as podman create/run.
(Or, to always use localhost when a short name is provided with the auto-update label.)

I'm laying out some options, because I like using short names for local images.

@vrothberg
Copy link
Member

Another approach would be for auto-update to use the same image resolution for short-names as podman create/run.
(Or, to always use localhost when a short name is provided with the auto-update label.)

Those can be subject to change. It is non-deterministic. While images can still be rewritten via /etc/containers/registries.conf, I think that forcing users to specify a fully-qualified image name is the right thing to do.

Since auto updates are security relevant, I think it's important to have as less "magic" as possible.

I'm laying out some options, because I like using short names for local images.

Using localhost/foo:bar will still work. Only foo or foo:bar will be rejected.

@vrothberg
Copy link
Member

To elaborate on the local images: The lookup order can vary based on the images present. Requesting a FQN there as well reduces ambiguity.

@rhatdan
Copy link
Member

rhatdan commented Sep 26, 2022

I agree FQN should be required.

vrothberg added a commit to vrothberg/libpod that referenced this issue Sep 26, 2022
Auto updates require container to be created with a fully-qualified
image reference.  Short names are not supported due the ambiguity of
their source registry.  Initially, container creation errored out for
non FQN images but it seems that Podman has regressed.

Also remove a redundant auto-update test from the 250-systemd.bats.

Fixes: containers#15879
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
vrothberg added a commit to vrothberg/libpod that referenced this issue Sep 26, 2022
Auto updates using the "registry" policy require container to be created
with a fully-qualified image reference.  Short names are not supported
due the ambiguity of their source registry.  Initially, container
creation errored out for non FQN images but it seems that Podman has
regressed.

Fixes: containers#15879
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
vrothberg added a commit to vrothberg/libpod that referenced this issue Sep 26, 2022
Auto updates using the "registry" policy require container to be created
with a fully-qualified image reference.  Short names are not supported
due the ambiguity of their source registry.  Initially, container
creation errored out for non FQN images but it seems that Podman has
regressed.

Fixes: containers#15879
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 14, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/bug Categorizes issue or PR as related to a bug. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants