resource/route53_zone and data-source/route53_zone: remove trailing period from name attributes #14220

anGie44 · 2020-07-16T22:36:53Z

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 #13510
Closes #13010
Closes #7501
Closes #7800
Closes #6389
Closes #241
Related #1031

Release note for CHANGELOG:

service/route53: remove trailing period from name attributes in zone resource and data-source

Output from acceptance testing:

--- PASS: TestAccAWSRoute53Zone_basic (51.18s)
--- PASS: TestAccAWSRoute53Zone_multiple (53.74s)
--- PASS: TestAccAWSRoute53Zone_DelegationSetID (46.19s)
--- PASS: TestAccAWSRoute53Zone_Comment (59.44s)
--- PASS: TestAccAWSRoute53Zone_disappears (41.69s)
--- PASS: TestAccAWSRoute53Zone_Tags (63.88s)
--- PASS: TestAccAWSRoute53Zone_VPC_Single (86.49s)
--- PASS: TestAccAWSRoute53Zone_VPC_Multiple (163.38s)
--- PASS: TestAccAWSRoute53Zone_ForceDestroy (190.02s)
--- PASS: TestAccAWSRoute53Zone_ForceDestroy_TrailingPeriod (186.48s)
--- PASS: TestAccAWSRoute53Zone_VPC_Updates (251.37s)

--- PASS: TestAccAWSRoute53ZoneAssociation_region (171.75s)
--- PASS: TestAccAWSRoute53ZoneAssociation_disappears (144.77s)
--- PASS: TestAccAWSRoute53ZoneAssociation_disappears_Zone (150.37s)
--- PASS: TestAccAWSRoute53ZoneAssociation_disappears_VPC (164.37s)
--- PASS: TestAccAWSRoute53ZoneAssociation_basic (164.55s)

anGie44 · 2020-07-27T22:23:49Z

Ready for re-review :)

Output of acceptance tests:

--- PASS: TestSuppressEquivalentTypeStringBoolean (0.00s)
--- PASS: TestSuppressEquivalentJsonDiffsWhitespaceAndNoWhitespace (0.00s)
--- PASS: TestSuppressCloudFormationTemplateBodyDiffs (0.00s)
--- PASS: TestAccAWSAcmCertificateDataSource_noMatchReturnsError (7.66s)
--- PASS: TestAccAWSAcmCertificate_imported_IpAddress (108.98s)
--- PASS: TestAccAWSAcmCertificate_san_single (120.36s)
--- PASS: TestAccAWSAcmCertificate_wildcard (121.21s)
--- PASS: TestAccAWSAcmCertificate_wildcardAndRootSan (133.47s)
--- PASS: TestAccAWSAcmCertificate_root_TrailingPeriod (149.46s)
--- PASS: TestAccAWSAcmCertificate_rootAndWildcardSan (150.02s)
--- PASS: TestAccAWSAcmCertificate_disableCTLogging (177.62s)
--- PASS: TestAccAWSAcmCertificate_root (177.66s)
--- PASS: TestAccAWSAcmCertificate_dnsValidation (177.67s)
--- PASS: TestAccAWSAcmCertificate_privateCert (178.54s)
--- PASS: TestAccAWSAcmCertificate_san_multiple (178.78s)
--- PASS: TestAccAWSAcmCertificate_emailValidation (191.74s)
--- PASS: TestAccAWSAcmCertificate_san_TrailingPeriod (193.02s)
--- PASS: TestAccAWSAcmCertificate_imported_DomainName (204.93s)
--- PASS: TestAccAWSAcmCertificate_tags (215.43s)
--- PASS: TestAccAWSAcmCertificateValidation_validationRecordFqdnsEmail (15.02s)
--- PASS: TestAccAWSAcmCertificateValidation_timeout (17.08s)
--- PASS: TestAccAWSAcmCertificateValidation_validationRecordFqdns (128.92s)
--- PASS: TestAccAWSAcmCertificateValidation_basic (197.69s)
--- PASS: TestAccAWSAcmCertificateValidation_validationRecordFqdnsSan (200.71s)
--- PASS: TestAccAWSAcmCertificateValidation_validationRecordFqdnsRootAndWildcard (424.06s)
--- PASS: TestAccAWSAcmCertificateValidation_validationRecordFqdnsWildcardAndRoot (424.97s)
--- PASS: TestAccAWSAcmCertificateValidation_validationRecordFqdnsRoot (144.67s)
--- PASS: TestAccDataSourceAwsRoute53ResolverRule_SharedByMe (238.96s)
--- PASS: TestCleanRecordName (0.00s)
--- PASS: TestExpandRecordName (0.00s)
--- PASS: TestNormalizeAwsAliasName (0.00s)
--- PASS: TestParseRecordId (0.00s)
--- PASS: TestAccDataSourceAwsRoute53ResolverRules_basic (13.10s)
--- PASS: TestAccAWSAcmCertificate_imported_IpAddress (14.54s)
--- PASS: TestAccAWSAcmCertificate_privateCert (18.49s)
--- PASS: TestAccAWSAcmCertificateDataSource_KeyTypes (19.38s)
--- PASS: TestAccAWSAcmCertificate_imported_DomainName (28.46s)
--- PASS: TestAccDataSourceAwsRoute53ResolverRule_basic (35.18s)
--- PASS: TestAccDataSourceAwsRoute53Zone_name (55.68s)
--- PASS: TestAccDataSourceAwsRoute53Zone_id (59.90s)
--- PASS: TestAccDataSourceAwsRoute53Zone_serviceDiscovery (109.69s)
--- PASS: TestAccAWSRoute53Record_disappears (130.63s)
--- PASS: TestAccAWSRoute53Record_basic (132.54s)
--- PASS: TestAccAWSRoute53Record_spfSupport (138.40s)
--- PASS: TestAccAWSRoute53Record_txtSupport (147.18s)
--- PASS: TestAccAWSRoute53Record_basic_fqdn (147.45s)
--- PASS: TestAccDataSourceAwsRoute53Zone_vpc (149.80s)
--- PASS: TestAccAWSRoute53Record_underscored (148.96s)
--- PASS: TestAccDataSourceAwsRoute53Zone_tags (159.49s)
--- PASS: TestAccAWSRoute53Record_caaSupport (147.99s)
--- PASS: TestAccAWSRoute53Record_generatesSuffix (149.07s)
--- PASS: TestAccAWSRoute53Record_failover (148.38s)
--- PASS: TestAccAWSRoute53Record_weighted_basic (148.55s)
--- PASS: TestAccAWSRoute53Record_Alias_S3 (140.70s)
--- PASS: TestAccAWSRoute53Record_Alias_Elb (150.63s)
--- PASS: TestAccAWSRoute53Record_wildcard (191.90s)
--- PASS: TestAccAWSRoute53Record_disappears_MultipleRecords (214.14s)
--- PASS: TestAccDataSourceAwsRoute53ResolverRule_ResolverEndpointIdWithTags (221.04s)
--- PASS: TestAccDataSourceAwsRoute53ResolverRules_ResolverEndpointId (221.58s)
--- PASS: TestAccAWSRoute53Record_weighted_to_simple_basic (194.91s)
--- PASS: TestAccAwsRoute53ResolverRule_basic (29.60s)
--- PASS: TestAccAwsRoute53ResolverRule_tags (42.24s)
--- PASS: TestAccAwsRoute53ResolverRule_updateName (45.92s)
--- PASS: TestAccAWSRoute53Record_geolocation_basic (133.36s)
--- PASS: TestAccAWSRoute53Record_Alias_Uppercase (142.51s)
--- PASS: TestAccAWSRoute53Record_latency_basic (142.68s)
--- PASS: TestAccAWSRoute53Record_empty (148.49s)
--- PASS: TestCleanZoneID (0.00s)
--- PASS: TestCleanChangeID (0.00s)
--- PASS: TestTrimTrailingPeriod (0.00s)
--- PASS: TestAccAWSRoute53Record_longTXTrecord (139.76s)
--- PASS: TestAccAWSRoute53Record_TypeChange (190.80s)
--- PASS: TestAccAWSRoute53Record_HealthCheckId_TypeChange (192.42s)
--- PASS: TestAccAWSRoute53Record_multivalue_answer_basic (146.55s)
--- PASS: TestAccAWSRoute53Record_SetIdentifierChange (188.02s)
--- PASS: TestAccAWSRoute53Zone_disappears (46.25s)
--- PASS: TestAccAWSRoute53Zone_basic (47.94s)
--- PASS: TestAccAWSRoute53Record_AliasChange (200.65s)
--- PASS: TestAccAWSRoute53Record_HealthCheckId_SetIdentifierChange (221.51s)
--- PASS: TestAccAWSRoute53Record_allowOverwrite (177.97s)
--- PASS: TestAccAWSRoute53Record_NameChange (232.35s)
--- PASS: TestAccAwsRoute53ResolverRuleAssociation_basic (181.43s)
--- PASS: TestAccAWSRoute53Zone_multiple (53.11s)
--- PASS: TestAccAWSSESDomainIdentity_disappears (6.69s)
--- PASS: TestAccAWSSESDomainIdentity_trailingPeriod (9.45s)
--- PASS: TestAccAWSRoute53Zone_DelegationSetID (54.07s)
--- PASS: TestAccAWSRoute53Zone_Comment (59.97s)
--- PASS: TestAccAWSSESDomainIdentity_basic (10.56s)
--- PASS: TestAccAwsSesDomainIdentityVerification_nonexistent (3.84s)
--- PASS: TestAccAwsSesDomainIdentityVerification_timeout (9.74s)
--- PASS: TestAccAWSRoute53ZoneAssociation_disappears (152.22s)
--- PASS: TestAccAWSRoute53Zone_Tags (65.16s)
--- PASS: TestAccAWSRoute53ZoneAssociation_basic (167.38s)
--- PASS: TestAccAWSRoute53Record_weighted_alias (296.35s)
--- PASS: TestAccAWSRoute53ZoneAssociation_disappears_Zone (162.56s)
--- PASS: TestAccAWSRoute53ZoneAssociation_disappears_VPC (175.40s)
--- PASS: TestAccAWSRoute53Zone_VPC_Single (85.78s)
--- PASS: TestAccAWSRoute53ZoneAssociation_region (161.13s)
--- PASS: TestAccAwsRoute53ResolverRule_forward (276.48s)
--- PASS: TestAccAWSRoute53Zone_VPC_Multiple (145.82s)
--- PASS: TestAccAWSRoute53Zone_ForceDestroy (218.16s)
--- PASS: TestAccAWSRoute53Zone_ForceDestroy_TrailingPeriod (205.26s)
--- PASS: TestAccAWSRoute53Record_Alias_VpcEndpoint (502.90s)
--- PASS: TestAccAWSRoute53Zone_VPC_Updates (243.11s)
--- PASS: TestAccAwsRoute53ResolverRule_forwardEndpointRecreate (437.44s)

--- FAIL: TestAccDataSourceAwsRoute53ResolverRule_SharedWithMe (1.44s) -- also failing in master related to explicit provider config
--- SKIP: TestAccAWSAcmCertificateDataSource_singleIssued (0.00s)
--- SKIP: TestAccAWSAcmCertificateDataSource_multipleIssued (0.00s)
--- SKIP: TestAccAwsSesDomainIdentityVerification_basic (0.00s)

bflad

Hey @anGie44 👋 Just a few more things and this should be good to go. Thanks!

aws/data_source_aws_route53_resolver_rule.go

bflad · 2020-07-29T12:32:28Z

aws/resource_aws_acm_certificate.go

-					// trailing period, which generates an ACM API error
-					return strings.TrimSuffix(v.(string), ".")
-				},
+				StateFunc:     trimTrailingPeriod,


If the ACM API does not accept trailing periods, which if I remember correctly does not, I think our goal during this major release should be fixing up the resource to match the API expectations and return a validation error. Since we are fixing the source of the generally unexpected trailing periods by removing them from the Route 53 data sources/resources, my recommendation would be to remove the special handling like this and the custom resource logic from downstream attribute handling. I'm worried about future upkeep and harder code generation. 👍 We can add a ValidateFunc to catch trailing periods to help catch these sooner.

aws/resource_aws_acm_certificate.go

aws/resource_aws_acm_certificate_test.go

aws/resource_aws_ses_domain_identity.go

aws/resource_aws_ses_domain_identity_verification.go

website/docs/guides/version-3-upgrade.html.md

anGie44 · 2020-07-29T21:03:34Z

(Almost)Ready-- for re-review. Just held up on test results for TestAccAWSAcmCertificateDataSource_singleIssued and TestAccAWSAcmCertificateDataSource_multipleIssued

Output of acceptance tests with current branch state:

--- PASS: TestAccAWSSESDomainIdentity_trailingPeriod (4.48s)
--- PASS: TestAccAWSSESDomainIdentity_disappears (14.36s)
--- PASS: TestAccAWSSESDomainIdentity_basic (15.81s)

--- PASS: TestAccAWSAcmCertificate_root_TrailingPeriod (2.03s)
--- PASS: TestAccAWSAcmCertificate_wildcard (58.62s)
--- PASS: TestAccAWSAcmCertificate_emailValidation (58.71s)
--- PASS: TestAccAWSAcmCertificate_san_TrailingPeriod (63.54s)
--- PASS: TestAccAWSAcmCertificate_root (64.18s)
--- PASS: TestAccAWSAcmCertificate_privateCert (64.62s)
--- PASS: TestAccAWSAcmCertificate_rootAndWildcardSan (65.75s)
--- PASS: TestAccAWSAcmCertificate_dnsValidation (67.03s)
--- PASS: TestAccAWSAcmCertificate_imported_DomainName (93.52s)
--- PASS: TestAccAWSAcmCertificate_tags (147.12s)
--- PASS: TestAccAWSAcmCertificate_disableCTLogging (24.12s)
--- PASS: TestAccAWSAcmCertificate_wildcardAndRootSan (21.79s)
--- PASS: TestAccAWSAcmCertificate_san_single (24.20s)
--- PASS: TestAccAWSAcmCertificate_san_multiple (25.24s)
--- PASS: TestAccAWSAcmCertificate_imported_IpAddress (19.67s)

--- PASS: TestAccAWSAcmCertificateValidation_validationRecordFqdnsEmail (16.79s)
--- PASS: TestAccAWSAcmCertificateValidation_timeout (21.45s)
--- PASS: TestAccAWSAcmCertificateValidation_basic (213.07s)
--- PASS: TestAccAWSAcmCertificateValidation_validationRecordFqdnsSan (275.08s)
--- PASS: TestAccAWSAcmCertificateValidation_validationRecordFqdnsWildcard (349.75s)
--- PASS: TestAccAWSAcmCertificateValidation_validationRecordFqdnsWildcardAndRoot (351.08s)
--- PASS: TestAccAWSAcmCertificateValidation_validationRecordFqdnsRootAndWildcard (352.75s)
--- PASS: TestAccAWSAcmCertificateValidation_validationRecordFqdns (380.46s)
--- PASS: TestAccAWSAcmCertificateValidation_validationRecordFqdnsRoot (385.38s)

--- PASS: TestAccAWSAcmCertificateDataSource_noMatchReturnsError (10.98s)
--- PASS: TestAccAWSAcmCertificateDataSource_KeyTypes (18.51s)

--- PASS: TestSuppressEquivalentJsonDiffsWhitespaceAndNoWhitespace (0.00s)
--- PASS: TestSuppressCloudFormationTemplateBodyDiffs (0.00s)
--- PASS: TestSuppressEquivalentTypeStringBoolean (0.00s)
--- PASS: TestCleanRecordName (0.00s)
--- PASS: TestExpandRecordName (0.00s)
--- PASS: TestNormalizeAwsAliasName (0.00s)
--- PASS: TestParseRecordId (0.00s)

--- PASS: TestAccDataSourceAwsRoute53ResolverRule_SharedByMe (227.21s)
--- FAIL: TestAccDataSourceAwsRoute53ResolverRule_SharedWithMe (3.42s) -- failing in master as well
--- PASS: TestAccDataSourceAwsRoute53ResolverRule_basic (36.18s)
--- PASS: TestAccDataSourceAwsRoute53ResolverRule_ResolverEndpointIdWithTags (212.02s)

--- PASS: TestAccDataSourceAwsRoute53ResolverRules_basic (13.13s)
--- PASS: TestAccDataSourceAwsRoute53ResolverRules_ResolverEndpointId (201.69s)

--- PASS: TestAccDataSourceAwsRoute53Zone_name (52.90s)
--- PASS: TestAccDataSourceAwsRoute53Zone_id (56.21s)
--- PASS: TestAccDataSourceAwsRoute53Zone_serviceDiscovery (108.44s)
--- PASS: TestAccDataSourceAwsRoute53Zone_vpc (115.00s)
--- PASS: TestAccDataSourceAwsRoute53Zone_tags (175.52s)

--- PASS: TestAccAWSRoute53Record_spfSupport (124.03s)
--- PASS: TestAccAWSRoute53Record_basic (126.80s)
--- PASS: TestAccAWSRoute53Record_underscored (126.94s)
--- PASS: TestAccAWSRoute53Record_txtSupport (148.98s)
--- PASS: TestAccAWSRoute53Record_caaSupport (140.52s)
--- PASS: TestAccAWSRoute53Record_basic_fqdn (152.92s)
--- PASS: TestAccAWSRoute53Record_generatesSuffix (143.67s)
--- PASS: TestAccAWSRoute53Record_disappears (165.82s)
--- PASS: TestAccAWSRoute53Record_failover (157.65s)
--- PASS: TestAccAWSRoute53Record_disappears_MultipleRecords (208.53s)
--- PASS: TestAccAWSRoute53Record_Alias_Elb (160.39s)
--- PASS: TestAccAWSRoute53Record_weighted_basic (186.06s)
--- PASS: TestAccAWSRoute53Record_wildcard (205.17s)
--- PASS: TestAccAWSRoute53Record_Alias_S3 (171.37s)
--- PASS: TestAccAWSRoute53Record_weighted_to_simple_basic (212.98s)
--- PASS: TestAccAWSRoute53Record_Alias_Uppercase (167.34s)
--- PASS: TestAccAWSRoute53Record_geolocation_basic (157.47s)
--- PASS: TestAccAWSRoute53Record_latency_basic (173.48s)
--- PASS: TestAccAWSRoute53Record_HealthCheckId_SetIdentifierChange (201.78s)
--- PASS: TestAccAWSRoute53Record_empty (157.02s)
--- PASS: TestAccAWSRoute53Record_longTXTrecord (149.95s)
--- PASS: TestAccAWSRoute53Record_HealthCheckId_TypeChange (200.09s)
--- PASS: TestAccAWSRoute53Record_multivalue_answer_basic (143.59s)
--- PASS: TestAccAWSRoute53Record_AliasChange (201.22s)
--- PASS: TestAccAWSRoute53Record_SetIdentifierChange (211.69s)
--- PASS: TestAccAWSRoute53Record_TypeChange (225.01s)
--- PASS: TestAccAWSRoute53Record_allowOverwrite (190.06s)
--- PASS: TestAccAWSRoute53Record_weighted_alias (292.63s)
--- PASS: TestAccAWSRoute53Record_NameChange (267.46s)
--- PASS: TestAccAWSRoute53Record_Alias_VpcEndpoint (489.62s)

--- PASS: TestAccAwsRoute53ResolverRule_basic (29.93s)
--- PASS: TestAccAwsRoute53ResolverRule_tags (42.48s)
--- PASS: TestAccAwsRoute53ResolverRule_updateName (46.58s)
--- PASS: TestAccAwsRoute53ResolverRule_forward (253.83s)
--- PASS: TestAccAwsRoute53ResolverRule_forwardEndpointRecreate (426.16s)

--- PASS: TestAccAwsRoute53ResolverRuleAssociation_basic (141.60s)

--- PASS: TestAccAWSRoute53ZoneAssociation_disappears (174.30s)
--- PASS: TestAccAWSRoute53ZoneAssociation_basic (186.10s)
--- PASS: TestAccAWSRoute53ZoneAssociation_disappears_Zone (171.13s)
--- PASS: TestAccAWSRoute53ZoneAssociation_disappears_VPC (197.16s)
--- PASS: TestAccAWSRoute53ZoneAssociation_region (182.72s)

--- PASS: TestCleanZoneID (0.00s)
--- PASS: TestCleanChangeID (0.00s)
--- PASS: TestTrimTrailingPeriod (0.00s)

--- PASS: TestAccAWSRoute53Zone_disappears (46.60s)
--- PASS: TestAccAWSRoute53Zone_basic (59.60s)
--- PASS: TestAccAWSRoute53Zone_multiple (55.23s)
--- PASS: TestAccAWSRoute53Zone_DelegationSetID (52.90s)
--- PASS: TestAccAWSRoute53Zone_Comment (59.71s)
--- PASS: TestAccAWSRoute53Zone_Tags (69.87s)
--- PASS: TestAccAWSRoute53Zone_VPC_Single (85.39s)
--- PASS: TestAccAWSRoute53Zone_VPC_Updates (344.10s)
--- PASS: TestAccAWSRoute53Zone_VPC_Multiple (153.35s)
--- PASS: TestAccAWSRoute53Zone_ForceDestroy (229.52s)
--- PASS: TestAccAWSRoute53Zone_ForceDestroy_TrailingPeriod (237.52s)

