Allow use of protocol numbers for ah and esp

[mootpt]

It appears the aws sdk does not accept the protocol name for esp or ah. This fix allows the user to simply specify the protocol number in their security group.

This fix covers: #6888

[catsby]

LGTM

TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSSecurityGroup_vpcProtoNumIngress -timeout 120m
=== RUN   TestAccAWSSecurityGroup_vpcProtoNumIngress
--- PASS: TestAccAWSSecurityGroup_vpcProtoNumIngress (20.35s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    20.368s
Test:

TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSNetworkAcl_ -timeout 120m
=== RUN   TestAccAWSNetworkAcl_importBasic
--- PASS: TestAccAWSNetworkAcl_importBasic (20.20s)
=== RUN   TestAccAWSNetworkAcl_EgressAndIngressRules
--- PASS: TestAccAWSNetworkAcl_EgressAndIngressRules (18.86s)
=== RUN   TestAccAWSNetworkAcl_OnlyIngressRules_basic
--- PASS: TestAccAWSNetworkAcl_OnlyIngressRules_basic (23.93s)
=== RUN   TestAccAWSNetworkAcl_OnlyIngressRules_update
--- PASS: TestAccAWSNetworkAcl_OnlyIngressRules_update (37.39s)
=== RUN   TestAccAWSNetworkAcl_OnlyEgressRules
--- PASS: TestAccAWSNetworkAcl_OnlyEgressRules (20.27s)
=== RUN   TestAccAWSNetworkAcl_SubnetChange
--- PASS: TestAccAWSNetworkAcl_SubnetChange (36.87s)
=== RUN   TestAccAWSNetworkAcl_Subnets
--- PASS: TestAccAWSNetworkAcl_Subnets (42.03s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    199.580s
Test:

[jrnt30]

The same thing will apply I believe to my comment here: #6915 (comment)

The network_acl_entry's call to AWS-SDK does support these two port mappings (the sdk clearly is not consistent). As a heads up, I believe this will then be a breaking change for anyone using them.

[catsby]

Thanks for the ping @jrnt30 , I'll take a look at that comment/PR thread.
Re: breaking change, it's my understanding that if a user is using esp that it isn't working at all (e.g. the rule fails to create at all and errors), as opposed to silently not working or something. So, I don't feel it's breaking anything if it was broken and never worked before 😄 Am I wrong there? Is there some way / scenario that users are using ah or esp successfully, that I'm missing?

[jrnt30]

That is correct in the security_group_rule resource which this fixes. I need to take a look at the code again, however before that list that was modified in the PR was also used by a few other resources, which under the covers the AWS SDK did support the logical names for.

I can take a look tonight if it's not cleared up, just really wanted to give a public service announcement :)

[catsby]

@jrnt30 hey, sorry, I think I misread your first comment, so you are likely correct that any aws_network_acl user will have a bad time now. Let me do some thinking and digging to see what I can come up with.

[catsby]

@jrnt30 this should be sorted in #8999

[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.