storage: fix drop-in config order and windows special case#826
Open
Luap99 wants to merge 3 commits intocontainers:mainfrom
Open
storage: fix drop-in config order and windows special case#826Luap99 wants to merge 3 commits intocontainers:mainfrom
Luap99 wants to merge 3 commits intocontainers:mainfrom
Conversation
On windows the uid will always be reported as -1 as there is no uid concept. As such using the special root/rootless paths make no sense there so lets just not add them to the search paths to begin with. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
When have the same filename in the per $UID rootless directory than that should have higher priority over the same filename in the general rootless directory and that also should be higher than the main conf.d location. This is how it used to be before I switch the logic accidentally in commit 046e21b ("storage: add GetSearchPaths() to pkg/configfile"). Signed-off-by: Paul Holzinger <pholzing@redhat.com>
The docs uses a slightly incorrect order, it makes a difference if we start with the per user location or the /usr one for the special drop-in directories like containers.rootless.conf.d/$UID and just containers.rootless.conf.d/. So ensure the registries.conf and containers.conf and certs.d pages are actually correct now. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Luap99
changed the title
github-actions
Bot
added
storage
Member
Author
mtrmac
approved these changes
May 6, 2026
Contributor
mtrmac
left a comment
mtrmac
left a comment
There was a problem hiding this comment.
Thanks!
LGTM otherwise, OK to merge without another review round.
| specialName = "rootful" | ||
| } | ||
| // insert the name after the main config name but before the extension if it has one. | ||
| mainPath, cut := strings.CutSuffix(mainPath, suffix) |
Contributor
There was a problem hiding this comment.
After the reordering, this shadows the mainPath variable outside the if. It does work correctly but it looks a bit confusing / risky.
Maybe something like prefix or specialPrefix?
Comment on lines
60
to
73
| { | ||
| name: "storage.conf", | ||
| mainPath: "/usr/share/containers/storage.conf", | ||
| suffix: ".conf", | ||
| uid: 0, | ||
| want: []string{"/usr/share/containers/storage.conf.d", "/usr/share/containers/storage.rootful.conf.d"}, | ||
| want: []string{"/usr/share/containers/storage.rootful.conf.d", "/usr/share/containers/storage.conf.d"}, | ||
| }, | ||
| { | ||
| name: "storage.conf", | ||
| mainPath: "/usr/share/containers/storage.conf", | ||
| suffix: ".conf", | ||
| uid: 0, | ||
| want: []string{"/usr/share/containers/storage.conf.d", "/usr/share/containers/storage.rootful.conf.d"}, | ||
| want: []string{"/usr/share/containers/storage.rootful.conf.d", "/usr/share/containers/storage.conf.d"}, | ||
| }, |
Contributor
There was a problem hiding this comment.
(Absolutely non-blocking: Unrelated and pre-existing, these entries are exactly identical.)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
see commits