feat(docker): Support corporate proxies and package mirror overrides for base image builds.#1991
Merged
jackluo923 merged 17 commits intoy-scope:mainfrom Mar 7, 2026
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds corporate-proxy support and optional mirror overrides to multiple Docker base-image builds by introducing a proxy helper, a CA installer, updated Dockerfiles that stage host CA and set system CA env vars, and build scripts that inject proxy/mirror build args and finalize builds. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@components/core/tools/docker-images/clp-env-base-ubuntu-jammy/Dockerfile`:
- Around line 28-36: The sed replacement in the Dockerfile block that runs when
ARG APT_MIRROR_URL is set only matches "http://" and should be made
protocol-agnostic; update the sed expressions used in the RUN (the sed -i ...
/etc/apt/sources.list invocation) to match both http and https (e.g., use a
pattern that allows an optional "s" after "http") so the mirror override works
for HTTPS mirrors, then leave the apt-get update and conditional logic (ARG
APT_MIRROR_URL and the surrounding RUN) unchanged.
In `@components/core/tools/docker-images/proxy-lib.sh`:
- Around line 22-38: detect_ca_bundle currently only scans hardcoded file paths
and misses user-specified bundles; update the detect_ca_bundle function to first
check the SSL_CERT_FILE environment variable (e.g., if [[ -n
"${SSL_CERT_FILE:-}" && -f "${SSL_CERT_FILE}" ]] then echo it and return 0)
before iterating the ca_paths array so any custom or corporate CA bundle pointed
to by SSL_CERT_FILE is used with priority.
jackluo923
force-pushed
the
feat/docker-corporate-proxy-support
branch
from
869010c to
8eb4d74
Compare
…e image builds Add proper CA certificate injection, proxy env var forwarding, package mirror overrides, and build convenience flags to all 6 Docker base image variants. Builds behind TLS-intercepting proxies now work without disabling SSL verification.
jackluo923
force-pushed
the
feat/docker-corporate-proxy-support
branch
from
8eb4d74 to
1a7f65b
Compare
Member
Author
|
Addressed both review comments:
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@components/core/tools/docker-images/clp-env-base-musllinux_1_2-x86_64/Dockerfile`:
- Around line 27-31: The sed invocation that replaces the default repository URL
using ARG APK_MIRROR_URL can break if the user-supplied APK_MIRROR_URL contains
the '|' delimiter; modify the RUN block that contains the sed command so it
first validates/handles that case: either choose a safer delimiter (e.g., '@' or
use awk/printf to rewrite /etc/apk/repositories) or add a guard that checks
APK_MIRROR_URL for '|' and fails early with a clear error, ensuring the
replacement step (the sed/rewriting of /etc/apk/repositories) only runs when the
value is safe.
In `@components/core/tools/docker-images/proxy-lib.sh`:
- Around line 97-104: The git commands can emit errors to stderr when not in a
repo; update the checks and label retrievals to suppress stderr by redirecting
it (e.g., add 2>/dev/null) for the git invocations used in the condition and
when setting labels (the calls to git -C "$script_dir" rev-parse
--is-inside-work-tree, git -C "$script_dir" rev-parse HEAD and git -C
"$script_dir" remote get-url origin) so the conditional still detects a worktree
but any error output is discarded and build output stays clean.
Member
Author
|
Addressed the second round of review comments:
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@components/core/tools/docker-images/proxy-lib.sh`:
- Around line 130-137: The localhost check in the loop over proxy_vars (used to
set _build_cmd and has_localhost_proxy) currently regex-matches any value
containing "localhost" and misses IPv6 [::1]; change the conditional to only
match exact loopback addresses (allowing optional :port) such as "localhost",
"127.0.0.1", or "[::1]" (e.g. regex like
^(\[::1\]|localhost|127\.0\.0\.1)(:[0-9]+)?$) so hostnames containing
"localhost" don't false-match and IPv6 loopback is detected; update the
conditional that sets has_localhost_proxy accordingly while leaving the rest of
the loop logic intact.
- Around line 97-104: The git remote URL lookup can fail and produce an empty
label; update the block that populates _build_cmd_ref (the commands using git -C
"$script_dir" rev-parse HEAD and git -C "$script_dir" remote get-url origin) to
silence stderr and/or verify the origin remote exists before adding the source
label: run the rev-parse and remote get-url commands with stderr redirected to
/dev/null (or test for the remote with git -C "$script_dir" remote get-url
origin >/dev/null 2>&1 or git -C "$script_dir" remote show origin >/dev/null
2>&1) and only append the org.opencontainers.image.source label if the remote
lookup succeeds, keeping the rev-parse HEAD label behavior consistent.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@components/core/tools/docker-images/proxy-lib.sh`:
- Around line 49-61: The prepare_ca_cert_for_build function currently ignores
failures from cp and proceeds to touch the dest file; update it to propagate
errors by either enabling errexit locally or explicitly checking cp's exit
status: after detect_ca_bundle returns a path, attempt cp "$ca_bundle" "$dest"
and if cp fails, print an error to stderr (including $ca_bundle and $dest) and
return a non-zero status (or exit) so the caller/build fails; keep the existing
success and no-op branches for the no-bundle case and reference the
prepare_ca_cert_for_build and detect_ca_bundle symbols when making the change.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@components/core/tools/scripts/lib_install/setup-corporate-proxy.sh`:
- Line 47: The file ends with the line rm "$ca_cert" but lacks the
POSIX-required trailing newline; update the script so that the last line (the rm
"$ca_cert" statement) is followed by a newline character (i.e., ensure the file
ends with a final blank line) so tools that expect newline-terminated files
behave correctly.
junhaoliao
requested changes
Feb 25, 2026
Member
junhaoliao
left a comment
junhaoliao
left a comment
There was a problem hiding this comment.
lgtm besides the posted questions. if time allows, let's add some docs in docs/src/dev-docs/tooling-containers.md in this PR or a future one
Co-authored-by: Junhao Liao <junhao@junhao.ca>
- Rename _build_cmd_ref to _build_cmd in finalize_build for consistency - Default DOCKER_PULL to true to match prior build.sh behavior - Extract git revision into a local variable for line-length compliance - Add corporate proxy and mirror documentation to tooling-containers.md
junhaoliao
requested changes
Feb 26, 2026
Member
junhaoliao
left a comment
junhaoliao
left a comment
There was a problem hiding this comment.
to fix MD060/table-column-style
(detectable by npx markdownlint-cli2 --fix docs/src/dev-docs/tooling-containers.md --config tools/yscope-dev-utils/exports/lint-configs/.markdownlint-cli2.yaml)
Member
|
for the title, how about: |
Co-authored-by: Junhao Liao <junhao@junhao.ca>
Co-authored-by: Junhao Liao <junhao@junhao.ca>
Co-authored-by: Junhao Liao <junhao@junhao.ca>
junhaoliao
approved these changes
Mar 3, 2026
Member
junhaoliao
left a comment
junhaoliao
left a comment
There was a problem hiding this comment.
how do you think about the previously proposed title? i.e.,
feat(docker): Support corporate proxies and package mirror overrides for base image builds.
Member
|
@coderabbitai create a bug report
junhao@ASUS-X870E:~/workspace/8-clp/components/core/tools/docker-images/clp-env-base-ubuntu-jammy$ ./build.sh
Corporate proxy support: copying CA bundle from /etc/ssl/certs/ca-certificates.crt
Running: docker build --tag clp-core-dependencies-x86-ubuntu-jammy:dev /home/junhao/workspace/8-clp/components/core/tools/docker-images/clp-env-base-ubuntu-jammy/../../../ --file /home/junhao/workspace/8-clp/components/core/tools/docker-images/clp-env-base-ubuntu-jammy/Dockerfile --pull --label org.opencontainers.image.revision=e3d23ee49a84b6eb3862d173c77d50611ff45bad --label org.opencontainers.image.source=https://github.com/y-scope/clp.git
[+] Building 1.4s (15/18) docker:default
=> [internal] load build definition from Dockerfile 0.0s
=> => transferring dockerfile: 2.82kB 0.0s
=> resolve image config for docker-image://docker.io/docker/dockerfile:1 0.4s
=> [auth] docker/dockerfile:pull token for registry-1.docker.io 0.0s
=> CACHED docker-image://docker.io/docker/dockerfile:1@sha256:4a43a54dd1fedceb30ba47e76cfcf2 0.0s
=> => resolve docker.io/docker/dockerfile:1@sha256:4a43a54dd1fedceb30ba47e76cfcf2b47304f4161 0.0s
=> [internal] load metadata for docker.io/library/ubuntu:jammy 0.3s
=> [auth] library/ubuntu:pull token for registry-1.docker.io 0.0s
=> [internal] load .dockerignore 0.0s
=> => transferring context: 2B 0.0s
=> [base 1/8] FROM docker.io/library/ubuntu:jammy@sha256:ce4a593b4e323dcc3dd728e397e0a866a1b 0.0s
=> => resolve docker.io/library/ubuntu:jammy@sha256:ce4a593b4e323dcc3dd728e397e0a866a1bf516a 0.0s
=> [internal] load build context 0.1s
=> => transferring context: 246.91kB 0.0s
=> CACHED [base 2/8] WORKDIR /root 0.0s
=> [base 3/8] RUN mkdir -p ./tools/scripts/lib_install 0.2s
=> [base 4/8] COPY --link ./tools/scripts/lib_install ./tools/scripts/lib_install 0.1s
=> => merging 0.0s
=> ERROR [base 5/8] RUN ./tools/scripts/lib_install/setup-corporate-proxy.sh 0.1s
------
> [base 5/8] RUN ./tools/scripts/lib_install/setup-corporate-proxy.sh:
0.126 setup-corporate-proxy: installing corporate CA certificates...
0.126 setup-corporate-proxy: WARNING: no recognized CA trust update tool found.
------
Dockerfile:13
--------------------
11 | # proxies (e.g., Zscaler, Fortinet, Palo Alto). No-op when not behind a
12 | # corporate proxy. See proxy-lib.sh for details.
13 | >>> RUN ./tools/scripts/lib_install/setup-corporate-proxy.sh
14 |
15 | # Point pip/pipx and curl at the system trust store. By default, Python's
--------------------
ERROR: failed to build: failed to solve: process "/bin/sh -c ./tools/scripts/lib_install/setup-corporate-proxy.sh" did not complete successfully: exit code: 1 |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainLet me first read the relevant files to understand the build order and confirm the root cause before creating the bug report. 🏁 Script executed: Length of output: 1871 The root cause is clear: Creating the bug report now. 🏁 Script executed: Length of output: 42
Here's a summary of the root cause identified:
Suggested fix: Add 🧠 Learnings used |
3 tasks
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Docker base image builds fail in corporate environments where TLS-intercepting proxies (e.g., Zscaler, Fortinet, Palo Alto) replace upstream SSL certificates with the organization's own CA. Package managers (dnf, apk, apt, pip) reject the proxy's certificate because it isn't in the container's trust store.
This PR adds proper corporate proxy support to all 6 base image variants by:
CA certificate injection —
build.shauto-detects the host's CA bundle and copies it into the Docker build context. An in-container script (setup-corporate-proxy.sh) installs it into the system trust store. When no corporate CA is present, this is a no-op.Python/pip trust store override —
ENVvars (SSL_CERT_FILE,CURL_CA_BUNDLE,REQUESTS_CA_BUNDLE,PIP_CERT) point pip/pipx at the system trust store instead of certifi's bundled CA store.Proxy env var forwarding —
HTTP_PROXY,HTTPS_PROXY,NO_PROXY,ALL_PROXYare forwarded as Docker build args.Package mirror overrides — Each distro supports an env var to override default mirrors (for organization-internal or regional mirrors):
DNF_MIRROR_BASE_URLfor manylinux_2_28 (AlmaLinux) and centos-stream-9APK_MIRROR_URLfor musllinux_1_2 (Alpine)APT_MIRROR_URLfor ubuntu-jammyDocker network mode — Auto-enables
--network hostwhen proxy URLs point to localhost/127.0.0.1. Explicit override viaDOCKER_NETWORKenv var.DOCKER_PULL=true— Opt-in flag to force-pull the latest base image.Error hints — On
install-all.shfailure, displays a helpful banner suggesting mirror override if the error is network-related.All changes are backward-compatible. Without proxy env vars or mirror overrides, builds work identically to before.
Checklist
Validation performed
manylinux_2_28-aarch64image successfully with mirror override (DNF_MIRROR_BASE_URL=<url> ./build.sh)setup-corporate-proxy.shis a no-op when no CA bundle is provided (CI-safe — the GitHub Actions workflow usesdocker/build-push-actiondirectly, bypassingbuild.sh)install-all.shfailure--network hostis only added when proxy URLs contain localhost/127.0.0.1Summary by CodeRabbit
Release Notes
New Features
Improvements