Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug with top-level modules/ directory. #13

Merged
merged 1 commit into from
Feb 21, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion server/events/apply_executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func (a *ApplyExecutor) Execute(ctx *CommandContext) CommandResponse {
}
ctx.Log.Info("found %d plan(s) in our workspace: %v", len(plans), paths)

results := []ProjectResult{}
var results []ProjectResult
for _, plan := range plans {
ctx.Log.Info("running apply for project at path %q", plan.Project.Path)
result := a.apply(ctx, repoDir, plan)
Expand Down
28 changes: 16 additions & 12 deletions server/events/mocks/mock_project_finder.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ func NewMockProjectFinder() *MockProjectFinder {
return &MockProjectFinder{fail: pegomock.GlobalFailHandler}
}

func (mock *MockProjectFinder) FindModified(log *logging.SimpleLogger, modifiedFiles []string, repoFullName string) []models.Project {
params := []pegomock.Param{log, modifiedFiles, repoFullName}
result := pegomock.GetGenericMockFrom(mock).Invoke("FindModified", params, []reflect.Type{reflect.TypeOf((*[]models.Project)(nil)).Elem()})
func (mock *MockProjectFinder) DetermineProjects(log *logging.SimpleLogger, modifiedFiles []string, repoFullName string, repoDir string) []models.Project {
params := []pegomock.Param{log, modifiedFiles, repoFullName, repoDir}
result := pegomock.GetGenericMockFrom(mock).Invoke("DetermineProjects", params, []reflect.Type{reflect.TypeOf((*[]models.Project)(nil)).Elem()})
var ret0 []models.Project
if len(result) != 0 {
if result[0] != nil {
Expand Down Expand Up @@ -49,23 +49,23 @@ type VerifierProjectFinder struct {
inOrderContext *pegomock.InOrderContext
}

func (verifier *VerifierProjectFinder) FindModified(log *logging.SimpleLogger, modifiedFiles []string, repoFullName string) *ProjectFinder_FindModified_OngoingVerification {
params := []pegomock.Param{log, modifiedFiles, repoFullName}
methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "FindModified", params)
return &ProjectFinder_FindModified_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations}
func (verifier *VerifierProjectFinder) DetermineProjects(log *logging.SimpleLogger, modifiedFiles []string, repoFullName string, repoDir string) *ProjectFinder_DetermineProjects_OngoingVerification {
params := []pegomock.Param{log, modifiedFiles, repoFullName, repoDir}
methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "DetermineProjects", params)
return &ProjectFinder_DetermineProjects_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations}
}

type ProjectFinder_FindModified_OngoingVerification struct {
type ProjectFinder_DetermineProjects_OngoingVerification struct {
mock *MockProjectFinder
methodInvocations []pegomock.MethodInvocation
}

func (c *ProjectFinder_FindModified_OngoingVerification) GetCapturedArguments() (*logging.SimpleLogger, []string, string) {
log, modifiedFiles, repoFullName := c.GetAllCapturedArguments()
return log[len(log)-1], modifiedFiles[len(modifiedFiles)-1], repoFullName[len(repoFullName)-1]
func (c *ProjectFinder_DetermineProjects_OngoingVerification) GetCapturedArguments() (*logging.SimpleLogger, []string, string, string) {
log, modifiedFiles, repoFullName, repoDir := c.GetAllCapturedArguments()
return log[len(log)-1], modifiedFiles[len(modifiedFiles)-1], repoFullName[len(repoFullName)-1], repoDir[len(repoDir)-1]
}

func (c *ProjectFinder_FindModified_OngoingVerification) GetAllCapturedArguments() (_param0 []*logging.SimpleLogger, _param1 [][]string, _param2 []string) {
func (c *ProjectFinder_DetermineProjects_OngoingVerification) GetAllCapturedArguments() (_param0 []*logging.SimpleLogger, _param1 [][]string, _param2 []string, _param3 []string) {
params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations)
if len(params) > 0 {
_param0 = make([]*logging.SimpleLogger, len(params[0]))
Expand All @@ -80,6 +80,10 @@ func (c *ProjectFinder_FindModified_OngoingVerification) GetAllCapturedArguments
for u, param := range params[2] {
_param2[u] = param.(string)
}
_param3 = make([]string, len(params[3]))
for u, param := range params[3] {
_param3[u] = param.(string)
}
}
return
}
11 changes: 6 additions & 5 deletions server/events/plan_executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,17 +57,18 @@ func (p *PlanExecutor) Execute(ctx *CommandContext) CommandResponse {
if err != nil {
return CommandResponse{Error: errors.Wrap(err, "getting modified files")}
}
ctx.Log.Info("found %d files modified in this pull request", len(modifiedFiles))
projects := p.ProjectFinder.FindModified(ctx.Log, modifiedFiles, ctx.BaseRepo.FullName)
if len(projects) == 0 {
return CommandResponse{Failure: "No Terraform files were modified."}
}

cloneDir, err := p.Workspace.Clone(ctx.Log, ctx.BaseRepo, ctx.HeadRepo, ctx.Pull, ctx.Command.Workspace)
if err != nil {
return CommandResponse{Error: err}
}

ctx.Log.Info("found %d files modified in this pull request", len(modifiedFiles))
projects := p.ProjectFinder.DetermineProjects(ctx.Log, modifiedFiles, ctx.BaseRepo.FullName, cloneDir)
if len(projects) == 0 {
return CommandResponse{Failure: "No Terraform files were modified."}
}

var results []ProjectResult
for _, project := range projects {
ctx.Log.Info("running plan for project at path %q", project.Path)
Expand Down
84 changes: 67 additions & 17 deletions server/events/project_finder.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package events

import (
"os"
"path"
"path/filepath"
"strings"

"github.com/runatlantis/atlantis/server/events/models"
Expand All @@ -12,19 +14,19 @@ import (

// ProjectFinder determines what are the terraform project(s) within a repo.
type ProjectFinder interface {
// FindModified returns the list of projects that were modified based on
// DetermineProjects returns the list of projects that were modified based on
// the modifiedFiles. The list will be de-duplicated.
FindModified(log *logging.SimpleLogger, modifiedFiles []string, repoFullName string) []models.Project
DetermineProjects(log *logging.SimpleLogger, modifiedFiles []string, repoFullName string, repoDir string) []models.Project
}

// DefaultProjectFinder implements ProjectFinder.
type DefaultProjectFinder struct{}

var excludeList = []string{"terraform.tfstate", "terraform.tfstate.backup"}

// FindModified returns the list of projects that were modified based on
// DetermineProjects returns the list of projects that were modified based on
// the modifiedFiles. The list will be de-duplicated.
func (p *DefaultProjectFinder) FindModified(log *logging.SimpleLogger, modifiedFiles []string, repoFullName string) []models.Project {
func (p *DefaultProjectFinder) DetermineProjects(log *logging.SimpleLogger, modifiedFiles []string, repoFullName string, repoDir string) []models.Project {
var projects []models.Project

modifiedTerraformFiles := p.filterToTerraform(modifiedFiles)
Expand All @@ -36,7 +38,10 @@ func (p *DefaultProjectFinder) FindModified(log *logging.SimpleLogger, modifiedF

var paths []string
for _, modifiedFile := range modifiedTerraformFiles {
paths = append(paths, p.getProjectPath(modifiedFile))
projectPath := p.getProjectPath(modifiedFile, repoDir)
if projectPath != "" {
paths = append(paths, projectPath)
}
}
uniquePaths := p.unique(paths)
for _, uniquePath := range uniquePaths {
Expand Down Expand Up @@ -66,23 +71,68 @@ func (p *DefaultProjectFinder) isInExcludeList(fileName string) bool {
return false
}

// getProjectPath returns the path to the project relative to the repo root
// if the project is at the root returns ".".
func (p *DefaultProjectFinder) getProjectPath(modifiedFilePath string) string {
// getProjectPath attempts to determine based on the location of a modified
// file, where the root of the Terraform project is. It also attempts to verify
// if the root is valid by looking for a main.tf file. It returns a relative
// path. If the project is at the root returns ".". If modified file doesn't
// lead to a valid project path, returns an empty string.
func (p *DefaultProjectFinder) getProjectPath(modifiedFilePath string, repoDir string) string {
dir := path.Dir(modifiedFilePath)
if path.Base(dir) == "env" {
// If the modified file was inside an env/ directory, we treat this
// specially and run plan one level up.
// specially and run plan one level up. This supports directory structures
// like:
// root/
// main.tf
// env/
// dev.tfvars
// staging.tfvars
return path.Dir(dir)
}
// Need to add a trailing slash before splitting on modules/ because if
// the input was modules/file.tf then path.Dir will be "modules" and so our
// split on "modules/" will fail.
dirWithTrailingSlash := dir + "/"
// It's safe to do this split even if there is no modules/ in the path
// because SplitN will just return the original string.
modulesSplit := strings.SplitN(dirWithTrailingSlash, "modules/", 2)
return path.Clean(modulesSplit[0])

// Surrounding dir with /'s so we can match on /modules/ even if dir is
// "modules" or "project1/modules"
if strings.Contains("/"+dir+"/", "/modules/") {
// We treat changes inside modules/ folders specially. There are two cases:
// 1. modules folder inside project:
// root/
// main.tf
// modules/
// ...
// In this case, if we detect a change in modules/, we will determine
// the project root to be at root/.
//
// 2. shared top-level modules folder
// root/
// project1/
// main.tf # uses modules via ../modules
// project2/
// main.tf # uses modules via ../modules
// modules/
// ...
// In this case, if we detect a change in modules/ we don't know which
// project was using this module so we can't suggest a project root, but we
// also detect that there's no main.tf in the parent folder of modules/
// so we won't suggest that as a project. So in this case we return nothing.
// The code below makes this happen.

// Need to add a trailing slash before splitting on modules/ because if
// the input was modules/file.tf then path.Dir will be "modules" and so our
// split on "modules/" will fail.
dirWithTrailingSlash := dir + "/"
modulesSplit := strings.SplitN(dirWithTrailingSlash, "modules/", 2)
modulesParent := modulesSplit[0]

// Now we check whether there is a main.tf in the parent.
if _, err := os.Stat(filepath.Join(repoDir, modulesParent, "main.tf")); os.IsNotExist(err) {
return ""
}
return path.Clean(modulesParent)
}

// If it wasn't a modules directory, we assume we're in a project and return
// this directory.
return dir
}

func (p *DefaultProjectFinder) unique(strs []string) []string {
Expand Down
92 changes: 83 additions & 9 deletions server/events/project_finder_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package events_test

import (
"io/ioutil"
"os"
"path/filepath"
"testing"

"github.com/runatlantis/atlantis/server/events"
Expand All @@ -11,72 +14,143 @@ import (
var noopLogger = logging.NewNoopLogger()
var modifiedRepo = "owner/repo"
var m = events.DefaultProjectFinder{}
var nestedModules1 string
var nestedModules2 string
var topLevelModules string

func setupTmpRepos(t *testing.T) {
// Create different repo structures for testing.

// 1. Nested modules directory inside a project
// project1/
// main.tf
// modules/
// main.tf
var err error
nestedModules1, err = ioutil.TempDir("", "")
Ok(t, err)
err = os.MkdirAll(filepath.Join(nestedModules1, "project1/modules"), 0700)
Ok(t, err)
_, err = os.Create(filepath.Join(nestedModules1, "project1/main.tf"))
Ok(t, err)
_, err = os.Create(filepath.Join(nestedModules1, "project1/modules/main.tf"))
Ok(t, err)

// 2. Nested modules dir inside top-level project
// main.tf
// modules/
// main.tf
// We can just re-use part of the previous dir structure.
nestedModules2 = filepath.Join(nestedModules1, "project1")

// 3. Top-level modules
// modules/
// main.tf
// project1/
// main.tf
// project2/
// main.tf
topLevelModules, err = ioutil.TempDir("", "")
Ok(t, err)
for _, path := range []string{"modules", "project1", "project2"} {
err = os.MkdirAll(filepath.Join(topLevelModules, path), 0700)
Ok(t, err)
_, err = os.Create(filepath.Join(topLevelModules, path, "main.tf"))
Ok(t, err)
}
}

func TestDetermineProjects(t *testing.T) {
setupTmpRepos(t)

func TestGetModified(t *testing.T) {
cases := []struct {
description string
files []string
expProjectPaths []string
repoDir string
}{
{
"If no files were modified then should return an empty list",
nil,
nil,
"",
},
{
"Should ignore non .tf files and return an empty list",
[]string{"non-tf"},
nil,
"",
},
{
"Should plan in the parent directory from modules if that dir has a main.tf",
[]string{"project1/modules/main.tf"},
[]string{"project1"},
nestedModules1,
},
{
"Should plan in the parent directory from modules",
[]string{"modules/file.tf"},
"Should plan in the parent directory from modules if that dir has a main.tf",
[]string{"modules/main.tf"},
[]string{"."},
nestedModules2,
},
{
"Should plan in the parent directory from modules when module is in a subdir",
[]string{"modules/subdir/file.tf"},
"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,
},
{
"Should not plan in the parent directory from modules if that dir does not have a main.tf",
[]string{"modules/main.tf"},
[]string{},
topLevelModules,
},
{
"Should plan in the parent directory from modules when project is in its own dir",
[]string{"projectdir/modules/file.tf"},
[]string{"projectdir"},
"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,
},
{
"Should ignore tfstate files and return an empty list",
[]string{"terraform.tfstate", "terraform.tfstate.backup", "parent/terraform.tfstate", "parent/terraform.tfstate.backup"},
nil,
"",
},
{
"Should ignore tfstate files and return an empty list",
[]string{"terraform.tfstate", "terraform.tfstate.backup", "parent/terraform.tfstate", "parent/terraform.tfstate.backup"},
nil,
"",
},
{
"Should return '.' when changed file is at root",
[]string{"a.tf"},
[]string{"."},
"",
},
{
"Should return directory when changed file is in a dir",
[]string{"parent/a.tf"},
[]string{"parent"},
"",
},
{
"Should return parent dir when changed file is in an env/ dir",
[]string{"env/a.tfvars"},
[]string{"."},
"",
},
{
"Should de-duplicate when multiple files changed in the same dir",
[]string{"root.tf", "env/env.tfvars", "parent/parent.tf", "parent/parent2.tf", "parent/child/child.tf", "parent/child/env/env.tfvars"},
[]string{".", "parent", "parent/child"},
"",
},
}
for _, c := range cases {
t.Log(c.description)
projects := m.FindModified(noopLogger, c.files, modifiedRepo)
projects := m.DetermineProjects(noopLogger, c.files, modifiedRepo, c.repoDir)

// Extract the paths from the projects. We use a slice here instead of a
// map so we can test whether there are duplicates returned.
Expand Down