refactor(resolution): replace go-scm with internal scmclient by ary82 · Pull Request #10208 · tektoncd/pipeline

ary82 · 2026-06-07T21:02:33Z

Changes

The git resolver's API depended on github.com/jenkins-x/go-scm for fetching file content from SCM providers, causing a heavy dependency and ongoing maintenance pain.

This change introduces the scmclient internal package, which defines lightweight implementations of the SCM providers according to the SCMClient interface. It has three methods (GetFileContent, GetCommitSHA, GetCloneURL)

/kind cleanup

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

Has Docs if any changes are user facing, including updates to minimum requirements e.g. Kubernetes version bumps
Has Tests included if any functionality added or changed
pre-commit Passed
Follows the commit message standard
Meets the Tekton contributor standards (including functionality, content, code)
Has a kind label. You can add one by adding a comment on this PR that contains /kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep
Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings). See some examples of good release notes.
Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes

go-scm dependency in the git resolver will be replaced by an internal HTTP client (pkg/internal/scmclient), removing a source of transitive dependency conflicts with k8s.io/apimachinery. Supported git resolutions scm providers are: Github, Gitlab, Gitea, Bitbucket Cloud, Bitbucket Server and Azure. Support for Gogs will be dropped.

Copilot

Pull request overview

This PR refactors the git resolver’s API-mode SCM integration to stop depending on github.com/jenkins-x/go-scm by introducing a new internal pkg/internal/scmclient package that implements a lightweight SCMClient interface (GetFileContent / GetCommitSHA / GetCloneURL) and wiring the resolver(s) to use it.

Changes:

Added pkg/internal/scmclient with HTTP-based clients (GitHub/GitLab/Gitea) behind a small interface plus a fake for unit tests.
Updated both in-tree git resolvers to construct SCM clients via scmclient.New instead of go-scm/factory.NewClient.
Expanded/adjusted e2e tests to validate API mode for additional providers (GitHub/GitLab) and refactored Gitea API-mode tests.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
`test/resolvers_git_test.go`	E2E updates: adds GitHub/GitLab API-mode tests and refactors Gitea API-mode tests/config handling.
`pkg/resolution/resolver/git/resolver.go`	Switches API-mode resolution to use `pkg/internal/scmclient` instead of go-scm.
`pkg/resolution/resolver/git/resolver_test.go`	Updates unit tests to mock via `scmclient.FakeSCMClient` and adjusts expected errors.
`pkg/remoteresolution/resolver/git/resolver.go`	Switches remoteresolution git resolver to use `pkg/internal/scmclient`.
`pkg/remoteresolution/resolver/git/resolver_test.go`	Updates remoteresolution unit tests to mock via `scmclient.FakeSCMClient`.
`pkg/internal/scmclient/client.go`	Adds `SCMClient` interface and provider factory.
`pkg/internal/scmclient/http.go`	Adds a small HTTP helper for request/response handling.
`pkg/internal/scmclient/github.go` / `github_test.go`	Adds GitHub REST client + unit tests.
`pkg/internal/scmclient/gitlab.go` / `gitlab_test.go`	Adds GitLab REST client + unit tests.
`pkg/internal/scmclient/gitea.go` / `gitea_test.go`	Adds Gitea REST client + unit tests.
`pkg/internal/scmclient/fake.go`	Adds a fake implementation for resolver unit tests.
`pkg/internal/scmclient/bitbucketcloud.go`	Placeholder Bitbucket Cloud client (currently panics).
`pkg/internal/scmclient/bitbucketserver.go`	Placeholder Bitbucket Server client (currently panics).
`pkg/internal/scmclient/azure.go`	Placeholder Azure DevOps client (currently panics).
`pkg/internal/scmclient/client_test.go`	Tests factory behavior for supported provider keys.

Comments suppressed due to low confidence (1)

test/resolvers_git_test.go:558

This test verification request places the auth token in the URL query (...&token=...) and uses http.Get without a context. Tokens in query strings are easily leaked via logs/proxies, and omitting context can cause the call to hang beyond the test deadline. Prefer sending the token via Authorization header and using http.NewRequestWithContext.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated 15 comments.

Comments suppressed due to low confidence (1)

test/resolvers_git_test.go:619

The verification request puts the Gitea token into the query string and uses http.Get without context. Tokens in URLs can leak via logs/proxies, and the request won’t be canceled if the test context is canceled. Prefer an Authorization header with a context-aware request, and ensure the response body is closed via defer.

vdemeester · 2026-09-15T08:35:40Z

@ary82 can you rebase the PR ? Sorry for the delay on the review.. Summer can be a bit challenging 😛

vdemeester

SGTM, but defenitely needs more eyes on it 👼🏼

tekton-robot · 2026-09-16T08:48:59Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vdemeester

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~~ [vdemeester]

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

ary82 · 2026-09-16T13:25:59Z

@ary82 can you rebase the PR ? Sorry for the delay on the review.. Summer can be a bit challenging 😛

No worries :D I'll push a rebase to main today

