#pr-reviews (2020-04)
Pull Request Reviews for Cloud Posse Projects
2020-04-01
![Adam Crews avatar](https://secure.gravatar.com/avatar/a89eb6a92f44dab8ce702283b7a38820.jpg?s=72&d=https%3A%2F%2Fa.slack-edge.com%2Fdf10d%2Fimg%2Favatars%2Fava_0002-72.png)
Would it be possible to get a review of https://github.com/cloudposse/terraform-aws-s3-log-storage/pull/25 ?
This is a change to the current default behavior, where buckets will be left in a state where they could be made public, and instead makes the buckets block public access by default.
![Erik Osterman (Cloud Posse) avatar](https://secure.gravatar.com/avatar/88c480d4f73b813904e00a5695a454cb.jpg?s=72&d=https%3A%2F%2Fa.slack-edge.com%2Fdf10d%2Fimg%2Favatars%2Fava_0023-72.png)
@Andriy Knysh (Cloud Posse) looks like a quick one (@Maxim Mironenko (Cloud Posse) is on vacation this week)
![party_parrot](/assets/images/custom_emojis/party_parrot.gif)
![Andriy Knysh (Cloud Posse) avatar](https://avatars.slack-edge.com/2018-06-13/382332470551_54ed1a5d986e2068fd9c_72.jpg)
checking
2020-04-02
![Matt Gowie avatar](https://avatars.slack-edge.com/2023-02-06/4762019351860_44dadfaff89f62cba646_72.jpg)
Hey any chance I could get a push in the right direction on this PR? https://github.com/cloudposse/terraform-aws-ssm-parameter-store/pull/12
Info Following up on #10, this PR uses that fork and then applies the most recent requirement: Updates the type for parameter_write variable so terraform can run without errors.
![Mikhail Naletov avatar](https://avatars.slack-edge.com/2021-08-27/2436309147940_5870ad432ab4889ee4a9_72.png)
@Mikhail Naletov has joined the channel
![Mikhail Naletov avatar](https://avatars.slack-edge.com/2021-08-27/2436309147940_5870ad432ab4889ee4a9_72.png)
Hi! I’d be very glad if somebody took a look at my PRs
https://github.com/cloudposse/terraform-aws-elastic-beanstalk-environment/pull/108 https://github.com/cloudposse/terraform-aws-elastic-beanstalk-application/pull/19
Thanks!
what Environment variable why Most cloudposse modules have environment variable. This one doesn’t. The PR fixes it.
![Erik Osterman (Cloud Posse) avatar](https://secure.gravatar.com/avatar/88c480d4f73b813904e00a5695a454cb.jpg?s=72&d=https%3A%2F%2Fa.slack-edge.com%2Fdf10d%2Fimg%2Favatars%2Fava_0023-72.png)
Next week we should be faster at reviewing
![Erik Osterman (Cloud Posse) avatar](https://secure.gravatar.com/avatar/88c480d4f73b813904e00a5695a454cb.jpg?s=72&d=https%3A%2F%2Fa.slack-edge.com%2Fdf10d%2Fimg%2Favatars%2Fava_0023-72.png)
sorry for the hold up
2020-04-06
![jose.amengual avatar](https://secure.gravatar.com/avatar/32f267b819eac9e0ea6a8324b53064a0.jpg?s=72&d=https%3A%2F%2Fa.slack-edge.com%2Fdf10d%2Fimg%2Favatars%2Fava_0024-72.png)
@jose.amengual has joined the channel
![jose.amengual avatar](https://secure.gravatar.com/avatar/32f267b819eac9e0ea6a8324b53064a0.jpg?s=72&d=https%3A%2F%2Fa.slack-edge.com%2Fdf10d%2Fimg%2Favatars%2Fava_0024-72.png)
@here https://github.com/cloudposse/terraform-aws-s3-bucket/pull/21 when yoy have a chance
![Mikhail Naletov avatar](https://avatars.slack-edge.com/2021-08-27/2436309147940_5870ad432ab4889ee4a9_72.png)
Thanks for the reviews
2020-04-08
![Joe Niland avatar](https://secure.gravatar.com/avatar/b90c8e752dd648ef229096c60ba2408f.jpg?s=72&d=https%3A%2F%2Fa.slack-edge.com%2Fdf10d%2Fimg%2Favatars%2Fava_0022-72.png)
@Joe Niland has joined the channel
![Joe Niland avatar](https://secure.gravatar.com/avatar/b90c8e752dd648ef229096c60ba2408f.jpg?s=72&d=https%3A%2F%2Fa.slack-edge.com%2Fdf10d%2Fimg%2Favatars%2Fava_0022-72.png)
[reposted from #terraform]
Hi @Maxim Mironenko (Cloud Posse) would you mind reviewing this PR? I need a bit of help with the bats tests as well. When you have time, I’ll explain what I tried already. Thanks!! https://github.com/cloudposse/terraform-aws-ec2-bastion-server/pull/27
I have also added one example and tests, however I will need a bit of help to get the tests fully working.
![Maxim Mironenko (Cloud Posse) avatar](https://avatars.slack-edge.com/2019-01-14/522357805937_0a3ceb59638dd335f2c8_72.jpg)
sure, whats up with bats?
I have also added one example and tests, however I will need a bit of help to get the tests fully working.
![Maxim Mironenko (Cloud Posse) avatar](https://avatars.slack-edge.com/2019-01-14/522357805937_0a3ceb59638dd335f2c8_72.jpg)
@Joe Niland give me some context of your problems
![Joe Niland avatar](https://secure.gravatar.com/avatar/b90c8e752dd648ef229096c60ba2408f.jpg?s=72&d=https%3A%2F%2Fa.slack-edge.com%2Fdf10d%2Fimg%2Favatars%2Fava_0022-72.png)
Sure, one moment
![Joe Niland avatar](https://secure.gravatar.com/avatar/b90c8e752dd648ef229096c60ba2408f.jpg?s=72&d=https%3A%2F%2Fa.slack-edge.com%2Fdf10d%2Fimg%2Favatars%2Fava_0022-72.png)
I copied the tests from another cloudposse module and modified them. They are here: https://github.com/cloudposse/terraform-aws-ec2-bastion-server/pull/27/files#diff-920672592109ed71b04a314cf153cb22 I know I need to add a couple more tests but just getting started.
I run make
to run the tests.
All the initial ones are OK until check if terraform code is valid
.
This just seems to run terraform validate
.
If I run this manually it is returning success.
I have also added one example and tests, however I will need a bit of help to get the tests fully working.
![Joe Niland avatar](https://secure.gravatar.com/avatar/b90c8e752dd648ef229096c60ba2408f.jpg?s=72&d=https%3A%2F%2Fa.slack-edge.com%2Fdf10d%2Fimg%2Favatars%2Fava_0022-72.png)
Once I sort that out, I’ll see if the apply test works.
![Joe Niland avatar](https://secure.gravatar.com/avatar/b90c8e752dd648ef229096c60ba2408f.jpg?s=72&d=https%3A%2F%2Fa.slack-edge.com%2Fdf10d%2Fimg%2Favatars%2Fava_0022-72.png)
Should be something silly I guess!
![Maxim Mironenko (Cloud Posse) avatar](https://avatars.slack-edge.com/2019-01-14/522357805937_0a3ceb59638dd335f2c8_72.jpg)
ok, I will pull your branch and test it on my local
![Joe Niland avatar](https://secure.gravatar.com/avatar/b90c8e752dd648ef229096c60ba2408f.jpg?s=72&d=https%3A%2F%2Fa.slack-edge.com%2Fdf10d%2Fimg%2Favatars%2Fava_0022-72.png)
Thank you
![Maxim Mironenko (Cloud Posse) avatar](https://avatars.slack-edge.com/2019-01-14/522357805937_0a3ceb59638dd335f2c8_72.jpg)
will let you know
![Joe Niland avatar](https://secure.gravatar.com/avatar/b90c8e752dd648ef229096c60ba2408f.jpg?s=72&d=https%3A%2F%2Fa.slack-edge.com%2Fdf10d%2Fimg%2Favatars%2Fava_0022-72.png)
@Maxim Mironenko (Cloud Posse) sorry just pushed a small fix for one of the test conditions
![Maxim Mironenko (Cloud Posse) avatar](https://avatars.slack-edge.com/2019-01-14/522357805937_0a3ceb59638dd335f2c8_72.jpg)
ok
![Maxim Mironenko (Cloud Posse) avatar](https://avatars.slack-edge.com/2019-01-14/522357805937_0a3ceb59638dd335f2c8_72.jpg)
@Joe Niland, please, check for my commit. I’ve fixed validation for example
module
![Joe Niland avatar](https://secure.gravatar.com/avatar/b90c8e752dd648ef229096c60ba2408f.jpg?s=72&d=https%3A%2F%2Fa.slack-edge.com%2Fdf10d%2Fimg%2Favatars%2Fava_0022-72.png)
@Maxim Mironenko (Cloud Posse) thanks - i’ll check it out
![Joe Niland avatar](https://secure.gravatar.com/avatar/b90c8e752dd648ef229096c60ba2408f.jpg?s=72&d=https%3A%2F%2Fa.slack-edge.com%2Fdf10d%2Fimg%2Favatars%2Fava_0022-72.png)
ah I see - the validation was failing for the examples/complete module
![Joe Niland avatar](https://secure.gravatar.com/avatar/b90c8e752dd648ef229096c60ba2408f.jpg?s=72&d=https%3A%2F%2Fa.slack-edge.com%2Fdf10d%2Fimg%2Favatars%2Fava_0022-72.png)
@Maxim Mironenko (Cloud Posse) PR is updated
![Maxim Mironenko (Cloud Posse) avatar](https://avatars.slack-edge.com/2019-01-14/522357805937_0a3ceb59638dd335f2c8_72.jpg)
thanks, will check for it
![Joe Niland avatar](https://secure.gravatar.com/avatar/b90c8e752dd648ef229096c60ba2408f.jpg?s=72&d=https%3A%2F%2Fa.slack-edge.com%2Fdf10d%2Fimg%2Favatars%2Fava_0022-72.png)
Thanks!
![Joe Niland avatar](https://secure.gravatar.com/avatar/b90c8e752dd648ef229096c60ba2408f.jpg?s=72&d=https%3A%2F%2Fa.slack-edge.com%2Fdf10d%2Fimg%2Favatars%2Fava_0022-72.png)
Hi @Maxim Mironenko (Cloud Posse) I was wondering if you have had time to look at this one. Let me know if I can fix anything.
![Maxim Mironenko (Cloud Posse) avatar](https://avatars.slack-edge.com/2019-01-14/522357805937_0a3ceb59638dd335f2c8_72.jpg)
Hi @Joe Niland! Will rise the priority of this one. Looks like the only thing left is to setup pipeline for tests. Should be done shortly. Stay tuned and thanks for reminder!
![Joe Niland avatar](https://secure.gravatar.com/avatar/b90c8e752dd648ef229096c60ba2408f.jpg?s=72&d=https%3A%2F%2Fa.slack-edge.com%2Fdf10d%2Fimg%2Favatars%2Fava_0022-72.png)
Thanks @Maxim Mironenko (Cloud Posse)!
2020-04-10
![jose.amengual avatar](https://secure.gravatar.com/avatar/32f267b819eac9e0ea6a8324b53064a0.jpg?s=72&d=https%3A%2F%2Fa.slack-edge.com%2Fdf10d%2Fimg%2Favatars%2Fava_0024-72.png)
another PR for RDS https://github.com/cloudposse/terraform-aws-rds-cluster/pull/64
Adding configurable timeouts for create, update and delete actions. Default is to 120m base onf the Terraform default.
![jose.amengual avatar](https://secure.gravatar.com/avatar/32f267b819eac9e0ea6a8324b53064a0.jpg?s=72&d=https%3A%2F%2Fa.slack-edge.com%2Fdf10d%2Fimg%2Favatars%2Fava_0024-72.png)
any takers ?
Adding configurable timeouts for create, update and delete actions. Default is to 120m base onf the Terraform default.
![jose.amengual avatar](https://secure.gravatar.com/avatar/32f267b819eac9e0ea6a8324b53064a0.jpg?s=72&d=https%3A%2F%2Fa.slack-edge.com%2Fdf10d%2Fimg%2Favatars%2Fava_0024-72.png)
anyone @here available ?
![Maxim Mironenko (Cloud Posse) avatar](https://avatars.slack-edge.com/2019-01-14/522357805937_0a3ceb59638dd335f2c8_72.jpg)
@jose.amengual I am on it
![jose.amengual avatar](https://secure.gravatar.com/avatar/32f267b819eac9e0ea6a8324b53064a0.jpg?s=72&d=https%3A%2F%2Fa.slack-edge.com%2Fdf10d%2Fimg%2Favatars%2Fava_0024-72.png)
awesome sir
2020-04-13
2020-04-14
2020-04-20
![cabrinha avatar](https://secure.gravatar.com/avatar/a60e998ca395399f6ec8cdd190fac1ab.jpg?s=72&d=https%3A%2F%2Fa.slack-edge.com%2Fdf10d%2Fimg%2Favatars%2Fava_0022-72.png)
@cabrinha has joined the channel
![cabrinha avatar](https://secure.gravatar.com/avatar/a60e998ca395399f6ec8cdd190fac1ab.jpg?s=72&d=https%3A%2F%2Fa.slack-edge.com%2Fdf10d%2Fimg%2Favatars%2Fava_0022-72.png)
Hello there, I’d like to disable the creation of the s3 endpoint when using the EMR module: https://github.com/cloudposse/terraform-aws-emr-cluster/pull/14 – I’ve already got an S3 endpoint managed somewhere else.
2020-04-22
2020-04-23
![Adam Crews avatar](https://secure.gravatar.com/avatar/a89eb6a92f44dab8ce702283b7a38820.jpg?s=72&d=https%3A%2F%2Fa.slack-edge.com%2Fdf10d%2Fimg%2Favatars%2Fava_0002-72.png)
I’d love a review of https://github.com/cloudposse/terraform-aws-eks-workers/pull/33 if someone has a few moments. It pretty simple, and just adds the environment
value from the label module into the rest of the resources.
2020-04-24
![Matt Gowie avatar](https://avatars.slack-edge.com/2023-02-06/4762019351860_44dadfaff89f62cba646_72.jpg)
Hey folks — I just opened up Issue #207 on build-harness
. Is there a reason ya’ll at CloudPosse are printing that message and not running terraform validate
? Wanted to check here before I submitted a PR to fix.
Describe the Bug Running make terraform/validate on Terraform v0.12.X prints the following message: Terraform 0.12 does not support validate without skipping variables Expected Behavior I would exp…
![Matt Gowie avatar](https://avatars.slack-edge.com/2023-02-06/4762019351860_44dadfaff89f62cba646_72.jpg)
Aha this is due to https://github.com/terraform-providers/terraform-provider-aws/issues/11058 / https://github.com/hashicorp/terraform/issues/21408
That’s a PITA…
This issue was originally opened by @Craigfis as hashicorp/terraform#23517. It was migrated here as a result of the provider split. The original body of the issue is below. I'm using Terraform …
I ran the command terraform validate -check-variables=false and I got the below output. It seems like the command is not well format. However, when I run terraform validate then the command seems f…
![Matt Gowie avatar](https://avatars.slack-edge.com/2023-02-06/4762019351860_44dadfaff89f62cba646_72.jpg)
Aha — there is a workaround though… providing AWS_DEFAULT_REGION
before tf validate
allows validate to not throw that error. I’m going to put up a PR to fix…
I ran the command terraform validate -check-variables=false and I got the below output. It seems like the command is not well format. However, when I run terraform validate then the command seems f…
![Matt Gowie avatar](https://avatars.slack-edge.com/2023-02-06/4762019351860_44dadfaff89f62cba646_72.jpg)
what Updates the terraform/validate target to actually run terraform validate instead of logging "unsupported" message. why This is to provide actual validation on v0.12 Seems the work…
![Erik Osterman (Cloud Posse) avatar](https://secure.gravatar.com/avatar/88c480d4f73b813904e00a5695a454cb.jpg?s=72&d=https%3A%2F%2Fa.slack-edge.com%2Fdf10d%2Fimg%2Favatars%2Fava_0023-72.png)
@Matt Gowie won’t get to this until next week. @Maxim Mironenko (Cloud Posse) working on some other fixes at the moment.
![Matt Gowie avatar](https://avatars.slack-edge.com/2023-02-06/4762019351860_44dadfaff89f62cba646_72.jpg)
@Erik Osterman (Cloud Posse) No rush at all — I just ran into a handful of CP build-harness/test-harness issues while working on https://github.com/cloudposse/terraform-aws-ssm-parameter-store/pull/12.
Info Following up on #10, this PR uses that fork and then applies the most recent requirement: Updates the type for parameter_write variable so terraform can run without errors.