#pr-reviews (2023-04)
Pull Request Reviews for Cloud Posse Projects
2023-04-03
timeouts added
what why references
2023-04-05
Real quick one just fixing template provider. There will be a follow up in terraform-aws-components
too
https://github.com/cloudposse/terraform-aws-datadog-lambda-forwarder/pull/51
what
• template provider • fixed in https://github.com/cloudposse/terraform-external-module-artifact/releases/tag/0.7.2
why references
thanks @Matt Gowie
follow up: https://github.com/cloudposse/terraform-aws-components/pull/626
what
• update datadog-lambda-forwarder module for darwin_arm64
why
• run on Darwin_arm64 hardware
Bats is failing.
Running output-descriptions in modules/tgw/cross-region-spoke
1..1
Test: check if terraform outputs have descriptions
File: output-descriptions.bats
---------------------------------
tgw_routes_home_region is missing a description
tgw_routes_in_region is missing a description
vpc_routes_home is missing a description
vpc_routes_this is missing a description
---------------------------------
not ok 1 check if terraform outputs have descriptions
# (in test file /test/terraform/output-descriptions.bats, line 16)
# `[ -z "$output" ]' failed
+ status=fail
But that seems related to other components.
Not sure what the deal is there. CP may be updating / adding additional testing and you’re hitting it?
@kevcube Pinged Jeremy + Dan on that issue. We’ll see if there is anything to do there.
2023-04-06
2023-04-11
@Linda Pham (Cloud Posse) cc @Andriy Knysh (Cloud Posse), let’s get this one reviewed from @joshmyers (relates indirectly to the work by @Jonathan on the bastion)
https://github.com/cloudposse/github-authorized-keys/pull/37
what
This is a bit of a grab bag of a PR, mostly because this code base hasn’t been touched in a while.
• Adds support for Github Enterprise. • Update deps. • Removes Glide and add uses Go Modules - but Make targets from the build-harness are still expecting to use Glide.
Bumping go-github to latest caused a few breakages due to deprecated methods. I have tried to change as little core logic as possible, while noting that newer go-github probably means some logic could be removed/made more efficient.
why
Because current gig uses GHE. Teleport is unfortunately not a great fit for us down to requirements.
testing
I’ve updated the tests but not added new GHE tests as getting your hands on a GHE installation is non trivial. I have however successfully built and run this branch to test working. Can pull teams/users/keys etc from GHE.
2023-04-13
@Max Lobur (Cloud Posse) @Jonathan https://github.com/cloudposse/bastion/pull/79
This disables session multiplexing to fix #42.
More information regarding MaxSessions
:
MaxSessions
Specifies the maximum number of open shell, login or subsystem (e.g. sftp) sessions permitted per network connection. Multiple sessions may be established by clients that support connection multiplexing. Setting MaxSessions to 1 will effectively disable session multiplexing, whereas setting it to 0 will prevent all shell, login and subsystem sessions while still permitting forwarding. The default is 10.
what
• Disables session multiplexing.
why
• To ensure 2FA is always used.
references
@Jonathan has joined the channel
2023-04-19
0.02 second PR that is a very quick win for the ES module - https://github.com/cloudposse/terraform-aws-elasticsearch/pull/159
We actively need it for deploying smaller dev clusters ro better leverage CloudPosse tools
what
Removing errant validation preventing the creation of Amazon OpenSearch Service ElasticSearch (ES) or OpenSearch (OS) clusters which live in a single Available Zone on Amazon Web Services (AWS)
Simple Process:
• You merge this PR • You release a new version • We can create clusters which live in one AZ • Everyone wins
why
Development and testing clusters may not want or need two or more Availability Zones for cost (among other reasons, but FinOps - saving money - is important)
references
Identical errors in internal company environment
@Linda Pham (Cloud Posse) can we get a review on this one?
what
Removing errant validation preventing the creation of Amazon OpenSearch Service ElasticSearch (ES) or OpenSearch (OS) clusters which live in a single Available Zone on Amazon Web Services (AWS)
Simple Process:
• You merge this PR • You release a new version • We can create clusters which live in one AZ • Everyone wins
why
Development and testing clusters may not want or need two or more Availability Zones for cost (among other reasons, but FinOps - saving money - is important)
references
Identical errors in internal company environment
@Jacob Hudson comment added
2023-04-21
Hello Can i please get a review on a PR for adding transit_gateway_id
support for the module cloudposse/terraform-aws-vpn-connection
- https://github.com/cloudposse/terraform-aws-vpn-connection/pull/27
what
• This PR adds ability to add transit-gateway id to the site to site vpn, with which we can attach multiple vpcs to one site-to-site vpn.
• By default it will create the site-to-site vpn with Vitrual private gateway the way it does now.
• With this PR we give the option to pass transit_gateway_enabled
to set to true then we can pass in existing_transit_gateway_id
then it will setup the VPN with transit-gateway. By default transit_gateway_enabled
is set to false
and the module sets up the Virtual Private gateway.
why
• Currently we cannot setup a VPN with transit gateway id, we can only setup with vpn virtual private gateway which is only attached to one 1 VPC.
• With Transit gateway we can attach multiple VPC’s and subnets to one transit gateway id and then specify that id in transit_gateway_id
in resource aws_vpn_connection
references
• This PR closes #28