From 78eea78ef3dcb680d28b44842ba87f32270d9f22 Mon Sep 17 00:00:00 2001 From: Sarvar Muminov <43311+msarvar@users.noreply.github.com> Date: Tue, 22 Feb 2022 14:44:04 -0800 Subject: [PATCH] Orca 4417 refactor merge config (#183) * Add platform mode flag to GlobalCfg * Added server side config support for platform mode * Adding flag to enable platform mode * fix tests * Moved some of the project level validations to project struct Added platform mode config support for project level workflow overrides. We do not support custom platform mode workflows on repo configs * fix broken test * moved validations to repo_cfg * moved some repetitive code to helper functino * remove platform mode changes * clean up for loop * remove platform mode values from project --- server/core/config/raw/global_cfg.go | 6 +- server/core/config/valid/global_cfg.go | 252 +++++++------------- server/core/config/valid/global_cfg_test.go | 27 +-- server/core/config/valid/project.go | 119 +++++++++ server/core/config/valid/project_test.go | 181 ++++++++++++++ server/core/config/valid/repo_cfg.go | 67 ++++-- 6 files changed, 444 insertions(+), 208 deletions(-) create mode 100644 server/core/config/valid/project.go create mode 100644 server/core/config/valid/project_test.go diff --git a/server/core/config/raw/global_cfg.go b/server/core/config/raw/global_cfg.go index 7d7dde4650..69f8d9814c 100644 --- a/server/core/config/raw/global_cfg.go +++ b/server/core/config/raw/global_cfg.go @@ -41,7 +41,7 @@ type Repo struct { func (g GlobalCfg) GetWorkflowNames() []string { names := make([]string, 0) - for name, _ := range g.Workflows { + for name := range g.Workflows { names = append(names, name) } return names @@ -49,7 +49,7 @@ func (g GlobalCfg) GetWorkflowNames() []string { func (g GlobalCfg) GetPullRequestWorkflowNames() []string { names := make([]string, 0) - for name, _ := range g.PullRequestWorkflows { + for name := range g.PullRequestWorkflows { names = append(names, name) } return names @@ -57,7 +57,7 @@ func (g GlobalCfg) GetPullRequestWorkflowNames() []string { func (g GlobalCfg) GetDeploymentWorkflowNames() []string { names := make([]string, 0) - for name, _ := range g.DeploymentWorkflows { + for name := range g.DeploymentWorkflows { names = append(names, name) } return names diff --git a/server/core/config/valid/global_cfg.go b/server/core/config/valid/global_cfg.go index ceeed64b53..c9487ace84 100644 --- a/server/core/config/valid/global_cfg.go +++ b/server/core/config/valid/global_cfg.go @@ -252,10 +252,29 @@ func (g GlobalCfg) PlatformModeEnabled() bool { // final config. It assumes that all configs have been validated. func (g GlobalCfg) MergeProjectCfg(log logging.SimpleLogging, repoID string, proj Project, rCfg RepoCfg) MergedProjectCfg { log.Debug("MergeProjectCfg started") - applyReqs, workflow, pullRequestWorkflow, deploymentWorkflow, allowedOverrides, allowCustomWorkflows, deleteSourceBranchOnMerge := g.getMatchingCfg(log, repoID) + var applyReqs []string + var workflow Workflow + var pullRequestWorkflow Workflow + var deploymentWorkflow Workflow + var allowCustomWorkflows bool + var deleteSourceBranchOnMerge bool + + repo := g.foldMatchingRepos(repoID) + + applyReqs = repo.ApplyRequirements + allowCustomWorkflows = *repo.AllowCustomWorkflows + deleteSourceBranchOnMerge = *repo.DeleteSourceBranchOnMerge + workflow = *repo.Workflow + + // If platform mode is enabled there will be at least default workflows, + // otherwise values will be nil + if g.PlatformModeEnabled() { + pullRequestWorkflow = *repo.PullRequestWorkflow + deploymentWorkflow = *repo.DeploymentWorkflow + } // If repos are allowed to override certain keys then override them. - for _, key := range allowedOverrides { + for _, key := range repo.AllowedOverrides { switch key { case ApplyRequirementsKey: if proj.ApplyRequirements != nil { @@ -270,20 +289,16 @@ func (g GlobalCfg) MergeProjectCfg(log logging.SimpleLogging, repoID string, pro // define its own workflow. We also know that a workflow will // exist with this name due to earlier validation. name := *proj.WorkflowName - for k, v := range g.Workflows { - if k == name { - workflow = v - } + if w, ok := g.Workflows[name]; ok { + workflow = w } - if allowCustomWorkflows { - for k, v := range rCfg.Workflows { - if k == name { - workflow = v - } - } + + if w, ok := rCfg.Workflows[name]; allowCustomWorkflows && ok { + workflow = w } log.Debug("overriding server-defined %s with repo-specified workflow: %q", WorkflowKey, workflow.Name) } + case DeleteSourceBranchOnMergeKey: //We check whether the server configured value and repo-root level //config is different. If it is then we change to the more granular. @@ -302,8 +317,14 @@ func (g GlobalCfg) MergeProjectCfg(log logging.SimpleLogging, repoID string, pro log.Debug("MergeProjectCfg completed") } - log.Debug("final settings: %s: [%s], %s: %s", - ApplyRequirementsKey, strings.Join(applyReqs, ","), WorkflowKey, workflow.Name) + if g.PlatformModeEnabled() { + log.Debug("final settings: %s: %s, %s: %s, %s: %s", + WorkflowKey, workflow.Name, DeploymentWorkflowKey, deploymentWorkflow.Name, PullRequestWorkflowKey, pullRequestWorkflow.Name) + + } else { + log.Debug("final settings: %s: [%s], %s: %s", + ApplyRequirementsKey, strings.Join(applyReqs, ","), WorkflowKey, workflow.Name) + } return MergedProjectCfg{ ApplyRequirements: applyReqs, @@ -326,186 +347,89 @@ func (g GlobalCfg) MergeProjectCfg(log logging.SimpleLogging, repoID string, pro // repo with id repoID. It is used when there is no repo config. func (g GlobalCfg) DefaultProjCfg(log logging.SimpleLogging, repoID string, repoRelDir string, workspace string) MergedProjectCfg { log.Debug("building config based on server-side config") - applyReqs, workflow, pullRequestWorkflow, deploymentWorkflow, _, _, deleteSourceBranchOnMerge := g.getMatchingCfg(log, repoID) - return MergedProjectCfg{ - ApplyRequirements: applyReqs, - Workflow: workflow, - PullRequestWorkflow: pullRequestWorkflow, - DeploymentWorkflow: deploymentWorkflow, + repo := g.foldMatchingRepos(repoID) + + mrgPrj := MergedProjectCfg{ + ApplyRequirements: repo.ApplyRequirements, + Workflow: *repo.Workflow, RepoRelDir: repoRelDir, Workspace: workspace, Name: "", AutoplanEnabled: DefaultAutoPlanEnabled, TerraformVersion: nil, PolicySets: g.PolicySets, - DeleteSourceBranchOnMerge: deleteSourceBranchOnMerge, - } -} - -// ValidateRepoCfg validates that rCfg for repo with id repoID is valid based -// on our global config. -func (g GlobalCfg) ValidateRepoCfg(rCfg RepoCfg, repoID string) error { - - sliceContainsF := func(slc []string, str string) bool { - for _, s := range slc { - if s == str { - return true - } - } - return false - } - mapContainsF := func(m map[string]Workflow, key string) bool { - for k := range m { - if k == key { - return true - } - } - return false - } - - // Check allowed overrides. - var allowedOverrides []string - for _, repo := range g.Repos { - if repo.IDMatches(repoID) { - if repo.AllowedOverrides != nil { - allowedOverrides = repo.AllowedOverrides - } - } - } - for _, p := range rCfg.Projects { - if p.WorkflowName != nil && !sliceContainsF(allowedOverrides, WorkflowKey) { - return fmt.Errorf("repo config not allowed to set '%s' key: server-side config needs '%s: [%s]'", WorkflowKey, AllowedOverridesKey, WorkflowKey) - } - if p.ApplyRequirements != nil && !sliceContainsF(allowedOverrides, ApplyRequirementsKey) { - return fmt.Errorf("repo config not allowed to set '%s' key: server-side config needs '%s: [%s]'", ApplyRequirementsKey, AllowedOverridesKey, ApplyRequirementsKey) - } - if p.DeleteSourceBranchOnMerge != nil && !sliceContainsF(allowedOverrides, DeleteSourceBranchOnMergeKey) { - return fmt.Errorf("repo config not allowed to set '%s' key: server-side config needs '%s: [%s]'", DeleteSourceBranchOnMergeKey, AllowedOverridesKey, DeleteSourceBranchOnMergeKey) - } - } - - // Check custom workflows. - var allowCustomWorkflows bool - for _, repo := range g.Repos { - if repo.IDMatches(repoID) { - if repo.AllowCustomWorkflows != nil { - allowCustomWorkflows = *repo.AllowCustomWorkflows - } - } + DeleteSourceBranchOnMerge: *repo.DeleteSourceBranchOnMerge, } - if len(rCfg.Workflows) > 0 && !allowCustomWorkflows { - return fmt.Errorf("repo config not allowed to define custom workflows: server-side config needs '%s: true'", AllowCustomWorkflowsKey) - } - - // Check if the repo has set a workflow name that doesn't exist. - for _, p := range rCfg.Projects { - if p.WorkflowName != nil { - name := *p.WorkflowName - if !mapContainsF(rCfg.Workflows, name) && !mapContainsF(g.Workflows, name) { - return fmt.Errorf("workflow %q is not defined anywhere", name) - } - } - } - - // Check workflow is allowed - var allowedWorkflows []string - for _, repo := range g.Repos { - if repo.IDMatches(repoID) { - - if repo.AllowedWorkflows != nil { - allowedWorkflows = repo.AllowedWorkflows - } - } - } - - for _, p := range rCfg.Projects { - // default is always allowed - if p.WorkflowName != nil && len(allowedWorkflows) != 0 { - name := *p.WorkflowName - if allowCustomWorkflows { - // If we allow CustomWorkflows we need to check that workflow name is defined inside repo and not global. - if mapContainsF(rCfg.Workflows, name) { - break - } - } - - if !sliceContainsF(allowedWorkflows, name) { - return fmt.Errorf("workflow '%s' is not allowed for this repo", name) - } - } - } - - return nil + return mrgPrj } -// getMatchingCfg returns the key settings for repoID. -func (g GlobalCfg) getMatchingCfg(log logging.SimpleLogging, repoID string) ( - applyReqs []string, - workflow Workflow, - pullRequestWorkflow Workflow, - deploymentWorkflow Workflow, - allowedOverrides []string, - allowCustomWorkflows bool, - deleteSourceBranchOnMerge bool, -) { - toLog := make(map[string]string) - traceF := func(repoIdx int, repoID string, key string, val interface{}) string { - from := "default server config" - if repoIdx > 0 { - from = fmt.Sprintf("repos[%d], id: %s", repoIdx, repoID) - } - var valStr string - switch v := val.(type) { - case string: - valStr = fmt.Sprintf("%q", v) - case []string: - valStr = fmt.Sprintf("[%s]", strings.Join(v, ",")) - case bool: - valStr = fmt.Sprintf("%t", v) - default: - valStr = "this is a bug" - } - - return fmt.Sprintf("setting %s: %s from %s", key, valStr, from) +// foldMatchingRepos will return a pseudo repo instance that will iterate over +// the matching repositories and assign relevant fields if they're defined. +// This means returned object will contain the last matching repo's value as a it's fields +func (g GlobalCfg) foldMatchingRepos(repoID string) Repo { + foldedRepo := Repo{ + AllowedWorkflows: make([]string, 0), + AllowedOverrides: make([]string, 0), + ApplyRequirements: make([]string, 0), } - for i, repo := range g.Repos { + for _, repo := range g.Repos { if repo.IDMatches(repoID) { if repo.ApplyRequirements != nil { - toLog[ApplyRequirementsKey] = traceF(i, repo.IDString(), ApplyRequirementsKey, repo.ApplyRequirements) - applyReqs = repo.ApplyRequirements + foldedRepo.ApplyRequirements = repo.ApplyRequirements } if repo.Workflow != nil { - toLog[WorkflowKey] = traceF(i, repo.IDString(), WorkflowKey, repo.Workflow.Name) - workflow = *repo.Workflow + foldedRepo.Workflow = repo.Workflow } if repo.PullRequestWorkflow != nil { - toLog[PullRequestWorkflowKey] = traceF(i, repo.IDString(), PullRequestWorkflowKey, repo.PullRequestWorkflow.Name) - pullRequestWorkflow = *repo.PullRequestWorkflow + foldedRepo.PullRequestWorkflow = repo.PullRequestWorkflow } if repo.DeploymentWorkflow != nil { - toLog[DeploymentWorkflowKey] = traceF(i, repo.IDString(), DeploymentWorkflowKey, repo.DeploymentWorkflow.Name) - deploymentWorkflow = *repo.DeploymentWorkflow + foldedRepo.DeploymentWorkflow = repo.DeploymentWorkflow + } + if repo.AllowedWorkflows != nil { + foldedRepo.AllowedWorkflows = repo.AllowedWorkflows } if repo.AllowedOverrides != nil { - toLog[AllowedOverridesKey] = traceF(i, repo.IDString(), AllowedOverridesKey, repo.AllowedOverrides) - allowedOverrides = repo.AllowedOverrides + foldedRepo.AllowedOverrides = repo.AllowedOverrides } if repo.AllowCustomWorkflows != nil { - toLog[AllowCustomWorkflowsKey] = traceF(i, repo.IDString(), AllowCustomWorkflowsKey, *repo.AllowCustomWorkflows) - allowCustomWorkflows = *repo.AllowCustomWorkflows + foldedRepo.AllowCustomWorkflows = repo.AllowCustomWorkflows } if repo.DeleteSourceBranchOnMerge != nil { - toLog[DeleteSourceBranchOnMergeKey] = traceF(i, repo.IDString(), DeleteSourceBranchOnMergeKey, *repo.DeleteSourceBranchOnMerge) - deleteSourceBranchOnMerge = *repo.DeleteSourceBranchOnMerge + foldedRepo.DeleteSourceBranchOnMerge = repo.DeleteSourceBranchOnMerge } } } - for _, l := range toLog { - log.Debug(l) + + return foldedRepo +} + +// ValidateRepoCfg validates that rCfg for repo with id repoID is valid based +// on our global config. +func (g GlobalCfg) ValidateRepoCfg(rCfg RepoCfg, repoID string) error { + repo := g.foldMatchingRepos(repoID) + + // Check allowed overrides. + allowedOverrides := repo.AllowedOverrides + + if err := rCfg.ValidateAllowedOverrides(allowedOverrides); err != nil { + return err + } + + allowCustomWorkflows := *repo.AllowCustomWorkflows + // Check custom workflows. + if len(rCfg.Workflows) > 0 && !allowCustomWorkflows { + return fmt.Errorf("repo config not allowed to define custom workflows: server-side config needs '%s: true'", AllowCustomWorkflowsKey) + } + + // Check if the repo has set a workflow name that doesn't exist and if workflow is allowed + if err := rCfg.ValidateWorkflows(g.Workflows, repo.AllowedWorkflows, allowCustomWorkflows); err != nil { + return err } - return + + return nil } // MatchingRepo returns an instance of Repo which matches a given repoID. diff --git a/server/core/config/valid/global_cfg_test.go b/server/core/config/valid/global_cfg_test.go index f61c68ca7f..c034f5aaad 100644 --- a/server/core/config/valid/global_cfg_test.go +++ b/server/core/config/valid/global_cfg_test.go @@ -386,7 +386,7 @@ func TestGlobalCfg_ValidateRepoCfg(t *testing.T) { }, }, repoID: "github.com/owner/repo", - expErr: "workflow 'forbidden' is not allowed for this repo", + expErr: "workflow \"forbidden\" is not allowed for this repo", }, "repo uses workflow that is defined server side but not allowed (without custom workflows)": { gCfg: valid.GlobalCfg{ @@ -419,7 +419,7 @@ func TestGlobalCfg_ValidateRepoCfg(t *testing.T) { }, }, repoID: "github.com/owner/repo", - expErr: "workflow 'forbidden' is not allowed for this repo", + expErr: "workflow \"forbidden\" is not allowed for this repo", }, "repo uses workflow that is defined in both places with same name (without custom workflows)": { gCfg: valid.GlobalCfg{ @@ -1024,28 +1024,21 @@ repos: tmp, cleanup := TempDir(t) defer cleanup() var global valid.GlobalCfg + globalCfgArgs := valid.GlobalCfgArgs{ + AllowRepoCfg: false, + MergeableReq: false, + ApprovedReq: false, + UnDivergedReq: false, + PlatformModeEnabled: c.platformMode, + } + if c.gCfg != "" { path := filepath.Join(tmp, "config.yaml") Ok(t, ioutil.WriteFile(path, []byte(c.gCfg), 0600)) var err error - globalCfgArgs := valid.GlobalCfgArgs{ - AllowRepoCfg: false, - MergeableReq: false, - ApprovedReq: false, - UnDivergedReq: false, - PlatformModeEnabled: c.platformMode, - } - global, err = (&config.ParserValidator{}).ParseGlobalCfg(path, valid.NewGlobalCfgFromArgs(globalCfgArgs)) Ok(t, err) } else { - globalCfgArgs := valid.GlobalCfgArgs{ - AllowRepoCfg: false, - MergeableReq: false, - ApprovedReq: false, - UnDivergedReq: false, - PlatformModeEnabled: c.platformMode, - } global = valid.NewGlobalCfgFromArgs(globalCfgArgs) } diff --git a/server/core/config/valid/project.go b/server/core/config/valid/project.go new file mode 100644 index 0000000000..410c2dc99c --- /dev/null +++ b/server/core/config/valid/project.go @@ -0,0 +1,119 @@ +package valid + +import ( + "fmt" + + version "github.com/hashicorp/go-version" +) + +type workflowType string + +const ( + DefaultWorkflowType workflowType = "workflow" +) + +type Project struct { + Dir string + Workspace string + Name *string + WorkflowName *string + PullRequestWorkflowName *string + DeploymentWorkflowName *string + TerraformVersion *version.Version + Autoplan Autoplan + ApplyRequirements []string + DeleteSourceBranchOnMerge *bool + Tags map[string]string +} + +// GetName returns the name of the project or an empty string if there is no +// project name. +func (p Project) GetName() string { + if p.Name != nil { + return *p.Name + } + // TODO + // Upstream atlantis only requires project name to be set if there's more than one project + // with same dir and workspace. If a project name has not been set, we'll use the dir and + // workspace to build project key. + // Source: https://www.runatlantis.io/docs/repo-level-atlantis-yaml.html#reference + return "" +} + +func (p Project) ValidateAllowedOverrides(allowedOverrides []string) error { + if p.WorkflowName != nil && !sliceContains(allowedOverrides, WorkflowKey) { + return fmt.Errorf("repo config not allowed to set '%s' key: server-side config needs '%s: [%s]'", WorkflowKey, AllowedOverridesKey, WorkflowKey) + } + if p.ApplyRequirements != nil && !sliceContains(allowedOverrides, ApplyRequirementsKey) { + return fmt.Errorf("repo config not allowed to set '%s' key: server-side config needs '%s: [%s]'", ApplyRequirementsKey, AllowedOverridesKey, ApplyRequirementsKey) + } + if p.DeleteSourceBranchOnMerge != nil && !sliceContains(allowedOverrides, DeleteSourceBranchOnMergeKey) { + return fmt.Errorf("repo config not allowed to set '%s' key: server-side config needs '%s: [%s]'", DeleteSourceBranchOnMergeKey, AllowedOverridesKey, DeleteSourceBranchOnMergeKey) + } + + return nil +} + +func (p Project) getWorkflowName(workflowType workflowType) *string { + var name *string + switch workflowType { + case DefaultWorkflowType: + name = p.WorkflowName + } + return name +} + +func (p Project) ValidateWorkflow(repoWorkflows map[string]Workflow, globalWorkflows map[string]Workflow) error { + return p.validateWorkflowForType(DefaultWorkflowType, repoWorkflows, globalWorkflows) +} + +func (p Project) ValidateWorkflowAllowed(allowedWorkflows []string) error { + return p.validateWorkflowAllowedForType(DefaultWorkflowType, allowedWorkflows) +} + +func (p Project) validateWorkflowForType( + workflowType workflowType, + repoWorkflows map[string]Workflow, + globalWorkflows map[string]Workflow, +) error { + name := p.getWorkflowName(workflowType) + + if name != nil { + if !mapContains(repoWorkflows, *name) && !mapContains(globalWorkflows, *name) { + return fmt.Errorf("%s %q is not defined anywhere", workflowType, *name) + } + } + + return nil +} + +func (p Project) validateWorkflowAllowedForType( + workflowType workflowType, + allowedWorkflows []string, +) error { + name := p.getWorkflowName(workflowType) + + if name != nil { + if !sliceContains(allowedWorkflows, *name) { + return fmt.Errorf("%s %q is not allowed for this repo", workflowType, *name) + } + } + + return nil +} + +// helper function to check if string is in array +func sliceContains(slc []string, str string) bool { + for _, s := range slc { + if s == str { + return true + } + } + return false +} + +// helper function to check if map contains a key +func mapContains(m map[string]Workflow, key string) bool { + _, ok := m[key] + return ok +} diff --git a/server/core/config/valid/project_test.go b/server/core/config/valid/project_test.go new file mode 100644 index 0000000000..f1f6e296be --- /dev/null +++ b/server/core/config/valid/project_test.go @@ -0,0 +1,181 @@ +package valid_test + +import ( + "testing" + + "github.com/runatlantis/atlantis/server/core/config/valid" + . "github.com/runatlantis/atlantis/testing" +) + +func TestProject_ValidateWorkflow(t *testing.T) { + defaultWorklfow := valid.Workflow{ + Name: "default", + } + customWorkflow := valid.Workflow{ + Name: "custom", + } + undefinedWorkflowName := "undefined" + cases := map[string]struct { + repoWorflows map[string]valid.Workflow + globalWorkflows map[string]valid.Workflow + project valid.Project + expErr string + }{ + "project with repo workflows": { + repoWorflows: map[string]valid.Workflow{ + "custom": customWorkflow, + }, + project: valid.Project{ + WorkflowName: &customWorkflow.Name, + }, + }, + "failed validation with undefined workflow": { + repoWorflows: map[string]valid.Workflow{ + "custom": customWorkflow, + }, + globalWorkflows: map[string]valid.Workflow{ + "default": defaultWorklfow, + }, + project: valid.Project{ + WorkflowName: &undefinedWorkflowName, + }, + expErr: "workflow \"undefined\" is not defined anywhere", + }, + "workflow defined in global config": { + repoWorflows: map[string]valid.Workflow{ + "default": defaultWorklfow, + }, + globalWorkflows: map[string]valid.Workflow{ + "default": defaultWorklfow, + "custom": customWorkflow, + }, + project: valid.Project{ + WorkflowName: &customWorkflow.Name, + }, + }, + "missing workflow name is valid": { + repoWorflows: map[string]valid.Workflow{ + "default": defaultWorklfow, + }, + globalWorkflows: map[string]valid.Workflow{ + "default": defaultWorklfow, + "custom": customWorkflow, + }, + project: valid.Project{ + WorkflowName: nil, + }, + }, + } + + for name, c := range cases { + t.Run(name, func(t *testing.T) { + actErr := c.project.ValidateWorkflow(c.repoWorflows, c.globalWorkflows) + if c.expErr == "" { + Ok(t, actErr) + } else { + ErrEquals(t, c.expErr, actErr) + } + }) + } +} + +func TestProject_ValidateWorkflowAllowed(t *testing.T) { + undefinedWorkflowName := "undefined" + customWorkflowName := "custom" + + cases := map[string]struct { + allowedWorkflows []string + project valid.Project + expErr string + }{ + "failed validation with undefined workflow": { + allowedWorkflows: []string{"custom"}, + project: valid.Project{ + WorkflowName: &undefinedWorkflowName, + }, + expErr: "workflow \"undefined\" is not allowed for this repo", + }, + "workflow is allowed": { + allowedWorkflows: []string{"custom"}, + project: valid.Project{ + WorkflowName: &customWorkflowName, + }, + }, + "missing workflow name is valid": { + allowedWorkflows: []string{"custom"}, + project: valid.Project{ + WorkflowName: nil, + }, + }, + } + + for name, c := range cases { + t.Run(name, func(t *testing.T) { + actErr := c.project.ValidateWorkflowAllowed(c.allowedWorkflows) + if c.expErr == "" { + Ok(t, actErr) + } else { + ErrEquals(t, c.expErr, actErr) + } + }) + } +} +func TestProject_ValidateAllowedOverrides(t *testing.T) { + workflowName := "custom" + deleteSourceBranch := true + cases := map[string]struct { + allowedOverrides []string + overrideKey string + project valid.Project + expErr string + }{ + "workflow is not allowed override": { + allowedOverrides: []string{}, + project: valid.Project{ + WorkflowName: &workflowName, + }, + expErr: "repo config not allowed to set 'workflow' key: server-side config needs 'allowed_overrides: [workflow]'", + }, + "apply_requirements is not allowed override": { + allowedOverrides: []string{}, + project: valid.Project{ + ApplyRequirements: []string{"mergeable"}, + }, + expErr: "repo config not allowed to set 'apply_requirements' key: server-side config needs 'allowed_overrides: [apply_requirements]'", + }, + "delete_source_branch_on_merge is not allowed override": { + allowedOverrides: []string{}, + project: valid.Project{ + DeleteSourceBranchOnMerge: &deleteSourceBranch, + }, + expErr: "repo config not allowed to set 'delete_source_branch_on_merge' key: server-side config needs 'allowed_overrides: [delete_source_branch_on_merge]'", + }, + "no errors when allowed override": { + allowedOverrides: []string{"apply_requirements", "workflow", "delete_source_branch_on_merge"}, + project: valid.Project{ + DeleteSourceBranchOnMerge: &deleteSourceBranch, + ApplyRequirements: []string{"mergeable"}, + WorkflowName: &workflowName, + }, + }, + "no errors if override attributes nil": { + allowedOverrides: []string{"apply_requirements", "workflow", "delete_source_branch_on_merge"}, + project: valid.Project{ + DeleteSourceBranchOnMerge: nil, + ApplyRequirements: nil, + WorkflowName: nil, + }, + }, + } + + for name, c := range cases { + t.Run(name, func(t *testing.T) { + actErr := c.project.ValidateAllowedOverrides(c.allowedOverrides) + if c.expErr == "" { + Ok(t, actErr) + } else { + ErrEquals(t, c.expErr, actErr) + } + }) + } +} diff --git a/server/core/config/valid/repo_cfg.go b/server/core/config/valid/repo_cfg.go index 0ee68b03e3..3846096d65 100644 --- a/server/core/config/valid/repo_cfg.go +++ b/server/core/config/valid/repo_cfg.go @@ -6,8 +6,6 @@ import ( "fmt" "regexp" "strings" - - version "github.com/hashicorp/go-version" ) // RepoCfg is the atlantis.yaml config after it's been parsed and validated. @@ -70,6 +68,18 @@ func (r RepoCfg) FindProjectsByName(name string) []Project { return ps } +// ValidateAllowedOverrides iterates over projects and checks that each project +// is only using allowed overrides defined in global config +func (r RepoCfg) ValidateAllowedOverrides(allowedOverrides []string) error { + for _, p := range r.Projects { + if err := p.ValidateAllowedOverrides(allowedOverrides); err != nil { + return err + } + } + + return nil +} + // validateWorkspaceAllowed returns an error if repoCfg defines projects in // repoRelDir but none of them use workspace. We want this to be an error // because if users have gone to the trouble of defining projects in repoRelDir @@ -100,30 +110,39 @@ func (r RepoCfg) ValidateWorkspaceAllowed(repoRelDir string, workspace string) e ) } -type Project struct { - Dir string - Workspace string - Name *string - WorkflowName *string - TerraformVersion *version.Version - Autoplan Autoplan - ApplyRequirements []string - DeleteSourceBranchOnMerge *bool - Tags map[string]string -} +// ValidateWorkflows ensures that all projects with custom workflow +// names exists either on repo level config or server level config +// Additionally it validates that workflow is allowed to be defined +func (r RepoCfg) ValidateWorkflows( + globalWorkflows map[string]Workflow, + allowedWorkflows []string, + allowCustomWorkflows bool, +) error { + // Check if the repo has set a workflow name that doesn't exist. + for _, p := range r.Projects { + if err := p.ValidateWorkflow(r.Workflows, globalWorkflows); err != nil { + return err + } + } -// GetName returns the name of the project or an empty string if there is no -// project name. -func (p Project) GetName() string { - if p.Name != nil { - return *p.Name + if len(allowedWorkflows) == 0 { + return nil } - // TODO - // Upstream atlantis only requires project name to be set if there's more than one project - // with same dir and workspace. If a project name has not been set, we'll use the dir and - // workspace to build project key. - // Source: https://www.runatlantis.io/docs/repo-level-atlantis-yaml.html#reference - return "" + + // Check workflow is allowed + for _, p := range r.Projects { + if allowCustomWorkflows { + if err := p.ValidateWorkflow(r.Workflows, map[string]Workflow{}); err == nil { + break + } + } + + if err := p.ValidateWorkflowAllowed(allowedWorkflows); err != nil { + return err + } + } + + return nil } type Autoplan struct {