diff --git a/pkg/git/git_repo.go b/pkg/git/git_repo.go index d5184d4..5fee4bd 100644 --- a/pkg/git/git_repo.go +++ b/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 +} diff --git a/pkg/git/git_repo_test.go b/pkg/git/git_repo_test.go index 54f9fa0..c2510bd 100644 --- a/pkg/git/git_repo_test.go +++ b/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)) + }) +} diff --git a/pkg/github/client.go b/pkg/github/client.go index 1cfedb3..f6a90d7 100644 --- a/pkg/github/client.go +++ b/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, diff --git a/pkg/github/hooks/stream_worker.go b/pkg/github/hooks/stream_worker.go index bb12bf6..5772995 100644 --- a/pkg/github/hooks/stream_worker.go +++ b/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 diff --git a/pkg/gitlab/git_actions.go b/pkg/gitlab/git_actions.go index 610b826..b8448c4 100644 --- a/pkg/gitlab/git_actions.go +++ b/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, diff --git a/pkg/gitlab_hooks/comment_actions.go b/pkg/gitlab_hooks/comment_actions.go index a6fbc27..555f987 100644 --- a/pkg/gitlab_hooks/comment_actions.go +++ b/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 diff --git a/pkg/gitlab_hooks/comment_actions_test.go b/pkg/gitlab_hooks/comment_actions_test.go index 2105490..0f96f64 100644 --- a/pkg/gitlab_hooks/comment_actions_test.go +++ b/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") } } diff --git a/pkg/mocks/helpers.go b/pkg/mocks/helpers.go index 8e192e1..2451ccd 100644 --- a/pkg/mocks/helpers.go +++ b/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, diff --git a/pkg/tfc_trigger/tfc_trigger.go b/pkg/tfc_trigger/tfc_trigger.go index 803d3e2..7c07415 100644 --- a/pkg/tfc_trigger/tfc_trigger.go +++ b/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