domain-protect / terraform-aws-domain-protect

Terraform module for OWASP Domain Protect on AWS
Apache License 2.0
3 stars 0 forks source link

feat: add vpc_config #7

Closed nitrocode closed 1 month ago

nitrocode commented 2 months ago

what

why

references

github-actions[bot] commented 2 months ago

Test Results

79 tests  ±0   79 :white_check_mark: ±0   0s :stopwatch: ±0s  1 suites ±0    0 :zzz: ±0   1 files   ±0    0 :x: ±0 

Results for commit c8ab0b47. ± Comparison against base commit 62008b34.

:recycle: This comment has been updated with latest results.

github-actions[bot] commented 2 months ago

Integration Test Results

20 tests  ±0   20 :white_check_mark: ±0   6s :stopwatch: ±0s  1 suites ±0    0 :zzz: ±0   1 files   ±0    0 :x: ±0 

Results for commit c8ab0b47. ± Comparison against base commit 62008b34.

:recycle: This comment has been updated with latest results.

nitrocode commented 2 months ago

I agree that we could probably provide a security group.

For now, we can allow the consumer to provider both a VPC and security group.

We can add a new option to say if the security group is not provided (so it can be optional in the variable definition), the module will provide a security group on behalf of the consumer and provide it to each module.

Or we can wait until I test out the security group rules and we provide both the vpc_config and module security group in the same PR.

What do you think?

paulschwarzenberger commented 2 months ago

I generally favour smaller PRs, so I'm inclined to keep this one with just the VPC part, and then follow that up with a 2nd PR to provide a security group, and as you suggest also provide the option to reference an existing security group outside of Terraform. The security group ID should be an output of the Terraform module in case the user wants to add extra rules to it.