From 609bf84b9a40b1d8d3a54fef20fb60aa02db85eb Mon Sep 17 00:00:00 2001 From: Luke Kysow <1034429+lkysow@users.noreply.github.com> Date: Tue, 2 Apr 2019 15:37:33 -0300 Subject: [PATCH] Fix apply all with project name bug Previously, if you were using project names and you had two projects with the same directory and workspace but different workflows, running "atlantis apply" would fail. This was because we weren't figuring out what the project name was for each pending plan. This change fixes this by extracting the project name from the planfile. We also handle /'s in the project name by substituting them with '::'. This allows us to convert them back to /'s when we're figuring out the project names from the filenames. It's safe to do this because : isn't an allowed character for project names. We also remove a final substitution of invalid filename characters because we rely on the validation of the project names instead which happens when validating the atlantis.yaml file. --- server/events/pending_plan_finder.go | 16 +++++--- server/events/pending_plan_finder_test.go | 23 +++++++++-- server/events/project_command_builder.go | 2 +- server/events/runtime/runtime.go | 42 ++++++++++++------- server/events/runtime/runtime_test.go | 50 +++++++++++++++++++---- server/events/yaml/valid/repo_cfg.go | 4 +- 6 files changed, 105 insertions(+), 32 deletions(-) diff --git a/server/events/pending_plan_finder.go b/server/events/pending_plan_finder.go index 6818c4bbe1..7337219959 100644 --- a/server/events/pending_plan_finder.go +++ b/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)) } diff --git a/server/events/pending_plan_finder_test.go b/server/events/pending_plan_finder_test.go index 6fb86b29a6..07e6fd2edc 100644 --- a/server/events/pending_plan_finder_test.go +++ b/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", }, }, }, diff --git a/server/events/project_command_builder.go b/server/events/project_command_builder.go index a842861df9..4cf68f395a 100644 --- a/server/events/project_command_builder.go +++ b/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) } diff --git a/server/events/runtime/runtime.go b/server/events/runtime/runtime.go index 830ea9bf21..12567aa408 100644 --- a/server/events/runtime/runtime.go +++ b/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 } diff --git a/server/events/runtime/runtime_test.go b/server/events/runtime/runtime_test.go index 2f6a5e6fdf..6c840fa6a8 100644 --- a/server/events/runtime/runtime_test.go +++ b/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) + }) + } +} diff --git a/server/events/yaml/valid/repo_cfg.go b/server/events/yaml/valid/repo_cfg.go index 3767618077..e5aaa4c136 100644 --- a/server/events/yaml/valid/repo_cfg.go +++ b/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) } }