Skip to content

Commit

Permalink
feat: Add --max-comments-per-command configuration (runatlantis#3905)
Browse files Browse the repository at this point in the history
Co-authored-by: PePe Amengual <[email protected]>
Co-authored-by: Benoit Toulme <[email protected]>
  • Loading branch information
3 people authored and terakoya76 committed Dec 31, 2024
1 parent a646d30 commit 3203ee1
Show file tree
Hide file tree
Showing 16 changed files with 130 additions and 76 deletions.
13 changes: 11 additions & 2 deletions cmd/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -167,6 +168,7 @@ const (
DefaultGitlabHostname = "gitlab.com"
DefaultLockingDBType = "boltdb"
DefaultLogLevel = "info"
DefaultMaxCommentsPerCommand = 100
DefaultParallelPoolSize = 15
DefaultStatsNamespace = "atlantis"
DefaultPort = 4141
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down
1 change: 1 addition & 0 deletions cmd/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ var testFlags = map[string]interface{}{
LockingDBType: "boltdb",
LogLevelFlag: "debug",
MarkdownTemplateOverridesDirFlag: "/path2",
MaxCommentsPerCommand: 10,
StatsNamespace: "atlantis",
AllowDraftPRs: true,
PortFlag: 8181,
Expand Down
8 changes: 8 additions & 0 deletions runatlantis.io/docs/server-configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion server/controllers/github_app_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion server/events/vcs/azuredevops_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion server/events/vcs/bitbucketserver/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
53 changes: 32 additions & 21 deletions server/events/vcs/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
35 changes: 26 additions & 9 deletions server/events/vcs/common/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand All @@ -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])
Expand All @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion server/events/vcs/gh_app_creds_rotator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
30 changes: 18 additions & 12 deletions server/events/vcs/github_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions server/events/vcs/github_client_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit 3203ee1

Please sign in to comment.