From a1f945f3d6dca86cc35acaa8423a2f65b808409f Mon Sep 17 00:00:00 2001 From: mvaal Date: Fri, 26 Jun 2020 11:16:43 -0500 Subject: [PATCH 1/7] feat: Add aws_security_group_rule.workers_egress_internet to output values --- README.md | 1 + outputs.tf | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/README.md b/README.md index fa565c847d..85d932521e 100644 --- a/README.md +++ b/README.md @@ -235,6 +235,7 @@ MIT Licensed. See [LICENSE](https://github.com/terraform-aws-modules/terraform-a | node\_groups | Outputs from EKS node groups. Map of maps, keyed by var.node\_groups keys | | oidc\_provider\_arn | The ARN of the OIDC Provider if `enable_irsa = true`. | | security\_group\_rule\_cluster\_https\_worker\_ingress | Security group rule responsible for allowing pods to communicate with the EKS cluster API. | +| security\_group\_rule\_cluster\_workers\_egress\_internet | Security group rule responsible for allowing pods to communicate with the Internet. | | worker\_iam\_instance\_profile\_arns | default IAM instance profile ARN for EKS worker groups | | worker\_iam\_instance\_profile\_names | default IAM instance profile name for EKS worker groups | | worker\_iam\_role\_arn | default IAM role ARN for EKS worker groups | diff --git a/outputs.tf b/outputs.tf index 0e00989a83..e85d0343d1 100644 --- a/outputs.tf +++ b/outputs.tf @@ -170,3 +170,8 @@ output "security_group_rule_cluster_https_worker_ingress" { description = "Security group rule responsible for allowing pods to communicate with the EKS cluster API." value = aws_security_group_rule.cluster_https_worker_ingress } + +output "security_group_rule_cluster_workers_egress_internet" { + description = "Security group rule responsible for allowing pods to communicate with the Internet." + value = aws_security_group_rule.workers_egress_internet +} From 73927c33e1d2d7fe550ac8d1ffa0dbad59449f1e Mon Sep 17 00:00:00 2001 From: mvaal Date: Fri, 26 Jun 2020 11:36:29 -0500 Subject: [PATCH 2/7] Changed name from cluster_workers_egress_internet to workers_egress_internet --- README.md | 2 +- outputs.tf | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 85d932521e..f5ac1fda26 100644 --- a/README.md +++ b/README.md @@ -234,7 +234,7 @@ MIT Licensed. See [LICENSE](https://github.com/terraform-aws-modules/terraform-a | kubeconfig\_filename | The filename of the generated kubectl config. | | node\_groups | Outputs from EKS node groups. Map of maps, keyed by var.node\_groups keys | | oidc\_provider\_arn | The ARN of the OIDC Provider if `enable_irsa = true`. | -| security\_group\_rule\_cluster\_https\_worker\_ingress | Security group rule responsible for allowing pods to communicate with the EKS cluster API. | +| security\_group\_rule\_https\_worker\_ingress | Security group rule responsible for allowing pods to communicate with the EKS cluster API. | | security\_group\_rule\_cluster\_workers\_egress\_internet | Security group rule responsible for allowing pods to communicate with the Internet. | | worker\_iam\_instance\_profile\_arns | default IAM instance profile ARN for EKS worker groups | | worker\_iam\_instance\_profile\_names | default IAM instance profile name for EKS worker groups | diff --git a/outputs.tf b/outputs.tf index e85d0343d1..ceb29ba7a8 100644 --- a/outputs.tf +++ b/outputs.tf @@ -171,7 +171,7 @@ output "security_group_rule_cluster_https_worker_ingress" { value = aws_security_group_rule.cluster_https_worker_ingress } -output "security_group_rule_cluster_workers_egress_internet" { +output "security_group_rule_workers_egress_internet" { description = "Security group rule responsible for allowing pods to communicate with the Internet." value = aws_security_group_rule.workers_egress_internet } From c9e7189ec33000d984f3a1af320a26cfe2c5ed40 Mon Sep 17 00:00:00 2001 From: mvaal Date: Fri, 26 Jun 2020 11:37:14 -0500 Subject: [PATCH 3/7] Changed name from cluster_workers_egress_internet to workers_egress_internet --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index f5ac1fda26..e5de7c6984 100644 --- a/README.md +++ b/README.md @@ -234,8 +234,8 @@ MIT Licensed. See [LICENSE](https://github.com/terraform-aws-modules/terraform-a | kubeconfig\_filename | The filename of the generated kubectl config. | | node\_groups | Outputs from EKS node groups. Map of maps, keyed by var.node\_groups keys | | oidc\_provider\_arn | The ARN of the OIDC Provider if `enable_irsa = true`. | -| security\_group\_rule\_https\_worker\_ingress | Security group rule responsible for allowing pods to communicate with the EKS cluster API. | -| security\_group\_rule\_cluster\_workers\_egress\_internet | Security group rule responsible for allowing pods to communicate with the Internet. | +| security\_group\_rule\_cluster\_https\_worker\_ingress | Security group rule responsible for allowing pods to communicate with the EKS cluster API. | +| security\_group\_rule\_workers\_egress\_internet | Security group rule responsible for allowing pods to communicate with the Internet. | | worker\_iam\_instance\_profile\_arns | default IAM instance profile ARN for EKS worker groups | | worker\_iam\_instance\_profile\_names | default IAM instance profile name for EKS worker groups | | worker\_iam\_role\_arn | default IAM role ARN for EKS worker groups | From 6695d24edd60bbba705d12807ba21ba10081b8a7 Mon Sep 17 00:00:00 2001 From: mvaal Date: Fri, 26 Jun 2020 12:09:51 -0500 Subject: [PATCH 4/7] Changed aws_autoscaling_group.workers to depend on SGs and policies --- README.md | 1 - outputs.tf | 5 ----- workers.tf | 16 ++++++++++++++++ 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index e5de7c6984..fa565c847d 100644 --- a/README.md +++ b/README.md @@ -235,7 +235,6 @@ MIT Licensed. See [LICENSE](https://github.com/terraform-aws-modules/terraform-a | node\_groups | Outputs from EKS node groups. Map of maps, keyed by var.node\_groups keys | | oidc\_provider\_arn | The ARN of the OIDC Provider if `enable_irsa = true`. | | security\_group\_rule\_cluster\_https\_worker\_ingress | Security group rule responsible for allowing pods to communicate with the EKS cluster API. | -| security\_group\_rule\_workers\_egress\_internet | Security group rule responsible for allowing pods to communicate with the Internet. | | worker\_iam\_instance\_profile\_arns | default IAM instance profile ARN for EKS worker groups | | worker\_iam\_instance\_profile\_names | default IAM instance profile name for EKS worker groups | | worker\_iam\_role\_arn | default IAM role ARN for EKS worker groups | diff --git a/outputs.tf b/outputs.tf index ceb29ba7a8..0e00989a83 100644 --- a/outputs.tf +++ b/outputs.tf @@ -170,8 +170,3 @@ output "security_group_rule_cluster_https_worker_ingress" { description = "Security group rule responsible for allowing pods to communicate with the EKS cluster API." value = aws_security_group_rule.cluster_https_worker_ingress } - -output "security_group_rule_workers_egress_internet" { - description = "Security group rule responsible for allowing pods to communicate with the Internet." - value = aws_security_group_rule.workers_egress_internet -} diff --git a/workers.tf b/workers.tf index 2e2a80f6d8..9ba9c2ae3d 100644 --- a/workers.tf +++ b/workers.tf @@ -132,6 +132,22 @@ resource "aws_autoscaling_group" "workers" { create_before_destroy = true ignore_changes = [desired_capacity] } + + # Prevent premature access of security group roles and policies by pods that + # require permissions on create/destroy that depend on workers. + depends_on = [ + aws_security_group_rule.workers_egress_internet, + aws_security_group_rule.workers_ingress_self, + aws_security_group_rule.workers_ingress_cluster, + aws_security_group_rule.workers_ingress_cluster_kubelet, + aws_security_group_rule.workers_ingress_cluster_https, + aws_security_group_rule.workers_ingress_cluster_primary, + aws_security_group_rule.cluster_primary_ingress_workers, + aws_iam_role_policy_attachment.workers_AmazonEKSWorkerNodePolicy, + aws_iam_role_policy_attachment.workers_AmazonEKS_CNI_Policy, + aws_iam_role_policy_attachment.workers_AmazonEC2ContainerRegistryReadOnly, + aws_iam_role_policy_attachment.workers_additional_policies + ] } resource "aws_launch_configuration" "workers" { From 4992038c11b6e99dd84cc8883152dbc3db96d42f Mon Sep 17 00:00:00 2001 From: mvaal Date: Fri, 26 Jun 2020 13:21:12 -0500 Subject: [PATCH 5/7] Add cluster SG rules to cluster dependencies Modified aws_security_group.workers to not depend on aws_eks_cluster.this as it would cause circular dependency. This should prevent SGs from being destroyed until after cluster is destroyed. --- cluster.tf | 2 ++ workers.tf | 6 +++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/cluster.tf b/cluster.tf index 68a07369df..41b5f2bbf3 100644 --- a/cluster.tf +++ b/cluster.tf @@ -39,6 +39,8 @@ resource "aws_eks_cluster" "this" { } depends_on = [ + aws_security_group_rule.cluster_egress_internet, + aws_security_group_rule.cluster_https_worker_ingress, aws_iam_role_policy_attachment.cluster_AmazonEKSClusterPolicy, aws_iam_role_policy_attachment.cluster_AmazonEKSServicePolicy, aws_cloudwatch_log_group.this diff --git a/workers.tf b/workers.tf index 9ba9c2ae3d..3e1cef6487 100644 --- a/workers.tf +++ b/workers.tf @@ -287,14 +287,14 @@ resource "random_pet" "workers" { resource "aws_security_group" "workers" { count = var.worker_create_security_group && var.create_eks ? 1 : 0 - name_prefix = aws_eks_cluster.this[0].name + name_prefix = var.cluster_name description = "Security group for all nodes in the cluster." vpc_id = var.vpc_id tags = merge( var.tags, { - "Name" = "${aws_eks_cluster.this[0].name}-eks_worker_sg" - "kubernetes.io/cluster/${aws_eks_cluster.this[0].name}" = "owned" + "Name" = "${var.cluster_name}-eks_worker_sg" + "kubernetes.io/cluster/${var.cluster_name}" = "owned" }, ) } From a84f4eb1a1e88f9ec8cfd86b3c09a9502317e72e Mon Sep 17 00:00:00 2001 From: mvaal Date: Fri, 26 Jun 2020 13:59:18 -0500 Subject: [PATCH 6/7] Moved worker dependencies into aws_launch_configuration.workers aws_autoscaling_groups.workers already depends on aws_launch_configuration.workers and aws_launch_configuration.workers is where the security group dependencies are set, makes the most sense here instead of at the ASG level. --- workers.tf | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/workers.tf b/workers.tf index 3e1cef6487..6c9d924f0e 100644 --- a/workers.tf +++ b/workers.tf @@ -132,22 +132,6 @@ resource "aws_autoscaling_group" "workers" { create_before_destroy = true ignore_changes = [desired_capacity] } - - # Prevent premature access of security group roles and policies by pods that - # require permissions on create/destroy that depend on workers. - depends_on = [ - aws_security_group_rule.workers_egress_internet, - aws_security_group_rule.workers_ingress_self, - aws_security_group_rule.workers_ingress_cluster, - aws_security_group_rule.workers_ingress_cluster_kubelet, - aws_security_group_rule.workers_ingress_cluster_https, - aws_security_group_rule.workers_ingress_cluster_primary, - aws_security_group_rule.cluster_primary_ingress_workers, - aws_iam_role_policy_attachment.workers_AmazonEKSWorkerNodePolicy, - aws_iam_role_policy_attachment.workers_AmazonEKS_CNI_Policy, - aws_iam_role_policy_attachment.workers_AmazonEC2ContainerRegistryReadOnly, - aws_iam_role_policy_attachment.workers_additional_policies - ] } resource "aws_launch_configuration" "workers" { @@ -272,6 +256,22 @@ resource "aws_launch_configuration" "workers" { lifecycle { create_before_destroy = true } + + # Prevent premature access of security group roles and policies by pods that + # require permissions on create/destroy that depend on workers. + depends_on = [ + aws_security_group_rule.workers_egress_internet, + aws_security_group_rule.workers_ingress_self, + aws_security_group_rule.workers_ingress_cluster, + aws_security_group_rule.workers_ingress_cluster_kubelet, + aws_security_group_rule.workers_ingress_cluster_https, + aws_security_group_rule.workers_ingress_cluster_primary, + aws_security_group_rule.cluster_primary_ingress_workers, + aws_iam_role_policy_attachment.workers_AmazonEKSWorkerNodePolicy, + aws_iam_role_policy_attachment.workers_AmazonEKS_CNI_Policy, + aws_iam_role_policy_attachment.workers_AmazonEC2ContainerRegistryReadOnly, + aws_iam_role_policy_attachment.workers_additional_policies + ] } resource "random_pet" "workers" { From 37ddfef52fcf407f54913dc855c7d9165c6c7068 Mon Sep 17 00:00:00 2001 From: mvaal Date: Mon, 29 Jun 2020 09:34:00 -0500 Subject: [PATCH 7/7] Duplicated worker dependencies into workers_launch_template aws_launch_template aws_launch_template also depends on worker_security_group_id --- workers_launch_template.tf | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/workers_launch_template.tf b/workers_launch_template.tf index 2e0bd9759d..5ac64267a2 100644 --- a/workers_launch_template.tf +++ b/workers_launch_template.tf @@ -423,6 +423,22 @@ resource "aws_launch_template" "workers_launch_template" { lifecycle { create_before_destroy = true } + + # Prevent premature access of security group roles and policies by pods that + # require permissions on create/destroy that depend on workers. + depends_on = [ + aws_security_group_rule.workers_egress_internet, + aws_security_group_rule.workers_ingress_self, + aws_security_group_rule.workers_ingress_cluster, + aws_security_group_rule.workers_ingress_cluster_kubelet, + aws_security_group_rule.workers_ingress_cluster_https, + aws_security_group_rule.workers_ingress_cluster_primary, + aws_security_group_rule.cluster_primary_ingress_workers, + aws_iam_role_policy_attachment.workers_AmazonEKSWorkerNodePolicy, + aws_iam_role_policy_attachment.workers_AmazonEKS_CNI_Policy, + aws_iam_role_policy_attachment.workers_AmazonEC2ContainerRegistryReadOnly, + aws_iam_role_policy_attachment.workers_additional_policies + ] } resource "random_pet" "workers_launch_template" {