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

Jump to bottom

resources/aws_route53_record: Fix set_identifier with underscore #13453

Merged
merged 2 commits into from
Aug 27, 2021
Merged

resources/aws_route53_record: Fix set_identifier with underscore #13453

merged 2 commits into from
Aug 27, 2021

Conversation

angeloskaltsikis
Copy link
Contributor

@angeloskaltsikis angeloskaltsikis commented May 21, 2020 •
edited
Loading

As mentioned in detail in #11677
& #6298
there was a bug if we supplied a set_identifier which should contain an
underscore in.
This fix changes the logic by searching the first occurrences of underscores
to calculate the correct values for each field and as a result for the
set_identifier as well.
The tests included proves that the change doesn't break something and also
fixes the aforementioned bug.

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 #11677

Release note for CHANGELOG:

resources/aws_route53_record: Fix set_identifier with underscore

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccXXX'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./... -v -count 1 -parallel 20 -run=TestAccXXX -timeout 120m
?   	github.com/terraform-providers/terraform-provider-aws	[no test files]
testing: warning: no tests to run
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	1.418s [no tests to run]
testing: warning: no tests to run
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws/internal/flatmap	0.744s [no tests to run]
testing: warning: no tests to run
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags	0.418s [no tests to run]
testing: warning: no tests to run
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws/internal/naming	0.759s [no tests to run]
?   	github.com/terraform-providers/terraform-provider-aws/aws/internal/service/apigatewayv2/waiter	[no test files]
testing: warning: no tests to run
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws/internal/service/batch/equivalency	0.304s [no tests to run]
testing: warning: no tests to run
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws/internal/service/eks/token	0.250s [no tests to run]
?   	github.com/terraform-providers/terraform-provider-aws/aws/internal/service/guardduty/waiter	[no test files]
?   	github.com/terraform-providers/terraform-provider-aws/aws/internal/service/iam/waiter	[no test files]
?   	github.com/terraform-providers/terraform-provider-aws/aws/internal/service/kinesisanalytics/waiter	[no test files]
?   	github.com/terraform-providers/terraform-provider-aws/aws/internal/service/kms/waiter	[no test files]
?   	github.com/terraform-providers/terraform-provider-aws/aws/internal/service/neptune/waiter	[no test files]
?   	github.com/terraform-providers/terraform-provider-aws/aws/internal/service/rds/waiter	[no test files]
?   	github.com/terraform-providers/terraform-provider-aws/aws/internal/service/secretsmanager/waiter	[no test files]
?   	github.com/terraform-providers/terraform-provider-aws/aws/internal/service/servicediscovery/waiter	[no test files]
?   	github.com/terraform-providers/terraform-provider-aws/awsproviderlint	[no test files]
?   	github.com/terraform-providers/terraform-provider-aws/awsproviderlint/helper/awsprovidertype/keyvaluetags	[no test files]
testing: warning: no tests to run
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/awsproviderlint/passes	0.931s [no tests to run]
testing: warning: no tests to run
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/awsproviderlint/passes/AWSAT001	1.079s [no tests to run]
testing: warning: no tests to run
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/awsproviderlint/passes/AWSR001	0.244s [no tests to run]
testing: warning: no tests to run
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/awsproviderlint/passes/AWSR002	0.387s [no tests to run]
testing: warning: no tests to run
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/awsproviderlint/passes/fmtsprintfcallexpr	0.361s [no tests to run]

@angeloskaltsikis angeloskaltsikis requested a review from a team May 21, 2020 11:11
@ghost ghost added size/XS 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. needs-triage Waiting for first response or review from a maintainer. labels May 21, 2020
@angeloskaltsikis angeloskaltsikis mentioned this pull request May 21, 2020
Closed
@ewbankkit
Copy link
Contributor

Closes #13457.

@bflad bflad added bug Addresses a defect in current functionality. and removed needs-triage Waiting for first response or review from a maintainer. labels May 21, 2020
@bflad bflad linked an issue May 21, 2020 that may be closed by this pull request
Route 53 record ID parse fails when set ID contains underscores #13457
Closed
@angeloskaltsikis
Copy link
Contributor Author

Hello friends,
is something missing from this PR to be approved ?

