feat: Remove flag for allow repo config (#3911)

* Remove option to allow repo * Fix internal test * Fix fmt * Fmt * Fix rebase --------- Co-authored-by: PePe Amengual <[email protected]> Co-authored-by: Dylan Page <[email protected]>
runatlantis · Dec 12, 2023 · d643c52 · d643c52
1 parent 95c5fe0
commit d643c52
Show file tree

Hide file tree

Showing 11 changed files with 246 additions and 296 deletions.
diff --git a/cmd/server.go b/cmd/server.go
@@ -51,7 +51,6 @@ const (
 	ADHostnameFlag                   = "azuredevops-hostname"
 	AllowCommandsFlag                = "allow-commands"
 	AllowForkPRsFlag                 = "allow-fork-prs"
-	AllowRepoConfigFlag              = "allow-repo-config"
 	AtlantisURLFlag                  = "atlantis-url"
 	AutoDiscoverModeFlag             = "autodiscover-mode"
 	AutomergeFlag                    = "automerge"
@@ -422,13 +421,6 @@ var boolFlags = map[string]boolFlag{
 		description:  "Allow Atlantis to run on pull requests from forks. A security issue for public repos.",
 		defaultValue: false,
 	},
-	AllowRepoConfigFlag: {
-		description: "Allow repositories to use atlantis.yaml files to customize the commands Atlantis runs." +
-			" Should only be enabled in a trusted environment since it enables a pull request to run arbitrary commands" +
-			" on the Atlantis server.",
-		defaultValue: false,
-		hidden:       true,
-	},
 	AutoplanModules: {
 		description:  "Automatically plan projects that have a changed module from the local repository.",
 		defaultValue: false,
@@ -1091,11 +1083,6 @@ func (s *ServerCmd) deprecationWarnings(userConfig *server.UserConfig) error {
 		jsonCfg += fmt.Sprintf(`, "apply_requirements":["%s"]`, strings.Join(commandReqs, "\", \""))
 		jsonCfg += fmt.Sprintf(`, "import_requirements":["%s"]`, strings.Join(commandReqs, "\", \""))
 	}
-	if userConfig.AllowRepoConfig {
-		deprecatedFlags = append(deprecatedFlags, AllowRepoConfigFlag)
-		yamlCfg += "\n  allowed_overrides: [plan_requirements, apply_requirements, import_requirements, workflow, policy_check]\n  allow_custom_workflows: true"
-		jsonCfg += `, "allowed_overrides":["plan_requirements","apply_requirements","import_requirements","workflow", "policy_check"], "allow_custom_workflows":true`
-	}
 	jsonCfg += "}]}"
 
 	if len(deprecatedFlags) > 0 {

diff --git a/cmd/server_test.go b/cmd/server_test.go
@@ -59,7 +59,6 @@ var testFlags = map[string]interface{}{
 	AtlantisURLFlag:                  "url",
 	AllowCommandsFlag:                "version,plan,apply,unlock,import,approve_policies",
 	AllowForkPRsFlag:                 true,
-	AllowRepoConfigFlag:              true,
 	AutoDiscoverModeFlag:             "auto",
 	AutomergeFlag:                    true,
 	AutoplanFileListFlag:             "**/*.tf,**/*.yml",

diff --git a/server/controllers/events/events_controller_e2e_test.go b/server/controllers/events/events_controller_e2e_test.go
@@ -1340,11 +1340,11 @@ func setupE2E(t *testing.T, repoDir string, opt setupOption) (events_controllers
 	parser := &config.ParserValidator{}
 
 	globalCfgArgs := valid.GlobalCfgArgs{
-		RepoConfigFile:   opt.repoConfigFile,
-		AllowRepoCfg:     true,
-		MergeableReq:     false,
-		ApprovedReq:      false,
-		PreWorkflowHooks: preWorkflowHooks,
+		RepoConfigFile:       opt.repoConfigFile,
+		AllowAllRepoSettings: true,
+		MergeableReq:         false,
+		ApprovedReq:          false,
+		PreWorkflowHooks:     preWorkflowHooks,
 		PostWorkflowHooks: []*valid.WorkflowHook{
 			{
 				StepName:   "global_hook",

diff --git a/server/core/config/parser_validator_test.go b/server/core/config/parser_validator_test.go
@@ -16,10 +16,10 @@ import (
 )
 
 var globalCfgArgs = valid.GlobalCfgArgs{
-	AllowRepoCfg:  true,
-	MergeableReq:  false,
-	ApprovedReq:   false,
-	UnDivergedReq: false,
+	AllowAllRepoSettings: true,
+	MergeableReq:         false,
+	ApprovedReq:          false,
+	UnDivergedReq:        false,
 }
 
 var globalCfg = valid.NewGlobalCfgFromArgs(globalCfgArgs)
@@ -105,7 +105,6 @@ func TestParseCfgs_InvalidYAML(t *testing.T) {
 			_, err = r.ParseRepoCfg(tmpDir, globalCfg, "", "")
 			ErrContains(t, c.expErr, err)
 			globalCfgArgs := valid.GlobalCfgArgs{
-				AllowRepoCfg:  false,
 				MergeableReq:  false,
 				ApprovedReq:   false,
 				UnDivergedReq: false,
@@ -1146,7 +1145,6 @@ workflows:
 
 	r := config.ParserValidator{}
 	globalCfgArgs := valid.GlobalCfgArgs{
-		AllowRepoCfg:  false,
 		MergeableReq:  false,
 		ApprovedReq:   false,
 		UnDivergedReq: false,
@@ -1159,7 +1157,6 @@ workflows:
 func TestParseGlobalCfg_NotExist(t *testing.T) {
 	r := config.ParserValidator{}
 	globalCfgArgs := valid.GlobalCfgArgs{
-		AllowRepoCfg:  false,
 		MergeableReq:  false,
 		ApprovedReq:   false,
 		UnDivergedReq: false,
@@ -1170,7 +1167,6 @@ func TestParseGlobalCfg_NotExist(t *testing.T) {
 
 func TestParseGlobalCfg(t *testing.T) {
 	globalCfgArgs := valid.GlobalCfgArgs{
-		AllowRepoCfg:  false,
 		MergeableReq:  false,
 		ApprovedReq:   false,
 		UnDivergedReq: false,
@@ -1627,7 +1623,6 @@ workflows:
 			Ok(t, os.WriteFile(path, []byte(c.input), 0600))
 
 			globalCfgArgs := valid.GlobalCfgArgs{
-				AllowRepoCfg:       false,
 				MergeableReq:       false,
 				ApprovedReq:        false,
 				UnDivergedReq:      false,
@@ -1734,7 +1729,6 @@ func TestParserValidator_ParseGlobalCfgJSON(t *testing.T) {
 		"empty object": {
 			json: "{}",
 			exp: valid.NewGlobalCfgFromArgs(valid.GlobalCfgArgs{
-				AllowRepoCfg:  false,
 				MergeableReq:  false,
 				ApprovedReq:   false,
 				UnDivergedReq: false,
@@ -1806,7 +1800,6 @@ func TestParserValidator_ParseGlobalCfgJSON(t *testing.T) {
 			exp: valid.GlobalCfg{
 				Repos: []valid.Repo{
 					valid.NewGlobalCfgFromArgs(valid.GlobalCfgArgs{
-						AllowRepoCfg:  false,
 						MergeableReq:  false,
 						ApprovedReq:   false,
 						UnDivergedReq: false,
@@ -1831,7 +1824,6 @@ func TestParserValidator_ParseGlobalCfgJSON(t *testing.T) {
 				},
 				Workflows: map[string]valid.Workflow{
 					"default": valid.NewGlobalCfgFromArgs(valid.GlobalCfgArgs{
-						AllowRepoCfg:  false,
 						MergeableReq:  false,
 						ApprovedReq:   false,
 						UnDivergedReq: false,
@@ -1857,7 +1849,6 @@ func TestParserValidator_ParseGlobalCfgJSON(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			pv := &config.ParserValidator{}
 			globalCfgArgs := valid.GlobalCfgArgs{
-				AllowRepoCfg:  false,
 				MergeableReq:  false,
 				ApprovedReq:   false,
 				UnDivergedReq: false,
@@ -1922,10 +1913,10 @@ func TestParseRepoCfg_V2ShellParsing(t *testing.T) {
 
 			p := &config.ParserValidator{}
 			globalCfgArgs := valid.GlobalCfgArgs{
-				AllowRepoCfg:  true,
-				MergeableReq:  false,
-				ApprovedReq:   false,
-				UnDivergedReq: false,
+				AllowAllRepoSettings: true,
+				MergeableReq:         false,
+				ApprovedReq:          false,
+				UnDivergedReq:        false,
 			}
 			v2Cfg, err := p.ParseRepoCfg(v2Dir, valid.NewGlobalCfgFromArgs(globalCfgArgs), "", "")
 			if c.expV2Err != "" {
@@ -1936,10 +1927,10 @@ func TestParseRepoCfg_V2ShellParsing(t *testing.T) {
 				Equals(t, c.expV2, v2Cfg.Workflows["custom"].Apply.Steps[0].RunCommand)
 			}
 			globalCfgArgs = valid.GlobalCfgArgs{
-				AllowRepoCfg:  true,
-				MergeableReq:  false,
-				ApprovedReq:   false,
-				UnDivergedReq: false,
+				AllowAllRepoSettings: true,
+				MergeableReq:         false,
+				ApprovedReq:          false,
+				UnDivergedReq:        false,
 			}
 			v3Cfg, err := p.ParseRepoCfg(v3Dir, valid.NewGlobalCfgFromArgs(globalCfgArgs), "", "")
 			Ok(t, err)

diff --git a/server/core/config/valid/global_cfg.go b/server/core/config/valid/global_cfg.go
@@ -174,42 +174,18 @@ var DefaultStateRmStage = Stage{
 	},
 }
 
-// Deprecated: use NewGlobalCfgFromArgs
-func NewGlobalCfgWithHooks(allowRepoCfg bool, mergeableReq bool, approvedReq bool, unDivergedReq bool, preWorkflowHooks []*WorkflowHook, postWorkflowHooks []*WorkflowHook) GlobalCfg {
-	return NewGlobalCfgFromArgs(GlobalCfgArgs{
-		AllowRepoCfg:      allowRepoCfg,
-		MergeableReq:      mergeableReq,
-		ApprovedReq:       approvedReq,
-		UnDivergedReq:     unDivergedReq,
-		PreWorkflowHooks:  preWorkflowHooks,
-		PostWorkflowHooks: postWorkflowHooks,
-	})
-}
-
-// NewGlobalCfg returns a global config that respects the parameters.
-// allowRepoCfg is true if users want to allow repos full config functionality.
-// mergeableReq is true if users want to set the mergeable apply requirement
-// for all repos.
-// approvedReq is true if users want to set the approved apply requirement
-// for all repos.
-// Deprecated: use NewGlobalCfgFromArgs
-func NewGlobalCfg(allowRepoCfg bool, mergeableReq bool, approvedReq bool) GlobalCfg {
-	return NewGlobalCfgFromArgs(GlobalCfgArgs{
-		AllowRepoCfg: allowRepoCfg,
-		MergeableReq: mergeableReq,
-		ApprovedReq:  approvedReq,
-	})
-}
-
 type GlobalCfgArgs struct {
-	RepoConfigFile     string
-	AllowRepoCfg       bool
-	MergeableReq       bool
-	ApprovedReq        bool
-	UnDivergedReq      bool
-	PolicyCheckEnabled bool
-	PreWorkflowHooks   []*WorkflowHook
-	PostWorkflowHooks  []*WorkflowHook
+	RepoConfigFile string
+	// No longer a user option as of https://github.com/runatlantis/atlantis/pull/3911,
+	// 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
 }
 
 func NewGlobalCfgFromArgs(args GlobalCfgArgs) GlobalCfg {
@@ -246,7 +222,7 @@ func NewGlobalCfgFromArgs(args GlobalCfgArgs) GlobalCfg {
 	repoLockingKey := true
 	customPolicyCheck := false
 	autoDiscover := AutoDiscover{Mode: AutoDiscoverAutoMode}
-	if args.AllowRepoCfg {
+	if args.AllowAllRepoSettings {
 		allowedOverrides = []string{PlanRequirementsKey, ApplyRequirementsKey, ImportRequirementsKey, WorkflowKey, DeleteSourceBranchOnMergeKey, RepoLockingKey, PolicyCheckKey}
 		allowCustomWorkflows = true
 	}