Libgit2 ED25519 check & clone respect context

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 '<url>', error: Certificate"),
+				*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.GitOperationFailedReason, "Failed to checkout and determine revision: unable to clone '<url>': Certificate"),
 			},
 		},
 		{

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

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=

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,17 +110,18 @@ 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{
 				CredentialsCallback:      auth.CredCallback,
 				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,17 +201,18 @@ 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{
 				CredentialsCallback:      auth.CredCallback,
 				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)

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"))
+}

pkg/git/libgit2/testdata/git/repo/foo.txt

@@ -0,0 +1 @@
+test file