diff --git a/controllers/gitrepository_controller_test.go b/controllers/gitrepository_controller_test.go index 975ad028e..3ad146608 100644 --- a/controllers/gitrepository_controller_test.go +++ b/controllers/gitrepository_controller_test.go @@ -245,7 +245,7 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) { }, wantErr: true, assertConditions: []metav1.Condition{ - *conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.GitOperationFailedReason, "Failed to checkout and determine revision: unable to clone '', error: Certificate"), + *conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.GitOperationFailedReason, "Failed to checkout and determine revision: unable to clone '': Certificate"), }, }, { diff --git a/go.mod b/go.mod index f9debcdf5..8fe726fc4 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,7 @@ require ( github.com/Masterminds/semver/v3 v3.1.1 github.com/cyphar/filepath-securejoin v0.2.2 github.com/fluxcd/pkg/apis/meta v0.11.0-rc.1 - github.com/fluxcd/pkg/gittestserver v0.3.2 + github.com/fluxcd/pkg/gittestserver v0.4.0 github.com/fluxcd/pkg/gitutil v0.1.0 github.com/fluxcd/pkg/helmtestserver v0.2.0 github.com/fluxcd/pkg/lockedfile v0.1.0 diff --git a/go.sum b/go.sum index 8397a9ecc..f65c3d32f 100644 --- a/go.sum +++ b/go.sum @@ -230,8 +230,8 @@ github.com/fluxcd/pkg/apis/acl v0.0.1 h1:biCgZMjpDSv3Q4mZPikUJILx3t2MuNXR4Oa5jRQ github.com/fluxcd/pkg/apis/acl v0.0.1/go.mod h1:y3qOXUFObVWk7jzOjubMnr/u18j1kCeSi6olycnxr/E= github.com/fluxcd/pkg/apis/meta v0.11.0-rc.1 h1:RHHrztAFv9wmjM+Pk7Svt1UdD+1SdnQSp76MWFiM7Hg= github.com/fluxcd/pkg/apis/meta v0.11.0-rc.1/go.mod h1:yUblM2vg+X8TE3A2VvJfdhkGmg+uqBlSPkLk7dxi0UM= -github.com/fluxcd/pkg/gittestserver v0.3.2 h1:oc1OoZ4b+kAu0vu/RT9wUwuQZxSqEjBOlQWYYA+YeLM= -github.com/fluxcd/pkg/gittestserver v0.3.2/go.mod h1:8j36Z6B0BuKNZZ6exAWoyDEpyQoFcjz1IX3WBT7PZNg= +github.com/fluxcd/pkg/gittestserver v0.4.0 h1:VQzQ5TcHzohxbYGWpnQ/79w7/rnS2SQGC7FSDtbIsCA= +github.com/fluxcd/pkg/gittestserver v0.4.0/go.mod h1:hUPx21fe/6oox336Wih/XF1fnmzLmptNMOvATbTZXNY= github.com/fluxcd/pkg/gitutil v0.1.0 h1:VO3kJY/CKOCO4ysDNqfdpTg04icAKBOSb3lbR5uE/IE= github.com/fluxcd/pkg/gitutil v0.1.0/go.mod h1:Ybz50Ck5gkcnvF0TagaMwtlRy3X3wXuiri1HVsK5id4= github.com/fluxcd/pkg/helmtestserver v0.2.0 h1:cE7YHDmrWI0hr9QpaaeQ0vQ16Z0IiqZKiINDpqdY610= @@ -899,7 +899,6 @@ golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8U golang.org/x/crypto v0.0.0-20200414173820-0848c9571904/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/crypto v0.0.0-20200709230013-948cd5f35899/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= -golang.org/x/crypto v0.0.0-20200728195943-123391ffb6de/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/crypto v0.0.0-20201002170205-7f63de1d35b0/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/crypto v0.0.0-20201203163018-be400aefbc4c/go.mod h1:jdWPYTVW3xRLrWPugEBEK3UY2ZEsg3UU495nc5E+M+I= golang.org/x/crypto v0.0.0-20201221181555-eec23a3978ad/go.mod h1:jdWPYTVW3xRLrWPugEBEK3UY2ZEsg3UU495nc5E+M+I= diff --git a/pkg/git/libgit2/checkout.go b/pkg/git/libgit2/checkout.go index 01363f8fa..8733458b4 100644 --- a/pkg/git/libgit2/checkout.go +++ b/pkg/git/libgit2/checkout.go @@ -23,10 +23,10 @@ import ( "time" "github.com/Masterminds/semver/v3" - "github.com/fluxcd/pkg/version" git2go "github.com/libgit2/git2go/v31" "github.com/fluxcd/pkg/gitutil" + "github.com/fluxcd/pkg/version" sourcev1 "github.com/fluxcd/source-controller/api/v1beta1" "github.com/fluxcd/source-controller/pkg/git" @@ -53,12 +53,34 @@ func CheckoutStrategyForRef(ref *sourcev1.GitRepositoryRef, opt git.CheckoutOpti } } +// clone is a wrapper around git2go.Clone that respects the provided context. +func clone(ctx context.Context, path, url string, auth *git.Auth, opts *git2go.CloneOptions) (*git2go.Repository, error) { + var repo *git2go.Repository + errCh := make(chan error, 1) + go func() { + var err error + repo, err = git2go.Clone(url, path, opts) + errCh <- err + }() + + select { + case err := <-errCh: + if err != nil { + return nil, fmt.Errorf("unable to clone '%s': %w", url, gitutil.LibGit2Error(err)) + } + case <-ctx.Done(): + return nil, fmt.Errorf("clone context cancelled: %w", ctx.Err()) + } + + return repo, nil +} + type CheckoutBranch struct { branch string } func (c *CheckoutBranch) Checkout(ctx context.Context, path, url string, auth *git.Auth) (git.Commit, string, error) { - repo, err := git2go.Clone(url, path, &git2go.CloneOptions{ + cloneOpts := &git2go.CloneOptions{ FetchOptions: &git2go.FetchOptions{ DownloadTags: git2go.DownloadTagsNone, RemoteCallbacks: git2go.RemoteCallbacks{ @@ -67,9 +89,10 @@ func (c *CheckoutBranch) Checkout(ctx context.Context, path, url string, auth *g }, }, CheckoutBranch: c.branch, - }) + } + repo, err := clone(ctx, path, url, auth, cloneOpts) if err != nil { - return nil, "", fmt.Errorf("unable to clone '%s', error: %w", url, gitutil.LibGit2Error(err)) + return nil, "", err } head, err := repo.Head() if err != nil { @@ -87,7 +110,7 @@ type CheckoutTag struct { } func (c *CheckoutTag) Checkout(ctx context.Context, path, url string, auth *git.Auth) (git.Commit, string, error) { - repo, err := git2go.Clone(url, path, &git2go.CloneOptions{ + cloneOpts := &git2go.CloneOptions{ FetchOptions: &git2go.FetchOptions{ DownloadTags: git2go.DownloadTagsAll, RemoteCallbacks: git2go.RemoteCallbacks{ @@ -95,9 +118,10 @@ func (c *CheckoutTag) Checkout(ctx context.Context, path, url string, auth *git. CertificateCheckCallback: auth.CertCallback, }, }, - }) + } + repo, err := clone(ctx, path, url, auth, cloneOpts) if err != nil { - return nil, "", fmt.Errorf("unable to clone '%s', error: %w", url, err) + return nil, "", err } ref, err := repo.References.Dwim(c.tag) if err != nil { @@ -131,7 +155,7 @@ type CheckoutCommit struct { } func (c *CheckoutCommit) Checkout(ctx context.Context, path, url string, auth *git.Auth) (git.Commit, string, error) { - repo, err := git2go.Clone(url, path, &git2go.CloneOptions{ + cloneOpts := &git2go.CloneOptions{ FetchOptions: &git2go.FetchOptions{ DownloadTags: git2go.DownloadTagsNone, RemoteCallbacks: git2go.RemoteCallbacks{ @@ -140,9 +164,10 @@ func (c *CheckoutCommit) Checkout(ctx context.Context, path, url string, auth *g }, }, CheckoutBranch: c.branch, - }) + } + repo, err := clone(ctx, path, url, auth, cloneOpts) if err != nil { - return nil, "", fmt.Errorf("unable to clone '%s', error: %w", url, err) + return nil, "", err } oid, err := git2go.NewOid(c.commit) if err != nil { @@ -176,7 +201,7 @@ func (c *CheckoutSemVer) Checkout(ctx context.Context, path, url string, auth *g return nil, "", fmt.Errorf("semver parse range error: %w", err) } - repo, err := git2go.Clone(url, path, &git2go.CloneOptions{ + cloneOpts := &git2go.CloneOptions{ FetchOptions: &git2go.FetchOptions{ DownloadTags: git2go.DownloadTagsAll, RemoteCallbacks: git2go.RemoteCallbacks{ @@ -184,9 +209,10 @@ func (c *CheckoutSemVer) Checkout(ctx context.Context, path, url string, auth *g CertificateCheckCallback: auth.CertCallback, }, }, - }) + } + repo, err := clone(ctx, path, url, auth, cloneOpts) if err != nil { - return nil, "", fmt.Errorf("unable to clone '%s', error: %w", url, err) + return nil, "", err } tags := make(map[string]string) diff --git a/pkg/git/libgit2/checkout_test.go b/pkg/git/libgit2/checkout_test.go index 6de5484d8..a7428c964 100644 --- a/pkg/git/libgit2/checkout_test.go +++ b/pkg/git/libgit2/checkout_test.go @@ -21,11 +21,18 @@ import ( "crypto/sha256" "encoding/hex" "io" + "net/url" "os" "path" + "path/filepath" "testing" + "time" + "github.com/fluxcd/pkg/gittestserver" + "github.com/fluxcd/pkg/ssh" git2go "github.com/libgit2/git2go/v31" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" "github.com/fluxcd/source-controller/pkg/git" ) @@ -77,3 +84,73 @@ func TestCheckoutTagSemVer_Checkout(t *testing.T) { t.Errorf("expected semver hash %s, got %s", cTag.Hash(), cSemVer.Hash()) } } + +// This test is specifically to detect regression in libgit2's ED25519 key +// support. +// Refer: https://github.com/fluxcd/source-controller/issues/399 +func TestCheckout_ED25519(t *testing.T) { + g := NewWithT(t) + timeout := 5 * time.Second + + // Create a git test server. + server, err := gittestserver.NewTempGitServer() + g.Expect(err).ToNot(HaveOccurred()) + defer os.RemoveAll(server.Root()) + server.Auth("test-user", "test-pswd") + server.AutoCreate() + + server.KeyDir(filepath.Join(server.Root(), "keys")) + g.Expect(server.ListenSSH()).To(Succeed()) + + go func() { + server.StartSSH() + }() + defer server.StopSSH() + + repoPath := "test.git" + + err = server.InitRepo("testdata/git/repo", git.DefaultBranch, repoPath) + g.Expect(err).NotTo(HaveOccurred()) + + sshURL := server.SSHAddress() + repoURL := sshURL + "/" + repoPath + + // Fetch host key. + u, err := url.Parse(sshURL) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(u.Host).ToNot(BeEmpty()) + knownHosts, err := ssh.ScanHostKey(u.Host, timeout) + g.Expect(err).ToNot(HaveOccurred()) + + kp, err := ssh.NewEd25519Generator().Generate() + g.Expect(err).ToNot(HaveOccurred()) + + secret := corev1.Secret{ + Data: map[string][]byte{ + "identity": kp.PrivateKey, + "known_hosts": knownHosts, + }, + } + + authStrategy, err := AuthSecretStrategyForURL(repoURL) + g.Expect(err).ToNot(HaveOccurred()) + gitAuth, err := authStrategy.Method(secret) + g.Expect(err).ToNot(HaveOccurred()) + + // Prepare for checkout. + branchCheckoutStrat := &CheckoutBranch{branch: git.DefaultBranch} + tmpDir, _ := os.MkdirTemp("", "test") + defer os.RemoveAll(tmpDir) + + ctx, cancel := context.WithTimeout(context.TODO(), timeout) + defer cancel() + + // Checkout the repo. + // This should always fail because the generated key above isn't present in + // the git server. + _, _, err = branchCheckoutStrat.Checkout(ctx, tmpDir, repoURL, gitAuth) + g.Expect(err).To(HaveOccurred()) + // NOTE: libgit2 v1.2+ supports ED25519. Flip this condition after updating + // to libgit2 v1.2+. + g.Expect(err.Error()).To(ContainSubstring("Unable to extract public key from private key")) +} diff --git a/pkg/git/libgit2/testdata/git/repo/foo.txt b/pkg/git/libgit2/testdata/git/repo/foo.txt new file mode 100644 index 000000000..16b14f5da --- /dev/null +++ b/pkg/git/libgit2/testdata/git/repo/foo.txt @@ -0,0 +1 @@ +test file