fix(KONFLUX-15531): override ITS revision for private GitLab repos by dirgim · Pull Request #1696 · konflux-ci/integration-service

dirgim · 2026-09-03T10:57:24Z

When org/repo/serverURL are used in ITS Pipeline, PipelineRun or Task definitions, construct the git URL from those params and match it against the component's git source
when determining if the revision should be overriden.

If the change is coming from a fork, parse the URL in order to detect it and then override the org and repo so it matches the behavior when handling simple git urls.

Maintainers will complete the following section

Commit messages are descriptive enough (hints)
Code coverage from testing does not decrease and new code is covered (check the PR coverage on codecov)
Controllers diagrams are updated when PR changes controllers code (if applicable)

qodo-app-for-konflux-ci · 2026-09-03T10:58:03Z

PR Summary by Qodo

Fix ITS revision overrides for split Git resolver parameters

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

AI Description

• Matches split Git resolver parameters against component sources before overriding revisions.
• Rewrites fork organization and repository parameters from the pull request source URL.
• Covers GitLab namespaces, resolver matching, and pipeline and task updates.

Diagram

graph TD
  A["Snapshot annotations"] --> B["Resolver parameters"] --> C{"Target matches?"}
  C -- No --> D["Keep resolver"]
  C -- Yes --> E{"Fork source?"}
  E -- No --> F["Override revision"]
  E -- Yes --> G["Parse source URL"] --> H["Override org repo"] --> F

High-Level Assessment

The current approach is appropriate: shared URL normalization and parsing keep Pipeline, PipelineRun, and Task behavior consistent while preserving existing URL-based resolvers. Directly comparing individual split parameters was considered but would duplicate normalization logic and handle nested GitLab namespaces less reliably.

Files changed (5) +411 / -9

Bug fix (3) +99 / -9

integration.goAdd repository construction, parsing, and fork helpers +52/-0

Add repository construction, parsing, and fork helpers

• Adds helpers to construct normalized Git URLs from split resolver parameters, compare source and target repositories, and parse HTTPS repository URLs. Nested GitLab namespaces are preserved, and fork source organization and repository values can be derived safely.

helpers/integration.go

consts.goDefine split git resolver parameter constants +15/-6

Define split git resolver parameter constants
• Adds constants for the Tekton git resolver's serverURL, org, and repo parameters. Existing constant comments are also clarified.
tekton/consts/consts.go

integration_pipeline.goSupport split resolver matching and fork rewrites +32/-3

Support split resolver matching and fork rewrites

• Matches split serverURL, org, and repo parameters against the component repository when deciding whether to override revisions. For fork changes, pipeline and task resolver updates now replace org and repo with values parsed from the source repository URL.

tekton/integration_pipeline.go

Tests (2) +312 / -0

integration_test.goTest Git URL and fork helper behavior +53/-0

Test Git URL and fork helper behavior

• Covers URL construction, normalized fork detection, HTTPS parsing, nested GitLab groups, unsupported URL formats, and fork organization and repository extraction.

helpers/integration_test.go

integration_pipeline_test.goCover split resolver and fork update paths +259/-0

Cover split resolver and fork update paths

• Adds scenarios for matching and nonmatching split resolver parameters, same-repository and fork snapshots, and both pipeline-level and task-level updates. Verifies revision, organization, and repository values after rewriting.

tekton/integration_pipeline_test.go

fullsend-ai-review · 2026-09-03T10:58:42Z

🤖 Finished Review · ✅ Success · Started 10:58 AM UTC · Completed 11:17 AM UTC

Commit: 9ee3c25 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $5.99

qodo-app-for-konflux-ci · 2026-09-03T11:00:29Z

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (2)

1. Trailing slash corrupts repo 🐞 Bug ≡ Correctness

Description

ParseGitHttpsUrl turns a valid source URL ending in .git/ into repo.git.git, then removes only
one suffix and returns repo.git as the repository name. Fork handling writes that value into the
Tekton repo parameter, causing resolution of the wrong repository.

Code

helpers/integration.go[558]
+	normalized := strings.TrimSuffix(UrlToGitUrl(gitUrl), ".git")

Relevance

●●● Strong
Specific correctness bug can corrupt valid .git/ URLs and resolver repository selection.
ⓘ Recommendations generated based on similar findings in past PRs

Evidence

UrlToGitUrl only checks for .git before removing a trailing slash, so .git/ becomes .git.git
(helpers/integration.go[533-538]). The new parser strips one suffix and uses the remaining final
segment as repo (helpers/integration.go[558-575]); fork update paths then assign that malformed
value to resolver parameters (tekton/integration_pipeline.go[81-85] and
tekton/integration_pipeline.go[368-370]).

helpers/integration.go[533-558]
helpers/integration.go[570-575]
tekton/integration_pipeline.go[81-85]
tekton/integration_pipeline.go[368-370]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Fork source URLs ending in `.git/` are parsed as repository names ending in `.git`, producing an incorrect Tekton git-resolver `repo` parameter.

## Issue Context
`UrlToGitUrl("https://host/org/repo.git/")` trims the slash and then appends `.git`, yielding `repo.git.git`. `ParseGitHttpsUrl` removes only one `.git`, leaving `repo.git` as the parsed repository.

## Fix Focus Areas
- helpers/integration.go[533-558]
- helpers/integration.go[570-575]
- helpers/integration_test.go[1121-1139]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. ~~Helper tests skip BDD nesting~~ ✗ Dismissed 📜 Skill insight ✧ Quality

Description

The newly added helper tests are placed directly under Describe without a Context("When..."),
and their It descriptions do not begin with should. This violates the required Ginkgo BDD
nesting and naming convention.

Code

helpers/integration_test.go[1104]
+	It("constructs git URLs from server URL, org and repo", func() {

Relevance

●●● Strong
Explicit BDD naming and nesting convention is a straightforward, locally deterministic test fix.
PR-#1680

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 2028 requires every Ginkgo scenario to use Describe, a Context prefixed with
When, and an It prefixed with should. The suite begins its Describe at line 43, while the
added tests beginning at line 1104 are direct It children with noncompliant descriptions.

helpers/integration_test.go[43-43]
helpers/integration_test.go[1104-1155]
Skill: running-unit-tests

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new Ginkgo tests skip the required `Context("When...")` level and use `It` descriptions that do not begin with `should`.

## Issue Context
All added helper scenarios should follow `Describe > Context("When...") > It("should...")`.

## Fix Focus Areas
- helpers/integration_test.go[1104-1155]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. It descriptions omit should 📜 Skill insight ✧ Quality

Description

The new resolver tests use the required Context("When...") level, but their It descriptions do
not start with should. Consequently, these Ginkgo scenarios do not conform to the mandated BDD
naming convention.

Code

tekton/integration_pipeline_test.go[1104]
+		It("updates org and repo in ReplaceGitResolverUpdateMap for fork PR snapshots", func() {

Relevance

●●● Strong
Explicit BDD naming convention makes adding the should prefix a deterministic test-quality fix.
PR-#1680

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 2028 requires It descriptions to begin with should. The new context is
correctly named, but additions such as It("updates org and repo...") omit that prefix, as do the
other scenarios in the block.
tekton/integration_pipeline_test.go[1094-1104]
tekton/integration_pipeline_test.go[1127-1150]
Skill: running-unit-tests

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new Ginkgo `It` descriptions do not begin with `should`.

## Issue Context
The enclosing `Context` is correctly prefixed with `When`; update each newly added `It` description to complete the required BDD convention.

## Fix Focus Areas
- tekton/integration_pipeline_test.go[1094-1341]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

4. Resolver updates lack idempotency coverage 📜 Skill insight ▣ Testability

Description

The new resolver-update tests invoke each mutating operation only once and do not verify that
applying the operation again produces the same result. This leaves the checklist's required
idempotency behavior uncovered.

Code

tekton/integration_pipeline_test.go[R1121-1124]

+			result := tekton.ReplaceGitResolverUpdateMap(snapshot, resolverParams)
+			Expect(getResolverParamValue(result, tektonconsts.TektonResolverGitParamOrg)).To(Equal(forkSourceOrg))
+			Expect(getResolverParamValue(result, tektonconsts.TektonResolverGitParamRepo)).To(Equal(forkTargetRepo))
+			Expect(getResolverParamValue(result, tektonconsts.TektonResolverGitParamRevision)).To(Equal(forkSourceRevision))

Relevance

●● Moderate
Idempotency coverage is plausible for mutating logic, but historical evidence does not closely
establish this exact requirement.
PR-#1549

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 2067 requires changed application logic to include idempotency coverage. The added
test calls ReplaceGitResolverUpdateMap once and immediately asserts the result; the analogous
pipeline and task resolver tests likewise contain no second application and comparison.
tekton/integration_pipeline_test.go[1121-1124]
tekton/integration_pipeline_test.go[1268-1272]
tekton/integration_pipeline_test.go[1336-1340]
Skill: pr-definition-of-done

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The tests do not verify that the new resolver mutation behavior is idempotent when applied repeatedly.

## Issue Context
Call the relevant update operation a second time using the first result and assert that resolver parameters and return values remain unchanged.

## Fix Focus Areas
- tekton/integration_pipeline_test.go[1104-1125]
- tekton/integration_pipeline_test.go[1214-1273]
- tekton/integration_pipeline_test.go[1275-1341]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

View medium (1)

5. consts.go lacks colocated tests ✗ Dismissed 📘 Rule violation ▣ Testability

Description

The PR adds exported Git resolver constants under tekton/consts, but that directory has no
corresponding *_test.go file. The changed declarations therefore lack the colocated unit tests
required by the checklist.

Code

tekton/consts/consts.go[R52-53]

+	// TektonResolverGitParamServerUrl is the name of tekton git resolver param serverURL
+	TektonResolverGitParamServerUrl = "serverURL"

Relevance

●● Moderate
Checklist supports the finding, but no close historical precedent confirms constants require
dedicated tests.
ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 513 requires every modified production Go source file to have a corresponding test
file in the same directory. The PR adds TektonResolverGitParamServerUrl,
TektonResolverGitParamOrg, and TektonResolverGitParamRepo, while tekton/consts contains only
consts.go.

Rule 513: Co-locate unit tests with code and use Ginkgo + envtest
tekton/consts/consts.go[52-59]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The modified production file in `tekton/consts` has no colocated `*_test.go` file covering its newly added exported resolver constants.

## Issue Context
Tests elsewhere consume these constants, but the compliance rule explicitly requires a corresponding unit-test file in the production code's directory.

## Fix Focus Areas
- tekton/consts/consts.go[52-59]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Context sources

✅ Compliance rules (platform): 40 rules

✅ Skills: ci-cd-quirks, running-unit-tests, pr-definition-of-done

Tip of the day

💡 Did you know, you can add REVIEW.md to your repo root and Qodo follows it on every PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

codecov-commenter · 2026-09-03T11:03:10Z

Codecov Report

❌ Patch coverage is 92.59259% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.55%. Comparing base (ef6599a) to head (1aad35b).

Files with missing lines	Patch %	Lines
helpers/integration.go	87.87%	2 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1696      +/-   ##
==========================================
+ Coverage   73.97%   75.55%   +1.57%     
==========================================
  Files          74       74              
  Lines       10617    10667      +50     
==========================================
+ Hits         7854     8059     +205     
+ Misses       2006     1875     -131     
+ Partials      757      733      -24

Flag	Coverage Δ
e2e-tests	`40.81% <25.92%> (+6.20%)`	⬆️
unit-tests	`68.99% <92.59%> (-0.03%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
tekton/integration_pipeline.go	`86.30% <100.00%> (+3.29%)`	⬆️
helpers/integration.go	`77.77% <87.87%> (+2.32%)`	⬆️

... and 13 files with indirect coverage changes

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ef6599a...1aad35b. Read the comment docs.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

fullsend-ai-review · 2026-09-03T11:17:26Z

Risk Assessment: low (1/5)

Details

Targeted bug fix with stable git history, no protected paths, minimal churn, and 40% test coverage; medium size offset by bug-fix scope and comprehensive tests.

Previous run

Risk Assessment: moderate (2/5)

Details

Medium-sized change to stable files with good test coverage and no security-sensitive paths, authored by experienced contributor — standard review adequate.

fullsend-ai-review · 2026-09-03T11:17:29Z

Review

Findings

Low

[edge-case] tekton/integration_pipeline.go:393 — doTargetAndSourceRevisionParamsMatch requires all three of serverURL, org, and repo params to be present for the org/repo/serverURL matching path to activate. If a user configures only org and repo (relying on Tekton's ConfigMap default for serverURL), the function falls through to return false and the resolver will not be updated for PR events. In practice this is unlikely since Tekton documentation shows all three params specified together.
Remediation: When serverURL is absent but org and repo are present, consider defaulting to https://github.com or comparing only the path portion.
[edge-case] tekton/integration_pipeline.go:68 — In ReplaceGitResolverUpdateMap, when the resolver uses serverURL/org/repo style and it is a fork, the serverURL param is never updated. If the fork source repo were hosted on a different server than the target, serverURL would remain stale. In practice, git forge forks always reside on the same server.
[test-inadequate] helpers/integration_test.go — ParseGitHttpsUrl tests do not cover URLs with a port (e.g., https://gitlab.example.com:8443/group/project), common for self-hosted GitLab instances. The function uses net/url.Parse which handles ports correctly, but a test would confirm this for the private GitLab use case this PR targets.
[naming-convention] tekton/consts/consts.go:49 — Comment has double space: TektonResolverGitParamURL is the name of tekton git resolver param url.
Remediation: Change is the name of to is the name of.
[naming-convention] tekton/consts/consts.go:61 — Comment has double space: TektonResolverGitParamRevision is the name of tekton git resolver param revision.
Remediation: Change is the name of to is the name of.

Next steps:

/fs-fix — agent addresses review findings automatically
/fs-fix <your instruction> — agent fixes with your specific guidance
Push commits directly — review re-runs automatically on push
/fs-fix-stop — disable automatic fix runs for this PR

Previous run

Review

Findings

Low

[edge-case] tekton/integration_pipeline.go:391 — In doTargetAndSourceRevisionParamsMatch, the org/repo/serverURL matching requires all three params to be present. When serverURL is absent, the function falls through to return false. This is the correct safe default since Tekton's git resolver requires serverURL when using org/repo mode, but a test documenting this behavior would be beneficial.
[naming-convention] helpers/integration.go — New function names use Url (e.g., ConstructGitUrl, ParseGitHttpsUrl) instead of URL. This follows the existing codebase convention (UrlToGitUrl, PipelineRunImageUrlParamName, DefaultRenovateImageUrl). The casing inconsistency with TektonResolverGitParamURL is pre-existing.
[naming-convention] tekton/consts/consts.go — TektonResolverGitParamServerUrl uses Url casing while the existing TektonResolverGitParamURL uses URL. Pre-existing casing inconsistency across the codebase.
[scope-alignment] helpers/integration.go — New git URL parsing functions are placed in helpers/ rather than tekton/. This follows the existing pattern where UrlToGitUrl already lives in helpers/integration.go.

When org/repo/serverURL are used in ITS Pipeline, PipelineRun or Task definitions, construct the git URL from those params and match it against the component's git source when determining if the revision should be overriden. If the change is coming from a fork, parse the URL in order to detect it and then override the org and repo so it matches the behavior when handling simple git urls. Signed-off-by: dirgim <kpavic@redhat.com>

fullsend-ai-review · 2026-09-03T12:25:28Z

🤖 Finished Review · ✅ Success · Started 12:25 PM UTC · Completed 12:45 PM UTC

Commit: 9ee3c25 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $6.27

fullsend-ai-review

See the review comment for full details.

jencull

lgtm

jsztuka

Lgtm

dirgim · 2026-09-04T10:10:30Z

/retest

kasemAlem · 2026-09-06T07:14:35Z

/retest

kasemAlem · 2026-09-06T08:44:22Z

/retest

konflux-ci-qe-bot · 2026-09-06T10:13:31Z

Scenario: konflux-e2e
@dirgim: The following test has Failed, say /retest to rerun failed tests.

PipelineRun Name	Status	Rerun command	Build Log	Test Log
`konflux-e2e-fb8d9`	Failed	`/retest`	View Pipeline Log	View Test Logs

Inspecting Test Artifacts

To inspect your test artifacts, follow these steps:

Install ORAS (see the ORAS installation guide).
Download artifacts with the following commands:

mkdir -p oras-artifacts
cd oras-artifacts
oras pull quay.io/konflux-test-storage/konflux-team/integration-service:konflux-e2e-fb8d9

Test results analysis

🚨 Error occurred while running the E2E tests, list of failed Spec(s):

➡️ [failed] [It] [integration-service-suite Forgejo Status Reporting of Integration tests] Forgejo with status reporting of Integration tests in the associated merge request when a new Component with specified custom branch is created triggers a Build PipelineRun [integration-service, forgejo-status-reporting, custom-branch]

Click to view logs

Unexpected error:
    <*fmt.wrapError | 0xc0010acb60>: 
    fork of konflux-qe/konflux-test-integration to konflux-qe/konflux-test-integration-4cihpj already exists but failed to fetch: The target couldn't be found.
    {
        msg: "fork of konflux-qe/konflux-test-integration to konflux-qe/konflux-test-integration-4cihpj already exists but failed to fetch: The target couldn't be found.",
        err: <*errors.errorString | 0xc0001359b0>{
            s: "The target couldn't be found.",
        },
    }
occurred

OCI Artifact Browser URL

View in Artifact Browser

dirgim requested review from 14rcole, Josh-Everett, NigelByrne1, ainephelan365, jencull, jsztuka, kasemAlem and sonam1412 as code owners September 3, 2026 10:57

github-actions Bot added the size: XL label Sep 3, 2026

qodo-app-for-konflux-ci Bot reviewed Sep 3, 2026

View reviewed changes

Comment thread helpers/integration_test.go

Comment thread tekton/integration_pipeline_test.go

Comment thread tekton/consts/consts.go

Comment thread tekton/integration_pipeline_test.go

Comment thread helpers/integration.go

fullsend-ai-review Bot added the risk/moderate PR risk: moderate label Sep 3, 2026

This comment was marked as outdated.

Sign in to view

fullsend-ai-review Bot added the ready-for-merge All reviewers approved — ready to merge label Sep 3, 2026

dirgim force-pushed the KONFLUX-15531 branch from 8fadeb5 to 1aad35b Compare September 3, 2026 12:23

fullsend-ai-review Bot added risk/low PR risk: low and removed risk/moderate PR risk: moderate labels Sep 3, 2026

fullsend-ai-review Bot suggested changes Sep 3, 2026

View reviewed changes

Comment thread tekton/integration_pipeline.go

Comment thread tekton/integration_pipeline.go

Comment thread tekton/consts/consts.go

Comment thread tekton/consts/consts.go

fullsend-ai-review Bot removed the ready-for-merge All reviewers approved — ready to merge label Sep 3, 2026

jencull approved these changes Sep 3, 2026

View reviewed changes

jsztuka approved these changes Sep 3, 2026

View reviewed changes

Uh oh!

Conversation

dirgim commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Maintainers will complete the following section

Uh oh!

qodo-app-for-konflux-ci Bot commented Sep 3, 2026

PR Summary by Qodo

Uh oh!

fullsend-ai-review Bot commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qodo-app-for-konflux-ci Bot commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

fullsend-ai-review Bot commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fullsend-ai-review Bot commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review

Findings

Low

Review

Findings

Low

Uh oh!

This comment was marked as outdated.

Uh oh!

fullsend-ai-review Bot commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fullsend-ai-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jencull left a comment

Choose a reason for hiding this comment

Uh oh!

jsztuka left a comment

Choose a reason for hiding this comment

Uh oh!

dirgim commented Sep 4, 2026

Uh oh!

kasemAlem commented Sep 6, 2026

Uh oh!

kasemAlem commented Sep 6, 2026

Uh oh!

konflux-ci-qe-bot commented Sep 6, 2026

Inspecting Test Artifacts

Test results analysis

OCI Artifact Browser URL

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

dirgim commented Sep 3, 2026 •

edited

Loading

fullsend-ai-review Bot commented Sep 3, 2026 •

edited

Loading

qodo-app-for-konflux-ci Bot commented Sep 3, 2026 •

edited

Loading

codecov-commenter commented Sep 3, 2026 •

edited

Loading

fullsend-ai-review Bot commented Sep 3, 2026 •

edited

Loading

fullsend-ai-review Bot commented Sep 3, 2026 •

edited

Loading

fullsend-ai-review Bot commented Sep 3, 2026 •

edited

Loading