build: forward proxy settings into the container image builds - #113
build: forward proxy settings into the container image builds#113cleverhu wants to merge 1 commit into
Conversation
The `--setup-only` layer downloads regctl, firecracker, the kernel, the overlaybd package and the tools image, which is the slowest part of a cold build behind a restricted network. Forward the caller's proxy environment into every build stage from both `make k8s-build` and docker compose, and enable reqwest's socks feature so socks5:// proxy variables are honoured rather than silently ignored. The proxy variables are passed as predefined BuildKit build args, so they reach every stage without an ARG declaration and stay out of the image environment, docker history and the layer cache key. Unset variables expand to nothing and leave the build commands unchanged.
|
🔍 OpenCodeReview found 2 issue(s) in this PR.
|
| reqwest = { version = "0.13", features = ["json", "stream"] } | ||
| # `socks` lets dependency downloads honour socks5:// values in the standard | ||
| # proxy environment variables; without it reqwest silently ignores them. | ||
| reqwest = { version = "0.13", features = ["json", "stream", "socks"] } |
There was a problem hiding this comment.
[bug · high]
The committed Cargo.lock was not regenerated for this feature change: its reqwest 0.13.3 entry has no SOCKS dependency (such as tokio-socks). Consequently, reproducible/locked builds will reject the stale lockfile, while unlocked builds produce an uncommitted lockfile update. Run cargo update -p reqwest@0.13.3 (or cargo check) and stage the resulting Cargo.lock changes.
| DOCKER_PROXY_ARGS := \ | ||
| $(if $(HTTP_PROXY),--build-arg HTTP_PROXY="$(HTTP_PROXY)" --build-arg http_proxy="$(HTTP_PROXY)",) \ | ||
| $(if $(HTTPS_PROXY),--build-arg HTTPS_PROXY="$(HTTPS_PROXY)" --build-arg https_proxy="$(HTTPS_PROXY)",) \ | ||
| $(if $(ALL_PROXY),--build-arg ALL_PROXY="$(ALL_PROXY)" --build-arg all_proxy="$(ALL_PROXY)",) \ | ||
| $(if $(NO_PROXY),--build-arg NO_PROXY="$(NO_PROXY)" --build-arg no_proxy="$(NO_PROXY)",) |
There was a problem hiding this comment.
[security · medium]
Proxy URLs commonly contain credentials. Expanding their values directly into the recipe causes make to echo those credentials into terminal/CI logs, and also exposes them in the docker build process arguments. Export the upper/lower-case proxy variables and use valueless options such as --build-arg HTTPS_PROXY --build-arg https_proxy; Docker will read their values from the client environment without placing them in the displayed command line. Merely prefixing the recipe with @ would address logs but not process-argument exposure.
What
Forward the caller's proxy environment into the container image builds from both
make k8s-buildand docker compose, and enable reqwest'ssocksfeature.Why
The
--setup-onlylayer ofDockerfile.agentenvdownloads regctl, firecracker, the kernel, the overlaybd package and the ghcr.io tools image. Behind a restricted network that is by far the slowest part of a cold build, and there was no way to point it at a proxy short of editing the Dockerfile.The
socksfeature is needed because reqwest reads the standard proxy environment variables but silently ignoressocks5://values when the feature is off. A build that appears to honourHTTPS_PROXYthen goes direct and hangs, with no diagnostic.Related issue
None; build-time developer experience.
Scope and non-goals
Included: proxy passthrough in the Makefile, docker compose and a Dockerfile comment, plus the reqwest feature.
Explicitly excluded, because they were part of the same internal change but are site-specific:
--pushtok8s-build.APT_MIRROR_BASEto a regional mirror. It stays empty.Design and behavior changes
DOCKER_PROXY_ARGSis assembled fromHTTP_PROXY/HTTPS_PROXY/ALL_PROXY/NO_PROXY, each defaulting to its lowercase counterpart. Both spellings are passed through because Go and Rust HTTP clients differ in which they look up. Unset variables expand to nothing, so with no proxy exported the build commands are byte-for-byte what they are today.These four are predefined BuildKit build args. Passing them with
--build-argmakes them available to every stage'sRUNwithout anARGdeclaration, and keeps them out of the image environment,docker history, and the layer cache key — which matters because a proxy URL often carries credentials. The Dockerfile comment records this, since declaringARGexplicitly would scope them to a single stage and leave thedeps-stagedownloads on a direct connection, which is the opposite of the intent.docker-compose.ymlforwards the same four to the agentenv build.Compatibility and operations
sockspulls in reqwest's SOCKS support;Cargo.lockneeds no update andcargo metadata --lockedstill passes.Validation
make fmtmake clippymake test-unitmake -C services test(required whenservices/changes)maketargetCommands and results:
Skipped checks and reasons:
make test-unitnot run: the only Rust change is a Cargo feature flag, covered by a full--all-targetsclippy build.make k8s-buildwas not executed here; the passthrough is verified withmake -nin both the proxy and no-proxy cases, and the equivalent change has been used to build these images behind a SOCKS proxy internally.Risks and reviewer notes
The no-proxy path is the one to check, since it affects everyone:
make -noutput above confirms the commands are unchanged apart from whitespace when no variable is exported.Second point for review: proxy URLs frequently embed credentials. Using the predefined BuildKit args rather than declaring
ARGkeeps them out of the image and out ofdocker history; if maintainers would rather not accept proxy values through the Makefile at all, the compose and Dockerfile parts still stand on their own.Most important file:
Makefile.Checklist