feat!: Refactor modules to group single resource modules into higher order modules

[bryantbiggs]

Description

• Refactor modules to group single resource modules into higher order modules
• Terraform v1.5.7 is now minimum supported version
• AWS provider v6.3 is now minimum supported version
• Replace zones and records module with new zone module that creates a Route53 zone, records, as well as association authorization, and DNSSEC signing key
• Remove delegation-sets module; users are encouraged to use the standalone resource for this functionality
• Remove resolver-rule-associations; this functionality can be implemented with the standalone resource or used within whats provided in the resolver-endpoint module
• Remove zone-cross-account-vpc-association; this functionality is split up with the association authorization resource added within the new zone module, and the association resource should be implemented on its own by users
• A new resolver-firewall-rule-group is added from the experimental proposal here
• Add support for Terragrunt wrappers which follow the norms of our other modules

Motivation and Context

• The prior zones and records modules were a single resource module over the route53 zone and record resources which on their own doesn't offer a lot of value and seemed to cause more issues than benefit. Moving resources that have a strong dependency on ordering alleviates a lot of these issues and makes the module more broadly applicable (if you create a zone, you'll inevitably want to create records. you do not have to create records; those can be added externally through the record resource itself)
• The delegation-sets module was another single resource module - this type of resource is better used on its own (bare resource) since its likely to be passed into a number of modules (zones) so that all zones have the same name servers
• The zone-cross-account-vpc-association was a peculiar module. It seems its functionality was intended to assist with associating zones with other accounts/regions. However, these sharing resources are typically better left as one-sided; where the resource creation side allows/extends the sharing of said resource, and elsewhere the accepting side receives/accepts said shared resource. This module is now facing issues due to its design requiring two different aws providers. Some of the functionality is preserved - the association authorization resource resides within the new zone module which allows the zone to be associated with a different account/region. However, performing the actual association is left to the user through the aws_route53_zone_association resource (this could be in their same statefile, in separate statefile, etc.). tl;dr - it still supports associations but without the hard assumption on how that is performed
• Resolves DNS records are recreated on hosted zone change #66
• Resolves The "for_each" value depends on resource attributes that cannot be determined until apply #92
• Resolves Zones outputs are not easily consumable, they are not primitives but objects. #93
• Resolves feat: Ability to enable DNSSEC for Route53 zone #97
• Resolves feat: Add CIDR routing policy support #98
• Resolves Updated aws_route53_zone to use vpc_id. Created variable route53_vpc_id #99
• Resolves feat: Update module to include vpc for route53 #100
• Resolves Variable Type not working when specific #103
• Resolves Set more than 1 A record for same domain #110
• Resolves feat: Complex variable types for module zones #111
• Resolves support "comment" attribute for aws_route53_record #115
• Resolves Redundant empty provider block on cross-account module. #117
• Resolves Don't destroy records when changing types between A/CNAME/NS #119
• Resolves feat: Allow skipping data source lookup #120
• Closes fix: Remove redundant empty provider blocks #121

Breaking Changes

• Yes; there are no attempts at trying to migrate resources from v5.0 and prior into this new module structure. Users are free to continue using the prior versions as is, forking at a prior version and utilizing their fork if they require changes, or removing the resources in question from Terraform state and performing imports into the new module structure

How Has This Been Tested?

• I have updated at least one of the examples/* to demonstrate and validate my change(s)
• I have tested and validated these changes using one or more of the provided examples/* projects
• I have executed pre-commit run -a on my pull request

[antonbabenko]

Pretty good, just a few comments here and there.

[antonbabenko]

Hi from Terragrunt users :) Let's keep these single-resource modules, please.

[bryantbiggs]

re-added in 3c4c32c

[antonbabenko]

It is good to get rid of this pure magic, but Terragrunt users will have to find their way to migrate to the newer (simpler) version of this module.

[bryantbiggs]

ya, I don't know how to tackle that one. perhaps folks will help collaborate on this upgrade path; I've never used Terragrunt

[antonbabenko]

While here, in this PR, can you add support for using the existing zone conditionally without creating it?

I think it is a rather common case that a zone has been created separately, and users want to manage just the records or other properties using this module.

[bryantbiggs]

yep, makes sense - added in 923cce5

[antonbabenko]

Idea: What if we have this module moved to the root instead of having it under modules/zone? I think this is rather logical because this module does a lot more than just a zone.

WDYT?

[bryantbiggs]

that makes sense - done in e4f5551

[bryantbiggs]

cc @antonbabenko if you have time/feedback on the proposed changes

[antonbabenko]

cc @antonbabenko if you have time/feedback on the proposed changes

I certainly should look into these... I was (and still am) rather busy with other things, but I will do it this week.

[bryantbiggs]

no worries and no rush - a few other fires I'm dealing with at the moment anyways 😓

[antonbabenko]

Looks very good to me. Sorry that it took ages for me to review!

[antonbabenko]

This PR is included in version 6.0.0 🎉

[nikolay]

@bryantbiggs Is your goal to really make us stop using these modules?! The time we spend refactoring is not worth the time initially saved by these modules! I sincerely regret getting lured into these modules!

[bryantbiggs]

What a dumb question. Nobody is forcing you to upgrade to the next major version of the module. However, upgrades are a normal part of software development lifecycle as we try to both reduce the maintenance burden and add support for the latest features of both the AWS provider, the underlying resources themselves, as well as the Terraform language itself

[bryantbiggs]

Take a look at the number of issues/PRs this change alone resolved. Do better as a human

[nikolay]

@bryantbiggs "Dumb question," says the person who didn't bother to write upgrade/migration docs, made really dumb mistakes in the past with these "upgrades", and who gives this type of "highly professional" attitude. I was sponsoring @antonbabenko, and he was doing an amazing job, unlike you, but I'm done with it since you hijacked this project and started to "improve" it! I'm fairly certain you don't use these modules at work, given that you work for Amazon. Maybe if you used them daily, you would know what I'm talking about.

[antonbabenko]

@nikolay I appreciate that you've been using these modules for a long time. The changes we are implementing are, as Bryant says, optional to adopt (upgrade is always fully optional), but required for us to support better practices overall.

This specific module is quite old and had numerous hacks related to Terragrunt and older versions of Terraform, so improvements were inevitable.

Probably, the only thing we could have implemented differently in this release is related to the upgrade instructions, which could have minimized the amount of resource recreation. The rest is pretty solid in my mind.

[petegallagher]

@bryantbiggs and @antonbabenko thank you so much for all the time and effort you have poured into these modules. It is unfortunate that many people in the community do not appreciate these efforts sufficiently. Keep fighting the good fight!

[Sharsie]

I appreciate the effort, at the same time I understand where @nikolay is coming from. The recent changes made to some of these AWS modules introduced many breaking changes and/or bugs without any possible workarounds.

It's not necessary to be rude though. It is a huge effort to maintain and develop the modules. And noone is entitled to it. If you don't like it, don't use it.

With that said, I would appreciate if there was a bigger focus on testing and documentation. Having to go through the code to figure out how to migrate between such a huge breaking change takes a lot of effort (and frustration) as well. E.g. how do I specify an APEX/ROOT dns record? Using full_name with zone name and name with empty string. This was handled automatically in v5.0.0 so it's counter intuitive to go searching how to provide such options in v6.0.0. Was there a reason this wasn't provided? In other instances, I would call this a regression.

It's just an example, but I believe having a v5.0.0 example and trying to do a migration to v6.0.0 to see some of these issues arise would benefit the community.

Aproaching it with "We could not be bothered to provide a migration path and if you don't like it, don't upgrade" is backwards thinking in my opinion. Obviously people will want to (and eventually need to) upgrade and this is just delegating time from one person to many. It is my belief that maintaining such repositories comes with a responsibility to the community as well.

Again, I see both sides and I appreciate the effort that is being put into developing the modules. In the end, it is yours to maintain and do as you wish. Just sayin'

[bryantbiggs]

Thank you for the thoughtful post - migrations between major versions of a module that include breaking changes are one of the hardest things in Terraform. Both for maintainers and consumers. Unfortunately the Terraform state migration utilities are quite basic today and do not support the majority of the use cases required (namely - migrating from count to for_each and resource consolidation). But I want to hone in on one point:

Aproaching it with "We could not be bothered to provide a migration path and if you don't like it, don't upgrade" is backwards thinking in my opinion. Obviously people will want to (and eventually need to) upgrade and this is just delegating time from one person to many. It is my belief that maintaining such repositories comes with a responsibility to the community as well.

Ignoring the part that these modules are provided for free and are widely used to generate billions of dollars (yes, billions - I know this for a fact); how would you approach situations where a migration path is not possible? Meaning, there is no clear path from a v1.x to a v2.0 (again, where you start is a whole separate topic - the assumption is that users migrate from the greatest minor/path version of the most recent major version prior to the new major version).

If we don't make changes, we continue to have users adopt a module with know deficiencies and a long, difficult path to reach the latest features of the service(s), provider, and Terraform language. This is akin to continuing to grow and add to the user technical debt. It also forces users who want/need to adopt the latest for whatever reason, to fork or seek alternative solutions.

If we do make changes, those who want/need to get to the latest have a path that must be navigated (unfortunately). Which path they choose, is up to them, and we do strive to provide guidance for navigating at least one path (i.e. - a migration guide of guidance). Those who do not need the latest, who are content with the current functionality of the module are able to simply stay on the version of their choosing.

So given the two paths, the latter seems more beneficial given the pros/cons as well as in terms of the options afforded users. But again, there are no guarantees that there is a "clean" path to get from A to B (clean meaning - without disruption, destructive changes, etc.). So in those scenarios, what do you propose should be done?

[bryantbiggs]

Missed the whole point of one statement above - adding here to avoid edits (that muddies the water and feels dishonest on the interwebs)

Ignoring the part that these modules are provided for free and are widely used to generate billions of dollars (yes, billions - I know this for a fact)

• of which Anton and myself, as well as any other maintainers past and present, have not seen any material benefits in terms of shared revenue for the value provided

[Sharsie]

I do not disagree (aehm) with any motivation or views that you shared (well maybe except for one)

Unfortunately the Terraform state migration utilities are quite basic today and do not support the majority of the use cases required (namely - migrating from count to for_each and resource consolidation).

Well that depends. I am not saying there has to be some automated way to migrate. But instructions can be provided that describe the steps a user needs to take... e.g. modules/zones removal could use the following instructions (pseudocode)

how would you approach situations where a migration path is not possible? Meaning, there is no clear path from a v1.x to a v2.0

Lets make one thing clear, when you refer to migration, do you mean state migration or migrating between module versions? When I'm referring to migration, I mean instructions on how to get from vN -> vN+1. Yes, there may be cases where you stop supporting some parts of the module and those resources get removed - which are ideally deprecated in advance (and not removed straight off the bat). Only in such cases, I would argue it's ok to say "resource XYZ is no longer supported by this module". Ideally, give options to use other modules or raw TF. The user would always have an option of copy pasting code from prior version of the module into their source code - this can also be offered as an option with provided snippets using moved blocks.

Other than that, I would argue all other migrations can be documented, even if it means creating separate TF state, using terraform state rm and terraform import there is always a way to do that. As long as you can import existing infrastructure into terraform, there (probably?) is a way.

Additionally, I would never expect anyone to provide migration paths between more than 1 major version, even though some projects do it. But to me, migration path means you can get from 1 -> 1+N in N steps, not in 1 step.

If we don't make changes, we continue to have users adopt a module with know deficiencies and a long

That's an argument you may be making for others, but I absolutely do agree that stuff needs to move, even if it's breaking. I do not oppose that in any way.

Those who do not need the latest, who are content with the current functionality of the module are able to simply stay on the version of their choosing.

I do disagree with this statement completely. Yes, people can stay for a while, but eventually, you are forced to upgrade. Provider API may change and a pinned module version will eventually produce warnings and then break. Unless you expect to support v5.0.0 going forward and upgrade that branch to work with the latest provider (I don't see why would you), this statement in my opinion does not hold.

Even if it would be possible to use multiple provider versions in a single terraform root module to support this as a legacy module, underlying API (aws) may also change and it's just not true that people can stay if they chose to.

---

these modules are provided for free

and all other related comments

I absolutely do understand what you are providing for the community and I really do appreciate that. Unforunately human nature does not. It is honestly extremely difficult explaining to my colleagues over and over that not every change can be easily tested in all possible variations and all possible environments. And that maintainers don't break their code on purpose.

Unfortunately what is happening is:

1. I am using this great module
2. This guy broke it again
3. I'm mad
4. I channel my frustration without care

- Humans

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