Skip to content

Commit 318006c

Browse files
aThorp96tekton-robot
authored andcommitted
fix: resolve Git Anonymous Resolver excessive memory usage
Switch git resolver from go-git library to use git binary. The go-git library does not resolve deltas efficiently, and as a result is not recommended to be used to clone repositories with full-depth. In one example RemoteResolutionRequest targeting a repository which summed 145Mb, configuring the resolution timeout to 10 minutes and giving the resolver to have 25Gb of memory, the resolver pod was OOM killed after ~6 minutes. Additionally, since go-git's delta resolution does not accept any contexts, the time required and memory used during resolving a large repository will not be cutoff when the context is canceled, impacting the resolver's performance for other concurrent remote resolutions. Since the git resolver can be provided a revision which is not tracked at any ref in the repository, and because go-git only supports fetching fully-qualified refs, go-git does not support fetching arbitrary revisions. Therefore, in order to guarantee the requested revision is fetched, if we continue to use the go-git library we must fetch all revisions. Switching to the git binary enables the git resolver to take advantage of the git-fetch's support for fetching arbitrary revisions. Note that if the revision is not at any ref head, fetching the revision does depend on the git server enabling uploadpack.allowReachableSHA1InWant. Resolves #8652 See also: https://git-scm.com/docs/protocol-capabilities#_allow_reachable_sha1_in_want NOTE: This feature is enabled and supported in Github and Gitlab but not Gitea: go-gitea/gitea#11958
1 parent ea94e3a commit 318006c

File tree

12 files changed

+499
-193
lines changed

12 files changed

+499
-193
lines changed

.ko.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
baseImageOverrides:
2+
github.com/tektoncd/pipeline/cmd/resolvers: cgr.dev/chainguard/git@sha256:566235a8ef752f37d285042ee05fc37dbb04293e50f116a231984080fb835693

config/resolvers/resolvers-deployment.yaml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,9 @@ spec:
106106
value: "https://artifacthub.io/"
107107
- name: TEKTON_HUB_API
108108
value: "https://api.hub.tekton.dev/"
109+
volumeMounts:
110+
- name: tmp-clone-volume
111+
mountPath: "/tmp"
109112
securityContext:
110113
allowPrivilegeEscalation: false
111114
readOnlyRootFilesystem: true
@@ -115,3 +118,7 @@ spec:
115118
- "ALL"
116119
seccompProfile:
117120
type: RuntimeDefault
121+
volumes:
122+
- name: tmp-clone-volume
123+
emptyDir:
124+
sizeLimit: 4Gi

docs/git-resolver.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,15 @@ The differences between the two modes are:
7676
### Git Clone with git clone
7777

7878
Git clone with `git clone` is supported for anonymous and authenticated cloning.
79+
This mode shallow clones the git repo before fetching and checking out the
80+
provided revision.
81+
82+
**Note**: if the revision is a commit SHA which is not pointed-at by a Branch
83+
or Tag ref, the revision might not be able to be fetched, depending on the
84+
git provider's [uploadpack.allowReachableSHA1InWant](https://git-scm.com/docs/protocol-capabilities#_allow_reachable_sha1_in_want)
85+
setting. This is not an issue for major git providers such as Github and
86+
Gitlab, but may be of note for smaller or self-hosted providers such as
87+
Gitea.
7988

8089
#### Task Resolution
8190

pkg/remoteresolution/resolver/git/resolver_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -429,7 +429,7 @@ func TestResolve(t *testing.T) {
429429
pathInRepo: "foo/new",
430430
url: anonFakeRepoURL,
431431
},
432-
expectedErr: createError("revision error: reference not found"),
432+
expectedErr: createError("git fetch error: fatal: couldn't find remote ref non-existent-revision: exit status 128"),
433433
}, {
434434
name: "api: successful task from params api information",
435435
args: &params{
Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
/*
2+
Copyright 2025 The Tekton Authors
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package git
18+
19+
import (
20+
"context"
21+
"encoding/base64"
22+
"errors"
23+
"fmt"
24+
"os"
25+
"os/exec"
26+
"path/filepath"
27+
"strings"
28+
)
29+
30+
type cmdExecutor = func(context.Context, string, ...string) *exec.Cmd
31+
32+
type remote struct {
33+
url string
34+
username string
35+
password string
36+
cmdExecutor cmdExecutor
37+
}
38+
39+
func (r remote) clone(ctx context.Context) (*repository, func(), error) {
40+
urlParts := strings.Split(r.url, "/")
41+
repoName := urlParts[len(urlParts)-1]
42+
tmpDir, err := os.MkdirTemp("", repoName+"-*")
43+
if err != nil {
44+
return nil, func() {}, err
45+
}
46+
cleanupFunc := func() {
47+
os.RemoveAll(tmpDir)
48+
}
49+
50+
repo := repository{
51+
url: r.url,
52+
username: r.username,
53+
password: r.password,
54+
directory: tmpDir,
55+
executor: r.cmdExecutor,
56+
}
57+
58+
_, err = repo.execGit(ctx, "clone", repo.url, tmpDir, "--depth=1", "--no-checkout")
59+
if err != nil {
60+
if strings.Contains(err.Error(), "could not read Username") {
61+
err = errors.New("clone error: authentication required")
62+
}
63+
return nil, cleanupFunc, err
64+
}
65+
return &repo, cleanupFunc, nil
66+
}
67+
68+
type repository struct {
69+
url string
70+
username string
71+
password string
72+
directory string
73+
executor cmdExecutor
74+
}
75+
76+
func (repo *repository) currentRevision(ctx context.Context) (string, error) {
77+
revisionSha, err := repo.execGit(ctx, "rev-list", "-n1", "HEAD")
78+
if err != nil {
79+
return "", err
80+
}
81+
return strings.TrimSpace(string(revisionSha)), nil
82+
}
83+
84+
func (repo *repository) checkout(ctx context.Context, revision string) error {
85+
_, err := repo.execGit(ctx, "fetch", "origin", revision, "--depth=1")
86+
if err != nil {
87+
return err
88+
}
89+
90+
_, err = repo.execGit(ctx, "checkout", "FETCH_HEAD")
91+
if err != nil {
92+
return err
93+
}
94+
95+
return nil
96+
}
97+
98+
func (repo *repository) execGit(ctx context.Context, subCmd string, args ...string) ([]byte, error) {
99+
if repo.executor == nil {
100+
repo.executor = exec.CommandContext
101+
}
102+
103+
args = append([]string{subCmd}, args...)
104+
105+
// We need to configure which directory contains the cloned repository since `cd`ing
106+
// into the repository directory is not concurrency-safe
107+
configArgs := []string{"-C", repo.directory}
108+
env := []string{"GIT_TERMINAL_PROMPT=false"}
109+
if subCmd == "clone" {
110+
// NOTE: Since this is only HTTP basic auth, authentication only supports http
111+
// cloning, while unauthenticated cloning works for any other protocol supported
112+
// by the git binary which doesn't require authentication.
113+
if repo.username != "" && repo.password != "" {
114+
token := base64.URLEncoding.EncodeToString([]byte(repo.username + ":" + repo.password))
115+
env = append(
116+
env,
117+
"GIT_AUTH_HEADER=Authorization=Basic "+token,
118+
)
119+
configArgs = append(configArgs, "--config-env", "http.extraHeader=GIT_AUTH_HEADER")
120+
}
121+
}
122+
cmd := repo.executor(ctx, "git", append(configArgs, args...)...)
123+
cmd.Env = append(cmd.Env, env...)
124+
125+
out, err := cmd.Output()
126+
if err != nil {
127+
msg := string(out)
128+
var exitErr *exec.ExitError
129+
if errors.As(err, &exitErr) {
130+
msg = string(exitErr.Stderr)
131+
}
132+
err = fmt.Errorf("git %s error: %s: %w", subCmd, strings.TrimSpace(msg), err)
133+
}
134+
return out, err
135+
}
136+
137+
func (repo *repository) getFileContent(path string) ([]byte, error) {
138+
if _, err := os.Stat(repo.directory); errors.Is(err, os.ErrNotExist) {
139+
return nil, fmt.Errorf("repository clone no longer exists, used after cleaned? %w", err)
140+
}
141+
fileContents, err := os.ReadFile(filepath.Join(repo.directory, path))
142+
if err != nil {
143+
if errors.Is(err, os.ErrNotExist) {
144+
return nil, errors.New("file does not exist")
145+
}
146+
return nil, err
147+
}
148+
return fileContents, nil
149+
}
Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,164 @@
1+
/*
2+
Copyright 2025 The Tekton Authors
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
16+
*/
17+
18+
package git
19+
20+
import (
21+
"context"
22+
"encoding/base64"
23+
"os/exec"
24+
"reflect"
25+
"testing"
26+
)
27+
28+
func TestClone(t *testing.T) {
29+
type testCase struct {
30+
url string
31+
username string
32+
password string
33+
expectErr string
34+
}
35+
36+
testCases := map[string]testCase{
37+
"normal usage": {url: "https://github.com/tektoncd/pipeline"},
38+
"normal usage with .git": {url: "https://github.com/tektoncd/pipeline.git"},
39+
"private repository": {url: "https://github.com/tektoncd/not-a-repository.git"},
40+
"with crendentials": {url: "https://github.com/tektoncd/not-a-repository.git", username: "fake", password: "fake"},
41+
}
42+
43+
for name, test := range testCases {
44+
t.Run(name, func(t *testing.T) {
45+
executions := []*exec.Cmd{}
46+
47+
executor := func(ctx context.Context, name string, args ...string) *exec.Cmd {
48+
args = append([]string{name}, args...)
49+
// Run the command as `echo` args to avoid side effects
50+
cmd := exec.CommandContext(ctx, "echo", args...)
51+
executions = append(executions, cmd)
52+
return cmd
53+
}
54+
55+
mockCmdRemote := remote{url: test.url, username: test.username, password: test.password, cmdExecutor: executor}
56+
repo, cleanup, err := mockCmdRemote.clone(context.Background())
57+
defer cleanup()
58+
if test.expectErr != "" {
59+
if err.Error() != test.expectErr {
60+
t.Fatalf("Expected error %q but got %q", test.expectErr, err)
61+
}
62+
} else {
63+
if err != nil {
64+
t.Fatalf("Error cloning repository %q: %v", test.url, err)
65+
}
66+
}
67+
68+
expectedEnv := []string{"GIT_TERMINAL_PROMPT=false"}
69+
expectedCmd := []string{"git", "-C", repo.directory}
70+
if test.username != "" {
71+
token := base64.URLEncoding.EncodeToString([]byte(test.username + ":" + test.password))
72+
expectedCmd = append(expectedCmd, "--config-env", "http.extraHeader=GIT_AUTH_HEADER")
73+
expectedEnv = append(expectedEnv, "GIT_AUTH_HEADER=Authorization=Basic "+token)
74+
}
75+
expectedCmd = append(expectedCmd, "clone", test.url, repo.directory, "--depth=1", "--no-checkout")
76+
77+
if len(executions) != 1 {
78+
t.Fatalf("Expected 1 command execution during cloning, got %d: %v", len(executions), executions)
79+
}
80+
81+
cmd := executions[0]
82+
// Remove the `echo` prefix
83+
cmdParts := cmd.Args[1:]
84+
if !reflect.DeepEqual(cmdParts, expectedCmd) {
85+
t.Fatalf("Expected clone command to be %v but got %v", expectedCmd, cmdParts)
86+
}
87+
if !reflect.DeepEqual(cmd.Env, expectedEnv) {
88+
t.Fatalf("Expected clone command env vars to be %v but got %v", expectedEnv, cmd.Env)
89+
}
90+
})
91+
}
92+
}
93+
94+
func TestCheckout(t *testing.T) {
95+
repoPath, revisions := createTestRepo(
96+
t,
97+
[]commitForRepo{
98+
{
99+
Filename: "README.md",
100+
Content: "some content",
101+
Branch: "non-main",
102+
Tag: "1.0.0",
103+
},
104+
{
105+
Filename: "otherfile.yaml",
106+
Content: "some data",
107+
Branch: "to-be-deleted",
108+
},
109+
},
110+
)
111+
gitCmd := getGitCmd(t, repoPath)
112+
if err := gitCmd("checkout", "main").Run(); err != nil {
113+
t.Fatalf("cloud not checkout main branch after repo initialization: %v", err)
114+
}
115+
if err := gitCmd("branch", "-D", "to-be-deleted").Run(); err != nil {
116+
t.Fatalf("coun't delete branch to orphan commit: %v", err)
117+
}
118+
119+
ctx := context.Background()
120+
121+
type testCase struct {
122+
revision string
123+
expectedRevision string
124+
expectErr string
125+
}
126+
testCases := map[string]testCase{
127+
"revision is branch": {revision: "non-main", expectedRevision: revisions[0]},
128+
"revision is tag": {revision: "1.0.0", expectedRevision: revisions[0]},
129+
"revision is sha": {revision: revisions[0], expectedRevision: revisions[0]},
130+
"revision is unreachable sha": {revision: revisions[1], expectedRevision: revisions[1]},
131+
"non-existent revision": {revision: "fake-revision", expectErr: "git fetch error: fatal: couldn't find remote ref fake-revision: exit status 128"},
132+
}
133+
134+
for name, test := range testCases {
135+
t.Run(name, func(t *testing.T) {
136+
repo, cleanup, err := remote{url: repoPath}.clone(ctx)
137+
defer cleanup()
138+
139+
if err != nil {
140+
t.Fatalf("Error cloning repository %v", err)
141+
}
142+
143+
err = repo.checkout(ctx, test.revision)
144+
if test.expectErr != "" {
145+
if err == nil {
146+
t.Fatal("Expected error checking out revision but got none")
147+
} else if err.Error() != test.expectErr {
148+
t.Fatalf("Expected error %q but got %q", test.expectErr, err)
149+
}
150+
return
151+
} else if err != nil {
152+
t.Fatalf("Error checking out revision: %v", err)
153+
}
154+
155+
revision, err := repo.currentRevision(ctx)
156+
if err != nil {
157+
t.Fatal(err)
158+
}
159+
if revision != test.expectedRevision {
160+
t.Fatalf("Expected revision to be %q but got %q", test.expectedRevision, revision)
161+
}
162+
})
163+
}
164+
}

0 commit comments

Comments
 (0)