From 91e0d480e3b844cd3c3b0914720d73c8ad97c2b3 Mon Sep 17 00:00:00 2001 From: Brian Zoetewey Date: Wed, 31 Mar 2021 11:51:41 -0400 Subject: [PATCH] Set default value for `--autoplan-file-list` and add `terragrunt.hcl` to the list. --- cmd/server.go | 14 +++- cmd/server_test.go | 47 +++++++++++++ runatlantis.io/docs/server-configuration.md | 3 +- server/events/project_command_builder.go | 2 +- .../project_command_builder_internal_test.go | 6 +- server/events/project_command_builder_test.go | 18 ++--- server/events/project_finder.go | 26 +++---- server/events/project_finder_test.go | 69 +++++-------------- 8 files changed, 103 insertions(+), 82 deletions(-) diff --git a/cmd/server.go b/cmd/server.go index 51a036e2f8..14c55e9f96 100644 --- a/cmd/server.go +++ b/cmd/server.go @@ -20,6 +20,7 @@ import ( "path/filepath" "strings" + "github.com/docker/docker/pkg/fileutils" homedir "github.com/mitchellh/go-homedir" "github.com/pkg/errors" "github.com/runatlantis/atlantis/server" @@ -102,6 +103,7 @@ const ( // NOTE: Must manually set these as defaults in the setDefaults function. DefaultADBasicUser = "" DefaultADBasicPassword = "" + DefaultAutoplanFileList = "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl" DefaultCheckoutStrategy = "branch" DefaultBitbucketBaseURL = bitbucketcloud.BaseURL DefaultDataDir = "~/.atlantis" @@ -140,9 +142,9 @@ var stringFlags = map[string]stringFlag{ AutoplanFileListFlag: { description: "Comma separated list of file patterns that Atlantis will use to check if a directory contains modified files that should trigger project planning." + " Patterns use the dockerignore (https://docs.docker.com/engine/reference/builder/#dockerignore-file) syntax." + - " Use single quotes to avoid shell expansion of '*'. Defaults to '**/*.tf,**/*.tfvars,**/*.tfvars.json'." + + " Use single quotes to avoid shell expansion of '*'. Defaults to '" + DefaultAutoplanFileList + "'." + " A custom Workflow that uses autoplan 'when_modified' will ignore this value.", - defaultValue: "", + defaultValue: DefaultAutoplanFileList, }, BitbucketUserFlag: { description: "Bitbucket username of API user.", @@ -572,6 +574,9 @@ func (s *ServerCmd) run() error { } func (s *ServerCmd) setDefaults(c *server.UserConfig) { + if c.AutoplanFileList == "" { + c.AutoplanFileList = DefaultAutoplanFileList + } if c.CheckoutStrategy == "" { c.CheckoutStrategy = DefaultCheckoutStrategy } @@ -689,6 +694,11 @@ func (s *ServerCmd) validate(userConfig server.UserConfig) error { return fmt.Errorf("if setting --%s, must set --%s", TFEHostnameFlag, TFETokenFlag) } + _, patternErr := fileutils.NewPatternMatcher(strings.Split(userConfig.AutoplanFileList, ",")) + if patternErr != nil { + return errors.Wrapf(patternErr, "invalid pattern in --%s, %s", AutoplanFileListFlag, userConfig.AutoplanFileList) + } + return nil } diff --git a/cmd/server_test.go b/cmd/server_test.go index 9990faea34..6af2043a74 100644 --- a/cmd/server_test.go +++ b/cmd/server_test.go @@ -737,6 +737,53 @@ func TestExecute_RepoWhitelistDeprecation(t *testing.T) { Equals(t, "*", passedConfig.RepoAllowlist) } +func TestExecute_AutoplanFileList(t *testing.T) { + cases := []struct { + description string + flags map[string]interface{} + expectErr string + }{ + { + "default value", + map[string]interface{}{ + AutoplanFileListFlag: DefaultAutoplanFileList, + }, + "", + }, + { + "valid value", + map[string]interface{}{ + AutoplanFileListFlag: "**/*.tf", + }, + "", + }, + { + "invalid exclusion pattern", + map[string]interface{}{ + AutoplanFileListFlag: "**/*.yml,!", + }, + "invalid pattern in --autoplan-file-list, **/*.yml,!: illegal exclusion pattern: \"!\"", + }, + { + "invalid pattern", + map[string]interface{}{ + AutoplanFileListFlag: "?[", + }, + "invalid pattern in --autoplan-file-list, ?[: syntax error in pattern", + }, + } + for _, testCase := range cases { + t.Log("Should validate autoplan file list when " + testCase.description) + c := setupWithDefaults(testCase.flags) + err := c.Execute() + if testCase.expectErr != "" { + ErrEquals(t, testCase.expectErr, err) + } else { + Ok(t, err) + } + } +} + func setup(flags map[string]interface{}) *cobra.Command { vipr := viper.New() for k, v := range flags { diff --git a/runatlantis.io/docs/server-configuration.md b/runatlantis.io/docs/server-configuration.md index ada8d9244f..cff2eb4272 100644 --- a/runatlantis.io/docs/server-configuration.md +++ b/runatlantis.io/docs/server-configuration.md @@ -121,7 +121,8 @@ Values are chosen in this order: * Accepts a comma separated list, ex. `pattern1,pattern2`. * Patterns use the [`.dockerignore` syntax](https://docs.docker.com/engine/reference/builder/#dockerignore-file) * List of file patterns will be used by both automatic and manually run plans. - * When not set, defaults to all `.tf`, `.tfvars` and `.tfvars.json` files (`--autoplan-file-list='**/*.tf,**/*.tfvars,**/*.tfvars.json'`). + * When not set, defaults to all `.tf`, `.tfvars`, `.tfvars.json` and `terragrunt.hcl` files + (`--autoplan-file-list='**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl'`). * Setting `--autoplan-file-list` will override the defaults. You **must** add `**/*.tf` and other defaults if you want to include them. * A custom [Workflow](repo-level-atlantis-yaml.html#configuring-planning) that uses autoplan `when_modified` will ignore this value. diff --git a/server/events/project_command_builder.go b/server/events/project_command_builder.go index 000b1fd346..4da9ccc17c 100644 --- a/server/events/project_command_builder.go +++ b/server/events/project_command_builder.go @@ -244,7 +244,7 @@ func (p *DefaultProjectCommandBuilder) buildPlanAllCommands(ctx *CommandContext, // If there is no config file, then we'll plan each project that // our algorithm determines was modified. ctx.Log.Info("found no %s file", yaml.AtlantisYAMLFilename) - modifiedProjects, err := p.ProjectFinder.DetermineProjects(ctx.Log, modifiedFiles, ctx.Pull.BaseRepo.FullName, repoDir, p.AutoplanFileList) + modifiedProjects := p.ProjectFinder.DetermineProjects(ctx.Log, modifiedFiles, ctx.Pull.BaseRepo.FullName, repoDir, p.AutoplanFileList) if err != nil { return nil, errors.Wrapf(err, "finding modified projects: %s", modifiedFiles) } diff --git a/server/events/project_command_builder_internal_test.go b/server/events/project_command_builder_internal_test.go index 1fe0c0d1cc..35c0fe232b 100644 --- a/server/events/project_command_builder_internal_test.go +++ b/server/events/project_command_builder_internal_test.go @@ -589,7 +589,7 @@ projects: &CommentParser{}, false, false, - "", + "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", ) // We run a test for each type of command. @@ -775,7 +775,7 @@ projects: &CommentParser{}, false, true, - "", + "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", ) // We run a test for each type of command, again specific projects @@ -979,7 +979,7 @@ workflows: &CommentParser{}, false, false, - "", + "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", ) cmd := models.PolicyCheckCommand diff --git a/server/events/project_command_builder_test.go b/server/events/project_command_builder_test.go index 13c2b6ac7b..6cecb186ee 100644 --- a/server/events/project_command_builder_test.go +++ b/server/events/project_command_builder_test.go @@ -146,7 +146,7 @@ projects: &events.CommentParser{}, false, false, - "", + "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", ) ctxs, err := builder.BuildAutoplanCommands(&events.CommandContext{ @@ -373,7 +373,7 @@ projects: &events.CommentParser{}, false, true, - "", + "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", ) var actCtxs []models.ProjectCommandContext @@ -511,7 +511,7 @@ projects: &events.CommentParser{}, false, false, - "", + "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", ) ctxs, err := builder.BuildPlanCommands( @@ -587,7 +587,7 @@ func TestDefaultProjectCommandBuilder_BuildMultiApply(t *testing.T) { &events.CommentParser{}, false, false, - "", + "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", ) ctxs, err := builder.BuildApplyCommands( @@ -658,7 +658,7 @@ projects: &events.CommentParser{}, false, false, - "", + "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", ) ctx := &events.CommandContext{ @@ -724,7 +724,7 @@ func TestDefaultProjectCommandBuilder_EscapeArgs(t *testing.T) { &events.CommentParser{}, false, false, - "", + "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", ) var actCtxs []models.ProjectCommandContext @@ -892,7 +892,7 @@ projects: &events.CommentParser{}, false, false, - "", + "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", ) actCtxs, err := builder.BuildPlanCommands( @@ -944,7 +944,7 @@ projects: &events.CommentParser{}, true, false, - "", + "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", ) var actCtxs []models.ProjectCommandContext @@ -986,7 +986,7 @@ func TestDefaultProjectCommandBuilder_WithPolicyCheckEnabled_BuildAutoplanComman &events.CommentParser{}, false, false, - "", + "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", ) ctxs, err := builder.BuildAutoplanCommands(&events.CommandContext{ diff --git a/server/events/project_finder.go b/server/events/project_finder.go index 46d806ccfc..aab5abeb8d 100644 --- a/server/events/project_finder.go +++ b/server/events/project_finder.go @@ -33,7 +33,7 @@ type ProjectFinder interface { // DetermineProjects returns the list of projects that were modified based on // the modifiedFiles. The list will be de-duplicated. // absRepoDir is the path to the cloned repo on disk. - DetermineProjects(log *logging.SimpleLogger, modifiedFiles []string, repoFullName string, absRepoDir string, autoplanFileList string) ([]models.Project, error) + DetermineProjects(log *logging.SimpleLogger, modifiedFiles []string, repoFullName string, absRepoDir string, autoplanFileList string) []models.Project // DetermineProjectsViaConfig returns the list of projects that were modified // based on modifiedFiles and the repo's config. // absRepoDir is the path to the cloned repo on disk. @@ -47,16 +47,12 @@ var ignoredFilenameFragments = []string{"terraform.tfstate", "terraform.tfstate. type DefaultProjectFinder struct{} // See ProjectFinder.DetermineProjects. -func (p *DefaultProjectFinder) DetermineProjects(log *logging.SimpleLogger, modifiedFiles []string, repoFullName string, absRepoDir string, autoplanFileList string) ([]models.Project, error) { +func (p *DefaultProjectFinder) DetermineProjects(log *logging.SimpleLogger, modifiedFiles []string, repoFullName string, absRepoDir string, autoplanFileList string) []models.Project { var projects []models.Project - modifiedTerraformFiles, err := p.filterToFileList(log, modifiedFiles, autoplanFileList) - if err != nil { - return nil, errors.Wrapf(err, "filtering modified files by autoplanFileList: %s", autoplanFileList) - } - + modifiedTerraformFiles := p.filterToFileList(log, modifiedFiles, autoplanFileList) if len(modifiedTerraformFiles) == 0 { - return projects, nil + return projects } log.Info("filtered modified files to %d .tf or terragrunt.hcl files: %v", len(modifiedTerraformFiles), modifiedTerraformFiles) @@ -81,7 +77,7 @@ func (p *DefaultProjectFinder) DetermineProjects(log *logging.SimpleLogger, modi } log.Info("there are %d modified project(s) at path(s): %v", len(projects), strings.Join(exists, ", ")) - return projects, nil + return projects } // See ProjectFinder.DetermineProjectsViaConfig. @@ -149,17 +145,15 @@ func (p *DefaultProjectFinder) DetermineProjectsViaConfig(log *logging.SimpleLog } // filterToFileList filters out files not included in the file list -func (p *DefaultProjectFinder) filterToFileList(log *logging.SimpleLogger, files []string, fileList string) ([]string, error) { +func (p *DefaultProjectFinder) filterToFileList(log *logging.SimpleLogger, files []string, fileList string) []string { var filtered []string if fileList == "" { fileList = "**/*.tf,**/*.tfvars,**/*.tfvars.json" } patterns := strings.Split(fileList, ",") - patternMatcher, err := fileutils.NewPatternMatcher(patterns) - if err != nil { - return nil, errors.Wrapf(err, "filtering modified files with patterns: %v", patterns) - } + // Ignore pattern matcher error here as it was checked for errors in server validation + patternMatcher, _ := fileutils.NewPatternMatcher(patterns) for _, fileName := range files { if p.shouldIgnore(fileName) { @@ -170,12 +164,12 @@ func (p *DefaultProjectFinder) filterToFileList(log *logging.SimpleLogger, files log.Debug("filter err for file %q: %s", fileName, err) continue } - if match || filepath.Base(fileName) == "terragrunt.hcl" { + if match { filtered = append(filtered, fileName) } } - return filtered, nil + return filtered } // shouldIgnore returns true if we shouldn't trigger a plan on changes to this file. diff --git a/server/events/project_finder_test.go b/server/events/project_finder_test.go index 2b79353cae..f46f6b1790 100644 --- a/server/events/project_finder_test.go +++ b/server/events/project_finder_test.go @@ -107,149 +107,126 @@ func setupTmpRepos(t *testing.T) { func TestDetermineProjects(t *testing.T) { setupTmpRepos(t) + defaultAutoplanFileList := "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl" + cases := []struct { description string files []string expProjectPaths []string repoDir string autoplanFileList string - expErr string }{ { "If no files were modified then should return an empty list", nil, nil, nestedModules1, - "", - "", + defaultAutoplanFileList, }, { "Should ignore non .tf files and return an empty list", []string{"non-tf", "non.tf.suffix"}, nil, nestedModules1, - "", - "", + defaultAutoplanFileList, }, { "Should ignore .tflint.hcl files and return an empty list", []string{".tflint.hcl", "project1/.tflint.hcl"}, nil, nestedModules1, - "", - "", + defaultAutoplanFileList, }, { "Should plan in the parent directory from modules if that dir has a main.tf", []string{"project1/modules/main.tf"}, []string{"project1"}, nestedModules1, - "", - "", + defaultAutoplanFileList, }, { "Should plan in the parent directory from modules if that dir has a main.tf", []string{"modules/main.tf"}, []string{"."}, nestedModules2, - "", - "", + defaultAutoplanFileList, }, { "Should plan in the parent directory from modules when module is in a subdir if that dir has a main.tf", []string{"modules/subdir/main.tf"}, []string{"."}, nestedModules2, - "", - "", + defaultAutoplanFileList, }, { "Should not plan in the parent directory from modules if that dir does not have a main.tf", []string{"modules/main.tf"}, []string{}, topLevelModules, - "", - "", + defaultAutoplanFileList, }, { "Should not plan in the parent directory from modules if that dir does not have a main.tf", []string{"modules/main.tf", "project1/main.tf"}, []string{"project1"}, topLevelModules, - "", - "", + defaultAutoplanFileList, }, { "Should ignore tfstate files and return an empty list", []string{"terraform.tfstate", "terraform.tfstate.backup", "parent/terraform.tfstate", "parent/terraform.tfstate.backup"}, nil, nestedModules1, - "", - "", + defaultAutoplanFileList, }, { "Should return '.' when changed file is at root", []string{"a.tf"}, []string{"."}, nestedModules2, - "", - "", + defaultAutoplanFileList, }, { "Should return directory when changed file is in a dir", []string{"project1/a.tf"}, []string{"project1"}, nestedModules1, - "", - "", + defaultAutoplanFileList, }, { "Should return parent dir when changed file is in an env/ dir", []string{"env/staging.tfvars"}, []string{"."}, envDir, - "", - "", + defaultAutoplanFileList, }, { "Should de-duplicate when multiple files changed in the same dir", []string{"env/staging.tfvars", "main.tf", "other.tf"}, []string{"."}, "", - "", - "", + defaultAutoplanFileList, }, { "Should ignore changes in a dir that was deleted", []string{"wasdeleted/main.tf"}, []string{}, "", - "", - "", + defaultAutoplanFileList, }, { "Should not ignore terragrunt.hcl files", []string{"terragrunt.hcl"}, []string{"."}, nestedModules2, - "", - "", + defaultAutoplanFileList, }, { "Should find terragrunt.hcl file inside a nested directory", []string{"project1/terragrunt.hcl"}, []string{"project1"}, nestedModules1, - "", - "", - }, - { - "Should error on invalid pattern", - []string{"project1/main.tf"}, - []string{"project1"}, - nestedModules1, - "*[]", - "filtering modified files by autoplanFileList: *[]: filtering modified files with patterns: [*[]]: syntax error in pattern", + defaultAutoplanFileList, }, { "Should find packer files and ignore default tf files", @@ -257,7 +234,6 @@ func TestDetermineProjects(t *testing.T) { []string{"project1"}, topLevelModules, "**/*.pkr.hcl", - "", }, { "Should find yaml files in addition to defaults", @@ -265,7 +241,6 @@ func TestDetermineProjects(t *testing.T) { []string{"project1", "project2"}, topLevelModules, "**/*.tf,**/*.yml", - "", }, { "Should find yaml files unless excluded", @@ -273,17 +248,11 @@ func TestDetermineProjects(t *testing.T) { []string{"project1"}, topLevelModules, "**/*.yml,!project2/*.yml", - "", }, } for _, c := range cases { t.Run(c.description, func(t *testing.T) { - projects, err := m.DetermineProjects(noopLogger, c.files, modifiedRepo, c.repoDir, c.autoplanFileList) - if c.expErr != "" { - ErrEquals(t, c.expErr, err) - return - } - Ok(t, err) + projects := m.DetermineProjects(noopLogger, c.files, modifiedRepo, c.repoDir, c.autoplanFileList) // Extract the paths from the projects. We use a slice here instead of a // map so we can test whether there are duplicates returned.