From f44302eed08ab24d271babf6668b5f2a34f8abe1 Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Tue, 8 Feb 2022 18:38:01 +0000 Subject: [PATCH 1/5] Bump libgit2 to 1.3.0 and git2go to V33. Downstream breaking changes introduced since git2go@V31: - git2go.ErrorCode was deprecated in favour of the native error type. - FetchOptions no longer expects a pointer, but rather the actual value of git2go.FetchOptions. Signed-off-by: Paulo Gomes --- go.mod | 2 +- go.sum | 4 +- pkg/git/libgit2/checkout.go | 10 ++-- pkg/git/libgit2/checkout_test.go | 2 +- pkg/git/libgit2/transport.go | 42 +++++++-------- pkg/git/libgit2/transport_test.go | 86 ++++++++++++++++++++----------- 6 files changed, 86 insertions(+), 60 deletions(-) diff --git a/go.mod b/go.mod index eccc2f0f7..7d496b53a 100644 --- a/go.mod +++ b/go.mod @@ -23,7 +23,7 @@ require ( github.com/go-git/go-billy/v5 v5.3.1 github.com/go-git/go-git/v5 v5.4.2 github.com/go-logr/logr v1.2.2 - github.com/libgit2/git2go/v31 v31.7.6 + github.com/libgit2/git2go/v33 v33.0.6 github.com/minio/minio-go/v7 v7.0.15 github.com/onsi/ginkgo v1.16.5 github.com/onsi/gomega v1.17.0 diff --git a/go.sum b/go.sum index 5581610f4..ddb126abd 100644 --- a/go.sum +++ b/go.sum @@ -622,8 +622,8 @@ github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0/go.mod h1:vmVJ0l/dxyfGW6Fm github.com/lib/pq v1.2.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo= github.com/lib/pq v1.10.0 h1:Zx5DJFEYQXio93kgXnQ09fXNiUKsqv4OUEu2UtGcB1E= github.com/lib/pq v1.10.0/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o= -github.com/libgit2/git2go/v31 v31.7.6 h1:jg/pNomrQULnafmfF6XTkozPX5ypyELoWErWkJuYPcI= -github.com/libgit2/git2go/v31 v31.7.6/go.mod h1:c/rkJcBcUFx6wHaT++UwNpKvIsmPNqCeQ/vzO4DrEec= +github.com/libgit2/git2go/v33 v33.0.6 h1:F//bA3/pgSTVq2hLNahhnof9NxyCzFF/c3MB6lb93Qo= +github.com/libgit2/git2go/v33 v33.0.6/go.mod h1:KdpqkU+6+++4oHna/MIOgx4GCQ92IPCdpVRMRI80J+4= github.com/liggitt/tabwriter v0.0.0-20181228230101-89fcab3d43de h1:9TO3cAIGXtEhnIaL+V+BEER86oLrvS+kWobKpbJuye0= github.com/liggitt/tabwriter v0.0.0-20181228230101-89fcab3d43de/go.mod h1:zAbeS9B/r2mtpb6U+EI2rYA5OAXxsYw6wTamcNW+zcE= github.com/lithammer/dedent v1.1.0/go.mod h1:jrXYCQtgg0nJiN+StA2KgR7w6CiQNv9Fd/Z9BP0jIOc= diff --git a/pkg/git/libgit2/checkout.go b/pkg/git/libgit2/checkout.go index 37be8eeee..9f8a874ae 100644 --- a/pkg/git/libgit2/checkout.go +++ b/pkg/git/libgit2/checkout.go @@ -25,7 +25,7 @@ import ( "github.com/Masterminds/semver/v3" "github.com/go-logr/logr" - git2go "github.com/libgit2/git2go/v31" + git2go "github.com/libgit2/git2go/v33" "github.com/fluxcd/pkg/gitutil" "github.com/fluxcd/pkg/version" @@ -61,7 +61,7 @@ type CheckoutBranch struct { func (c *CheckoutBranch) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (*git.Commit, error) { repo, err := git2go.Clone(url, path, &git2go.CloneOptions{ - FetchOptions: &git2go.FetchOptions{ + FetchOptions: git2go.FetchOptions{ DownloadTags: git2go.DownloadTagsNone, RemoteCallbacks: RemoteCallbacks(ctx, opts), ProxyOptions: git2go.ProxyOptions{Type: git2go.ProxyTypeAuto}, @@ -91,7 +91,7 @@ type CheckoutTag struct { func (c *CheckoutTag) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (*git.Commit, error) { repo, err := git2go.Clone(url, path, &git2go.CloneOptions{ - FetchOptions: &git2go.FetchOptions{ + FetchOptions: git2go.FetchOptions{ DownloadTags: git2go.DownloadTagsAll, RemoteCallbacks: RemoteCallbacks(ctx, opts), ProxyOptions: git2go.ProxyOptions{Type: git2go.ProxyTypeAuto}, @@ -115,7 +115,7 @@ type CheckoutCommit struct { func (c *CheckoutCommit) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (*git.Commit, error) { repo, err := git2go.Clone(url, path, &git2go.CloneOptions{ - FetchOptions: &git2go.FetchOptions{ + FetchOptions: git2go.FetchOptions{ DownloadTags: git2go.DownloadTagsNone, RemoteCallbacks: RemoteCallbacks(ctx, opts), ProxyOptions: git2go.ProxyOptions{Type: git2go.ProxyTypeAuto}, @@ -147,7 +147,7 @@ func (c *CheckoutSemVer) Checkout(ctx context.Context, path, url string, opts *g } repo, err := git2go.Clone(url, path, &git2go.CloneOptions{ - FetchOptions: &git2go.FetchOptions{ + FetchOptions: git2go.FetchOptions{ DownloadTags: git2go.DownloadTagsAll, RemoteCallbacks: RemoteCallbacks(ctx, opts), ProxyOptions: git2go.ProxyOptions{Type: git2go.ProxyTypeAuto}, diff --git a/pkg/git/libgit2/checkout_test.go b/pkg/git/libgit2/checkout_test.go index 0e82986d0..e55b87ade 100644 --- a/pkg/git/libgit2/checkout_test.go +++ b/pkg/git/libgit2/checkout_test.go @@ -25,7 +25,7 @@ import ( "testing" "time" - git2go "github.com/libgit2/git2go/v31" + git2go "github.com/libgit2/git2go/v33" . "github.com/onsi/gomega" ) diff --git a/pkg/git/libgit2/transport.go b/pkg/git/libgit2/transport.go index d8d120a24..41ea151a4 100644 --- a/pkg/git/libgit2/transport.go +++ b/pkg/git/libgit2/transport.go @@ -31,7 +31,7 @@ import ( "strings" "time" - git2go "github.com/libgit2/git2go/v31" + git2go "github.com/libgit2/git2go/v33" "golang.org/x/crypto/ssh" "golang.org/x/crypto/ssh/knownhosts" @@ -61,16 +61,16 @@ func RemoteCallbacks(ctx context.Context, opts *git.AuthOptions) git2go.RemoteCa // libgit2 it should stop the transfer when the given context is closed (due to // e.g. a timeout). func transferProgressCallback(ctx context.Context) git2go.TransferProgressCallback { - return func(p git2go.TransferProgress) git2go.ErrorCode { + return func(p git2go.TransferProgress) error { // Early return if all the objects have been received. if p.ReceivedObjects == p.TotalObjects { - return git2go.ErrorCodeOK + return nil } select { case <-ctx.Done(): - return git2go.ErrorCodeUser + return fmt.Errorf("transport close - potentially due to a timeout") default: - return git2go.ErrorCodeOK + return nil } } } @@ -79,12 +79,12 @@ func transferProgressCallback(ctx context.Context) git2go.TransferProgressCallba // libgit2 it should cancel the network operation when the given context is // closed. func transportMessageCallback(ctx context.Context) git2go.TransportMessageCallback { - return func(_ string) git2go.ErrorCode { + return func(_ string) error { select { case <-ctx.Done(): - return git2go.ErrorCodeUser + return fmt.Errorf("transport closed") default: - return git2go.ErrorCodeOK + return nil } } } @@ -93,16 +93,16 @@ func transportMessageCallback(ctx context.Context) git2go.TransportMessageCallba // signals libgit2 it should stop the push transfer when the given context is // closed (due to e.g. a timeout). func pushTransferProgressCallback(ctx context.Context) git2go.PushTransferProgressCallback { - return func(current, total uint32, _ uint) git2go.ErrorCode { + return func(current, total uint32, _ uint) error { // Early return if current equals total. if current == total { - return git2go.ErrorCodeOK + return nil } select { case <-ctx.Done(): - return git2go.ErrorCodeUser + return fmt.Errorf("transport close - potentially due to a timeout") default: - return git2go.ErrorCodeOK + return nil } } } @@ -155,10 +155,10 @@ func certificateCallback(opts *git.AuthOptions) git2go.CertificateCheckCallback // x509Callback returns a CertificateCheckCallback that verifies the // certificate against the given caBundle for git.HTTPS Transports. func x509Callback(caBundle []byte) git2go.CertificateCheckCallback { - return func(cert *git2go.Certificate, valid bool, hostname string) git2go.ErrorCode { + return func(cert *git2go.Certificate, valid bool, hostname string) error { roots := x509.NewCertPool() if ok := roots.AppendCertsFromPEM(caBundle); !ok { - return git2go.ErrorCodeCertificate + return fmt.Errorf("x509 cert could not be appended") } opts := x509.VerifyOptions{ @@ -167,9 +167,9 @@ func x509Callback(caBundle []byte) git2go.CertificateCheckCallback { CurrentTime: now(), } if _, err := cert.X509.Verify(opts); err != nil { - return git2go.ErrorCodeCertificate + return fmt.Errorf("x509 cert could not be verified") } - return git2go.ErrorCodeOK + return nil } } @@ -177,10 +177,10 @@ func x509Callback(caBundle []byte) git2go.CertificateCheckCallback { // the key of Git server against the given host and known_hosts for // git.SSH Transports. func knownHostsCallback(host string, knownHosts []byte) git2go.CertificateCheckCallback { - return func(cert *git2go.Certificate, valid bool, hostname string) git2go.ErrorCode { + return func(cert *git2go.Certificate, valid bool, hostname string) error { kh, err := parseKnownHosts(string(knownHosts)) if err != nil { - return git2go.ErrorCodeCertificate + return fmt.Errorf("failed to parse known_hosts: %w", err) } // First, attempt to split the configured host and port to validate @@ -200,7 +200,7 @@ func knownHostsCallback(host string, knownHosts []byte) git2go.CertificateCheckC } if hostnameWithoutPort != hostWithoutPort { - return git2go.ErrorCodeUser + return fmt.Errorf("host mismatch: %q %q\n", hostWithoutPort, hostnameWithoutPort) } // We are now certain that the configured host and the hostname @@ -210,10 +210,10 @@ func knownHostsCallback(host string, knownHosts []byte) git2go.CertificateCheckC h := knownhosts.Normalize(host) for _, k := range kh { if k.matches(h, cert.Hostkey) { - return git2go.ErrorCodeOK + return nil } } - return git2go.ErrorCodeCertificate + return fmt.Errorf("hostkey could not be verified") } } diff --git a/pkg/git/libgit2/transport_test.go b/pkg/git/libgit2/transport_test.go index 4a14b3af5..a5b330aeb 100644 --- a/pkg/git/libgit2/transport_test.go +++ b/pkg/git/libgit2/transport_test.go @@ -23,10 +23,11 @@ import ( "encoding/base64" "encoding/pem" "errors" + "fmt" "testing" "time" - git2go "github.com/libgit2/git2go/v31" + git2go "github.com/libgit2/git2go/v33" . "github.com/onsi/gomega" ) @@ -144,42 +145,42 @@ func Test_x509Callback(t *testing.T) { certificate string host string caBundle []byte - want git2go.ErrorCode + want error }{ { name: "Valid certificate authority bundle", certificate: googleLeafFixture, host: "www.google.com", caBundle: []byte(giag2IntermediateFixture + "\n" + geoTrustRootFixture), - want: git2go.ErrorCodeOK, + want: nil, }, { name: "Invalid certificate", certificate: googleLeafWithInvalidHashFixture, host: "www.google.com", caBundle: []byte(giag2IntermediateFixture + "\n" + geoTrustRootFixture), - want: git2go.ErrorCodeCertificate, + want: fmt.Errorf("x509 cert could not be verified"), }, { name: "Invalid certificate authority bundle", certificate: googleLeafFixture, host: "www.google.com", caBundle: bytes.Trim([]byte(giag2IntermediateFixture+"\n"+geoTrustRootFixture), "-"), - want: git2go.ErrorCodeCertificate, + want: fmt.Errorf("x509 cert could not be appended"), }, { name: "Missing intermediate in bundle", certificate: googleLeafFixture, host: "www.google.com", caBundle: []byte(geoTrustRootFixture), - want: git2go.ErrorCodeCertificate, + want: fmt.Errorf("x509 cert could not be verified"), }, { name: "Invalid host", certificate: googleLeafFixture, host: "www.google.co", caBundle: []byte(giag2IntermediateFixture + "\n" + geoTrustRootFixture), - want: git2go.ErrorCodeCertificate, + want: fmt.Errorf("x509 cert could not be verified"), }, } for _, tt := range tests { @@ -194,7 +195,12 @@ func Test_x509Callback(t *testing.T) { } callback := x509Callback(tt.caBundle) - g.Expect(callback(cert, false, tt.host)).To(Equal(tt.want)) + result := g.Expect(callback(cert, false, tt.host)) + if tt.want == nil { + result.To(BeNil()) + } else { + result.To(Equal(tt.want)) + } }) } } @@ -206,7 +212,7 @@ func Test_knownHostsCallback(t *testing.T) { expectedHost string knownHosts []byte hostkey git2go.HostkeyCertificate - want git2go.ErrorCode + want error }{ { name: "Match", @@ -214,7 +220,7 @@ func Test_knownHostsCallback(t *testing.T) { knownHosts: []byte(knownHostsFixture), hostkey: git2go.HostkeyCertificate{Kind: git2go.HostkeySHA1 | git2go.HostkeyMD5, HashSHA1: sha1Fingerprint("v2toJdKXfFEaR1u++4iq1UqSrHM")}, expectedHost: "github.com", - want: git2go.ErrorCodeOK, + want: nil, }, { name: "Match with port", @@ -222,7 +228,7 @@ func Test_knownHostsCallback(t *testing.T) { knownHosts: []byte(knownHostsFixture), hostkey: git2go.HostkeyCertificate{Kind: git2go.HostkeySHA1 | git2go.HostkeyMD5, HashSHA1: sha1Fingerprint("v2toJdKXfFEaR1u++4iq1UqSrHM")}, expectedHost: "github.com:22", - want: git2go.ErrorCodeOK, + want: nil, }, { name: "Hostname mismatch", @@ -230,7 +236,7 @@ func Test_knownHostsCallback(t *testing.T) { knownHosts: []byte(knownHostsFixture), hostkey: git2go.HostkeyCertificate{Kind: git2go.HostkeySHA1 | git2go.HostkeyMD5, HashSHA1: sha1Fingerprint("v2toJdKXfFEaR1u++4iq1UqSrHM")}, expectedHost: "example.com", - want: git2go.ErrorCodeUser, + want: fmt.Errorf("host mismatch: %q %q\n", "example.com", "github.com"), }, { name: "Hostkey mismatch", @@ -238,7 +244,7 @@ func Test_knownHostsCallback(t *testing.T) { knownHosts: []byte(knownHostsFixture), hostkey: git2go.HostkeyCertificate{Kind: git2go.HostkeyMD5, HashMD5: md5Fingerprint("\xb6\x03\x0e\x39\x97\x9e\xd0\xe7\x24\xce\xa3\x77\x3e\x01\x42\x09")}, expectedHost: "github.com", - want: git2go.ErrorCodeCertificate, + want: fmt.Errorf("hostkey could not be verified"), }, } for _, tt := range tests { @@ -247,7 +253,12 @@ func Test_knownHostsCallback(t *testing.T) { cert := &git2go.Certificate{Hostkey: tt.hostkey} callback := knownHostsCallback(tt.expectedHost, tt.knownHosts) - g.Expect(callback(cert, false, tt.host)).To(Equal(tt.want)) + result := g.Expect(callback(cert, false, tt.host)) + if tt.want == nil { + result.To(BeNil()) + } else { + result.To(Equal(tt.want)) + } }) } } @@ -352,7 +363,7 @@ func Test_transferProgressCallback(t *testing.T) { name string progress git2go.TransferProgress cancelFunc func(context.CancelFunc) - wantErr git2go.ErrorCode + wantErr error }{ { name: "ok - in progress", @@ -361,7 +372,7 @@ func Test_transferProgressCallback(t *testing.T) { ReceivedObjects: 21, }, cancelFunc: func(cf context.CancelFunc) {}, - wantErr: git2go.ErrorCodeOK, + wantErr: nil, }, { name: "ok - transfer complete", @@ -370,7 +381,7 @@ func Test_transferProgressCallback(t *testing.T) { ReceivedObjects: 30, }, cancelFunc: func(cf context.CancelFunc) {}, - wantErr: git2go.ErrorCodeOK, + wantErr: nil, }, { name: "ok - transfer complete, context cancelled", @@ -379,7 +390,7 @@ func Test_transferProgressCallback(t *testing.T) { ReceivedObjects: 30, }, cancelFunc: func(cf context.CancelFunc) { cf() }, - wantErr: git2go.ErrorCodeOK, + wantErr: nil, }, { name: "error - context cancelled", @@ -388,7 +399,7 @@ func Test_transferProgressCallback(t *testing.T) { ReceivedObjects: 21, }, cancelFunc: func(cf context.CancelFunc) { cf() }, - wantErr: git2go.ErrorCodeUser, + wantErr: fmt.Errorf("transport close - potentially due to a timeout"), }, } @@ -403,7 +414,12 @@ func Test_transferProgressCallback(t *testing.T) { tt.cancelFunc(cancel) - g.Expect(tpcb(tt.progress)).To(Equal(tt.wantErr)) + result := g.Expect(tpcb(tt.progress)) + if tt.wantErr == nil { + result.To(BeNil()) + } else { + result.To(Equal(tt.wantErr)) + } }) } } @@ -412,17 +428,17 @@ func Test_transportMessageCallback(t *testing.T) { tests := []struct { name string cancelFunc func(context.CancelFunc) - wantErr git2go.ErrorCode + wantErr error }{ { name: "ok - transport open", cancelFunc: func(cf context.CancelFunc) {}, - wantErr: git2go.ErrorCodeOK, + wantErr: nil, }, { name: "error - transport closed", cancelFunc: func(cf context.CancelFunc) { cf() }, - wantErr: git2go.ErrorCodeUser, + wantErr: fmt.Errorf("transport closed"), }, } @@ -437,7 +453,12 @@ func Test_transportMessageCallback(t *testing.T) { tt.cancelFunc(cancel) - g.Expect(tmcb("")).To(Equal(tt.wantErr)) + result := g.Expect(tmcb("")) + if tt.wantErr == nil { + result.To(BeNil()) + } else { + result.To(Equal(tt.wantErr)) + } }) } } @@ -452,31 +473,31 @@ func Test_pushTransferProgressCallback(t *testing.T) { name string progress pushProgress cancelFunc func(context.CancelFunc) - wantErr git2go.ErrorCode + wantErr error }{ { name: "ok - in progress", progress: pushProgress{current: 20, total: 25}, cancelFunc: func(cf context.CancelFunc) {}, - wantErr: git2go.ErrorCodeOK, + wantErr: nil, }, { name: "ok - transfer complete", progress: pushProgress{current: 25, total: 25}, cancelFunc: func(cf context.CancelFunc) {}, - wantErr: git2go.ErrorCodeOK, + wantErr: nil, }, { name: "ok - transfer complete, context cancelled", progress: pushProgress{current: 25, total: 25}, cancelFunc: func(cf context.CancelFunc) { cf() }, - wantErr: git2go.ErrorCodeOK, + wantErr: nil, }, { name: "error - context cancelled", progress: pushProgress{current: 20, total: 25}, cancelFunc: func(cf context.CancelFunc) { cf() }, - wantErr: git2go.ErrorCodeUser, + wantErr: fmt.Errorf("transport close - potentially due to a timeout"), }, } @@ -491,7 +512,12 @@ func Test_pushTransferProgressCallback(t *testing.T) { tt.cancelFunc(cancel) - g.Expect(ptpcb(tt.progress.current, tt.progress.total, tt.progress.bytes)).To(Equal(tt.wantErr)) + result := g.Expect(ptpcb(tt.progress.current, tt.progress.total, tt.progress.bytes)) + if tt.wantErr == nil { + result.To(BeNil()) + } else { + result.To(Equal(tt.wantErr)) + } }) } } From e5d032fe9c83b5bc6f71e79e71b81164c67264ec Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Tue, 8 Feb 2022 18:39:07 +0000 Subject: [PATCH 2/5] Add libgit2 checkout test with ED25519 key This adds a test to detect any regression in libgit2's ED25519 key support. go-git supports ED25519 but not the current version of libgit2 used in flux. The updates to libgit2 in v1.2.0 adds support for ED25519. This test would help ensure the right version of libgit2 is used. Signed-off-by: Sunny Signed-off-by: Paulo Gomes --- pkg/git/libgit2/checkout_test.go | 71 +++++++++++++++++++++++ pkg/git/libgit2/testdata/git/repo/foo.txt | 1 + 2 files changed, 72 insertions(+) create mode 100644 pkg/git/libgit2/testdata/git/repo/foo.txt diff --git a/pkg/git/libgit2/checkout_test.go b/pkg/git/libgit2/checkout_test.go index e55b87ade..7d96eb1b6 100644 --- a/pkg/git/libgit2/checkout_test.go +++ b/pkg/git/libgit2/checkout_test.go @@ -20,13 +20,19 @@ import ( "context" "errors" "fmt" + "net/url" "os" "path/filepath" "testing" "time" + "github.com/fluxcd/pkg/gittestserver" + "github.com/fluxcd/pkg/ssh" git2go "github.com/libgit2/git2go/v33" . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + + "github.com/fluxcd/source-controller/pkg/git" ) func TestCheckoutBranch_Checkout(t *testing.T) { @@ -444,3 +450,68 @@ func mockSignature(time time.Time) *git2go.Signature { When: time, } } + +// This test is specifically to detect regression in libgit2's ED25519 key +// support for client authentication. +// 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, + }, + } + + authOpts, err := git.AuthOptionsFromSecret(repoURL, &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, authOpts) + g.Expect(err).To(BeNil()) +} 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 From 514126c4b88db1e42ae1286328d500d1b5f8a2bc Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Wed, 9 Feb 2022 11:33:34 +0000 Subject: [PATCH 3/5] Fix make test on arm64 runners The environment variables set at the Makefile were causing go install to yield a corrupted file for setup-envtest. To fix the issue, such operation is now always executed in a clean bash. Signed-off-by: Paulo Gomes --- .github/workflows/e2e.yaml | 6 +----- Makefile | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml index 2ce9efcb3..cd00250b4 100644 --- a/.github/workflows/e2e.yaml +++ b/.github/workflows/e2e.yaml @@ -57,11 +57,7 @@ jobs: with: go-version: 1.17.x - name: Run tests - run: | - mkdir tmp-download; cd tmp-download; go mod init go-download; - GOBIN="${GITHUB_WORKSPACE}/build/gobin" go install sigs.k8s.io/controller-runtime/tools/setup-envtest@latest - cd ..; rm -rf tmp-download - make test + run: make test - name: Prepare id: prep run: | diff --git a/Makefile b/Makefile index 3537a6065..02cd94022 100644 --- a/Makefile +++ b/Makefile @@ -224,7 +224,7 @@ TMP_DIR=$$(mktemp -d) ;\ cd $$TMP_DIR ;\ go mod init tmp ;\ echo "Downloading $(2)" ;\ -go install $(2) ;\ +env -i bash -c "GOBIN=$(GOBIN) PATH=$(PATH) GOPATH=$(shell go env GOPATH) GOCACHE=$(shell go env GOCACHE) go install $(2)" ;\ rm -rf $$TMP_DIR ;\ } endef From f0d7a6bb48f4b41ddc41d2a621b3d37c98ff6fcd Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Wed, 16 Feb 2022 10:30:27 +0000 Subject: [PATCH 4/5] Update libgit2 attributions Signed-off-by: Paulo Gomes --- ATTRIBUTIONS.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/ATTRIBUTIONS.md b/ATTRIBUTIONS.md index 20d5ada82..054d70c73 100644 --- a/ATTRIBUTIONS.md +++ b/ATTRIBUTIONS.md @@ -1203,6 +1203,18 @@ STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +---------------------------------------------------------------------- + +The xoroshiro256** implementation is licensed in the public domain: + +Written in 2018 by David Blackman and Sebastiano Vigna (vigna@acm.org) + +To the extent possible under law, the author has dedicated all copyright +and related and neighboring rights to this software to the public domain +worldwide. This software is distributed without any warranty. + +See . + *** ## zlib From 842970899702a8c2105202a7c3526785b3ed827d Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Wed, 16 Feb 2022 11:39:11 +0000 Subject: [PATCH 5/5] Upgrade libgit2 to libgit2-1.3.0-2 Signed-off-by: Paulo Gomes --- Dockerfile | 2 +- Makefile | 9 ++++++++- hack/install-libraries.sh | 2 -- pkg/git/libgit2/checkout_test.go | 5 +++-- pkg/git/libgit2/transport.go | 10 +++++----- pkg/git/libgit2/transport_test.go | 20 ++++++++++---------- tests/fuzz/gitrepository_fuzzer.go | 9 +++++---- tests/fuzz/oss_fuzz_build.sh | 20 ++++++++++++++++++-- 8 files changed, 50 insertions(+), 27 deletions(-) diff --git a/Dockerfile b/Dockerfile index acf4f2866..3f9802f1b 100644 --- a/Dockerfile +++ b/Dockerfile @@ -3,7 +3,7 @@ ARG GO_VERSION=1.17 ARG XX_VERSION=1.1.0 ARG LIBGIT2_IMG=ghcr.io/fluxcd/golang-with-libgit2 -ARG LIBGIT2_TAG=libgit2-1.1.1-7 +ARG LIBGIT2_TAG=libgit2-1.3.0-2 FROM ${LIBGIT2_IMG}:${LIBGIT2_TAG} AS libgit2-libs diff --git a/Makefile b/Makefile index 02cd94022..3b0e5b876 100644 --- a/Makefile +++ b/Makefile @@ -4,7 +4,7 @@ TAG ?= latest # Base image used to build the Go binary LIBGIT2_IMG ?= ghcr.io/fluxcd/golang-with-libgit2 -LIBGIT2_TAG ?= libgit2-1.1.1-7 +LIBGIT2_TAG ?= libgit2-1.3.0-2 # Allows for defining additional Docker buildx arguments, # e.g. '--push'. @@ -136,6 +136,7 @@ tidy: ## Run go mod tidy fmt: ## Run go fmt against code go fmt ./... cd api; go fmt ./... + cd tests/fuzz; go fmt . vet: $(LIBGIT2) ## Run go vet against code go vet ./... @@ -208,6 +209,12 @@ ifneq ($(shell grep -o 'LIBGIT2_IMG ?= \w.*' Makefile | cut -d ' ' -f 3):$(shell exit 1; \ } endif +ifneq ($(shell grep -o 'LIBGIT2_TAG ?= \w.*' Makefile | cut -d ' ' -f 3), $(shell grep -o "LIBGIT2_TAG=.*" tests/fuzz/oss_fuzz_build.sh | sed 's;LIBGIT2_TAG="$${LIBGIT2_TAG:-;;g' | sed 's;}";;g')) + @{ \ + echo "LIBGIT2_TAG must match in both Makefile and tests/fuzz/oss_fuzz_build.sh"; \ + exit 1; \ + } +endif ifneq (, $(shell git status --porcelain --untracked-files=no)) @{ \ echo "working directory is dirty:"; \ diff --git a/hack/install-libraries.sh b/hack/install-libraries.sh index 270ce1915..70866eea1 100755 --- a/hack/install-libraries.sh +++ b/hack/install-libraries.sh @@ -45,8 +45,6 @@ function setup_current() { mkdir -p "./build/libgit2" if [[ $OSTYPE == 'darwin'* ]]; then # For MacOS development environments, download the amd64 static libraries released from from golang-with-libgit2. - - #TODO: update URL with official URL + TAG: curl -o output.tar.gz -LO "https://github.com/fluxcd/golang-with-libgit2/releases/download/${TAG}/darwin-libs.tar.gz" DIR=libgit2-darwin diff --git a/pkg/git/libgit2/checkout_test.go b/pkg/git/libgit2/checkout_test.go index 7d96eb1b6..ff2f5ccd5 100644 --- a/pkg/git/libgit2/checkout_test.go +++ b/pkg/git/libgit2/checkout_test.go @@ -26,12 +26,13 @@ import ( "testing" "time" - "github.com/fluxcd/pkg/gittestserver" - "github.com/fluxcd/pkg/ssh" git2go "github.com/libgit2/git2go/v33" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" + "github.com/fluxcd/pkg/gittestserver" + "github.com/fluxcd/pkg/ssh" + "github.com/fluxcd/source-controller/pkg/git" ) diff --git a/pkg/git/libgit2/transport.go b/pkg/git/libgit2/transport.go index 41ea151a4..22efa054a 100644 --- a/pkg/git/libgit2/transport.go +++ b/pkg/git/libgit2/transport.go @@ -68,7 +68,7 @@ func transferProgressCallback(ctx context.Context) git2go.TransferProgressCallba } select { case <-ctx.Done(): - return fmt.Errorf("transport close - potentially due to a timeout") + return fmt.Errorf("transport close (potentially due to a timeout)") default: return nil } @@ -100,7 +100,7 @@ func pushTransferProgressCallback(ctx context.Context) git2go.PushTransferProgre } select { case <-ctx.Done(): - return fmt.Errorf("transport close - potentially due to a timeout") + return fmt.Errorf("transport close (potentially due to a timeout)") default: return nil } @@ -158,7 +158,7 @@ func x509Callback(caBundle []byte) git2go.CertificateCheckCallback { return func(cert *git2go.Certificate, valid bool, hostname string) error { roots := x509.NewCertPool() if ok := roots.AppendCertsFromPEM(caBundle); !ok { - return fmt.Errorf("x509 cert could not be appended") + return fmt.Errorf("PEM CA bundle could not be appended to x509 certificate pool") } opts := x509.VerifyOptions{ @@ -167,7 +167,7 @@ func x509Callback(caBundle []byte) git2go.CertificateCheckCallback { CurrentTime: now(), } if _, err := cert.X509.Verify(opts); err != nil { - return fmt.Errorf("x509 cert could not be verified") + return fmt.Errorf("verification failed: %w", err) } return nil } @@ -200,7 +200,7 @@ func knownHostsCallback(host string, knownHosts []byte) git2go.CertificateCheckC } if hostnameWithoutPort != hostWithoutPort { - return fmt.Errorf("host mismatch: %q %q\n", hostWithoutPort, hostnameWithoutPort) + return fmt.Errorf("host mismatch: %q %q", hostWithoutPort, hostnameWithoutPort) } // We are now certain that the configured host and the hostname diff --git a/pkg/git/libgit2/transport_test.go b/pkg/git/libgit2/transport_test.go index a5b330aeb..0028fad58 100644 --- a/pkg/git/libgit2/transport_test.go +++ b/pkg/git/libgit2/transport_test.go @@ -159,28 +159,28 @@ func Test_x509Callback(t *testing.T) { certificate: googleLeafWithInvalidHashFixture, host: "www.google.com", caBundle: []byte(giag2IntermediateFixture + "\n" + geoTrustRootFixture), - want: fmt.Errorf("x509 cert could not be verified"), + want: fmt.Errorf(`verification failed: x509: certificate signed by unknown authority (possibly because of "x509: cannot verify signature: algorithm unimplemented" while trying to verify candidate authority certificate "Google Internet Authority G2")`), }, { name: "Invalid certificate authority bundle", certificate: googleLeafFixture, host: "www.google.com", caBundle: bytes.Trim([]byte(giag2IntermediateFixture+"\n"+geoTrustRootFixture), "-"), - want: fmt.Errorf("x509 cert could not be appended"), + want: fmt.Errorf("PEM CA bundle could not be appended to x509 certificate pool"), }, { name: "Missing intermediate in bundle", certificate: googleLeafFixture, host: "www.google.com", caBundle: []byte(geoTrustRootFixture), - want: fmt.Errorf("x509 cert could not be verified"), + want: fmt.Errorf("verification failed: x509: certificate signed by unknown authority"), }, { name: "Invalid host", certificate: googleLeafFixture, host: "www.google.co", caBundle: []byte(giag2IntermediateFixture + "\n" + geoTrustRootFixture), - want: fmt.Errorf("x509 cert could not be verified"), + want: fmt.Errorf("verification failed: x509: certificate is valid for www.google.com, not www.google.co"), }, } for _, tt := range tests { @@ -195,11 +195,11 @@ func Test_x509Callback(t *testing.T) { } callback := x509Callback(tt.caBundle) - result := g.Expect(callback(cert, false, tt.host)) + result := callback(cert, false, tt.host) if tt.want == nil { - result.To(BeNil()) + g.Expect(result).To(BeNil()) } else { - result.To(Equal(tt.want)) + g.Expect(result.Error()).To(Equal(tt.want.Error())) } }) } @@ -236,7 +236,7 @@ func Test_knownHostsCallback(t *testing.T) { knownHosts: []byte(knownHostsFixture), hostkey: git2go.HostkeyCertificate{Kind: git2go.HostkeySHA1 | git2go.HostkeyMD5, HashSHA1: sha1Fingerprint("v2toJdKXfFEaR1u++4iq1UqSrHM")}, expectedHost: "example.com", - want: fmt.Errorf("host mismatch: %q %q\n", "example.com", "github.com"), + want: fmt.Errorf("host mismatch: %q %q", "example.com", "github.com"), }, { name: "Hostkey mismatch", @@ -399,7 +399,7 @@ func Test_transferProgressCallback(t *testing.T) { ReceivedObjects: 21, }, cancelFunc: func(cf context.CancelFunc) { cf() }, - wantErr: fmt.Errorf("transport close - potentially due to a timeout"), + wantErr: fmt.Errorf("transport close (potentially due to a timeout)"), }, } @@ -497,7 +497,7 @@ func Test_pushTransferProgressCallback(t *testing.T) { name: "error - context cancelled", progress: pushProgress{current: 20, total: 25}, cancelFunc: func(cf context.CancelFunc) { cf() }, - wantErr: fmt.Errorf("transport close - potentially due to a timeout"), + wantErr: fmt.Errorf("transport close (potentially due to a timeout)"), }, } diff --git a/tests/fuzz/gitrepository_fuzzer.go b/tests/fuzz/gitrepository_fuzzer.go index 9ccc0fdf0..01c4cc949 100644 --- a/tests/fuzz/gitrepository_fuzzer.go +++ b/tests/fuzz/gitrepository_fuzzer.go @@ -38,10 +38,6 @@ import ( "time" fuzz "github.com/AdaLogics/go-fuzz-headers" - "github.com/fluxcd/pkg/gittestserver" - "github.com/fluxcd/pkg/runtime/testenv" - sourcev1 "github.com/fluxcd/source-controller/api/v1beta1" - "github.com/fluxcd/source-controller/controllers" "github.com/go-git/go-billy/v5" "github.com/go-git/go-billy/v5/memfs" "github.com/go-git/go-git/v5" @@ -61,6 +57,11 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/envtest" "sigs.k8s.io/controller-runtime/pkg/manager" + + "github.com/fluxcd/pkg/gittestserver" + "github.com/fluxcd/pkg/runtime/testenv" + sourcev1 "github.com/fluxcd/source-controller/api/v1beta1" + "github.com/fluxcd/source-controller/controllers" ) var ( diff --git a/tests/fuzz/oss_fuzz_build.sh b/tests/fuzz/oss_fuzz_build.sh index b70ceca2f..2878342a1 100755 --- a/tests/fuzz/oss_fuzz_build.sh +++ b/tests/fuzz/oss_fuzz_build.sh @@ -16,7 +16,7 @@ set -euxo pipefail -LIBGIT2_TAG="${LIBGIT2_TAG:-libgit2-1.1.1-7}" +LIBGIT2_TAG="${LIBGIT2_TAG:-libgit2-1.3.0-2}" GOPATH="${GOPATH:-/root/go}" GO_SRC="${GOPATH}/src" PROJECT_PATH="github.com/fluxcd/source-controller" @@ -54,10 +54,25 @@ export PKG_CONFIG_PATH="${TARGET_DIR}/lib/pkgconfig:${TARGET_DIR}/lib64/pkgconfi export CGO_CFLAGS="-I${TARGET_DIR}/include -I${TARGET_DIR}/include/openssl" export CGO_LDFLAGS="$(pkg-config --libs --static --cflags libssh2 openssl libgit2)" -go mod tidy -compat=1.17 +go mod tidy + +# The implementation of libgit2 is sensitive to the versions of git2go. +# Leaving it to its own devices, the minimum version of git2go used may not +# be compatible with the currently implemented version. Hence the modifications +# of the existing go.mod. +sed "s;\./api;$(/bin/pwd)/api;g" go.mod > tests/fuzz/go.mod +sed -i 's;module github.com/fluxcd/source-controller;module github.com/fluxcd/source-controller/tests/fuzz;g' tests/fuzz/go.mod +echo "replace github.com/fluxcd/source-controller => $(/bin/pwd)/" >> tests/fuzz/go.mod + +cp go.sum tests/fuzz/go.sum pushd "tests/fuzz" +go mod download + +go get -d github.com/AdaLogics/go-fuzz-headers +go get -d github.com/fluxcd/source-controller + # Setup files to be embedded into controllers_fuzzer.go's testFiles variable. mkdir -p testdata/crd cp ../../config/crd/bases/*.yaml testdata/crd/ @@ -89,6 +104,7 @@ go_compile FuzzRandomGitFiles fuzz_gitrepository_fuzzer go_compile FuzzGitResourceObject fuzz_git_resource_object # By now testdata is embedded in the binaries and no longer needed. +# Remove the dir given that it will be owned by root otherwise. rm -rf testdata/ popd