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.

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

r/route53_record - support zero ttl #12481

Closed

Conversation

DrFaust92
Copy link
Collaborator

@DrFaust92 DrFaust92 commented Mar 19, 2020

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Closes #11381, #13527

Release note for CHANGELOG:

resource_aws_route53_record: allow zero value `ttl`

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccAWSRoute53Record_'
--- PASS: TestAccAWSRoute53Record_ttl (471.09s)
--- PASS: TestAccAWSRoute53Record_underscored (242.93s)
--- PASS: TestAccAWSRoute53Record_disappears (214.35s)
--- PASS: TestAccAWSRoute53Record_disappears_MultipleRecords (213.68s)
--- PASS: TestAccAWSRoute53Record_basic_fqdn (283.47s)
--- PASS: TestAccAWSRoute53Record_txtSupport (176.48s)
--- PASS: TestAccAWSRoute53Record_spfSupport (185.99s)
--- PASS: TestAccAWSRoute53Record_caaSupport (182.23s)
--- PASS: TestAccAWSRoute53Record_generatesSuffix (169.05s)
--- PASS: TestAccAWSRoute53Record_wildcard (254.55s)
--- PASS: TestAccAWSRoute53Record_failover (170.21s)
--- PASS: TestAccAWSRoute53Record_weighted_basic (171.75s)
--- PASS: TestAccAWSRoute53Record_weighted_to_simple_basic (243.66s)
--- PASS: TestAccAWSRoute53Record_Alias_Elb (186.29s)
--- PASS: TestAccAWSRoute53Record_Alias_S3 (255.33s)
--- PASS: TestAccAWSRoute53Record_Alias_VpcEndpoint (735.25s)
--- PASS: TestAccAWSRoute53Record_Alias_Uppercase (192.54s)
--- PASS: TestAccAWSRoute53Record_weighted_alias (335.67s)
--- PASS: TestAccAWSRoute53Record_geolocation_basic (170.57s)
--- PASS: TestAccAWSRoute53Record_latency_basic (157.21s)
--- PASS: TestAccAWSRoute53Record_TypeChange (241.77s)
--- PASS: TestAccAWSRoute53Record_SetIdentifierChange (270.79s)
--- PASS: TestAccAWSRoute53Record_empty (172.47s)
--- PASS: TestAccAWSRoute53Record_longTXTrecord (175.81s)
--- PASS: TestAccAWSRoute53Record_multivalue_answer_basic (188.49s)
--- PASS: TestAccAWSRoute53Record_allowOverwrite (264.61s)

@DrFaust92 DrFaust92 requested a review from a team March 19, 2020 17:24
@ghost ghost added needs-triage Waiting for first response or review from a maintainer. size/XL Managed by automation to categorize the size of a PR. service/route53 Issues and PRs that pertain to the route53 service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. labels Mar 19, 2020
@ewbankkit
Copy link
Contributor

Validated acceptance test:

$ make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSRoute53Record_ttl'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -count 1 -parallel 4 -run=TestAccAWSRoute53Record_ttl -timeout 120m
=== RUN   TestAccAWSRoute53Record_ttl
=== PAUSE TestAccAWSRoute53Record_ttl
=== CONT  TestAccAWSRoute53Record_ttl
--- PASS: TestAccAWSRoute53Record_ttl (220.24s)
PASS github.com/terraform-providers/terraform-provider-aws/aws	221.925s

@DrFaust92 DrFaust92 added the bug Addresses a defect in current functionality. label May 22, 2020
@Unichron
Copy link

so why not merge this?

@bflad bflad removed the needs-triage Waiting for first response or review from a maintainer. label Jul 1, 2020
@ewbankkit ewbankkit requested review from ewbankkit and removed request for a team July 25, 2020 20:49
@ewbankkit
Copy link
Contributor

ewbankkit commented Jul 25, 2020

$ make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSRoute53Record_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -count 1 -parallel 20 -run=TestAccAWSRoute53Record_ -timeout 120m
=== RUN   TestAccAWSRoute53Record_basic
=== PAUSE TestAccAWSRoute53Record_basic
=== RUN   TestAccAWSRoute53Record_ttl
=== PAUSE TestAccAWSRoute53Record_ttl
=== RUN   TestAccAWSRoute53Record_underscored
=== PAUSE TestAccAWSRoute53Record_underscored
=== RUN   TestAccAWSRoute53Record_disappears
=== PAUSE TestAccAWSRoute53Record_disappears
=== RUN   TestAccAWSRoute53Record_disappears_MultipleRecords
=== PAUSE TestAccAWSRoute53Record_disappears_MultipleRecords
=== RUN   TestAccAWSRoute53Record_basic_fqdn
=== PAUSE TestAccAWSRoute53Record_basic_fqdn
=== RUN   TestAccAWSRoute53Record_txtSupport
=== PAUSE TestAccAWSRoute53Record_txtSupport
=== RUN   TestAccAWSRoute53Record_spfSupport
=== PAUSE TestAccAWSRoute53Record_spfSupport
=== RUN   TestAccAWSRoute53Record_caaSupport
=== PAUSE TestAccAWSRoute53Record_caaSupport
=== RUN   TestAccAWSRoute53Record_generatesSuffix
=== PAUSE TestAccAWSRoute53Record_generatesSuffix
=== RUN   TestAccAWSRoute53Record_wildcard
=== PAUSE TestAccAWSRoute53Record_wildcard
=== RUN   TestAccAWSRoute53Record_failover
=== PAUSE TestAccAWSRoute53Record_failover
=== RUN   TestAccAWSRoute53Record_weighted_basic
=== PAUSE TestAccAWSRoute53Record_weighted_basic
=== RUN   TestAccAWSRoute53Record_weighted_to_simple_basic
=== PAUSE TestAccAWSRoute53Record_weighted_to_simple_basic
=== RUN   TestAccAWSRoute53Record_Alias_Elb
=== PAUSE TestAccAWSRoute53Record_Alias_Elb
=== RUN   TestAccAWSRoute53Record_Alias_S3
=== PAUSE TestAccAWSRoute53Record_Alias_S3
=== RUN   TestAccAWSRoute53Record_Alias_VpcEndpoint
=== PAUSE TestAccAWSRoute53Record_Alias_VpcEndpoint
=== RUN   TestAccAWSRoute53Record_Alias_Uppercase
=== PAUSE TestAccAWSRoute53Record_Alias_Uppercase
=== RUN   TestAccAWSRoute53Record_weighted_alias
=== PAUSE TestAccAWSRoute53Record_weighted_alias
=== RUN   TestAccAWSRoute53Record_geolocation_basic
=== PAUSE TestAccAWSRoute53Record_geolocation_basic
=== RUN   TestAccAWSRoute53Record_HealthCheckId_SetIdentifierChange
=== PAUSE TestAccAWSRoute53Record_HealthCheckId_SetIdentifierChange
=== RUN   TestAccAWSRoute53Record_HealthCheckId_TypeChange
=== PAUSE TestAccAWSRoute53Record_HealthCheckId_TypeChange
=== RUN   TestAccAWSRoute53Record_latency_basic
=== PAUSE TestAccAWSRoute53Record_latency_basic
=== RUN   TestAccAWSRoute53Record_TypeChange
=== PAUSE TestAccAWSRoute53Record_TypeChange
=== RUN   TestAccAWSRoute53Record_NameChange
=== PAUSE TestAccAWSRoute53Record_NameChange
=== RUN   TestAccAWSRoute53Record_SetIdentifierChange
=== PAUSE TestAccAWSRoute53Record_SetIdentifierChange
=== RUN   TestAccAWSRoute53Record_AliasChange
=== PAUSE TestAccAWSRoute53Record_AliasChange
=== RUN   TestAccAWSRoute53Record_empty
=== PAUSE TestAccAWSRoute53Record_empty
=== RUN   TestAccAWSRoute53Record_longTXTrecord
=== PAUSE TestAccAWSRoute53Record_longTXTrecord
=== RUN   TestAccAWSRoute53Record_multivalue_answer_basic
=== PAUSE TestAccAWSRoute53Record_multivalue_answer_basic
=== RUN   TestAccAWSRoute53Record_allowOverwrite
=== PAUSE TestAccAWSRoute53Record_allowOverwrite
=== CONT  TestAccAWSRoute53Record_ttl
=== CONT  TestAccAWSRoute53Record_Alias_VpcEndpoint
=== CONT  TestAccAWSRoute53Record_allowOverwrite
=== CONT  TestAccAWSRoute53Record_multivalue_answer_basic
=== CONT  TestAccAWSRoute53Record_longTXTrecord
=== CONT  TestAccAWSRoute53Record_empty
=== CONT  TestAccAWSRoute53Record_AliasChange
=== CONT  TestAccAWSRoute53Record_SetIdentifierChange
=== CONT  TestAccAWSRoute53Record_NameChange
=== CONT  TestAccAWSRoute53Record_TypeChange
=== CONT  TestAccAWSRoute53Record_latency_basic
=== CONT  TestAccAWSRoute53Record_HealthCheckId_TypeChange
=== CONT  TestAccAWSRoute53Record_HealthCheckId_SetIdentifierChange
=== CONT  TestAccAWSRoute53Record_geolocation_basic
=== CONT  TestAccAWSRoute53Record_weighted_alias
=== CONT  TestAccAWSRoute53Record_Alias_Uppercase
=== CONT  TestAccAWSRoute53Record_caaSupport
=== CONT  TestAccAWSRoute53Record_Alias_S3
=== CONT  TestAccAWSRoute53Record_Alias_Elb
=== CONT  TestAccAWSRoute53Record_weighted_to_simple_basic
--- PASS: TestAccAWSRoute53Record_empty (139.29s)
=== CONT  TestAccAWSRoute53Record_weighted_basic
--- FAIL: TestAccAWSRoute53Record_AliasChange (142.21s)
    testing.go:684: Step 2 error: errors during apply:
        
        Error: [ERR]: Error building changeset: InvalidInput: Invalid request: Expected exactly one of [AliasTarget, all of [TTL, and ResourceRecords], or TrafficPolicyInstanceId], but found more than one in Change with [Action=DELETE, Name=alias-change.notexample.com, Type=A, SetIdentifier=null]
        	status code: 400, request id: 1b38078c-e97a-4c9c-b6be-1e5b14b34850
        
          on /tmp/tf-test836368083/main.tf line 6:
          (source code not available)
        
        
    testing.go:745: Error destroying resource! WARNING: Dangling resources
        may exist. The full state and error is shown below.
        
        Error: errors during apply: error deleting Route53 Hosted Zone (Z0424096387P6YIE06R85): HostedZoneNotEmpty: The specified hosted zone contains non-required resource record sets  and so cannot be deleted.
        	status code: 400, request id: 4858116d-fad2-4225-a03d-07a0505e915f
        
        State: aws_route53_zone.test:
          ID = Z0424096387P6YIE06R85
          provider = provider.aws
          comment = Managed by Terraform
          delegation_set_id = 
          force_destroy = false
          name = notexample.com.
          name_servers.# = 4
          name_servers.0 = ns-1029.awsdns-00.org
          name_servers.1 = ns-1873.awsdns-42.co.uk
          name_servers.2 = ns-499.awsdns-62.com
          name_servers.3 = ns-898.awsdns-48.net
          tags.% = 0
          vpc.# = 0
          zone_id = Z0424096387P6YIE06R85
=== CONT  TestAccAWSRoute53Record_failover
--- PASS: TestAccAWSRoute53Record_latency_basic (154.26s)
=== CONT  TestAccAWSRoute53Record_wildcard
--- PASS: TestAccAWSRoute53Record_Alias_Uppercase (158.37s)
=== CONT  TestAccAWSRoute53Record_generatesSuffix
--- PASS: TestAccAWSRoute53Record_longTXTrecord (159.33s)
=== CONT  TestAccAWSRoute53Record_disappears_MultipleRecords
--- PASS: TestAccAWSRoute53Record_Alias_Elb (160.49s)
=== CONT  TestAccAWSRoute53Record_spfSupport
--- PASS: TestAccAWSRoute53Record_Alias_S3 (180.73s)
=== CONT  TestAccAWSRoute53Record_txtSupport
--- PASS: TestAccAWSRoute53Record_HealthCheckId_TypeChange (187.48s)
=== CONT  TestAccAWSRoute53Record_basic_fqdn
--- PASS: TestAccAWSRoute53Record_caaSupport (192.99s)
=== CONT  TestAccAWSRoute53Record_underscored
--- PASS: TestAccAWSRoute53Record_multivalue_answer_basic (195.78s)
=== CONT  TestAccAWSRoute53Record_disappears
--- PASS: TestAccAWSRoute53Record_TypeChange (206.12s)
=== CONT  TestAccAWSRoute53Record_basic
--- PASS: TestAccAWSRoute53Record_SetIdentifierChange (206.51s)
--- PASS: TestAccAWSRoute53Record_weighted_to_simple_basic (208.59s)
--- PASS: TestAccAWSRoute53Record_allowOverwrite (226.29s)
--- PASS: TestAccAWSRoute53Record_HealthCheckId_SetIdentifierChange (229.47s)
--- PASS: TestAccAWSRoute53Record_ttl (235.07s)
--- PASS: TestAccAWSRoute53Record_geolocation_basic (238.95s)
--- PASS: TestAccAWSRoute53Record_NameChange (256.64s)
--- PASS: TestAccAWSRoute53Record_weighted_basic (123.96s)
--- PASS: TestAccAWSRoute53Record_failover (121.82s)
--- PASS: TestAccAWSRoute53Record_spfSupport (133.37s)
--- PASS: TestAccAWSRoute53Record_generatesSuffix (138.02s)
--- PASS: TestAccAWSRoute53Record_txtSupport (127.48s)
--- PASS: TestAccAWSRoute53Record_basic_fqdn (128.06s)
--- PASS: TestAccAWSRoute53Record_weighted_alias (317.74s)
--- PASS: TestAccAWSRoute53Record_underscored (126.97s)
--- PASS: TestAccAWSRoute53Record_disappears (127.14s)
--- PASS: TestAccAWSRoute53Record_disappears_MultipleRecords (171.60s)
--- PASS: TestAccAWSRoute53Record_basic (126.04s)
--- PASS: TestAccAWSRoute53Record_wildcard (199.24s)
--- PASS: TestAccAWSRoute53Record_Alias_VpcEndpoint (458.76s)
FAIL
FAIL	github.com/terraform-providers/terraform-provider-aws/aws	458.874s
FAIL
GNUmakefile:26: recipe for target 'testacc' failed
make: *** [testacc] Error 1

