#pr-reviews (2022-08)

Pull Request Reviews for Cloud Posse Projects

2022-08-01

Cody Moore avatar
Cody Moore
12:53:26 AM

Hey crew, a buddy and I are working on some EKS changes, would love a review on this whenever someone gets a chance: https://github.com/cloudposse/terraform-aws-eks-node-group/pull/126 (needs another reviewer besides me )

What did I do

• Add detailed monitoring flag to the launch template of EC2 nodes

Why did I do this

• Some compliance tools will flag nodes used by this module because they don’t have detailed monitoring. This also allows metrics to be reported every minute as opposed to five minute intervals

Helpful references

• More AWS Documentation

1
Cody Moore avatar
Cody Moore
05:19:51 PM

If I can get a team member approval from @cloudposse-team that would be appreciated

What did I do

• Add detailed monitoring flag to the launch template of EC2 nodes

Why did I do this

• Some compliance tools will flag nodes used by this module because they don’t have detailed monitoring. This also allows metrics to be reported every minute as opposed to five minute intervals

Helpful references

• More AWS Documentation

Jeremy G (Cloud Posse) avatar
Jeremy G (Cloud Posse)
06:27:28 PM

Approved, merged, released as v2.6.0.

1

2022-08-02

roth.andy avatar
roth.andy
11:38:56 PM

https://github.com/cloudposse/terraform-aws-eks-workers/pull/93 pretty please

Useful for deploying to GovCloud since their partition there is “aws-us-gov” instead of just “aws”

what

Support other AWS partitions by templatizing ARNs that are currently hard coded

why

So the module can be used in other AWS partitions like GovCloud

references

Closes #92

1

2022-08-04

Sam Skynner avatar
Sam Skynner
09:09:38 AM

https://github.com/cloudposse/terraform-aws-ssm-tls-self-signed-cert/pull/14 If possible This breaks a number of other modules right now which are dependent on this

what

• Remove key_algorithm from tls_cert_request

why

• Deprecated

references

• Closes #13 • https://registry.terraform.io/providers/hashicorp/tls/latest/docs • https://registry.terraform.io/providers/hashicorp/tls/latest/docs/resources/cert_request#key_algorithm

1
RB (Ronak) (Cloud Posse) avatar
RB (Ronak) (Cloud Posse)
12:21:12 PM

Cc @Andriy Knysh (Cloud Posse) @Jeremy G (Cloud Posse)

what

• Remove key_algorithm from tls_cert_request

why

• Deprecated

references

• Closes #13 • https://registry.terraform.io/providers/hashicorp/tls/latest/docs • https://registry.terraform.io/providers/hashicorp/tls/latest/docs/resources/cert_request#key_algorithm

1
Jeremy G (Cloud Posse) avatar
Jeremy G (Cloud Posse)
06:49:45 PM

Released as v1.0.0

Sam Skynner avatar
Sam Skynner
06:51:12 PM

thank you so much for the quick reply

SergeiV avatar
SergeiV
08:20:59 PM

I think vpn module will still fail because version for ssm-tls module is pinned to 0.5 : https://github.com/cloudposse/terraform-aws-ec2-client-vpn/blob/master/main.tf#L25

  version = "0.5.0"
RB (Ronak) (Cloud Posse) avatar
RB (Ronak) (Cloud Posse)
08:30:37 PM

https://github.com/cloudposse/terraform-aws-ec2-client-vpn/pull/50#pullrequestreview-1062552564

RB (Ronak) (Cloud Posse) avatar
RB (Ronak) (Cloud Posse)
08:30:48 PM

https://github.com/cloudposse/terraform-aws-ec2-client-vpn/releases/tag/0.12.6

RB (Ronak) (Cloud Posse) avatar
RB (Ronak) (Cloud Posse)
08:31:04 PM

this release may take about 20 min or so to be added to the registry

SergeiV avatar
SergeiV
08:32:13 PM

excellent, thank you very much !

2022-08-08

Oscar Jara avatar
Oscar Jara
05:51:05 PM

Hi all. I think I found a bug in the terraform-aws-ecs-web-app module. EFS volumes are not being correctly set in the resulting task definition by the the terraform-aws-ecs-alb-service-task Draft PR here. Who can I discuss this with? Thanks!

what

Creates a separated variable for EFS volumes.

why

• The way terraform-aws-ecs-web-app defines and uses the EFS volumes with the terraform-aws-ecs-alb-service-task module is not working correctly and the EFS volume is not being set in the resulting task definition. • By separating EFS volumes and Docker volumes they can be assigned to the docker_volumes and efs_volumes as expected by the module.

mihai avatar
mihai
02:48:07 PM

looking forward to this

what

Creates a separated variable for EFS volumes.

why

• The way terraform-aws-ecs-web-app defines and uses the EFS volumes with the terraform-aws-ecs-alb-service-task module is not working correctly and the EFS volume is not being set in the resulting task definition. • By separating EFS volumes and Docker volumes they can be assigned to the docker_volumes and efs_volumes as expected by the module.

2022-08-09

    keyboard_arrow_up