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

fix: Remove prefix_list_ids attribute from *_with_self resouces #324

Conversation

SSW-SCIENTIFIC
Copy link

Description

aws_security_group_rule with both self = true and non-empty prefix_list_ids generates multiple (# of self + # of prefix_list_ids) rules for self and each prefix_list_ids, for example,

resource "aws_security_group_rule" "ingress_with_self" {
  security_group_id = local.this_sg_id
  type              = "ingress"

  self            = true
  prefix_list_ids = ["id1", "id2"]
  description     = "sample"

  from_port = -1
  to_port   = -1
  protocol  = "-1"
}

then we get the rules not only allow all-all from self SG, but also allow all-all from prefix-list id1 and allow all-all from prefix-list id2. I think this is unexpected result, ingress_with_self itself should only add rule to allow self SG, so remove prefix_list_ids attribute from *_with_self resouces.

Motivation and Context

Described as above.

Breaking Changes

This change avoid creating SG rules allow all-all from each prefix-list, this is breaking change.

How Has This Been Tested?

  • I have updated at least one of the examples/* to demonstrate and validate my change(s)
  • I have tested and validated these changes using one or more of the provided examples/* projects
  • I have executed pre-commit run -a on my pull request

`aws_security_group_rule` with both `self = true` and non-empty `prefix_list_ids` generates multiple (# of self + # of prefix_list_ids) rules for self and each prefix_list_ids, for example,

```terraform
resource "aws_security_group_rule" "ingress_with_self" {
  security_group_id = local.this_sg_id
  type              = "ingress"

  self            = true
  prefix_list_ids = ["id1", "id2"]
  description     = "sample"

  from_port = -1
  to_port   = -1
  protocol  = "-1"
}
```

then we get the rules **not only** allow all-all from self SG, **but also**  allow all-all from prefix-list `id1` and allow all-all from prefix-list `id2`.
I think this is unexpected result, `ingress_with_self` itself should only add rule to allow self SG, so remove `prefix_list_ids` attribute from `*_with_self` resouces.
@SSW-SCIENTIFIC SSW-SCIENTIFIC changed the title Remove prefix_list_ids attribute from *_with_self resouces fix: Remove prefix_list_ids attribute from *_with_self resouces Jul 4, 2024
@titouan-joseph
Copy link
Contributor

titouan-joseph commented Jul 22, 2024

Same thing with *_with_cidr_blocks

Edit : Open #325 for this

Copy link

This PR has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this PR will be closed in 10 days

@github-actions github-actions bot added the stale label Aug 22, 2024
@titouan-joseph
Copy link
Contributor

@antonbabenko @bryantbiggs
This PR is similar to #325, please have a look on both PR

We are still blocking for production, we are waiting for this update

@github-actions github-actions bot removed the stale label Aug 23, 2024
Copy link

This PR has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this PR will be closed in 10 days

@github-actions github-actions bot added the stale label Sep 22, 2024
Copy link

github-actions bot commented Oct 3, 2024

This PR was automatically closed because of stale in 10 days

@github-actions github-actions bot closed this Oct 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants