Add HOLMESGPT_REF validation and align Dockerfile with gatekeeper source-build pattern#4756
Add HOLMESGPT_REF validation and align Dockerfile with gatekeeper source-build pattern#4756
Conversation
This adds a maintained HolmesGPT container image definition and Make targets to build and push it through the existing ARO-RP workflow with configurable versioning and base registry. Made-with: Cursor
ArrisLee
requested review from
bennerv,
cadenmarchese,
hawkowl,
hlipsig,
jharrington22,
mrWinston,
rogbas,
tiguelu and
yjst2012
as code owners
ArrisLee
requested review from
alcasim,
kevinobriendotca,
kimorris27,
mociarain,
sankur-codes,
tsatam,
tuxerrante,
ventifus and
wanghaoran1988
as code owners
There was a problem hiding this comment.
Pull request overview
This PR adds a first-class build/publish workflow for a HolmesGPT container image within the ARO-RP repo, keeping the image definition versioned alongside the codebase and providing Makefile targets to build and push it.
Changes:
- Add
Dockerfile.holmesgptto build a HolmesGPT runtime image (installed from PyPI) and include kubectl tooling. - Add Makefile variables and targets to build (
image-holmesgpt) and publish (publish-image-holmesgpt) the HolmesGPT image. - Introduce
HOLMESGPT_BASE_REGISTRYto decouple the base-image source registry from the output image registry.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| Makefile | Adds HolmesGPT version/registry/image variables plus build/publish targets. |
| Dockerfile.holmesgpt | Defines the HolmesGPT multi-stage build and installs runtime tooling (kubectl). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Remove the accidentally duplicated second half of Dockerfile.holmesgpt so the image definition remains single-sourced and easier to maintain. Made-with: Cursor
|
Similar to gatekeeper, you have to download the source code in the dockerfile and build the container image. |
This switches HolmesGPT builds to UBI Python base images, installs source from upstream tarball refs, updates Makefile build args, and aligns runtime entrypoint to python holmes_cli.py while keeping non-root execution. Made-with: Cursor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ARG HOLMESGPT_REPO=https://github.com/robusta-dev/holmesgpt | ||
| ARG HOLMESGPT_REF | ||
|
|
There was a problem hiding this comment.
HOLMESGPT_REF is declared but has no default and isn’t validated. If someone builds this Dockerfile directly (without the Makefile target), it will attempt to download /tags/.tar.gz and fail with a non-obvious curl error. Add an explicit check early in the build (with a clear error message) or provide a sensible default so failures are easier to diagnose.
There was a problem hiding this comment.
@copilot apply changes based on this feedback
There was a problem hiding this comment.
Added an early validation check in the builder stage (commit 1989b88) that fails immediately with a clear error message if HOLMESGPT_REF is not set:
RUN test -n "${HOLMESGPT_REF}" || { echo "ERROR: HOLMESGPT_REF build arg must be set (e.g., --build-arg HOLMESGPT_REF=v0.23.0)"; exit 1; }| RUN HOLMESGPT_TARBALL_URL="${HOLMESGPT_REPO}/archive/refs/tags/${HOLMESGPT_REF}.tar.gz" \ | ||
| && curl -fsSL "${HOLMESGPT_TARBALL_URL}" -o /tmp/holmesgpt.tar.gz \ | ||
| || (case "${HOLMESGPT_REF}" in \ | ||
| v*) ALT_REF="${HOLMESGPT_REF#v}" ;; \ | ||
| *) ALT_REF="v${HOLMESGPT_REF}" ;; \ | ||
| esac \ | ||
| && curl -fsSL "${HOLMESGPT_REPO}/archive/refs/tags/${ALT_REF}.tar.gz" -o /tmp/holmesgpt.tar.gz) \ | ||
| && mkdir -p /build/src \ | ||
| && tar -xzf /tmp/holmesgpt.tar.gz -C /build/src --strip-components=1 \ | ||
| && rm -f /tmp/holmesgpt.tar.gz \ | ||
| && pip install --no-cache-dir /build/src |
There was a problem hiding this comment.
The HolmesGPT source tarball is downloaded and executed (via pip install) without any integrity verification. This is a supply-chain risk; consider pinning and verifying a checksum (or signature) for the downloaded artifact, or using a mechanism that enforces hashes for dependencies/artifacts during the build.
There was a problem hiding this comment.
@copilot apply changes based on this feedback
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Agent-Logs-Url: https://github.com/Azure/ARO-RP/sessions/b62e8f78-8d31-455c-8660-144286b69658 Co-authored-by: ArrisLee <20032458+ArrisLee@users.noreply.github.com>
The current |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Agent-Logs-Url: https://github.com/Azure/ARO-RP/sessions/58c8ddc6-751a-4f57-b300-70fe4330a4e4 Co-authored-by: ArrisLee <20032458+ArrisLee@users.noreply.github.com>
This reverts commit d0b686e. Co-authored-by: ArrisLee <20032458+ArrisLee@users.noreply.github.com>
HOLMESGPT_REFhad no default and no validation — omitting it caused a silent, non-obvious curl failure when building the image directly without the Makefile.Changes
Dockerfile.holmesgpt: Add earlyRUNvalidation in the builder stage that fails fast with a clear error ifHOLMESGPT_REFis unset: