image: port registries.conf to pkg/configfile by Luap99 · Pull Request #773 · containers/container-libs

Luap99 · 2026-04-20T12:50:11Z

see commits

Luap99 · 2026-04-20T12:50:32Z

Luap99 · 2026-04-20T13:28:56Z

libimage tests set

SystemRegistriesConfDirPath: "/dev/null",

This now fails since we cannot open this as dir, the only reason it worked before is due the use of a IMO incorrect WalkDir() function which does not error if the passed in path is not a dir.

Should we keep supporting this? I guess I could special case /dev/null to set DoNotLoadDropInFiles? Or make pkg/configfile ignore the case when we do not get a directory?

Or just fix the tests and hope no actual users depend on this option? I guess this ties back to the behavior that setting a custom main file will always still try to parse drop ins. So I assume the /dev/null as used to disable that?

mtrmac · 2026-04-20T15:35:12Z

libimage tests set

SystemRegistriesConfDirPath: "/dev/null",

Doing this in tests might have started as a habit of mine — POSIX specifies properties of remarkably few paths, and I used to prefer writing POSIX-portable software. (See /dev/null/this/does/not/exist in c/image.) The goal was just to lazily refer to something which does not exist without having to actually create a directory, or without having root access to create one.

I guess this ties back to the behavior that setting a custom main file will always still try to parse drop ins. So I assume the /dev/null as used to disable that?

That’s how I read this, and what various other tests have had to do — the goal is to use only the test config file without any of the system-wide drop-ins which might have been added by root to the system.

Just fix the tests please, I don’t think we ever promised that this would work.

mtrmac · 2026-04-20T16:58:46Z

 		}

-		if conf.EnvironmentName != "" && !conf.DoNotLoadDropInFiles {
+		if conf.EnvironmentName != "" && !conf.DoNotLoadDropInFiles && conf.CustomConfigFileDropInDirectory == "" {


A sort of semantics question — should this be driven by CustomConfigFilePath, the DropInDirectory, or both?

I started thinking that CustomConfigFilePath should turn this off (I think base on some renamed version of shouldLoadMainFile) … but, on reflection, the override is sort of a drop-in.

I am not to sure either, I guess the simplest would be to always use it even when CustomConfigFilePath or CustomConfigFileDropInDirectory set. Then we could always have a env override but then it means cli options may not win over the env which is unexpected.

And yes since I considered this a drop in I only turned it off on CustomConfigFileDropInDirectory

I agree that the most important part is that a caller can opt out of the environment variables in a documented way.

I’m fine with leaving it as is, but I’ll leave the thread marked unresolved to allow an opportunity for other opinions/views.

Luap99 · 2026-04-20T17:02:29Z

Ok added docs and some other minor fixes I found. I will go thourhg your comments tomorrow.

Then one thing that is still missing here is the use of configfile.Slice over []string when parsing the toml but that seem not so trivial with the way the custom overwrites are already handled.
Maybe I leave that our for the time being, adding that syntax would be backwards compatible so should not block for podman 6.0.

mtrmac

Just a fairly quick look, I didn’t carefully read the configfile parts nor all of the tests.

mtrmac · 2026-04-20T17:09:20Z

+				},
+			},
+		},
+		// TODO: add more test cases


(It would be sort of elegant for Read and GetSearchPaths to share the test cases — but, as is, that would be a lot of duplication.)

mtrmac · 2026-04-20T17:18:48Z


 // configWrapper is used to store the paths from ConfigPath and ConfigDirPath
 // and acts as a key to the internal cache.
 type configWrapper struct {


configWrapper changes from “resolved paths” to “inputs to path resolution”. Is that safe?

It might very well be fine in practice, but e.g. a change to CONTAINERS_REGISTRIES_CONF (or HOMEDIR or XDG…) at runtime won’t have effect. That would probably only affect in-process tests, but I’m not sure.

OTOH it’s not clear that we can safely do much better. We could run ToConfigFileOptions().GetSearchPaths() at construction time, but that would be racy WRT later additions/removals of files on the filesystem (much more likely than environment variable changes during the lifetime of a process), and we’d need to somehow turn the result into a valid map key, yuck.

yeah I mean we could try to resolve the envs here here and store the value as part of the hash key, but then we duplicate some intentionally abstracted details and also still have a race. And yes GetSearchPaths() does not feel any better either.
Also the fact that you cannot use []string as map hash key, so that woudl need to be turned into a string first as well.

I can add a comment that this is the best we could do realistically.

We already have a public InvalidateCache, so that should be sufficient. It’s sort of an API break, but, meh.

jankaluza · 2026-04-21T04:31:30Z

I do not see anything obviously wrong here. I had the very similar patch ready, but then replaced it with ErrConfigFileNotFound. However, for your use-case, it makes sense to me.

packit-as-a-service · 2026-04-21T10:52:37Z

Packit jobs failed. @containers/packit-build please check.

packit-as-a-service · 2026-04-21T10:52:37Z

Packit jobs failed. @containers/packit-build please check.

packit-as-a-service · 2026-04-21T10:52:37Z

Packit jobs failed. @containers/packit-build please check.

packit-as-a-service · 2026-04-21T10:52:38Z

Packit jobs failed. @containers/packit-build please check.

Luap99 · 2026-04-21T11:00:35Z

Podman PR containers/podman#28552
Buildah PR containers/buildah#6799

containers/container-libs#773 Signed-off-by: Paul Holzinger <pholzing@redhat.com>

mtrmac

A full review now. ACK overall, just nits.

mtrmac · 2026-04-21T15:27:46Z

 		}