--- PASS: TestAccAWSSESDomainIdentity_trailingPeriod (1.02s)
--- PASS: TestAccAWSSESDomainIdentity_basic (7.02s)
--- PASS: TestAccAWSSESDomainIdentity_disappears (6.10s)

--- PASS: TestAccAwsSesDomainIdentityVerification_nonexistent (3.56s)
--- PASS: TestAccAwsSesDomainIdentityVerification_timeout (9.80s)

bflad

Looks good to me, let's get this in. 🚀

anGie44 · 2020-07-30T03:31:09Z

Missed an unintended side-effect after last commit 😓 Another round of test runs returned failures in acm_certificate_validation as configs that use the aws_route53_record reference the acm_certificate domain_validation_options.resource_record_name argument as input to name, resulting in plan-time validation errors. 3349867 addresses this as well as removes extraneous cleanup of subjectAlternativeNames as they are validated at plan-time for the trailing period and are not returned by the API with trailing periods

Output of acceptance tests:

Previously Failing:

--- PASS: TestAccAWSAcmCertificateValidation_validationRecordFqdnsEmail (19.89s)
--- PASS: TestAccAWSAcmCertificateValidation_validationRecordFqdnsWildcard (141.82s)
--- PASS: TestAccAWSAcmCertificateValidation_validationRecordFqdnsWildcardAndRoot (142.77s)
--- PASS: TestAccAWSAcmCertificateValidation_validationRecordFqdnsRootAndWildcard (143.07s)
--- PASS: TestAccAWSAcmCertificateValidation_validationRecordFqdnsRoot (173.55s)
--- PASS: TestAccAWSAcmCertificateValidation_validationRecordFqdnsSan (223.74s)
--- PASS: TestAccAWSAcmCertificateValidation_validationRecordFqdns (224.76s)
--- PASS: TestAccAWSAcmCertificateValidation_timeout (22.92s)
--- PASS: TestAccAWSAcmCertificateValidation_basic (201.40s)

Re-confirmed

--- PASS: TestAccDataSourceAwsRoute53ResolverRule_SharedByMe (227.21s)
--- FAIL: TestAccDataSourceAwsRoute53ResolverRule_SharedWithMe (2.86s) -- known failure in Alternate
--- PASS: TestAccAWSAcmCertificateDataSource_KeyTypes (19.65s)
--- PASS: TestAccAWSAcmCertificate_imported_DomainName (28.93s)
--- PASS: TestAccAWSAcmCertificate_imported_IpAddress (13.50s)
--- PASS: TestAccAWSAcmCertificate_privateCert (19.56s)
--- PASS: TestAccAWSRoute53Record_AliasChange (176.44s)
--- PASS: TestAccAWSRoute53Record_Alias_Elb (143.66s)
--- PASS: TestAccAWSRoute53Record_Alias_S3 (152.97s)
--- PASS: TestAccAWSRoute53Record_Alias_Uppercase (120.21s)
--- PASS: TestAccAWSRoute53Record_Alias_VpcEndpoint (519.64s)
--- PASS: TestAccAWSRoute53Record_HealthCheckId_SetIdentifierChange (181.08s)
--- PASS: TestAccAWSRoute53Record_HealthCheckId_TypeChange (192.02s)
--- PASS: TestAccAWSRoute53Record_NameChange (233.22s)
--- PASS: TestAccAWSRoute53Record_SetIdentifierChange (176.41s)
--- PASS: TestAccAWSRoute53Record_TypeChange (186.60s)
--- PASS: TestAccAWSRoute53Record_allowOverwrite (183.83s)
--- PASS: TestAccAWSRoute53Record_basic (250.34s)
--- PASS: TestAccAWSRoute53Record_basic_fqdn (153.08s)
--- PASS: TestAccAWSRoute53Record_basic_name_trailingPeriod (3.82s)
--- PASS: TestAccAWSRoute53Record_caaSupport (142.84s)
--- PASS: TestAccAWSRoute53Record_disappears (135.24s)
--- PASS: TestAccAWSRoute53Record_disappears_MultipleRecords (185.05s)
--- PASS: TestAccAWSRoute53Record_empty (116.13s)
--- PASS: TestAccAWSRoute53Record_failover (165.19s)
--- PASS: TestAccAWSRoute53Record_generatesSuffix (151.73s)
--- PASS: TestAccAWSRoute53Record_geolocation_basic (155.50s)
--- PASS: TestAccAWSRoute53Record_latency_basic (149.83s)
--- PASS: TestAccAWSRoute53Record_longTXTrecord (123.63s)
--- PASS: TestAccAWSRoute53Record_multivalue_answer_basic (129.51s)
--- PASS: TestAccAWSRoute53Record_spfSupport (139.49s)
--- PASS: TestAccAWSRoute53Record_txtSupport (144.56s)
--- PASS: TestAccAWSRoute53Record_underscored (153.56s)
--- PASS: TestAccAWSRoute53Record_weighted_alias (265.66s)
--- PASS: TestAccAWSRoute53Record_weighted_basic (137.93s)
--- PASS: TestAccAWSRoute53Record_weighted_to_simple_basic (191.75s)
--- PASS: TestAccAWSRoute53Record_wildcard (188.57s)
--- PASS: TestAccAWSRoute53ZoneAssociation_basic (135.17s)
--- PASS: TestAccAWSRoute53ZoneAssociation_disappears (131.70s)
--- PASS: TestAccAWSRoute53ZoneAssociation_disappears_VPC (142.62s)
--- PASS: TestAccAWSRoute53ZoneAssociation_disappears_Zone (139.52s)
--- PASS: TestAccAWSRoute53ZoneAssociation_region (140.09s)
--- PASS: TestAccAWSRoute53Zone_Comment (54.67s)
--- PASS: TestAccAWSRoute53Zone_DelegationSetID (62.55s)
--- PASS: TestAccAWSRoute53Zone_ForceDestroy (199.64s)
--- PASS: TestAccAWSRoute53Zone_ForceDestroy_TrailingPeriod (200.53s)
--- PASS: TestAccAWSRoute53Zone_Tags (72.33s)
--- PASS: TestAccAWSRoute53Zone_VPC_Multiple (163.80s)
--- PASS: TestAccAWSRoute53Zone_VPC_Single (85.89s)
--- PASS: TestAccAWSRoute53Zone_VPC_Updates (251.89s)
--- PASS: TestAccAWSRoute53Zone_basic (50.36s)
--- PASS: TestAccAWSRoute53Zone_disappears (41.61s)
--- PASS: TestAccAWSRoute53Zone_multiple (48.79s)
--- PASS: TestAccAWSSESDomainIdentity_basic (6.69s)
--- PASS: TestAccAWSSESDomainIdentity_disappears (6.40s)
--- PASS: TestAccAWSSESDomainIdentity_trailingPeriod (1.01s)
--- PASS: TestAccAwsRoute53ResolverRuleAssociation_basic (171.64s)
--- PASS: TestAccAwsRoute53ResolverRule_basic (50.08s)
--- PASS: TestAccAwsRoute53ResolverRule_forward (344.71s)
--- PASS: TestAccAwsRoute53ResolverRule_forwardEndpointRecreate (416.60s)
--- PASS: TestAccAwsRoute53ResolverRule_tags (43.30s)
--- PASS: TestAccAwsRoute53ResolverRule_updateName (66.60s)
--- PASS: TestAccAwsSesDomainIdentityVerification_nonexistent (3.70s)
--- PASS: TestAccAwsSesDomainIdentityVerification_timeout (9.72s)
--- PASS: TestAccDataSourceAwsRoute53ResolverRule_ResolverEndpointIdWithTags (241.68s)
--- PASS: TestAccDataSourceAwsRoute53ResolverRule_basic (35.67s)
--- PASS: TestAccDataSourceAwsRoute53ResolverRules_ResolverEndpointId (231.77s)
--- PASS: TestAccDataSourceAwsRoute53ResolverRules_basic (14.01s)
--- PASS: TestAccDataSourceAwsRoute53Zone_id (55.10s)
--- PASS: TestAccDataSourceAwsRoute53Zone_name (55.32s)
--- PASS: TestAccDataSourceAwsRoute53Zone_serviceDiscovery (108.43s)
--- PASS: TestAccDataSourceAwsRoute53Zone_tags (126.86s)
--- PASS: TestAccDataSourceAwsRoute53Zone_vpc (114.10s)
--- PASS: TestCleanChangeID (0.00s)
--- PASS: TestCleanRecordName (0.00s)
--- PASS: TestCleanZoneID (0.00s)
--- PASS: TestExpandRecordName (0.00s)
--- PASS: TestNormalizeAwsAliasName (0.00s)
--- PASS: TestParseRecordId (0.00s)
--- PASS: TestSuppressCloudFormationTemplateBodyDiffs (0.00s)
--- PASS: TestSuppressEquivalentJsonDiffsWhitespaceAndNoWhitespace (0.00s)
--- PASS: TestSuppressEquivalentTypeStringBoolean (0.00s)
--- PASS: TestTrimTrailingPeriod (0.00s)

--- PASS: TestAccAWSAcmCertificate_root_TrailingPeriod (2.17s)
--- PASS: TestAccAWSAcmCertificate_imported_IpAddress (27.62s)
--- PASS: TestAccAWSAcmCertificate_wildcard (31.74s)
--- PASS: TestAccAWSAcmCertificate_rootAndWildcardSan (48.15s)
--- PASS: TestAccAWSAcmCertificate_wildcardAndRootSan (49.78s)
--- PASS: TestAccAWSAcmCertificate_san_TrailingPeriod (52.99s)
--- PASS: TestAccAWSAcmCertificate_privateCert (53.13s)
--- PASS: TestAccAWSAcmCertificate_san_single (90.29s)
--- PASS: TestAccAWSAcmCertificate_san_multiple (58.88s)
--- PASS: TestAccAWSAcmCertificate_emailValidation (65.68s)
--- PASS: TestAccAWSAcmCertificate_disableCTLogging (70.28s)
--- PASS: TestAccAWSAcmCertificate_imported_DomainName (108.38s)
--- PASS: TestAccAWSAcmCertificate_tags (127.21s)
--- PASS: TestAccAWSAcmCertificate_root_TrailingPeriod (1.71s)
--- PASS: TestAccAWSAcmCertificate_root (21.90s)
--- PASS: TestAccAWSAcmCertificate_rootAndWildcardSan (22.30s)
--- PASS: TestAccAWSAcmCertificate_dnsValidation (24.41s)

--- PASS: TestAccAWSAcmCertificateDataSource_KeyTypes (28.76s)

bflad · 2020-07-30T14:59:44Z

aws/resource_aws_route53_record.go

