From c0c5046e5f516a80de892e96f5af59a1a6e4adb6 Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Fri, 6 May 2022 08:39:00 +0100 Subject: [PATCH 1/2] libgit2: recover from git2go panic Some specific scenarios may lead libgit2 or git2go to panic. Adding a recovery logic, ensures a predictable execution path for callers, and safeguards the controller's stability. Signed-off-by: Paulo Gomes --- pkg/git/libgit2/checkout.go | 21 +++++++++++++---- pkg/git/libgit2/checkout_test.go | 39 ++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 4 deletions(-) diff --git a/pkg/git/libgit2/checkout.go b/pkg/git/libgit2/checkout.go index 6e9fccb1b..046019dff 100644 --- a/pkg/git/libgit2/checkout.go +++ b/pkg/git/libgit2/checkout.go @@ -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{ + repo, err := safeClone(url, path, &git2go.CloneOptions{ FetchOptions: git2go.FetchOptions{ DownloadTags: git2go.DownloadTagsNone, RemoteCallbacks: RemoteCallbacks(ctx, opts), @@ -94,7 +94,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{ + repo, err := safeClone(url, path, &git2go.CloneOptions{ FetchOptions: git2go.FetchOptions{ DownloadTags: git2go.DownloadTagsAll, RemoteCallbacks: RemoteCallbacks(ctx, opts), @@ -118,7 +118,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{ + repo, err := safeClone(url, path, &git2go.CloneOptions{ FetchOptions: git2go.FetchOptions{ DownloadTags: git2go.DownloadTagsNone, RemoteCallbacks: RemoteCallbacks(ctx, opts), @@ -150,7 +150,7 @@ func (c *CheckoutSemVer) Checkout(ctx context.Context, path, url string, opts *g return nil, fmt.Errorf("semver parse error: %w", err) } - repo, err := git2go.Clone(url, path, &git2go.CloneOptions{ + repo, err := safeClone(url, path, &git2go.CloneOptions{ FetchOptions: git2go.FetchOptions{ DownloadTags: git2go.DownloadTagsAll, RemoteCallbacks: RemoteCallbacks(ctx, opts), @@ -239,6 +239,19 @@ func (c *CheckoutSemVer) Checkout(ctx context.Context, path, url string, opts *g return buildCommit(cc, "refs/tags/"+t), nil } +// safeClone wraps git2go calls with panic recovering logic, ensuring +// a predictable execution path for callers. +func safeClone(url, path string, cloneOpts *git2go.CloneOptions) (repo *git2go.Repository, err error) { + defer func() { + if r := recover(); r != nil { + err = fmt.Errorf("recovered from git2go panic: %v", r) + } + }() + + repo, err = git2go.Clone(url, path, cloneOpts) + return +} + // checkoutDetachedDwim attempts to perform a detached HEAD checkout by first DWIMing the short name // to get a concrete reference, and then calling checkoutDetachedHEAD. func checkoutDetachedDwim(repo *git2go.Repository, name string) (*git2go.Commit, error) { diff --git a/pkg/git/libgit2/checkout_test.go b/pkg/git/libgit2/checkout_test.go index a649607fa..dadb58820 100644 --- a/pkg/git/libgit2/checkout_test.go +++ b/pkg/git/libgit2/checkout_test.go @@ -506,3 +506,42 @@ func TestCheckout_ED25519(t *testing.T) { _, err = branchCheckoutStrat.Checkout(ctx, tmpDir, repoURL, authOpts) g.Expect(err).To(BeNil()) } + +func TestSafeClone(t *testing.T) { + g := NewWithT(t) + + // 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() + + sshURL := server.SSHAddress() + repoURL := sshURL + "/test.git" + + u, err := url.Parse(sshURL) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(u.Host).ToNot(BeEmpty()) + + repo, err := safeClone(repoURL, t.TempDir(), &git2go.CloneOptions{ + FetchOptions: git2go.FetchOptions{ + RemoteCallbacks: git2go.RemoteCallbacks{ + CertificateCheckCallback: func(cert *git2go.Certificate, valid bool, hostname string) error { + panic("Oops!") + }, + }, + }}) + + g.Expect(repo).To(BeNil()) + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).Should(ContainSubstring("recovered from git2go panic")) +} From d86ea25e87fcef50ce4e89020fd97a60f1502257 Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Fri, 6 May 2022 08:59:56 +0100 Subject: [PATCH 2/2] Fix minio test by changing region to us-west-2 Signed-off-by: Paulo Gomes --- pkg/minio/minio_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/minio/minio_test.go b/pkg/minio/minio_test.go index d1c3caed8..c99c3ea46 100644 --- a/pkg/minio/minio_test.go +++ b/pkg/minio/minio_test.go @@ -39,7 +39,7 @@ import ( const ( objectName string = "test.yaml" objectEtag string = "2020beab5f1711919157756379622d1d" - region string = "us-east-1" + region string = "us-west-2" ) var (