brontolinux
commented
1 year ago I confirm we have a similar problem: we needed custom rules that reference prefix lists only (no CIDR blocks, no self, no security group ID), and we have found no way to do that natively in a module. We have a workaround, but it's not optimal. I will document the workaround here in case it's useful for others.
All the security groups that were originally CIDR-range based are defined in a local map, e.g.:
# security group sets
locals {
range_based_sgs = {
common-lb-internal = {
description = "Allow HTTP(S) access to the load balancer from all internal networks"
ingress_prefix_list_ids = local.trusted_private_ips_prefix_list_ids
ingress_rules = local.proto_http_https
egress_prefix_list_ids = local.trusted_private_ips_prefix_list_ids
egress_rules = ["all-all"]
},
...Where we need custom rules, we added a key in the map that defines a security group's rules, called custom_prefix_list_rules, e.g.:
...
common-windows-app = {
...
custom_prefix_list_rules = [
{
type = "ingress"
from_port = "5985"
to_port = "5986"
protocol = "tcp"
description = "WinRM"
prefix_list_ids = local.trusted_private_ips_prefix_list_ids
},and then we add those rules separately with normal aws_security_group_rule resources:
locals {
# We need to process the information in local.range_based_sgs to extract custom_prefix_list_rules where present,
# and then process them as aws_security_group_rule resources.
# This first loop extracts custom_prefix_list_rules from security groups defined in local.range_based_sgs,
# where present. The result is a map that matches a security group name with its custom rules.
per_group_custom_prefix_list_rules = {
for sg_name, rules in local.range_based_sgs : sg_name => rules.custom_prefix_list_rules
if length(lookup(rules, "custom_prefix_list_rules", [])) > 0
}
# This second, nested for loop iterates over the keys of the map local.per_group_custom_prefix_list_rules, calculated
# above; the keys are security group names. It then iterates over the rules associated to each security group, and
# adds a key sg_name to each of them. The sg_name value is needed in the for_each loop below to associate a
# unique key to each rule.
custom_prefix_list_rules = flatten([
for sg_name in keys(local.per_group_custom_prefix_list_rules) : [
for rule in local.per_group_custom_prefix_list_rules[sg_name] :
merge(rule, { sg_name = sg_name })
]
])
}
resource "aws_security_group_rule" "custom_prefix_list_rules" {
for_each = { for rule in local.custom_prefix_list_rules : "${rule.sg_name}/${rule.from_port}-${rule.to_port}/${rule.protocol}" => rule }
from_port = each.value.from_port
protocol = each.value.protocol
security_group_id = module.range_based_sgs[each.value.sg_name].security_group_id
to_port = each.value.to_port
type = each.value.type
prefix_list_ids = each.value.prefix_list_ids
description = lookup(each.value, "description", "The author of this rule was too lazy to provide a decent description. SHAME!")
}The reason why we have defined these custom rules while ingress rules for winrm-http-tcp, winrm-https-tcp is: security group rules quotas. The security group where we implemented these custom rules specifies winrm, rdp, and a number of consul-related rules. Rules are created for each of these rules and for each prefix list specified, which made the number of rules explode and exceed the quota. We defined custom rules to compact winrm and consul related rules, which has brought us from >100 rules to ~60 -- still many, but manageable.
I hope it helps!
-- bronto
antonbabenko
github-actions[bot]
Description
Please provide a clear and concise description of the issue you are encountering, and a reproduction of your configuration (see the
examples/*directory for references that you can copy+paste and tailor to match your configs if you are unable to copy your exact configuration). The reproduction MUST be executable by runningterraform init && terraform applywithout any further changes.If your request is for a new feature, please use the
Feature requesttemplate.⚠️ Note
Before you submit an issue, please perform the following first:
.terraformdirectory (! ONLY if state is stored remotely, which hopefully you are following that best practice!):rm -rf .terraform/terraform initVersions
Module version [Required]: 4.16.2 (latest)
Terraform version: Terraform v1.3.6 on darwin_arm64
Provider version(s):
provider registry.terraform.io/hashicorp/aws v4.45.0
provider registry.terraform.io/hashicorp/local v2.2.3
provider registry.terraform.io/hashicorp/time v0.9.1
provider registry.terraform.io/hashicorp/tls v4.0.4
Reproduction Code [Required]
Steps to reproduce the behavior:
No. Yes. Attempted to use module to create security group with custom rules and prefix lists. ## Expected behavior After using this module extensively in other contexts, I have started migrating my CIDR blocks into prefix lists. I first updated my named rules, which worked as-expected. When I got to my custom rules -- previously specified with CIDR blocks -- I expected the `ingress_with_cidr_blocks` variable to work (without specifying a CIDR block) after exchanging "ingress_cidr_blocks" for "ingress_prefix_list_ids". ## Actual behavior The Terraform interpolation for the "cidr_blocks" variable appears to return a list with an empty string instead of an empty list, which results in an error in the resource. ### Terminal Output Screenshot(s) ``` │ Error: "" is not a valid CIDR block: invalid CIDR address: │ │ with module.sg_with_prefix_lists.aws_security_group_rule.ingress_with_cidr_blocks[0], │ on .terraform/modules/sg_with_prefix_lists/main.tf line 197, in resource "aws_security_group_rule" "ingress_with_cidr_blocks": │ 197: cidr_blocks = split( │ 198: ",", │ 199: lookup( │ 200: var.ingress_with_cidr_blocks[count.index], │ 201: "cidr_blocks", │ 202: join(",", var.ingress_cidr_blocks), │ 203: ), │ 204: ) │ ``` ## Additional context FYI: I'm wrapping Terraform with Terragrunt.