search

rancher / tf-rancher-up

MIT License
14 stars 5 forks source link

Review of AWS infra modules and tests #158

Open glovecchi0 opened 1 month ago

glovecchi0 commented 1 month ago

Division of the PR #145

dkeightley commented 2 weeks ago

Thanks @glovecchi0, for a long term approach is there any hesitance to using official modules for the VPC resources?

These modules follow best practices with dependency management etc., but will also unlock all the flexibility to users and reduce maintenance from our side without repeating code.

Is there a reason to move the module into ../ec2/? This will break existing environments and seems to be the only module nested further into a directory, or am I overlooking some info on why that needs to happen?

glovecchi0 commented 2 weeks ago

Thanks @glovecchi0, for a long term approach is there any hesitance to using official modules for the VPC resources?

These modules follow best practices with dependency management etc., but will also unlock all the flexibility to users and reduce maintenance from our side without repeating code.

Is there a reason to move the module into ../ec2/? This will break existing environments and seems to be the only module nested further into a directory, or am I overlooking some info on why that needs to happen?

Hey @dkeightley!! 😄 The idea of ​​creating the ec2 folder (as also done for Google forms) is to give the possibility of creating the various infra resources in different locations, for order and cleanliness. This is also a stylistic choice; it can be changed.

It makes absolute sense to use the official modules of the various Cloud Providers; I think the same goes for EC2, etc. not just for the Network. Maybe we can open an FR for this.

suryaboorlu commented 2 weeks ago

@glovecchi0 Overall, everything looks solid to me. I've included a few suggestions that you might find helpful—please take a look and see if they resonate with you.