#pr-reviews - December 2020

hey team @Brandon Wilson I am going through a bunch of your modules that I use but I am on TF 0.14….

https://github.com/cloudposse/terraform-aws-iam-system-user/pull/38
https://github.com/cloudposse/terraform-aws-dynamodb-autoscaler/pull/27
https://github.com/cloudposse/terraform-aws-route53-cluster-hostname/pull/29

once these are in, i’ll do the next round

Hi! Could someone take a look?

https://github.com/cloudposse/terraform-aws-ecs-cloudwatch-autoscaling/pull/9
https://github.com/cloudposse/terraform-aws-ecs-web-app/pull/79

@here PSA: we're working on some of the underlying scaffolding and tooling to better support 0.14 and future updates. 0.13 was a big pain, and we learned a lot. A few things are happening behind the scenes:

• We're switching everything on our side over to the terraform registry notation so we can use rennovatebot (which doesn't like our ref=tags/... format).
• In order to support the registry notation, we needed to update our test-harness to support bats (this is done, but is not backwards compatible).
• We're adding mergify to quickly merge automated PRs, but were quickly blocked by the next hurdle: mergify cannot be a CODEOWNER because it's a GitHub App. So we need to upgrade our account. Working on that (but it's expensive!)
• We've added added the make targets to quickly convert a module to use the "new" (but not so new) provider and registry notation. But turns out many have trouble running this with the build-harness natively, so we're going to add a make docker/shell target to run it in a container and mount the cwd into the container. This will also help with make readme and other things like it.
• We've added the github actions to automatically rebuild the README nightly, but it's blocked on the mergify issue above.
• We've added the github actions to automatically update the context.tf from the central copy in terraform-null-label
• We've added the make target to the build-harness which will update the lower-bound pinning for modules pinned to >= 0.12 to be >=0.12.26 (to support new provider syntax)
• We've drafted the rennovatebot configuration to automatically update module pinning and run tests, then merge when they pass.
All of this work will make all future upgrades of terraform breezy. Unfortunately, with so many changes, we ran into the inevitable rough edges.

This is all to say, we're currently blocked on testing PRs because tests are failing due to our changes. We're working to fix those. ETA is by end-of-the-week.

Can someone please let me know if this PR is good enough?
This is my first PR, so if something is missing please educate me.
https://github.com/cloudposse/terraform-aws-rds/pull/79

Hello 🙂 I have a PR for you.
This adds a delivery policy to an SNS topic (to override the default). I’m still new to making PRs here so please tell me if I’m doing something wrong 🙂
https://github.com/cloudposse/terraform-aws-sns-topic/pull/23

hey hey, requesting a test run and quick review on https://github.com/cloudposse/terraform-aws-cloudfront-s3-cdn/pull/116

Hey guys,
tiny PR for enabling terraform-aws-rds to work with terraform 14
https://github.com/cloudposse/terraform-aws-rds/pull/80

Hey y'all, can somebody take a look at this one: https://github.com/cloudposse/terraform-aws-ec2-bastion-server/pull/45

I was trying to fix it locally and submit a PR before I saw this one and it looks good to me! 😄

@Jeremy G (Cloud Posse) May I ask you to fix validate-codeowners for repos, please:
https://github.com/cloudposse/terraform-aws-rds/pull/81
https://github.com/cloudposse/terraform-tls-ssh-key-pair/pull/21
https://github.com/cloudposse/terraform-aws-ecs-cloudwatch-autoscaling/pull/10

Announcement: We have created some tools to help update our Terraform modules so that they work with Terraform 0.14. Please read the instructions at https://docs.cloudposse.com/community/updating-modules-for-terraform-14/ if you want to help by converting a module.

Hey Cloudposse, someone can review @Maxim Mironenko (Cloud Posse) PR?
https://github.com/cloudposse/terraform-aws-rds/pull/81

@Jeremy G (Cloud Posse) Please, fix this repo with proper codeowners, when you have time:
<https://github.com/cloudposse/terraform-aws-vpc-peering/pull/28>

@Jeremy G (Cloud Posse) and this one, please:
<https://github.com/cloudposse/terraform-aws-cloudformation-stack/pull/8>

@Jeremy G (Cloud Posse) and this, please:
<https://github.com/cloudposse/terraform-aws-s3-website/pull/46>

First time submitting a PR to one of these repos. Would be grateful for any feedback to make this more in line with expectations. Hoping this is an easy merge. 🙂 https://github.com/cloudposse/terraform-aws-s3-bucket/pull/66

@Jeremy G (Cloud Posse) if still available, I think this one needs the Codeowners validation fix: https://github.com/cloudposse/terraform-aws-ec2-bastion-server/pull/49

@Jeremy G (Cloud Posse) Please, fix these repos with proper codeowners, when you have time:
<https://github.com/cloudposse/terraform-aws-documentdb-cluster/pull/20>
<https://github.com/cloudposse/terraform-aws-ec2-instance-group/pull/18>
<https://github.com/cloudposse/terraform-aws-ec2-autoscale-group/pull/39>
<https://github.com/cloudposse/terraform-aws-vpn-connection/pull/9>

hello there! can someone review this PR (https://github.com/cloudposse/terraform-aws-ecs-container-definition/pull/107) please? cheers

@Makeshift (Connor Bell) @Joe Niland

Hello, another PR here for you (Allow configuring SCRAM authentication for MSK): https://github.com/cloudposse/terraform-aws-msk-apache-kafka-cluster/pull/6