wbrefvem · 2026-09-18T02:49:13Z

@ary82 I have a couple of concerns:

We're silently dropping support for Gogs & Stash. There may be good reasons -- the Stash name change happened a long time ago, and most users probably prefer Gitea to Gogs. But while the blast radius of this change is likely small, it is probably not zero. At the very least, I think we should explicitly notify users that support has been dropped, or in the case of Stash, just silently map to Bitbucket Server (which is what jenkins-x/go-scm does).
jenkins-x/go-scm does some URL normalization for GitHub Enterprise and Bitbucket Cloud (cf. here), and if that is not replicated here, we risk breaking users that pass unnormalized values. In my opinion this needs to be fixed.

ary82 · 2026-09-22T05:23:12Z

@ary82 I have a couple of concerns:

1. We're silently dropping support for Gogs & Stash. There may be good reasons -- the Stash name change happened a long time ago, and most users probably prefer Gitea to Gogs. But while the blast radius of this change is likely small, it is probably not zero. At the very least, I think we should explicitly notify users that support has been dropped, or in the case of Stash, just silently map to Bitbucket Server (which is what `jenkins-x/go-scm` does).

2. `jenkins-x/go-scm` does some URL normalization for GitHub Enterprise and Bitbucket Cloud (cf. [here](https://github.com/jenkins-x/go-scm/blob/bbc2ec558d1f6ee800aaefb010b9ba0d550cfc03/scm/factory/factory.go#L270)), and if that is not replicated here, we risk breaking users that pass unnormalized values. In my opinion this needs to be fixed.

Thanks for the review @wbrefvem!

I've updated the mapping for stash->bitbucketserver and also for bitbucket->bitbucketcloud. Also made an addition to the release note for dropping support for Gogs. lmk if it should be notified anywhere else.
I'll check this out and also go through any more url normalizations i may have missed

ary82 · 2026-10-09T03:30:07Z

Normalization added for GitHub Enterprise and Bitbucket Cloud

codecov · 2026-10-09T03:33:09Z

Codecov Report

❌ Patch coverage is 84.74576% with 72 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.63%. Comparing base (7758213) to head (37564e6).
⚠️ Report is 10 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/internal/scmclient/azure.go	87.71%	14 Missing ⚠️
pkg/internal/scmclient/gitea.go	81.81%	10 Missing ⚠️
pkg/internal/scmclient/github.go	82.75%	10 Missing ⚠️
pkg/internal/scmclient/gitlab.go	82.75%	10 Missing ⚠️
pkg/internal/scmclient/bitbucketcloud.go	85.24%	9 Missing ⚠️
pkg/internal/scmclient/bitbucketserver.go	84.48%	9 Missing ⚠️
pkg/internal/scmclient/fake.go	76.19%	5 Missing ⚠️
pkg/internal/scmclient/http.go	80.00%	2 Missing ⚠️
pkg/resolution/resolver/git/resolver.go	88.23%	2 Missing ⚠️
pkg/remoteresolution/resolver/git/resolver.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #10208      +/-   ##
==========================================
- Coverage   90.74%   90.63%   -0.12%     
==========================================
  Files         298      307       +9     
  Lines       22603    23058     +455     
==========================================
+ Hits        20512    20898     +386     
- Misses       2089     2158      +69     
  Partials        2        2

Flag	Coverage Δ
unit-tests	`90.63% <84.74%> (-0.12%)`	⬇️

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

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

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

waveywaves

The replacement looks good overall, but the Azure client has two blocking compatibility issues.

waveywaves · 2026-10-15T17:31:53Z

+		return "", err
+	}
+	rawURL := fmt.Sprintf("%s/%s/%s/_apis/git/repositories/%s/commits/%s?api-version=7.1",
+		c.baseURL, organization, project, repo, url.PathEscape(ref))


Blocking: this endpoint requires a commit ID, but this method receives branch and tag revisions such as main. Azure API mode will fail for normal named refs. Please resolve the ref to a commit SHA before calling this endpoint.

Thanks for the review @waveywaves
I've updated it so that full SHAs still use commits/{id}, but branch and tag names resolve to a commit SHA through the Refs API first. Also added tests for branches, tags and missing refs.

waveywaves · 2026-10-15T17:31:53Z

+// Handles refs/heads/, refs/tags/, and bare branch/tag names or commit SHAs.
+func generateVersionDescriptor(ref string) string {
+	if branch, ok := strings.CutPrefix(ref, "refs/heads/"); ok {
+		return fmt.Sprintf("versionDescriptor.version=%s&versionDescriptor.versionType=branch", branch)


Blocking: the ref is added to the query without escaping. A valid branch such as feature/a&x=y becomes feature/a plus a separate x=y parameter. Please encode the query value before building the URL.

Fixed, the version is now query-escaped so refs like feature/a&x=y stay a single value.

The git resolver's API depended on `github.com/jenkins-x/go-scm` for fetching file content from SCM providers, causing a heavy dependency and ongoing maintenance pain. This change introduces the `scmclient` internal package, which defines lightweight implementations of the SCM providers according to the `SCMClient` interface. It has three methods (GetFileContent, GetCommitSHA, GetCloneURL). Implementations include github, gitlab, gitea, bitbucketcloud, bitbucketserver and azure Signed-off-by: Aryan Goyal <mail@ary82.dev>

tekton-robot added release-note-none Denotes a PR that doesnt merit a release note. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. labels Jun 7, 2026

tekton-robot requested review from afrittoli and khrm June 7, 2026 21:02

tekton-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jun 7, 2026

ary82 force-pushed the cleanup/resolution-replace-go-scm-lib-9989 branch from 351a9f9 to f6f9cff Compare June 8, 2026 02:59

vdemeester self-assigned this Jun 8, 2026

vdemeester requested a review from Copilot June 8, 2026 08:02

Copilot started reviewing on behalf of vdemeester June 8, 2026 08:02

View session

Copilot AI reviewed Jun 8, 2026

View reviewed changes

ary82 force-pushed the cleanup/resolution-replace-go-scm-lib-9989 branch 2 times, most recently from 8f43882 to 3e7d091 Compare June 8, 2026 15:12

vdemeester requested a review from Copilot June 17, 2026 14:42

Copilot started reviewing on behalf of vdemeester June 17, 2026 14:43

View session

Copilot AI reviewed Jun 17, 2026

View reviewed changes

ary82 force-pushed the cleanup/resolution-replace-go-scm-lib-9989 branch 4 times, most recently from 8afdeef to 7d12375 Compare June 23, 2026 15:26

tekton-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesnt merit a release note. labels Jun 23, 2026

ary82 force-pushed the cleanup/resolution-replace-go-scm-lib-9989 branch from 7d12375 to d9b6985 Compare June 23, 2026 17:34

ary82 changed the title ~~[WIP] refactor(resolution): replace go-scm with internal scmclient~~ refactor(resolution): replace go-scm with internal scmclient Jun 23, 2026

ary82 force-pushed the cleanup/resolution-replace-go-scm-lib-9989 branch 3 times, most recently from ba8b481 to 44d7a56 Compare June 29, 2026 08:21

ary82 marked this pull request as ready for review June 29, 2026 08:21

tekton-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 29, 2026

tekton-robot requested a review from abayer June 29, 2026 08:21

tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 18, 2026

vdemeester approved these changes Sep 16, 2026

View reviewed changes

tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 16, 2026

ary82 force-pushed the cleanup/resolution-replace-go-scm-lib-9989 branch from 0dd5bdc to 807a369 Compare September 16, 2026 18:23

tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 16, 2026

tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 21, 2026

ary82 force-pushed the cleanup/resolution-replace-go-scm-lib-9989 branch from 807a369 to 4415c5b Compare September 22, 2026 04:59

tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 22, 2026

ary82 force-pushed the cleanup/resolution-replace-go-scm-lib-9989 branch from 4415c5b to 09a6a0c Compare September 22, 2026 05:10

tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 29, 2026

ary82 force-pushed the cleanup/resolution-replace-go-scm-lib-9989 branch from 09a6a0c to 77422cb Compare October 9, 2026 03:29

tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 9, 2026

tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 9, 2026

waveywaves requested changes Oct 15, 2026

View reviewed changes

ary82 force-pushed the cleanup/resolution-replace-go-scm-lib-9989 branch from 77422cb to 118d204 Compare October 21, 2026 04:08

tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 21, 2026

ary82 force-pushed the cleanup/resolution-replace-go-scm-lib-9989 branch 2 times, most recently from befe178 to 9a44518 Compare October 21, 2026 04:21

ary82 requested a review from waveywaves October 21, 2026 04:23

tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 22, 2026

ary82 force-pushed the cleanup/resolution-replace-go-scm-lib-9989 branch from 9a44518 to 37564e6 Compare October 22, 2026 05:47

tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 22, 2026

Conversation

ary82 commented Jun 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Submitter Checklist

Release Notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vdemeester commented Sep 15, 2026

Uh oh!

vdemeester left a comment

Choose a reason for hiding this comment

Uh oh!

tekton-robot commented Sep 16, 2026

Uh oh!

ary82 commented Sep 16, 2026

Uh oh!

wbrefvem commented Sep 18, 2026

Uh oh!

ary82 commented Sep 22, 2026

Uh oh!

ary82 commented Oct 9, 2026

Uh oh!

codecov Bot commented Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

waveywaves left a comment

Choose a reason for hiding this comment

Uh oh!

waveywaves Oct 15, 2026

Choose a reason for hiding this comment

Uh oh!

ary82 Oct 21, 2026

Choose a reason for hiding this comment

Uh oh!

waveywaves Oct 15, 2026

Choose a reason for hiding this comment

Uh oh!

ary82 Oct 21, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ary82 commented Jun 7, 2026 •

edited

Loading

codecov Bot commented Oct 9, 2026 •

edited

Loading