diff --git a/command/apply.go b/command/apply.go index c433400a9..d84745f19 100644 --- a/command/apply.go +++ b/command/apply.go @@ -42,10 +42,15 @@ func (c *ApplyCommand) Run(args []string) int { cmdFlags.DurationVar(&c.Meta.stateLockTimeout, "lock-timeout", 0, "lock timeout") cmdFlags.Usage = func() { c.Ui.Error(c.Help()) } if err := cmdFlags.Parse(args); err != nil { + c.Ui.Error(fmt.Sprintf("Error parsing command-line flags: %s\n", err.Error())) return 1 } - var diags tfdiags.Diagnostics + diags := c.parseTargetFlags() + if diags.HasErrors() { + c.showDiagnostics(diags) + return 1 + } args = cmdFlags.Args() var planPath string diff --git a/command/apply_destroy_test.go b/command/apply_destroy_test.go index 6bc51accb..ec3b2e9ea 100644 --- a/command/apply_destroy_test.go +++ b/command/apply_destroy_test.go @@ -358,7 +358,10 @@ func TestApply_destroyPath(t *testing.T) { } } -func TestApply_destroyTargeted(t *testing.T) { +// Config with multiple resources with dependencies, targeting destroy of a +// root node, expecting all other resources to be destroyed due to +// dependencies. +func TestApply_destroyTargetedDependencies(t *testing.T) { // Create a temporary working directory that is empty td := tempDir(t) testCopyDir(t, testFixturePath("apply-destroy-targeted"), td) @@ -488,6 +491,156 @@ func TestApply_destroyTargeted(t *testing.T) { } } +// Config with multiple resources with dependencies, targeting destroy of a +// leaf node, expecting the other resources to remain. +func TestApply_destroyTargeted(t *testing.T) { + // Create a temporary working directory that is empty + td := tempDir(t) + testCopyDir(t, testFixturePath("apply-destroy-targeted"), td) + defer os.RemoveAll(td) + defer testChdir(t, td)() + + originalState := states.BuildState(func(s *states.SyncState) { + s.SetResourceInstanceCurrent( + addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "test_instance", + Name: "foo", + }.Instance(addrs.IntKey(0)).Absolute(addrs.RootModuleInstance), + &states.ResourceInstanceObjectSrc{ + AttrsJSON: []byte(`{"id":"i-ab123"}`), + Status: states.ObjectReady, + }, + addrs.AbsProviderConfig{ + Provider: addrs.NewDefaultProvider("test"), + Module: addrs.RootModule, + }, + ) + s.SetResourceInstanceCurrent( + addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "test_load_balancer", + Name: "foo", + }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), + &states.ResourceInstanceObjectSrc{ + AttrsJSON: []byte(`{"id":"i-abc123"}`), + Dependencies: []addrs.ConfigResource{mustResourceAddr("test_instance.foo")}, + Status: states.ObjectReady, + }, + addrs.AbsProviderConfig{ + Provider: addrs.NewDefaultProvider("test"), + Module: addrs.RootModule, + }, + ) + }) + wantState := states.BuildState(func(s *states.SyncState) { + s.SetResourceInstanceCurrent( + addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "test_instance", + Name: "foo", + }.Instance(addrs.IntKey(0)).Absolute(addrs.RootModuleInstance), + &states.ResourceInstanceObjectSrc{ + AttrsJSON: []byte(`{"id":"i-ab123"}`), + Status: states.ObjectReady, + }, + addrs.AbsProviderConfig{ + Provider: addrs.NewDefaultProvider("test"), + Module: addrs.RootModule, + }, + ) + }) + statePath := testStateFile(t, originalState) + + p := testProvider() + p.GetSchemaResponse = &providers.GetSchemaResponse{ + ResourceTypes: map[string]providers.Schema{ + "test_instance": { + Block: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "id": {Type: cty.String, Computed: true}, + }, + }, + }, + "test_load_balancer": { + Block: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "id": {Type: cty.String, Computed: true}, + "instances": {Type: cty.List(cty.String), Optional: true}, + }, + }, + }, + }, + } + p.PlanResourceChangeFn = func(req providers.PlanResourceChangeRequest) providers.PlanResourceChangeResponse { + return providers.PlanResourceChangeResponse{ + PlannedState: req.ProposedNewState, + } + } + + ui := new(cli.MockUi) + c := &ApplyCommand{ + Destroy: true, + Meta: Meta{ + testingOverrides: metaOverridesForProvider(p), + Ui: ui, + }, + } + + // Run the apply command pointing to our existing state + args := []string{ + "-auto-approve", + "-target", "test_load_balancer.foo", + "-state", statePath, + } + if code := c.Run(args); code != 0 { + t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String()) + } + + // Verify a new state exists + if _, err := os.Stat(statePath); err != nil { + t.Fatalf("err: %s", err) + } + + f, err := os.Open(statePath) + if err != nil { + t.Fatalf("err: %s", err) + } + defer f.Close() + + stateFile, err := statefile.Read(f) + if err != nil { + t.Fatalf("err: %s", err) + } + if stateFile == nil || stateFile.State == nil { + t.Fatal("state should not be nil") + } + + actualStr := strings.TrimSpace(stateFile.State.String()) + expectedStr := strings.TrimSpace(wantState.String()) + if actualStr != expectedStr { + t.Fatalf("bad:\n\nactual:\n%s\n\nexpected:\nb%s", actualStr, expectedStr) + } + + // Should have a backup file + f, err = os.Open(statePath + DefaultBackupExtension) + if err != nil { + t.Fatalf("err: %s", err) + } + + backupStateFile, err := statefile.Read(f) + f.Close() + if err != nil { + t.Fatalf("err: %s", err) + } + + backupActualStr := strings.TrimSpace(backupStateFile.State.String()) + backupExpectedStr := strings.TrimSpace(originalState.String()) + if backupActualStr != backupExpectedStr { + t.Fatalf("bad:\n\nactual:\n%s\n\nexpected:\nb%s", backupActualStr, backupExpectedStr) + } +} + const testApplyDestroyStr = ` ` diff --git a/command/apply_test.go b/command/apply_test.go index ebc22e9b8..8857c2aaf 100644 --- a/command/apply_test.go +++ b/command/apply_test.go @@ -1702,6 +1702,92 @@ output = test testStateOutput(t, statePath, expected) } +// Config with multiple resources, targeting apply of a subset +func TestApply_targeted(t *testing.T) { + td := tempDir(t) + testCopyDir(t, testFixturePath("apply-targeted"), td) + defer os.RemoveAll(td) + defer testChdir(t, td)() + + p := testProvider() + p.GetSchemaResponse = &providers.GetSchemaResponse{ + ResourceTypes: map[string]providers.Schema{ + "test_instance": { + Block: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "id": {Type: cty.String, Computed: true}, + }, + }, + }, + }, + } + p.PlanResourceChangeFn = func(req providers.PlanResourceChangeRequest) providers.PlanResourceChangeResponse { + return providers.PlanResourceChangeResponse{ + PlannedState: req.ProposedNewState, + } + } + + ui := new(cli.MockUi) + c := &ApplyCommand{ + Meta: Meta{ + testingOverrides: metaOverridesForProvider(p), + Ui: ui, + }, + } + + args := []string{ + "-auto-approve", + "-target", "test_instance.foo", + "-target", "test_instance.baz", + } + if code := c.Run(args); code != 0 { + t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String()) + } + + if got, want := ui.OutputWriter.String(), "3 added, 0 changed, 0 destroyed"; !strings.Contains(got, want) { + t.Fatalf("bad change summary, want %q, got:\n%s", want, got) + } +} + +// Diagnostics for invalid -target flags +func TestApply_targetFlagsDiags(t *testing.T) { + testCases := map[string]string{ + "test_instance.": "Dot must be followed by attribute name.", + "test_instance": "Resource specification must include a resource type and name.", + } + + for target, wantDiag := range testCases { + t.Run(target, func(t *testing.T) { + td := testTempDir(t) + defer os.RemoveAll(td) + defer testChdir(t, td)() + + ui := new(cli.MockUi) + c := &ApplyCommand{ + Meta: Meta{ + Ui: ui, + }, + } + + args := []string{ + "-auto-approve", + "-target", target, + } + if code := c.Run(args); code != 1 { + t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String()) + } + + got := ui.ErrorWriter.String() + if !strings.Contains(got, target) { + t.Fatalf("bad error output, want %q, got:\n%s", target, got) + } + if !strings.Contains(got, wantDiag) { + t.Fatalf("bad error output, want %q, got:\n%s", wantDiag, got) + } + }) + } +} + // applyFixtureSchema returns a schema suitable for processing the // configuration in testdata/apply . This schema should be // assigned to a mock provider named "test". diff --git a/command/flag_kv.go b/command/flag_kv.go index 9f38018af..b084c5135 100644 --- a/command/flag_kv.go +++ b/command/flag_kv.go @@ -3,11 +3,6 @@ package command import ( "fmt" "strings" - - "github.com/hashicorp/hcl/v2" - "github.com/hashicorp/hcl/v2/hclsyntax" - "github.com/hashicorp/terraform/addrs" - "github.com/hashicorp/terraform/tfdiags" ) // FlagStringKV is a flag.Value implementation for parsing user variables @@ -46,34 +41,3 @@ func (v *FlagStringSlice) Set(raw string) error { return nil } - -// FlagTargetSlice is a flag.Value implementation for parsing target addresses -// from the command line, such as -target=aws_instance.foo -target=aws_vpc.bar . -type FlagTargetSlice []addrs.Targetable - -func (v *FlagTargetSlice) String() string { - return "" -} - -func (v *FlagTargetSlice) Set(raw string) error { - // FIXME: This is not an ideal way to deal with this because it requires - // us to do parsing in a context where we can't nicely return errors - // to the user. - - var diags tfdiags.Diagnostics - synthFilename := fmt.Sprintf("-target=%q", raw) - traversal, syntaxDiags := hclsyntax.ParseTraversalAbs([]byte(raw), synthFilename, hcl.Pos{Line: 1, Column: 1}) - diags = diags.Append(syntaxDiags) - if syntaxDiags.HasErrors() { - return diags.Err() - } - - target, targetDiags := addrs.ParseTarget(traversal) - diags = diags.Append(targetDiags) - if targetDiags.HasErrors() { - return diags.Err() - } - - *v = append(*v, target.Subject) - return nil -} diff --git a/command/meta.go b/command/meta.go index d4f66da98..033c6f39b 100644 --- a/command/meta.go +++ b/command/meta.go @@ -15,6 +15,8 @@ import ( "time" plugin "github.com/hashicorp/go-plugin" + "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/hclsyntax" "github.com/hashicorp/terraform-svchost/disco" "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/backend" @@ -164,7 +166,8 @@ type Meta struct { input bool // Targets for this context (private) - targets []addrs.Targetable + targets []addrs.Targetable + targetFlags []string // Internal fields color bool @@ -523,7 +526,7 @@ func (m *Meta) extendedFlagSet(n string) *flag.FlagSet { f := m.defaultFlagSet(n) f.BoolVar(&m.input, "input", true, "input") - f.Var((*FlagTargetSlice)(&m.targets), "target", "resource to target") + f.Var((*FlagStringSlice)(&m.targetFlags), "target", "resource to target") f.BoolVar(&m.compactWarnings, "compact-warnings", false, "use compact warnings") if m.variableArgs.items == nil { @@ -541,6 +544,43 @@ func (m *Meta) extendedFlagSet(n string) *flag.FlagSet { return f } +// parseTargetFlags must be called for any commands supporting -target +// arguments. This method attempts to parse each -target flag into an +// addrs.Target, storing in the Meta.targets slice. +// +// If any flags cannot be parsed, we rewrap the first error diagnostic with a +// custom title to clarify the source of the error. The normal approach of +// directly returning the diags from HCL or the addrs package results in +// confusing incorrect "source" results when presented. +func (m *Meta) parseTargetFlags() tfdiags.Diagnostics { + var diags tfdiags.Diagnostics + m.targets = nil + for _, tf := range m.targetFlags { + traversal, syntaxDiags := hclsyntax.ParseTraversalAbs([]byte(tf), "", hcl.Pos{Line: 1, Column: 1}) + if syntaxDiags.HasErrors() { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + fmt.Sprintf("Invalid target %q", tf), + syntaxDiags[0].Detail, + )) + continue + } + + target, targetDiags := addrs.ParseTarget(traversal) + if targetDiags.HasErrors() { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + fmt.Sprintf("Invalid target %q", tf), + targetDiags[0].Description().Detail, + )) + continue + } + + m.targets = append(m.targets, target.Subject) + } + return diags +} + // process will process the meta-parameters out of the arguments. This // will potentially modify the args in-place. It will return the resulting // slice. diff --git a/command/plan.go b/command/plan.go index 7175c0602..ce5ea0800 100644 --- a/command/plan.go +++ b/command/plan.go @@ -32,6 +32,13 @@ func (c *PlanCommand) Run(args []string) int { cmdFlags.DurationVar(&c.Meta.stateLockTimeout, "lock-timeout", 0, "lock timeout") cmdFlags.Usage = func() { c.Ui.Error(c.Help()) } if err := cmdFlags.Parse(args); err != nil { + c.Ui.Error(fmt.Sprintf("Error parsing command-line flags: %s\n", err.Error())) + return 1 + } + + diags := c.parseTargetFlags() + if diags.HasErrors() { + c.showDiagnostics(diags) return 1 } @@ -47,8 +54,6 @@ func (c *PlanCommand) Run(args []string) int { return 1 } - var diags tfdiags.Diagnostics - var backendConfig *configs.Backend var configDiags tfdiags.Diagnostics backendConfig, configDiags = c.loadBackendConfig(configPath) diff --git a/command/plan_test.go b/command/plan_test.go index 357371fd4..c9c09476e 100644 --- a/command/plan_test.go +++ b/command/plan_test.go @@ -901,6 +901,90 @@ func TestPlan_init_required(t *testing.T) { } } +// Config with multiple resources, targeting plan of a subset +func TestPlan_targeted(t *testing.T) { + td := tempDir(t) + testCopyDir(t, testFixturePath("apply-targeted"), td) + defer os.RemoveAll(td) + defer testChdir(t, td)() + + p := testProvider() + p.GetSchemaResponse = &providers.GetSchemaResponse{ + ResourceTypes: map[string]providers.Schema{ + "test_instance": { + Block: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "id": {Type: cty.String, Computed: true}, + }, + }, + }, + }, + } + p.PlanResourceChangeFn = func(req providers.PlanResourceChangeRequest) providers.PlanResourceChangeResponse { + return providers.PlanResourceChangeResponse{ + PlannedState: req.ProposedNewState, + } + } + + ui := new(cli.MockUi) + c := &PlanCommand{ + Meta: Meta{ + testingOverrides: metaOverridesForProvider(p), + Ui: ui, + }, + } + + args := []string{ + "-target", "test_instance.foo", + "-target", "test_instance.baz", + } + if code := c.Run(args); code != 0 { + t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String()) + } + + if got, want := ui.OutputWriter.String(), "3 to add, 0 to change, 0 to destroy"; !strings.Contains(got, want) { + t.Fatalf("bad change summary, want %q, got:\n%s", want, got) + } +} + +// Diagnostics for invalid -target flags +func TestPlan_targetFlagsDiags(t *testing.T) { + testCases := map[string]string{ + "test_instance.": "Dot must be followed by attribute name.", + "test_instance": "Resource specification must include a resource type and name.", + } + + for target, wantDiag := range testCases { + t.Run(target, func(t *testing.T) { + td := testTempDir(t) + defer os.RemoveAll(td) + defer testChdir(t, td)() + + ui := new(cli.MockUi) + c := &PlanCommand{ + Meta: Meta{ + Ui: ui, + }, + } + + args := []string{ + "-target", target, + } + if code := c.Run(args); code != 1 { + t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String()) + } + + got := ui.ErrorWriter.String() + if !strings.Contains(got, target) { + t.Fatalf("bad error output, want %q, got:\n%s", target, got) + } + if !strings.Contains(got, wantDiag) { + t.Fatalf("bad error output, want %q, got:\n%s", wantDiag, got) + } + }) + } +} + // planFixtureSchema returns a schema suitable for processing the // configuration in testdata/plan . This schema should be // assigned to a mock provider named "test". diff --git a/command/refresh.go b/command/refresh.go index 42ed0c4d1..5c512d711 100644 --- a/command/refresh.go +++ b/command/refresh.go @@ -25,6 +25,13 @@ func (c *RefreshCommand) Run(args []string) int { cmdFlags.DurationVar(&c.Meta.stateLockTimeout, "lock-timeout", 0, "lock timeout") cmdFlags.Usage = func() { c.Ui.Error(c.Help()) } if err := cmdFlags.Parse(args); err != nil { + c.Ui.Error(fmt.Sprintf("Error parsing command-line flags: %s\n", err.Error())) + return 1 + } + + diags := c.parseTargetFlags() + if diags.HasErrors() { + c.showDiagnostics(diags) return 1 } @@ -34,8 +41,6 @@ func (c *RefreshCommand) Run(args []string) int { return 1 } - var diags tfdiags.Diagnostics - // Check for user-supplied plugin path if c.pluginPath, err = c.loadPluginPath(); err != nil { c.Ui.Error(fmt.Sprintf("Error loading plugin path: %s", err)) diff --git a/command/refresh_test.go b/command/refresh_test.go index 339290677..e8e30f1bd 100644 --- a/command/refresh_test.go +++ b/command/refresh_test.go @@ -736,6 +736,97 @@ func TestRefresh_displaysOutputs(t *testing.T) { } } +// Config with multiple resources, targeting refresh of a subset +func TestRefresh_targeted(t *testing.T) { + td := tempDir(t) + testCopyDir(t, testFixturePath("refresh-targeted"), td) + defer os.RemoveAll(td) + defer testChdir(t, td)() + + state := testState() + statePath := testStateFile(t, state) + + p := testProvider() + p.GetSchemaResponse = &providers.GetSchemaResponse{ + ResourceTypes: map[string]providers.Schema{ + "test_instance": { + Block: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "id": {Type: cty.String, Computed: true}, + }, + }, + }, + }, + } + p.PlanResourceChangeFn = func(req providers.PlanResourceChangeRequest) providers.PlanResourceChangeResponse { + return providers.PlanResourceChangeResponse{ + PlannedState: req.ProposedNewState, + } + } + + ui := new(cli.MockUi) + c := &RefreshCommand{ + Meta: Meta{ + testingOverrides: metaOverridesForProvider(p), + Ui: ui, + }, + } + + args := []string{ + "-target", "test_instance.foo", + "-state", statePath, + } + if code := c.Run(args); code != 0 { + t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String()) + } + + got := ui.OutputWriter.String() + if want := "test_instance.foo: Refreshing"; !strings.Contains(got, want) { + t.Fatalf("expected output to contain %q, got:\n%s", want, got) + } + if doNotWant := "test_instance.bar: Refreshing"; strings.Contains(got, doNotWant) { + t.Fatalf("expected output not to contain %q, got:\n%s", doNotWant, got) + } +} + +// Diagnostics for invalid -target flags +func TestRefresh_targetFlagsDiags(t *testing.T) { + testCases := map[string]string{ + "test_instance.": "Dot must be followed by attribute name.", + "test_instance": "Resource specification must include a resource type and name.", + } + + for target, wantDiag := range testCases { + t.Run(target, func(t *testing.T) { + td := testTempDir(t) + defer os.RemoveAll(td) + defer testChdir(t, td)() + + ui := new(cli.MockUi) + c := &RefreshCommand{ + Meta: Meta{ + Ui: ui, + }, + } + + args := []string{ + "-target", target, + } + if code := c.Run(args); code != 1 { + t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String()) + } + + got := ui.ErrorWriter.String() + if !strings.Contains(got, target) { + t.Fatalf("bad error output, want %q, got:\n%s", target, got) + } + if !strings.Contains(got, wantDiag) { + t.Fatalf("bad error output, want %q, got:\n%s", wantDiag, got) + } + }) + } +} + // configuration in testdata/refresh . This schema should be // assigned to a mock provider named "test". func refreshFixtureSchema() *providers.GetSchemaResponse { diff --git a/command/testdata/apply-destroy-targeted/main.tf b/command/testdata/apply-destroy-targeted/main.tf index 45ebc5b97..0f249b384 100644 --- a/command/testdata/apply-destroy-targeted/main.tf +++ b/command/testdata/apply-destroy-targeted/main.tf @@ -3,5 +3,5 @@ resource "test_instance" "foo" { } resource "test_load_balancer" "foo" { - instances = ["${test_instance.foo.*.id}"] + instances = test_instance.foo.*.id } diff --git a/command/testdata/apply-targeted/main.tf b/command/testdata/apply-targeted/main.tf new file mode 100644 index 000000000..1b6c42450 --- /dev/null +++ b/command/testdata/apply-targeted/main.tf @@ -0,0 +1,9 @@ +resource "test_instance" "foo" { + count = 2 +} + +resource "test_instance" "bar" { +} + +resource "test_instance" "baz" { +} diff --git a/command/testdata/refresh-targeted/main.tf b/command/testdata/refresh-targeted/main.tf new file mode 100644 index 000000000..734f58549 --- /dev/null +++ b/command/testdata/refresh-targeted/main.tf @@ -0,0 +1,7 @@ +resource "test_instance" "foo" { + id = "foo" +} + +resource "test_instance" "bar" { + id = "bar" +}