Base automatically changed from master to main January 23, 2021 00:57
@breathingdust breathingdust requested a review from a team as a code owner January 23, 2021 00:57
@ewbankkit ewbankkit self-assigned this Aug 27, 2021
angeloskaltsikis and others added 2 commits August 27, 2021 16:34
@angeloskaltsikis @ewbankkit
resources/aws_route53_record: Fix set_identifier with underscore
f15427d 
As mentioned in detail in hashicorp#11677
& hashicorp#6298
there was a bug if we supplied a `set_identifier` which should contain an
underscore in.
This fix changes the logic by searching the first occurrences of underscores
to calculate the correct values for each field and as a result for the
`set_identifier` as well.
The tests included proves that the change doesn't break something and also
fixes the aforementioned bug.
@ewbankkit
Add CHANGELOG entry.
33b2970
ewbankkit
ewbankkit approved these changes Aug 27, 2021
View reviewed changes
Copy link
Contributor

@ewbankkit ewbankkit left a comment

Choose a reason for hiding this comment

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

LGTM 🚀.

% 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 180m
=== RUN   TestAccAWSRoute53Record_basic
=== PAUSE TestAccAWSRoute53Record_basic
=== 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_basic_trailingPeriodAndZoneID
=== PAUSE TestAccAWSRoute53Record_basic_trailingPeriodAndZoneID
=== RUN   TestAccAWSRoute53Record_txtSupport
=== PAUSE TestAccAWSRoute53Record_txtSupport
=== RUN   TestAccAWSRoute53Record_spfSupport
=== PAUSE TestAccAWSRoute53Record_spfSupport
=== RUN   TestAccAWSRoute53Record_caaSupport
=== PAUSE TestAccAWSRoute53Record_caaSupport
=== RUN   TestAccAWSRoute53Record_dsSupport
=== PAUSE TestAccAWSRoute53Record_dsSupport
=== 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_doNotAllowOverwrite
=== PAUSE TestAccAWSRoute53Record_doNotAllowOverwrite
=== RUN   TestAccAWSRoute53Record_allowOverwrite
=== PAUSE TestAccAWSRoute53Record_allowOverwrite
=== CONT  TestAccAWSRoute53Record_basic
=== CONT  TestAccAWSRoute53Record_Alias_VpcEndpoint
=== CONT  TestAccAWSRoute53Record_Alias_Elb
=== CONT  TestAccAWSRoute53Record_basic_fqdn
=== CONT  TestAccAWSRoute53Record_spfSupport
=== CONT  TestAccAWSRoute53Record_Alias_S3
=== CONT  TestAccAWSRoute53Record_caaSupport
=== CONT  TestAccAWSRoute53Record_basic_trailingPeriodAndZoneID
=== CONT  TestAccAWSRoute53Record_disappears_MultipleRecords
=== CONT  TestAccAWSRoute53Record_dsSupport
=== CONT  TestAccAWSRoute53Record_allowOverwrite
=== CONT  TestAccAWSRoute53Record_doNotAllowOverwrite
=== CONT  TestAccAWSRoute53Record_multivalue_answer_basic
=== CONT  TestAccAWSRoute53Record_longTXTrecord
=== CONT  TestAccAWSRoute53Record_empty
=== CONT  TestAccAWSRoute53Record_AliasChange
=== CONT  TestAccAWSRoute53Record_SetIdentifierChange
=== CONT  TestAccAWSRoute53Record_txtSupport
=== CONT  TestAccAWSRoute53Record_NameChange
=== CONT  TestAccAWSRoute53Record_TypeChange
--- PASS: TestAccAWSRoute53Record_Alias_Elb (130.57s)
=== CONT  TestAccAWSRoute53Record_failover
--- PASS: TestAccAWSRoute53Record_doNotAllowOverwrite (138.19s)
=== CONT  TestAccAWSRoute53Record_latency_basic
--- PASS: TestAccAWSRoute53Record_caaSupport (144.54s)
=== CONT  TestAccAWSRoute53Record_weighted_to_simple_basic
--- PASS: TestAccAWSRoute53Record_basic_trailingPeriodAndZoneID (154.87s)
=== CONT  TestAccAWSRoute53Record_HealthCheckId_TypeChange
--- PASS: TestAccAWSRoute53Record_longTXTrecord (155.14s)
=== CONT  TestAccAWSRoute53Record_weighted_basic
--- PASS: TestAccAWSRoute53Record_txtSupport (165.90s)
=== CONT  TestAccAWSRoute53Record_HealthCheckId_SetIdentifierChange
--- PASS: TestAccAWSRoute53Record_basic (167.35s)
=== CONT  TestAccAWSRoute53Record_geolocation_basic
--- PASS: TestAccAWSRoute53Record_empty (168.11s)
=== CONT  TestAccAWSRoute53Record_disappears
--- PASS: TestAccAWSRoute53Record_spfSupport (172.19s)
=== CONT  TestAccAWSRoute53Record_weighted_alias
--- PASS: TestAccAWSRoute53Record_multivalue_answer_basic (175.98s)
=== CONT  TestAccAWSRoute53Record_Alias_Uppercase
--- PASS: TestAccAWSRoute53Record_Alias_S3 (177.15s)
=== CONT  TestAccAWSRoute53Record_generatesSuffix
--- PASS: TestAccAWSRoute53Record_dsSupport (179.82s)
=== CONT  TestAccAWSRoute53Record_wildcard
--- PASS: TestAccAWSRoute53Record_basic_fqdn (181.24s)
=== CONT  TestAccAWSRoute53Record_underscored
--- PASS: TestAccAWSRoute53Record_SetIdentifierChange (190.84s)
--- PASS: TestAccAWSRoute53Record_disappears_MultipleRecords (217.51s)
--- PASS: TestAccAWSRoute53Record_AliasChange (219.00s)
--- PASS: TestAccAWSRoute53Record_allowOverwrite (225.86s)
--- PASS: TestAccAWSRoute53Record_TypeChange (232.18s)
--- PASS: TestAccAWSRoute53Record_NameChange (277.80s)
--- PASS: TestAccAWSRoute53Record_failover (151.22s)
--- PASS: TestAccAWSRoute53Record_latency_basic (151.95s)
--- PASS: TestAccAWSRoute53Record_weighted_basic (136.28s)
--- PASS: TestAccAWSRoute53Record_generatesSuffix (135.22s)
--- PASS: TestAccAWSRoute53Record_disappears (145.73s)
--- PASS: TestAccAWSRoute53Record_weighted_to_simple_basic (171.57s)
--- PASS: TestAccAWSRoute53Record_Alias_Uppercase (141.89s)
--- PASS: TestAccAWSRoute53Record_underscored (138.52s)
--- PASS: TestAccAWSRoute53Record_geolocation_basic (160.58s)
--- PASS: TestAccAWSRoute53Record_HealthCheckId_TypeChange (176.85s)
--- PASS: TestAccAWSRoute53Record_wildcard (204.54s)
--- PASS: TestAccAWSRoute53Record_HealthCheckId_SetIdentifierChange (253.20s)
--- PASS: TestAccAWSRoute53Record_weighted_alias (297.85s)
--- PASS: TestAccAWSRoute53Record_Alias_VpcEndpoint (570.23s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	573.903s

@ewbankkit ewbankkit merged commit f4f2d2b into hashicorp:main Aug 27, 2021
@ewbankkit
Copy link
Contributor

@angeloskaltsikis Thanks for the contribution 🎉 👏.

@github-actions github-actions bot added this to the v3.57.0 milestone Aug 27, 2021
@angeloskaltsikis angeloskaltsikis deleted the fix-11677 branch August 30, 2021 09:26
@github-actions
Copy link

github-actions bot commented Sep 2, 2021

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

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

@jferris jferris mentioned this pull request Sep 5, 2021
Closed
@ewbankkit ewbankkit mentioned this pull request Sep 12, 2021
Closed
@github-actions
Copy link

I'm going to lock this pull request 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 related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ewbankkit ewbankkit ewbankkit approved these changes

Assignees

@ewbankkit ewbankkit

Labels
bug Addresses a defect in current functionality. service/route53 Issues and PRs that pertain to the route53 service. size/XS 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
Milestone
v3.57.0
3 participants
@angeloskaltsikis @ewbankkit @bflad