feat: Remove flag for disable apply (#3912)

* Add code for disabling apply flag

* Better comment

cmd/server.go

@@ -28,7 +28,6 @@ import (
 
 	"github.com/runatlantis/atlantis/server"
 	"github.com/runatlantis/atlantis/server/core/config/valid"
-	"github.com/runatlantis/atlantis/server/events/command"
 	"github.com/runatlantis/atlantis/server/events/vcs/bitbucketcloud"
 	"github.com/runatlantis/atlantis/server/logging"
 )
@@ -71,7 +70,6 @@ const (
 	DataDirFlag                      = "data-dir"
 	DefaultTFVersionFlag             = "default-tf-version"
 	DisableApplyAllFlag              = "disable-apply-all"
-	DisableApplyFlag                 = "disable-apply"
 	DisableAutoplanFlag              = "disable-autoplan"
 	DisableAutoplanLabelFlag         = "disable-autoplan-label"
 	DisableMarkdownFoldingFlag       = "disable-markdown-folding"
@@ -443,10 +441,6 @@ var boolFlags = map[string]boolFlag{
 		description:  "Disable \"atlantis apply\" command without any flags (i.e. apply all). A specific project/workspace/directory has to be specified for applies.",
 		defaultValue: false,
 	},
-	DisableApplyFlag: {
-		description:  "Disable all \"atlantis apply\" command regardless of which flags are passed with it.",
-		defaultValue: false,
-	},
 	DisableAutoplanFlag: {
 		description:  "Disable atlantis auto planning feature",
 		defaultValue: false,
@@ -1084,16 +1078,6 @@ func (s *ServerCmd) deprecationWarnings(userConfig *server.UserConfig) error {
 		deprecatedFlags = append(deprecatedFlags, RequireMergeableFlag)
 		commandReqs = append(commandReqs, valid.MergeableCommandReq)
 	}
-	if userConfig.DisableApply {
-		deprecatedFlags = append(deprecatedFlags, DisableApplyFlag)
-		var filtered []string
-		for _, allowCommand := range strings.Split(userConfig.AllowCommands, ",") {
-			if allowCommand != command.Apply.String() {
-				filtered = append(filtered, allowCommand)
-			}
-		}
-		userConfig.AllowCommands = strings.Join(filtered, ",")
-	}
 
 	// Build up strings with what the recommended yaml and json config should
 	// be instead of using the deprecated flags.

cmd/server_test.go

@@ -57,7 +57,7 @@ var testFlags = map[string]interface{}{
 	ADWebhookPasswordFlag:            "ad-wh-pass",
 	ADWebhookUserFlag:                "ad-wh-user",
 	AtlantisURLFlag:                  "url",
-	AllowCommandsFlag:                "version,plan,unlock,import,approve_policies", // apply is disabled by DisableApply
+	AllowCommandsFlag:                "version,plan,apply,unlock,import,approve_policies",
 	AllowForkPRsFlag:                 true,
 	AllowRepoConfigFlag:              true,
 	AutoDiscoverModeFlag:             "auto",
@@ -71,7 +71,6 @@ var testFlags = map[string]interface{}{
 	DataDirFlag:                      "/path",
 	DefaultTFVersionFlag:             "v0.11.0",
 	DisableApplyAllFlag:              true,
-	DisableApplyFlag:                 true,
 	DisableMarkdownFoldingFlag:       true,
 	DisableRepoLockingFlag:           true,
 	DiscardApprovalOnPlanFlag:        true,
@@ -763,16 +762,6 @@ func TestExecute_AllowAndWhitelist(t *testing.T) {
 	ErrEquals(t, "--repo-allowlist must be set for security purposes", err)
 }
 
-func TestExecute_DisableApplyDeprecation(t *testing.T) {
-	c := setupWithDefaults(map[string]interface{}{
-		DisableApplyFlag:  true,
-		AllowCommandsFlag: "plan,apply,unlock",
-	}, t)
-	err := c.Execute()
-	Ok(t, err)
-	Equals(t, "plan,unlock", passedConfig.AllowCommands)
-}
-
 func TestExecute_AutoDetectModulesFromProjects_Env(t *testing.T) {
 	t.Setenv("ATLANTIS_AUTOPLAN_MODULES_FROM_PROJECTS", "**/init.tf")
 	c := setupWithDefaults(map[string]interface{}{}, t)

server/controllers/events/events_controller_e2e_test.go

@@ -87,8 +87,6 @@ func TestGitHubWorkflow(t *testing.T) {
 		ModifiedFiles []string
 		// Comments are what our mock user writes to the pull request.
 		Comments []string
-		// DisableApply flag used by userConfig object when initializing atlantis server.
-		DisableApply bool
 		// ApplyLock creates an apply lock that temporarily disables apply command
 		ApplyLock bool
 		// AllowCommands flag what kind of atlantis commands are available.
@@ -457,7 +455,6 @@ func TestGitHubWorkflow(t *testing.T) {
 			Description:   "global apply lock disables apply commands",
 			RepoDir:       "simple-yaml",
 			ModifiedFiles: []string{"main.tf"},
-			DisableApply:  false,
 			ApplyLock:     true,
 			ExpAutoplan:   true,
 			Comments: []string{
@@ -470,18 +467,22 @@ func TestGitHubWorkflow(t *testing.T) {
 			},
 		},
 		{
-			Description:   "disable apply flag always takes presedence",
+			Description:   "omitting apply from allow commands always takes presedence",
 			RepoDir:       "simple-yaml",
 			ModifiedFiles: []string{"main.tf"},
-			DisableApply:  true,
+			AllowCommands: []command.Name{command.Plan},
 			ApplyLock:     false,
 			ExpAutoplan:   true,
 			Comments: []string{
 				"atlantis apply",
 			},
+			ExpParseFailedCount:         1,
+			ExpAllowResponseCommentBack: true,
 			ExpReplies: [][]string{
 				{"exp-output-autoplan.txt"},
-				{"exp-output-apply-locked.txt"},
+				// Disabling apply is implementing by omitting it from the apply list
+				// See: https://github.com/runatlantis/atlantis/pull/2877
+				{"exp-output-allow-command-unknown-apply.txt"},
 				{"exp-output-merge.txt"},
 			},
 		},
@@ -628,7 +629,6 @@ func TestGitHubWorkflow(t *testing.T) {
 
 			// reset userConfig
 			userConfig = server.UserConfig{}
-			userConfig.DisableApply = c.DisableApply
 
 			opt := setupOption{
 				repoConfigFile:          c.RepoConfigFile,
@@ -799,7 +799,6 @@ func TestSimpleWorkflow_terraformLockFile(t *testing.T) {
 
 			// reset userConfig
 			userConfig = server.UserConfig{}
-			userConfig.DisableApply = true
 
 			ctrl, vcsClient, githubGetter, atlantisWorkspace := setupE2E(t, c.RepoDir, setupOption{})
 			// Set the repo to be cloned through the testing backdoor.
@@ -1295,6 +1294,13 @@ func setupE2E(t *testing.T, repoDir string, opt setupOption) (events_controllers
 	if opt.allowCommands != nil {
 		allowCommands = opt.allowCommands
 	}
+	disableApply := true
+	for _, allowCommand := range allowCommands {
+		if allowCommand == command.Apply {
+			disableApply = false
+			break
+		}
+	}
 	commentParser := &events.CommentParser{
 		GithubUser:     "github-user",
 		GitlabUser:     "gitlab-user",
@@ -1308,7 +1314,7 @@ func setupE2E(t *testing.T, repoDir string, opt setupOption) (events_controllers
 	backend := boltdb
 	lockingClient := locking.NewClient(boltdb)
 	noOpLocker := locking.NewNoOpLocker()
-	applyLocker = locking.NewApplyClient(boltdb, userConfig.DisableApply)
+	applyLocker = locking.NewApplyClient(boltdb, disableApply)
 	projectLocker := &events.DefaultProjectLocker{
 		Locker:     lockingClient,
 		NoOpLocker: noOpLocker,

server/controllers/events/testdata/test-repos/simple-yaml/exp-output-allow-command-unknown-apply.txt

@@ -0,0 +1,5 @@
+```
+Error: unknown command "apply".
+Run 'atlantis --help' for usage.
+Available commands(--allow-commands): plan
+```

server/core/locking/apply_locking.go

@@ -31,7 +31,7 @@ type ApplyLocker interface {
 // ApplyCommandLock contains information about apply command lock status.
 type ApplyCommandLock struct {
 	// Locked is true is when apply commands are locked
-	// Either by using DisableApply flag or creating a global ApplyCommandLock
+	// Either by using omitting apply from AllowCommands or creating a global ApplyCommandLock
 	// DisableApply lock take precedence when set
 	Locked  bool
 	Time    time.Time
@@ -39,25 +39,25 @@ type ApplyCommandLock struct {
 }
 
 type ApplyClient struct {
-	backend          Backend
+	backend      Backend
-	disableApplyFlag bool
+	disableApply bool
 }
 
-func NewApplyClient(backend Backend, disableApplyFlag bool) ApplyLocker {
+func NewApplyClient(backend Backend, disableApply bool) ApplyLocker {
 	return &ApplyClient{
-		backend:          backend,
+		backend:      backend,
-		disableApplyFlag: disableApplyFlag,
+		disableApply: disableApply,
 	}
 }
 
 // LockApply acquires global apply lock.
-// DisableApplyFlag takes presedence to any existing locks, if it is set to true
+// DisableApply takes presedence to any existing locks, if it is set to true
 // this function returns an error
 func (c *ApplyClient) LockApply() (ApplyCommandLock, error) {
 	response := ApplyCommandLock{}
 
-	if c.disableApplyFlag {
+	if c.disableApply {
-		return response, errors.New("DisableApplyFlag is set; Apply commands are locked globally until flag is unset")
+		return response, errors.New("apply is omitted from AllowCommands; Apply commands are locked globally until flag is updated")
 	}
 
 	applyCmdLock, err := c.backend.LockCommand(command.Apply, time.Now())
@@ -73,11 +73,11 @@ func (c *ApplyClient) LockApply() (ApplyCommandLock, error) {
 }
 
 // UnlockApply releases a global apply lock.
-// DisableApplyFlag takes presedence to any existing locks, if it is set to true
+// DisableApply takes presedence to any existing locks, if it is set to true
 // this function returns an error
 func (c *ApplyClient) UnlockApply() error {
-	if c.disableApplyFlag {
+	if c.disableApply {
-		return errors.New("apply commands are disabled until DisableApply flag is unset")
+		return errors.New("apply commands are disabled until AllowCommands flag is updated")
 	}
 
 	err := c.backend.UnlockCommand(command.Apply)
@@ -89,11 +89,11 @@ func (c *ApplyClient) UnlockApply() error {
 }
 
 // CheckApplyLock retrieves an apply command lock if present.
-// If DisableApplyFlag is set it will always return a lock.
+// If DisableApply is set it will always return a lock.
 func (c *ApplyClient) CheckApplyLock() (ApplyCommandLock, error) {
 	response := ApplyCommandLock{}
 
-	if c.disableApplyFlag {
+	if c.disableApply {
 		return ApplyCommandLock{
 			Locked: true,
 		}, nil

server/core/locking/locking_test.go

@@ -202,12 +202,12 @@ func TestApplyLocker(t *testing.T) {
 			Assert(t, !lock.Locked, "exp false")
 		})
 
-		t.Run("can't lock if userConfig.DisableApply is set", func(t *testing.T) {
+		t.Run("can't lock if apply is omitted from userConfig.AllowCommands", func(t *testing.T) {
 			backend := mocks.NewMockBackend()
 
 			l := locking.NewApplyClient(backend, true)
 			_, err := l.LockApply()
-			ErrEquals(t, "DisableApplyFlag is set; Apply commands are locked globally until flag is unset", err)
+			ErrEquals(t, "apply is omitted from AllowCommands; Apply commands are locked globally until flag is updated", err)
 
 			backend.VerifyWasCalled(Never()).LockCommand(Any[command.Name](), Any[time.Time]())
 		})
@@ -232,12 +232,12 @@ func TestApplyLocker(t *testing.T) {
 			Equals(t, errExpected, err)
 		})
 
-		t.Run("can't unlock if userConfig.DisableApply is set", func(t *testing.T) {
+		t.Run("can't lock if apply is omitted from userConfig.AllowCommands", func(t *testing.T) {
 			backend := mocks.NewMockBackend()
 
 			l := locking.NewApplyClient(backend, true)
 			err := l.UnlockApply()
-			ErrEquals(t, "apply commands are disabled until DisableApply flag is unset", err)
+			ErrEquals(t, "apply commands are disabled until AllowCommands flag is updated", err)
 
 			backend.VerifyWasCalled(Never()).UnlockCommand(Any[command.Name]())
 		})
@@ -264,7 +264,7 @@ func TestApplyLocker(t *testing.T) {
 			Equals(t, lock.Locked, false)
 		})
 
-		t.Run("when DisableApply flag is set always return a lock", func(t *testing.T) {
+		t.Run("when apply is not in AllowCommands always return a lock", func(t *testing.T) {
 			backend := mocks.NewMockBackend()
 
 			l := locking.NewApplyClient(backend, true)

server/events/apply_command_runner.go

@@ -68,7 +68,7 @@ func (a *ApplyCommandRunner) Run(ctx *command.Context, cmd *CommentCommand) {
 	pull := ctx.Pull
 
 	locked, err := a.IsLocked()
-	// CheckApplyLock falls back to DisableApply flag if fetching the lock
+	// CheckApplyLock falls back to AllowedCommand flag if fetching the lock
 	// raises an error
 	// We will log failure as warning
 	if err != nil {

server/events/apply_command_runner_test.go

@@ -34,13 +34,13 @@ func TestApplyCommandRunner_IsLocked(t *testing.T) {
 			ExpComment:     "**Error:** Running `atlantis apply` is disabled.",
 		},
 		{
-			Description:    "When no global apply lock is present and DisableApply flag is false IsDisabled returns false",
+			Description:    "When no global apply lock is present IsDisabled returns false",
 			ApplyLocked:    false,
 			ApplyLockError: nil,
 			ExpComment:     "Ran Apply for 0 projects:",
 		},
 		{
-			Description:    "If ApplyLockChecker returns an error IsDisabled return value of DisableApply flag",
+			Description:    "If ApplyLockChecker returns an error IsDisabled returns false",
 			ApplyLockError: errors.New("error"),
 			ApplyLocked:    false,
 			ExpComment:     "Ran Apply for 0 projects:",

server/user_config.go

@@ -33,7 +33,6 @@ type UserConfig struct {
 	CheckoutStrategy            string `mapstructure:"checkout-strategy"`
 	DataDir                     string `mapstructure:"data-dir"`
 	DisableApplyAll             bool   `mapstructure:"disable-apply-all"`
-	DisableApply                bool   `mapstructure:"disable-apply"`
 	DisableAutoplan             bool   `mapstructure:"disable-autoplan"`
 	DisableAutoplanLabel        string `mapstructure:"disable-autoplan-label"`
 	DisableMarkdownFolding      bool   `mapstructure:"disable-markdown-folding"`