Skip to content

Feat: Add Broker Node Security Groups #41

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

Closed
wants to merge 11 commits into from

Conversation

nitrocode
Copy link
Member

@nitrocode nitrocode commented Oct 25, 2021

what

  • Add broker node security groups.

why

  • Allows expanding the list of the security groups to associate with the elastic network interfaces to control who can communicate with the cluster.

references

@nitrocode nitrocode requested review from a team as code owners October 25, 2021 20:37
@nitrocode nitrocode requested review from bradj and RothAndrew October 25, 2021 20:37
@nitrocode nitrocode changed the title Broker security groups Add broker node security groups Oct 25, 2021
Copy link

@bridgecrew bridgecrew bot left a comment

Choose a reason for hiding this comment

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

Bridgecrew has found 1 infrastructure configuration error in this PR ⬇️

@korenyoni korenyoni force-pushed the broker_security_groups branch from 7c36501 to 6e464ee Compare October 26, 2021 12:50
@korenyoni
Copy link
Member

/test all

@korenyoni korenyoni changed the title Add broker node security groups Feat: Add Broker Node Security Groups; Use Security Group Module; Restrict Security Group Ingress Based on Enabled MSK Protocols. Oct 26, 2021
@korenyoni
Copy link
Member

/test all

@korenyoni korenyoni force-pushed the broker_security_groups branch from aa90eb9 to fce6147 Compare October 26, 2021 13:02
@korenyoni
Copy link
Member

/test all

@korenyoni korenyoni added the enhancement New feature or request label Oct 26, 2021
@nitrocode
Copy link
Member Author

@korenyoni look at cloudposse/terraform-aws-vpc#100 on new standardized sg 4 inputs and how to deprecate vars

cc: @Nuru

@korenyoni
Copy link
Member

korenyoni commented Oct 26, 2021

@korenyoni look at cloudposse/terraform-aws-vpc#100 on new standardized sg 4 inputs and how to deprecate vars

cc: @Nuru

@nitrocode cloudposse/terraform-aws-elasticache-redis#133 is a better example for this type of module (deployed to a VPC)

@korenyoni
Copy link
Member

Closing in favor of #44

@korenyoni korenyoni closed this Oct 26, 2021
@korenyoni
Copy link
Member

This is superseded by #44 as the standardized SG variable var.associated_security_group_ids removes the needs for a custom var.broker_node_security_groups variable, which was the original change included in this PR by @nitrocode

@korenyoni korenyoni changed the title Feat: Add Broker Node Security Groups; Use Security Group Module; Restrict Security Group Ingress Based on Enabled MSK Protocols. Feat: Add Broker Node Security Groups; Restrict Security Group Ingress Based on Enabled MSK Protocols. Oct 26, 2021
@korenyoni korenyoni changed the title Feat: Add Broker Node Security Groups; Restrict Security Group Ingress Based on Enabled MSK Protocols. Feat: Add Broker Node Security Groups Oct 26, 2021
Comment on lines +16 to +89
# var.client_broker types
plaintext = "PLAINTEXT"
tls_plaintext = "TLS_PLAINTEXT"
tls = "TLS"

# The following ports are not configurable. See: https://docs.aws.amazon.com/msk/latest/developerguide/client-access.html#port-info
protocols = {
plaintext = {
name = "plaintext"
# https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/msk_cluster#bootstrap_brokers
enabled = contains([local.plaintext, local.tls_plaintext], var.client_broker)
port = 9092
}
tls = {
name = "TLS"
# https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/msk_cluster#bootstrap_brokers_tls
enabled = contains([local.tls_plaintext, local.tls], var.client_broker)
port = 9094
}
sasl_scram = {
name = "SASL/SCRAM"
# https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/msk_cluster#bootstrap_brokers_sasl_scram
enabled = var.client_sasl_scram_enabled && contains([local.tls_plaintext, local.tls], var.client_broker)
port = 9096
}
sasl_iam = {
name = "SASL/IAM"
# https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/msk_cluster#bootstrap_brokers_sasl_iam
enabled = var.client_sasl_iam_enabled && contains([local.tls_plaintext, local.tls], var.client_broker)
port = 9098
}
# The following two protocols are always enabled.
# See: https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/msk_cluster#zookeeper_connect_string
# and https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/msk_cluster#zookeeper_connect_string_tls
zookeeper_plaintext = {
name = "Zookeeper plaintext"
enabled = true
port = 2181
}
zookeeper_tls = {
name = "Zookeeper TLS"
enabled = true
port = 2182
}
}
}

resource "aws_security_group_rule" "ingress_cidr_blocks" {
count = local.enabled && length(var.allowed_cidr_blocks) > 0 ? 1 : 0
description = "Allow inbound traffic from CIDR blocks"
type = "ingress"
from_port = 0
to_port = 65535
protocol = "tcp"
cidr_blocks = var.allowed_cidr_blocks
security_group_id = join("", aws_security_group.default.*.id)
}
module "broker_security_group" {
source = "cloudposse/security-group/aws"
version = "0.4.2"

attributes = ["broker"]

security_group_description = "Allow inbound MSK-related traffic from Security Groups and CIDRs. Allow all outbound traffic"
allow_all_egress = true
rule_matrix = [
{
source_security_group_ids = var.security_groups
cidr_blocks = var.allowed_cidr_blocks
rules = [
for protocol_key, protocol in local.protocols : {
key = protocol_key
type = "ingress"
from_port = protocol.port
to_port = protocol.port
protocol = "tcp"
description = "Allow inbound ${protocol.name} traffic"
} if protocol.enabled
]
}
]
vpc_id = var.vpc_id

resource "aws_security_group_rule" "egress" {
count = local.enabled ? 1 : 0
description = "Allow all egress traffic"
type = "egress"
from_port = 0
to_port = 65535
protocol = "tcp"
cidr_blocks = ["0.0.0.0/0"]
security_group_id = join("", aws_security_group.default.*.id)
context = module.this.context
Copy link
Member

Choose a reason for hiding this comment

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

Not part of original PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Restrict security group to tls port 9094 input variable security_groups is not being utilized
3 participants