#terraform-aws-modules (2022-03)

I remember I started working on a module for this and I never finished

there is a lambda that is required and such

yeah, it seems need some quite effort to implement this .. It would be awesome if AWS can provide this functionality out of the box

Ya, I don’t think so

cc @matt

@matt

i don’t believe so

what’s the need for so many different security group rules?

@Jeremy G (Cloud Posse) @Erik Osterman (Cloud Posse) are the best to talk to.

We aim very hard to not make any breaking changes for patch releases. However we use release drafter and sometimes contributors and even cloud Posse engineers forget to apply the correct labels.

Which modules in particular?

@ekristen We are using major version zero which is treated specially in SemVer, to mean “anything may change at any time”. Nevertheless, we try hard to avoid making breaking changes at any time, and when we do make breaking changes, we label the release as such and provide migration details. It is never our intention to release a breaking change in a patch release; we intend breaking changes to be a minor release bump while we are still operating under major version zero. However, we do rely heavily on volunteers to maintain the modules and sometimes mistakes are made. Please give us details so we can address them.

A number of modules have breaking changes not due to anything Cloud Posse did, but due to breaking changes in the AWS Terraform provider version 4.0 which was recently released. There’s nothing we can do about that: previously working code stopped working if you upgraded to the latest provider. However, we did take the opportunity of being forced to make breaking changes to introduce additional breaking changes as we refactor our S3 implementations. Previously we had several modules which each had their own implementation of creating S3 buckets; we are migrating all our modules to instead use our terraform-aws-s3-bucket module for all S3 bucket operations so that we can provide consistent and full support of S3 in a manageable fashion.

For several months we have been working on doing the same kind of thing with AWS Security Groups, going from every module having their own implementation to every module delegating to terraform-aws-security-group. Unfortunately, our first attempt at this (security-group v0.3.x) went badly, and we are still in the process of rolling it back and upgrading to the current (v0.4.x) security-group module. We have fixed some of the modules, but not all of them. These modules with security-group v0.3.x in the main branch are in the worst state, in that any updates to them are breaking changes that we do not want people to adapt to because there will be more and different breaking changes when we finally get to v0.4.x. We have tried to prevent volunteers from doing this, but sometimes it happens anyway, for which all we have left is to apologize.

Hey thanks for the reply! I want to be clear that I’m very thankful for what you’ve all done and not trying to suggest anything has been done or missed in malice. I tried raising and issue in the s3 bucket module for example 3 weeks ago with no response.

I’m familiar with the 0 indicator for semver but not all your modules use that some are in 1.

Also related, https://sweetops.slack.com/archives/CB6GHNLG0/p1646176401153779

https://docs.cloudposse.com/reference/best-practices/semver/

https://docs.cloudposse.com/community/faq/#what-is-our-versioning-strategy

Thanks for that. I would then ask what is the criteria for stability? For example I’ve been using the s3 bucket for well over a year with zero issues but it’s not 1.x

Happy to contribute or help in any way I can too.

Thanks for the feedback and discussion

Hey @Erik Osterman (Cloud Posse) why not just rev everything to 1.x? I mean I get the purpose of 0.x but if the api changes you are just going to rev the major anyways? Perhaps after initial development of a period of time just automatically go to 1.x and if the api changes rev the major. It might mean a few modules get into the 3-6.x range but then at least expectations of semver are there?

Whoops didn’t mean to send that last to channel. Must have accidentally checked the box,

It’s more about our “contract” with anyone depending on our modules. We wanted to be transparent that the inferfaces could change and that we wouldn’t be managing older versions. They changed a lot. Keep in mind that we’ve been at this since pre 0.10. Now everything is more mature. Now we need to mature our management of it. It’s part technical and part managerial. I think we will get there this year.

@ekristen wrote:
I’m familiar with the 0 indicator for semver but not all your modules use that some are in 1. Which modules are you referring to?

I was mistaken @Jeremy G (Cloud Posse)

Tests are running for all and I’ve approved the the simple ones. The ram_principals one I’ll likely ask @Andriy Knysh (Cloud Posse) to review again since you were working with him originally, but it LGTM.

Ah looks like because all of these PRs touch the CODEOWNERS file as part of our automation, it requires an approval from the Admins group, which means Andriy, Jeremy, or Erik need to review.

Once one of your PRs merges though, those changes will be in and then the other PRs won’t require an admin review so maybe we’ll just wait until Andriy has a minute.

i’ll review

@Alex Jurkiewicz @Matt Gowie reviewed the PRs, merged one, commented on the other two, thank you

I’ve updated https://github.com/cloudposse/terraform-aws-transit-gateway/pull/18, it should be ready to merge.

I will update the other more complex PR once this is merged

Updated https://github.com/cloudposse/terraform-aws-transit-gateway/pull/19 I provided more info for two of your suggestions Andriy let me know if you disagree with the approach still and I will go with your preference