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

Switch to list of ip cidrs to block #1613

Merged
merged 10 commits into from
Mar 15, 2024
Merged

Switch to list of ip cidrs to block #1613

merged 10 commits into from
Mar 15, 2024

Conversation

cweibel
Copy link
Contributor

@cweibel cweibel commented Mar 15, 2024

Changes proposed in this pull request:

  • Switch for a single cidr range to a list of cidr ranges which will dynamically be created starting at rule 20
  • This solution has a maximum of 80 cidr ranges without intervention
  • Tested manually against dev
  • For issue Expand netblock to support an array of values #1612

security considerations

None, end result is the exact same acl rules just future proofing if we need to expand the list of ip cidrs

@cweibel cweibel requested a review from a team March 15, 2024 14:02
variable "cidr_blocks" {
type = list(string)
default = ["192.168.0.0/32", "192.168.0.1/32"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we want to set any default values. Because if we happened to not pass the real values through in CI automatically, then traffic from these default CIDRs would get blocked

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Valid point, I had these for testing but removed with a new commit

markdboyd
markdboyd previously approved these changes Mar 15, 2024
markdboyd
markdboyd previously approved these changes Mar 15, 2024
@cweibel cweibel merged commit 9b2956a into main Mar 15, 2024
2 checks passed
@cweibel cweibel deleted the cidr_blocks branch March 15, 2024 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants