provider/aws: Make the type of a route53_record changeable

[johanneswuerbach]

Utilize the ChangeResourceRecordSets to change the type of a record by
deleting and recreating with a new type.

As change batches are considered transactional changes, Amazon Route 53
either makes all or none of the changes in the batch request ensuring the
update will never be partially applied.

Fixes #9732

There was already an acceptance test for a type change present

terraform/builtin/providers/aws/resource_aws_route53_record_test.go

Line 317 in cb8d7bf

func TestAccAWSRoute53Record_TypeChange(t *testing.T) {

so I just added one for "upgrading" a regular A record to a weighted one, which would previously also required remove & add and one for changing the type of an alias record.

[catsby]

Hey @johanneswuerbach thanks for the fix! This looks good to me, but I'm going to delay merging it a bit. #9732 probably shouldn't happen, especially as you point out we have a test that does just that without failing 🤔

That said, I think this change is good by itself; leveraging R53's transactional changes sounds like a good move, but I still want to dig into #9732 a bit first.

Test runs:

TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSRoute53Record_ -timeout 120m
=== RUN   TestAccAWSRoute53Record_basic
--- PASS: TestAccAWSRoute53Record_basic (118.55s)
=== RUN   TestAccAWSRoute53Record_basic_fqdn
--- PASS: TestAccAWSRoute53Record_basic_fqdn (151.96s)
=== RUN   TestAccAWSRoute53Record_txtSupport
--- PASS: TestAccAWSRoute53Record_txtSupport (129.82s)
=== RUN   TestAccAWSRoute53Record_spfSupport
--- PASS: TestAccAWSRoute53Record_spfSupport (153.31s)
=== RUN   TestAccAWSRoute53Record_generatesSuffix
--- PASS: TestAccAWSRoute53Record_generatesSuffix (581.60s)
=== RUN   TestAccAWSRoute53Record_wildcard
--- PASS: TestAccAWSRoute53Record_wildcard (180.88s)
=== RUN   TestAccAWSRoute53Record_failover
--- PASS: TestAccAWSRoute53Record_failover (129.84s)
=== RUN   TestAccAWSRoute53Record_weighted_basic
--- PASS: TestAccAWSRoute53Record_weighted_basic (138.50s)
=== RUN   TestAccAWSRoute53Record_alias
--- PASS: TestAccAWSRoute53Record_alias (142.58s)
=== RUN   TestAccAWSRoute53Record_s3_alias
--- PASS: TestAccAWSRoute53Record_s3_alias (151.02s)
=== RUN   TestAccAWSRoute53Record_weighted_alias
--- PASS: TestAccAWSRoute53Record_weighted_alias (270.54s)
=== RUN   TestAccAWSRoute53Record_geolocation_basic
--- PASS: TestAccAWSRoute53Record_geolocation_basic (130.21s)
=== RUN   TestAccAWSRoute53Record_latency_basic
--- PASS: TestAccAWSRoute53Record_latency_basic (144.45s)
=== RUN   TestAccAWSRoute53Record_TypeChange
--- PASS: TestAccAWSRoute53Record_TypeChange (175.59s)
=== RUN   TestAccAWSRoute53Record_SetIdentiferChange
--- PASS: TestAccAWSRoute53Record_SetIdentiferChange (188.76s)
=== RUN   TestAccAWSRoute53Record_AliasChange
--- PASS: TestAccAWSRoute53Record_AliasChange (163.21s)
=== RUN   TestAccAWSRoute53Record_empty
--- PASS: TestAccAWSRoute53Record_empty (118.32s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    3069.176s

[johanneswuerbach]

Thanks @catsby for the update, I currently don't have much time to test this, but maybe the way IDs are build https://github.com/johanneswuerbach/terraform/blob/ef0196f75477896504e91426c3912feaeefa9eff/builtin/providers/aws/resource_aws_route53_record.go#L391-L396 is the issue?

Shouldn't the zone & name be enough to identify a record, especially as the same zone can't have the same name multiple times?

[catsby]

Going to pull this in, thanks again!

[catsby]

👍

[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.