Switch to UBI9 minimal multi-stage image#146
Open
SudipSinha wants to merge 5 commits into
Open
Conversation
Replace single-stage ubi9/python-312 with a two-stage build: - Builder: ubi9/python-312 (full) compiles wheels via uv - Runtime: ubi9/python-312-minimal (no compilers, no dev headers, no uv) Reduces Trivy findings from ~3,000 to ~108 with zero scan workarounds. FIPS crypto policy and MariaDB support preserved. Also bumps pip 26.0.1 → 26.1 (CVE-2026-6357). Co-Authored-By: Claude Opus 4.6 <[email protected]>
📝 WalkthroughWalkthroughThe Containerfile is refactored from a single-stage build into a two-stage build that separates dependency installation (builder) from runtime execution (minimal). The builder stage installs Python dependencies with optional MariaDB dev packages, while the runtime stage applies conditional FIPS policy, conditionally installs MariaDB shared libraries only, and copies built artifacts from the builder image. ChangesMulti-Stage Container Build Refactor
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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 |
Contributor
Reviewer's GuideConverts the container build to a two-stage UBI9 image (full ubi9/python-312 builder + minimal ubi9/python-312-minimal runtime), compiling dependencies in the builder, slimming the runtime (no compilers/uv/dev headers), preserving FIPS and MariaDB support, and bumping pip to address a CVE while adding base-image metadata. Flow diagram for conditional FIPS and MariaDB setup in runtime imageflowchart TD
A[Build_arguments] --> B{ENABLE_FIPS_POLICY_equals_true?}
B -- yes --> C[Install_crypto_policies_scripts_with_microdnf]
C --> D[Run_update_crypto_policies_set_FIPS]
D --> E[Clean_microdnf_and_remove_caches]
B -- no --> F[Skip_FIPS_crypto_policy_setup]
A --> G{EXTRAS_contains_mariadb?}
G -- yes --> H[Download_and_verify_mariadb_repo_setup]
H --> I[Run_mariadb_repo_setup_for_11_4]
I --> J[Install_MariaDB_shared_11_4_with_microdnf]
J --> K[Clean_microdnf_and_remove_caches]
G -- no --> L[Skip_MariaDB_shared_install]
E --> M[Runtime_image_ready_for_pip_upgrade_and_artifact_copy]
F --> M
K --> M
L --> M
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
- Add script validation to runtime MariaDB step (parity with builder) - Standardize on pip (not pip3) in both stages - Bump pip 26.1 → 26.1.1 Co-Authored-By: Claude Opus 4.6 <[email protected]>
SudipSinha
added
security
technical-debt
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Containerfile (1)
13-13: ⚡ Quick winPin base images by digest instead of
:latestAt Line 13 and Line 43, using
:latestmakes builds non-reproducible and can silently change vulnerability posture between runs. Prefer immutable digests for both stages.Also applies to: 43-43
🤖 Prompt for 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. In `@Containerfile` at line 13, Replace both mutable base image tags that currently use :latest with immutable image digests: update the builder stage "FROM registry.access.redhat.com/ubi9/python-312:latest AS builder" and the final stage FROM line (the second use of :latest at line 43) to use the specific sha256@<digest> for each image; fetch the correct digests from the registry (or your image manifest) and substitute them so both FROM lines are pinned to their respective digests to ensure reproducible, immutable builds.
🤖 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 `@Containerfile`:
- Around line 19-23: Replace the current weak checks around the downloaded
mariadb_repo_setup script with a cryptographic integrity check: after
downloading mariadb_repo_setup and before chmod/./mariadb_repo_setup, fetch a
trusted SHA256 checksum (hardcoded or from a verified source), compute the
downloaded file's SHA256 (e.g., via sha256sum) and compare it, and abort the
build if it doesn't match; apply the same SHA256 verification and abort logic to
the second Stage 2 block that also downloads and runs ./mariadb_repo_setup so
both occurrences (the mariadb_repo_setup download + ./mariadb_repo_setup
execution) only run after successful checksum validation.
- Around line 35-37: The Dockerfile RUN line installs the package using the
EXTRAS variable which expands to an invalid ".[]" when EXTRAS is empty; change
the pip install logic so it installs "." when EXTRAS is empty and installs
".[$EXTRAS]" only when EXTRAS is non-empty (i.e., add a shell conditional around
the pip install step that checks EXTRAS before choosing between "." and
".[$EXTRAS]"); update the RUN sequence that contains the pip install command
(the line with uv pip install --no-cache ".[$EXTRAS]") to use that conditional
while keeping the surrounding upgrade/uninstall steps intact.
---
Nitpick comments:
In `@Containerfile`:
- Line 13: Replace both mutable base image tags that currently use :latest with
immutable image digests: update the builder stage "FROM
registry.access.redhat.com/ubi9/python-312:latest AS builder" and the final
stage FROM line (the second use of :latest at line 43) to use the specific
sha256@<digest> for each image; fetch the correct digests from the registry (or
your image manifest) and substitute them so both FROM lines are pinned to their
respective digests to ensure reproducible, immutable builds.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
The ci-build workflow injects a quay.expires-after label after the last FIPS documentation label using sed. Moving base.name before it restores the expected line-ending pattern. Co-Authored-By: Claude Opus 4.6 <[email protected]>
GitHub Actions runners lack RHEL entitlement certificates, so no rhel-* repos exist and microdnf treats the missing pattern as an error. The UBI repos alone provide crypto-policies-scripts. Co-Authored-By: Claude Opus 4.6 <[email protected]>
|
PR image build and manifest generation completed successfully! 📦 PR image: 🗂️ CI manifests |
…repo The builder stage inherits user 1001 from the base image, but dnf install requires root. Add USER root before and USER 1001 after the MariaDB block. Replace four architecture-specific --disablerepo flags with a single glob (--disablerepo='rhel-*') that works on any arch without warnings. Co-Authored-By: Claude Opus 4.6 <[email protected]>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Consider extracting the pip/uv versions into build ARGs used in both stages so they stay in sync without having to update multiple hard‑coded occurrences.
- The MariaDB repository setup and installation logic is now duplicated (and slightly diverging) between builder and runtime; factoring this into a common pattern or clearly separating responsibilities would make it easier to keep behavior consistent over time.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider extracting the pip/uv versions into build ARGs used in both stages so they stay in sync without having to update multiple hard‑coded occurrences.
- The MariaDB repository setup and installation logic is now duplicated (and slightly diverging) between builder and runtime; factoring this into a common pattern or clearly separating responsibilities would make it easier to keep behavior consistent over time.
## Individual Comments
### Comment 1
<location path="Containerfile" line_range="20-29" />
<code_context>
-# For details, see: docs/FIPS_compliance.md
-# ===========================================================================================
+# Install MariaDB dev libraries if needed
+RUN if [[ "$EXTRAS" == *"mariadb"* ]]; then \
+ curl --fail -LsSO https://r.mariadb.com/downloads/mariadb_repo_setup && \
+ grep -q "mariadb_repo_setup" mariadb_repo_setup || { echo "ERROR: Downloaded script appears invalid"; exit 1; } && \
+ [ -s mariadb_repo_setup ] || { echo "ERROR: Downloaded script is empty"; exit 1; } && \
+ chmod +x mariadb_repo_setup && \
+ ./mariadb_repo_setup --mariadb-server-version="mariadb-11.4" --skip-check-installed && \
+ dnf install -y --disablerepo='rhel-*' \
+ MariaDB-shared-11.4* MariaDB-devel-11.4* && \
+ dnf clean all && \
+ rm -rf /var/cache/dnf /var/cache/yum /root/.cache mariadb_repo_setup; \
+ fi
+
+USER 1001
</code_context>
<issue_to_address>
**🚨 suggestion (security):** MariaDB repo script download has only a basic integrity check; consider strengthening verification.
The current checks help detect corruption but not a malicious script served from the correct URL. Please consider verifying a published checksum/signature from MariaDB, or at least pinning a specific versioned URL, to better protect this root-run remote script path.
Suggested implementation:
```
ARG MARIADB_REPO_SETUP_VERSION="2024-05-24"
ARG MARIADB_REPO_SETUP_SHA256=""
# Install MariaDB dev libraries if needed
RUN if [[ "$EXTRAS" == *"mariadb"* ]]; then \
REPO_SETUP_URL="https://r.mariadb.com/downloads/mariadb_repo_setup-${MARIADB_REPO_SETUP_VERSION}" && \
curl --fail --show-error --location --proto '=https' --tlsv1.2 -o mariadb_repo_setup "${REPO_SETUP_URL}" && \
if [[ -n "${MARIADB_REPO_SETUP_SHA256}" ]]; then \
echo "${MARIADB_REPO_SETUP_SHA256} mariadb_repo_setup" | sha256sum -c - || { echo "ERROR: mariadb_repo_setup checksum verification failed"; exit 1; }; \
fi && \
grep -q "mariadb_repo_setup" mariadb_repo_setup || { echo "ERROR: Downloaded script appears invalid"; exit 1; } && \
[ -s mariadb_repo_setup ] || { echo "ERROR: Downloaded script is empty"; exit 1; } && \
chmod +x mariadb_repo_setup && \
./mariadb_repo_setup --mariadb-server-version="mariadb-11.4" --skip-check-installed && \
dnf install -y --disablerepo='rhel-*' \
MariaDB-shared-11.4* MariaDB-devel-11.4* && \
dnf clean all && \
rm -rf /var/cache/dnf /var/cache/yum /root/.cache mariadb_repo_setup; \
fi
USER 1001
```
To fully benefit from the checksum verification, set `MARIADB_REPO_SETUP_SHA256` (and optionally override `MARIADB_REPO_SETUP_VERSION`) as build arguments in your CI/CD pipeline or build command, using the official checksum published by MariaDB for the pinned `mariadb_repo_setup` version.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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
1 participant
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.
Summary
ubi9/python-312with a two-stage build usingubi9/python-312-minimalas runtimeTest plan
podman build -f Containerfile -t trustyai:test .succeeds🤖 Generated with Claude Code
Summary by Sourcery
Switch the container build to a multi-stage UBI9 Python 3.12 minimal runtime image while preserving FIPS and MariaDB support and reducing base image vulnerabilities.
Enhancements:
Build:
Summary by CodeRabbit