Merge pull request #9 from zapier/git-clone-depth

Configurable git clone depth

pkg/git/git_repo.go

@@ -3,6 +3,8 @@ package git
 import (
 	"fmt"
 	"io/fs"
+	"os"
+	"strconv"
 
 	"github.com/go-git/go-git/v5"
 	"github.com/go-git/go-git/v5/config"
@@ -121,3 +123,15 @@ func WalkRepo(s string, d fs.DirEntry, err error) error {
 	}
 	return nil
 }
+
+// GetCloneDepth reads the provided env var and returns an int to be used as git clone depth. Default is 0.
+func GetCloneDepth(envVar string) int {
+	val := os.Getenv(envVar)
+	if val != "" {
+		depth, err := strconv.Atoi(val)
+		if err == nil {
+			return depth
+		}
+	}
+	return 0
+}

pkg/git/git_repo_test.go

@@ -1,6 +1,7 @@
 package git
 
 import (
+	"os"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -92,3 +93,20 @@ func TestGetModifiedFileNamesBetweenCommitsNoResults(t *testing.T) {
 	assert.Equal(t, nil, err)
 	assert.Equal(t, len(modifiedFiles), 0, "expected no files modified between master and test")
 }
+
+func TestGitCloneDepth(t *testing.T) {
+	testVar := "git-clone-test"
+	defer os.Unsetenv(testVar)
+	t.Run("no value", func(t *testing.T) {
+		os.Unsetenv(testVar)
+		assert.Equal(t, 0, GetCloneDepth(testVar))
+	})
+	t.Run("500 depth", func(t *testing.T) {
+		os.Setenv(testVar, "500")
+		assert.Equal(t, 500, GetCloneDepth(testVar))
+	})
+	t.Run("invalid value", func(t *testing.T) {
+		os.Setenv(testVar, "somestuff")
+		assert.Equal(t, 0, GetCloneDepth(testVar))
+	})
+}

pkg/github/client.go

@@ -119,6 +119,8 @@ func (c *Client) GetMergeRequestModifiedFiles(prID int, fullName string) ([]stri
 	return []string{}, nil
 }
 
+const GITHUB_CLONE_DEPTH_ENV = "TFBUDDY_GITHUB_CLONE_DEPTH"
+
 func (c *Client) CloneMergeRequest(project string, mr vcs.MR, dest string) (vcs.GitRepo, error) {
 	parts, err := splitFullName(project)
 	if err != nil {
@@ -140,13 +142,13 @@ func (c *Client) CloneMergeRequest(project string, mr vcs.MR, dest string) (vcs.
 	if log.Trace().Enabled() {
 		progress = os.Stdout
 	}
-
+	cloneDepth := zgit.GetCloneDepth(GITHUB_CLONE_DEPTH_ENV)
 	gitRepo, err := git.PlainClone(dest, false, &git.CloneOptions{
 		Auth:          auth,
 		URL:           *repo.CloneURL,
 		ReferenceName: ref,
 		SingleBranch:  true,
-		Depth:         10,
+		Depth:         cloneDepth,
 	})
 
 	if err != nil && err != git.ErrRepositoryAlreadyExists {
@@ -158,8 +160,8 @@ func (c *Client) CloneMergeRequest(project string, mr vcs.MR, dest string) (vcs.
 		//RemoteName:        "",
 		ReferenceName: ref,
 		//SingleBranch:      false,
-		//Depth:             0,
-		Auth: auth,
+		Depth: cloneDepth,
+		Auth:  auth,
 		//RecurseSubmodules: 0,
 		Progress: progress,
 		Force:    false,

pkg/github/hooks/stream_worker.go

@@ -96,11 +96,9 @@ func (h *GithubHooksHandler) processIssueCommentEvent(msg *GithubIssueCommentEve
 	}
 	executedWorkspaces, tfError := trigger.TriggerTFCEvents()
 	if tfError == nil && len(executedWorkspaces.Errored) > 0 {
-		failedMsg := ""
 		for _, failedWS := range executedWorkspaces.Errored {
-			failedMsg += fmt.Sprintf("%s could not be run because: %s\n", failedWS.Name, failedWS.Error)
+			h.postPullRequestComment(event, fmt.Sprintf(":no_entry: %s could not be run because: %s", failedWS.Name, failedWS.Error))
 		}
-		h.postPullRequestComment(event, fmt.Sprintf(":no_entry: %s", failedMsg))
 		return nil
 	}
 	return tfError

pkg/gitlab/git_actions.go

@@ -15,6 +15,8 @@ import (
 	"gopkg.in/errgo.v2/fmt/errors"
 )
 
+const GITLAB_CLONE_DEPTH_ENV = "TFBUDDY_GITLAB_CLONE_DEPTH"
+
 // CloneMergeRequest performs a git clone of the target Gitlab project & merge request branch to the `dest` path.
 func (c *GitlabClient) CloneMergeRequest(project string, mr vcs.MR, dest string) (vcs.GitRepo, error) {
 	proj, _, err := c.client.Projects.GetProject(project, &gitlab.GetProjectOptions{
@@ -36,13 +38,14 @@ func (c *GitlabClient) CloneMergeRequest(project string, mr vcs.MR, dest string)
 	if log.Trace().Enabled() {
 		progress = os.Stdout
 	}
+	cloneDepth := zgit.GetCloneDepth(GITLAB_CLONE_DEPTH_ENV)
 
 	repo, err := git.PlainClone(dest, false, &git.CloneOptions{
 		Auth:          auth,
 		URL:           proj.HTTPURLToRepo,
 		ReferenceName: ref,
 		SingleBranch:  true,
-		Depth:         10,
+		Depth:         cloneDepth,
 		Progress:      progress,
 	})
 
@@ -55,8 +58,8 @@ func (c *GitlabClient) CloneMergeRequest(project string, mr vcs.MR, dest string)
 		//RemoteName:        "",
 		ReferenceName: ref,
 		//SingleBranch:      false,
-		//Depth:             0,
-		Auth: auth,
+		Depth: cloneDepth,
+		Auth:  auth,
 		//RecurseSubmodules: 0,
 		Progress: progress,
 		Force:    false,

pkg/gitlab_hooks/comment_actions.go

@@ -83,11 +83,9 @@ func (w *GitlabEventWorker) processNoteEvent(event vcs.MRCommentEvent) (projectN
 	}
 	executedWorkspaces, tfError := trigger.TriggerTFCEvents()
 	if tfError == nil && len(executedWorkspaces.Errored) > 0 {
-		failedMsg := ""
 		for _, failedWS := range executedWorkspaces.Errored {
-			failedMsg += fmt.Sprintf("%s could not be run because: %s\n", failedWS.Name, failedWS.Error)
+			w.postMessageToMergeRequest(event, fmt.Sprintf(":no_entry: %s could not be run because: %s", failedWS.Name, failedWS.Error))
 		}
-		w.postMessageToMergeRequest(event, fmt.Sprintf(":no_entry: %s", failedMsg))
 		return proj, nil
 	}
 	return proj, tfError

pkg/gitlab_hooks/comment_actions_test.go

@@ -219,16 +219,12 @@ func TestProcessNoteEventPlanFailedWorkspace(t *testing.T) {
 	defer os.Unsetenv(allow_list.GitlabProjectAllowListEnv)
 	mockCtrl := gomock.NewController(t)
 	defer mockCtrl.Finish()
-	mockGitClient := mocks.NewMockGitClient(mockCtrl)
-
-	mockGitClient.EXPECT().CreateMergeRequestComment(101, "zapier/service-tf-buddy", ":no_entry: service-tf-buddy could not be run because: could not fetch upstream\n").Return(nil)
+	testSuite := mocks.CreateTestSuite(mockCtrl, mocks.TestOverrides{}, t)
 
-	mockApiClient := mocks.NewMockApiClient(mockCtrl)
-	mockStreamClient := mocks.NewMockStreamClient(mockCtrl)
-	mockProject := mocks.NewMockProject(mockCtrl)
-	mockProject.EXPECT().GetPathWithNamespace().Return("zapier/service-tf-buddy").Times(2)
+	testSuite.MockGitClient.EXPECT().CreateMergeRequestComment(101, testSuite.MetaData.ProjectNameNS, ":no_entry: service-tf-buddy could not be run because: could not fetch upstream").Return(nil)
 
 	mockLastCommit := mocks.NewMockCommit(mockCtrl)
+
 	mockLastCommit.EXPECT().GetSHA().Return("abvc12345")
 
 	mockAttributes := mocks.NewMockMRAttributes(mockCtrl)
@@ -237,21 +233,15 @@ func TestProcessNoteEventPlanFailedWorkspace(t *testing.T) {
 	mockAttributes.EXPECT().GetType().Return("SomeNote")
 
 	mockMREvent := mocks.NewMockMRCommentEvent(mockCtrl)
-	mockMREvent.EXPECT().GetProject().Return(mockProject).Times(2)
+	mockMREvent.EXPECT().GetProject().Return(testSuite.MockProject).Times(2)
 	mockMREvent.EXPECT().GetAttributes().Return(mockAttributes).Times(2)
 	mockMREvent.EXPECT().GetLastCommit().Return(mockLastCommit)
 
-	mockSimpleMR := mocks.NewMockMR(mockCtrl)
-	mockSimpleMR.EXPECT().GetSourceBranch().Return("DTA-2009")
-
-	mockSimpleMR.EXPECT().GetInternalID().Return(101).Times(2)
-	mockMREvent.EXPECT().GetMR().Return(mockSimpleMR).Times(3)
+	mockMREvent.EXPECT().GetMR().Return(testSuite.MockGitMR).Times(3)
 
 	mockTFCTrigger := mocks.NewMockTrigger(mockCtrl)
-	mockTFCConfig := mocks.NewMockTriggerConfig(mockCtrl)
-	mockTFCConfig.EXPECT().SetAction(tfc_trigger.PlanAction)
-	mockTFCConfig.EXPECT().SetWorkspace("service-tf-buddy")
-	mockTFCTrigger.EXPECT().GetConfig().Return(mockTFCConfig).Times(2)
+
+	mockTFCTrigger.EXPECT().GetConfig().Return(testSuite.MockTriggerConfig).Times(2)
 	mockTFCTrigger.EXPECT().TriggerTFCEvents().Return(&tfc_trigger.TriggeredTFCWorkspaces{
 		Errored: []*tfc_trigger.ErroredWorkspace{{
 			Name:  "service-tf-buddy",
@@ -260,10 +250,12 @@ func TestProcessNoteEventPlanFailedWorkspace(t *testing.T) {
 		},
 	}, nil)
 
+	testSuite.InitTestSuite()
+
 	worker := &GitlabEventWorker{
-		tfc:       mockApiClient,
-		gl:        mockGitClient,
-		runstream: mockStreamClient,
+		tfc:       testSuite.MockApiClient,
+		gl:        testSuite.MockGitClient,
+		runstream: testSuite.MockStreamClient,
 		triggerCreation: func(gl vcs.GitClient, tfc tfc_api.ApiClient, runstream runstream.StreamClient, cfg tfc_trigger.TriggerConfig) tfc_trigger.Trigger {
 			return mockTFCTrigger
 		},
@@ -273,7 +265,7 @@ func TestProcessNoteEventPlanFailedWorkspace(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if proj != "zapier/service-tf-buddy" {
+	if proj != testSuite.MetaData.ProjectNameNS {
 		t.Fatal("expected a project name to be returned")
 	}
 }
@@ -283,15 +275,12 @@ func TestProcessNoteEventPlanFailedMultipleWorkspaces(t *testing.T) {
 	defer os.Unsetenv(allow_list.GitlabProjectAllowListEnv)
 	mockCtrl := gomock.NewController(t)
 	defer mockCtrl.Finish()
-	mockGitClient := mocks.NewMockGitClient(mockCtrl)
-
-	mockGitClient.EXPECT().CreateMergeRequestComment(101, "zapier/service-tf-buddy", ":no_entry: service-tf-buddy could not be run because: could not fetch upstream\nservice-tf-buddy-staging could not be run because: workspace has been modified on target branch\n").Return(nil)
-
-	mockApiClient := mocks.NewMockApiClient(mockCtrl)
-	mockStreamClient := mocks.NewMockStreamClient(mockCtrl)
-	mockProject := mocks.NewMockProject(mockCtrl)
-	mockProject.EXPECT().GetPathWithNamespace().Return("zapier/service-tf-buddy").Times(2)
+	testSuite := mocks.CreateTestSuite(mockCtrl, mocks.TestOverrides{}, t)
 
+	gomock.InOrder(
+		testSuite.MockGitClient.EXPECT().CreateMergeRequestComment(101, testSuite.MetaData.ProjectNameNS, ":no_entry: service-tf-buddy could not be run because: could not fetch upstream").Return(nil),
+		testSuite.MockGitClient.EXPECT().CreateMergeRequestComment(101, testSuite.MetaData.ProjectNameNS, ":no_entry: service-tf-buddy-staging could not be run because: workspace has been modified on target branch").Return(nil),
+	)
 	mockLastCommit := mocks.NewMockCommit(mockCtrl)
 	mockLastCommit.EXPECT().GetSHA().Return("abvc12345")
 
@@ -301,15 +290,10 @@ func TestProcessNoteEventPlanFailedMultipleWorkspaces(t *testing.T) {
 	mockAttributes.EXPECT().GetType().Return("SomeNote")
 
 	mockMREvent := mocks.NewMockMRCommentEvent(mockCtrl)
-	mockMREvent.EXPECT().GetProject().Return(mockProject).Times(2)
+	mockMREvent.EXPECT().GetProject().Return(testSuite.MockProject).AnyTimes()
 	mockMREvent.EXPECT().GetAttributes().Return(mockAttributes).Times(2)
 	mockMREvent.EXPECT().GetLastCommit().Return(mockLastCommit)
-
-	mockSimpleMR := mocks.NewMockMR(mockCtrl)
-	mockSimpleMR.EXPECT().GetSourceBranch().Return("DTA-2009")
-
-	mockSimpleMR.EXPECT().GetInternalID().Return(101).Times(2)
-	mockMREvent.EXPECT().GetMR().Return(mockSimpleMR).Times(3)
+	mockMREvent.EXPECT().GetMR().Return(testSuite.MockGitMR).AnyTimes()
 
 	mockTFCTrigger := mocks.NewMockTrigger(mockCtrl)
 	mockTFCConfig := mocks.NewMockTriggerConfig(mockCtrl)
@@ -328,10 +312,12 @@ func TestProcessNoteEventPlanFailedMultipleWorkspaces(t *testing.T) {
 		},
 	}, nil)
 
+	testSuite.InitTestSuite()
+
 	client := &GitlabEventWorker{
-		gl:        mockGitClient,
-		tfc:       mockApiClient,
-		runstream: mockStreamClient,
+		gl:        testSuite.MockGitClient,
+		tfc:       testSuite.MockApiClient,
+		runstream: testSuite.MockStreamClient,
 		triggerCreation: func(gl vcs.GitClient, tfc tfc_api.ApiClient, runstream runstream.StreamClient, cfg tfc_trigger.TriggerConfig) tfc_trigger.Trigger {
 			return mockTFCTrigger
 		},
@@ -341,7 +327,7 @@ func TestProcessNoteEventPlanFailedMultipleWorkspaces(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if proj != "zapier/service-tf-buddy" {
+	if proj != testSuite.MetaData.ProjectNameNS {
 		t.Fatal("expected a project name to be returned")
 	}
 }

pkg/mocks/helpers.go

@@ -127,6 +127,7 @@ type TestSuite struct {
 	MockTriggerConfig *MockTriggerConfig
 	MockApiClient     *MockApiClient
 	MockStreamClient  *MockStreamClient
+	MockProject       *MockProject
 	MetaData          *TestMetaData
 }
 type TestOverrides struct {
@@ -171,10 +172,12 @@ func (ts *TestSuite) InitTestSuite() {
 	ts.MockGitClient.EXPECT().CreateMergeRequestDiscussion(ts.MetaData.MRIID, ts.MetaData.ProjectNameNS, &RegexMatcher{regex: regexp.MustCompile("Starting TFC apply for Workspace: `([A-z\\-]){1,}/([A-z\\-]){1,}`.")}).Return(ts.MockGitDisc, nil).AnyTimes()
 
 	ts.MockTriggerConfig.EXPECT().GetAction().Return(tfc_trigger.ApplyAction).AnyTimes()
+	ts.MockTriggerConfig.EXPECT().SetAction(gomock.Any()).AnyTimes()
 	ts.MockTriggerConfig.EXPECT().GetMergeRequestIID().Return(ts.MetaData.MRIID).AnyTimes()
 	ts.MockTriggerConfig.EXPECT().GetProjectNameWithNamespace().Return(ts.MetaData.ProjectNameNS).AnyTimes()
 	ts.MockTriggerConfig.EXPECT().GetBranch().Return(ts.MetaData.SourceBranch).AnyTimes()
 	ts.MockTriggerConfig.EXPECT().GetWorkspace().Return("").AnyTimes()
+	ts.MockTriggerConfig.EXPECT().SetWorkspace(gomock.Any()).AnyTimes()
 	ts.MockTriggerConfig.EXPECT().SetMergeRequestDiscussionID(gomock.Any()).AnyTimes()
 	ts.MockTriggerConfig.EXPECT().SetMergeRequestRootNoteID(gomock.Any()).AnyTimes()
 	ts.MockTriggerConfig.EXPECT().GetCommitSHA().Return("abcd12233").AnyTimes()
@@ -188,6 +191,8 @@ func (ts *TestSuite) InitTestSuite() {
 
 	ts.MockStreamClient.EXPECT().AddRunMeta(gomock.Any()).AnyTimes()
 
+	ts.MockProject.EXPECT().GetPathWithNamespace().Return(ts.MetaData.ProjectNameNS).AnyTimes()
+
 }
 
 const TF_WORKSPACE_NAME = "service-tfbuddy"
@@ -226,6 +231,8 @@ func CreateTestSuite(mockCtrl *gomock.Controller, overrides TestOverrides, t *te
 
 	mockStreamClient := NewMockStreamClient(mockCtrl)
 
+	mockProject := NewMockProject(mockCtrl)
+
 	return &TestSuite{
 		MockGitClient:     mockGitClient,
 		MockGitMR:         mockGitMR,
@@ -235,6 +242,7 @@ func CreateTestSuite(mockCtrl *gomock.Controller, overrides TestOverrides, t *te
 		MockTriggerConfig: mockTriggerConfig,
 		MockApiClient:     mockApiClient,
 		MockStreamClient:  mockStreamClient,
+		MockProject:       mockProject,
 		MetaData: &TestMetaData{
 			MRIID:         mrIID,
 			ProjectNameNS: projectNameNS,

pkg/tfc_trigger/tfc_trigger.go

@@ -198,6 +198,12 @@ func (t *TFCTrigger) handleError(err error, msg string) error {
 	return err
 }
 
+// / postUpdate puts a message on a relevant MR
+func (t *TFCTrigger) postUpdate(msg string) error {
+	return t.gl.CreateMergeRequestComment(t.cfg.GetMergeRequestIID(), t.cfg.GetProjectNameWithNamespace(), msg)
+
+}
+
 func (t *TFCTrigger) getLockingMR(workspace string) string {
 	tags, err := t.tfc.GetTagsByQuery(context.Background(), workspace, tfPrefix)
 	if err != nil {
@@ -252,8 +258,8 @@ type TriggeredTFCWorkspaces struct {
 	Executed []string
 }
 
-func (t *TFCTrigger) getModifiedWorkspaceBetweenMergeBaseTargetBranch(mr vcs.MR, repo vcs.GitRepo) (map[string]byte, error) {
-	modifiedWSMap := make(map[string]byte, 0)
+func (t *TFCTrigger) getModifiedWorkspaceBetweenMergeBaseTargetBranch(mr vcs.MR, repo vcs.GitRepo) (map[string]struct{}, error) {
+	modifiedWSMap := make(map[string]struct{}, 0)
 	// update the cloned repo to have the latest commits from target branch (usually main)
 	err := repo.FetchUpstreamBranch(mr.GetTargetBranch())
 	if err != nil {
@@ -264,14 +270,14 @@ func (t *TFCTrigger) getModifiedWorkspaceBetweenMergeBaseTargetBranch(mr vcs.MR,
 	if err != nil {
 		return nil, t.handleError(err, "could not find merge base")
 	}
-	log.Info().Msgf("got common %s", commonSHA)
+	log.Debug().Msgf("got common hash %s for source branch %s and target branch %s", commonSHA, mr.GetSourceBranch(), mr.GetTargetBranch())
 	// got a merge base and two up to date branches. We can grab the target diffs now
 	// we find files modified between the merge base and the HEAD of the target branch
 	targetModifiedFiles, err := repo.GetModifiedFileNamesBetweenCommits(commonSHA, mr.GetTargetBranch())
 	if err != nil {
 		return modifiedWSMap, t.handleError(err, fmt.Sprintf("could not find file diffs between %s and %s", commonSHA, mr.GetTargetBranch()))
 	}
-	log.Debug().Msgf("%+v files modified between merge base and target branch", targetModifiedFiles)
+	log.Debug().Msgf("%+v files modified between merge base and target branch (%s)", targetModifiedFiles, mr.GetTargetBranch())
 	// if there's no modified files we can assume it's safe to continue
 	if len(targetModifiedFiles) > 0 {
 		// use the same logic to find triggeredWorkspaces based on files modified between when the source branch was
@@ -281,7 +287,7 @@ func (t *TFCTrigger) getModifiedWorkspaceBetweenMergeBaseTargetBranch(mr vcs.MR,
 			return modifiedWSMap, t.handleError(err, "could not find modified workspaces for target branch")
 		}
 		for _, ws := range targetBranchWorkspaces {
-			modifiedWSMap[ws.Name] = 0
+			modifiedWSMap[ws.Name] = struct{}{}
 		}
 	}
 	return modifiedWSMap, err
@@ -333,8 +339,10 @@ func (t *TFCTrigger) TriggerTFCEvents() (*TriggeredTFCWorkspaces, error) {
 
 		modifiedWSMap, err := t.getModifiedWorkspaceBetweenMergeBaseTargetBranch(mr, repo)
 		if err != nil {
-			// this could just log and continue since the function will always return a valid lookup map
-			return nil, t.handleError(err, "could not identify modified workspaces on target branch")
+			err = t.postUpdate(":warning: Could not identify modified workspaces on target branch. Please review the plan carefully for unrelated changes.")
+			if err != nil {
+				log.Error().Err(err).Msg("could not update MR with message")
+			}
 		}
 		for _, cfgWS := range triggeredWorkspaces {
 			// check allow / deny lists