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

chore: Remove settings for require approval, mergeable, undiverged #4047

Merged
merged 2 commits into from
Dec 27, 2023
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
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