feat(docker): Support CA certificates during image builds, and add docker build helpers. - #119
Open
jackluo923 wants to merge 1 commit into
Open
feat(docker): Support CA certificates during image builds, and add docker build helpers.#119jackluo923 wants to merge 1 commit into
jackluo923 wants to merge 1 commit into
Conversation
Contributor
WalkthroughAdded host and container CA trust libraries, Java PKCS#12 generation, Docker Buildx helpers, Docker-free tests, shell lint tasks, and documentation links. ChangesDocker trust and build workflows
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Host as CA trust host library
participant Docker as Docker BuildKit
participant Container as CA trust container setup
participant Java as Java PKCS#12 generator
Host->>Host: Discover and stage CA bundle
Host->>Docker: Add trust build context and runtime mount
Docker->>Container: Provide CA_TRUST_DIR
Container->>Container: Export PEM trust variables
Container->>Java: Generate truststore when CA_TRUST_JVM is enabled
Java-->>Container: Return truststore path and JVM options
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
jackluo923
marked this pull request as draft
August 2, 2026 18:27
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/index.md`:
- Around line 21-22: Update the links in the docs index around “CA trust for
containerized builds” and “Docker build helpers” so they resolve entirely within
the docs tree. Copy or symlink the corresponding export README content into docs
and point the index entries to those in-tree locations, or generate the index
during a documentation build that supports the external targets.
In `@exports/docker/build/host.sh`:
- Around line 118-121: Sanitize the origin URL before assigning it to the OCI
source label so embedded userinfo or credentials are never stored in image
metadata. Update the diagnostic output around the Docker build command to redact
userinfo in proxy URLs and build-argument values, or log only the command name
and non-sensitive flags. Document the revised logging behavior in the Docker
build README.
- Around line 1-13: Update the macOS GitHub Actions shell configuration to
invoke an installed Bash 4.3+ binary for the Docker library tests, and add an
early version check in the host.sh initialization before any nameref-dependent
helpers are parsed. Emit a clear error and stop when the running Bash version is
older than 4.3, while preserving the existing loaded guard behavior.
In `@exports/docker/ca-trust/container.sh`:
- Around line 26-27: Add an explicit keep-in-sync comment beside the hardcoded
ca-bundle.pem assignment in the container setup, referencing host.sh's
CA_TRUST_BUNDLE_FILENAME constant. Do not change the value or surrounding path
logic; document that this filename must be updated together with the host
constant.
- Around line 37-42: Update the JVM trust-store setup condition around
CA_TRUST_JVM so an explicit opt-in that cannot proceed emits a clear diagnostic
when HOST_CA_BUNDLE is empty or keytool is unavailable. Preserve the existing
trust-store generation path when all prerequisites are met, and follow the
nearby host.sh/findmnt skip-message pattern.
In `@exports/docker/ca-trust/generators/java-pkcs12/generate.sh`:
- Around line 29-32: Update the output_trust_store validation in generate.sh to
explicitly reject symbolic links with a -L check before the existing
non-regular-file validation, matching the guard used by ca_trust_stage_host in
host.sh. Preserve the current error-and-exit behavior for invalid output paths.
In `@exports/docker/ca-trust/host.sh`:
- Around line 206-213: Update ca_trust_add_build_args and the corresponding
ca_trust_add_run_args helper to avoid local -n namerefs so the CI path works on
bash 3.2, preserving their existing argument validation and array mutations;
alternatively, add an explicit bash-version guard that clearly reports the
unsupported version before these helpers are used.
In `@exports/docker/ca-trust/README.md`:
- Around line 30-41: Update the Dockerfile example’s ca_trust bind mount in the
RUN instruction to explicitly enable read-write access so CA_TRUST_JVM=1
trust-store generation works during docker build; retain the existing
conditional container.sh and container-exec.sh flow.
In `@taskfiles/docker/test-docker-libs.sh`:
- Around line 186-194: Update the assertion around docker_build_add_proxy_args
to retrieve the HTTPS_PROXY argument from cmd by matching its content rather
than accessing cmd[2]. Preserve the existing verbatim metacharacter check and
ensure the lookup remains safe when nounset is enabled and the helper emits
fewer arguments.
In `@taskfiles/docker/tests.yaml`:
- Around line 10-13: Update the docker-libs-test task to invoke a Bash 4.3+
executable explicitly instead of resolving bash through PATH; use the
repository’s established Bash-version selection or path convention shared with
test-docker-libs.sh, while preserving the existing script invocation.
- Around line 10-13: The Docker library test suite requires Bash 4.3+ but may
run under macOS Bash 3.2. Update taskfiles/docker/tests.yaml:10-13 to invoke a
compatible interpreter or reject older Bash versions; add a BASH_VERSINFO guard
near the strict-mode setup in taskfiles/docker/test-docker-libs.sh:117-146 with
a clear error; and replace the positional cmd[2] access in
taskfiles/docker/test-docker-libs.sh:186-194 with content-based selection of the
HTTPS_PROXY= element so nounset produces a readable assertion failure.
In `@taskfiles/lint/shell.yaml`:
- Around line 6-18: Update the shellcheck command in the lint task to search
both {{.G_EXPORTS_DIR}} and the taskfiles directory, ensuring
taskfiles/docker/test-docker-libs.sh is included while preserving the existing
optional shellcheck behavior and flags.
- Around line 17-18: Update the lint task’s shell script discovery command to
replace the nonportable xargs invocation with find -exec ... +, preserving the
null-safe handling and shellcheck --external-sources arguments while ensuring
shellcheck is not run when no scripts are found.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 50a6b32e-3181-42a7-b810-db9467eecfe6
⛔ Files ignored due to path filters (2)
taskfiles/docker/fixtures/expired-ca.pemis excluded by!**/*.pemtaskfiles/docker/fixtures/valid-ca.pemis excluded by!**/*.pem
📒 Files selected for processing (14)
docs/index.mdexports/docker/build/README.mdexports/docker/build/host.shexports/docker/ca-trust/README.mdexports/docker/ca-trust/container-exec.shexports/docker/ca-trust/container.shexports/docker/ca-trust/generators/java-pkcs12/README.mdexports/docker/ca-trust/generators/java-pkcs12/generate.shexports/docker/ca-trust/host.shtaskfiles/docker/test-docker-libs.shtaskfiles/docker/tests.yamltaskfiles/lint/main.yamltaskfiles/lint/shell.yamltaskfiles/tests.yaml
3 tasks
jackluo923
force-pushed
the
feat/ca-cert-lib
branch
from
August 2, 2026 18:52
7b0e0b8 to
d5c8ce4
Compare
3 tasks
jackluo923
force-pushed
the
feat/ca-cert-lib
branch
3 times, most recently
from
August 2, 2026 22:20
34da2b9 to
8d18e31
Compare
jackluo923
marked this pull request as ready for review
August 2, 2026 22:35
jackluo923
force-pushed
the
feat/ca-cert-lib
branch
2 times, most recently
from
August 2, 2026 23:00
29254ce to
063bbd7
Compare
…cker build helpers.
jackluo923
force-pushed
the
feat/ca-cert-lib
branch
from
August 2, 2026 23:24
063bbd7 to
980bf32
Compare
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
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
Why
Corporate networks often run a TLS-inspecting gateway that re-signs HTTPS traffic with the company's own certificate authority. Your laptop trusts it; a container doesn't. Builds then fail with certificate errors that give no hint of the cause, so each project invents its own fix —
y-scope/clpandy-scope/clp-plugin-presto-connectorboth had one, and they agreed on almost nothing.Stacked on #120, which vendors the connector's library as-is. That library only covered containers that are already running; the connector handled the image-build case separately, in its own build scripts. This PR brings that second half into the library too, so one implementation serves both projects — and nothing from the host's trust store is ever written into an image.
What changed
1. Certificates during image builds (~140 lines in
ca-trust).Until now the library could only hand certificates to a container that was already running.
y-scope/clpneeds them earlier — while an image is being built, because that's when it downloads packages.The mechanism isn't new.
y-scope/clp-plugin-presto-connectoralready does exactly this for its dependency image: the certificates are passed in as a folder that Docker attaches for the duration of a single build step and then discards, so they never become part of the finished image. It got there the hard way — Docker's purpose-built feature for passing files into a build (--secret) rejects anything over 500KiB, and a customer's certificate bundle exceeded that. What's new here is lifting that proven approach out of the connector's own build scripts and into the shared library, so it isn't reimplemented per project.A project opts in by adding one line to its Dockerfile that supplies an empty folder by default. Builds that pass nothing — which is all of CI — behave exactly as before. New functions:
ca_trust_stage_build_context,ca_trust_add_build_args,ca_trust_add_run_args, pluscontainer-exec.sh, a small wrapper that exists because Docker runs build steps with a shell that can't read the library directly.2. A separate
docker/buildlibrary (~210 lines).Nothing to do with certificates: it assembles the repetitive parts of a
docker buildcommand — forwarding proxy settings, passing through build options, refreshing the base image, and stamping the image with which commit produced it. It's here because they-scope/clpscript this replaces did all of that in the same file, so it has to land somewhere.It keeps the command as a list of separate words rather than one long string, so a value containing a space, quote, or
$is never re-split by the shell — proxy and mirror URLs routinely contain all three. It runs on bash 3.2, which is what macOS ships, since a corporate laptop is a likely place to need this.It also strips credentials from URLs before they leave the build: a git remote like
https://user:token@github.com/org/repowould otherwise be written into the image's source label, and a proxy password would be printed into the build log. The command still runs with the real values.Breaking:
stage_host_ca_bundleis renamed toca_trust_stage_host_bundle, for consistency with the new function names. The library is unreleased and its only consumer is updated in lockstep in y-scope/clp-plugin-presto-connector#47.The README shows deletions because this PR rewrites the version #120 adds, to document the build-time half. Against
mainthe two PRs are purely additive.Checklist
Validation performed
Tests for both libraries are stacked on top in #122, including regressions for the two bugs fixed here:
CA_TRUST_JVMbeing passed by reference, anddocker_build_add_env_build_argsleaking a nonzero status to anerrexitcaller when its last variable was unset.On real builds: a 28MB bundle transfers intact through a named build context and leaves no trace in any layer or in
docker history, while Docker's--secretrejects the same file withsecret too big. max size 500KiB.Exercised end to end by the two consumer PRs — y-scope/clp-plugin-presto-connector#47 and y-scope/clp#2451 — including a build through a real TLS-inspecting proxy, which reports 24 certificate errors without the staged bundle and 0 with it.