Skip to content

Commit 6073888

Browse files
committed
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 99c805a commit 6073888

File tree

10 files changed

+169
-71
lines changed

10 files changed

+169
-71
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: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,9 @@ The differences between the two modes are:
7575

7676
### Git Clone with git clone
7777

78-
Git clone with `git clone` is supported for anonymous and authenticated cloning.
78+
Git clone with `git clone` is supported for anonymous and authenticated cloning
79+
This mode shallow-clones the git repo before shallow-fetching and checking
80+
out the provided revision.
7981

8082
#### Task Resolution
8183

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("error resolving repository: 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{

pkg/resolution/resolver/git/git.go

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
/*
2+
Copyright 2022 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 repository struct {
31+
url string
32+
username string
33+
password string
34+
directory string
35+
revision string
36+
}
37+
38+
func resolveRepository(ctx context.Context, url, username, password, revision string) (*repository, func(), error) {
39+
urlParts := strings.Split(url, "/")
40+
repoName := urlParts[len(urlParts)-1]
41+
tmpDir, err := os.MkdirTemp("", repoName+"-*")
42+
if err != nil {
43+
return nil, func() {}, err
44+
}
45+
cleanupFunc := func() {
46+
os.RemoveAll(tmpDir)
47+
}
48+
49+
repo := repository{
50+
url: url,
51+
username: username,
52+
password: password,
53+
directory: tmpDir,
54+
revision: revision,
55+
}
56+
57+
_, err = repo.execGit(ctx, "clone", repo.url, tmpDir, "--depth=1", "--no-checkout")
58+
if err != nil {
59+
if strings.Contains(err.Error(), "unable to get password from user") {
60+
err = errors.New("clone error: authentication required")
61+
}
62+
return nil, cleanupFunc, err
63+
}
64+
65+
_, err = repo.execGit(ctx, "fetch", "origin", repo.revision, "--depth=1")
66+
if err != nil {
67+
return nil, cleanupFunc, err
68+
}
69+
70+
_, err = repo.execGit(ctx, "checkout", "FETCH_HEAD")
71+
if err != nil {
72+
return nil, cleanupFunc, err
73+
}
74+
75+
revisionSha, err := repo.execGit(ctx, "rev-list", "-n1", "HEAD")
76+
if err != nil {
77+
return nil, cleanupFunc, err
78+
}
79+
repo.revision = strings.TrimSpace(string(revisionSha))
80+
81+
return &repo, cleanupFunc, nil
82+
}
83+
84+
func (repo *repository) execGit(ctx context.Context, subCmd string, args ...string) ([]byte, error) {
85+
args = append([]string{subCmd}, args...)
86+
87+
var env []string
88+
var configArgs []string
89+
if subCmd == "clone" {
90+
configArgs = []string{"-c", "credential.interactive=false"}
91+
// NOTE: Since this is only HTTP basic auth, authentication only supports http
92+
// cloning, while unauthenticated cloning works for any other protocol supported
93+
// by the git binary.
94+
if repo.username != "" && repo.password != "" {
95+
token := base64.URLEncoding.EncodeToString([]byte(repo.username + ":" + repo.password))
96+
env = append(
97+
env,
98+
"GIT_AUTH_HEADER=Authorization=Basic "+token,
99+
)
100+
configArgs = append(configArgs, "--config-env", "http.extraHeader=GIT_AUTH_HEADER")
101+
}
102+
} else {
103+
// If we're not cloning the repository, then we need to configure
104+
// which directory contains the cloned repository since `cd`ing
105+
// into the repository directory is not concurrency-safe
106+
configArgs = []string{"-C", repo.directory}
107+
}
108+
cmd := exec.CommandContext(ctx, "git", append(configArgs, args...)...) //nolint:gosec
109+
cmd.Env = append(cmd.Env, env...)
110+
111+
out, err := cmd.Output()
112+
if err != nil {
113+
msg := string(out)
114+
var exitErr *exec.ExitError
115+
if errors.As(err, &exitErr) {
116+
msg = string(exitErr.Stderr)
117+
}
118+
err = fmt.Errorf("git %s error: %s: %w", subCmd, strings.TrimSpace(msg), err)
119+
}
120+
return out, err
121+
}
122+
123+
func (repo *repository) getFileContent(path string) ([]byte, error) {
124+
if _, err := os.Stat(repo.directory); errors.Is(err, os.ErrNotExist) {
125+
return nil, fmt.Errorf("repository clone no longer exists, used after cleaned? %w", err)
126+
}
127+
fileContents, err := os.ReadFile(filepath.Join(repo.directory, path))
128+
if err != nil {
129+
if errors.Is(err, os.ErrNotExist) {
130+
return nil, errors.New("file does not exist")
131+
}
132+
return nil, err
133+
}
134+
return fileContents, nil
135+
}

pkg/resolution/resolver/git/resolver.go

Lines changed: 16 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -17,23 +17,14 @@ limitations under the License.
1717
package git
1818

1919
import (
20-
"bytes"
2120
"context"
2221
"errors"
2322
"fmt"
24-
"io"
2523
"os"
2624
"regexp"
2725
"strings"
2826
"time"
2927

30-
"github.com/go-git/go-billy/v5/memfs"
31-
"github.com/go-git/go-git/v5"
32-
gitcfg "github.com/go-git/go-git/v5/config"
33-
"github.com/go-git/go-git/v5/plumbing"
34-
plumbTransport "github.com/go-git/go-git/v5/plumbing/transport"
35-
"github.com/go-git/go-git/v5/plumbing/transport/http"
36-
"github.com/go-git/go-git/v5/storage/memory"
3728
"github.com/jenkins-x/go-scm/scm"
3829
"github.com/jenkins-x/go-scm/scm/factory"
3930
resolverconfig "github.com/tektoncd/pipeline/pkg/apis/config/resolver"
@@ -180,8 +171,8 @@ func (g *GitResolver) ResolveGitClone(ctx context.Context) (framework.ResolvedRe
180171
if err != nil {
181172
return nil, err
182173
}
183-
repo := g.Params[UrlParam]
184-
if repo == "" {
174+
repoURL := g.Params[UrlParam]
175+
if repoURL == "" {
185176
urlString := conf.URL
186177
if urlString == "" {
187178
return nil, errors.New("default Git Repo Url was not set during installation of the git resolver")
@@ -195,9 +186,8 @@ func (g *GitResolver) ResolveGitClone(ctx context.Context) (framework.ResolvedRe
195186
}
196187
}
197188

198-
cloneOpts := &git.CloneOptions{
199-
URL: repo,
200-
}
189+
var username string
190+
var password string
201191

202192
secretRef := &secretCacheKey{
203193
name: g.Params[GitTokenParam],
@@ -212,73 +202,34 @@ func (g *GitResolver) ResolveGitClone(ctx context.Context) (framework.ResolvedRe
212202
secretRef = nil
213203
}
214204

215-
auth := plumbTransport.AuthMethod(nil)
216205
if secretRef != nil {
217206
gitToken, err := g.getAPIToken(ctx, secretRef, GitTokenKeyParam)
218207
if err != nil {
219208
return nil, err
220209
}
221-
auth = &http.BasicAuth{
222-
Username: "git",
223-
Password: string(gitToken),
224-
}
225-
cloneOpts.Auth = auth
226-
}
227-
228-
filesystem := memfs.New()
229-
repository, err := git.Clone(memory.NewStorage(), filesystem, cloneOpts)
230-
if err != nil {
231-
return nil, fmt.Errorf("clone error: %w", err)
232-
}
233-
234-
// try fetch the branch when the given revision refers to a branch name
235-
refSpec := gitcfg.RefSpec(fmt.Sprintf("+refs/heads/%s:refs/remotes/%s", revision, revision))
236-
err = repository.Fetch(&git.FetchOptions{
237-
RefSpecs: []gitcfg.RefSpec{refSpec},
238-
Auth: auth,
239-
})
240-
if err != nil {
241-
var fetchErr git.NoMatchingRefSpecError
242-
if !errors.As(err, &fetchErr) {
243-
return nil, fmt.Errorf("unexpected fetch error: %w", err)
244-
}
245-
}
246-
247-
w, err := repository.Worktree()
248-
if err != nil {
249-
return nil, fmt.Errorf("worktree error: %w", err)
250-
}
251-
252-
h, err := repository.ResolveRevision(plumbing.Revision(revision))
253-
if err != nil {
254-
return nil, fmt.Errorf("revision error: %w", err)
255-
}
256-
257-
err = w.Checkout(&git.CheckoutOptions{
258-
Hash: *h,
259-
})
260-
if err != nil {
261-
return nil, fmt.Errorf("checkout error: %w", err)
210+
username = "git"
211+
password = string(gitToken)
262212
}
263213

264214
path := g.Params[PathParam]
265215

266-
f, err := filesystem.Open(path)
216+
repo, cleanupFunc, err := resolveRepository(ctx, repoURL, username, password, revision)
217+
defer cleanupFunc()
267218
if err != nil {
268-
return nil, fmt.Errorf("error opening file %q: %w", path, err)
219+
return nil, fmt.Errorf("error resolving repository: %w", err)
269220
}
270221

271-
buf := &bytes.Buffer{}
272-
_, err = io.Copy(buf, f)
222+
fileContents, err := repo.getFileContent(path)
273223
if err != nil {
274-
return nil, fmt.Errorf("error reading file %q: %w", path, err)
224+
return nil, fmt.Errorf("error opening file %q: %w", path, err)
275225
}
276226

277227
return &resolvedGitResource{
278-
Revision: h.String(),
279-
Content: buf.Bytes(),
280-
URL: g.Params[UrlParam],
281-
Path: g.Params[PathParam],
228+
229+
Revision: repo.revision,
230+
Content: fileContents,
231+
URL: repo.url,
232+
Path: path,
282233
}, nil
283234
}
284235

pkg/resolution/resolver/git/resolver_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -448,7 +448,7 @@ func TestResolve(t *testing.T) {
448448
pathInRepo: "foo/new",
449449
url: anonFakeRepoURL,
450450
},
451-
expectedErr: createError("revision error: reference not found"),
451+
expectedErr: createError("error resolving repository: git fetch error: fatal: couldn't find remote ref non-existent-revision: exit status 128"),
452452
}, {
453453
name: "api: successful task from params api information",
454454
args: &params{

tekton/publish.yaml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ spec:
2929
description: Username to be used to login to the container registry
3030
default: "_json_key"
3131
- name: releaseAsLatest
32-
description: Whether to tag and publish this release as Pipelines' latest
32+
description: Whether to tag and publish this release as Pipelines latest
3333
default: "true"
3434
- name: platforms
3535
description: Platforms to publish for the images (e.g. linux/amd64,linux/arm64)
@@ -130,6 +130,7 @@ spec:
130130
$(params.package)/cmd/entrypoint: ${COMBINED_BASE_IMAGE}
131131
$(params.package)/cmd/nop: ${COMBINED_BASE_IMAGE}
132132
$(params.package)/cmd/workingdirinit: ${COMBINED_BASE_IMAGE}
133+
$(params.package)/cmd/resolvers: cgr.dev/chainguard/git@sha256:566235a8ef752f37d285042ee05fc37dbb04293e50f116a231984080fb835693
133134
EOF
134135
135136
cat /workspace/.ko.yaml

test/presubmit-tests.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ function ko_resolve() {
5555
github.com/tektoncd/pipeline/cmd/nop: ghcr.io/tektoncd/pipeline/github.com/tektoncd/pipeline/combined-base-image:latest
5656
github.com/tektoncd/pipeline/cmd/workingdirinit: ghcr.io/tektoncd/pipeline/github.com/tektoncd/pipeline/combined-base-image:latest
5757
58-
github.com/tektoncd/pipeline/cmd/git-init: cgr.dev/chainguard/git
58+
github.com/tektoncd/pipeline/cmd/resolvers: cgr.dev/chainguard/git@sha256:566235a8ef752f37d285042ee05fc37dbb04293e50f116a231984080fb835693
5959
EOF
6060

6161
KO_DOCKER_REPO=example.com ko resolve -l 'app.kubernetes.io/component!=resolvers' --platform=all --push=false -R -f config 1>/dev/null

test/resolvers_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ func TestGitResolver_Clone_Failure(t *testing.T) {
282282
}, {
283283
name: "commit does not exist",
284284
commit: "abcd0123",
285-
expectedErr: "revision error: reference not found",
285+
expectedErr: "error resolving repository: git fetch error: fatal: couldn't find remote ref abcd0123: exit status 128",
286286
},
287287
}
288288

0 commit comments

Comments
 (0)