honor containers.conf http_proxy and env settings during build by unlimitedsola · Pull Request #6996 · podman-container-tools/buildah

unlimitedsola · 2026-07-26T19:24:17Z

What type of PR is this?

/kind bug

What this PR does / why we need it

This PR resolves an issue where podman build ignored containers.conf settings (http_proxy = false and [containers] env). Please also see the linked issue for more context.

Binds the --http-proxy CLI flag default in pkg/cli/common.go to Containers.HTTPProxy from containers.conf instead of hardcoding true. This makes build proxy flag defaults consistent with podman run, allowing http_proxy = false in containers.conf to disable host environment proxy passthrough by default.
Updates configureEnvironment() in run_common.go to inject environment variables defined under [containers] env in containers.conf into the transient container process spec (g.AddProcessEnv) during RUN steps. This fixes the regression from pre-6.0.x where containers.conf environment settings applied to build execution commands (e.g. package management and network requests).

How to verify it

Configure containers.conf:

[containers]
http_proxy = false
env = [
  "http_proxy=http://host.containers.internal:1080",
]

Export host shell environment variables:
```
export http_proxy=http://127.0.0.1:1080
```

Run build:

podman build --no-cache -f - . << 'EOF'
FROM alpine
RUN env | grep -i proxy
EOF

Verify output contains http_proxy=http://host.containers.internal:1080 instead of http://127.0.0.1:1080.

Which issue(s) this PR fixes

podman-container-tools/podman#29299

Special notes for your reviewer

g.AddProcessEnv in configureEnvironment() sets the process spec environment (g.Config.Process.Env) for transient container execution during RUN steps. It does not touch builder.OCIv1.Config.Env, ensuring that environment variables configured in containers.conf apply to build execution without polluting committed image layers.

Does this PR introduce a user-facing change?

Not sure if a release note is needed for regressions, but this does contain user-facing change:

Honor `containers.conf` `http_proxy` and `env` settings during image builds, aligning the behavior with `podman run` and fixed regression introduced in 6.0.x.

simonbrauner · 2026-07-27T08:42:15Z

 		}
 	}

+	if conf, err := config.Default(); err == nil {


I would suggest against silently ignoring the error, similarly to other calls of config.Default in the file.

if err != nil { return nil, fmt.Errorf("failed to get container config: %w", err) }

Thanks for the feedback! I changed the function signature so that the error can now bubble up instead of silently ignored.

nalind

LGTM, but I expect you'll need to run make fmt to fix the linter formatter warning.

TomSweeneyRedHat · 2026-07-28T19:14:04Z

LGTM
once the gofmt issue on line 87 in the test is made happy

nalind · 2026-07-28T20:17:58Z

Please squash your commits before merging. Thanks!

packit-as-a-service · 2026-07-28T21:35:57Z

Ephemeral COPR build failed. @containers/packit-build please check.

unlimitedsola · 2026-07-29T14:51:33Z

Ephemeral COPR build failed. @containers/packit-build please check.

I've squashed and rebased my commits. I think this was caused by me force-pushed twice too quickly?

unlimitedsola · 2026-08-11T07:38:50Z

The CI failure seems unrelated to the code change. Let me know if there is anything that I can help to move this forward.

simonbrauner

I added a few more comments. Otherwise, LGTM

Don't worry about unrelated test failures, somebody who has the permissions can rerun the tests later.

simonbrauner · 2026-08-11T13:29:22Z

@@ -123,7 +123,9 @@ func (b *Builder) Run(command []string, options RunOptions) error {
 	}

 	// hardwire the environment to match docker build to avoid subtle and hard-to-debug differences due to containers.conf


This comment implies that containers.conf won't be used at all, while the PR introduces the possibility of loading proxy variables from containers.conf.

I would propose updating it with something like this:

Suggested change

// hardwire the environment to match docker build to avoid subtle and hard-to-debug differences due to containers.conf

// Pass a fixed environment to match docker build because passing the whole containers.conf instead would cause

// subtle and hard-to-debug differences. Proxy variables are the exception, as they are read from containers.conf in configureEnvironment.

Appreciate your feedback! I've updated the comment as you suggested.

simonbrauner · 2026-08-11T13:29:34Z

@@ -230,7 +230,9 @@ func (b *Builder) Run(command []string, options RunOptions) error {
 	}

 	// hardwire the environment to match docker build to avoid subtle and hard-to-debug differences due to containers.conf


Same as https://github.com/podman-container-tools/buildah/pull/6996/changes#r3758367668.

also updated as suggested

simonbrauner · 2026-08-11T14:14:10Z

+`
+	err := os.WriteFile(confPath, []byte(confContent), 0o644)
+	require.NoError(t, err)
+


As far as I can tell, the t.Setenv followed by config.New(&config.Options{SetDefault: true}) overwrites the config cache there https://github.com/podman-container-tools/container-libs/blob/16d801f1d5b2e615f9eb11f34ce007e428a294a6/common/pkg/config/new.go#L93C1-L96C3 and does not revert the change, which could cause problems later in the tests.

I'd propose running config.New(&config.Options{SetDefault: true}) on clean up (registered before the t.Setenvs because they call Cleanup internally and the clean up happens in the LIFO order), after the environment change of t.Setenv is restored already, so that we have the initial state again:

Suggested change

t.Cleanup(func() { _, _ = config.New(&config.Options{SetDefault: true}) })

Interesting, I wasn't aware that the config cache would persist across tests. I now registered the cleanup callback before the set envs and added comments to explain what it is for.

I'd improve the wording of the comment. It is not the cause that t.Cleanup before t.Setenv causes t.Cleanup run in LIFO order, but rather than cleanups always run in LIFO order, which is the reason why we need to register the reset function before t.Setenv register their own.

// Reset the default container config cache on test cleanup. Register before t.Setenv so that // the cleanups caused by `t.Setenv` calls run before it (because cleanup happens in LIFO order).

This is indeed less ambiguous. I've updated both comments as suggested.

simonbrauner · 2026-08-11T14:15:00Z

+`
+	err := os.WriteFile(confPath, []byte(confContent), 0o644)
+	require.NoError(t, err)
+


Same as https://github.com/podman-container-tools/buildah/pull/6996/changes#r3758728121.

Added the same cleanup and comment as above.

Default the `--http-proxy` CLI flag to `Containers.HTTPProxy` from `containers.conf` instead of hardcoding `true`. This ensures the behavior is consistent with `podman run`, allowing `http_proxy = false` in `containers.conf` to disable host proxy passthrough by default. Additionally, update `configureEnvironment()` to inject proxy environment variables defined under `[containers] env` in `containers.conf` into the transient container process environment during `RUN` steps. This restores the pre-6.0.x behavior where `containers.conf` environment settings applied to build execution steps, while ensuring runtime proxy settings do not leak `ENV` directives into committed image metadata. Signed-off-by: Sola <dev@sola.love>

simonbrauner · 2026-08-12T12:43:23Z

My comments were addressed.

simonbrauner reviewed Jul 27, 2026

View reviewed changes

nalind reviewed Jul 27, 2026

View reviewed changes

Comment thread run_common.go

nalind approved these changes Jul 28, 2026

View reviewed changes

unlimitedsola force-pushed the honor-containers-conf branch 2 times, most recently from fb9edc4 to 0b2dad6 Compare July 28, 2026 21:35

nalind approved these changes Jul 29, 2026

View reviewed changes

unlimitedsola force-pushed the honor-containers-conf branch from 0b2dad6 to 96d1c94 Compare August 8, 2026 03:56

simonbrauner reviewed Aug 11, 2026

View reviewed changes

unlimitedsola force-pushed the honor-containers-conf branch from 96d1c94 to aea385c Compare August 12, 2026 06:53

unlimitedsola force-pushed the honor-containers-conf branch from aea385c to aee594d Compare August 12, 2026 12:31

		@@ -123,7 +123,9 @@ func (b *Builder) Run(command []string, options RunOptions) error {
		}

		// hardwire the environment to match docker build to avoid subtle and hard-to-debug differences due to containers.conf

	// hardwire the environment to match docker build to avoid subtle and hard-to-debug differences due to containers.conf
	// Pass a fixed environment to match docker build because passing the whole containers.conf instead would cause
	// subtle and hard-to-debug differences. Proxy variables are the exception, as they are read from containers.conf in configureEnvironment.

		@@ -230,7 +230,9 @@ func (b *Builder) Run(command []string, options RunOptions) error {
		}

		// hardwire the environment to match docker build to avoid subtle and hard-to-debug differences due to containers.conf



	t.Cleanup(func() { _, _ = config.New(&config.Options{SetDefault: true}) })

Uh oh!

Conversation

unlimitedsola commented Jul 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What type of PR is this?

What this PR does / why we need it

How to verify it

Which issue(s) this PR fixes

Special notes for your reviewer

Does this PR introduce a user-facing change?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nalind left a comment

Choose a reason for hiding this comment

Uh oh!

TomSweeneyRedHat commented Jul 28, 2026

Uh oh!

nalind commented Jul 28, 2026

Uh oh!

packit-as-a-service Bot commented Jul 28, 2026

Uh oh!

unlimitedsola commented Jul 29, 2026

Uh oh!

unlimitedsola commented Aug 11, 2026

Uh oh!

simonbrauner left a comment • 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!

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

simonbrauner commented Aug 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

unlimitedsola commented Jul 26, 2026 •

edited

Loading

simonbrauner left a comment •

edited

Loading