Skip to content

config: add env_merge support to containers.conf#791

Open
jiwahn wants to merge 1 commit into
containers:mainfrom
jiwahn:feat/env-merge-containers-conf-28410
Open

config: add env_merge support to containers.conf#791
jiwahn wants to merge 1 commit into
containers:mainfrom
jiwahn:feat/env-merge-containers-conf-28410

Conversation

@jiwahn
Copy link
Copy Markdown

@jiwahn jiwahn commented Apr 25, 2026

Allow users to set default env_merge values in containers.conf. Intended to work the same as the --env-merge CLI option.

Changes

  • Add EnvMerge []string field to ContainersConfig struct (config.go)
  • Set default value to empty slice (default.go)
  • Add commented-out env_merge = [] entry to containers.conf template
  • Add documentation to containers.conf.5.md

@github-actions github-actions Bot added the common Related to "common" package label Apr 25, 2026
@TomSweeneyRedHat
Copy link
Copy Markdown
Member

restarted failed tests, not related to the change.
IDK if possible, but a test would be nice if one could be created.
@Luap99 WDYT?

Comment thread common/pkg/config/default.go Outdated
@jankaluza
Copy link
Copy Markdown
Member

I think adding test into common/pkg/config/config_local_test.go would be great. I did not explore it a lot, but maybe adding env_merge into some testdata in common/pkg/config/testdata/containers_default and verifying it is loaded properly in config_local_test.go would be enough.

You could also test append=true with modules_test.go. It already does that with env.

@Luap99
Copy link
Copy Markdown
Member

Luap99 commented May 5, 2026

as for a tests here is an example commit b2db08d

@jiwahn jiwahn force-pushed the feat/env-merge-containers-conf-28410 branch from 79cd99a to a711841 Compare May 7, 2026 05:42
@jiwahn
Copy link
Copy Markdown
Author

jiwahn commented May 7, 2026

Thanks guys for the comments and the helpful example. I have updated the changes.

changes

  • Changed ContainersConfig.EnvMerge from []string to configfile.Slice so env_merge supports string-array features such as append=true.
  • Updated the default EnvMerge value to use configfile.Slice{}.
  • Added a config parsing test to verify that env_merge is loaded from containers_default.conf.
  • Extended the modules test to verify that env_merge supports append=true across module configs.

result

go test ./common/pkg/config -ginkgo.focus="Config Modules"
go test ./common/pkg/config -ginkgo.focus="should parse env_merge from config file"
ok      go.podman.io/common/pkg/config  0.006s
ok      go.podman.io/common/pkg/config  0.007s

@jiwahn jiwahn force-pushed the feat/env-merge-containers-conf-28410 branch from a711841 to 5031f6b Compare May 7, 2026 05:47
@jiwahn jiwahn requested review from Luap99 and jankaluza May 7, 2026 05:56
@Luap99
Copy link
Copy Markdown
Member

Luap99 commented May 7, 2026

please squash the commits into a single one

@jiwahn jiwahn force-pushed the feat/env-merge-containers-conf-28410 branch from 5031f6b to 44aeee5 Compare May 7, 2026 10:38
@jiwahn
Copy link
Copy Markdown
Author

jiwahn commented May 7, 2026

of course, rebased

Allow users to set default env_merge values in containers.conf.
Works the same as the --env-merge CLI option.

Fixes: containers/podman#28410
Signed-off-by: Jiwoo Ahn <ikwydls1314@gmail.com>
@jiwahn jiwahn force-pushed the feat/env-merge-containers-conf-28410 branch from 44aeee5 to e1e7a15 Compare May 13, 2026 02:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

common Related to "common" package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants