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

[Bug]: aws_vpc_security_group_ingress_rule doesn't match the API Schema #29893

Open
mattburgess opened this issue Mar 9, 2023 · 2 comments
Open
Labels
bug Addresses a defect in current functionality. service/vpc Issues and PRs that pertain to the vpc service.

Comments

@mattburgess
Copy link
Collaborator

mattburgess commented Mar 9, 2023

Terraform Core Version

1.4.0

AWS Provider Version

4.57.1

Affected Resource(s)

  • aws_vpc_security_group_ingress_rule

Expected Behavior

Trying this resource out for the first time, I was surprised by some of my IDE's ideas on what the required and optional fields were. For example, if I try to create an aws_vpc_security_group_ingress_rule with no attributes, the only thing I get told I must specify is the ip_protocol.

resource "aws_vpc_security_group_ingress_rule" "test" {
}
$ terraform apply
╷
│ Error: Missing required argument
│ 
│   on vpc.tf line 31, in resource "aws_vpc_security_group_ingress_rule" "test":
│   31: resource "aws_vpc_security_group_ingress_rule" "test" {
│ 
│ The argument "ip_protocol" is required, but no definition was found.
╵

Being an inquisitive sort of chap, I wondered what SG that might actually apply the rule to if I blindly applied its suggestion:

resource "aws_vpc_security_group_ingress_rule" "test" {
  ip_protocol = "-1"
}
$ terraform apply
╷
│ Error: Missing Attribute Configuration
│ 
│   with aws_vpc_security_group_ingress_rule.test,
│   on vpc.tf line 31, in resource "aws_vpc_security_group_ingress_rule" "test":
│   31: resource "aws_vpc_security_group_ingress_rule" "test" {
│ 
│ Exactly one of these attributes must be configured: [cidr_ipv4,cidr_ipv6,prefix_list_id,referenced_security_group_id]
╵

Although that doesn't appear to match the API (and I'm completely unfamiliar at the moment with the Framework approach that this resource uses, so I don't know how it's magically worked out that those attributes are now required), at initial glance it would seem to make sense, so let's plough on! Never mind, resourcevalidator looks cool!

resource "aws_vpc_security_group_ingress_rule" "test" {
  ip_protocol = "-1"
  cidr_ipv4   = "172.16.0.2/32"
}
$ terraform apply
data.aws_servicequotas_service.vpc: Reading...
data.aws_servicequotas_service.vpc: Read complete after 1s [id=vpc]
data.aws_servicequotas_service_quota.rules_per_sg: Reading...
data.aws_servicequotas_service_quota.rules_per_sg: Read complete after 0s [id=arn:aws:servicequotas:eu-west-2::vpc/L-0EA8095F]

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  + create

Terraform will perform the following actions:

  # aws_security_group.test will be created
  + resource "aws_security_group" "test" {
      + arn                    = (known after apply)
      + description            = "Test"
      + egress                 = (known after apply)
      + id                     = (known after apply)
      + ingress                = (known after apply)
      + name                   = (known after apply)
      + name_prefix            = (known after apply)
      + owner_id               = (known after apply)
      + revoke_rules_on_delete = false
      + tags_all               = (known after apply)
      + vpc_id                 = (known after apply)
    }

  # aws_vpc.test will be created
  + resource "aws_vpc" "test" {
      + arn                                  = (known after apply)
      + cidr_block                           = "192.168.0.0/16"
      + default_network_acl_id               = (known after apply)
      + default_route_table_id               = (known after apply)
      + default_security_group_id            = (known after apply)
      + dhcp_options_id                      = (known after apply)
      + enable_classiclink                   = (known after apply)
      + enable_classiclink_dns_support       = (known after apply)
      + enable_dns_hostnames                 = (known after apply)
      + enable_dns_support                   = true
      + enable_network_address_usage_metrics = (known after apply)
      + id                                   = (known after apply)
      + instance_tenancy                     = "default"
      + ipv6_association_id                  = (known after apply)
      + ipv6_cidr_block                      = (known after apply)
      + ipv6_cidr_block_network_border_group = (known after apply)
      + main_route_table_id                  = (known after apply)
      + owner_id                             = (known after apply)
      + tags_all                             = (known after apply)
    }

  # aws_vpc_security_group_ingress_rule.test will be created
  + resource "aws_vpc_security_group_ingress_rule" "test" {
      + arn                    = (known after apply)
      + cidr_ipv4              = "172.16.0.2/32"
      + id                     = (known after apply)
      + ip_protocol            = "-1"
      + security_group_rule_id = (known after apply)
      + tags_all               = {}
    }

Plan: 3 to add, 0 to change, 0 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

aws_vpc.test: Creating...
aws_vpc_security_group_ingress_rule.test: Creating...
aws_vpc.test: Creation complete after 1s [id=vpc-0bb5e19f1099eed14]
aws_security_group.test: Creating...
aws_security_group.test: Creation complete after 1s [id=sg-0d50eabaacdda46a2]
╷
│ Error: creating VPC Security Group Rule
│ 
│   with aws_vpc_security_group_ingress_rule.test,
│   on vpc.tf line 31, in resource "aws_vpc_security_group_ingress_rule" "test":
│   31: resource "aws_vpc_security_group_ingress_rule" "test" {
│ 
│ MissingParameter: The request must contain the parameter groupName or groupId
│ 	status code: 400, request id: 3aa4c32a-20a1-4736-b929-dd1a8ef2c394
╵

Oh dear 😄 Now that error is something that AuthorizeSecurityGroupIngress does actually stipulate. As we don't have security_group_name in our schema, it would make sense to mark security_group_id as Required: true.

I'm raising this rather than just a trivial PR because I'm hoping to have more time to dig in and see if there are further issues.

Actual Behavior

Terraform failed to create the SG rule

Relevant Error/Panic Output Snippet

╷
│ Error: creating VPC Security Group Rule
│ 
│   with aws_vpc_security_group_ingress_rule.test,
│   on vpc.tf line 31, in resource "aws_vpc_security_group_ingress_rule" "test":
│   31: resource "aws_vpc_security_group_ingress_rule" "test" {
│ 
│ MissingParameter: The request must contain the parameter groupName or groupId
│ 	status code: 400, request id: 3aa4c32a-20a1-4736-b929-dd1a8ef2c394
╵

Terraform Configuration Files

resource "aws_vpc_security_group_ingress_rule" "test" {
  ip_protocol = "-1"
  cidr_ipv4   = "172.16.0.2/32"
}

Steps to Reproduce

  1. terraform apply

Debug Output

No response

Panic Output

No response

Important Factoids

No response

References

No response

Would you like to implement a fix?

Yes

@mattburgess mattburgess added bug Addresses a defect in current functionality. needs-triage Waiting for first response or review from a maintainer. labels Mar 9, 2023
@github-actions
Copy link

github-actions bot commented Mar 9, 2023

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.

@github-actions github-actions bot added the service/vpc Issues and PRs that pertain to the vpc service. label Mar 9, 2023
@mattburgess
Copy link
Collaborator Author

OK, next issue is this:

resource "aws_vpc_security_group_ingress_rule" "test" {
  security_group_id = aws_security_group.test.id
  ip_protocol       = "tcp"
  cidr_ipv4         = "172.16.0.2/32"
}

causes:

╷
│ Error: updating VPC Security Group Rule (sgr-0b4a70de184cd8f0a)
│ 
│   with aws_vpc_security_group_ingress_rule.test,
│   on vpc.tf line 31, in resource "aws_vpc_security_group_ingress_rule" "test":
│   31: resource "aws_vpc_security_group_ingress_rule" "test" {
│ 
│ InvalidParameterValue: Invalid value for portRange. Must specify both from and to ports with TCP/UDP.
│ 	status code: 400, request id: 73bc9ba8-1c01-43a8-adbe-917213a8c7f2
╵

That looks like I'm after something like hashicorp/terraform-plugin-framework-validators/issues/113 so I think we'll just have to park plan-time validation of that one for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Addresses a defect in current functionality. service/vpc Issues and PRs that pertain to the vpc service.
Projects
None yet
Development

No branches or pull requests

2 participants