Skip to content

Conversation

@tzhou5
Copy link
Contributor

@tzhou5 tzhou5 commented Jan 14, 2026

This fixes the CI image build failure: 'error obtaining VCS status: exit status 128'

The -buildvcs=false flag prevents Go from attempting to stamp VCS information into the binary, which fails in containerized build environments where .git directory is not available.

Summary by CodeRabbit

  • New Features

    • Adds a CLI "version" command that prints Version, Commit, and Build date.
  • Chores

    • Build now embeds version metadata and is driven via project build targets.
    • Runtime image runs as a non-root user, installs CA certificates, and exposes a default entrypoint/command.
    • Adds a .dockerignore to reduce build context size.

✏️ Tip: You can customize this high-level summary in your review settings.

@openshift-ci openshift-ci bot requested a review from rafabene January 14, 2026 03:59
@coderabbitai
Copy link

coderabbitai bot commented Jan 14, 2026

Walkthrough

The PR adds build/versioning and container runtime changes: new .dockerignore; Dockerfile now accepts ARG GIT_COMMIT, uses make build (output to /workspace/bin/pull-secret), installs CA certificates, and runs as non-root USER 1000 with ENTRYPOINT /usr/local/bin/pull-secret and default CMD ["run-job","pull-secret"]. The Makefile embeds version metadata via GIT_COMMIT, BUILD_DATE and LDFLAGS and forwards GIT_COMMIT into the image build. cmd/pull-secret/main.go adds package-level variables for version/commit/buildDate and a version Cobra command that prints them.

Sequence Diagram(s)

(Section omitted — conditions for diagram generation were not met.)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main changes: adding version info embedding and fixing Docker build issues by implementing make-based builds and .dockerignore.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.



📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b467a3e and 4bc7722.

📒 Files selected for processing (2)
  • Dockerfile
  • Makefile
🧰 Additional context used
🪛 checkmake (0.2.2)
Makefile

[warning] 87-87: Target body for "build" exceeds allowed length of 5 (16).

(maxbodylength)

🔇 Additional comments (8)
Makefile (4)

13-22: LGTM! Clean version embedding scaffolding.

The pattern correctly handles both local builds (via git commands) and containerized builds (via passed GIT_COMMIT arg). The LDFLAGS structure with -w -s for stripping and -X for embedding is idiomatic Go.


86-95: LGTM! Build target correctly integrates version embedding.

The -ldflags="$(LDFLAGS)" integration is correct, and the echo provides useful build-time visibility for debugging version issues.


181-185: LGTM!

Correctly propagates GIT_COMMIT to the Dockerfile build-arg, enabling version embedding in containerized builds.


198-201: LGTM!

Consistent with the image target pattern for version embedding.

Dockerfile (4)

5-24: LGTM! Build stage correctly implements version embedding.

The ARG placement, layer caching strategy (go.mod/go.sum first), and make build invocation are well-structured. The GIT_COMMIT passthrough correctly enables version embedding when .git is unavailable in the container.


27-33: LGTM! Good security practices.

Minimal base image, CA certificates for TLS, and non-root user with no login shell are all appropriate security hardening measures.


38-45: LGTM!

Binary path correctly matches the Makefile output (bin/pull-secret), and using numeric UID for USER is container best practice.


47-51: LGTM!

The ENTRYPOINT + CMD pattern correctly allows runtime argument override. Users can verify the embedded version with docker run <image> version.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@openshift-ci openshift-ci bot requested a review from xueli181114 January 14, 2026 03:59
@openshift-ci
Copy link

openshift-ci bot commented Jan 14, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign vkareh for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@rafabene
Copy link
Contributor

Shouldn't we change go build to make build? Example: https://github.com/openshift-hyperfleet/hyperfleet-adapter/pull/30/files#diff-dd2c0eb6ea5cfc6c4bd4eac30934e2d5746747af48fef6da689e85b752f39557R21

VCS parameter is captured by make image and propageted to make build.

@tzhou5
Copy link
Contributor Author

tzhou5 commented Jan 15, 2026

Changes:

  • Add .dockerignore to exclude .git directory (fixes VCS status error)
  • Update Dockerfile to use 'make build GIT_COMMIT=${GIT_COMMIT}'
  • Update Makefile build target with ldflags for version/commit/date
  • Update Makefile image target to pass GIT_COMMIT as build-arg
  • Add version command to main.go displaying build info

Aligns with hyperfleet-adapter pattern and addresses:

  • VCS stamping errors in CI environment
  • Binary version and build time embedding
  • Consistent build process inside/outside container

Ref: HYPERFLEET-370, HYPERFLEET-499

@rafabene would you please help review again? thanks in advance.

@tzhou5 tzhou5 changed the title Fix Dockerfile: add -buildvcs=false to go build HYPERFLEET-499 | Add version info embedding and fix Docker build Jan 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants