honor containers.conf http_proxy and env settings during build - #6996
honor containers.conf http_proxy and env settings during build#6996unlimitedsola wants to merge 1 commit into
Conversation
| } | ||
| } | ||
|
|
||
| if conf, err := config.Default(); err == nil { |
There was a problem hiding this comment.
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)
}There was a problem hiding this comment.
Thanks for the feedback! I changed the function signature so that the error can now bubble up instead of silently ignored.
nalind
left a comment
There was a problem hiding this comment.
LGTM, but I expect you'll need to run make fmt to fix the linter formatter warning.
|
LGTM |
|
Please squash your commits before merging. Thanks! |
6efe36d to
fb9edc4
Compare
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>
fb9edc4 to
0b2dad6
Compare
|
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? |
What type of PR is this?
/kind bug
What this PR does / why we need it
This PR resolves an issue where
podman buildignoredcontainers.confsettings (http_proxy = falseand[containers] env). Please also see the linked issue for more context.--http-proxyCLI flag default inpkg/cli/common.gotoContainers.HTTPProxyfromcontainers.confinstead of hardcodingtrue. This makes build proxy flag defaults consistent withpodman run, allowinghttp_proxy = falseincontainers.confto disable host environment proxy passthrough by default.configureEnvironment()inrun_common.goto inject environment variables defined under[containers] envincontainers.confinto the transient container process spec (g.AddProcessEnv) duringRUNsteps. This fixes the regression from pre-6.0.x wherecontainers.confenvironment settings applied to build execution commands (e.g. package management and network requests).How to verify it
Configure
containers.conf:Export host shell environment variables:
export http_proxy=http://127.0.0.1:1080Run build:
Verify output contains
http_proxy=http://host.containers.internal:1080instead ofhttp://127.0.0.1:1080.Which issue(s) this PR fixes
podman-container-tools/podman#29299
Special notes for your reviewer
g.AddProcessEnvinconfigureEnvironment()sets the process spec environment (g.Config.Process.Env) for transient container execution duringRUNsteps. It does not touchbuilder.OCIv1.Config.Env, ensuring that environment variables configured incontainers.confapply 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: