Skip to content
Draft
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
342 changes: 342 additions & 0 deletions contrib/design-docs/config-file-parsing.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,342 @@
# Change Request

<!--
This template is used to propose and discuss major new features to be added to Podman, Buildah, Skopeo, Netavark, and associated libraries.
The creation of a design document prior to feature implementation is not mandatory, but is encouraged.
Before major features are implemented, a pull request should be opened against the Podman repository with a completed version of this template.
Discussion on the feature will occur in the pull request.
Merging the pull request will constitute approval by project maintainers to proceed with implementation work.
When the feature is completed and merged, this document should be removed to avoid cluttering the repository.
It will remain in the Git history for future retrieval if necessary.
-->

## **Short Summary**

Unify (and rework) our config file parsing logic to make the various config files all behave
the same parsing wise so users can better understand how it works.

## **Objective**

We have several config files such a containers.conf, storage.conf, registries.conf and more that
get all implement their own parsing logic and have a different feature set. The goal is to
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
get all implement their own parsing logic and have a different feature set. The goal is to
all implement their own parsing logic and have a different feature set. The goal is to

consolidate the parsing into a separate package and then port all files to use that package
instead making them behave consistently.

## **Detailed Description:**

### General

Add new package to the storage library (`go.podman.io/storage/pkg/configfile`) which implements
the core logic of how to read config files. The goal of the package is to define with config paths
to use and in what order.

It will however not define the structs and fields used for the actual content in each file, these
stay where there are defined currently and the plan is to have the code there call into the `configfile`
package to read the files in the same way.

#### Search locations:

Linux:

- `/usr/share/containers/<name>.conf`
- `/usr/share/containers/<name>.conf.d/`
- `/usr/share/containers/<name>.rootful.conf.d/` (only when UID == 0)
- `/usr/share/containers/<name>.rootless.conf.d/` (only when UID > 0)
- `/usr/share/containers/<name>.rootless.conf.d/<UID>/` (only when UID > 0)

- `/etc/containers/<name>.conf`
- `/etc/containers/<name>.conf.d/`
- `/etc/containers/<name>.rootful.conf.d/` (only when UID == 0)
- `/etc/containers/<name>.rootless.conf.d/` (only when UID > 0)
- `/etc/containers/<name>.rootless.conf.d/<UID>/` (only when UID > 0)
Comment on lines +43 to +51
Copy link
Member

Choose a reason for hiding this comment

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

How this will work with nested Podman? Will it be treated as rootful or rootless? It has EUID==0 but no capabilities in the initial namespace.

Should we instead differentiate by privileges (admin vs user) instead of (rootful vs rootless)?

Copy link
Member Author

Choose a reason for hiding this comment

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

The current containers.conf version uses euid.

I think that is the most logical one because if I am uid 0 I likely still want to use /var/lib/containers as storage.conf setting for example ,etc...
I find the way we deal with the rexec (CAP_SYS_ADAMIN) highly confusing and not something that would be easily understood by users. We also need to keep in mind that our podman re-exec is different from buildah, skopeo or other library users so I find using the uid 0 is the best way to keep it reasonable.


- `$XDG_CONFIG_HOME/containers/<name>.conf`
Copy link
Member

Choose a reason for hiding this comment

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

Is this defined and working on MacOS?

Copy link
Member Author

Choose a reason for hiding this comment

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

You mean the XDG_CONFIG_HOME env?

That env either is set or it doesn't in which case we fall back to $HOME/.config just like on linux. If your question is if macos uses the XDG_ vars at all then not that I know off, that would however not prevent a users from using it. And also that is the current behaviour for containers.conf on macos as well to use XDG_CONFIG_HOME so I didn't see a reason to change this.

Copy link
Contributor

Choose a reason for hiding this comment

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

https://developer.apple.com/library/archive/documentation/FileManagement/Conceptual/FileSystemProgrammingGuide/FileSystemOverview/FileSystemOverview.html#//apple_ref/doc/uid/TP40010672-CH2-SW1 is the traditional macOS expectation, not .config.

I don’t have a strong opinion here … consistency with Linux might be easier for everyone.

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah though in this case I find it easier to just keep doing what we have been doing for consistency.

And we haven't gotten any complains for macos users about AFAIACT so I didn't saw a reason to justify this changee

- `$XDG_CONFIG_HOME/containers/<name>.conf.d/`
(if $XDG_CONFIG_HOME is empty then it uses $HOME/.config)
This homedir lookup will also be done by root [1].

Where `<name>` is `containers`, `storage` or `registries` for each config file.

The `<name>.rootless.conf.d/<UID>/` is a directory named by the user id. Only the user with this
exact uid match will read the config files in this directory.
The use case is for admin to be able to set a default for a specific user without having to write
into their home directory. Note this is not intended as security mechanism, the user home directory
config files will still have higher priority.

FreeBSD:

Same as Linux except `/usr/share` is `/usr/local/share` and `/etc` is `/usr/local/etc`.

Windows:

There is no `/usr` equivalent, for `/etc` we instead lookup `ProgramData` env and use that one.
And instead of `XDG_CONFIG_HOME` which isn't used on windows we use `APPDATA`.

MacOS:

Same as Linux.
Copy link
Member

Choose a reason for hiding this comment

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

So, if I understand correctly, the Podman machine will have XDG_CONFIG_HOME set and will load configs from the host's filesystem through mounts. Am I correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

podman machine is fully out of scope of this doc

Right now we do lookup containers.conf and registries.conf on the client side as well thus macos paths need to be documented as well.

Now yes long term for machine since most files are still needed on the server it would make sense for machine to expose like a single mount on the host where they configure the inside from the host.


#### Load order

I propose adopting the UAPI config file specification for loading config files (version 1.0):
https://uapi-group.org/specifications/specs/configuration_files_specification/

Based on that the files must be loaded in this order:

Read `$XDG_CONFIG_HOME/containers/<name>.conf`, only if this file doesn't exists read
`/etc/containers/<name>.conf`, and if that doesn't exists read `/usr/share/containers/<name>.conf`
As such setting an empty file on `$XDG_CONFIG_HOME/containers/<name>.conf` would cause us to ignore
all possible options there were set in the other files.
Note: This is different from the current containers.conf loading where we would have read all files.

Regardless of which file has been loaded above it then must read the drop-in locations in the following order:

- `/usr/share/containers/<name>.conf.d/`
- `/usr/share/containers/<name>.rootful.conf.d/` (UID == 0)
- `/usr/share/containers/<name>.rootless.conf.d/` (UID > 0)
- `/usr/share/containers/containers.rootless.conf.d/<UID>/` (UID > 0)

- `/etc/containers/<name>.conf.d/`
- `/etc/containers/<name>.rootful.conf.d/` (UID == 0)
- `/etc/containers/<name>.rootless.conf.d/` (UID > 0)
- `/etc/containers/containers.rootless.conf.d/<UID>/` (UID > 0)

- `$XDG_CONFIG_HOME/containers/<name>.conf.d/`

Only files read with the `.conf` file extension are read.

If there is a drop-in file with the same filename as in a prior location it will replace
the prior one and only the latest match is read. Once we have the list of all drop-in files
they get sorted lexicographic. The later files have a higher priority so they can overwrite
options set in a prior file.

##### Example

Consider the following files:

`/usr/share/containers/containers.conf` (overridden by `/etc/containers/containers.conf`):
```
field_1 = a
```

`/etc/containers/containers.conf`:
```
field_2 = b
```

`/usr/share/containers/containers.conf.d/10-vendor.conf` (overridden by `$XDG_CONFIG_HOME/containers/containers.conf.d/10-vendor.conf`):
```
field_3 = c
```

`/usr/share/containers/containers.conf.d/99-important.conf`:
```
field_4 = d
```

`/usr/share/containers/containers.rootless.conf.d/50-my.conf`:
```
field_5 = e
```

`$XDG_CONFIG_HOME/containers/containers.conf.d/10-vendor.conf`:
```
# empty
```

`$XDG_CONFIG_HOME/containers/containers.conf.d/33-opt.conf` (this is read but field_4 is overridden by `/usr/share/containers/containers.conf.d/99-important.conf` as `99-important.conf` is sorted later):
```
field_4 = user
field_6 = f
```

Now parsing this as user with UID 1000 results in this final config:

```
field_2 = b
field_4 = d
field_5 = e
field_6 = f
```

#### Environment Variables

The following two envs should be defined for each config file:

`CONTAINERS_<name>_CONF`: If set only read the given file in this env and nothing else.
Copy link
Contributor

Choose a reason for hiding this comment

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

I’m generally not a fan of environment variables in libraries (I’d prefer to consolidate environment variable handling into an optionally-imported subpackage instead, compare c/image/pkg/cli/environment… never used anywhere it turns out), but, *shrug*, I’ll go with the majority.

`CONTAINERS_<name>_CONF_OVERRIDE`: If set append the given file as last file after parsing
all other files as normal. Useful to overwrite a single field for testing without overwriting
the rest of the system configuration.

As special case for containers.conf the name of the vars is `CONTAINERS_CONF` and `CONTAINERS_CONF_OVERRIDE`.

The handling of these should be part of the `configfile` package.

#### Appending arrays

The toml parser by default replaces arrays in each file which makes it impossible to append values in drop-ins, etc...

containers.conf already has a work for that with a custom syntax to trigger appending:
```
field = ["val", {append=true}]
```
I propose we adapt the same universally for the other config files.

https://github.com/containers/container-libs/blob/main/common/docs/containers.conf.5.md#appending-to-string-arrays

This means moving the `common/internal/attributedstring` into the new configfile package so all callers can use it.

#### Scope

##### containers.conf

No changes except `/etc/containers/containers.rootless.conf` search location has been removed. It has just been added
in 5.7 so I don't think it would cause major concerns to drop it again.

The reason to not support the main file with rootless/rootful now is that it is not obvious how this should interact
with the parsing, should it replace the main config file or act like a drop in? As such I think it is better to not
support this and we should generally push all users to use a drop instead of editing the main file.
Comment on lines +196 to +198
Copy link
Contributor

Choose a reason for hiding this comment

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

Is that practical? Wouldn't the rootful and rootless configs differ quite a bit, making a rootless drop-in pretty annoying to maintain, with most containers.conf updates requiring an immediate drop-in update to undo/replace that change for rootless?

(I have no idea.)

Copy link
Member Author

Choose a reason for hiding this comment

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

I am not sure I got your point.

The way I see it the default file a vendor ships should be a fully commented out file. Any customisations a package wants to do should be put into a drop in, i.e. 50-fedora.conf or so something.

Then if the fields are for everything put it into the regular conf.d, if it is root only rootful.conf.d and rootless into rootless.conf.d, so there should be no need to undo anything as rootless.

Copy link
Contributor

@mtrmac mtrmac Dec 4, 2025

Choose a reason for hiding this comment

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

Yes, that would work. It’s not a very natural workflow — tutorials are going to say “edit containers.conf” — but users who run into this can be pointed at this approach.


Also there was/is some discussion of splitting containers.conf in two files as currently there are fields in there
that are only read on the server side while others only get used on the client side which makes using it in a remote
context such as podman machine confusing. For now this is not part of this design doc, we may make another design docs
just for this in case we like to move forward on it.

containers.conf also supports "Modules", i.e. `podman --module name.conf ...` which adds additional drop-in files at
the end after the regular config files. This functionality should be preserved but don't expand module support to the
other files.

##### storage.conf

Deprecate `rootless_storage_path` option. With the `rootless/rootful` config location and admin could just use
`graphroot` in the location in the rootless file location. As such there is no need to special case these fields
in the parser.

String arrays in the config will need to get switched to the attributedstring type, as described under Appending arrays.

##### registries.conf
Copy link
Contributor

Choose a reason for hiding this comment

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

There’s another different behavior, see newConfigWrapperWithHomeDir : If there is a per-user registries.conf, the system-wide registries.conf.d are completely ignored. That was designed that way intentionally, historically; the UAPI spec explicitly defines the opposite.

I don’t know what we want to do here…

Copy link
Member Author

Choose a reason for hiding this comment

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

I think the UAPI behaviour is better and we should adopt that.

The best argument I can make is consider the shortnames currently shipped as /etc/containers/registries.conf.d/000-shortnames.conf

Now say I want to add a registry for my user and do this in my home dir I then loose access to all these shortnames and have to copy the file (or I guess better symlink to receive updates)

In practise I would say almost all people would want to keep the shortnames, now with UAPI if people want to unset the default shortnames all they would need to do is add an empty file ~/.config/containers/registries.conf.d/000-shortnames.conf so they still can do that if that is what they wanted.


Remove V1 config layout to simplify the parsing logic. If we do major config changes we might as well take the
chance and remove this old format.
Currently the V1 format is already rejected for drop-in files so this just effects the main config file.

Additionally there might be a few challenges here, c/image uses the SystemContext struct which allows
users to set `RootForImplicitAbsolutePaths`, `SystemRegistriesConfPath`, `SystemRegistriesConfDirPath`.
We must continue to support them as they are used by various tools.
For `RootForImplicitAbsolutePaths` we update it to check bot the `/usr` and `/etc` locations.
When `SystemRegistriesConfPath` or `SystemRegistriesConfDirPath` are used don't do the normal parsing
and just read the file/directory specified there.
Copy link
Contributor

Choose a reason for hiding this comment

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

… and ignore environment variables, to be explicit.


As described under the Environment Variables section the handling for `CONTAINERS_REGISTRIES_CONF` is moved
out of Podman, and common/libimage into the actual `configfile` package. As such all users of c/image will
be able to use this without having each caller specify their own env.
As part of this I propose removing support for the old `REGISTRIES_CONFIG_PATH` env which was never documented
and replaced by `CONTAINERS_REGISTRIES_CONF` in commit c9ef2607104a0b17e5146b3ee01852edb7d3d688 (over 4 years ago).
Currently there is an issue because Buildah doesn't support it [4].

String arrays in the config will need to get switched to the attributedstring type, as described under Appending arrays.
Copy link
Contributor

Choose a reason for hiding this comment

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

Assuming entries without the special {append=true} continue to be work without change, as a full override, WFM.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes without the special entry thing work the same. Now there is one caveat here of course that this means changing the data type of the config fields.
V2RegistriesConf seems to be exported in same cases (TryUpdatingCache, which says it is deprecated). I am not sure how important preserving type compatibility there is, it doesn't seem like there is much/any use for callers to use this struct directly but who knows. If we can break it that would be easiest to implement though if you prefer we don't we could still create a second type and copy filed into the current one before returning but it would mean a bit more extra code to maintain.

Copy link
Contributor

Choose a reason for hiding this comment

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

We already have a separate private parsedConfig.

The major external user I’m aware of is openshift/machine-config-operator + openshift/runtime-utils, parsing ~arbitrary files and updating fields in them, and that includes the affected UnqualifiedSearchRegistries. So, whether we add a new type or not, they will need to update c/image. (The good news is that they don’t ~export the type, and don’t have stable API commitments…)

Supporting external config file users / editors, as features evolve, is always tricky (compare podman image sign for registries.d elsewhere).

I’m inclined to freeze/deprecate the V2RegistriesConf type, and to… have AI write… a tedious encapsulated API where we don’t expose the data structure but provide a set of functions (“find registry entry for $scope; add mirror”…) — under the theory that we can always add more functions/methods — but every imaginable API always commits us to something about the design, so encapsulating is by no means future-proof.

Freezing+deprecating V2RegistriesConf as is, and adding … V2RegistriesConfV2 as a public type … would also not be too bad.


#### Maybe in scope?

##### registries.d

Note registries.d is confusingly a completely different config file format from registries.conf.d.

It is unclear to me what we should do with this. Do we want to adopt the drop in logic as explained above?
Right now it only uses `$HOME/.config/containers/registries.d` or `/etc/containers/registries.d`

I think for consistency it would be best if it uses all drop in paths like shown above.
At least we should add `/usr/share/containers` lookup locations.

Additionally there seems to exit code duplication in `podman/pkg/trust`. We should find a way to
not duplicate this logic at all.

##### certs.d
Copy link
Contributor

Choose a reason for hiding this comment

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

This is supposed to be compatible with Docker. I suppose we can add more paths, as long as the old one stays and its behavior does not incompatibly change.


Same point as for registries.d. Should we make it also use all paths?
Copy link
Contributor

Choose a reason for hiding this comment

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

I’d think there would not be much of a benefit. It is possible to configure multiple client certificates + private keys, but I don’t know why anyone would, or at least, why different administrators would define separate keys for the same server in separate locations.

Hypothetically we might allow drop-ins that add more accepted certificate authorities … I’d prefer not adding easy-to-miss ways to subvert security. That’s an intuitive guess.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah given the certs dir isn't really a config file I guess there is no point supporting a ton of lookup locations and I guess there would be no demand for the rootful/rootless directories?

How about we only add the /usr/share/containers/certs.d location for consistency so packages can ship their certs there if wanted.

Copy link
Contributor

Choose a reason for hiding this comment

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

I’d expect distributions to have a strict policy against ~random packages configuring system-wide trusted CAs. (But then again, elfutils-default-yama-scope has existed for 10 years and is only being really discussed this year…)