@@ -42,9 +41,12 @@ func resourceAwsRoute53Record() *schema.Resource {
 				Required: true,
 				ForceNew: true,
 				StateFunc: func(v interface{}) string {
-					value := strings.TrimSuffix(v.(string), ".")


If there are valid other places that can give this resource a trailing period, I guess we may want to walk back on adjusting this particular attribute and allow it to still handle both with and without the trailing period. I'd rather avoid changing the ACM validation record behavior since that could affect interactions with DNS systems outside of AWS.

gotcha -- does seem risky in cases where that ACM attribute I changed might be used in ways not covered by our tests; i'll revert and move back this state func /remove the validation here 👍

Thank you for updating the comment there, super helpful 👍

… cert computed attr val

bflad · 2020-07-30T16:19:52Z

aws/resource_aws_route53_record.go

@@ -974,6 +977,5 @@ func parseRecordId(id string) [4]string {
 			}
 		}
 	}
-	recName = strings.TrimSuffix(recName, ".")


I think we can keep this here now as well (and the previous unit test case) 👍

yess just discovered this one! interesting enough, i think our tests don't seem to cover a case where it would fail without this line back in b/c in TC the tests all just passed ... aside from the unit tests

We would need to implement an acceptance test with a custom ImportStateIDFunc since the ImportState testing pulls in the ID with the trimmed value. Maybe good to have, but doesn't need to happen right now. 😄

Hmm I think the TestAccAWSRoute53Record_basic_trailingPeriodAndZoneID I most recently added should hopefully account for catching the importstateverify error ... atleast it fails when the above code is omitted :)

bflad

Just the one last thing to allow trailing period imports still, otherwise looks good and thank you for sticking through this challenging one. 🙇

anGie44 · 2020-07-30T17:23:09Z

OK ready to go! Re-confirmed tests in TC and separately ran ACM tests:

Output:

--- PASS: TestAccAWSAcmCertificate_root_TrailingPeriod (1.42s)
--- PASS: TestAccAWSAcmCertificate_privateCert (161.93s)
--- PASS: TestAccAWSAcmCertificate_imported_IpAddress (206.57s)
--- PASS: TestAccAWSAcmCertificate_wildcard (222.99s)
--- PASS: TestAccAWSAcmCertificate_rootAndWildcardSan (223.23s)
--- PASS: TestAccAWSAcmCertificate_emailValidation (223.74s)
--- PASS: TestAccAWSAcmCertificate_wildcardAndRootSan (223.76s)
--- PASS: TestAccAWSAcmCertificate_san_single (234.98s)
--- PASS: TestAccAWSAcmCertificate_dnsValidation (234.99s)
--- PASS: TestAccAWSAcmCertificate_root (235.01s)
--- PASS: TestAccAWSAcmCertificate_disableCTLogging (235.25s)
--- PASS: TestAccAWSAcmCertificate_san_TrailingPeriod (235.33s)
--- PASS: TestAccAWSAcmCertificate_san_multiple (236.16s)
--- PASS: TestAccAWSAcmCertificate_imported_DomainName (265.83s)
--- PASS: TestAccAWSAcmCertificate_tags (277.30s)

--- PASS: TestAccAWSAcmCertificateValidation_validationRecordFqdnsEmail (35.66s)
--- PASS: TestAccAWSAcmCertificateValidation_timeout (36.14s)
--- PASS: TestAccAWSAcmCertificateValidation_validationRecordFqdnsRoot (177.20s)
--- PASS: TestAccAWSAcmCertificateValidation_validationRecordFqdnsRootAndWildcard (177.95s)
--- PASS: TestAccAWSAcmCertificateValidation_validationRecordFqdnsWildcardAndRoot (186.68s)
--- PASS: TestAccAWSAcmCertificateValidation_validationRecordFqdnsWildcard (189.72s)
--- PASS: TestAccAWSAcmCertificateValidation_basic (225.49s)
--- PASS: TestAccAWSAcmCertificateValidation_validationRecordFqdnsSan (226.39s)
--- PASS: TestAccAWSAcmCertificateValidation_validationRecordFqdns (238.18s)

ghost · 2020-07-31T02:17:00Z

This has been released in version 3.0.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 for triage. Thanks!

ghost · 2020-08-30T17:13:51Z

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!

anGie44 added the breaking-change Introduces a breaking change in current functionality; usually deferred to the next major release. label Jul 16, 2020

anGie44 added this to the v3.0.0 milestone Jul 16, 2020

anGie44 marked this pull request as ready for review July 17, 2020 15:41

anGie44 requested a review from a team July 17, 2020 15:41

anGie44 changed the title ~~service/route53: remove trailing period from domainname/name attributes~~ service/route53: remove trailing period from domainname/name attributes in resolver_rule and zone resources Jul 17, 2020

anGie44 marked this pull request as draft July 21, 2020 04:57

ghost added documentation Introduces or discusses updates to documentation. size/L Managed by automation to categorize the size of a PR. and removed size/M Managed by automation to categorize the size of a PR. labels Jul 21, 2020

anGie44 marked this pull request as ready for review July 21, 2020 08:17

anGie44 force-pushed the b-route53-zone-domain branch from 82c2362 to 44e6869 Compare July 21, 2020 09:30

anGie44 force-pushed the master branch from 54db70d to 48ba2f7 Compare July 21, 2020 18:26

anGie44 marked this pull request as draft July 21, 2020 19:35

