feat(packaging): Add manylinux-based packaging build environment image. - #20
Conversation
|
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:
📝 WalkthroughWalkthroughAdds local and CI support for building, resolving, and publishing content-addressed dependency images, including Docker tooling, host CA handling, and task configuration updates. ChangesDependency Image Build Pipeline
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant PlanJob
participant BuildJob
participant ManifestJob
GitHubActions->>PlanJob: derive build-env hash and image reference
PlanJob->>BuildJob: pass image and needs-build
BuildJob->>BuildJob: build amd64 and arm64 images
BuildJob->>ManifestJob: provide per-architecture image tags
ManifestJob->>ManifestJob: create and push multi-architecture manifest
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.github/workflows/build-dependency-image.yaml:
- Around line 23-25: The workflow’s IMAGE_REPO value is using github.repository
as-is, which can preserve uppercase characters and break GHCR image references.
Update the IMAGE_REPO expression in the build-dependency-image workflow to
normalize the repository owner/name to lowercase, matching the behavior in
image_repo_from_origin from build-dependency-image.sh, so the image reference is
valid and consistent across both paths.
In `@tools/build-packages/dependency-image/Dockerfile`:
- Line 6: The dependency-image Dockerfile is using the floating manylinux base
tag, which makes rebuilds non-reproducible. Update the FROM line in the
Dockerfile to reference a specific dated quay.io/pypa/manylinux_2_28 tag instead
of latest, so the base image contents stay stable over time.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d0d7bb45-5d18-4aaa-b55a-b9ca9101b6f4
📒 Files selected for processing (9)
.github/workflows/build-dependency-image.yamltaskfile.yamltaskfiles/velox-connector/deps.yamltools/build-packages/build-dependency-image.shtools/build-packages/dependency-image/Dockerfiletools/build-packages/dependency-image/lib.shtools/ca-bundle/ca-bundle.shtools/ca-bundle/install-ca-bundle.shtools/ca-bundle/stage-ca-bundle.sh
💤 Files with no reviewable changes (1)
- taskfiles/velox-connector/deps.yaml
9e84ed1 to
bb0bff0
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 @.github/workflows/build-dependency-image.yaml:
- Around line 46-47: The checkout steps in this workflow are persisting
GITHUB_TOKEN into the repo’s git config, which can leak into the Docker build
context; update both actions/checkout usages in the workflow to set
persist-credentials to false so no credentials are written during checkout. Use
the checkout step(s) and the build-dependency-image workflow as the targets for
this change.
In `@tools/build-packages/dependency-image/Dockerfile`:
- Around line 34-37: The Dockerfile build step using the curl-to-tar pipeline is
missing pipefail protection, so a curl failure can be hidden by tar. Update the
RUN command in the dependency-image Dockerfile to enable pipefail alongside the
existing set options, keeping the pipeline in the same step that downloads and
extracts task. This change should be applied to the shell invocation around the
curl | tar sequence so the build fails if either stage fails.
In `@tools/build-packages/dependency-image/lib.sh`:
- Around line 36-46: Guard the working-directory change in derive_build_env_hash
so the subshell stops if cd into _REPO_ROOT fails; otherwise git ls-files may
run from the caller’s directory and produce the wrong hash. Update the
derive_build_env_hash subshell to explicitly handle the cd failure before
calling ensure_yscope_dev_utils_submodule and git ls-files, keeping the hash
generation tied to _REPO_ROOT.
- Around line 63-68: The RETURN trap in the shell helper is expanding ca_stage
too early because it is wrapped in double quotes; update the trap in the staging
logic to use single quotes so the cleanup path defers variable expansion until
the trap fires. Keep the change localized to the ca_stage / ca_bundle setup in
lib.sh and preserve the existing rm -rf cleanup behavior.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 77c946da-9373-4891-8800-14c96b12fe40
📒 Files selected for processing (9)
.github/workflows/build-dependency-image.yamltaskfile.yamltaskfiles/velox-connector/deps.yamltools/build-packages/build-dependency-image.shtools/build-packages/dependency-image/Dockerfiletools/build-packages/dependency-image/lib.shtools/ca-bundle/ca-bundle.shtools/ca-bundle/install-ca-bundle.shtools/ca-bundle/stage-ca-bundle.sh
💤 Files with no reviewable changes (1)
- taskfiles/velox-connector/deps.yaml
…ecret limit BuildKit caps `--mount=type=secret` payloads at 500KiB, but corporate CA bundles routinely exceed that (e.g. Zscaler-intercepted bundles), so the dependency image failed to build in proxied environments. Mount the CA bundle via a named build context instead, which bind-mounts it without the size cap while still keeping the bytes out of the build context and image layers. Also give dnf its own `--setopt=sslcacert` since, unlike curl/git/pip, it talks to libcurl directly and ignores the `CURL_CA_BUNDLE`/`SSL_CERT_FILE` env vars.
20001020ycx
left a comment
There was a problem hiding this comment.
Did a pass for all files except the Github workflow.
One suggestion: would it be possible to isolate the certificate-related code into a dedicated utils.sh? I don't have any other comments on the code itself, but pulling it out would help if it can be done without disrupting the readability of the main flow. I found this part hard to follow, and it's only needed for a specific deployment environment anyway.
Co-authored-by: ChenXing Yang <60459812+20001020ycx@users.noreply.github.com>
Co-authored-by: ChenXing Yang <60459812+20001020ycx@users.noreply.github.com>
Co-authored-by: ChenXing Yang <60459812+20001020ycx@users.noreply.github.com>
Co-authored-by: ChenXing Yang <60459812+20001020ycx@users.noreply.github.com>
Co-authored-by: ChenXing Yang <60459812+20001020ycx@users.noreply.github.com>
20001020ycx
left a comment
There was a problem hiding this comment.
We discussed the artifact generated by the GitHub workflow offline and reached agreement. The high-level implementation is settled, but given the time constraint, and the fact that a similar change may need to be ported to CLP OSS, we may want to revisit this with a refactor afterward.
Co-authored-by: Junhao Liao <junhao@junhao.ca>
Co-authored-by: Junhao Liao <junhao@junhao.ca>
Co-authored-by: Junhao Liao <junhao@junhao.ca>
Co-authored-by: Junhao Liao <junhao@junhao.ca>
Co-authored-by: Junhao Liao <junhao@junhao.ca>
Co-authored-by: Junhao Liao <junhao@junhao.ca>
Co-authored-by: Junhao Liao <junhao@junhao.ca>
Co-authored-by: Junhao Liao <junhao@junhao.ca>
Co-authored-by: Junhao Liao <junhao@junhao.ca>
Co-authored-by: Junhao Liao <junhao@junhao.ca>
Co-authored-by: Junhao Liao <junhao@junhao.ca>
Co-authored-by: Junhao Liao <junhao@junhao.ca>
Co-authored-by: Junhao Liao <junhao@junhao.ca>
Co-authored-by: Junhao Liao <junhao@junhao.ca>
Co-authored-by: Junhao Liao <junhao@junhao.ca>
Co-authored-by: Junhao Liao <junhao@junhao.ca>
Co-authored-by: Junhao Liao <junhao@junhao.ca>
Co-authored-by: Junhao Liao <junhao@junhao.ca>
Summary
This PR adds a reusable
manylinux_2_28build-environment image for the CLP Presto connector. It provides the compiler, dependencies, and packaging tools required by the artifact-building flow that will be added separately.It makes clean containerized builds reliable on typical developer and CI machines, gives local and CI builds a consistent toolchain, preserves compatibility with older supported Linux environments, and supports corporate networks that require custom CA bundles.
Motivation
Reliable and repeatable builds
The current containerized build downloads and compiles all dependencies concurrently. During a clean build with normal network throughput, the overlapping compilations can exhaust all available memory and fail. It generally succeeds only when enough dependencies were installed by an earlier build or when slow downloads happen to stagger compilation.
The new environment builds one dependency at a time. Compilation within that dependency remains parallelized and defaults to the number of available processors. In testing, memory utilization remained below 1 GiB per core, making clean builds practical without a specialized high-memory machine.
The image also captures the compiler, packaging tools, and prebuilt dependencies. Local and CI builds resolve the same content-addressed
env-<hash>image, avoiding repeated environment setup and dependency compilation while keeping builds consistent across machines.Broad Linux compatibility
Linux binaries built against a newer glibc may use symbols that do not exist on older systems. For example, Ubuntu 22.04 provides glibc 2.35, while Debian 11 provides glibc 2.31 and remains relevant in corporate deployments, with extended support available through June 30, 2031. A native Velox worker plugin built directly on Ubuntu 22.04 may therefore fail on Debian 11 even though the latter remains supported.
Manylinux was created to solve this portability problem and is widely used by open-source projects, including the Python packaging ecosystem. Its community-maintained build images establish conservative ABI requirements so binary artifacts can run across many Linux distributions. PEP 600 formalized the
manylinux_<glibc-major>_<glibc-minor>policy.Using
manylinux_2_28applies that established model to the native Velox worker plugin and sets glibc 2.28 as its ABI floor. The resulting worker plugin can run on a broader range of supported customer systems instead of inheriting the build host's newer ABI requirement. The Java coordinator plugin is not subject to this glibc constraint.Corporate build environments
To reduce software supply-chain risk, many organizations require artifacts to be built from source in a controlled internal environment. This lets them govern build inputs, apply their own security and compliance checks, and publish only approved artifacts to private repositories or container registries. These environments may also sit behind restrictive firewalls and TLS-intercepting products such as Zscaler, requiring build tools to trust a corporate CA bundle.
The build helper makes the host CA bundle available only to networked build steps through temporary BuildKit mounts. This enables internal builds without permanently embedding an organization-specific trust store in the image.
What changed
manylinux_2_28build image containing JDK 17, go-task, CMake,patchelf,.deb/.rpmtooling, and the Velox connector's C/C++ dependencies.env-<hash>image when one does not already exist.Testing
Summary by CodeRabbit