From b47619eebf22373780c31832e7a47fe8f90622e4 Mon Sep 17 00:00:00 2001 From: Mike Ball Date: Thu, 14 Sep 2023 07:04:38 -0400 Subject: [PATCH] add skip_plan option to state migrator This addresses issue #151 to add a `skip_plan` option to single state migrations, similar to the `from_skip_plan` and `to_skip_plan` options supported by multi state migrations. --- README.md | 1 + tfmigrate/state_import_action_test.go | 2 +- tfmigrate/state_migrator.go | 37 +++++--- tfmigrate/state_migrator_test.go | 94 +++++++++++++++++-- tfmigrate/state_mv_action_test.go | 2 +- .../state_replace_provider_action_test.go | 4 +- tfmigrate/state_rm_action_test.go | 2 +- tfmigrate/state_xmv_action_test.go | 2 +- 8 files changed, 118 insertions(+), 26 deletions(-) diff --git a/README.md b/README.md index b7dde60..605c77c 100644 --- a/README.md +++ b/README.md @@ -575,6 +575,7 @@ The `state` migration updates the state in a single directory. It has the follow - `"import
"` - `"replace-provider
"` - `force` (optional): Apply migrations even if plan show changes +- `skip_plan` (optional): If true, `tfmigrate` will not perform and analyze a `terraform plan`. Note that `dir` is relative path to the current working directory where `tfmigrate` command is invoked. diff --git a/tfmigrate/state_import_action_test.go b/tfmigrate/state_import_action_test.go index 3991ab5..37c350a 100644 --- a/tfmigrate/state_import_action_test.go +++ b/tfmigrate/state_import_action_test.go @@ -40,7 +40,7 @@ resource "time_static" "baz" { triggers = {} } NewStateImportAction("time_static.baz", "2006-01-02T15:04:05Z"), } - m := NewStateMigrator(tf.Dir(), workspace, actions, &MigratorOption{}, false) + m := NewStateMigrator(tf.Dir(), workspace, actions, &MigratorOption{}, false, false) err = m.Plan(ctx) if err != nil { t.Fatalf("failed to run migrator plan: %s", err) diff --git a/tfmigrate/state_migrator.go b/tfmigrate/state_migrator.go index 7ae7dad..bf8466b 100644 --- a/tfmigrate/state_migrator.go +++ b/tfmigrate/state_migrator.go @@ -28,6 +28,8 @@ type StateMigratorConfig struct { // Force option controls behaviour in case of unexpected diff in plan. // When set forces applying even if plan shows diff. Force bool `hcl:"force,optional"` + // SkipPlan controls whether or not to run and analyze Terraform plan. + SkipPlan bool `hcl:"to_skip_plan,optional"` // Workspace is the state workspace which the migration works with. Workspace string `hcl:"workspace,optional"` } @@ -62,7 +64,7 @@ func (c *StateMigratorConfig) NewMigrator(o *MigratorOption) (Migrator, error) { c.Workspace = "default" } - return NewStateMigrator(dir, c.Workspace, actions, o, c.Force), nil + return NewStateMigrator(dir, c.Workspace, actions, o, c.Force, c.SkipPlan), nil } // StateMigrator implements the Migrator interface. @@ -74,6 +76,8 @@ type StateMigrator struct { // o is an option for migrator. // It is used for shared settings across Migrator instances. o *MigratorOption + // skipPlan controls whether or not to run and analyze Terraform plan. + skipPlan bool // force operation in case of unexpected diff force bool // workspace is the state workspace which the migration works with. @@ -84,7 +88,7 @@ var _ Migrator = (*StateMigrator)(nil) // NewStateMigrator returns a new StateMigrator instance. func NewStateMigrator(dir string, workspace string, actions []StateAction, - o *MigratorOption, force bool) *StateMigrator { + o *MigratorOption, force bool, skipPlan bool) *StateMigrator { e := tfexec.NewExecutor(dir, os.Environ()) tf := tfexec.NewTerraformCLI(e) if o != nil && len(o.ExecPath) > 0 { @@ -96,6 +100,7 @@ func NewStateMigrator(dir string, workspace string, actions []StateAction, actions: actions, o: o, force: force, + skipPlan: skipPlan, workspace: workspace, } } @@ -145,19 +150,23 @@ func (m *StateMigrator) plan(ctx context.Context) (currentState *tfexec.State, e planOpts = append(planOpts, "-out="+m.o.PlanOut) } - log.Printf("[INFO] [migrator@%s] check diffs\n", m.tf.Dir()) - _, err = m.tf.Plan(ctx, currentState, planOpts...) - if err != nil { - if exitErr, ok := err.(tfexec.ExitError); ok && exitErr.ExitCode() == 2 { - if !m.force { - log.Printf("[ERROR] [migrator@%s] unexpected diffs\n", m.tf.Dir()) - return nil, fmt.Errorf("terraform plan command returns unexpected diffs: %s", err) + if m.skipPlan { + log.Printf("[INFO] [migrator@%s] skipping check diffs\n", m.tf.Dir()) + } else { + log.Printf("[INFO] [migrator@%s] check diffs\n", m.tf.Dir()) + _, err = m.tf.Plan(ctx, currentState, planOpts...) + if err != nil { + if exitErr, ok := err.(tfexec.ExitError); ok && exitErr.ExitCode() == 2 { + if !m.force { + log.Printf("[ERROR] [migrator@%s] unexpected diffs\n", m.tf.Dir()) + return nil, fmt.Errorf("terraform plan command returns unexpected diffs: %s", err) + } + log.Printf("[INFO] [migrator@%s] unexpected diffs, ignoring as force option is true: %s", m.tf.Dir(), err) + // reset err to nil to intentionally ignore unexpected diffs. + err = nil + } else { + return nil, err } - log.Printf("[INFO] [migrator@%s] unexpected diffs, ignoring as force option is true: %s", m.tf.Dir(), err) - // reset err to nil to intentionally ignore unexpected diffs. - err = nil - } else { - return nil, err } } diff --git a/tfmigrate/state_migrator_test.go b/tfmigrate/state_migrator_test.go index aaf9185..d0a7173 100644 --- a/tfmigrate/state_migrator_test.go +++ b/tfmigrate/state_migrator_test.go @@ -104,6 +104,21 @@ func TestStateMigratorConfigNewMigrator(t *testing.T) { o: nil, ok: true, }, + { + desc: "with skip_plan true", + config: &StateMigratorConfig{ + Dir: "dir1", + Actions: []string{ + "mv null_resource.foo null_resource.foo2", + "mv null_resource.bar null_resource.bar2", + "rm time_static.baz", + "import time_static.qux 2006-01-02T15:04:05Z", + }, + SkipPlan: true, + }, + o: nil, + ok: true, + }, } for _, tc := range cases { @@ -166,7 +181,7 @@ resource "time_static" "qux" { triggers = {} } } force := false - m := NewStateMigrator(tf.Dir(), workspace, actions, &MigratorOption{}, force) + m := NewStateMigrator(tf.Dir(), workspace, actions, &MigratorOption{}, force, false) err = m.Plan(ctx) if err != nil { t.Fatalf("failed to run migrator plan: %s", err) @@ -236,7 +251,7 @@ resource "null_resource" "bar" {} } force := false - m := NewStateMigrator(tf.Dir(), workspace, actions, &MigratorOption{}, force) + m := NewStateMigrator(tf.Dir(), workspace, actions, &MigratorOption{}, force, false) err = m.Plan(ctx) if err != nil { t.Fatalf("failed to run migrator plan: %s", err) @@ -308,7 +323,7 @@ resource "null_resource" "baz" {} o := &MigratorOption{} o.PlanOut = "foo.tfplan" force := true - m := NewStateMigrator(tf.Dir(), workspace, actions, o, force) + m := NewStateMigrator(tf.Dir(), workspace, actions, o, force, false) err = m.Plan(ctx) if err != nil { t.Fatalf("failed to run migrator plan: %s", err) @@ -408,6 +423,73 @@ resource "null_resource" "baz" {} } } +func TestAccStateMigratorApplyWithSkipPlan(t *testing.T) { + tfexec.SkipUnlessAcceptanceTestEnabled(t) + + backend := tfexec.GetTestAccBackendS3Config(t.Name()) + + source := ` +resource "null_resource" "foo" {} +resource "null_resource" "bar" {} +` + + workspace := "default" + tf := tfexec.SetupTestAccWithApply(t, workspace, backend+source) + ctx := context.Background() + + updatedSource := source + + tfexec.UpdateTestAccSource(t, tf, backend+updatedSource) + + changed, err := tf.PlanHasChange(ctx, nil) + if err != nil { + t.Fatalf("failed to run PlanHasChange: %s", err) + } + if changed { + t.Fatalf("expect not to have changes") + } + + actions := []StateAction{ + NewStateMvAction("null_resource.foo", "null_resource.foo2"), + } + + force := false + skipPlan := true + m := NewStateMigrator(tf.Dir(), workspace, actions, &MigratorOption{}, force, skipPlan) + err = m.Plan(ctx) + if err != nil { + t.Fatalf("failed to run migrator plan: %s", err) + } + + err = m.Apply(ctx) + if err != nil { + t.Fatalf("failed to run migrator apply: %s", err) + } + + got, err := tf.StateList(ctx, nil, nil) + if err != nil { + t.Fatalf("failed to run terraform state list: %s", err) + } + + want := []string{ + "null_resource.foo2", + "null_resource.bar", + } + sort.Strings(got) + sort.Strings(want) + if !reflect.DeepEqual(got, want) { + t.Errorf("got state: %v, want state: %v", got, want) + } + + changed, err = tf.PlanHasChange(ctx, nil) + if err != nil { + t.Fatalf("failed to run PlanHasChange: %s", err) + } + if !changed { + t.Fatalf("expect to have changes") + } +} + func TestAccStateMigratorPlanWithSwitchBackToRemoteFuncError(t *testing.T) { tfexec.SkipUnlessAcceptanceTestEnabled(t) @@ -440,7 +522,7 @@ resource "null_resource" "bar" {} } force := false - m := NewStateMigrator(tf.Dir(), workspace, actions, &MigratorOption{}, force) + m := NewStateMigrator(tf.Dir(), workspace, actions, &MigratorOption{}, force, false) err := m.Plan(ctx) if err == nil { @@ -479,7 +561,7 @@ resource "null_resource" "bar" {} } force := false - m := NewStateMigrator(tf.Dir(), workspace, actions, &MigratorOption{}, force) + m := NewStateMigrator(tf.Dir(), workspace, actions, &MigratorOption{}, force, false) err := m.Plan(ctx) if err == nil { @@ -524,7 +606,7 @@ resource "null_resource" "bar" {} } force := false - m := NewStateMigrator(tf.Dir(), workspace, actions, &MigratorOption{}, force) + m := NewStateMigrator(tf.Dir(), workspace, actions, &MigratorOption{}, force, false) err := m.Plan(ctx) if err == nil { diff --git a/tfmigrate/state_mv_action_test.go b/tfmigrate/state_mv_action_test.go index 9dd2193..76e86aa 100644 --- a/tfmigrate/state_mv_action_test.go +++ b/tfmigrate/state_mv_action_test.go @@ -43,7 +43,7 @@ resource "null_resource" "baz" {} NewStateMvAction("null_resource.bar", "null_resource.bar2"), } - m := NewStateMigrator(tf.Dir(), workspace, actions, &MigratorOption{}, false) + m := NewStateMigrator(tf.Dir(), workspace, actions, &MigratorOption{}, false, false) err = m.Plan(ctx) if err != nil { t.Fatalf("failed to run migrator plan: %s", err) diff --git a/tfmigrate/state_replace_provider_action_test.go b/tfmigrate/state_replace_provider_action_test.go index 6b9e9aa..d551d42 100644 --- a/tfmigrate/state_replace_provider_action_test.go +++ b/tfmigrate/state_replace_provider_action_test.go @@ -36,7 +36,7 @@ resource "null_resource" "foo" {} } expected := "replace-provider action requires Terraform version >= 0.13.0" - m := NewStateMigrator(tf.Dir(), workspace, actions, &MigratorOption{}, false) + m := NewStateMigrator(tf.Dir(), workspace, actions, &MigratorOption{}, false, false) err := m.Plan(ctx) if err == nil || strings.Contains(err.Error(), expected) { t.Fatalf("expected to receive '%s' error using legacy Terraform; got: %s", expected, err) @@ -96,7 +96,7 @@ func TestAccStateReplaceProviderAction(t *testing.T) { NewStateReplaceProviderAction("registry.terraform.io/-/null", "registry.terraform.io/hashicorp/null"), } - m := NewStateMigrator(tf.Dir(), workspace, actions, &MigratorOption{}, false) + m := NewStateMigrator(tf.Dir(), workspace, actions, &MigratorOption{}, false, false) err = m.Plan(ctx) if err != nil { t.Fatalf("failed to run migrator plan: %s", err) diff --git a/tfmigrate/state_rm_action_test.go b/tfmigrate/state_rm_action_test.go index 6493b8a..3a94f0b 100644 --- a/tfmigrate/state_rm_action_test.go +++ b/tfmigrate/state_rm_action_test.go @@ -42,7 +42,7 @@ resource "null_resource" "baz" {} NewStateRmAction([]string{"null_resource.qux"}), } - m := NewStateMigrator(tf.Dir(), workspace, actions, &MigratorOption{}, false) + m := NewStateMigrator(tf.Dir(), workspace, actions, &MigratorOption{}, false, false) err = m.Plan(ctx) if err != nil { t.Fatalf("failed to run migrator plan: %s", err) diff --git a/tfmigrate/state_xmv_action_test.go b/tfmigrate/state_xmv_action_test.go index 8befa24..c1a9df3 100644 --- a/tfmigrate/state_xmv_action_test.go +++ b/tfmigrate/state_xmv_action_test.go @@ -39,7 +39,7 @@ resource "null_resource" "bar2" {} NewStateXmvAction("null_resource.*", "null_resource.${1}2"), } - m := NewStateMigrator(tf.Dir(), workspace, actions, &MigratorOption{}, false) + m := NewStateMigrator(tf.Dir(), workspace, actions, &MigratorOption{}, false, false) err = m.Plan(ctx) if err != nil { t.Fatalf("failed to run migrator plan: %s", err)