#pr-reviews (2021-04)
Pull Request Reviews for Cloud Posse Projects
2021-04-05
Can this one be reviewed (again) please? https://github.com/cloudposse/terraform-aws-cloudfront-s3-cdn/pull/147
@Frank there is issues with your PR, it looks like the logic on the count is not correct, I left a message on the PR merged and tagged you on the Bug, if you can’t figure out please let me know and I will revert the change
@Frank I saw the replies to the bug, thanks
2021-04-06
2021-04-08
2021-04-14
Can you review this PR please: https://github.com/cloudposse/terraform-aws-eks-node-group/pull/65
2021-04-19
Can someone please take a look at this one? https://github.com/cloudposse/terraform-aws-dynamic-subnets/pull/131
I’ll resurface with the contributor team to get an admin approval on this one.
@Frank merged
Thanks!
Hi folks, who is in charge of reviewing the PRs on cloudposse / terraform-aws-rds-cloudwatch-sns-alarms ?
2021-04-21
2021-04-22
not sure how to rerun the failed bridgecrew test
Can you share the output of the failed test?
i cannot see it
ah, i’ve talked to you a few times on here and assumed you were affiliated with CP. my bad haha
I will do it
Ensure the ELBv2 (Application/Network) has access logging enabled
Ensure that ALB drops HTTP headers
this is because bridgecrew thinks they need to be enable but we have a var to enable or disable it
it expect it to be enabled
@jose.amengual and other CloudPosse staff, is there a way to expose what needs to be fixed through bridgecrew? I’m guessing their TOS won’t allow that though, not sure. Alternatively, do you have any opensource ambassadors that have read access to the BridgeCrew results? With the latest updates to Checkov you’ll probably need some help with these broken builds.
not yet, we do not have a way to do that so far
I think for this we need to anotate exception rules
Yeah, I’m famiar with this part of the process. Just curious about the best way to make this part of the workflow a bit more automated and to make it so that users can either find the results or replicate the process on their side. The quick and easy solution is documenting something like this https://github.com/bridgecrewio/checkov#using-docker so that the user can do it themselves.
They said they were working on a solution for this but I have not heard back
Any chance we could we get another PR review on this one?
merged
anyone know why auto format fails like this? https://github.com/cloudposse/terraform-aws-cicd/pull/94/checks
@mihai.plesa looks to be an issue with there not being a related entry in the README.yaml. Check out other README.yaml files and copy the related key from them and remove any elements and you should be good.
thank you Matt, that worked. just needs review now
and someone to start the tests
Started the tests, but asked a question as I’m not familiar with this module. Someone else from the contributor team might be more helpful.
I’m testing now without passing website_bucket_name
@Matt Gowie please have a look now, it’s ready for tests again
@mihai.plesa merged
awesome, thank you
lol
requesting reviews on the following - please https://github.com/cloudposse/terraform-aws-cloudfront-cdn/pull/61 https://github.com/cloudposse/terraform-aws-cicd/pull/94
2021-04-23
2021-04-26
thanks @jose.amengual
Would have a look at this PR? - please Not sure what to do about the BC Scanner…
https://github.com/cloudposse/terraform-aws-cloudwatch-flow-logs/pull/16
https://github.com/cloudposse/terraform-aws-s3-bucket/pull/86 One-liner bug fix Thank you in advance!
