Merge pull request #565 from runatlantis/fix-apply-all-proj

Fix apply all with project name bug

server/events/pending_plan_finder.go

@@ -1,6 +1,7 @@
 package events
 
 import (
+	"github.com/runatlantis/atlantis/server/events/runtime"
 	"io/ioutil"
 	"os"
 	"os/exec"
@@ -29,7 +30,8 @@ type PendingPlan struct {
 	// the plan is for.
 	RepoRelDir string
 	// Workspace is the workspace this plan should execute in.
-	Workspace string
+	Workspace   string
+	ProjectName string
 }
 
 // Find finds all pending plans in pullDir. pullDir should be the working
@@ -67,11 +69,15 @@ func (p *DefaultPendingPlanFinder) findWithAbsPaths(pullDir string) ([]PendingPl
 					continue
 				}
 
-				repoRelDir := filepath.Dir(file)
+				projectName, err := runtime.ProjectNameFromPlanfile(workspace, filepath.Base(file))
+				if err != nil {
+					return nil, nil, err
+				}
 				plans = append(plans, PendingPlan{
-					RepoDir:    repoDir,
-					RepoRelDir: repoRelDir,
-					Workspace:  workspace,
+					RepoDir:     repoDir,
+					RepoRelDir:  filepath.Dir(file),
+					Workspace:   workspace,
+					ProjectName: projectName,
 				})
 				absPaths = append(absPaths, filepath.Join(repoDir, file))
 			}

server/events/pending_plan_finder_test.go

@@ -54,9 +54,26 @@ func TestPendingPlanFinder_Find(t *testing.T) {
 			},
 			[]events.PendingPlan{
 				{
-					RepoDir:    "???/default",
-					RepoRelDir: ".",
-					Workspace:  "default",
+					RepoDir:     "???/default",
+					RepoRelDir:  ".",
+					Workspace:   "default",
+					ProjectName: "projectname",
+				},
+			},
+		},
+		{
+			"root dir project plan with slashes",
+			map[string]interface{}{
+				"default": map[string]interface{}{
+					"project::name-default.tfplan": nil,
+				},
+			},
+			[]events.PendingPlan{
+				{
+					RepoDir:     "???/default",
+					RepoRelDir:  ".",
+					Workspace:   "default",
+					ProjectName: "project/name",
 				},
 			},
 		},

server/events/project_command_builder.go

