Skip to content

Commit

Permalink
Remove a positional dir parameter from TerraformCLI.Plan()
Browse files Browse the repository at this point in the history
  • Loading branch information
minamijoyo committed Oct 21, 2021
1 parent 8b9521b commit e5e3e8d
Show file tree
Hide file tree
Showing 13 changed files with 38 additions and 76 deletions.
8 changes: 4 additions & 4 deletions tfexec/terraform.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ type TerraformCLI interface {

// Plan computes expected changes.
// If a state is given, use it for the input state.
Plan(ctx context.Context, state *State, dir string, opts ...string) (*Plan, error)
Plan(ctx context.Context, state *State, opts ...string) (*Plan, error)

// Apply applies changes.
// If a plan is given, use it for the input plan.
Expand Down Expand Up @@ -129,7 +129,7 @@ type TerraformCLI interface {
OverrideBackendToLocal(ctx context.Context, filename string, workspace string) (func(), error)

// PlanHasChange is a helper method which runs plan and return true if the plan has change.
PlanHasChange(ctx context.Context, state *State, dir string, opts ...string) (bool, error)
PlanHasChange(ctx context.Context, state *State, opts ...string) (bool, error)
}

// terraformCLI implements the TerraformCLI interface.
Expand Down Expand Up @@ -265,10 +265,10 @@ terraform {
}

// PlanHasChange is a helper method which runs plan and return true only if the plan has change.
func (c *terraformCLI) PlanHasChange(ctx context.Context, state *State, dir string, opts ...string) (bool, error) {
func (c *terraformCLI) PlanHasChange(ctx context.Context, state *State, opts ...string) (bool, error) {

merged := mergeOptions(opts, []string{"-input=false", "-no-color", "-detailed-exitcode"})
_, err := c.Plan(ctx, state, dir, merged...)
_, err := c.Plan(ctx, state, merged...)
if err != nil {
if exitErr, ok := err.(ExitError); ok && exitErr.ExitCode() == 2 {
return true, nil
Expand Down
2 changes: 1 addition & 1 deletion tfexec/terraform_apply_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ func TestAccTerraformCLIApply(t *testing.T) {
t.Fatalf("failed to run terraform init: %s", err)
}

plan, err := terraformCLI.Plan(context.Background(), nil, "", "-input=false", "-no-color")
plan, err := terraformCLI.Plan(context.Background(), nil, "-input=false", "-no-color")
if err != nil {
t.Fatalf("failed to run terraform plan: %s", err)
}
Expand Down
4 changes: 2 additions & 2 deletions tfexec/terraform_import_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ func TestAccTerraformCLIImport(t *testing.T) {
t.Fatalf("failed to run terraform init: %s", err)
}

_, err = terraformCLI.Plan(context.Background(), nil, "", "-input=false", "-no-color", "-detailed-exitcode")
_, err = terraformCLI.Plan(context.Background(), nil, "-input=false", "-no-color", "-detailed-exitcode")
if err != nil {
if exitErr, ok := err.(*exitError); ok {
if exitErr.ExitCode() != 2 {
Expand Down Expand Up @@ -196,7 +196,7 @@ func TestAccTerraformCLIImport(t *testing.T) {
t.Errorf("got: %v, want: %v", got, want)
}

_, err = terraformCLI.Plan(context.Background(), state, "", "-input=false", "-no-color", "-detailed-exitcode")
_, err = terraformCLI.Plan(context.Background(), state, "-input=false", "-no-color", "-detailed-exitcode")
if err != nil {
t.Fatalf("failed to run terraform plan after import (expected no diff): %s", err)
}
Expand Down
6 changes: 1 addition & 5 deletions tfexec/terraform_plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (

// Plan computes expected changes.
// If a state is given, use it for the input state.
func (c *terraformCLI) Plan(ctx context.Context, state *State, dir string, opts ...string) (*Plan, error) {
func (c *terraformCLI) Plan(ctx context.Context, state *State, opts ...string) (*Plan, error) {
args := []string{"plan"}

if state != nil {
Expand Down Expand Up @@ -45,10 +45,6 @@ func (c *terraformCLI) Plan(ctx context.Context, state *State, dir string, opts

args = append(args, opts...)

if len(dir) > 0 {
args = append(args, dir)
}

_, _, err := c.Run(ctx, args...)

// terraform plan -detailed-exitcode returns 2 if there is a diff.
Expand Down
50 changes: 8 additions & 42 deletions tfexec/terraform_plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,12 @@ func TestTerraformCLIPlan(t *testing.T) {
desc string
mockCommands []*mockCommand
state *State
dir string
opts []string
want *Plan
ok bool
}{
{
desc: "no dir and no opts",
desc: "no opts",
mockCommands: []*mockCommand{
{
args: []string{"terraform", "plan", "-out=/path/to/planfile"},
Expand All @@ -63,21 +62,6 @@ func TestTerraformCLIPlan(t *testing.T) {
want: NewPlan([]byte{}),
ok: false,
},
{
desc: "with dir",
mockCommands: []*mockCommand{
{
args: []string{"terraform", "plan", "-out=/path/to/planfile", "foo"},
argsRe: regexp.MustCompile(`^terraform plan -out=.+ foo$`),
runFunc: runFunc,
exitCode: 0,
},
},
dir: "foo",
state: nil,
want: plan,
ok: true,
},
{
desc: "with opts",
mockCommands: []*mockCommand{
Expand All @@ -93,33 +77,16 @@ func TestTerraformCLIPlan(t *testing.T) {
want: plan,
ok: true,
},
{
desc: "with dir and opts",
mockCommands: []*mockCommand{
{
args: []string{"terraform", "plan", "-out=/path/to/planfile", "-input=false", "-no-color", "foo"},
argsRe: regexp.MustCompile(`^terraform plan -out=.+ -input=false -no-color foo$`),
runFunc: runFunc,
exitCode: 0,
},
},
dir: "foo",
opts: []string{"-input=false", "-no-color"},
state: nil,
want: plan,
ok: true,
},
{
desc: "with state",
mockCommands: []*mockCommand{
{
args: []string{"terraform", "plan", "-state=/path/to/tempfile", "-out=/path/to/planfile", "-input=false", "-no-color", "foo"},
argsRe: regexp.MustCompile(`^terraform plan -state=.+ -out=.+ -input=false -no-color foo$`),
args: []string{"terraform", "plan", "-state=/path/to/tempfile", "-out=/path/to/planfile", "-input=false", "-no-color"},
argsRe: regexp.MustCompile(`^terraform plan -state=.+ -out=.+ -input=false -no-color$`),
runFunc: runFunc,
exitCode: 0,
},
},
dir: "foo",
opts: []string{"-input=false", "-no-color"},
state: state,
want: plan,
Expand All @@ -129,13 +96,12 @@ func TestTerraformCLIPlan(t *testing.T) {
desc: "with state and -state= (conflict error)",
mockCommands: []*mockCommand{
{
args: []string{"terraform", "plan", "-state=/path/to/tempfile", "-out=/path/to/planfile", "-input=false", "-state=foo.tfstate", "foo"},
argsRe: regexp.MustCompile(`^terraform plan -state=\S+ -out=.+ -input=false -no-color -state=foo.tfstate foo$`),
args: []string{"terraform", "plan", "-state=/path/to/tempfile", "-out=/path/to/planfile", "-input=false", "-state=foo.tfstate"},
argsRe: regexp.MustCompile(`^terraform plan -state=\S+ -out=.+ -input=false -no-color -state=foo.tfstate$`),
runFunc: runFunc,
exitCode: 0,
},
},
dir: "foo",
opts: []string{"-input=false", "-state=foo.tfstate"},
state: state,
want: nil,
Expand All @@ -147,7 +113,7 @@ func TestTerraformCLIPlan(t *testing.T) {
t.Run(tc.desc, func(t *testing.T) {
e := NewMockExecutor(tc.mockCommands)
terraformCLI := NewTerraformCLI(e)
got, err := terraformCLI.Plan(context.Background(), tc.state, tc.dir, tc.opts...)
got, err := terraformCLI.Plan(context.Background(), tc.state, tc.opts...)
if tc.ok && err != nil {
t.Fatalf("unexpected err: %s", err)
}
Expand All @@ -173,7 +139,7 @@ func TestAccTerraformCLIPlan(t *testing.T) {
t.Fatalf("failed to run terraform init: %s", err)
}

plan, err := terraformCLI.Plan(context.Background(), nil, "", "-input=false", "-no-color")
plan, err := terraformCLI.Plan(context.Background(), nil, "-input=false", "-no-color")
if err != nil {
t.Fatalf("failed to run terraform plan: %s", err)
}
Expand All @@ -196,7 +162,7 @@ func TestAccTerraformCLIPlanWithOut(t *testing.T) {
}

planOut := "foo.tfplan"
plan, err := terraformCLI.Plan(context.Background(), nil, "", "-input=false", "-no-color", "-out="+planOut)
plan, err := terraformCLI.Plan(context.Background(), nil, "-input=false", "-no-color", "-out="+planOut)
if err != nil {
t.Fatalf("failed to run terraform plan: %s", err)
}
Expand Down
10 changes: 5 additions & 5 deletions tfexec/terraform_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ resource "aws_security_group" "bar" {}
`
UpdateTestAccSource(t, terraformCLI, backend+updatedSource)

changed, err := terraformCLI.PlanHasChange(context.Background(), nil, "")
changed, err := terraformCLI.PlanHasChange(context.Background(), nil)
if err != nil {
t.Fatalf("failed to run PlanHasChange: %s", err)
}
Expand Down Expand Up @@ -139,7 +139,7 @@ resource "aws_security_group" "bar" {}
t.Fatalf("failed to run terraform state mv: %s", err)
}

changed, err = terraformCLI.PlanHasChange(context.Background(), updatedState, "")
changed, err = terraformCLI.PlanHasChange(context.Background(), updatedState)
if err != nil {
t.Fatalf("failed to run PlanHasChange: %s", err)
}
Expand All @@ -153,7 +153,7 @@ resource "aws_security_group" "bar" {}
t.Fatalf("the override file wasn't removed: %s", err)
}

changed, err = terraformCLI.PlanHasChange(context.Background(), nil, "")
changed, err = terraformCLI.PlanHasChange(context.Background(), nil)
if err != nil {
t.Fatalf("failed to run PlanHasChange: %s", err)
}
Expand All @@ -174,7 +174,7 @@ func TestAccTerraformCLIPlanHasChange(t *testing.T) {
t.Fatalf("failed to run terraform init: %s", err)
}

changed, err := terraformCLI.PlanHasChange(context.Background(), nil, "")
changed, err := terraformCLI.PlanHasChange(context.Background(), nil)
if err != nil {
t.Fatalf("failed to run PlanHasChange: %s", err)
}
Expand All @@ -187,7 +187,7 @@ func TestAccTerraformCLIPlanHasChange(t *testing.T) {
t.Fatalf("failed to run terraform apply: %s", err)
}

changed, err = terraformCLI.PlanHasChange(context.Background(), nil, "")
changed, err = terraformCLI.PlanHasChange(context.Background(), nil)
if err != nil {
t.Fatalf("failed to run PlanHasChange: %s", err)
}
Expand Down
4 changes: 2 additions & 2 deletions tfmigrate/multi_state_migrator.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ func (m *MultiStateMigrator) plan(ctx context.Context) (*tfexec.State, *tfexec.S

// check if a plan in fromDir has no changes.
log.Printf("[INFO] [migrator@%s] check diffs\n", m.fromTf.Dir())
_, err = m.fromTf.Plan(ctx, fromCurrentState, "", planOpts...)
_, err = m.fromTf.Plan(ctx, fromCurrentState, planOpts...)
if err != nil {
if exitErr, ok := err.(tfexec.ExitError); ok && exitErr.ExitCode() == 2 {
if m.force {
Expand All @@ -155,7 +155,7 @@ func (m *MultiStateMigrator) plan(ctx context.Context) (*tfexec.State, *tfexec.S

// check if a plan in toDir has no changes.
log.Printf("[INFO] [migrator@%s] check diffs\n", m.toTf.Dir())
_, err = m.toTf.Plan(ctx, toCurrentState, "", planOpts...)
_, err = m.toTf.Plan(ctx, toCurrentState, planOpts...)
if err != nil {
if exitErr, ok := err.(tfexec.ExitError); ok && exitErr.ExitCode() == 2 {
if m.force {
Expand Down
12 changes: 6 additions & 6 deletions tfmigrate/multi_state_migrator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -250,14 +250,14 @@ func TestAccMultiStateMigratorApply(t *testing.T) {
//update terraform resource files for migration
tfexec.UpdateTestAccSource(t, fromTf, fromBackend+tc.fromUpdatedSource)
tfexec.UpdateTestAccSource(t, toTf, toBackend+tc.toUpdatedSource)
changed, err := fromTf.PlanHasChange(ctx, nil, "")
changed, err := fromTf.PlanHasChange(ctx, nil)
if err != nil {
t.Fatalf("failed to run PlanHasChange in fromDir: %s", err)
}
if !changed {
t.Fatalf("expect to have changes in fromDir")
}
changed, err = toTf.PlanHasChange(ctx, nil, "")
changed, err = toTf.PlanHasChange(ctx, nil)
if err != nil {
t.Fatalf("failed to run PlanHasChange in toDir: %s", err)
}
Expand Down Expand Up @@ -310,14 +310,14 @@ func TestAccMultiStateMigratorApply(t *testing.T) {
if !reflect.DeepEqual(toGot, tc.toUpdatedState) {
t.Errorf("got state: %v, want state: %v in toDir", toGot, tc.toUpdatedState)
}
changed, err = fromTf.PlanHasChange(ctx, nil, "")
changed, err = fromTf.PlanHasChange(ctx, nil)
if err != nil {
t.Fatalf("failed to run PlanHasChange in fromDir: %s", err)
}
if changed != tc.fromStateExpectChange {
t.Fatalf("expected change in fromDir is %t but actual value is %t", tc.fromStateExpectChange, changed)
}
changed, err = toTf.PlanHasChange(ctx, nil, "")
changed, err = toTf.PlanHasChange(ctx, nil)
if err != nil {
t.Fatalf("failed to run PlanHasChange in toDir: %s", err)
}
Expand Down Expand Up @@ -382,14 +382,14 @@ func TestAccMultiStateMigratorApply(t *testing.T) {
}

// confirm no changes
changed, err := fromTf.PlanHasChange(ctx, nil, "")
changed, err := fromTf.PlanHasChange(ctx, nil)
if err != nil {
t.Fatalf("failed to run PlanHasChange in fromDir: %s", err)
}
if changed {
t.Fatalf("expect not to have changes in fromDir")
}
changed, err = toTf.PlanHasChange(ctx, nil, "")
changed, err = toTf.PlanHasChange(ctx, nil)
if err != nil {
t.Fatalf("failed to run PlanHasChange in toDir: %s", err)
}
Expand Down
2 changes: 1 addition & 1 deletion tfmigrate/state_import_action_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ resource "aws_iam_user" "baz" {
t.Fatalf("failed to run terraform state rm: %s", err)
}

changed, err := tf.PlanHasChange(ctx, nil, "")
changed, err := tf.PlanHasChange(ctx, nil)
if err != nil {
t.Fatalf("failed to run PlanHasChange: %s", err)
}
Expand Down
2 changes: 1 addition & 1 deletion tfmigrate/state_migrator.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ func (m *StateMigrator) plan(ctx context.Context) (*tfexec.State, error) {
}

log.Printf("[INFO] [migrator@%s] check diffs\n", m.tf.Dir())
_, err = m.tf.Plan(ctx, currentState, "", planOpts...)
_, err = m.tf.Plan(ctx, currentState, planOpts...)
if err != nil {
if exitErr, ok := err.(tfexec.ExitError); ok && exitErr.ExitCode() == 2 {
if m.force {
Expand Down
10 changes: 5 additions & 5 deletions tfmigrate/state_migrator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ resource "aws_iam_user" "qux" {
t.Fatalf("failed to run terraform state rm: %s", err)
}

changed, err := tf.PlanHasChange(ctx, nil, "")
changed, err := tf.PlanHasChange(ctx, nil)
if err != nil {
t.Fatalf("failed to run PlanHasChange: %s", err)
}
Expand Down Expand Up @@ -175,7 +175,7 @@ resource "aws_iam_user" "qux" {
t.Errorf("got state: %v, want state: %v", got, want)
}

changed, err = tf.PlanHasChange(ctx, nil, "")
changed, err = tf.PlanHasChange(ctx, nil)
if err != nil {
t.Fatalf("failed to run PlanHasChange: %s", err)
}
Expand Down Expand Up @@ -204,7 +204,7 @@ resource "aws_security_group" "baz" {}

tfexec.UpdateTestAccSource(t, tf, backend+updatedSource)

changed, err := tf.PlanHasChange(ctx, nil, "")
changed, err := tf.PlanHasChange(ctx, nil)
if err != nil {
t.Fatalf("failed to run PlanHasChange: %s", err)
}
Expand Down Expand Up @@ -245,7 +245,7 @@ resource "aws_security_group" "baz" {}
t.Errorf("got state: %v, want state: %v", got, want)
}

changed, err = tf.PlanHasChange(ctx, nil, "")
changed, err = tf.PlanHasChange(ctx, nil)
if err != nil {
t.Fatalf("failed to run PlanHasChange: %s", err)
}
Expand Down Expand Up @@ -286,7 +286,7 @@ resource "aws_security_group" "baz" {}
}

// confirm no changes
changed, err = tf.PlanHasChange(ctx, nil, "")
changed, err = tf.PlanHasChange(ctx, nil)
if err != nil {
t.Fatalf("failed to run PlanHasChange: %s", err)
}
Expand Down
2 changes: 1 addition & 1 deletion tfmigrate/state_mv_action_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ resource "aws_security_group" "baz" {}

tfexec.UpdateTestAccSource(t, tf, backend+updatedSource)

changed, err := tf.PlanHasChange(ctx, nil, "")
changed, err := tf.PlanHasChange(ctx, nil)
if err != nil {
t.Fatalf("failed to run PlanHasChange: %s", err)
}
Expand Down
2 changes: 1 addition & 1 deletion tfmigrate/state_rm_action_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ resource "aws_security_group" "baz" {}

tfexec.UpdateTestAccSource(t, tf, backend+updatedSource)

changed, err := tf.PlanHasChange(ctx, nil, "")
changed, err := tf.PlanHasChange(ctx, nil)
if err != nil {
t.Fatalf("failed to run PlanHasChange: %s", err)
}
Expand Down

0 comments on commit e5e3e8d

Please sign in to comment.