From bf736f36d6f8117c13b3ffe4a23a4b59d853d643 Mon Sep 17 00:00:00 2001 From: Jenna Macdonald Date: Thu, 13 Jul 2023 13:14:43 +1000 Subject: [PATCH 1/3] Add in deletion of old comments, but keep the urls --- pkg/utils/formatter.go | 75 ++++++++++++++++++++++++++++++++++++++++ pkg/vcs/github/client.go | 64 +++++++++++++++++++++++++++++++++- pkg/vcs/gitlab/client.go | 54 +++++++++++++++++++++++++++++ 3 files changed, 192 insertions(+), 1 deletion(-) create mode 100644 pkg/utils/formatter.go diff --git a/pkg/utils/formatter.go b/pkg/utils/formatter.go new file mode 100644 index 0000000..df98631 --- /dev/null +++ b/pkg/utils/formatter.go @@ -0,0 +1,75 @@ +package utils + +import ( + "fmt" + "strings" +) + +// We format the URL as **RUN URL**:
under tfc_status_update.go +const ( + URL_RUN_PREFIX = "**Run URL**: " + URL_RUN_SUFFIX = "
" + URL_RUN_GROUP_PREFIX = "
Previous TFC URLS\n" + URL_RUN_GROUP_SUFFIX = "
" + URL_RUN_STATUS_PREFIX = "**Status**: " +) + +func CaptureSubstring(body string, prefix string, suffix string) string { + startIndex := strings.Index(body, prefix) + if startIndex == -1 { + return "" + } + + subBody := body[startIndex+len(prefix):] + endIndex := strings.Index(subBody, suffix) + if endIndex == -1 { + return "" + } + + return subBody[:endIndex] +} + +// AI generated these, may not be compresphenive +func FormatStatus(status string) string { + switch status { + case "applied": + return "✅ Applied" + case "planned": + return "📝 Planned" + case "policy_checked": + return "📝 Policy Checked" + case "policy_soft_failed": + return "📝 Policy Soft Failed" + case "errored": + return "❌ Errored" + case "canceled": + return "❌ Canceled" + case "discarded": + return "❌ Discarded" + case "planned_and_finished": + return "📝 Planned and Finished" + default: + // anything we can't match just preserve with the ticks + return fmt.Sprintf("`%s`", status) + } +} + +// Try and find the "Run URL:" string in the body and return it +// If we can't find it, return nothing +func CaptureRunURLFromBody(body string) string { + // First, see if we already have a grouping, which means this has run before + startIndex := strings.Index(body, URL_RUN_PREFIX) + if startIndex == -1 { + // Can't find a URL to pull out, so we'll just return + return "" + } + + // At this point we've found a starting index, and se tthe appropriate suffix, now to try and pull a URL out + subBody := body[startIndex+len(URL_RUN_PREFIX):] + endIndex := strings.Index(subBody, URL_RUN_SUFFIX) + if endIndex == -1 { + return "" // Couldn't determine where to cut + } + + return subBody[:endIndex] +} diff --git a/pkg/vcs/github/client.go b/pkg/vcs/github/client.go index fdeadcd..b4246d4 100644 --- a/pkg/vcs/github/client.go +++ b/pkg/vcs/github/client.go @@ -15,6 +15,7 @@ import ( githttp "github.com/go-git/go-git/v5/plumbing/transport/http" gogithub "github.com/google/go-github/v49/github" "github.com/rs/zerolog/log" + "github.com/spf13/viper" zgit "github.com/zapier/tfbuddy/pkg/git" "github.com/zapier/tfbuddy/pkg/utils" "github.com/zapier/tfbuddy/pkg/vcs" @@ -61,8 +62,69 @@ func (c *Client) GetMergeRequestApprovals(id int, project string) (vcs.MRApprove return pr, nil } +// Go over all comments on a PR, trying to grab any old TFC run urls and deleting the bodies +func (c *Client) pruneComments(prID int, fullName string) (string, error) { + projectParts, err := splitFullName(fullName) + if err != nil { + return "", utils.CreatePermanentError(err) + } + comments, _, err := c.client.Issues.ListComments(c.ctx, projectParts[0], projectParts[1], prID, &gogithub.IssueListCommentsOptions{}) + if err != nil { + return "", utils.CreatePermanentError(err) + } + + currentUser, _, err := c.client.Users.Get(context.Background(), "") + if err != nil { + return "", utils.CreatePermanentError(err) + } + + var oldRunUrls []string + var oldRunBlock string + for _, comment := range comments { + // If this token user made the comment, and we're making a new comment, pick the TFC url out of the body and delete the comment + if comment.GetUser().GetLogin() == currentUser.GetLogin() { + runUrl := utils.CaptureSubstring(comment.GetBody(), utils.URL_RUN_PREFIX, utils.URL_RUN_SUFFIX) + runStatus := utils.CaptureSubstring(comment.GetBody(), utils.URL_RUN_STATUS_PREFIX, utils.URL_RUN_SUFFIX) + if runUrl != "" && runStatus != "" { + // Example: - ✅ Applied + oldRunUrls = append(oldRunUrls, fmt.Sprintf("%s - %s", runUrl, runStatus)) + } + + // Github orders comments from earliest -> latest via ID, so we check each comment and take the last match on an "old url" block + oldRunBlockTest := utils.CaptureSubstring(comment.GetBody(), utils.URL_RUN_GROUP_PREFIX, utils.URL_RUN_GROUP_SUFFIX) + if oldRunBlockTest != "" { + oldRunBlock = oldRunBlockTest + } + + log.Debug().Msgf("Deleting comment %d", comment.GetID()) + _, err := c.client.Issues.DeleteComment(c.ctx, projectParts[0], projectParts[1], comment.GetID()) + if err != nil { + return "", utils.CreatePermanentError(err) + } + } + } + + // If we found any old run urls, return them formatted + if len(oldRunUrls) > 0 { + // Try and find any exisitng groupings of old urls, else make a new one + return fmt.Sprintf("%s\n%s\n%s\n%s", utils.URL_RUN_GROUP_PREFIX, oldRunBlock, strings.Join(oldRunUrls, "\n"), utils.URL_RUN_GROUP_SUFFIX), nil + } + return oldRunBlock, nil +} + func (c *Client) CreateMergeRequestComment(prID int, fullName string, comment string) error { - _, err := c.PostIssueComment(prID, fullName, comment) + var oldUrls string + var err error + if viper.GetString("prune-comments") != "" { + oldUrls, err = c.pruneComments(prID, fullName) + if err != nil { + return err + } + if oldUrls != "" { + comment = fmt.Sprintf("%s\n%s", oldUrls, comment) + } + } + _, err = c.PostIssueComment(prID, fullName, comment) return err } diff --git a/pkg/vcs/gitlab/client.go b/pkg/vcs/gitlab/client.go index 0363df2..43dcd4e 100644 --- a/pkg/vcs/gitlab/client.go +++ b/pkg/vcs/gitlab/client.go @@ -5,10 +5,12 @@ import ( "fmt" "net/url" "os" + "strings" "time" "github.com/cenkalti/backoff/v4" "github.com/rs/zerolog/log" + "github.com/spf13/viper" "github.com/zapier/tfbuddy/pkg/utils" "github.com/zapier/tfbuddy/pkg/vcs" @@ -109,9 +111,61 @@ func (c *GitlabClient) GetCommitStatuses(projectID, commitSHA string) []*gogitla return statuses } +// Crawl the comments on this MR for tfbuddy comments, grab any TFC urls out of them, and delete them. +func (c *GitlabClient) pruneNotes(mrIID int, projectID string) (string, error) { + notes, _, err := c.client.Notes.ListMergeRequestNotes(projectID, mrIID, &gogitlab.ListMergeRequestNotesOptions{}) + if err != nil { + return "", utils.CreatePermanentError(err) + } + + var oldRunUrls []string + var oldRunBlock string + for _, note := range notes { + if note.Author.Username == c.tokenUser { + runUrl := utils.CaptureSubstring(note.Body, utils.URL_RUN_PREFIX, utils.URL_RUN_SUFFIX) + runStatus := utils.CaptureSubstring(note.Body, utils.URL_RUN_STATUS_PREFIX, utils.URL_RUN_SUFFIX) + if runUrl != "" && runStatus != "" { + oldRunUrls = append(oldRunUrls, fmt.Sprintf("%s - %s", runUrl, runStatus)) + } + + // Gitlab default sort is order by created by, so take the last match on this + oldRunBlockTest := utils.CaptureSubstring(note.Body, utils.URL_RUN_GROUP_PREFIX, utils.URL_RUN_GROUP_SUFFIX) + if oldRunBlockTest != "" { + oldRunBlock = oldRunBlockTest + } + + log.Debug().Str("projectID", projectID).Int("mrIID", mrIID).Msgf("deleting note %d", note.ID) + _, err := c.client.Notes.DeleteMergeRequestNote(projectID, mrIID, note.ID) + if err != nil { + return "", utils.CreatePermanentError(err) + } + } + } + + // Add new urls into block + if len(oldRunUrls) > 0 { + return fmt.Sprintf("%s\n%s\n%s\n%s", utils.URL_RUN_GROUP_PREFIX, oldRunBlock, strings.Join(oldRunUrls, "\n"), utils.URL_RUN_GROUP_SUFFIX), nil + } + return oldRunBlock, nil +} + // CreateMergeRequestComment creates a comment on the merge request. func (c *GitlabClient) CreateMergeRequestComment(mrIID int, projectID, comment string) error { + var oldUrls string + var err error + if comment != "" { + // Delete old comments, but save the URLs for further evaluation + if viper.GetString("prune-comments") != "" { + oldUrls, err = c.pruneNotes(mrIID, projectID) + if err != nil { + return err + } + if oldUrls != "" { + comment = fmt.Sprintf("%s\n%s", oldUrls, comment) + } + } + return backoff.Retry(func() error { log.Debug().Str("projectID", projectID).Int("mrIID", mrIID).Msg("posting Gitlab comment") _, _, err := c.client.Notes.CreateMergeRequestNote(projectID, mrIID, &gogitlab.CreateMergeRequestNoteOptions{Body: gogitlab.String(comment)}) From 85753c441aaead55e0a50a578e0b0ac3ce015a96 Mon Sep 17 00:00:00 2001 From: Jenna Macdonald Date: Thu, 13 Jul 2023 15:19:18 +1000 Subject: [PATCH 2/3] Better, now to refactor more but this should do it --- pkg/comment_formatter/tfc_status_update.go | 4 +++ pkg/mocks/mock_vcs.go | 3 ++ pkg/utils/formatter.go | 33 ++++++----------- pkg/vcs/github/client.go | 27 +++++--------- pkg/vcs/gitlab/client.go | 42 ++++++++++------------ pkg/vcs/gitlab/mr_comment_updater.go | 13 +++++++ pkg/vcs/interfaces.go | 1 + 7 files changed, 58 insertions(+), 65 deletions(-) diff --git a/pkg/comment_formatter/tfc_status_update.go b/pkg/comment_formatter/tfc_status_update.go index 82cc9f5..f62cec2 100644 --- a/pkg/comment_formatter/tfc_status_update.go +++ b/pkg/comment_formatter/tfc_status_update.go @@ -169,3 +169,7 @@ var needToApplyFullWorkSpace = ` **Need to Apply Full Workspace Before Merging** ` + +const OLD_RUN_BLOCK = ` +### Previous TFC URLS +%s` diff --git a/pkg/mocks/mock_vcs.go b/pkg/mocks/mock_vcs.go index e9678f5..6b97303 100644 --- a/pkg/mocks/mock_vcs.go +++ b/pkg/mocks/mock_vcs.go @@ -58,6 +58,9 @@ func (m *MockGitClient) CloneMergeRequest(arg0 string, arg1 vcs.MR, arg2 string) return ret0, ret1 } +func (m *MockGitClient) GetOldRunUrls(mrIID int, project string, rootNoteID int, deleteNotes bool) (string, error) { + return "", nil +} // CloneMergeRequest indicates an expected call of CloneMergeRequest. func (mr *MockGitClientMockRecorder) CloneMergeRequest(arg0, arg1, arg2 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() diff --git a/pkg/utils/formatter.go b/pkg/utils/formatter.go index df98631..6963470 100644 --- a/pkg/utils/formatter.go +++ b/pkg/utils/formatter.go @@ -2,14 +2,19 @@ package utils import ( "fmt" + "log" "strings" ) // We format the URL as **RUN URL**:
under tfc_status_update.go const ( - URL_RUN_PREFIX = "**Run URL**: " - URL_RUN_SUFFIX = "
" - URL_RUN_GROUP_PREFIX = "
Previous TFC URLS\n" + URL_RUN_PREFIX = "**Run URL**: " + URL_RUN_SUFFIX = "
" + URL_RUN_GROUP_PREFIX = `
+ +### Previous TFC Urls: + +` URL_RUN_GROUP_SUFFIX = "
" URL_RUN_STATUS_PREFIX = "**Status**: " ) @@ -31,6 +36,8 @@ func CaptureSubstring(body string, prefix string, suffix string) string { // AI generated these, may not be compresphenive func FormatStatus(status string) string { + status = strings.Trim(status, "`") + log.Printf("Status: %s", status) switch status { case "applied": return "✅ Applied" @@ -53,23 +60,3 @@ func FormatStatus(status string) string { return fmt.Sprintf("`%s`", status) } } - -// Try and find the "Run URL:" string in the body and return it -// If we can't find it, return nothing -func CaptureRunURLFromBody(body string) string { - // First, see if we already have a grouping, which means this has run before - startIndex := strings.Index(body, URL_RUN_PREFIX) - if startIndex == -1 { - // Can't find a URL to pull out, so we'll just return - return "" - } - - // At this point we've found a starting index, and se tthe appropriate suffix, now to try and pull a URL out - subBody := body[startIndex+len(URL_RUN_PREFIX):] - endIndex := strings.Index(subBody, URL_RUN_SUFFIX) - if endIndex == -1 { - return "" // Couldn't determine where to cut - } - - return subBody[:endIndex] -} diff --git a/pkg/vcs/github/client.go b/pkg/vcs/github/client.go index b4246d4..d055f52 100644 --- a/pkg/vcs/github/client.go +++ b/pkg/vcs/github/client.go @@ -15,7 +15,6 @@ import ( githttp "github.com/go-git/go-git/v5/plumbing/transport/http" gogithub "github.com/google/go-github/v49/github" "github.com/rs/zerolog/log" - "github.com/spf13/viper" zgit "github.com/zapier/tfbuddy/pkg/git" "github.com/zapier/tfbuddy/pkg/utils" "github.com/zapier/tfbuddy/pkg/vcs" @@ -63,7 +62,8 @@ func (c *Client) GetMergeRequestApprovals(id int, project string) (vcs.MRApprove } // Go over all comments on a PR, trying to grab any old TFC run urls and deleting the bodies -func (c *Client) pruneComments(prID int, fullName string) (string, error) { +func (c *Client) GetOldRunUrls(prID int, fullName string, rootCommentID int, deleteComment bool) (string, error) { + log.Debug().Msg("pruneComments") projectParts, err := splitFullName(fullName) if err != nil { return "", utils.CreatePermanentError(err) @@ -96,10 +96,12 @@ func (c *Client) pruneComments(prID int, fullName string) (string, error) { oldRunBlock = oldRunBlockTest } - log.Debug().Msgf("Deleting comment %d", comment.GetID()) - _, err := c.client.Issues.DeleteComment(c.ctx, projectParts[0], projectParts[1], comment.GetID()) - if err != nil { - return "", utils.CreatePermanentError(err) + if deleteComment && comment.GetID() != int64(rootCommentID) { + log.Debug().Msgf("Deleting comment %d", comment.GetID()) + _, err := c.client.Issues.DeleteComment(c.ctx, projectParts[0], projectParts[1], comment.GetID()) + if err != nil { + return "", utils.CreatePermanentError(err) + } } } } @@ -113,18 +115,7 @@ func (c *Client) pruneComments(prID int, fullName string) (string, error) { } func (c *Client) CreateMergeRequestComment(prID int, fullName string, comment string) error { - var oldUrls string - var err error - if viper.GetString("prune-comments") != "" { - oldUrls, err = c.pruneComments(prID, fullName) - if err != nil { - return err - } - if oldUrls != "" { - comment = fmt.Sprintf("%s\n%s", oldUrls, comment) - } - } - _, err = c.PostIssueComment(prID, fullName, comment) + _, err := c.PostIssueComment(prID, fullName, comment) return err } diff --git a/pkg/vcs/gitlab/client.go b/pkg/vcs/gitlab/client.go index 43dcd4e..64eda75 100644 --- a/pkg/vcs/gitlab/client.go +++ b/pkg/vcs/gitlab/client.go @@ -10,7 +10,6 @@ import ( "github.com/cenkalti/backoff/v4" "github.com/rs/zerolog/log" - "github.com/spf13/viper" "github.com/zapier/tfbuddy/pkg/utils" "github.com/zapier/tfbuddy/pkg/vcs" @@ -112,8 +111,15 @@ func (c *GitlabClient) GetCommitStatuses(projectID, commitSHA string) []*gogitla } // Crawl the comments on this MR for tfbuddy comments, grab any TFC urls out of them, and delete them. -func (c *GitlabClient) pruneNotes(mrIID int, projectID string) (string, error) { - notes, _, err := c.client.Notes.ListMergeRequestNotes(projectID, mrIID, &gogitlab.ListMergeRequestNotesOptions{}) +func (c *GitlabClient) GetOldRunUrls(mrIID int, project string, rootNoteID int, deleteNotes bool) (string, error) { + log.Debug().Str("projectID", project).Int("mrIID", mrIID).Msg("pruning notes") + notes, _, err := c.client.Notes.ListMergeRequestNotes(project, mrIID, &gogitlab.ListMergeRequestNotesOptions{}) + if err != nil { + return "", utils.CreatePermanentError(err) + } + + currentUser, _, err := c.client.Users.CurrentUser() + if err != nil { return "", utils.CreatePermanentError(err) } @@ -121,11 +127,11 @@ func (c *GitlabClient) pruneNotes(mrIID int, projectID string) (string, error) { var oldRunUrls []string var oldRunBlock string for _, note := range notes { - if note.Author.Username == c.tokenUser { + if note.Author.Username == currentUser.Username { runUrl := utils.CaptureSubstring(note.Body, utils.URL_RUN_PREFIX, utils.URL_RUN_SUFFIX) runStatus := utils.CaptureSubstring(note.Body, utils.URL_RUN_STATUS_PREFIX, utils.URL_RUN_SUFFIX) if runUrl != "" && runStatus != "" { - oldRunUrls = append(oldRunUrls, fmt.Sprintf("%s - %s", runUrl, runStatus)) + oldRunUrls = append(oldRunUrls, fmt.Sprintf("%s - %s", runUrl, utils.FormatStatus(runStatus))) } // Gitlab default sort is order by created by, so take the last match on this @@ -133,11 +139,12 @@ func (c *GitlabClient) pruneNotes(mrIID int, projectID string) (string, error) { if oldRunBlockTest != "" { oldRunBlock = oldRunBlockTest } - - log.Debug().Str("projectID", projectID).Int("mrIID", mrIID).Msgf("deleting note %d", note.ID) - _, err := c.client.Notes.DeleteMergeRequestNote(projectID, mrIID, note.ID) - if err != nil { - return "", utils.CreatePermanentError(err) + if deleteNotes && note.ID != rootNoteID { + log.Debug().Str("projectID", project).Int("mrIID", mrIID).Msgf("deleting note %d", note.ID) + _, err := c.client.Notes.DeleteMergeRequestNote(project, mrIID, note.ID) + if err != nil { + return "", utils.CreatePermanentError(err) + } } } } @@ -151,21 +158,7 @@ func (c *GitlabClient) pruneNotes(mrIID int, projectID string) (string, error) { // CreateMergeRequestComment creates a comment on the merge request. func (c *GitlabClient) CreateMergeRequestComment(mrIID int, projectID, comment string) error { - var oldUrls string - var err error - if comment != "" { - // Delete old comments, but save the URLs for further evaluation - if viper.GetString("prune-comments") != "" { - oldUrls, err = c.pruneNotes(mrIID, projectID) - if err != nil { - return err - } - if oldUrls != "" { - comment = fmt.Sprintf("%s\n%s", oldUrls, comment) - } - } - return backoff.Retry(func() error { log.Debug().Str("projectID", projectID).Int("mrIID", mrIID).Msg("posting Gitlab comment") _, _, err := c.client.Notes.CreateMergeRequestNote(projectID, mrIID, &gogitlab.CreateMergeRequestNoteOptions{Body: gogitlab.String(comment)}) @@ -202,6 +195,7 @@ func (c *GitlabClient) CreateMergeRequestDiscussion(mrIID int, project, comment if comment == "" { return nil, errors.New("comment is empty") } + return backoff.RetryWithData(func() (vcs.MRDiscussionNotes, error) { log.Debug().Str("project", project).Int("mrIID", mrIID).Msg("create Gitlab discussion") dis, _, err := c.client.Discussions.CreateMergeRequestDiscussion(project, mrIID, &gogitlab.CreateMergeRequestDiscussionOptions{ diff --git a/pkg/vcs/gitlab/mr_comment_updater.go b/pkg/vcs/gitlab/mr_comment_updater.go index fc69404..cd80764 100644 --- a/pkg/vcs/gitlab/mr_comment_updater.go +++ b/pkg/vcs/gitlab/mr_comment_updater.go @@ -15,6 +15,19 @@ func (p *RunStatusUpdater) postRunStatusComment(run *tfe.Run, rmd runstream.RunM commentBody, topLevelNoteBody, resolveDiscussion := comment_formatter.FormatRunStatusCommentBody(p.tfc, run, rmd) + var oldUrls string + var err error + // TODO: Make this configurable behaviour + if run.Status == tfe.RunErrored || run.Status == tfe.RunCanceled || run.Status == tfe.RunDiscarded || run.Status == tfe.RunPlannedAndFinished { + oldUrls, err = p.client.GetOldRunUrls(rmd.GetMRInternalID(), rmd.GetMRProjectNameWithNamespace(), int(rmd.GetRootNoteID()), true) + if err != nil { + log.Error().Err(err).Msg("could not retrieve old run urls") + } + if oldUrls != "" { + topLevelNoteBody = fmt.Sprintf("%s\n%s", oldUrls, topLevelNoteBody) + } + } + //oldURLBlock := utils.CaptureSubstring(, prefix string, suffix string) if _, err := p.client.UpdateMergeRequestDiscussionNote( rmd.GetMRInternalID(), int(rmd.GetRootNoteID()), diff --git a/pkg/vcs/interfaces.go b/pkg/vcs/interfaces.go index b39e89c..a4b9d5e 100644 --- a/pkg/vcs/interfaces.go +++ b/pkg/vcs/interfaces.go @@ -15,6 +15,7 @@ type GitClient interface { AddMergeRequestDiscussionReply(mrIID int, project, discussionID, comment string) (MRNote, error) SetCommitStatus(projectWithNS string, commitSHA string, status CommitStatusOptions) (CommitStatus, error) GetPipelinesForCommit(projectWithNS string, commitSHA string) ([]ProjectPipeline, error) + GetOldRunUrls(mrIID int, project string, rootCommentID int, deleteNotes bool) (string, error) } type GitRepo interface { FetchUpstreamBranch(string) error From 0f1c03385610f12bcf9bd7ae9dcb0ce0cbd52f8e Mon Sep 17 00:00:00 2001 From: Jenna Macdonald Date: Wed, 19 Jul 2023 14:49:45 +1000 Subject: [PATCH 3/3] Put deleting behind env var, cleanup func sig --- pkg/mocks/mock_vcs.go | 2 +- pkg/vcs/github/client.go | 4 ++-- pkg/vcs/gitlab/client.go | 4 ++-- pkg/vcs/gitlab/mr_comment_updater.go | 4 ++-- pkg/vcs/interfaces.go | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/mocks/mock_vcs.go b/pkg/mocks/mock_vcs.go index 6b97303..ee502b1 100644 --- a/pkg/mocks/mock_vcs.go +++ b/pkg/mocks/mock_vcs.go @@ -58,7 +58,7 @@ func (m *MockGitClient) CloneMergeRequest(arg0 string, arg1 vcs.MR, arg2 string) return ret0, ret1 } -func (m *MockGitClient) GetOldRunUrls(mrIID int, project string, rootNoteID int, deleteNotes bool) (string, error) { +func (m *MockGitClient) GetOldRunUrls(mrIID int, project string, rootNoteID int) (string, error) { return "", nil } // CloneMergeRequest indicates an expected call of CloneMergeRequest. diff --git a/pkg/vcs/github/client.go b/pkg/vcs/github/client.go index d055f52..5b47293 100644 --- a/pkg/vcs/github/client.go +++ b/pkg/vcs/github/client.go @@ -62,7 +62,7 @@ func (c *Client) GetMergeRequestApprovals(id int, project string) (vcs.MRApprove } // Go over all comments on a PR, trying to grab any old TFC run urls and deleting the bodies -func (c *Client) GetOldRunUrls(prID int, fullName string, rootCommentID int, deleteComment bool) (string, error) { +func (c *Client) GetOldRunUrls(prID int, fullName string, rootCommentID int) (string, error) { log.Debug().Msg("pruneComments") projectParts, err := splitFullName(fullName) if err != nil { @@ -96,7 +96,7 @@ func (c *Client) GetOldRunUrls(prID int, fullName string, rootCommentID int, del oldRunBlock = oldRunBlockTest } - if deleteComment && comment.GetID() != int64(rootCommentID) { + if os.Getenv("TFBUDDY_DELETE_OLD_COMMENTS") != "" && comment.GetID() != int64(rootCommentID) { log.Debug().Msgf("Deleting comment %d", comment.GetID()) _, err := c.client.Issues.DeleteComment(c.ctx, projectParts[0], projectParts[1], comment.GetID()) if err != nil { diff --git a/pkg/vcs/gitlab/client.go b/pkg/vcs/gitlab/client.go index 64eda75..bb5f1f7 100644 --- a/pkg/vcs/gitlab/client.go +++ b/pkg/vcs/gitlab/client.go @@ -111,7 +111,7 @@ func (c *GitlabClient) GetCommitStatuses(projectID, commitSHA string) []*gogitla } // Crawl the comments on this MR for tfbuddy comments, grab any TFC urls out of them, and delete them. -func (c *GitlabClient) GetOldRunUrls(mrIID int, project string, rootNoteID int, deleteNotes bool) (string, error) { +func (c *GitlabClient) GetOldRunUrls(mrIID int, project string, rootNoteID int) (string, error) { log.Debug().Str("projectID", project).Int("mrIID", mrIID).Msg("pruning notes") notes, _, err := c.client.Notes.ListMergeRequestNotes(project, mrIID, &gogitlab.ListMergeRequestNotesOptions{}) if err != nil { @@ -139,7 +139,7 @@ func (c *GitlabClient) GetOldRunUrls(mrIID int, project string, rootNoteID int, if oldRunBlockTest != "" { oldRunBlock = oldRunBlockTest } - if deleteNotes && note.ID != rootNoteID { + if os.Getenv("TFBUDDY_DELETE_OLD_COMMENTS") != "" && note.ID != rootNoteID { log.Debug().Str("projectID", project).Int("mrIID", mrIID).Msgf("deleting note %d", note.ID) _, err := c.client.Notes.DeleteMergeRequestNote(project, mrIID, note.ID) if err != nil { diff --git a/pkg/vcs/gitlab/mr_comment_updater.go b/pkg/vcs/gitlab/mr_comment_updater.go index cd80764..f731bc0 100644 --- a/pkg/vcs/gitlab/mr_comment_updater.go +++ b/pkg/vcs/gitlab/mr_comment_updater.go @@ -17,9 +17,9 @@ func (p *RunStatusUpdater) postRunStatusComment(run *tfe.Run, rmd runstream.RunM var oldUrls string var err error - // TODO: Make this configurable behaviour + if run.Status == tfe.RunErrored || run.Status == tfe.RunCanceled || run.Status == tfe.RunDiscarded || run.Status == tfe.RunPlannedAndFinished { - oldUrls, err = p.client.GetOldRunUrls(rmd.GetMRInternalID(), rmd.GetMRProjectNameWithNamespace(), int(rmd.GetRootNoteID()), true) + oldUrls, err = p.client.GetOldRunUrls(rmd.GetMRInternalID(), rmd.GetMRProjectNameWithNamespace(), int(rmd.GetRootNoteID())) if err != nil { log.Error().Err(err).Msg("could not retrieve old run urls") } diff --git a/pkg/vcs/interfaces.go b/pkg/vcs/interfaces.go index a4b9d5e..975955c 100644 --- a/pkg/vcs/interfaces.go +++ b/pkg/vcs/interfaces.go @@ -15,7 +15,7 @@ type GitClient interface { AddMergeRequestDiscussionReply(mrIID int, project, discussionID, comment string) (MRNote, error) SetCommitStatus(projectWithNS string, commitSHA string, status CommitStatusOptions) (CommitStatus, error) GetPipelinesForCommit(projectWithNS string, commitSHA string) ([]ProjectPipeline, error) - GetOldRunUrls(mrIID int, project string, rootCommentID int, deleteNotes bool) (string, error) + GetOldRunUrls(mrIID int, project string, rootCommentID int) (string, error) } type GitRepo interface { FetchUpstreamBranch(string) error