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.

Sign up for GitHub

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

command/import: remove -provider command line argument #24090

Merged
merged 1 commit into from
Feb 12, 2020
Merged

command/import: remove -provider command line argument #24090

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
41 changes: 2 additions & 39 deletions command/import.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ func (c *ImportCommand) Run(args []string) int {
cmdFlags.StringVar(&c.Meta.stateOutPath, "state-out", "", "path")
cmdFlags.StringVar(&c.Meta.backupPath, "backup", "", "path")
cmdFlags.StringVar(&configPath, "config", pwd, "path")
cmdFlags.StringVar(&c.Meta.provider, "provider", "", "provider")
cmdFlags.BoolVar(&c.Meta.stateLock, "lock", true, "lock state")
cmdFlags.DurationVar(&c.Meta.stateLockTimeout, "lock-timeout", 0, "lock timeout")
cmdFlags.BoolVar(&c.Meta.allowMissingConfig, "allow-missing-config", false, "allow missing config")
Expand Down Expand Up @@ -156,36 +155,6 @@ func (c *ImportCommand) Run(args []string) int {
return 1
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Part of me wants to keep the -provider argument definition above and make it generate a helpful error message below if it's non-empty. 🤔

With that said, I don't have a ready-to-go suggestion here for an actionable direction that error message could give. "Specify the provider in the configuration instead" is the gist of it, but I expect that most folks won't really know what to do about it.

So with all of that said, I think it'll be okay to just break this here and then we can describe the migration path in the changelog and/or upgrade guide where we'll have more "space" to describe what the resolution is and link to other relevant documentation.

// Also parse the user-provided provider address, if any.
var providerAddr addrs.AbsProviderConfig
if c.Meta.provider != "" {
traversal, travDiags := hclsyntax.ParseTraversalAbs([]byte(c.Meta.provider), `-provider=...`, hcl.Pos{Line: 1, Column: 1})
diags = diags.Append(travDiags)
if travDiags.HasErrors() {
c.showDiagnostics(diags)
c.Ui.Info(importCommandInvalidAddressReference)
return 1
}
relAddr, addrDiags := configs.ParseProviderConfigCompact(traversal)
diags = diags.Append(addrDiags)
if addrDiags.HasErrors() {
c.showDiagnostics(diags)
return 1
}
providerAddr = relAddr.Absolute(addrs.RootModuleInstance)
} else {
// Use a default address inferred from the resource type.
// We assume the same module as the resource address here, which
// may get resolved to an inherited provider when we construct the
// import graph inside ctx.Import, called below.
if rc != nil && rc.ProviderConfigRef != nil {
providerAddr = rc.ProviderConfigAddr().Absolute(addr.Module)
} else {
providerType := resourceRelAddr.DefaultProvider()
providerAddr = addrs.NewDefaultLocalProviderConfig(providerType.LegacyString()).Absolute(addr.Module)
}
}

// 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))
Expand Down Expand Up @@ -254,9 +223,8 @@ func (c *ImportCommand) Run(args []string) int {
newState, importDiags := ctx.Import(&terraform.ImportOpts{
Targets: []*terraform.ImportTarget{
&terraform.ImportTarget{
Addr: addr,
ID: args[1],
ProviderAddr: providerAddr,
Addr: addr,
ID: args[1],
},
},
})
Expand Down Expand Up @@ -341,11 +309,6 @@ Options:

-no-color If specified, output won't contain any color.

-provider=provider Deprecated: Override the provider configuration to use
when importing the object. By default, Terraform uses the
provider specified in the configuration for the target
resource, and that is the best behavior in most cases.

-state=PATH Path to the source state file. Defaults to the configured
backend, or "terraform.tfstate"

Expand Down
151 changes: 0 additions & 151 deletions command/import_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"github.com/mitchellh/cli"
"github.com/zclconf/go-cty/cty"

"github.com/hashicorp/terraform/addrs"
"github.com/hashicorp/terraform/configs/configschema"
"github.com/hashicorp/terraform/helper/copy"
"github.com/hashicorp/terraform/plugin/discovery"
Expand Down Expand Up @@ -537,156 +536,6 @@ func TestImport_providerConfigWithVarFile(t *testing.T) {
testStateOutput(t, statePath, testImportStr)
}

func TestImport_customProvider(t *testing.T) {
defer testChdir(t, testFixturePath("import-provider-aliased"))()

statePath := testTempFile(t)

p := testProvider()
ui := new(cli.MockUi)
c := &ImportCommand{
Meta: Meta{
testingOverrides: metaOverridesForProvider(p),
Ui: ui,
},
}

p.ImportResourceStateFn = nil
p.ImportResourceStateResponse = providers.ImportResourceStateResponse{
ImportedResources: []providers.ImportedResource{
{
TypeName: "test_instance",
State: cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("yay"),
}),
},
},
}
p.GetSchemaReturn = &terraform.ProviderSchema{
Provider: &configschema.Block{
Attributes: map[string]*configschema.Attribute{
"foo": {Type: cty.String, Optional: true},
},
},
ResourceTypes: map[string]*configschema.Block{
"test_instance": {
Attributes: map[string]*configschema.Attribute{
"id": {Type: cty.String, Optional: true, Computed: true},
},
},
},
}

args := []string{
"-provider", "test.alias",
"-state", statePath,
"test_instance.foo",
"bar",
}
if code := c.Run(args); code != 0 {
t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String())
}

if !p.ImportResourceStateCalled {
t.Fatal("ImportResourceState should be called")
}

testStateOutput(t, statePath, testImportCustomProviderStr)
}

