Skip to content

Use ART-injected env vars for version in Konflux builds#186

Merged
Joeavaikath merged 1 commit into
migtools:oadp-devfrom
Joeavaikath:use-konflux-env-for-version
May 4, 2026
Merged

Use ART-injected env vars for version in Konflux builds#186
Joeavaikath merged 1 commit into
migtools:oadp-devfrom
Joeavaikath:use-konflux-env-for-version

Conversation

@Joeavaikath
Copy link
Copy Markdown
Contributor

@Joeavaikath Joeavaikath commented May 4, 2026

Summary

ART env vars used

Variable Source Purpose
BUILD_VERSION doozer, derived from group config OADP version (e.g. 1.6.0)
SOURCE_GIT_COMMIT doozer, from source git Full git SHA for buildinfo.GitSHA

Test plan

  • Verify next Konflux build produces binary with correct version (oc oadp version shows 1.6.0, not 1.25)
  • Verify local podman build -f konflux.Dockerfile . still works (falls back to dev)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Chores
    • Updated the build process to derive version information from environment variables with fallback defaults, improving CI/CD integration for platform-specific builds and enhancing version tracking in build logs.

ART/doozer injects BUILD_VERSION and SOURCE_GIT_COMMIT as ENV vars
in Konflux builds before the RUN step executes. Use these directly
instead of ARG-based version passing, which collided with
Konflux-injected VERSION (the Go toolchain version).

Falls back to "dev"/"unknown" for local builds where ART env vars
are not present.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 4, 2026

📝 Walkthrough

Walkthrough

The konflux.Dockerfile transitions from static build arguments (OADP_VERSION, GIT_COMMIT) to build-injected environment variables (BUILD_VERSION, SOURCE_GIT_COMMIT) with fallback defaults, ensuring version and commit metadata are properly embedded during container builds.

Changes

Build Variable Injection & Metadata Embedding

Layer / File(s) Summary
Variable Definition
konflux.Dockerfile (lines 15–17)
Replaced ARG OADP_VERSION=dev and ARG GIT_COMMIT=unknown with RUN environment-derived variables: OADP_VERSION="${BUILD_VERSION:-dev}" and OADP_GIT_COMMIT="${SOURCE_GIT_COMMIT:-unknown}".
Build Configuration & Linker Flags
konflux.Dockerfile (lines 27–32)
Updated per-platform go build linker flags to use OADP_GIT_COMMIT for buildinfo.GitSHA instead of the previous GIT_COMMIT; buildinfo.Version continues using OADP_VERSION.
Build Output Logging
konflux.Dockerfile (lines 27–32)
Platform-specific build logs now include the resolved OADP_VERSION for visibility into injected build metadata.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

Suggested labels

approved, lgtm

Suggested reviewers

  • mpryc
  • kaovilai
  • weshayutin

Poem

🐰 A Dockerfile hops with careful grace,
From hardcoded args to a cleaner place!
BUILD_VERSION flows, SOURCE_GIT_COMMIT too,
With fallbacks for safety—defaults shining through.
Metadata embedded, no static bind—
Injected and flexible, just what we find! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description provides detailed context but does not follow the required template structure with 'Why the changes were made' and 'How to test the changes made' sections. Restructure the description to match the template: add a 'Why the changes were made' section explaining the problem and linking issues, and a 'How to test the changes made' section with specific test commands and verification steps.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely describes the main change: using ART-injected environment variables instead of static build arguments for version information in Konflux builds.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
konflux.Dockerfile (1)

29-33: 💤 Low value

Consider quoting the -X flag values to guard against whitespace in version strings.

Both ${OADP_VERSION} and ${OADP_GIT_COMMIT} are embedded unquoted inside the outer -ldflags="…" string. If BUILD_VERSION ever contains a space (e.g., a pre-release tag like 1.6.0 rc1), the shell would split the word and go build would see a malformed extra flag. ART-injected values are well-controlled so the actual risk is very low, but quoting the value half of each -X assignment is the idiomatic defensive form:

🛡️ Defensive quoting for -X ldflag values
-            -ldflags="-s -w \
-                -X github.com/vmware-tanzu/velero/pkg/buildinfo.Version=${OADP_VERSION} \
-                -X github.com/vmware-tanzu/velero/pkg/buildinfo.GitSHA=${OADP_GIT_COMMIT} \
-                -X github.com/vmware-tanzu/velero/pkg/buildinfo.GitTreeState=clean" \
+            -ldflags="-s -w \
+                -X 'github.com/vmware-tanzu/velero/pkg/buildinfo.Version=${OADP_VERSION}' \
+                -X 'github.com/vmware-tanzu/velero/pkg/buildinfo.GitSHA=${OADP_GIT_COMMIT}' \
+                -X 'github.com/vmware-tanzu/velero/pkg/buildinfo.GitTreeState=clean'" \
🤖 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 `@konflux.Dockerfile` around lines 29 - 33, The -ldflags -X assignments for
buildinfo.Version and buildinfo.GitSHA are unquoted and can break if
OADP_VERSION or OADP_GIT_COMMIT contain whitespace; update the go build
invocation (the -ldflags string in the Docker build step) so each -X assignment
quotes its right-hand value (the assignments for
github.com/vmware-tanzu/velero/pkg/buildinfo.Version and
github.com/vmware-tanzu/velero/pkg/buildinfo.GitSHA) to prevent shell splitting
or malformed flags.
🤖 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.

Nitpick comments:
In `@konflux.Dockerfile`:
- Around line 29-33: The -ldflags -X assignments for buildinfo.Version and
buildinfo.GitSHA are unquoted and can break if OADP_VERSION or OADP_GIT_COMMIT
contain whitespace; update the go build invocation (the -ldflags string in the
Docker build step) so each -X assignment quotes its right-hand value (the
assignments for github.com/vmware-tanzu/velero/pkg/buildinfo.Version and
github.com/vmware-tanzu/velero/pkg/buildinfo.GitSHA) to prevent shell splitting
or malformed flags.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: bb17e42a-8e1b-4650-97b1-ca47414b8355

📥 Commits

Reviewing files that changed from the base of the PR and between 0c82a2f and 288b524.

📒 Files selected for processing (1)
  • konflux.Dockerfile

@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented May 4, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Joeavaikath, shubham-pampattiwar, weshayutin

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

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [Joeavaikath,shubham-pampattiwar,weshayutin]

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

@Joeavaikath
Copy link
Copy Markdown
Contributor Author

/cherry-pick oadp-1.6

@openshift-cherrypick-robot
Copy link
Copy Markdown

@Joeavaikath: once the present PR merges, I will cherry-pick it on top of oadp-1.6 in a new PR and assign it to you.

Details

In response to this:

/cherry-pick oadp-1.6

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@Joeavaikath Joeavaikath merged commit c548518 into migtools:oadp-dev May 4, 2026
12 of 13 checks passed
@openshift-cherrypick-robot
Copy link
Copy Markdown

@Joeavaikath: #186 failed to apply on top of branch "oadp-1.6":

Applying: Use ART-injected env vars for version in konflux.Dockerfile
Using index info to reconstruct a base tree...
M	konflux.Dockerfile
Falling back to patching base and 3-way merge...
Auto-merging konflux.Dockerfile
CONFLICT (content): Merge conflict in konflux.Dockerfile
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0001 Use ART-injected env vars for version in konflux.Dockerfile

Details

In response to this:

/cherry-pick oadp-1.6

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Joeavaikath added a commit that referenced this pull request May 4, 2026
Use ART-injected env vars for version in Konflux builds
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants