#pr-reviews (2023-10)

Pull Request Reviews for Cloud Posse Projects

2023-10-02

Frank

https://github.com/cloudposse/terraform-aws-ecs-container-definition/pull/170

#170 fix: remove duplicate name in portMappings

what

• Removed duplicate name in portMappings

why

• Object constructor map keys must be unique

1

1

jose.amengual

could you run make precommit/terraform and commit the changes please?

#170 fix: remove duplicate name in portMappings

what

• Removed duplicate name in portMappings

why

• Object constructor map keys must be unique

2023-10-03

2023-10-10

Nate McCurdy

Looking for review from cloudposse/engineering please: https://github.com/cloudposse/terraform-aws-lambda-function/pull/45

#45 fix: Add context tags to the IAM resources

what

Add tags = module.this.tags to each of the IAM resources so that they use the tags determined by the null/label context or the tags input.

why

Prior to this, the aws_iam_role and the aws_iam_policy created by this module did not include any of the tags passed via tags or via context.

1

Nate McCurdy

And a couple of other fixes for terraform-aws-lambda-function for review please:

• https://github.com/cloudposse/terraform-aws-lambda-function/pull/44

• https://github.com/cloudposse/terraform-aws-lambda-function/pull/46

#44 fix: Add null/label context tags to the aws_lambda_function resource

#46 fix: Allow for custom_iam_policy_arns that don't exist yet

1

Erik Osterman (Cloud Posse)

@Gabriela Campana (Cloud Posse)

#44 fix: Add null/label context tags to the aws_lambda_function resource

#46 fix: Allow for custom_iam_policy_arns that don't exist yet

Dan Miller (Cloud Posse)

merged. Thanks for the contribution!

1

Nate McCurdy

Thanks for reviewing!

1

2023-10-11

2023-10-12

kevcube

https://github.com/cloudposse/terraform-aws-components/pull/872

#872 [aurora-postgres] fix tflint and markdownlint

what

• fix tflint and markdownlint complaints

why

• we want to use this component and instead of just fixing in our repo, I upstream

references

2023-10-15

Ahmed Kamal

Not mine, but can we please get this simple one merged https://github.com/cloudposse/terraform-aws-ecs-web-app/pull/235/files

1

2023-10-16

kevcube

https://github.com/cloudposse/terraform-aws-components/pull/876

quick one plzplz

#876 [aurora-postgres] add intra_security_group_traffic_enabled

what

• allow traffic within db security group

why

• providing network access via shared security groups rather than exposing input/output rules in multiple security groups.

references

cloudposse/terraform-aws-rds-cluster#145

1

2023-10-17

Nitin

https://github.com/cloudposse/terraform-aws-nlb/pull/49

#49 Fix Readme Example

what

Fix typo in readme example.

Added support for IPv6 target group in ip mode

why

invalid variable name was there in the readme example
target groups in ip mode allow targets to register using ipv6. where service runs in ipv6 mode. only

1

2023-10-19

Dominique Dumont

Hi. This PR is currently blocked due a failed test in the CI. This looks like a temporary issue. I’ve run the same test on my client’s infra without problem.

Can anyone retry the CI on cloudposse’s infra ? https://github.com/cloudposse/terraform-aws-tfstate-backend/pull/142

#142 feat: add user_policy_document parameter

what

This parameter allows the user to specify policies that are applied to the S3 bucket with the policies defined by this module.

why

We want to add policies that forbid non admin users to access the bucket containing tfstates.

This commit allow us to specify a policy that implement these restriction without clobbering the policies put in place by this module.

Note that I have no problem to change the name of this new parameter if you want another.

references

Closes: #115

1

1

Jeremy G (Cloud Posse)

Updated, approved, and merged. Sorry it took so long.

#142 feat: add user_policy_document parameter

what

This parameter allows the user to specify policies that are applied to the S3 bucket with the policies defined by this module.

why

We want to add policies that forbid non admin users to access the bucket containing tfstates.

This commit allow us to specify a policy that implement these restriction without clobbering the policies put in place by this module.

Note that I have no problem to change the name of this new parameter if you want another.

references

Closes: #115

1