// This tests behavior when the provider name does not match the implied
// provider name
func TestImport_providerNameMismatch(t *testing.T) {
defer testChdir(t, testFixturePath("import-provider-mismatch"))()

statePath := testTempFile(t)

p := testProvider()
ui := new(cli.MockUi)
c := &ImportCommand{
Meta: Meta{
testingOverrides: &testingOverrides{
ProviderResolver: providers.ResolverFixed(
map[addrs.Provider]providers.Factory{
addrs.NewLegacyProvider("test-beta"): providers.FactoryFixed(p),
},
),
},
Ui: ui,
},
}

configured := false
p.ConfigureNewFn = func(req providers.ConfigureRequest) providers.ConfigureResponse {
configured = true

cfg := req.Config
if !cfg.Type().HasAttribute("foo") {
return providers.ConfigureResponse{
Diagnostics: tfdiags.Diagnostics{}.Append(fmt.Errorf("configuration has no foo argument")),
}
}
if got, want := cfg.GetAttr("foo"), cty.StringVal("baz"); !want.RawEquals(got) {
return providers.ConfigureResponse{
Diagnostics: tfdiags.Diagnostics{}.Append(fmt.Errorf("foo argument is %#v, but want %#v", got, want)),
}
}

return providers.ConfigureResponse{}
}

p.ImportResourceStateFn = nil
p.ImportResourceStateResponse = providers.ImportResourceStateResponse{
ImportedResources: []providers.ImportedResource{
{
TypeName: "test_instance",
State: cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("yay"),
}),
},
},
}
p.GetSchemaReturn = &terraform.ProviderSchema{
Provider: &configschema.Block{
Attributes: map[string]*configschema.Attribute{
"foo": {Type: cty.String, Optional: true},
},
},
ResourceTypes: map[string]*configschema.Block{
"test_instance": {
Attributes: map[string]*configschema.Attribute{
"id": {Type: cty.String, Optional: true, Computed: true},
},
},
},
}

args := []string{
"-provider", "test-beta",
"-state", statePath,
"test_instance.foo",
"bar",
}

if code := c.Run(args); code != 0 {
t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String())
}

// Verify that the test-beta provider was configured
if !configured {
t.Fatal("Configure should be called")
}

if !p.ImportResourceStateCalled {
t.Fatal("ImportResourceState (provider 'test-beta') should be called")
}

if !p.ReadResourceCalled {
t.Fatal("ReadResource (provider 'test-beta' should be called")
}

testStateOutput(t, statePath, testImportProviderMismatchStr)
}
func TestImport_allowMissingResourceConfig(t *testing.T) {
defer testChdir(t, testFixturePath("import-missing-resource-config"))()

Expand Down