@@ -206,7 +206,7 @@ func (p *DefaultProjectCommandBuilder) buildApplyAllCommands(ctx *CommandContext
 
 	var cmds []models.ProjectCommandContext
 	for _, plan := range plans {
-		cmd, err := p.buildProjectCommandCtx(ctx, models.ApplyCommand, commentCmd.ProjectName, commentCmd.Flags, plan.RepoDir, plan.RepoRelDir, plan.Workspace, commentCmd.Verbose)
+		cmd, err := p.buildProjectCommandCtx(ctx, models.ApplyCommand, plan.ProjectName, commentCmd.Flags, plan.RepoDir, plan.RepoRelDir, plan.Workspace, commentCmd.Verbose)
 		if err != nil {
 			return nil, errors.Wrapf(err, "building command for dir %q", plan.RepoRelDir)
 		}

server/events/runtime/runtime.go

@@ -4,17 +4,22 @@ package runtime
 
 import (
 	"fmt"
+	"github.com/pkg/errors"
 	"regexp"
+	"strings"
 
-	version "github.com/hashicorp/go-version"
+	"github.com/hashicorp/go-version"
 	"github.com/runatlantis/atlantis/server/events/models"
 	"github.com/runatlantis/atlantis/server/events/terraform"
 	"github.com/runatlantis/atlantis/server/logging"
 )
 
-// lineBeforeRunURL is the line output during a remote operation right before
-// a link to the run url will be output.
-const lineBeforeRunURL = "To view this run in a browser, visit:"
+const (
+	// lineBeforeRunURL is the line output during a remote operation right before
+	// a link to the run url will be output.
+	lineBeforeRunURL     = "To view this run in a browser, visit:"
+	planfileSlashReplace = "::"
+)
 
 // TerraformExec brings the interface from TerraformClient into this package
 // without causing circular imports.
@@ -51,19 +56,28 @@ func MustConstraint(constraint string) version.Constraints {
 	return c
 }
 
-// invalidFilenameChars matches chars that are invalid for linux and windows
-// filenames.
-// From https://www.oreilly.com/library/view/regular-expressions-cookbook/9781449327453/ch08s25.html
-var invalidFilenameChars = regexp.MustCompile(`[\\/:"*?<>|]`)
-
 // GetPlanFilename returns the filename (not the path) of the generated tf plan
 // given a workspace and project name.
 func GetPlanFilename(workspace string, projName string) string {
-	var unescapedFilename string
 	if projName == "" {
-		unescapedFilename = fmt.Sprintf("%s.tfplan", workspace)
-	} else {
-		unescapedFilename = fmt.Sprintf("%s-%s.tfplan", projName, workspace)
+		return fmt.Sprintf("%s.tfplan", workspace)
+	}
+	projName = strings.Replace(projName, "/", planfileSlashReplace, -1)
+	return fmt.Sprintf("%s-%s.tfplan", projName, workspace)
+}
+
+// ProjectNameFromPlanfile returns the project name that a planfile with name
+// filename is for. If filename is for a project without a name then it will
+// return an empty string. workspace is the workspace this project is in.
+func ProjectNameFromPlanfile(workspace string, filename string) (string, error) {
+	r, err := regexp.Compile(fmt.Sprintf(`(.*?)-%s\.tfplan`, workspace))
+	if err != nil {
+		return "", errors.Wrap(err, "compiling project name regex, this is a bug")
+	}
+	projMatch := r.FindAllStringSubmatch(filename, 1)
+	if projMatch == nil {
+		return "", nil
 	}
-	return invalidFilenameChars.ReplaceAllLiteralString(unescapedFilename, "-")
+	rawProjName := projMatch[0][1]
+	return strings.Replace(rawProjName, planfileSlashReplace, "/", -1), nil
 }

server/events/runtime/runtime_test.go

@@ -19,11 +19,6 @@ func TestGetPlanFilename(t *testing.T) {
 			"",
 			"workspace.tfplan",
 		},
-		{
-			"workspace",
-			"",
-			"workspace.tfplan",
-		},
 		{
 			"workspace",
 			"project",
@@ -32,7 +27,7 @@ func TestGetPlanFilename(t *testing.T) {
 		{
 			"workspace",
 			"project/with/slash",
-			"project-with-slash-workspace.tfplan",
+			"project::with::slash-workspace.tfplan",
 		},
 		{
 			"workspace",
@@ -44,10 +39,14 @@ func TestGetPlanFilename(t *testing.T) {
 			"project😀",
 			"project😀-workspace😀.tfplan",
 		},
+		// Previously we replaced invalid chars with -'s, however we now
+		// rely on validation of the atlantis.yaml file to ensure the name's
+		// don't contain chars that need to be url encoded. So now these
+		// chars shouldn't get replaced.
 		{
 			"default",
 			`all.invalid.chars \/"*?<>`,
-			"all.invalid.chars --------default.tfplan",
+			"all.invalid.chars \\::\"*?<>-default.tfplan",
 		},
 	}
 
@@ -57,3 +56,40 @@ func TestGetPlanFilename(t *testing.T) {
 		})
 	}
 }
+
+func TestProjectNameFromPlanfile(t *testing.T) {
+	cases := []struct {
+		workspace string
+		filename  string
+		exp       string
+	}{
+		{
+			"workspace",
+			"workspace.tfplan",
+			"",
+		},
+		{
+			"workspace",
+			"project-workspace.tfplan",
+			"project",
+		},
+		{
+			"workspace",
+			"project-workspace-workspace.tfplan",
+			"project-workspace",
+		},
+		{
+			"workspace",
+			"project::with::slashes::-workspace.tfplan",
+			"project/with/slashes/",
+		},
+	}
+
+	for i, c := range cases {
+		t.Run(fmt.Sprintf("case %d", i), func(t *testing.T) {
+			act, err := runtime.ProjectNameFromPlanfile(c.workspace, c.filename)
+			Ok(t, err)
+			Equals(t, c.exp, act)
+		})
+	}
+}

server/events/yaml/valid/repo_cfg.go

@@ -13,10 +13,10 @@ type RepoCfg struct {
 	Automerge bool
 }
 
-func (r RepoCfg) FindProjectsByDirWorkspace(dir string, workspace string) []Project {
+func (r RepoCfg) FindProjectsByDirWorkspace(repoRelDir string, workspace string) []Project {
 	var ps []Project
 	for _, p := range r.Projects {
-		if p.Dir == dir && p.Workspace == workspace {
+		if p.Dir == repoRelDir && p.Workspace == workspace {
 			ps = append(ps, p)
 		}
 	}