TestAccAWSRoute53Record_AliasChange passes on the current HEAD:

$ make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSRoute53Record_AliasChange'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSRoute53Record_AliasChange -timeout 120m
=== RUN   TestAccAWSRoute53Record_AliasChange
=== PAUSE TestAccAWSRoute53Record_AliasChange
=== CONT  TestAccAWSRoute53Record_AliasChange
--- PASS: TestAccAWSRoute53Record_AliasChange (149.29s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	149.351s

Some hints here:

It looks like something to do with TTL and alias records.

@ewbankkit ewbankkit self-requested a review July 25, 2020 22:13
@DrFaust92 DrFaust92 force-pushed the r/route53_record-zero-ttl branch 2 times, most recently from 9dae8f6 to 7da318a Compare July 31, 2020 19:20
@@ -1900,7 +1911,7 @@ resource "aws_elb" "live" {
}

resource "aws_route53_record" "elb_weighted_alias_live" {
zone_id = aws_route53_zone.main.zone_id
zone_id = aws_route53_zone.test.zone_id
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to change

resource "aws_route53_zone" "main" {
  name = "notexample.com"
}

above to

resource "aws_route53_zone" "test" {
  name = "notexample.com"
}

else get error

=== CONT  TestAccAWSRoute53Record_weighted_alias
    resource_aws_route53_record_test.go:658: Step 1/3 error: terraform failed: exit status 1
        
        stderr:
        
        Error: Reference to undeclared resource
        
          on config868206730/terraform_plugin_test.tf line 28, in resource "aws_route53_record" "elb_weighted_alias_live":
          28:   zone_id = aws_route53_zone.test.zone_id
        
        A managed resource "aws_route53_zone" "test" has not been declared in the root
        module.
        
        
        Error: Reference to undeclared resource
        
          on config868206730/terraform_plugin_test.tf line 58, in resource "aws_route53_record" "elb_weighted_alias_dev":
          58:   zone_id = aws_route53_zone.test.zone_id
        
        A managed resource "aws_route53_zone" "test" has not been declared in the root
        module.
        
--- FAIL: TestAccAWSRoute53Record_weighted_alias (1.39s)

@ewbankkit
Copy link
Contributor

ewbankkit commented Aug 23, 2020

Still getting failure in TestAccAWSRoute53Record_AliasChange:

=== CONT  TestAccAWSRoute53Record_AliasChange
    resource_aws_route53_record_test.go:917: Step 3/3 error: terraform failed: exit status 1
        
        stderr:
        
        Error: [ERR]: Error building changeset: InvalidInput: Invalid request: Expected exactly one of [AliasTarget, all of [TTL, and ResourceRecords], or TrafficPolicyInstanceId], but found more than one in Change with [Action=DELETE, Name=alias-change.notexample.com, Type=A, SetIdentifier=null]
        	status code: 400, request id: a2217f6f-a500-4da4-ae82-fa0df9941cca
        
        
=== CONT  TestAccAWSRoute53Record_AliasChange
    testing_new.go:22: WARNING: destroy failed, so remote objects may still exist and be subject to billing
    testing_new.go:22: failed to destroy: terraform failed: exit status 1
        
        stderr:
        
        Error: error deleting Route53 Hosted Zone (Z06426261RT8ANWYCHOFT): HostedZoneNotEmpty: The specified hosted zone contains non-required resource record sets  and so cannot be deleted.
        	status code: 400, request id: 37431717-a0a9-4f8e-92d0-53d6a5c96f06
        
        
        
--- FAIL: TestAccAWSRoute53Record_AliasChange (186.44s)

The error message suggests that a single route53.Change should contain either

@DrFaust92
Copy link
Collaborator Author

Hmm.. this might require some more and to split this to separate route53.Change events one after another. it doesn't lend itself to a terraform Declarative approach, WDYT?

@DrFaust92 DrFaust92 marked this pull request as draft August 27, 2020 17:12
@DrFaust92
Copy link
Collaborator Author

Closing this as i dont have attention to properly research this so if anyone want to take over and fork you are more than welcome.

@DrFaust92 DrFaust92 closed this Dec 1, 2020
@ghost
Copy link

ghost commented Jan 1, 2021

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!

@ghost ghost locked as resolved and limited conversation to collaborators Jan 1, 2021
@DrFaust92 DrFaust92 deleted the r/route53_record-zero-ttl branch April 15, 2021 10:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Addresses a defect in current functionality. service/route53 Issues and PRs that pertain to the route53 service. size/XL Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

aws_route53_record doesn't allow ttl = 0
4 participants