feat: Add --max-comments-per-command configuration (runatlantis#3905)

Co-authored-by: PePe Amengual <[email protected]>
Co-authored-by: Benoit Toulme <[email protected]>

Author: 3 people

cmd/server.go

@@ -110,6 +110,7 @@ const (
 	LockingDBType                    = "locking-db-type"
 	LogLevelFlag                     = "log-level"
 	MarkdownTemplateOverridesDirFlag = "markdown-template-overrides-dir"
+	MaxCommentsPerCommand            = "max-comments-per-command"
 	ParallelPoolSize                 = "parallel-pool-size"
 	StatsNamespace                   = "stats-namespace"
 	AllowDraftPRs                    = "allow-draft-prs"
@@ -167,6 +168,7 @@ const (
 	DefaultGitlabHostname               = "gitlab.com"
 	DefaultLockingDBType                = "boltdb"
 	DefaultLogLevel                     = "info"
+	DefaultMaxCommentsPerCommand        = 100
 	DefaultParallelPoolSize             = 15
 	DefaultStatsNamespace               = "atlantis"
 	DefaultPort                         = 4141
@@ -593,6 +595,10 @@ var intFlags = map[string]intFlag{
 			" If merge base is further behind than this number of commits from any of branches heads, full fetch will be performed.",
 		defaultValue: DefaultCheckoutDepth,
 	},
+	MaxCommentsPerCommand: {
+		description:  "If non-zero, the maximum number of comments to split command output into before truncating.",
+		defaultValue: DefaultMaxCommentsPerCommand,
+	},
 	GiteaPageSizeFlag: {
 		description:  "Optional value that specifies the number of results per page to expect from Gitea.",
 		defaultValue: DefaultGiteaPageSize,
@@ -783,7 +789,7 @@ func (s *ServerCmd) run() error {
 	if err := s.Viper.Unmarshal(&userConfig); err != nil {
 		return err
 	}
-	s.setDefaults(&userConfig)
+	s.setDefaults(&userConfig, s.Viper)
 
 	// Now that we've parsed the config we can set our local logger to the
 	// right level.
@@ -824,7 +830,7 @@ func (s *ServerCmd) run() error {
 	return server.Start()
 }
 
-func (s *ServerCmd) setDefaults(c *server.UserConfig) {
+func (s *ServerCmd) setDefaults(c *server.UserConfig, v *viper.Viper) {
 	if c.AzureDevOpsHostname == "" {
 		c.AzureDevOpsHostname = DefaultADHostname
 	}
@@ -873,6 +879,9 @@ func (s *ServerCmd) setDefaults(c *server.UserConfig) {
 	if c.MarkdownTemplateOverridesDir == "" {
 		c.MarkdownTemplateOverridesDir = DefaultMarkdownTemplateOverridesDir
 	}
+	if !v.IsSet("max-comments-per-command") {
+		c.MaxCommentsPerCommand = DefaultMaxCommentsPerCommand
+	}
 	if c.ParallelPoolSize == 0 {
 		c.ParallelPoolSize = DefaultParallelPoolSize
 	}

cmd/server_test.go

@@ -109,6 +109,7 @@ var testFlags = map[string]interface{}{
 	LockingDBType:                    "boltdb",
 	LogLevelFlag:                     "debug",
 	MarkdownTemplateOverridesDirFlag: "/path2",
+	MaxCommentsPerCommand:            10,
 	StatsNamespace:                   "atlantis",
 	AllowDraftPRs:                    true,
 	PortFlag:                         8181,

runatlantis.io/docs/server-configuration.md

@@ -903,6 +903,14 @@ This is useful when you have many projects and want to keep the pull request cle
 
   Defaults to the atlantis home directory `/home/atlantis/.markdown_templates/` in `/$HOME/.markdown_templates`.
 
+### `--max-comments-per-command`
+  ```bash
+  atlantis server --max-comments-per-command=100
+  # or
+  ATLANTIS_MAX_COMMENTS_PER_COMMAND=100
+  ```
+  Limit the number of comments published after a command is executed, to prevent spamming your VCS and Atlantis to get throttled as a result. Defaults to `100`. Set this option to `0` to disable log truncation. Note that the truncation will happen on the top of the command output, to preserve the most important parts of the output, often displayed at the end.
+
 ### `--parallel-apply`
 
   ```bash

server/controllers/github_app_controller.go

@@ -56,7 +56,8 @@ func (g *GithubAppController) ExchangeCode(w http.ResponseWriter, r *http.Reques
 	g.Logger.Debug("Exchanging GitHub app code for app credentials")
 	creds := &vcs.GithubAnonymousCredentials{}
 	config := vcs.GithubConfig{}
-	client, err := vcs.NewGithubClient(g.GithubHostname, creds, config, g.Logger)
+	// This client does not post comments, so we don't need to configure it with maxCommentsPerCommand.
+	client, err := vcs.NewGithubClient(g.GithubHostname, creds, config, 0, g.Logger)
 	if err != nil {
 		g.respond(w, logging.Error, http.StatusInternalServerError, "Failed to exchange code for github app: %s", err)
 		return

server/events/vcs/azuredevops_client.go

@@ -107,7 +107,7 @@ func (g *AzureDevopsClient) CreateComment(logger logging.SimpleLogging, repo mod
 	// or tested limit in Azure DevOps.
 	const maxCommentLength = 150000
 
-	comments := common.SplitComment(comment, maxCommentLength, sepEnd, sepStart)
+	comments := common.SplitComment(comment, maxCommentLength, sepEnd, sepStart, 0, "")
 	owner, project, repoName := SplitAzureDevopsRepoFullName(repo.FullName)
 
 	for i := range comments {

server/events/vcs/bitbucketserver/client.go

@@ -137,7 +137,7 @@ func (b *Client) GetProjectKey(repoName string, cloneURL string) (string, error)
 func (b *Client) CreateComment(logger logging.SimpleLogging, repo models.Repo, pullNum int, comment string, _ string) error {
 	sepEnd := "\n```\n**Warning**: Output length greater than max comment size. Continued in next comment."
 	sepStart := "Continued from previous comment.\n```diff\n"
-	comments := common.SplitComment(comment, maxCommentLength, sepEnd, sepStart)
+	comments := common.SplitComment(comment, maxCommentLength, sepEnd, sepStart, 0, "")
 	for _, c := range comments {
 		if err := b.postComment(repo, pullNum, c); err != nil {
 			return err

server/events/vcs/common/common.go

@@ -12,34 +12,45 @@ func AutomergeCommitMsg(pullNum int) string {
 	return fmt.Sprintf("[Atlantis] Automatically merging after successful apply: PR #%d", pullNum)
 }
 
-// SplitComment splits comment into a slice of comments that are under maxSize.
-// It appends sepEnd to all comments that have a following comment.
-// It prepends sepStart to all comments that have a preceding comment.
-func SplitComment(comment string, maxSize int, sepEnd string, sepStart string) []string {
+/*
+SplitComment splits comment into a slice of comments that are under maxSize.
+- It appends sepEnd to all comments that have a following comment.
+- It prepends sepStart to all comments that have a preceding comment.
+- If maxCommentsPerCommand is non-zero, it never returns more than maxCommentsPerCommand
+comments, and it truncates the beginning of the comment to preserve the end of the comment string,
+which usually contains more important information, such as warnings, errors, and the plan summary.
+- SplitComment appends the truncationHeader to the first comment if it would have produced more comments.
+*/
+func SplitComment(comment string, maxSize int, sepEnd string, sepStart string, maxCommentsPerCommand int, truncationHeader string) []string {
 	if len(comment) <= maxSize {
 		return []string{comment}
 	}
 
-	maxWithSep := maxSize - len(sepEnd) - len(sepStart)
+	// No comment contains both sepEnd and truncationHeader, so we only have to count their max.
+	maxWithSep := maxSize - max(len(sepEnd), len(truncationHeader)) - len(sepStart)
 	var comments []string
-	numComments := int(math.Ceil(float64(len(comment)) / float64(maxWithSep)))
-	for i := 0; i < numComments; i++ {
-		upTo := min(len(comment), (i+1)*maxWithSep)
-		portion := comment[i*maxWithSep : upTo]
-		if i < numComments-1 {
-			portion += sepEnd
-		}
-		if i > 0 {
+	numPotentialComments := int(math.Ceil(float64(len(comment)) / float64(maxWithSep)))
+	var numComments int
+	if maxCommentsPerCommand == 0 {
+		numComments = numPotentialComments
+	} else {
+		numComments = min(numPotentialComments, maxCommentsPerCommand)
+	}
+	isTruncated := numComments < numPotentialComments
+	upTo := len(comment)
+	for len(comments) < numComments {
+		downFrom := max(0, upTo-maxWithSep)
+		portion := comment[downFrom:upTo]
+		if len(comments)+1 != numComments {
 			portion = sepStart + portion
+		} else if len(comments)+1 == numComments && isTruncated {
+			portion = truncationHeader + portion
+		}
+		if len(comments) != 0 {
+			portion = portion + sepEnd
 		}
-		comments = append(comments, portion)
+		comments = append([]string{portion}, comments...)
+		upTo = downFrom
 	}
 	return comments
 }
-
-func min(a, b int) int {
-	if a < b {
-		return a
-	}
-	return b
-}

server/events/vcs/common/common_test.go

@@ -24,7 +24,7 @@ import (
 // If under the maximum number of chars, we shouldn't split the comments.
 func TestSplitComment_UnderMax(t *testing.T) {
 	comment := "comment under max size"
-	split := common.SplitComment(comment, len(comment)+1, "sepEnd", "sepStart")
+	split := common.SplitComment(comment, len(comment)+1, "sepEnd", "sepStart", 0, "")
 	Equals(t, []string{comment}, split)
 }
 
@@ -34,11 +34,11 @@ func TestSplitComment_TwoComments(t *testing.T) {
 	comment := strings.Repeat("a", 1000)
 	sepEnd := "-sepEnd"
 	sepStart := "-sepStart"
-	split := common.SplitComment(comment, len(comment)-1, sepEnd, sepStart)
+	split := common.SplitComment(comment, len(comment)-1, sepEnd, sepStart, 0, "")
 
 	expCommentLen := len(comment) - len(sepEnd) - len(sepStart) - 1
-	expFirstComment := comment[:expCommentLen]
-	expSecondComment := comment[expCommentLen:]
+	expFirstComment := comment[:len(comment)-expCommentLen]
+	expSecondComment := comment[len(comment)-expCommentLen:]
 	Equals(t, 2, len(split))
 	Equals(t, expFirstComment+sepEnd, split[0])
 	Equals(t, sepStart+expSecondComment, split[1])
@@ -51,14 +51,31 @@ func TestSplitComment_FourComments(t *testing.T) {
 	sepEnd := "-sepEnd"
 	sepStart := "-sepStart"
 	max := (len(comment) / 4) + len(sepEnd) + len(sepStart)
-	split := common.SplitComment(comment, max, sepEnd, sepStart)
+	split := common.SplitComment(comment, max, sepEnd, sepStart, 0, "")
 
 	expMax := len(comment) / 4
 	Equals(t, []string{
-		comment[:expMax] + sepEnd,
-		sepStart + comment[expMax:expMax*2] + sepEnd,
-		sepStart + comment[expMax*2:expMax*3] + sepEnd,
-		sepStart + comment[expMax*3:]}, split)
+		comment[:len(comment)-expMax*3] + sepEnd,
+		sepStart + comment[len(comment)-expMax*3:len(comment)-expMax*2] + sepEnd,
+		sepStart + comment[len(comment)-expMax*2:len(comment)-expMax] + sepEnd,
+		sepStart + comment[len(comment)-expMax:]}, split)
+}
+
+func TestSplitComment_Limited(t *testing.T) {
+	comment := strings.Repeat("a", 1000)
+	sepEnd := "-sepEnd"
+	sepStart := "-sepStart"
+	truncationHeader := "truncated-"
+	max := (len(comment) / 8) + max(len(sepEnd), len(truncationHeader)) + len(sepStart)
+	split := common.SplitComment(comment, max, sepEnd, sepStart, 5, truncationHeader)
+
+	expMax := len(comment) / 8
+	Equals(t, []string{
+		truncationHeader + comment[len(comment)-expMax*5:len(comment)-expMax*4] + sepEnd,
+		sepStart + comment[len(comment)-expMax*4:len(comment)-expMax*3] + sepEnd,
+		sepStart + comment[len(comment)-expMax*3:len(comment)-expMax*2] + sepEnd,
+		sepStart + comment[len(comment)-expMax*2:len(comment)-expMax] + sepEnd,
+		sepStart + comment[len(comment)-expMax:]}, split)
 }
 
 func TestAutomergeCommitMsg(t *testing.T) {

server/events/vcs/gh_app_creds_rotator_test.go

@@ -20,7 +20,7 @@ func Test_githubAppTokenRotator_GenerateJob(t *testing.T) {
 	Ok(t, err)
 
 	anonCreds := &vcs.GithubAnonymousCredentials{}
-	anonClient, err := vcs.NewGithubClient(testServer, anonCreds, vcs.GithubConfig{}, logging.NewNoopLogger(t))
+	anonClient, err := vcs.NewGithubClient(testServer, anonCreds, vcs.GithubConfig{}, 0, logging.NewNoopLogger(t))
 	Ok(t, err)
 	tempSecrets, err := anonClient.ExchangeCode(logger, "good-code")
 	Ok(t, err)

server/events/vcs/github_client.go

@@ -41,11 +41,12 @@ var (
 
 // GithubClient is used to perform GitHub actions.
 type GithubClient struct {
-	user     string
-	client   *github.Client
-	v4Client *githubv4.Client
-	ctx      context.Context
-	config   GithubConfig
+	user                  string
+	client                *github.Client
+	v4Client              *githubv4.Client
+	ctx                   context.Context
+	config                GithubConfig
+	maxCommentsPerCommand int
 }
 
 // GithubAppTemporarySecrets holds app credentials obtained from github after creation.
@@ -76,7 +77,8 @@ type GithubPRReviewSummary struct {
 }
 
 // NewGithubClient returns a valid GitHub client.
-func NewGithubClient(hostname string, credentials GithubCredentials, config GithubConfig, logger logging.SimpleLogging) (*GithubClient, error) {
+
+func NewGithubClient(hostname string, credentials GithubCredentials, config GithubConfig, maxCommentsPerCommand int, logger logging.SimpleLogging) (*GithubClient, error) {
 	logger.Debug("Creating new GitHub client for host: %s", hostname)
 	transport, err := credentials.Client()
 	if err != nil {
@@ -108,11 +110,12 @@ func NewGithubClient(hostname string, credentials GithubCredentials, config Gith
 		return nil, errors.Wrap(err, "getting user")
 	}
 	return &GithubClient{
-		user:     user,
-		client:   client,
-		v4Client: v4Client,
-		ctx:      context.Background(),
-		config:   config,
+		user:                  user,
+		client:                client,
+		v4Client:              v4Client,
+		ctx:                   context.Background(),
+		config:                config,
+		maxCommentsPerCommand: maxCommentsPerCommand,
 	}, nil
 }
 
@@ -191,7 +194,10 @@ func (g *GithubClient) CreateComment(logger logging.SimpleLogging, repo models.R
 			"```diff\n"
 	}
 
-	comments := common.SplitComment(comment, maxCommentLength, sepEnd, sepStart)
+	truncationHeader := "\n```\n</details>" +
+		"\n<br>\n\n**Warning**: Command output is larger than the maximum number of comments per command. Output truncated.\n\n[..]\n"
+
+	comments := common.SplitComment(comment, maxCommentLength, sepEnd, sepStart, g.maxCommentsPerCommand, truncationHeader)
 	for i := range comments {
 		_, resp, err := g.client.Issues.CreateComment(g.ctx, repo.Owner, repo.Name, pullNum, &github.IssueComment{Body: &comments[i]})
 		if resp != nil {

server/events/vcs/github_client_internal_test.go

@@ -22,14 +22,14 @@ import (
 
 // If the hostname is github.com, should use normal BaseURL.
 func TestNewGithubClient_GithubCom(t *testing.T) {
-	client, err := NewGithubClient("github.com", &GithubUserCredentials{"user", "pass"}, GithubConfig{}, logging.NewNoopLogger(t))
+	client, err := NewGithubClient("github.com", &GithubUserCredentials{"user", "pass"}, GithubConfig{}, 0, logging.NewNoopLogger(t))
 	Ok(t, err)
 	Equals(t, "https://api.github.com/", client.client.BaseURL.String())
 }
 
 // If the hostname is a non-github hostname should use the right BaseURL.
 func TestNewGithubClient_NonGithub(t *testing.T) {
-	client, err := NewGithubClient("example.com", &GithubUserCredentials{"user", "pass"}, GithubConfig{}, logging.NewNoopLogger(t))
+	client, err := NewGithubClient("example.com", &GithubUserCredentials{"user", "pass"}, GithubConfig{}, 0, logging.NewNoopLogger(t))
 	Ok(t, err)
 	Equals(t, "https://example.com/api/v3/", client.client.BaseURL.String())
 	// If possible in the future, test the GraphQL client's URL as well. But at the