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

[Enhancement]: Undetectable rule drift with security groups referencing each other #32743

Open
toadjaune opened this issue Jul 28, 2023 · 1 comment
Labels
enhancement Requests to existing resources that expand the functionality or scope. service/vpc Issues and PRs that pertain to the vpc service.

Comments

@toadjaune
Copy link

toadjaune commented Jul 28, 2023

Description

Let's say you want to create two security groups referencing each other, for example :

resource "aws_default_vpc" "default" {}

resource "aws_security_group" "source" {
  name   = "source"
  vpc_id = aws_default_vpc.default.id

  ingress = []
  egress {
    description     = "postgresql"
    from_port       = 5432
    to_port         = 5432
    protocol        = "tcp"
    security_groups = [aws_security_group.dest.id]
  }
}

resource "aws_security_group" "dest" {
  name   = "dest"
  vpc_id = aws_default_vpc.default.id

  ingress {
    description     = "postgresql"
    from_port       = 5432
    to_port         = 5432
    protocol        = "tcp"
    security_groups = [aws_security_group.source.id]
  }
  egress = []
}

As you might expect, this results in a dependency loop error :

Error: Cycle: aws_security_group.source, aws_security_group.dest

Since you can't reference security groups by anything else than their ids, we can't work around this by referring to them by name, or any other already known value.

Here come aws_security_group_rule, aws_vpc_security_group_egress_rule, aws_vpc_security_group_ingress_rule. We can then build something like that :

resource "aws_default_vpc" "default" {}

resource "aws_security_group" "source" {
  name   = "source"
  vpc_id = aws_default_vpc.default.id

  ingress = []
  egress {
    description     = "postgresql"
    from_port       = 5432
    to_port         = 5432
    protocol        = "tcp"
    security_groups = [aws_security_group.dest.id]
  }
}

resource "aws_security_group" "dest" {
  name   = "dest"
  vpc_id = aws_default_vpc.default.id

  egress = []
}

resource "aws_vpc_security_group_ingress_rule" "dest" {
  security_group_id            = aws_security_group.dest.id
  description                  = "postgresql"
  from_port                    = 5432
  to_port                      = 5432
  ip_protocol                  = "tcp"
  referenced_security_group_id = aws_security_group.source.id
}

This breaks the dependency loop, and works as expected.

The problem is that if an ingress rule is manually added to dest, terraform is unable to detect (and remove) it, which is not the case for egress rules on dest, and all rules on source.

Fix ideas

I can only think of one clean strategy to solve this problem : Adding a new type of resource, that is authoritative on the rules of a given security group.

We could for example add aws_vpc_security_group_egress_rules and aws_vpc_security_group_ingress_rules resources, that we could configure similarly to inline rules in aws_security_group. For example, something like :

resource "aws_default_vpc" "default" {}

resource "aws_security_group" "source" {
  name   = "source"
  vpc_id = aws_default_vpc.default.id

  ingress = []
  egress {
    description     = "postgresql"
    from_port       = 5432
    to_port         = 5432
    protocol        = "tcp"
    security_groups = [aws_security_group.dest.id]
  }
}

resource "aws_security_group" "dest" {
  name   = "dest"
  vpc_id = aws_default_vpc.default.id

  egress = []
}

resource "aws_vpc_security_group_ingress_rules" "dest" {
  security_group_id            = aws_security_group.dest.id
  rule {
    description                  = "postgresql"
    from_port                    = 5432
    to_port                      = 5432
    ip_protocol                  = "tcp"
    referenced_security_group_id = aws_security_group.source.id
  }
}

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

  • aws_security_group
  • aws_security_group_rule
  • aws_vpc_security_group_egress_rule
  • aws_vpc_security_group_ingress_rule

Would you like to implement a fix?

No

@toadjaune toadjaune added the enhancement Requests to existing resources that expand the functionality or scope. label Jul 28, 2023
@github-actions
Copy link

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 service/vpc Issues and PRs that pertain to the vpc service. needs-triage Waiting for first response or review from a maintainer. labels Jul 28, 2023
@justinretzolk justinretzolk removed the needs-triage Waiting for first response or review from a maintainer. label Jul 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Requests to existing resources that expand the functionality or scope. service/vpc Issues and PRs that pertain to the vpc service.
Projects
None yet
Development

No branches or pull requests

2 participants