fix: Fix an off-by-one error with validation records

[blaskov]

Description

Prevent duplicated validation record causing failures when validation_allow_overwrite_records is set to false.

Motivation and Context

PR #32 introduces an obscure off-by-one error that leads to a duplicated validation record because of a wrap-around in distinct_domain_names.

I assume this hasn't been spotted until now mainly because the duplicated record gets silently overwritten when validation_allow_overwrite_records is set to true (as it is by
default).

I tried to identify corner cases when this + 1 is required, but couldn't find any. Please let me know if you are aware why it may be needed.

Breaking Changes

I don't think this is breaking backwards compatibility.

How Has This Been Tested?

Used latest master with disabled validation_allow_overwrite_records in examples/complete-dns-validation and verified that Terraform fails to apply.

Then did the change in this PR and managed to apply successfully.

[blaskov]

I'm not really sure what to do for this "Semantic Pull Request" check... help?

[antonbabenko]

@blaskov You can safely ignore this for now. I will update PR template and explain how to deal with it in the near future. Meanwhile, you can prefix your PR title with feat: for new functionality or fix: if you are adding a bug-fix. More info here - https://www.conventionalcommits.org/en/v1.0.0/

[antonbabenko]

The main reason why some PRs are not merged yet is the lack of time for proper research and investigation but we are getting there with more people involved in different terraform-aws-modules.

[blaskov]

@antonbabenko Ok, I've added fix: to the title and that seems to have fixed the angry check problem 😄 Thanks!

As for the review - no worries, we've enabled validation_allow_overwrite_records as a workaround for time being, so this is not really urgent.

[e-moshaya]

This PR is definitely valid as it creates duplicate route53 validation records

[yehorb]

When (if) this PR gets applied, be careful when running terraform apply after updating.

Records created due to "wrapping" are stored as 2 different resources in the Terraform State, but they actually point to the same resource in AWS. So if in your plan you see something like this:

...
# aws_route53_record.validation[1] will be destroyed
...

be aware, that this will destroy the Record associated with aws_route53_record.validation[0]. In my case I was greeted with an Error: No matching records found, but potentially it can lead to an inconsistent state (resource aws_route53_record.validation[0] is present in Terraform state, but is missing from AWS).

This can be resolved with one more terraform apply, but still be careful.

It is better off to run terraform state rm aws_route53_record.validation[1] (or whatever the index of duplicated aws_route53_record in your case) and only then run apply.

Learned this the hard way just now by using the forked repo as the module source. Was not a big deal for me, but who knows.

[ricoli]

it's been almost a year since this PR was opened - any chance to get it merged? Also, it's now out of date, needs a rebase

[github-actions]

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.