ROX-34147: Add Konflux pipeline for Go version validation#19737
Open
ROX-34147: Add Konflux pipeline for Go version validation#19737
Conversation
|
Skipping CI for Draft Pull Request. |
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In the
verify-go-mod-tidyscript,cp go.sum go.sum.beforewill fail for modules that don't yet have ago.sum; consider guarding this copy and the subsequentdiffwith anif [ -f go.sum ]check so the task works for both cases. - The Slack notification message hardcodes the Konflux UI base URL; consider parameterizing the host (e.g., via a pipeline param) so the same pipeline can be reused across clusters or environments without edits.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the `verify-go-mod-tidy` script, `cp go.sum go.sum.before` will fail for modules that don't yet have a `go.sum`; consider guarding this copy and the subsequent `diff` with an `if [ -f go.sum ]` check so the task works for both cases.
- The Slack notification message hardcodes the Konflux UI base URL; consider parameterizing the host (e.g., via a pipeline param) so the same pipeline can be reused across clusters or environments without edits.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
📝 WalkthroughWalkthroughThis pull request introduces a new Tekton-based CI/CD pipeline for validating Go module tidiness. It adds GitHub PR labeler configuration to identify commits affecting Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/labeler.yml:
- Around line 97-100: The go-mod-check label currently triggers only when go.mod
changes (see label name "go-mod-check" and the "changed-files ->
any-glob-to-any-file -> - go.mod" entry); update that label's changed-files
pattern to include go.sum as well (add "- go.sum" alongside "- go.mod") so the
auto-trigger fires for changes to either file.
In @.tekton/go-mod-validation-pipeline.yaml:
- Around line 193-200: The current shell block silently continues if
/workspace/cachi2/cachi2.env is missing; change it to fail-fast by exiting with
a non-zero status when the file is not found. Modify the existing if/else that
checks /workspace/cachi2/cachi2.env so that the else branch echoes a clear error
(e.g., "ERROR: cachi2.env not found") and calls exit 1, ensuring the
PipelineRun/prefetch-input validation cannot be bypassed.
🪄 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: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 3e7c1a52-2e75-4261-913e-037c946bb74c
📒 Files selected for processing (3)
.github/labeler.yml.tekton/go-mod-validation-build.yaml.tekton/go-mod-validation-pipeline.yaml
Contributor
|
Images are ready for the commit at c578982. To use with deploy scripts, first |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #19737 +/- ##
=======================================
Coverage 49.67% 49.67%
=======================================
Files 2765 2765
Lines 209039 209039
=======================================
Hits 103834 103834
Misses 97527 97527
Partials 7678 7678
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
🚀 Build Images ReadyImages are ready for commit a18bb42. To use with deploy scripts: export MAIN_IMAGE_TAG=4.11.x-610-ga18bb42247 |
The openshift-golang-builder image doesn't include oras, which is needed to extract OCI artifacts. Install it at runtime from GitHub releases. This adds ~5 seconds to the pipeline but avoids needing a custom image. Alternative considered: Create a custom Dockerfile with oras pre-installed, but that adds maintenance overhead for a simple validation pipeline. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Problem: Custom inline task tried to manually extract OCI artifacts using oras, which failed with "invalid reference: invalid registry". Solution: Replace with standard buildah-remote-oci-ta task that natively handles SOURCE_ARTIFACT and CACHI2_ARTIFACT OCI references. This follows the same pattern as all other Konflux builds. Changes: - Add image/go-mod-validation/Dockerfile to run go mod tidy - Replace custom taskSpec with buildah-remote-oci-ta taskRef - Remove GO_VERSION result (validation still happens via build failure) Partially AI-generated. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Problem: Pipeline fails with "missing values for these params which have no default values: [PLATFORM]" when running buildah-remote-oci-ta task. Solution: Add PLATFORM parameter with value linux/amd64 to the verify-go-mod-tidy task. Since this is a validation build (not a multi-platform release build), a single platform is sufficient. Fix for previous commit 27472e1 which converted from custom inline task to buildah-remote-oci-ta but missed this required parameter. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This is an intentional test to verify that the go-mod-validation pipeline correctly detects and fails when an incompatible Go version is specified in go.mod. Expected result: Pipeline should fail because Go 1.26 does not exist yet and the validation container uses Go 1.25. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
The previous commit successfully validated that the go-mod-validation pipeline correctly detects and fails on incompatible Go versions. Reverting to Go 1.25.0 to restore branch to passing state. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Problem: buildah-remote-oci-ta approach was overcomplicating the validation task. The task is meant to run `go mod tidy` to validate Go version compatibility, not to build and push a container image. Solution: Revert to simpler inline taskSpec that: - Uses oras to extract SOURCE_ARTIFACT and CACHI2_ARTIFACT - Runs go mod tidy in openshift-golang-builder:rhel_9_golang_1.25 - Fails if Go version in go.mod is incompatible This is based on the working approach from commit 1081ccf, which had oras installation working correctly. Removes: image/go-mod-validation/Dockerfile (no longer needed) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add verbose debugging with set -x and detailed logging at each step to help diagnose where the pipeline is failing. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Problem: Inline taskSpec with manual OCI artifact extraction fails silently due to missing credentials to access quay.io. Solution: Completely simplify the approach: 1. Use standard git-clone task to clone source 2. Run go mod tidy directly on cloned source in a simple taskSpec 3. No OCI artifact extraction, no oras, no complexity This removes dependency on: - OCI artifact extraction - oras CLI installation - Cachi2 artifacts (validation doesn't need hermetic deps) Trade-off: Not hermetic, but validation is simple version check only. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Problem: git-clone task bundle reference was incorrect, and inline taskSpec doesn't have OCI registry credentials. Solution: Go back to buildah-remote-oci-ta approach (which has credentials) with a minimal Dockerfile that just runs: - go version (to show which version is being used) - go mod tidy (fails if go.mod requires incompatible version) This is the standard Konflux pattern that other pipelines use. The buildah task has proper credentials to access OCI artifacts. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Problem: Dockerfile used brew.registry.redhat.com which doesn't exist. Error: dial tcp: lookup brew.registry.redhat.com: no such host Solution: Use brew.registry.redhat.io (same as other konflux Dockerfiles). All other konflux.Dockerfile files in the repo use brew.registry.redhat.io which is accessible from the Konflux build environment. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Create a phony go.mod with only module name and go version directive, avoiding dependency resolution issues. Extract version from real go.mod to ensure we validate the correct version. This approach: - Avoids Cachi2 dependency prefetch limitations - Validates only what matters: Go version compatibility - Keeps validation fast and focused Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Since we use a minimal go.mod with no dependencies, we don't need Cachi2 prefetching. This simplifies the pipeline: - Removed prefetch-dependencies task - Removed prefetch-input parameter - Removed taskRunSpecs for prefetch-dependencies - verify-go-mod-tidy now uses SOURCE_ARTIFACT directly from clone-repository This makes the pipeline faster and simpler while maintaining the same validation guarantee. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
go mod graph -go requires an argument and isn't for validation. Instead, create a trivial main.go and build it - this will fail if go.mod requires a newer Go version than installed. This approach: - Creates minimal go.mod with just version directives - Creates trivial main.go - Runs go build which validates version compatibility - Fails fast if version is incompatible Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Simplify to just run go mod tidy on the actual go.mod. This will fail if the go.mod requires a newer Go version. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Co-authored-by: Tomasz Janiszewski <janiszt@gmail.com>
Problem: We rely on undocumented behavior of go mod tidy failing when go.mod specifies a version higher than installed. If this behavior changes silently, our validation pipeline would break without notice. Solution: Add test that creates a go.mod with Go 2.0.0 and verifies go mod tidy fails. If it succeeds, the build fails because our assumption about go mod tidy behavior is broken. This validates the assumption continuously on every pipeline run. Using Go 2.0.0 ensures the test won't need updating for years. User request: Create test to validate if go mod tidy failure detection is still working by testing with incompatible Go version 2.0. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Changes both RUN scripts to use set -euo pipefail for better error handling: - -e: exit on error - -u: error on undefined variables - -o pipefail: fail on pipe errors User request: add set -euo pipefail for both scripts Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Split long &&-chained commands into separate RUN statements. Each command is now isolated, making it immediately clear which step failed during Konflux builds. Layers don't matter here because: - Konflux squashes layers anyway - This is a throwaway validation image User request: split long commands for easier failure debugging Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Co-authored-by: Misha Sugakov <537715+msugakov@users.noreply.github.com>
Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>
a18bb42 to
b40d1b2
Compare
Contributor
|
/konflux-retest scanner-v4-db-on-push |
Contributor
|
/konflux-retest operator-bundle-on-push |
|
@janisz: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. I understand the commands that are listed here. |
msugakov
reviewed
Apr 17, 2026
Contributor
msugakov
left a comment
There was a problem hiding this comment.
You addressed my higher-level comments, thanks!
Now it's time for more detailed ones. This is the first pass. I may find more things on subsequent passes.
| # Test that go mod tidy actually fails on incompatible versions | ||
| # This validates we're not relying on behavior that silently changed | ||
| RUN echo "Testing go mod tidy failure detection..." | ||
| RUN go mod edit -go=1.200 |
Contributor
There was a problem hiding this comment.
Let's have it X.Y.Z (as currently in go.mod) and not X.Y:
RUN go mod edit -go=1.200.0| echo "Our assumption about go mod tidy behavior is broken!"; \ | ||
| exit 1; \ | ||
| else \ | ||
| echo "SUCCESS: go mod tidy correctly failed with incompatible version"; \ |
Contributor
There was a problem hiding this comment.
Suggested change
| echo "SUCCESS: go mod tidy correctly failed with incompatible version"; \ | |
| echo "SUCCESS: go mod tidy correctly detects an incompatible Go version"; \ |
Comment on lines
+23
to
+24
| echo "ERROR: go mod tidy succeeded with incompatible version"; \ | ||
| echo "Our assumption about go mod tidy behavior is broken!"; \ |
Contributor
There was a problem hiding this comment.
I don't think semicolons are needed:
Suggested change
| echo "ERROR: go mod tidy succeeded with incompatible version"; \ | |
| echo "Our assumption about go mod tidy behavior is broken!"; \ | |
| echo "ERROR: go mod tidy succeeded with incompatible version" \ | |
| echo "Our assumption about go mod tidy behavior is broken!" \ |
Note that I did not include the following lines where semicolons should also be removed. Please still edit them.
| @@ -0,0 +1,151 @@ | |||
| apiVersion: tekton.dev/v1 | |||
Contributor
There was a problem hiding this comment.
I would suggest you don't split PipelineRun and Pipeline in this case. Having just one file is possible and would be better.
You'd keep just the PipelineRun and put the contents of this Pipeline file under pipelineSpec (which replaces pipelineRef).
You can find existing examples of this in our repos:
Comment on lines
+12
to
+17
| pipelinesascode.tekton.dev/on-cel-expression: | | ||
| event == "pull_request" && | ||
| (has(body.pull_request) && has(body.pull_request.labels) && | ||
| body.pull_request.labels.exists(l, l.name == "go-mod-check")) && | ||
| body.action != "ready_for_review" | ||
| pipelinesascode.tekton.dev/on-label: "[]" |
Contributor
There was a problem hiding this comment.
Well, since this pipeline is cheap, I suggest running it always: for all PRs and all pushes.
That would be:
Suggested change
| pipelinesascode.tekton.dev/on-cel-expression: | | |
| event == "pull_request" && | |
| (has(body.pull_request) && has(body.pull_request.labels) && | |
| body.pull_request.labels.exists(l, l.name == "go-mod-check")) && | |
| body.action != "ready_for_review" | |
| pipelinesascode.tekton.dev/on-label: "[]" | |
| pipelinesascode.tekton.dev/on-cel-expression: | | |
| (event == "push" && target_branch.matches("^(master|release-.*|refs/tags/.*)$")) || | |
| (event == "pull_request" && body.action != "ready_for_review") |
Comment on lines
+85
to
+98
|
|
||
| - name: init | ||
| params: | ||
| - name: enable-cache-proxy | ||
| value: $(params.enable-cache-proxy) | ||
| taskRef: | ||
| params: | ||
| - name: name | ||
| value: init | ||
| - name: bundle | ||
| value: quay.io/konflux-ci/tekton-catalog/task-init:0.4@sha256:288f3106118edc1d0f0c79a89c960abf5841a4dd8bc3f38feb10527253105b19 | ||
| - name: kind | ||
| value: task | ||
| resolver: bundles |
Contributor
There was a problem hiding this comment.
Since this is a custom pipeline, we don't need this init task.
Suggested change
| - name: init | |
| params: | |
| - name: enable-cache-proxy | |
| value: $(params.enable-cache-proxy) | |
| taskRef: | |
| params: | |
| - name: name | |
| value: init | |
| - name: bundle | |
| value: quay.io/konflux-ci/tekton-catalog/task-init:0.4@sha256:288f3106118edc1d0f0c79a89c960abf5841a4dd8bc3f38feb10527253105b19 | |
| - name: kind | |
| value: task | |
| resolver: bundles |
Comment on lines
+32
to
+34
| - name: output-image-repo | ||
| # TODO: Change to quay.io/rhacs-eng/go-mod-validation once dedicated service account is created | ||
| value: quay.io/rhacs-eng/release-roxctl |
Contributor
There was a problem hiding this comment.
I can help create a repo for this, but I think we can come up with some name better than quay.io/rhacs-eng/go-mod-validation. Do you have any suggestions?
Comment on lines
+65
to
+68
| - default: "false" | ||
| description: Enable cache proxy configuration | ||
| name: enable-cache-proxy | ||
| type: string |
Contributor
There was a problem hiding this comment.
If you'd remove the init task (my other comment), this param can be removed too.
Suggested change
| - default: "false" | |
| description: Enable cache proxy configuration | |
| name: enable-cache-proxy | |
| type: string |
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
4 participants
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.
Description
Create a minimal, hermetic Konflux pipeline to verify Go version compatibility by running 'go mod tidy'.
This change was partially generated with AI assistance.
User-facing documentation
Testing and quality
Automated testing
How I validated my change
Changed go.mod to 1.26 and it failed https://konflux-ui.apps.stone-prd-rh01.pg1f.p1.openshiftapps.com/ns/rh-acs-tenant/applications/acs/pipelineruns/go-mod-validation-on-push-x94dl/logs?task=verify-go-mod-tidy
While it's passing for current version https://konflux-ui.apps.stone-prd-rh01.pg1f.p1.openshiftapps.com/ns/rh-acs-tenant/pipelinerun/go-mod-validation-on-push-f7xl6/logs/verify-go-mod-tidy