Make dnsimple_records importable

[yob]

This makes the dnsimple_record resource importable.

Unfortunately, the DNSimple v2 API requires a domain name and record ID to fetch a record, so the import command accepts both pieces of data as an underscore delimited string like so:

terraform import dnsimple_record.test example.com_1234
terraform import module.foo.dnsimple_record.test example.com_1234

The underscore delimited approach is very similar to the one used to import an aws_route53_record resource.

The numeric resource ID is visible in the URL when editing a record on the dnsimple dashboard (ie. the 1234 in https://dnsimple.com/a/xxx/domains/example.com/records/1234/edit

[yob]

Before I continue down this path much further, I'm interested in feedback on the approach.

Do any other providers require 2+ pieces of data as inputs to the import? Is a / delimited string a suitable approach or should I try something else?

[yob]

@stack72 are you able to offer any feedback on this PR as well?

I've attempted to add support for importing dnsimple_record's, and manually confirmed it works. The main challenge I have left is dealing with the Dnsimple APIs requirement that domain records are looked up via the domain+unique ID combo. I've built most of an acceptance test, but stumbled at the finish line.

[sj26]

This looks super neat!

Could you read the domain itself as well to confirm the ID/name? This would mean it could be imported with either ID or domain name as a single parameter, too.

[yob]

Thanks @sj26

Could you read the domain itself as well to confirm the ID/name? This would mean it could be imported with either ID or domain name as a single parameter, too.

Could you describe this in a bit more detail? I'm still wrapping my head around both terraform and the dnsimple API, so I haven't totally grokked your suggestion.

[yob]

@stack72 I've rebased this on to master (to get the dnsimple client changes from #10760) and manually confirmed it still works.

Can you offer any guidance before I go on to explore tests and documentation?

[stack72]

Hi @yob

This is looking good! We follow a similar pattern for importing AWS Route53 Records

Paul

[yob]

Cheers, I'll checkout the route53 example for tips

[yob]

@stack72 I've rebased this onto master, and adjusted the import syntax to use the same delimiter (_) that's used to import aws_route53_record records.

I've added a spec and some documentation, and I've confirmed the import works in my dev environment. Does this need anything else to be considered for merging?

[yob]

hi @stack72 - when you have time I'd love some additional feedback on this

[yob]

sorry to be a broken wheel @stack72, but this is the only remaining blocker to us using terraform in production. Is there anything I can do to get it over the line?

[stack72]

Hi @yob

Apologies for the lack of response here - this repo is crazy busy and this slipped past my radar! The code looks good to me :) The tests don't quite pass as expected:

% make testacc TEST=./builtin/providers/dnsimple                                                                          130 ↵ ✭
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/04/26 11:38:42 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/dnsimple -v  -timeout 120m
=== RUN   TestAccDnsimpleRecord_import
--- FAIL: TestAccDnsimpleRecord_import (4.60s)
	testing.go:280: Step 1 error: 1 error(s) occurred:

		* dnsimple_record.foobar (import id: 11520301): import dnsimple_record.foobar (id: 11520301): Error Importing dnsimple_record. Please make sure the record ID is in the form DOMAIN_RECORDID (i.e. example.com_1234
=== RUN   TestProvider
--- PASS: TestProvider (0.00s)
=== RUN   TestProvider_impl
--- PASS: TestProvider_impl (0.00s)
=== RUN   TestAccDNSimpleRecord_Basic
--- PASS: TestAccDNSimpleRecord_Basic (3.49s)
=== RUN   TestAccDNSimpleRecord_CreateMxWithPriority
--- PASS: TestAccDNSimpleRecord_CreateMxWithPriority (3.62s)
=== RUN   TestAccDNSimpleRecord_Updated
--- PASS: TestAccDNSimpleRecord_Updated (5.97s)
=== RUN   TestAccDNSimpleRecord_disappears
--- PASS: TestAccDNSimpleRecord_disappears (2.57s)
=== RUN   TestAccDNSimpleRecord_UpdatedMx
--- PASS: TestAccDNSimpleRecord_UpdatedMx (5.97s)
FAIL
exit status 1
FAIL	github.com/hashicorp/terraform/builtin/providers/dnsimple	26.233s
make: *** [testacc] Error 1

Paul

[yob]

Thanks for the feedback, and apologies for not noticing that the travis build doesn't include acceptance specs.

I've reproduced the failure locally and had a go at resolving it, but I'm bumping up against my terraform knowledge. The import command I've added requires the ID to be provided as <domain>_<id>, but the default ImportState test builder sets it to just <id>. I could override the default by setting ImportStateId in my TestStep - but how can I look up the dnsimple ID for the record that we want to import?

[yob]

@stack72 I think I found a solution. By rebasing onto master, I was able to use ImportStateIdPrefix (added in #13226) to get the acceptance spec green.

Apologies for the force push.

[stack72]

Yay! Nice work @yob :) Tests are now looking great!

% make testacc TEST=./builtin/providers/dnsimple                                                               130 ↵ ✭
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/04/27 02:06:36 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/dnsimple -v  -timeout 120m
=== RUN   TestAccDnsimpleRecord_import
--- PASS: TestAccDnsimpleRecord_import (2.75s)
=== RUN   TestProvider
--- PASS: TestProvider (0.00s)
=== RUN   TestProvider_impl
--- PASS: TestProvider_impl (0.00s)
=== RUN   TestAccDNSimpleRecord_Basic
--- PASS: TestAccDNSimpleRecord_Basic (1.87s)
=== RUN   TestAccDNSimpleRecord_CreateMxWithPriority
--- PASS: TestAccDNSimpleRecord_CreateMxWithPriority (1.86s)
=== RUN   TestAccDNSimpleRecord_Updated
--- PASS: TestAccDNSimpleRecord_Updated (2.96s)
=== RUN   TestAccDNSimpleRecord_disappears
--- PASS: TestAccDNSimpleRecord_disappears (1.32s)
=== RUN   TestAccDNSimpleRecord_UpdatedMx
--- PASS: TestAccDNSimpleRecord_UpdatedMx (3.39s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/dnsimple	14.163s

Thanks for this - merging now

Paul

[yob]

Thanks!

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.