fix: Set SG rules for HTTPS only (#96)

* fix: Set SG rules for HTTPS only * Update main.tf remove lifecycles from SGs * restored sane versions * restore sane versions * fmt * typo * added default for ipv6 * typo * typofix
wandb · Jun 27, 2023 · d842560 · d842560
1 parent 1cd2a6a
commit d842560
Show file tree

Hide file tree

Showing 8 changed files with 76 additions and 140 deletions.
diff --git a/examples/public-dns-external/variables.tf b/examples/public-dns-external/variables.tf
@@ -80,4 +80,11 @@ variable "allowed_inbound_cidr" {
   default  = ["0.0.0.0/0"]
   nullable = false
   type     = list(string)
-}
+}
+
+
+variable "allowed_inbound_ipv6_cidr" {
+  default  = ["::/0"]
+  nullable = false
+  type     = list(string)
+}
diff --git a/main.tf b/main.tf
@@ -140,7 +140,7 @@ module "app_eks" {
   network_id              = local.network_id
   network_private_subnets = local.network_private_subnets
 
-  lb_inbound_security_group_ids = module.app_lb.inbound_security_group_ids
+  lb_security_group_inbound_id = module.app_lb.security_group_inbound_id
   database_security_group_id    = module.database.security_group_id
 
   create_elasticache_security_group = var.create_elasticache

diff --git a/modules/app_eks/main.tf b/modules/app_eks/main.tf
@@ -205,11 +205,10 @@ resource "aws_security_group" "primary_workers" {
 }
 
 resource "aws_security_group_rule" "lb" {
-  count                    = length(var.lb_inbound_security_group_ids)
   description              = "Allow container NodePort service to receive load balancer traffic."
   protocol                 = "tcp"
   security_group_id        = aws_security_group.primary_workers.id
-  source_security_group_id = var.lb_inbound_security_group_ids[count.index]
+  source_security_group_id = var.lb_security_group_inbound_id
   from_port                = var.service_port
   to_port                  = var.service_port
   type                     = "ingress"

diff --git a/modules/app_eks/variables.tf b/modules/app_eks/variables.tf
@@ -1,92 +1,90 @@
-variable "bucket_arn" {
-  type = string
+variable "namespace" {
+  type        = string
+  description = "(Required) The name prefix for all resources created."
 }
 
-
-variable "bucket_kms_key_arn" {
-  description = "The Amazon Resource Name of the KMS key with which S3 storage bucket objects will be encrypted."
+variable "network_id" {
+  description = "(Required) The identity of the VPC in which the security group attached to the MySQL Aurora instances will be deployed."
   type        = string
 }
 
-
-variable "bucket_sqs_queue_arn" {
-  type    = string
-  default = null
+variable "network_private_subnets" {
+  description = "(Required) A list of the identities of the private subnetworks in which the MySQL Aurora instances will be deployed."
+  type        = list(string)
 }
 
+variable "cluster_version" {
+  description = "Indicates AWS EKS cluster version"
+  type        = string
+  default     = "1.21"
+}
 
 variable "cluster_endpoint_public_access" {
   type        = bool
   description = "(Optional) Indicates whether or not the Amazon EKS public API server endpoint is enabled."
   default     = true
 }
 
-
 variable "cluster_endpoint_public_access_cidrs" {
   description = "List of CIDR blocks which can access the Amazon EKS public API server endpoint."
   type        = list(string)
   default     = []
 }
 
-
-variable "cluster_version" {
-  description = "Indicates AWS EKS cluster version"
-  type        = string
-  default     = "1.21"
-}
-
-
-variable "create_elasticache_security_group" {
-  type    = bool
-  default = false
-}
-
-
-variable "database_security_group_id" {
+variable "lb_security_group_inbound_id" {
   type = string
 }
 
-
-variable "eks_policy_arns" {
-  description = "Additional IAM policy to apply to the EKS cluster"
-  type        = set(string)
-  default     = []
+variable "bucket_arn" {
+  type = string
 }
 
-
-variable "elasticache_security_group_id" {
+variable "bucket_sqs_queue_arn" {
   type    = string
   default = null
 }
 
+variable "bucket_kms_key_arn" {
+  description = "The Amazon Resource Name of the KMS key with which S3 storage bucket objects will be encrypted."
+  type        = string
+}
+
+variable "kms_key_arn" {
+  description = "(Required) The Amazon Resource Name of the KMS key with which EKS secrets will be encrypted."
+  type        = string
+}
 
 variable "instance_types" {
   description = "EC2 Instance type for primary node group."
   type        = list(string)
   default     = ["m4.large"]
 }
 
-
-variable "kms_key_arn" {
-  description = "(Required) The Amazon Resource Name of the KMS key with which EKS secrets will be encrypted."
-  type        = string
+variable "database_security_group_id" {
+  type = string
 }
 
+variable "elasticache_security_group_id" {
+  type    = string
+  default = null
+}
 
-variable "lb_inbound_security_group_ids" {
-  description = "IDs of security groups to be associated with the loadbalancer."
-  nullable    = false
-  type        = list(string)
+variable "create_elasticache_security_group" {
+  type    = bool
+  default = false
 }
 
+variable "service_port" {
+  type    = number
+  default = 32543
+}
 
 variable "map_accounts" {
   description = "Additional AWS account numbers to add to the aws-auth configmap. See examples/basic/variables.tf for example format."
   type        = list(string)
   default     = []
 }
 
-
 variable "map_roles" {
   description = "Additional IAM roles to add to the aws-auth configmap. See examples/basic/variables.tf for example format."
   type = list(object({
@@ -97,7 +95,6 @@ variable "map_roles" {
   default = []
 }
 
-
 variable "map_users" {
   description = "Additional IAM users to add to the aws-auth configmap. See examples/basic/variables.tf for example format."
   type = list(object({
@@ -108,28 +105,8 @@ variable "map_users" {
   default = []
 }
 
-
-variable "namespace" {
-  type        = string
-  description = "(Required) The name prefix for all resources created."
-}
-
-
-variable "network_id" {
-  description = "(Required) The identity of the VPC in which the security group attached to the MySQL Aurora instances will be deployed."
-  type        = string
-}
-
-
-variable "network_private_subnets" {
-  description = "(Required) A list of the identities of the private subnetworks in which the MySQL Aurora instances will be deployed."
-  type        = list(string)
-}
-
-
-variable "service_port" {
-  type    = number
-  default = 32543
+variable "eks_policy_arns" {
+  description = "Additional IAM policy to apply to the EKS cluster"
+  type        = set(string)
+  default     = []
 }
-
-
diff --git a/modules/app_lb/main.tf b/modules/app_lb/main.tf
@@ -3,41 +3,10 @@ locals {
   https_port = 443
 }
 
-
-////////////////////////////////////////////////////////////////////////////////////////////
-// the following security group definitions are created to handle a situation where 
-// we need to assign a large number of rules to a SG. Dependent on AWS quotas.
-// -> george.scott@wandb.com :: 2023-06-20
-////////////////////////////////////////////////////////////////////////////////////////////
-resource "aws_security_group" "inbound_http" {
-  name                   = "${var.namespace}-alb-inbound_http"
-  description            = "Allow http traffic to wandb"
-  revoke_rules_on_delete = true
-  vpc_id                 = var.network_id
-
-  ingress {
-    from_port        = local.http_port
-    to_port          = local.http_port
-    protocol         = "tcp"
-    description      = "Allow HTTP (port ${local.http_port}) traffic inbound to W&B LB"
-    cidr_blocks      = var.allowed_inbound_cidr
-    ipv6_cidr_blocks = var.allowed_inbound_ipv6_cidr
-  }
-
-  lifecycle {
-    create_before_destroy = true
-  }
-
-  timeouts {
-    delete = "3m"
-  }
-}
-
-resource "aws_security_group" "inbound_https" {
-  name                   = "${var.namespace}-alb-inbound_https"
-  description            = "Allow https traffic to wandb"
-  revoke_rules_on_delete = true
-  vpc_id                 = var.network_id
+resource "aws_security_group" "inbound" {
+  name        = "${var.namespace}-alb-inbound"
+  description = "Allow http(s) traffic to wandb"
+  vpc_id      = var.network_id
 
   ingress {
     from_port        = local.https_port
@@ -48,17 +17,16 @@ resource "aws_security_group" "inbound_https" {
     ipv6_cidr_blocks = var.allowed_inbound_ipv6_cidr
   }
 
-  lifecycle {
-    create_before_destroy = true
-  }
-
-  timeouts {
-    delete = "3m"
+  ingress {
+    from_port        = local.http_port
+    to_port          = local.http_port
+    protocol         = "tcp"
+    description      = "Allow HTTP (port ${local.http_port}) traffic inbound to W&B LB"
+    cidr_blocks      = ["0.0.0.0/0"]
+    ipv6_cidr_blocks = ["::/0"]
   }
 }
 
-
-
 resource "aws_security_group" "outbound" {
   name        = "${var.namespace}-alb-outbound"
   vpc_id      = var.network_id
@@ -77,7 +45,7 @@ resource "aws_lb" "alb" {
   name               = "${var.namespace}-alb"
   internal           = (var.load_balancing_scheme == "PRIVATE")
   load_balancer_type = "application"
-  security_groups    = [aws_security_group.inbound_https.id, aws_security_group.inbound_http.id, aws_security_group.outbound.id]
+  security_groups    = [aws_security_group.inbound.id, aws_security_group.outbound.id]
   subnets            = var.load_balancing_scheme == "PRIVATE" ? var.network_private_subnets : var.network_public_subnets
 }
 

diff --git a/modules/app_lb/outputs.tf b/modules/app_lb/outputs.tf
@@ -2,28 +2,14 @@ output "dns_name" {
   value = aws_lb.alb.dns_name
 }
 
-
-output "inbound_security_group_ids" {
-  value = tolist([aws_security_group.inbound_http.id, aws_security_group.inbound_https.id])
+output "security_group_inbound_id" {
+  value = aws_security_group.inbound.id
 }
 
-
 output "lb_arn" {
   value = aws_lb.alb.arn
 }
 
-
-output "security_group_inbound_http_id" {
-  value = aws_security_group.inbound_http.id
-}
-
-
-output "security_group_inbound_https_id" {
-  value = aws_security_group.inbound_https.id
-}
-
-
 output "tg_app_arn" {
   value = aws_lb_target_group.app.arn
-}
-
+}
diff --git a/modules/app_lb/variables.tf b/modules/app_lb/variables.tf
@@ -43,15 +43,15 @@ variable "load_balancing_scheme" {
 }
 
 variable "allowed_inbound_cidr" {
+  description = "CIDRs allowed to access wandb-server."
   type        = list(string)
-  default     = []
-  description = "(Optional) Allow HTTP(S) traffic to W&B. Defaults to no connections."
+  nullable    = false
 }
 
 variable "allowed_inbound_ipv6_cidr" {
+  description = "CIDRs allowed to access wandb-server."
   type        = list(string)
-  default     = []
-  description = "(Optional) Allow HTTP(S) traffic to W&B. Defaults to no connections."
+  nullable    = false
 }
 
 variable "network_id" {

diff --git a/variables.tf b/variables.tf
@@ -112,16 +112,15 @@ variable "acm_certificate_arn" {
 }
 
 variable "allowed_inbound_cidr" {
-  type        = list(string)
-  default     = ["0.0.0.0/0"]
+  description = "CIDRs allowed to access wandb-server."
   nullable    = false
-  description = "Allow HTTP(S) traffic to W&B. Defaults to no connections."
+  type        = list(string)
 }
 
 variable "allowed_inbound_ipv6_cidr" {
+  description = "CIDRs allowed to access wandb-server."
+  nullable    = false
   type        = list(string)
-  default     = []
-  description = "Allow HTTP(S) traffic to W&B. Defaults to no connections."
 }