hashicorp / terraform-provider-aws

The AWS Provider enables Terraform to manage AWS resources.
https://registry.terraform.io/providers/hashicorp/aws
Mozilla Public License 2.0
9.77k stars 9.12k forks source link

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

Open toadjaune opened 1 year ago

toadjaune commented 1 year ago

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)

Would you like to implement a fix?

No

github-actions[bot] commented 1 year ago

Community Note

Voting for Prioritization

Volunteering to Work on This Issue