Feature/readwrite ingress (#203)

* read-write metastore ingress * redundant ingress * fix * restrict apiary DB access to vpc * update changelog * Update VARIABLES.md Co-authored-by: Scott Barnhart <[email protected]> * Update variables.tf Co-authored-by: Scott Barnhart <[email protected]> Co-authored-by: Raj Poluri <[email protected]> Co-authored-by: Scott Barnhart <[email protected]>
ExpediaGroup · Oct 22, 2021 · 9512158 · 9512158
1 parent 3627828
commit 9512158
Show file tree

Hide file tree

Showing 9 changed files with 43 additions and 29 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,10 @@ All notable changes to this project will be documented in this file.
 
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
+## [6.10.5] - 2021-10-21
+### Changed
+- Add variable to configure read-write metastore service ingress.
+
 ## [6.10.4] - 2021-09-21
 ### Changed
 - Attach service account to s3_inventory job when using IRSA.

diff --git a/VARIABLES.md b/VARIABLES.md
@@ -57,6 +57,7 @@
 | hms_rw_heapsize | Heapsize for the read/write Hive Metastore. Valid values: https://docs.aws.amazon.com/AmazonECS/latest/developerguide/task-cpu-memory-error.html | string | - | yes |
 | iam_name_root | Name to identify Hive Metastore IAM roles. | string | `hms` | no |
 | ingress_cidr | Generally allowed ingress CIDR list. | list | - | yes |
+| rw_ingress_cidr | Read-Write metastore ingress CIDR list. | list | `var.ingress_cidr` | no |
 | instance_name | Apiary instance name to identify resources in multi-instance deployments. | string | `` | no |
 | k8s_docker_registry_secret| Docker Registry authentication K8s secret name. | string | `` | no |
 | kiam_arn | Kiam server IAM role ARN. | string | `` | no |

diff --git a/common.tf b/common.tf
@@ -19,11 +19,11 @@ locals {
         resource_suffix : replace(schema["schema_name"], "_", "-"),
         data_bucket : "${local.apiary_bucket_prefix}-${replace(schema["schema_name"], "_", "-")}"
         customer_accounts : lookup(schema, "customer_accounts", join(",", var.apiary_customer_accounts))
-        s3_lifecycle_policy_transition_period: lookup(schema, "s3_lifecycle_policy_transition_period", var.s3_lifecycle_policy_transition_period)
+        s3_lifecycle_policy_transition_period : lookup(schema, "s3_lifecycle_policy_transition_period", var.s3_lifecycle_policy_transition_period)
         # Need to change the default "null" value of s3_object_expiration_days to a number so we can compare it
         # later to s3_lifecycle_policy_transition_period without getting a TF error.  However, TF is doing weird things
         # when comparing them as actual "number" type (-1), so use a string type ("-1"), which works as expected.
-        s3_object_expiration_days_num: coalesce(lookup(schema, "s3_object_expiration_days", "-1"), "-1")
+        s3_object_expiration_days_num : coalesce(lookup(schema, "s3_object_expiration_days", "-1"), "-1")
         s3_storage_class = lookup(schema, "s3_storage_class", var.s3_storage_class)
       },
     schema)
@@ -52,6 +52,9 @@ locals {
   hms_rw_maxthreads = max(100, ceil((var.hms_rw_heapsize * 50) / 100))
 
   hms_alias = var.instance_name == "" ? "hms" : "hms-${var.instance_name}"
+
+  ro_ingress_cidr = var.ingress_cidr
+  rw_ingress_cidr = length(var.rw_ingress_cidr) == 0 ? var.ingress_cidr : var.rw_ingress_cidr
 }
 
 data "aws_iam_account_alias" "current" {}

