Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow for selectively disabling IPv6 addresses #217

Conversation

johnseekins-pathccm
Copy link

what

Allow for selectively disabling IPv6 addresses

why

We don't want IPv6 addresses in our private subnets, only public subnets. This PR lets that happen.

references

variables.tf Outdated Show resolved Hide resolved
Signed-off-by: John Seekins <[email protected]>
Signed-off-by: John Seekins <[email protected]>
variables.tf Outdated Show resolved Hide resolved
variables.tf Outdated Show resolved Hide resolved
Signed-off-by: John Seekins <[email protected]>
@RoseSecurity
Copy link

/terratest

@Nuru
Copy link
Contributor

Nuru commented Dec 10, 2024

Thank you @johnseekins-pathccm for this PR, and thank you @RoseSecurity for your help with it.

This is an unusual use case, as evidenced by no one asking for it before now, even though IPv6 support was added over 2 years ago. As such, I'm hesitant to add these variables, as they can cause confusion. We already had a "master switch" in ipv6_enabled, and now we are adding suppression switches to limit it. @RoseSecurity is correct that the Cloud Posse standard is for feature flags to end with _enabled, but whether we use enabled or disabled, it is still confusing to me that you have the private and public features that have no effect unless the master feature is enabled.

I know the proposal I'm about to make is more obscure and a bit trickier to implement, but in some ways I prefer that for such a rare use case.

How about we do not add any new variables, but instead leverage the ipv6_cidrs object, adding optional private_enabled and public_enabled booleans defaulting to true? We can do the same for ipv4_cidrs and maintain feature parity between IPv4 and IPv6.

It's true that we are still adding 2 (or 4) boolean suppression flags, but by keeping them somewhat buried, I think we make it easier for people to ignore them and not worry about them or expect too much from them.

@johnseekins-pathccm
Copy link
Author

The more I look at this, the less it feels useful. Closing.

@mergify mergify bot removed the triage Needs triage label Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants