Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add --max-comments-per-command configuration #3905

Merged
merged 19 commits into from
Jun 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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