From bc545844d3d3ff46e24b50b8c08dc293bad05e32 Mon Sep 17 00:00:00 2001 From: CJ Ketchum Date: Thu, 26 Mar 2020 09:23:47 -0700 Subject: [PATCH] Update autoplan so commit status is set only after finding projects (resolves #954) --- cmd/server.go | 5 +++++ cmd/server_test.go | 1 + server/events/command_runner.go | 25 ++++++++++++++++--------- server/server.go | 1 + server/user_config.go | 29 ++++++++++++++++------------- 5 files changed, 39 insertions(+), 22 deletions(-) diff --git a/cmd/server.go b/cmd/server.go index 5d97e56263..4e2ae6d28c 100644 --- a/cmd/server.go +++ b/cmd/server.go @@ -69,6 +69,7 @@ const ( RequireApprovalFlag = "require-approval" RequireMergeableFlag = "require-mergeable" SilenceForkPRErrorsFlag = "silence-fork-pr-errors" + SilenceVCSStatusNoPlans = "silence-vcs-status-no-plans" SilenceWhitelistErrorsFlag = "silence-whitelist-errors" SlackTokenFlag = "slack-token" SSLCertFileFlag = "ssl-cert-file" @@ -264,6 +265,10 @@ var boolFlags = map[string]boolFlag{ description: "Silences the posting of fork pull requests not allowed error comments.", defaultValue: false, }, + SilenceVCSStatusNoPlans: { + description: "Silences VCS commit status when autoplan finds no projects to plan.", + defaultValue: false, + }, SilenceWhitelistErrorsFlag: { description: "Silences the posting of whitelist error comments.", defaultValue: false, diff --git a/cmd/server_test.go b/cmd/server_test.go index b41a682c4d..9286659073 100644 --- a/cmd/server_test.go +++ b/cmd/server_test.go @@ -80,6 +80,7 @@ var testFlags = map[string]interface{}{ RequireMergeableFlag: true, SilenceForkPRErrorsFlag: true, SilenceWhitelistErrorsFlag: true, + SilenceVCSStatusNoPlans: true, SlackTokenFlag: "slack-token", SSLCertFileFlag: "cert-file", SSLKeyFileFlag: "key-file", diff --git a/server/events/command_runner.go b/server/events/command_runner.go index 9f63690ab9..11ec0a979b 100644 --- a/server/events/command_runner.go +++ b/server/events/command_runner.go @@ -85,6 +85,9 @@ type DefaultCommandRunner struct { // this in our error message back to the user on a forked PR so they know // how to disable error comment SilenceForkPRErrorsFlag string + // SilenceVCSStatusNoPlans is whether autoplan should set commit status if no plans + // are found + SilenceVCSStatusNoPlans bool ProjectCommandBuilder ProjectCommandBuilder ProjectCommandRunner ProjectCommandRunner // GlobalAutomerge is true if we should automatically merge pull requests if all @@ -110,10 +113,6 @@ func (c *DefaultCommandRunner) RunAutoplanCommand(baseRepo models.Repo, headRepo return } - if err := c.CommitStatusUpdater.UpdateCombined(ctx.BaseRepo, ctx.Pull, models.PendingCommitStatus, models.PlanCommand); err != nil { - ctx.Log.Warn("unable to update commit status: %s", err) - } - projectCmds, err := c.ProjectCommandBuilder.BuildAutoplanCommands(ctx) if err != nil { if statusErr := c.CommitStatusUpdater.UpdateCombined(ctx.BaseRepo, ctx.Pull, models.FailedCommitStatus, models.PlanCommand); statusErr != nil { @@ -125,15 +124,23 @@ func (c *DefaultCommandRunner) RunAutoplanCommand(baseRepo models.Repo, headRepo } if len(projectCmds) == 0 { log.Info("determined there was no project to run plan in") - // If there were no projects modified, we set a successful commit status - // with 0/0 projects planned successfully because we've already set an - // in-progress status and we don't want that to be "in progress" forever. - if err := c.CommitStatusUpdater.UpdateCombinedCount(baseRepo, pull, models.SuccessCommitStatus, models.PlanCommand, 0, 0); err != nil { - ctx.Log.Warn("unable to update commit status: %s", err) + if !c.SilenceVCSStatusNoPlans { + // If there were no projects modified, we set a successful commit status + // with 0/0 projects planned successfully because some users require + // the Atlantis status to be passing for all pull requests. + ctx.Log.Debug("setting VCS status to success with no projects found") + if err := c.CommitStatusUpdater.UpdateCombinedCount(baseRepo, pull, models.SuccessCommitStatus, models.PlanCommand, 0, 0); err != nil { + ctx.Log.Warn("unable to update commit status: %s", err) + } } return } + // At this point we are sure Atlantis has work to do, so set commit status to pending + if err := c.CommitStatusUpdater.UpdateCombined(ctx.BaseRepo, ctx.Pull, models.PendingCommitStatus, models.PlanCommand); err != nil { + ctx.Log.Warn("unable to update commit status: %s", err) + } + result := c.runProjectCmds(projectCmds, models.PlanCommand) if c.automergeEnabled(ctx, projectCmds) && result.HasErrors() { ctx.Log.Info("deleting plans because there were errors and automerge requires all plans succeed") diff --git a/server/server.go b/server/server.go index 7e49c55399..82fc58c2e0 100644 --- a/server/server.go +++ b/server/server.go @@ -316,6 +316,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { AllowForkPRsFlag: config.AllowForkPRsFlag, SilenceForkPRErrors: userConfig.SilenceForkPRErrors, SilenceForkPRErrorsFlag: config.SilenceForkPRErrorsFlag, + SilenceVCSStatusNoPlans: userConfig.SilenceVCSStatusNoPlans, DisableApplyAll: userConfig.DisableApplyAll, ProjectCommandBuilder: &events.DefaultProjectCommandBuilder{ ParserValidator: validator, diff --git a/server/user_config.go b/server/user_config.go index 439311546a..cbcb8db5ae 100644 --- a/server/user_config.go +++ b/server/user_config.go @@ -41,19 +41,22 @@ type UserConfig struct { RequireApproval bool `mapstructure:"require-approval"` // RequireMergeable is whether to require pull requests to be mergeable before // allowing terraform apply's to run. - RequireMergeable bool `mapstructure:"require-mergeable"` - SilenceForkPRErrors bool `mapstructure:"silence-fork-pr-errors"` - SilenceWhitelistErrors bool `mapstructure:"silence-whitelist-errors"` - SlackToken string `mapstructure:"slack-token"` - SSLCertFile string `mapstructure:"ssl-cert-file"` - SSLKeyFile string `mapstructure:"ssl-key-file"` - TFDownloadURL string `mapstructure:"tf-download-url"` - TFEHostname string `mapstructure:"tfe-hostname"` - TFEToken string `mapstructure:"tfe-token"` - VCSStatusName string `mapstructure:"vcs-status-name"` - DefaultTFVersion string `mapstructure:"default-tf-version"` - Webhooks []WebhookConfig `mapstructure:"webhooks"` - WriteGitCreds bool `mapstructure:"write-git-creds"` + RequireMergeable bool `mapstructure:"require-mergeable"` + SilenceForkPRErrors bool `mapstructure:"silence-fork-pr-errors"` + // SilenceVCSStatusNoPlans is whether autoplan should set commit status if no plans + // are found. + SilenceVCSStatusNoPlans bool `mapstructure:"silence-vcs-status-no-plans"` + SilenceWhitelistErrors bool `mapstructure:"silence-whitelist-errors"` + SlackToken string `mapstructure:"slack-token"` + SSLCertFile string `mapstructure:"ssl-cert-file"` + SSLKeyFile string `mapstructure:"ssl-key-file"` + TFDownloadURL string `mapstructure:"tf-download-url"` + TFEHostname string `mapstructure:"tfe-hostname"` + TFEToken string `mapstructure:"tfe-token"` + VCSStatusName string `mapstructure:"vcs-status-name"` + DefaultTFVersion string `mapstructure:"default-tf-version"` + Webhooks []WebhookConfig `mapstructure:"webhooks"` + WriteGitCreds bool `mapstructure:"write-git-creds"` } // ToLogLevel returns the LogLevel object corresponding to the user-passed