ghost added service/acm Issues and PRs that pertain to the acm service. service/ses Issues and PRs that pertain to the ses service. size/XL Managed by automation to categorize the size of a PR. and removed size/L Managed by automation to categorize the size of a PR. labels Jul 22, 2020

anGie44 force-pushed the b-route53-zone-domain branch from cfb923a to 4646ede Compare July 22, 2020 16:52

ghost added size/L Managed by automation to categorize the size of a PR. and removed size/XL Managed by automation to categorize the size of a PR. labels Jul 22, 2020

anGie44 force-pushed the b-route53-zone-domain branch from 4646ede to 44e6869 Compare July 22, 2020 16:52

ghost added size/XL Managed by automation to categorize the size of a PR. and removed size/L Managed by automation to categorize the size of a PR. labels Jul 22, 2020

anGie44 force-pushed the b-route53-zone-domain branch from 20ce3d6 to dfbc285 Compare July 22, 2020 17:20

ghost added size/XXL Managed by automation to categorize the size of a PR. and removed size/XL Managed by automation to categorize the size of a PR. labels Jul 22, 2020

ghost added size/XL Managed by automation to categorize the size of a PR. and removed size/XXL Managed by automation to categorize the size of a PR. labels Jul 27, 2020

anGie44 force-pushed the b-route53-zone-domain branch from e415f07 to b33c888 Compare July 27, 2020 20:04

fixup acctests with trailing period and documentation

03dd714

anGie44 force-pushed the b-route53-zone-domain branch 2 times, most recently from a9e1c8d to b5a357e Compare July 27, 2020 21:35

revert domain name trim at Create time and update docs

3ae10dd

anGie44 force-pushed the b-route53-zone-domain branch from b5a357e to 3ae10dd Compare July 28, 2020 11:36

bflad self-requested a review July 29, 2020 12:16

bflad suggested changes Jul 29, 2020

View reviewed changes

anGie44 force-pushed the b-route53-zone-domain branch from 19f5c6e to 709b581 Compare July 29, 2020 22:41

bflad approved these changes Jul 29, 2020

View reviewed changes

CR updates to remove custom period handling and add validation instead

dc19585

anGie44 force-pushed the b-route53-zone-domain branch from 709b581 to dc19585 Compare July 30, 2020 00:04

bflad reviewed Jul 30, 2020

View reviewed changes

update route53 record to allow trailing period in name and revert acm…

9eb3147

… cert computed attr val

anGie44 force-pushed the b-route53-zone-domain branch from 3349867 to 9eb3147 Compare July 30, 2020 16:10

bflad reviewed Jul 30, 2020

View reviewed changes

bflad approved these changes Jul 30, 2020

View reviewed changes

bring back trimming of name at import time

45cc659

anGie44 merged commit c1592e4 into master Jul 30, 2020

anGie44 deleted the b-route53-zone-domain branch July 30, 2020 17:35

anGie44 added a commit that referenced this pull request Jul 30, 2020

Update CHANGELOG for #14220

58a3ff3

ghost locked and limited conversation to collaborators Aug 30, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

resource/route53_zone and data-source/route53_zone: remove trailing period from name attributes #14220

resource/route53_zone and data-source/route53_zone: remove trailing period from name attributes #14220

anGie44 commented Jul 16, 2020 •

edited

Loading

anGie44 commented Jul 27, 2020

bflad left a comment

bflad Jul 29, 2020

anGie44 commented Jul 29, 2020 •

edited

Loading

bflad left a comment

anGie44 commented Jul 30, 2020 •

edited

Loading

bflad Jul 30, 2020

anGie44 Jul 30, 2020

bflad Jul 30, 2020

bflad Jul 30, 2020

anGie44 Jul 30, 2020 •

edited

Loading

bflad Jul 30, 2020

anGie44 Jul 30, 2020

bflad left a comment

anGie44 commented Jul 30, 2020

ghost commented Jul 31, 2020

ghost commented Aug 30, 2020

resource/route53_zone and data-source/route53_zone: remove trailing period from name attributes #14220

resource/route53_zone and data-source/route53_zone: remove trailing period from name attributes #14220

Conversation

anGie44 commented Jul 16, 2020 • edited Loading

Community Note

anGie44 commented Jul 27, 2020

bflad left a comment

Choose a reason for hiding this comment

bflad Jul 29, 2020

Choose a reason for hiding this comment

anGie44 commented Jul 29, 2020 • edited Loading

bflad left a comment

Choose a reason for hiding this comment

anGie44 commented Jul 30, 2020 • edited Loading

bflad Jul 30, 2020

Choose a reason for hiding this comment

anGie44 Jul 30, 2020

Choose a reason for hiding this comment

bflad Jul 30, 2020

Choose a reason for hiding this comment

bflad Jul 30, 2020

Choose a reason for hiding this comment

anGie44 Jul 30, 2020 • edited Loading

Choose a reason for hiding this comment

bflad Jul 30, 2020

Choose a reason for hiding this comment

anGie44 Jul 30, 2020

Choose a reason for hiding this comment

bflad left a comment

Choose a reason for hiding this comment

anGie44 commented Jul 30, 2020

ghost commented Jul 31, 2020

ghost commented Aug 30, 2020

anGie44 commented Jul 16, 2020 •

edited

Loading

anGie44 commented Jul 29, 2020 •

edited

Loading

anGie44 commented Jul 30, 2020 •

edited

Loading

anGie44 Jul 30, 2020 •

edited

Loading