diff --git a/db.tf b/db.tf
@@ -19,19 +19,11 @@ resource "aws_security_group" "db_sg" {
   vpc_id = var.vpc_id
   tags   = var.apiary_tags
 
-  ingress {
-    from_port   = 0
-    to_port     = 0
-    protocol    = "-1"
-    cidr_blocks = ["${data.aws_vpc.apiary_vpc.cidr_block}"]
-    self        = true
-  }
-
   ingress {
     from_port   = 3306
     to_port     = 3306
     protocol    = "tcp"
-    cidr_blocks = var.ingress_cidr
+    cidr_blocks = data.aws_vpc.apiary_vpc.cidr_block_associations.*.cidr_block
   }
 
   egress {

diff --git a/ecs.tf b/ecs.tf
@@ -58,7 +58,7 @@ resource "aws_ecs_service" "apiary_hms_readwrite_service" {
   }
 
   network_configuration {
-    security_groups = ["${aws_security_group.hms_sg.id}"]
+    security_groups = ["${aws_security_group.hms_rw[0].id}"]
     subnets         = var.private_subnets
   }
 
@@ -83,7 +83,7 @@ resource "aws_ecs_service" "apiary_hms_readonly_service" {
   }
 
   network_configuration {
-    security_groups = ["${aws_security_group.hms_sg.id}"]
+    security_groups = ["${aws_security_group.hms_ro[0].id}"]
     subnets         = var.private_subnets
   }
 

diff --git a/k8s-readonly.tf b/k8s-readonly.tf
@@ -208,7 +208,7 @@ resource "kubernetes_service" "hms_readonly" {
       target_port = 9083
     }
     type                        = var.enable_vpc_endpoint_services ? "LoadBalancer" : "ClusterIP"
-    load_balancer_source_ranges = var.enable_vpc_endpoint_services ? var.ingress_cidr : null
+    load_balancer_source_ranges = var.enable_vpc_endpoint_services ? local.ro_ingress_cidr : null
   }
 }
 

diff --git a/k8s-readwrite.tf b/k8s-readwrite.tf
@@ -248,7 +248,7 @@ resource "kubernetes_service" "hms_readwrite" {
       target_port = 9083
     }
     type                        = var.enable_vpc_endpoint_services ? "LoadBalancer" : "ClusterIP"
-    load_balancer_source_ranges = var.enable_vpc_endpoint_services ? var.ingress_cidr : null
+    load_balancer_source_ranges = var.enable_vpc_endpoint_services ? local.rw_ingress_cidr : null
   }
 }
 

diff --git a/sg.tf b/sg.tf
@@ -4,30 +4,38 @@
  * Licensed under the Apache License, Version 2.0 (the "License");
  */
 
-resource "aws_security_group" "hms_sg" {
-  name   = "${local.instance_alias}-hms"
+resource "aws_security_group" "hms_ro" {
+  count  = var.hms_instance_type == "ecs" ? 1 : 0
+  name   = "${local.instance_alias}-hms-ro"
   vpc_id = var.vpc_id
   tags   = var.apiary_tags
 
-  ingress {
-    from_port   = 22
-    to_port     = 22
-    protocol    = "tcp"
-    cidr_blocks = var.ingress_cidr
-  }
-
   ingress {
     from_port   = 9083
     to_port     = 9083
     protocol    = "tcp"
-    cidr_blocks = var.ingress_cidr
+    cidr_blocks = local.ro_ingress_cidr
   }
 
-  ingress {
+  egress {
     from_port   = 0
     to_port     = 0
     protocol    = "-1"
-    cidr_blocks = ["${data.aws_vpc.apiary_vpc.cidr_block}"]
+    cidr_blocks = ["0.0.0.0/0"]
+  }
+}
+
+resource "aws_security_group" "hms_rw" {
+  count  = var.hms_instance_type == "ecs" ? 1 : 0
+  name   = "${local.instance_alias}-hms-rw"
+  vpc_id = var.vpc_id
+  tags   = var.apiary_tags
+
+  ingress {
+    from_port   = 9083
+    to_port     = 9083
+    protocol    = "tcp"
+    cidr_blocks = local.rw_ingress_cidr
   }
 
   egress {

diff --git a/variables.tf b/variables.tf
@@ -147,7 +147,7 @@ variable "apiary_deny_iamroles" {
 variable "apiary_deny_iamrole_actions" {
   description = "List of S3 actions that 'apiary_deny_iamroles' are not allowed to perform."
   type        = list(string)
-  default     = [
+  default = [
     "s3:Abort*",
     "s3:Bypass*",
     "s3:Delete*",
@@ -340,7 +340,13 @@ variable "elb_timeout" {
 
 variable "ingress_cidr" {
   description = "Generally allowed ingress CIDR list."
-  type        = list(any)
+  type        = list(string)
+}
+
+variable "rw_ingress_cidr" {
+  description = "Read-Write metastore ingress CIDR list. If not set, defaults to `var.ingress_cidr`."
+  type        = list(string)
+  default     = []
 }
 
 variable "enable_gluesync" {