search

terraform-aws-modules / terraform-aws-ec2-instance

Terraform module to create AWS EC2 instance(s) resources 🇺🇦
https://registry.terraform.io/modules/terraform-aws-modules/ec2-instance/aws
Apache License 2.0
758 stars 1.88k forks source link

Allow 'source_dest_check' parameter to be applied regardless of number of network interfaces #345

Closed cristiangauma closed 1 year ago

cristiangauma commented 1 year ago

Hello,

I've encountered an issue with the current implementation of the source_dest_check parameter in the EC2 instance module. Currently, the source_dest_check parameter is only applied with the value that you are passing over the variable if you didn't assign any network_interface to your EC2. However, this implementation does not account for use cases where you add network_interfaces (for example, only one to control the network of the EC2), but the source and destination check still needs to be disabled.

The particular line would be these ones:

https://github.com/terraform-aws-modules/terraform-aws-ec2-instance/blob/d64e6b1e85efa814c4d74595496effc420d3c5d2/main.tf#L161

https://github.com/terraform-aws-modules/terraform-aws-ec2-instance/blob/d64e6b1e85efa814c4d74595496effc420d3c5d2/main.tf#L329

https://github.com/terraform-aws-modules/terraform-aws-ec2-instance/blob/d64e6b1e85efa814c4d74595496effc420d3c5d2/main.tf#L505

For instance, when using applications like Tailscale ( a zero config VPN for building secure networks), it's possible to create a Tailscale endpoint and establish a site-to-site connection while using only one interface. In such scenarios, the source_dest_check should be able to be disabled, but with the current implementation, it is not possible to do so if a single (or N) network_interfaces are added to the server.

This issue is causing problems in my current setup, and I believe it could affect others who are in similar situations.

Solution Proposal:

To resolve this issue and maintain backward compatibility, I propose that we modify the condition for applying the source_dest_check parameter. Instead of applying it only when there are no interfaces and it has been declared, we should allow it to be applied whenever it is specified, regardless of the number of network interfaces.

Something like this could work:

source_dest_check = var.source_dest_check

This change would not affect existing users who do not specify the source_dest_check parameter. It would provide the necessary flexibility for those who need to disable the source and destination check not matter how many network interfaces they have.

Any feedback about this?. Does it make sense to open a PR to modify it?. Any corner case that I'm skipping?

EDITED: I've edited the issue because my assumptions were not correct. Now it should be ok :).

cristiangauma commented 1 year ago

Nevermind :).

I've found the reasoning behind that. The source_dest_check must be put in each on the network_interfaces. I thought (as in the AWS console was put in general) that it was more on the EC2 configuration and not in each one of the network interfaces.

I'm closing this.

github-actions[bot] commented 1 year ago

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.