From 993060c52b98de7eed144cf17d67b5d8862d1c59 Mon Sep 17 00:00:00 2001
From: Somtochiama <somtochionyekwere@gmail.com>
Date: Wed, 4 May 2022 09:37:07 +0100
Subject: [PATCH] gogit: check if revision changed before cloning in checkout
 branch (#694)

* Check if revision has changed in gogit CheckoutBranch

Signed-off-by: Somtochi Onyekwere <somtochionyekwere@gmail.com>
---
 main.go                        |  2 +-
 pkg/git/gogit/checkout.go      | 39 +++++++++++++++++++++++++++++++++-
 pkg/git/gogit/checkout_test.go | 15 +++++++++++--
 3 files changed, 52 insertions(+), 4 deletions(-)

diff --git a/main.go b/main.go
index 483d7cb29..3f964a1f8 100644
--- a/main.go
+++ b/main.go
@@ -245,7 +245,7 @@ func main() {
 		ControllerName: controllerName,
 		Cache:          c,
 		TTL:            ttl,
-		CacheRecorder: cacheRecorder,
+		CacheRecorder:  cacheRecorder,
 	}).SetupWithManagerAndOptions(mgr, controllers.HelmChartReconcilerOptions{
 		MaxConcurrentReconciles: concurrent,
 		RateLimiter:             helper.GetRateLimiter(rateLimiterOptions),
diff --git a/pkg/git/gogit/checkout.go b/pkg/git/gogit/checkout.go
index c401e3dd5..6c5a70642 100644
--- a/pkg/git/gogit/checkout.go
+++ b/pkg/git/gogit/checkout.go
@@ -26,8 +26,10 @@ import (
 
 	"github.com/Masterminds/semver/v3"
 	extgogit "github.com/go-git/go-git/v5"
+	"github.com/go-git/go-git/v5/config"
 	"github.com/go-git/go-git/v5/plumbing"
 	"github.com/go-git/go-git/v5/plumbing/object"
+	"github.com/go-git/go-git/v5/storage/memory"
 
 	"github.com/fluxcd/pkg/gitutil"
 	"github.com/fluxcd/pkg/version"
@@ -50,13 +52,14 @@ func CheckoutStrategyForOptions(_ context.Context, opts git.CheckoutOptions) git
 		if branch == "" {
 			branch = git.DefaultBranch
 		}
-		return &CheckoutBranch{Branch: branch, RecurseSubmodules: opts.RecurseSubmodules}
+		return &CheckoutBranch{Branch: branch, RecurseSubmodules: opts.RecurseSubmodules, LastRevision: opts.LastRevision}
 	}
 }
 
 type CheckoutBranch struct {
 	Branch            string
 	RecurseSubmodules bool
+	LastRevision      string
 }
 
 func (c *CheckoutBranch) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (*git.Commit, error) {
@@ -64,7 +67,31 @@ func (c *CheckoutBranch) Checkout(ctx context.Context, path, url string, opts *g
 	if err != nil {
 		return nil, fmt.Errorf("failed to construct auth method with options: %w", err)
 	}
+
 	ref := plumbing.NewBranchReferenceName(c.Branch)
+	// check if previous revision has changed before attempting to clone
+	if c.LastRevision != "" {
+		config := &config.RemoteConfig{
+			Name: git.DefaultOrigin,
+			URLs: []string{url},
+		}
+		rem := extgogit.NewRemote(memory.NewStorage(), config)
+		refs, err := rem.List(&extgogit.ListOptions{
+			Auth: authMethod,
+		})
+		if err != nil {
+			return nil, fmt.Errorf("unable to list remote for '%s': %w", url, err)
+		}
+
+		currentRevision := filterRefs(refs, ref)
+		if currentRevision != "" && currentRevision == c.LastRevision {
+			return nil, git.NoChangesError{
+				Message:          "no changes since last reconcilation",
+				ObservedRevision: currentRevision,
+			}
+		}
+	}
+
 	repo, err := extgogit.PlainCloneContext(ctx, path, false, &extgogit.CloneOptions{
 		URL:               url,
 		Auth:              authMethod,
@@ -333,3 +360,13 @@ func recurseSubmodules(recurse bool) extgogit.SubmoduleRescursivity {
 	}
 	return extgogit.NoRecurseSubmodules
 }
+
+func filterRefs(refs []*plumbing.Reference, currentRef plumbing.ReferenceName) string {
+	for _, ref := range refs {
+		if ref.Name().String() == currentRef.String() {
+			return fmt.Sprintf("%s/%s", currentRef.Short(), ref.Hash().String())
+		}
+	}
+
+	return ""
+}
diff --git a/pkg/git/gogit/checkout_test.go b/pkg/git/gogit/checkout_test.go
index 019036b0b..4ce7f8c67 100644
--- a/pkg/git/gogit/checkout_test.go
+++ b/pkg/git/gogit/checkout_test.go
@@ -19,6 +19,7 @@ package gogit
 import (
 	"context"
 	"errors"
+	"fmt"
 	"os"
 	"path/filepath"
 	"testing"
@@ -60,6 +61,7 @@ func TestCheckoutBranch_Checkout(t *testing.T) {
 		filesCreated   map[string]string
 		expectedCommit string
 		expectedErr    string
+		lastRevision   string
 	}{
 		{
 			name:           "Default branch",
@@ -68,10 +70,18 @@ func TestCheckoutBranch_Checkout(t *testing.T) {
 			expectedCommit: firstCommit.String(),
 		},
 		{
-			name:           "Other branch",
+			name:         "skip clone if LastRevision hasn't changed",
+			branch:       "master",
+			filesCreated: map[string]string{"branch": "init"},
+			expectedErr:  fmt.Sprintf("no changes since last reconcilation: observed revision 'master/%s'", firstCommit.String()),
+			lastRevision: fmt.Sprintf("master/%s", firstCommit.String()),
+		},
+		{
+			name:           "Other branch - revision has changed",
 			branch:         "test",
 			filesCreated:   map[string]string{"branch": "second"},
 			expectedCommit: secondCommit.String(),
+			lastRevision:   fmt.Sprintf("master/%s", firstCommit.String()),
 		},
 		{
 			name:        "Non existing branch",
@@ -85,7 +95,8 @@ func TestCheckoutBranch_Checkout(t *testing.T) {
 			g := NewWithT(t)
 
 			branch := CheckoutBranch{
-				Branch: tt.branch,
+				Branch:       tt.branch,
+				LastRevision: tt.lastRevision,
 			}
 			tmpDir := t.TempDir()