[Enhancement]: Terraform Resources Call AWS EC2 Security Group APIs Inconsistently

[aishwarya-88]

Description

Introduction

Terraform provides two resources that allow a user to specify a security group rule--aws_security_group and aws_security_group_rule resources.

From the perspective of a customer, for both resources, Terraform states that the from_port and to_port fields are required parameters[1][2]. Terraform also uses values of 0 for ports when protocol is -1 or all in the documentation as examples[3]. For this reason, users always pass in some value for port parameters when making requests via Terraform and are more likely to pass in 0 as the value. In contrast, AWS EC2 Security Group APIs specify the port fields to be optional[4].

On the other hand, behind the scenes, both Terraform resources make (Authorize|Revoke)SecurityGroup(Ingress|Egress) API calls to AWS EC2 in order to authorize or revoke a rule for a security group. However the parameters used by Terraform to make AWS EC2 API calls for each resource is different. This results in inconsistent call patterns from Terraform to AWS EC2.

Current Behavior

Current behavior of aws_security_group_rule resource:

Today, if a Terraform aws_security_group_rule resource is specified with any combination of protocol=all or protocol=-1 and any value for from_port and to_port:

• The call made by a user always succeeds
• Terraform makes a request to AWS EC2 (Authorize|Revoke)SecurityGroup(Ingress|Egress) API without specifying any port parameters at all in the request.

Special logic for this behavior:

terraform-provider-aws/internal/service/ec2/vpc_security_group_rule.go

Lines 65 to 72 in 4a7da7e

	// Support existing configurations that have non-zero from_port and to_port defined with all protocols
	DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool {
	protocol := ProtocolForValue(d.Get("protocol").(string))
	if protocol == "-1" && old == "0" {
	return true
	}
	return false
	},

Current behavior of aws_security_group resource:

Based on different combinations of input parameters passed in, the Terraform aws_security_group resource specified with protocol=-1 or protocol=all and with from_port and to_port values passed in results in:

• The calls made by a user see inconsistent responses. If a Terraform aws_security_group resource is specified with the parameter combination of protocol=-1 and non-zero from_port and to_port values, then the Terraform request fails. The error message states that the port values “must be 0” but instead, if a value must be passed into the field then it should be -1 for the ports (indicating all ports since specifying port 0 means that the intention is to allow traffic only on port 0). For all other combinations of input, the requests succeed (see this outlined in the table below).
• The AWS EC2 (Authorize|Revoke)SecurityGroup(Ingress|Egress) API are called with the port parameters passed in the request.

No special logic for this behavior:

terraform-provider-aws/internal/service/ec2/vpc_security_group.go

Lines 137 to 140 in 4a7da7e

	"from_port": {
	Type: schema.TypeInt,
	Required: true,
	},

Proposed Solution

There are two changes we want to propose here:

• From the point of view of Terraform calling AWS EC2 APIs: We propose that the Terraform aws_security_group resource mimic the behavior of the aws_security_group_rule resource when calling AWS EC2 APIs. This means that in cases where protocol=-1 or protocol=all is passed in with any from_port and to_port parameter values, the aws_security_group resource must make AWS EC2 (Authorize|Revoke)SecurityGroup(Ingress|Egress) API calls without specifying the port parameters in the request.
• From the point of view of Terraform’s customer:
  • We propose changing the existing error message returned when a Terraform aws_security_group resource is specified with the parameter combination of protocol=-1 and non-zero from_port and to_port values. Instead of the error message stating port values should be 0, the error message should state port values should be -1.Therefore, the error message should be:

  Error: error updating Security Group (sg-066ef4f7b9897042b): from_port (1800) and to_port (2000) must both be -1 to use the 'ALL' "-1" protocol!
  

  • And in line with that, all documentation must be updated to reflect ports as -1 if protocol is -1 or all.

The solutions outlined here are chosen to be the least backwards breaking changes and are in line with AWS public documentation[5] which states any port values will be ignored when protocol is specified as all or -1.
: https://github.com/hashicorp/terraform-provider-aws/blame/main/internal/service/ec2/vpc_security_group_rule.go#L65-L72

Affected Resource(s) and/or Data Source(s)

• aws_security_group

Potential Terraform Configuration

There are no changes required to how a user will make Terraform requests. Instead one of the proposed solutions changes the Error message returned to users.

References

No response

Would you like to implement a fix?

No

[github-actions]

Community Note

Voting for Prioritization

• Please vote on this issue by adding a 👍 reaction to the original post to help the community and maintainers prioritize this request.
• Please see our prioritization guide for information on how we prioritize.
• Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.

Volunteering to Work on This Issue

• If you are interested in working on this issue, please leave a comment.
• If this would be your first contribution, please review the contribution guide.

[ewbankkit]

@aishwarya-88 Thank you for the very thorough write-up of the inconsistency 👏.

I have opened #27642 to do as you suggest in your first bullet - Not pass any FromPort or ToPort value to the AWS API if protocol is -1 or "all".
Unfortunately, due to the way that Terraform providers handle unconfigured/missing nested attribute values (we can't distinguish between null and the zero value for the type), and to maintain backwards compatibility, we must keep the from_port and to_port value at 0 when protocol is -1 (your second bullet).

We are considering adding a "new" security group rule resource to address #20104 and will ensure that this issue is take into account during implementation.

[github-actions]

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.