Skip to content

Commit

Permalink
chore: Remove settings for require approval, mergeable, undiverged (r…
Browse files Browse the repository at this point in the history
…unatlantis#4047)

* Remove settings for require approval, mergeable, undiverged

* Fix
  • Loading branch information
lukemassa authored and ijames-gc committed Feb 13, 2024
1 parent 8323114 commit 3f415eb
Show file tree
Hide file tree
Showing 12 changed files with 9 additions and 331 deletions.
49 changes: 6 additions & 43 deletions cmd/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (
"github.com/spf13/viper"

"github.com/runatlantis/atlantis/server"
"github.com/runatlantis/atlantis/server/core/config/valid"
"github.com/runatlantis/atlantis/server/events/vcs/bitbucketcloud"
"github.com/runatlantis/atlantis/server/logging"
)
Expand Down Expand Up @@ -117,8 +116,6 @@ const (
RepoConfigFlag = "repo-config"
RepoConfigJSONFlag = "repo-config-json"
RepoAllowlistFlag = "repo-allowlist"
RequireApprovalFlag = "require-approval"
RequireMergeableFlag = "require-mergeable"
SilenceNoProjectsFlag = "silence-no-projects"
SilenceForkPRErrorsFlag = "silence-fork-pr-errors"
SilenceVCSStatusNoPlans = "silence-vcs-status-no-plans"
Expand Down Expand Up @@ -497,16 +494,6 @@ var boolFlags = map[string]boolFlag{
description: "Controls whether the Redis client verifies the Redis server's certificate chain and host name. If true, accepts any certificate presented by the server and any host name in that certificate.",
defaultValue: DefaultRedisInsecureSkipVerify,
},
RequireApprovalFlag: {
description: "Require pull requests to be \"Approved\" before allowing the apply command to be run.",
defaultValue: false,
hidden: true,
},
RequireMergeableFlag: {
description: "Require pull requests to be mergeable before allowing the apply command to be run.",
defaultValue: false,
hidden: true,
},
SilenceNoProjectsFlag: {
description: "Silences Atlants from responding to PRs when it finds no projects.",
defaultValue: false,
Expand Down Expand Up @@ -1057,33 +1044,15 @@ func (s *ServerCmd) securityWarnings(userConfig *server.UserConfig) {
}

// deprecationWarnings prints a warning if flags that are deprecated are
// being used. Right now this only applies to flags that have been made obsolete
// due to server-side config.
// being used.
func (s *ServerCmd) deprecationWarnings(userConfig *server.UserConfig) error {
var commandReqs []string
var deprecatedFlags []string
if userConfig.RequireApproval {
deprecatedFlags = append(deprecatedFlags, RequireApprovalFlag)
commandReqs = append(commandReqs, valid.ApprovedCommandReq)
}
if userConfig.RequireMergeable {
deprecatedFlags = append(deprecatedFlags, RequireMergeableFlag)
commandReqs = append(commandReqs, valid.MergeableCommandReq)
}

// Build up strings with what the recommended yaml and json config should
// be instead of using the deprecated flags.
yamlCfg := "---\nrepos:\n- id: /.*/"
jsonCfg := `{"repos":[{"id":"/.*/"`
if len(commandReqs) > 0 {
yamlCfg += fmt.Sprintf("\n plan_requirements: [%s]", strings.Join(commandReqs, ", "))
yamlCfg += fmt.Sprintf("\n apply_requirements: [%s]", strings.Join(commandReqs, ", "))
yamlCfg += fmt.Sprintf("\n import_requirements: [%s]", strings.Join(commandReqs, ", "))
jsonCfg += fmt.Sprintf(`, "plan_requirements":["%s"]`, strings.Join(commandReqs, "\", \""))
jsonCfg += fmt.Sprintf(`, "apply_requirements":["%s"]`, strings.Join(commandReqs, "\", \""))
jsonCfg += fmt.Sprintf(`, "import_requirements":["%s"]`, strings.Join(commandReqs, "\", \""))
}
jsonCfg += "}]}"
// Currently there are no deprecated flags; if flags become deprecated, add them here like so
// if userConfig.SomeDeprecatedFlag {
// deprecatedFlags = append(deprecatedFlags, SomeDeprecatedFlag)
// }
//

if len(deprecatedFlags) > 0 {
warning := "WARNING: "
Expand All @@ -1092,12 +1061,6 @@ func (s *ServerCmd) deprecationWarnings(userConfig *server.UserConfig) error {
} else {
warning += fmt.Sprintf("Flags --%s and --%s have been deprecated.", strings.Join(deprecatedFlags[0:len(deprecatedFlags)-1], ", --"), deprecatedFlags[len(deprecatedFlags)-1:][0])
}
warning += fmt.Sprintf("\nCreate a --%s file with the following config instead:\n\n%s\n\nor use --%s='%s'\n",
RepoConfigFlag,
yamlCfg,
RepoConfigJSONFlag,
jsonCfg,
)
fmt.Println(warning)
}

Expand Down
2 changes: 0 additions & 2 deletions cmd/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,6 @@ var testFlags = map[string]interface{}{
RepoAllowlistFlag: "github.com/runatlantis/atlantis",
RepoConfigFlag: "",
RepoConfigJSONFlag: "",
RequireApprovalFlag: true,
RequireMergeableFlag: true,
SilenceNoProjectsFlag: false,
SilenceForkPRErrorsFlag: true,
SilenceAllowlistErrorsFlag: true,
Expand Down
4 changes: 0 additions & 4 deletions runatlantis.io/docs/command-requirements.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ by at least one person other than the author.

#### Usage
The `approved` requirement by:
1. Passing the `--require-approval` flag to `atlantis server` or
1. Creating a `repos.yaml` file with the `apply_requirements` key:
```yaml
repos:
Expand Down Expand Up @@ -62,7 +61,6 @@ The `mergeable` requirement will prevent applies unless a pull request is able t

#### Usage
Set the `mergeable` requirement by:
1. Passing the `--require-mergeable` flag to `atlantis server` or
1. Creating a `repos.yaml` file with the `apply_requirements` key:
```yaml
repos:
Expand Down Expand Up @@ -196,8 +194,6 @@ having that apply requirement set.

### Project-Specific Settings
If you only want some projects/repos to have apply requirements, then you must
1. Not set the `--require-approval` or `--require-mergeable` flags, since those
will override any `repos.yaml` or `atlantis.yaml` settings
1. Specifying which repos have which requirements via the `repos.yaml` file.
```yaml
repos:
Expand Down
2 changes: 0 additions & 2 deletions server/controllers/events/events_controller_e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1342,8 +1342,6 @@ func setupE2E(t *testing.T, repoDir string, opt setupOption) (events_controllers
globalCfgArgs := valid.GlobalCfgArgs{
RepoConfigFile: opt.repoConfigFile,
AllowAllRepoSettings: true,
MergeableReq: false,
ApprovedReq: false,
PreWorkflowHooks: preWorkflowHooks,
PostWorkflowHooks: []*valid.WorkflowHook{
{
Expand Down
36 changes: 0 additions & 36 deletions server/core/config/parser_validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,6 @@ import (

var globalCfgArgs = valid.GlobalCfgArgs{
AllowAllRepoSettings: true,
MergeableReq: false,
ApprovedReq: false,
UnDivergedReq: false,
}

var globalCfg = valid.NewGlobalCfgFromArgs(globalCfgArgs)
Expand Down Expand Up @@ -105,9 +102,6 @@ func TestParseCfgs_InvalidYAML(t *testing.T) {
_, err = r.ParseRepoCfg(tmpDir, globalCfg, "", "")
ErrContains(t, c.expErr, err)
globalCfgArgs := valid.GlobalCfgArgs{
MergeableReq: false,
ApprovedReq: false,
UnDivergedReq: false,
}
_, err = r.ParseGlobalCfg(confPath, valid.NewGlobalCfgFromArgs(globalCfgArgs))
ErrContains(t, c.expErr, err)
Expand Down Expand Up @@ -1145,9 +1139,6 @@ workflows:

r := config.ParserValidator{}
globalCfgArgs := valid.GlobalCfgArgs{
MergeableReq: false,
ApprovedReq: false,
UnDivergedReq: false,
}

_, err = r.ParseRepoCfg(tmpDir, valid.NewGlobalCfgFromArgs(globalCfgArgs), "repo_id", "branch")
Expand All @@ -1157,19 +1148,13 @@ workflows:
func TestParseGlobalCfg_NotExist(t *testing.T) {
r := config.ParserValidator{}
globalCfgArgs := valid.GlobalCfgArgs{
MergeableReq: false,
ApprovedReq: false,
UnDivergedReq: false,
}
_, err := r.ParseGlobalCfg("/not/exist", valid.NewGlobalCfgFromArgs(globalCfgArgs))
ErrEquals(t, "unable to read /not/exist file: open /not/exist: no such file or directory", err)
}

func TestParseGlobalCfg(t *testing.T) {
globalCfgArgs := valid.GlobalCfgArgs{
MergeableReq: false,
ApprovedReq: false,
UnDivergedReq: false,
}

defaultCfg := valid.NewGlobalCfgFromArgs(globalCfgArgs)
Expand Down Expand Up @@ -1623,9 +1608,6 @@ workflows:
Ok(t, os.WriteFile(path, []byte(c.input), 0600))

globalCfgArgs := valid.GlobalCfgArgs{
MergeableReq: false,
ApprovedReq: false,
UnDivergedReq: false,
PolicyCheckEnabled: false,
}

Expand Down Expand Up @@ -1729,9 +1711,6 @@ func TestParserValidator_ParseGlobalCfgJSON(t *testing.T) {
"empty object": {
json: "{}",
exp: valid.NewGlobalCfgFromArgs(valid.GlobalCfgArgs{
MergeableReq: false,
ApprovedReq: false,
UnDivergedReq: false,
}),
},
"setting all keys": {
Expand Down Expand Up @@ -1800,9 +1779,6 @@ func TestParserValidator_ParseGlobalCfgJSON(t *testing.T) {
exp: valid.GlobalCfg{
Repos: []valid.Repo{
valid.NewGlobalCfgFromArgs(valid.GlobalCfgArgs{
MergeableReq: false,
ApprovedReq: false,
UnDivergedReq: false,
}).Repos[0],
{
IDRegex: regexp.MustCompile(".*"),
Expand All @@ -1824,9 +1800,6 @@ func TestParserValidator_ParseGlobalCfgJSON(t *testing.T) {
},
Workflows: map[string]valid.Workflow{
"default": valid.NewGlobalCfgFromArgs(valid.GlobalCfgArgs{
MergeableReq: false,
ApprovedReq: false,
UnDivergedReq: false,
}).Workflows["default"],
"custom": customWorkflow,
},
Expand All @@ -1849,9 +1822,6 @@ func TestParserValidator_ParseGlobalCfgJSON(t *testing.T) {
t.Run(name, func(t *testing.T) {
pv := &config.ParserValidator{}
globalCfgArgs := valid.GlobalCfgArgs{
MergeableReq: false,
ApprovedReq: false,
UnDivergedReq: false,
}
cfg, err := pv.ParseGlobalCfgJSON(c.json, valid.NewGlobalCfgFromArgs(globalCfgArgs))
if c.expErr != "" {
Expand Down Expand Up @@ -1914,9 +1884,6 @@ func TestParseRepoCfg_V2ShellParsing(t *testing.T) {
p := &config.ParserValidator{}
globalCfgArgs := valid.GlobalCfgArgs{
AllowAllRepoSettings: true,
MergeableReq: false,
ApprovedReq: false,
UnDivergedReq: false,
}
v2Cfg, err := p.ParseRepoCfg(v2Dir, valid.NewGlobalCfgFromArgs(globalCfgArgs), "", "")
if c.expV2Err != "" {
Expand All @@ -1928,9 +1895,6 @@ func TestParseRepoCfg_V2ShellParsing(t *testing.T) {
}
globalCfgArgs = valid.GlobalCfgArgs{
AllowAllRepoSettings: true,
MergeableReq: false,
ApprovedReq: false,
UnDivergedReq: false,
}
v3Cfg, err := p.ParseRepoCfg(v3Dir, valid.NewGlobalCfgFromArgs(globalCfgArgs), "", "")
Ok(t, err)
Expand Down
12 changes: 0 additions & 12 deletions server/core/config/valid/global_cfg.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,9 +180,6 @@ type GlobalCfgArgs struct {
// but useful for tests to set to true to not require enumeration of allowed settings
// on the repo side
AllowAllRepoSettings bool
MergeableReq bool
ApprovedReq bool
UnDivergedReq bool
PolicyCheckEnabled bool
PreWorkflowHooks []*WorkflowHook
PostWorkflowHooks []*WorkflowHook
Expand All @@ -203,15 +200,6 @@ func NewGlobalCfgFromArgs(args GlobalCfgArgs) GlobalCfg {
allowedOverrides := []string{}
allowedWorkflows := []string{}
policyCheck := false
if args.MergeableReq {
commandReqs = append(commandReqs, MergeableCommandReq)
}
if args.ApprovedReq {
commandReqs = append(commandReqs, ApprovedCommandReq)
}
if args.UnDivergedReq {
commandReqs = append(commandReqs, UnDivergedCommandReq)
}
if args.PolicyCheckEnabled {
commandReqs = append(commandReqs, PoliciesPassedCommandReq)
policyCheck = true
Expand Down
Loading

0 comments on commit 3f415eb

Please sign in to comment.