resource/aws_service_discovery_service: Support routing policy and update the type of dns record

[atsushi-ishibashi]

Required: #3272
After merged, I'll remove the commit.

TF_ACC=1 go test ./aws -v -run=TestAccAwsServiceDiscoveryService_* -timeout 120m
=== RUN   TestAccAwsServiceDiscoveryService_private
--- PASS: TestAccAwsServiceDiscoveryService_private (115.73s)
=== RUN   TestAccAwsServiceDiscoveryService_public
--- PASS: TestAccAwsServiceDiscoveryService_public (83.12s)
=== RUN   TestAccAwsServiceDiscoveryService_import
--- PASS: TestAccAwsServiceDiscoveryService_import (66.99s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	265.892s

[radeksimko]

Hi @atsushi-ishibashi
do you mind raising a separate PR for vendor updates?

Thanks.

[radeksimko]

Ah, sorry it was raised in #3272 😅 - which was merged, in which case do you mind rebasing the PR? 😃

[atsushi-ishibashi]

@radeksimko OK👍

[radeksimko]

Thanks for the PR @atsushi-ishibashi

The old version of config should still work after upgrading the provider. We shouldn't be forcing the user to add the new field to their config - or that would be considered a breaking change.

We can avoid the breaking change by setting the field to MULTIVALUE during state migration. Do you think you could look into adding a state migration?

[radeksimko]

Some funny indentation here 👀

[radeksimko]

Since the API defaults to MULTIVALUE shouldn't we also set it as Default here to avoid diffs when the field is not defined?

[atsushi-ishibashi]

@radeksimko Ok👍

TF_ACC=1 go test ./aws -v -run=TestAwsServiceDiscoveryServiceMigrateState -timeout 120m
=== RUN   TestAwsServiceDiscoveryServiceMigrateState
2018/02/12 00:45:52 [INFO] Found AWS ServiceDiscovery Service State v0; migrating to v1
2018/02/12 00:45:52 [DEBUG] Attributes before migration: map[string]string{"dns_config.#":"1", "dns_config.0.namespace_id":"test-namespace", "dns_config.0.dns_records.#":"1", "dns_config.0.dns_records.0.ttl":"10", "dns_config.0.dns_records.0.type":"A", "arn":"arn", "name":"test-name"}
2018/02/12 00:45:52 [DEBUG] Attributes after migration: map[string]string{"dns_config.0.dns_records.0.type":"A", "arn":"arn", "dns_config.0.routing_policy":"MULTIVALUE", "name":"test-name", "dns_config.#":"1", "dns_config.0.namespace_id":"test-namespace", "dns_config.0.dns_records.#":"1", "dns_config.0.dns_records.0.ttl":"10"}
--- PASS: TestAwsServiceDiscoveryServiceMigrateState (0.00s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	0.032s

[radeksimko]

Thanks @atsushi-ishibashi
this LGTM, I just removed the new field from one test so we have some coverage in that area too. After all we shouldn't just assume everyone will use this field.

[bflad]

This has been released in version 1.10.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

[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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!