Individual sites might want to package their configuration in RPMs, and perhaps use /usr/ paths for that I suppose. I don’t think there is a clear benefit to that, to outweigh the risk of “hiding” unexpected CAs in another location, but I’m not strongly opposed. (Noting https://docs.fedoraproject.org/en-US/quick-docs/using-shared-system-certificates/ , it seems that /usr is already accepted for trusted CAs in most other contexts.)

There is also an open issue for proper XDG_CONFIG_HOME support [2].

##### policy.json

This is a json file so I don't think the normal drop-in logic would be particular useful here. Though
Copy link
Contributor

Choose a reason for hiding this comment

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

My thinking here was that I don’t want any ambiguities in the security-critical config, so duplicates within the single policy.json are strictly rejected. Drop-ins would sort of force us to support duplicates/overrides. Not a disaster but a bit of a worry.

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah I am not trying to push for drop-ins policy.json. I think single fine is fine for it atm.

I think it should read at least also `/usr/share/containers/policy.json`.
Copy link
Contributor

Choose a reason for hiding this comment

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

The worry here is “If you make a typo when installing /etc/containers/policy.json, your configuration is going to be ignored”.

I could see an argument that this is no different from any other config file… yet, right now, on a typo, the policy.json mechanism “fails closed” and forces the user to fix that before being able to pull images. (Admittedly there are other typos where the “fails closed” property can’t be upheld, e.g. a typo when installing an updated policy is undetectable by c/image.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Well the obvious argument is you have the exact same problem today.
If I typo the path for my custom homedir config it falls back to the /etc one which I think basically on all distro which ship is is a accept anything? So really adding one more location is not going to change that?

Users/Admins are responsible for configuring the systems we cannot protect for so things, podman image trust show outputs the policy so admin should verify that and I guess log-level debug etc...
If you have an important policy it is always the end user would should actually tests that the policy gets enforced by us.


It is also missing the XDG_CONFIG_HOME support [3].
Comment on lines +239 to +264
Copy link
Member Author

Choose a reason for hiding this comment

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

I am not to sure how to handle these best. I guess it would be nice to consolidate the logic there as well but it also increased the scope.


## **Use cases**

A better way to configure podman and a better understanding for users how to do it without having
to worry that each config file behaves differently.
Since we then have only once place that defines the load order we can have a single man page
documenting the load order as described above. All the config files man pages can then just
refer to that and we don't have to duplicate so much docs.

By adding `/usr/share/containers` locations for all config files vendors can properly ship default
configurations there without causing package/user conflicts in `/etc` when admins also want to set
a default config.
Comment on lines +274 to +276
Copy link
Contributor

Choose a reason for hiding this comment

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

A downside to this is that we lose control on upgrades. With /etc, there’s at least .rpmold/.rpmnew — or even a conflict and an upgrade failure might be better than silently continuing with an obsolete config.

Probably not enough of a downside, just something to think about.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it is the advantage of /usr to not have these kinds of package install conflicts at all,
systemd people are clearly advocating for this /usr model
Practically speaking I have no idea how people use the .rpmold/.rpmnew files, personally I never touched any of them nor looked at them anywhere on all my systems.

Copy link
Contributor

Choose a reason for hiding this comment

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

Practically speaking I have no idea how people use the .rpmold/.rpmnew files

diff / diff3 I guess. But that assumes an admin who pays attention and notices that the file was created at all — relevant only for “pet” systems.

Copy link
Contributor

Choose a reason for hiding this comment

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

… oh, and: a config file in /usr/ being automatically updated protects the packager’s configuration. The “benefit” of conflicts and failures is that it protects the user’s configuration. Sure, ideally users would read release notes.

This helps "Image Mode" or "Atomic" distributions which tend to prefer using /usr for configuration
when possible, i.e.
https://bootc-dev.github.io/bootc/building/guidance.html#configuration-in-usr-vs-etc
Given the increased importance of podman on such distributions it makes sense to support /usr configs
universally.

## **Target Podman Release**

6.0 (Because this is a breaking change a major release is required and this work must be finished in time)

## **Link(s)**


- [1] https://github.com/containers/podman/issues/27227
- [2] https://github.com/containers/container-libs/issues/183
- [3] https://github.com/containers/container-libs/issues/202
- [4] https://github.com/containers/buildah/issues/6468
- https://github.com/containers/container-libs/issues/164
- https://github.com/containers/container-libs/issues/476

- https://github.com/containers/container-libs/issues/234


## **Stakeholders**


- [x] Podman Users
- [x] Podman Developers
- [x] Buildah Users
- [x] Buildah Developers
- [x] Skopeo Users
- [x] Skopeo Developers
- [x] Podman Desktop
- [x] CRI-O
- [x] Storage library
- [x] Image library
- [x] Common library
- [ ] Netavark and aardvark-dns

## ** Assignee(s) **

- Paul Holzinger (@Luap99)

## **Impacts**

### **CLI**

The cli should not change based on this.
Copy link
Member

Choose a reason for hiding this comment

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

one thing worth thinking about is how --storage-driver and --storage-opt can be made to work nicely with the new configuration proposed here.

The fact that using these flags reset completely anything coming from the storage.conf configuration file is very annoying and it happened more than once that users got bitten by this.

Copy link
Member Author

Choose a reason for hiding this comment

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

That is a good point, I find it definitely relates to this work though it doesn't really intersect directly I think?

I don't know the history behind why this unset logic was chosen. If there is demand and others agree we can certainly add this.

Copy link
Member

Choose a reason for hiding this comment

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

I don't know the reason behind it, but it is highly confusing.

The intersection is that now we don't "merge" storage configuration files, so to behave like the CLI does, we would just need to pick the last storage.conf that overrides everything that was configured earlier (which is not nice).

Copy link
Member

Choose a reason for hiding this comment

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

I think the original logic was that if you overrode driver, your storage options would become invalid so it was necessary to unset them all. But, if that were the case, why not just say that it is required to pass --storage-opt if you pass --storage-driver? I don't see a good reason to retain the current behavior.

Copy link
Contributor

Choose a reason for hiding this comment

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

The intersection is that now we don't "merge" storage configuration files,

… that reminds me of containers/storage#1885, and the various “stringly typed” storage options. And how I am scared of the path + parsing control flow in c/storage/types.

Consider yourself warned that adding drop-in support to this is going to be demanding.


### **Libpod**

No changes to libpod.

### **Others**

The major work will happen in the container-libs monorepo as this contains the file parsing logic for all.

## **Further Description (Optional):**

<!--
Is there anything not covered above that needs to be mentioned?
-->

## **Test Descriptions (Optional):**

The code should be designed in a way to be unit testable and that they get parsed in the right order.