-		if conf.EnvironmentName != "" && !conf.DoNotLoadDropInFiles {
+		if conf.EnvironmentName != "" && !conf.DoNotLoadDropInFiles && conf.CustomConfigFileDropInDirectory == "" {


I agree that the most important part is that a caller can opt out of the environment variables in a documented way.

I’m fine with leaving it as is, but I’ll leave the thread marked unresolved to allow an opportunity for other opinions/views.

mtrmac · 2026-04-21T16:46:45Z

+			shouldLoadDropIns = false
+		} else {
+			// default search paths
+			mainFiles = append(mainFiles, userConfig, overrideConfig, defaultConfig)


Something somewhere should filter out the "" entries; if not in this subpackage, then at the latest in ConfigurationSourceDescription.

Absolutely non-blocking: Doing it here would also allow centralizing the filepath.Join(…, configFileName) code. (Well, getDropInPaths would need to be updated to add configFileName +"." + dropInSuffix, but I think that would simplify the handling of specialName.)

Deprecate V1RegistriesConf and no longer accept the V1 syntax as part of the config file. Signed-off-by: Paul Holzinger <pholzing@redhat.com>

These should mirror the current registries.conf behavior for SystemRegistriesConfPath and SystemRegistriesConfDirPath so I can port this over to the new package. CustomConfigFilePath sets the single config path to the main file which we parse first, we must error if that path does not exists. Also if set we still must read normal drop ins from the default locations to keep the current registries.conf behavior. CustomConfigFileDropInDirectory sets the directory from which we read the drop in files instead of the default locations. If that path does not exists no drop in will be parsed. In addition these options have higher priority then the environment variables as they are often used for cli options which should be more important then the env. Signed-off-by: Paul Holzinger <pholzing@redhat.com>

We need an API to get a list of all search paths to display it as part of error messages for registries.conf. This will also be needed to get the containers.conf module directories for the shell completion logic. Signed-off-by: Paul Holzinger <pholzing@redhat.com>

Rewrite registries.conf parsing to use our new configfile package. https://github.com/containers/podman/blob/34a4633d5fd4a502cef289b4f3f449535a7e1067/contrib/design-docs/config-file-parsing.md Signed-off-by: Paul Holzinger <pholzing@redhat.com>

/dev/null is not a valid directory, the new reworked logic correctly fails wehn given a non directory path while the old just ignored it. As such make sure we pass an valid empty directory here. Signed-off-by: Paul Holzinger <pholzing@redhat.com>

Update the docs for the new registries.conf parsing behavior. Remove the old containers-registries.conf.d.5 man page and just link to the main one to dedup some content. The install-docs Makefile target should install this "link" just fine as is. Signed-off-by: Paul Holzinger <pholzing@redhat.com>

mtrmac

Still #773 (comment) I think, not blocking — users are likely to only hit this once.

Feel free to merge as is, or update+merge without another review.

Luap99 · 2026-04-22T16:20:22Z

Still #773 (comment) I think, not blocking — users are likely to only hit this once.

Ah I though I dropped the commit in a rebase, I must have forgotten. Should be a quick fix up.

The config file name is registries.conf not registry.conf. Signed-off-by: Paul Holzinger <pholzing@redhat.com>

We already respect the CONTAINERS_REGISTRIES_CONF in the actual file reading code so this function should not be used anymore. Signed-off-by: Paul Holzinger <pholzing@redhat.com>

The CONTAINERS_REGISTRIES_CONF env is already read by the config parser, support for REGISTRIES_CONFIG_PATH is removed. Signed-off-by: Paul Holzinger <pholzing@redhat.com>

The file name is registries.conf. Signed-off-by: Paul Holzinger <pholzing@redhat.com>

containers/container-libs#773 Signed-off-by: Paul Holzinger <pholzing@redhat.com>

mtrmac · 2026-04-23T11:31:31Z

@Luap99 please merge whenever the consumers are ready.

Podman 6 is going to drop support for reading the old v1 format in containers/container-libs#773 The v2 format is supported for a long time already so this can just be switched and still works with older versions. Signed-off-by: Paul Holzinger <pholzing@redhat.com>

github-actions Bot added storage Related to "storage" package image Related to "image" package labels Apr 20, 2026

mtrmac reviewed Apr 20, 2026

View reviewed changes

Luap99 force-pushed the registries-conf branch from e3f2357 to 7e84af4 Compare April 20, 2026 16:59

github-actions Bot added the common Related to "common" package label Apr 20, 2026

mtrmac reviewed Apr 20, 2026

View reviewed changes

Comment thread image/docs/containers-registries.conf.5.md Outdated

jankaluza reviewed Apr 21, 2026

View reviewed changes

Comment thread image/docs/containers-registries.conf.5.md

jankaluza mentioned this pull request Apr 21, 2026

image/docker: use unified configfile for registries.d #753

Merged

Luap99 force-pushed the registries-conf branch 2 times, most recently from 7e21306 to 99fd74f Compare April 21, 2026 10:52

Luap99 marked this pull request as ready for review April 21, 2026 10:53

This was referenced Apr 21, 2026

vendor registries.conf rework containers/podman#28552

Merged

vendor registries.conf rework containers/buildah#6799

Merged

Luap99 mentioned this pull request Apr 21, 2026

move registries.conf format to v2 cockpit-project/cockpit-podman#2506

Merged

Luap99 added a commit to Luap99/buildah that referenced this pull request Apr 21, 2026

vendor registries.conf rework

be58875

containers/container-libs#773 Signed-off-by: Paul Holzinger <pholzing@redhat.com>

This was linked to issues Apr 21, 2026

$XDG_CONFIG_HOME support for registries.conf #231

Closed

Support /usr/*/registries.conf* #239

Closed

Luap99 added a commit to Luap99/libpod that referenced this pull request Apr 21, 2026

vendor registries.conf rework

79274dd

containers/container-libs#773 Signed-off-by: Paul Holzinger <pholzing@redhat.com>

Luap99 added the podman 6 breaking changes that should go only into podman 6 only label Apr 21, 2026

mtrmac reviewed Apr 21, 2026

View reviewed changes

Luap99 added 6 commits April 22, 2026 15:22

image: no longer accept V1 syntax in registries.conf

6d8b59c

Deprecate V1RegistriesConf and no longer accept the V1 syntax as part of the config file. Signed-off-by: Paul Holzinger <pholzing@redhat.com>

Luap99 force-pushed the registries-conf branch from 6f95864 to 496f83a Compare April 22, 2026 13:23

mtrmac approved these changes Apr 22, 2026

View reviewed changes

Comment thread storage/pkg/configfile/parse.go

Luap99 added 3 commits April 22, 2026 18:20

common: fix typo in registries.conf message

f73db9b

The config file name is registries.conf not registry.conf. Signed-off-by: Paul Holzinger <pholzing@redhat.com>

image: deprecate UpdateRegistriesConf()

b33d9fe

We already respect the CONTAINERS_REGISTRIES_CONF in the actual file reading code so this function should not be used anymore. Signed-off-by: Paul Holzinger <pholzing@redhat.com>

common: remove hard coded registries.conf override

b50e85a

The CONTAINERS_REGISTRIES_CONF env is already read by the config parser, support for REGISTRIES_CONFIG_PATH is removed. Signed-off-by: Paul Holzinger <pholzing@redhat.com>

Luap99 force-pushed the registries-conf branch from 496f83a to 674b11a Compare April 22, 2026 16:22

image: fix typo in registries.conf v1 format error message

79cd110

The file name is registries.conf. Signed-off-by: Paul Holzinger <pholzing@redhat.com>

Luap99 force-pushed the registries-conf branch from 674b11a to 79cd110 Compare April 22, 2026 16:23

mtrmac approved these changes Apr 22, 2026

View reviewed changes

Luap99 added a commit to Luap99/libpod that referenced this pull request Apr 22, 2026

vendor registries.conf rework

8d4d994

containers/container-libs#773 Signed-off-by: Paul Holzinger <pholzing@redhat.com>

Luap99 added a commit to Luap99/libpod that referenced this pull request Apr 23, 2026

vendor registries.conf rework

8cb5b5f

containers/container-libs#773 Signed-off-by: Paul Holzinger <pholzing@redhat.com>

Luap99 merged commit cbaa5f4 into containers:main Apr 23, 2026
37 checks passed

Luap99 deleted the registries-conf branch April 23, 2026 13:58

Conversation

Luap99 commented Apr 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Luap99 commented Apr 20, 2026

Uh oh!

Luap99 commented Apr 20, 2026

Uh oh!

mtrmac commented Apr 20, 2026

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Luap99 commented Apr 20, 2026

Uh oh!

mtrmac left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mtrmac Apr 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jankaluza commented Apr 21, 2026

Uh oh!

packit-as-a-service Bot commented Apr 21, 2026

Uh oh!

packit-as-a-service Bot commented Apr 21, 2026

Uh oh!

packit-as-a-service Bot commented Apr 21, 2026

Uh oh!

packit-as-a-service Bot commented Apr 21, 2026

Uh oh!

Luap99 commented Apr 21, 2026

Uh oh!

mtrmac left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mtrmac left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Luap99 commented Apr 22, 2026

Uh oh!

mtrmac commented Apr 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Luap99 commented Apr 20, 2026 •

edited

Loading

mtrmac Apr 20, 2026 •

edited

Loading