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

Warn if registries.conf contains config keys the consumer does not intend to use #2009

Open
brurucy opened this issue May 16, 2023 · 12 comments
Labels
kind/feature A request for, or a PR adding, new functionality

Comments

@brurucy
Copy link

brurucy commented May 16, 2023

Feature request description

Assume that the following can be found at $HOME/.config/containers/registries.conf, on macOS:

unqualified-search-registries = ['docker.io']

[[registry]]
prefix = "docker.io"
location = "docker.io"

[[registry.mirror]]
location = "<some-mirror-location>"

Running podman pull --log-level=debug <repo>/<image> yields the following:

INFO[0000] /$HOME/<podman-bin-path>/podman filtering at log level debug
DEBU[0000] Called pull.PersistentPreRunE(/$HOME/<podman-bin-path>/podman --log-level=debug pull <repo>/<image>)
DEBU[0000] DoRequest Method: GET URI: http://d/v4.4.2/libpod/_ping
DEBU[0000] Loading registries configuration "/$HOME/.config/containers/registries.conf"
DEBU[0000] DoRequest Method: POST URI: http://d/v4.4.2/libpod/images/pull
Resolving "<repo>/<image>" using unqualified-search registries (/etc/containers/registries.conf.d/999-podman-machine.conf)
Trying to pull docker.io/<repo>/<image>:latest...
Error: initializing source docker://<repo>/<image>:latest: reading manifest latest in docker.io/<repo>/<image>: requested access to the resource is denied
DEBU[0003] Shutting down engines

Notice how misleading it is to print out that: DEBU[0000] Loading registries configuration "/$HOME/.config/containers/registries.conf"

When on macOS, that is entirely ignored, that is, it will only work as intended, if the user creates a machine with that folder mounted on by default.

Suggest potential solution

To either:

  1. Provide a flag, that will mount $HOME/.config/containers when creating a machine, providing the same behaviour across linux and Mac
  2. Add a warning, or fix the debug print, that on Mac, that file is not taken into account

Have you considered any alternatives?

Yes. The alternative is to simply create a machine mounting $HOME//.config/containers, however, there doesn't seem to be explicit documentation about this. It took a lot of head-banging to figure out that registries.conf just isn't taken into account for Mac.

Additional context

I'm fine with tackling either of the solutions I've proposed, but I think that the second one might be easier and faster to put out.

@brurucy brurucy added the kind/feature A request for, or a PR adding, new functionality label May 16, 2023
@brurucy brurucy changed the title Misleading logs in macOS Misleading logs in macOS with respect to registries.conf May 16, 2023
@baude
Copy link
Member

baude commented May 17, 2023

@rhatdan i know we went around and around on this topic of where do containers-common configuration files land like this. As the user reports, I agree it is unclear. Logically, if we support containers.conf in the macos filesystem, seems we ought to for registries.conf?

@baude baude added the kind/bug A defect in an existing functionality (or a PR fixing it) label May 17, 2023
@baude
Copy link
Member

baude commented May 17, 2023

btw, @brurucy excellent writeup. Thank you for taking the time to do so.

@vrothberg
Copy link
Member

It's technically not possible to support registries.conf for remote clients since the images are pulled on the server side.

I think we should erase the code paths that lead to the config being loaded on the client side to get rid of the logs.

@rhatdan
Copy link
Member

rhatdan commented May 18, 2023

Agreed can not do this on the client side.

@Luap99
Copy link
Member

Luap99 commented May 19, 2023

Even for containers.conf most options will only be read on the server side. Only exception is the [machine] section the system connections, I am sure there might be some small others but generally speaking defaults should always come from the server: containers/podman@9770947

@vrothberg
Copy link
Member

I had a look at the exact call path and found that registries.conf is actually being used by remote clients but only for authentication, the credential-helpers in particular. So we cannot get rid of loading these files, nor of the debug log.

So I want to move the conversation into the direction of how to avoid users from thinking other fields (e.g., registries) could be set on the Mac. Cc: @mtrmac

Should Podman be able to specify a slice of supported keys and sysregistriesv2 would bark when other keys are loaded?

@mtrmac
Copy link
Collaborator

mtrmac commented May 22, 2023

Something like that would be possible, and it would make sense for configs built from scratch.

Consider a Linux system, with all the default configs set up for local use of Podman, running podman machine. Does it make sense to warn in that case?


Implementation: right now, sysregistriesv2 hides the actual config file load behind a fair amount of layers (caching, pre-processing, …). I’m not sure whether we want to re-load all of the data just to emit a warning, which would be the simplest way to implement this very local concern, without having to make this visible in SystemContext for all users.

@vrothberg
Copy link
Member

Consider a Linux system, with all the default configs set up for local use of Podman, running podman machine. Does it make sense to warn in that case?

Maybe a debug log plus some doc/man page changes would improve the situation?

I’m not sure whether we want to re-load all of the data just to emit a warning, which would be the simplest way to implement this very local concern, without having to make this visible in SystemContext for all users.

We could cache the specified keys to avoid reloading.

@mtrmac
Copy link
Collaborator

mtrmac commented May 23, 2023

… actually we probably can just read the cache for the warning.

@github-actions
Copy link

A friendly reminder that this issue had no activity for 30 days.

@rhatdan
Copy link
Member

rhatdan commented Jun 23, 2023

@vrothberg @mtrmac Should this be fixed in containers/image or Podman

@vrothberg
Copy link
Member

In c/image

@rhatdan rhatdan transferred this issue from containers/podman Jun 23, 2023
@mtrmac mtrmac changed the title Misleading logs in macOS with respect to registries.conf Warn if registries.conf contains config keys the consumer does not intend to use Jun 23, 2023
@mtrmac mtrmac removed the kind/bug A defect in an existing functionality (or a PR fixing it) label Jun 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature A request for, or a PR adding, new functionality
Projects
None yet
Development

No branches or pull requests

6 participants