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

Update cidrnetmask for IPv6 #30681

Closed
wants to merge 1 commit into from
Closed

Conversation

laurapacilio
Copy link
Contributor

Fix to prevent docs users from being confused until this issue is completed: #30680

This function does not actually produce an error if you use IPv6, even though it should. Rather than inaccurately saying it produces an error, we should make a statement to discourage users from using the function with IPv6.

@apparentlymart
Copy link
Member

I would prefer not to merge this because if we document that this function supports IPv6 addresses, even if not recommended, the current behavior will arguably become covered by the v1.0 compatibility promises and thus not correctable without a breaking change.

Currently the incorrect behavior is covered under this statement:

If the actual behavior of a feature differs from what we explicitly documented as the feature's behavior, we will usually treat that as a bug and change the feature to match the documentation, although we will avoid making such changes if they seem likely to cause broad compatibility problems. We cannot promise to always remain "bug-compatible" with previous releases, but we will consider such fixes carefully to minimize their impact.

My estimation is that this is not "likely to cause broad compatibility problems" because there is no real-world use to an IPv6 netmask and thus it should be pretty rare for a configuration to use one, and thus I think it's defensible to change Terraform to give better feedback when asked to produce a nonsensical result here. However, if we change the documentation to say that this function accepts IPv6 addresses then the above clause no longer applies.

@laurapacilio
Copy link
Contributor Author

okay! works for me! :-) Thanks for the feedback!

@mtslzr
Copy link

mtslzr commented Mar 24, 2022

Could the function in question be fixed to actually throw an error for IPv6 addresses, since that's the issue that tripped me up? If I put in a PR for that change, would that be more likely to get traction?

EDIT: Nevermind, ignore me. #30703

@github-actions
Copy link

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

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants