Fix AWS network ACL ICMP rules changing on every plan/apply

[fazy]

When ICMP rules are present and from_port or to_port are set to non-zero values, terraform always sees the rules as changed and recreates them (issue: #4423).

This change forces the affected values to zero for ICMP rules when creating a hash of the ACL values to detect if a change is needed.

Values are hashed in the same order as before, so users should not see any change unless they were affected by this issue already; affected users should see only one change, after which the new hash values will be stored in the .tfstate file.

Documentation updated to clarify that a 0 value is needed (as it is possible to guess "-1" after looking at the protocol "all" value).

[fazy]

I did also look at fixing this issue by making from_port and to_port optional, but validated later to make it required on non-ICMP protocols. This proved problematic though:

1. Because the validation happens very late (unless I rewrite the update function to do all checks up front).
2. If I make the port values optional, they default to 0. As this is a valid (albeit probably not useful) value, I can't use this as the basis of a validation check.

Anyway, I'm open to any suggestions on how to do this better.

[treyharris]

Is there any chance this will be incorporated? Now that the terraform.tfstate version has changed, and 0.7.0 still has this issue? Is the only workaround converting inline rules into separate rules?

[mmack]

I have the same problem...

[apparentlymart]

Hello @fazy, and thanks for working on this!

As part of the the Terraform 0.10 release earlier this year, all of the Terraform providers were moved to their own repositories in the terraform-providers GitHub organization, and removed from the Terraform Core repository.

Unfortunately due to the fact that new issues and pull requests are being opened constantly, it was not possible for the various provider maintainers to merge all outstanding pull requests before this split, and there is no automatic way to migrate a pull request to a new repository.

As a result, this pull request can sadly no longer be applied as-is, and so I'm going to close it.

If you or someone else has the time and motivation to apply same changes to the aws provider repository and open a new PR there, the maintainers of that provider should be able to review and merge it.

Thanks again for working on this, and sorry it was not able to be merged before the provider repository changes.

[fazy]

@apparentlymart thanks, that's a very nice message. :) I'll make a note to revisit this in the coming weeks, but if anyone beats me to it then I won't mind either.

[ghost]

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.