Skip to content

Commit

Permalink
Move workers SG to root module due to tf cycle during resources termi…
Browse files Browse the repository at this point in the history
…nation and improve logic when cluster is disabled
  • Loading branch information
Grzegorz Lisowski committed Sep 8, 2020
1 parent 5aa18ce commit da785fc
Show file tree
Hide file tree
Showing 11 changed files with 62 additions and 118 deletions.
3 changes: 2 additions & 1 deletion cluster.tf
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ resource "aws_security_group" "cluster" {
name_prefix = var.cluster_name
description = "EKS cluster security group."
vpc_id = var.vpc_id

tags = merge(
var.tags,
{
Expand All @@ -105,7 +106,7 @@ resource "aws_security_group_rule" "cluster_https_worker_ingress" {
description = "Allow pods to communicate with the EKS cluster API."
protocol = "tcp"
security_group_id = local.cluster_security_group_id
source_security_group_id = module.worker_groups.worker_security_group_id
source_security_group_id = local.worker_security_group_id
from_port = 443
to_port = 443
type = "ingress"
Expand Down
1 change: 1 addition & 0 deletions local.tf
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ locals {
cluster_primary_security_group_id = var.cluster_version >= 1.14 ? element(concat(aws_eks_cluster.this[*].vpc_config[0].cluster_security_group_id, list("")), 0) : null
cluster_iam_role_name = var.manage_cluster_iam_resources ? join("", aws_iam_role.cluster.*.name) : var.cluster_iam_role_name
cluster_iam_role_arn = var.manage_cluster_iam_resources ? join("", aws_iam_role.cluster.*.arn) : join("", data.aws_iam_role.custom_cluster_iam_role.*.arn)
worker_security_group_id = var.worker_create_security_group ? join("", aws_security_group.workers.*.id) : var.worker_security_group_id

default_iam_role_id = concat(aws_iam_role.workers.*.id, [""])[0]

Expand Down
9 changes: 1 addition & 8 deletions modules/worker_groups/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ No requirements.
| cluster\_auth\_base64 | Cluster auth data | `string` | n/a | yes |
| cluster\_endpoint | Cluster endpojnt | `string` | n/a | yes |
| cluster\_name | Cluster name | `string` | n/a | yes |
| cluster\_primary\_security\_group\_id | EKS cluster primary security group id. | `string` | n/a | yes |
| cluster\_security\_group\_id | EKS cluster security group id. | `string` | n/a | yes |
| cluster\_version | Kubernetes version to use for the EKS cluster. | `string` | n/a | yes |
| create\_workers | Controls if EKS resources should be created (it affects almost all resources) | `bool` | `true` | no |
| default\_iam\_role\_id | ARN of the default IAM worker role to use if one is not specified in `var.node_groups` or `var.node_groups_defaults` | `string` | n/a | yes |
Expand All @@ -49,17 +47,13 @@ No requirements.
| subnets | A list of subnets to place the EKS cluster and workers within. | `list(string)` | n/a | yes |
| tags | A map of tags to add to all resources | `map(string)` | n/a | yes |
| vpc\_id | VPC where the cluster and workers will be deployed. | `string` | n/a | yes |
| worker\_additional\_security\_group\_ids | A list of additional security group ids to attach to worker instances | `list(string)` | `[]` | no |
| worker\_ami\_name\_filter | Name filter for AWS EKS worker AMI. If not provided, the latest official AMI for the specified 'cluster\_version' is used. | `string` | `""` | no |
| worker\_ami\_name\_filter\_windows | Name filter for AWS EKS Windows worker AMI. If not provided, the latest official AMI for the specified 'cluster\_version' is used. | `string` | `""` | no |
| worker\_ami\_owner\_id | The ID of the owner for the AMI to use for the AWS EKS workers. Valid values are an AWS account ID, 'self' (the current account), or an AWS owner alias (e.g. 'amazon', 'aws-marketplace', 'microsoft'). | `string` | `"602401143452"` | no |
| worker\_ami\_owner\_id\_windows | The ID of the owner for the AMI to use for the AWS EKS Windows workers. Valid values are an AWS account ID, 'self' (the current account), or an AWS owner alias (e.g. 'amazon', 'aws-marketplace', 'microsoft'). | `string` | `"801119661308"` | no |
| worker\_create\_cluster\_primary\_security\_group\_rules | Whether to create security group rules to allow communication between pods on workers and pods using the primary cluster security group. | `bool` | `false` | no |
| worker\_create\_initial\_lifecycle\_hooks | Whether to create initial lifecycle hooks provided in worker groups. | `bool` | `false` | no |
| worker\_create\_security\_group | Whether to create a security group for the workers or attach the workers to `worker_security_group_id`. | `bool` | `true` | no |
| worker\_groups | A map of maps defining worker group configurations to be defined using AWS Launch Templates. See workers\_group\_defaults for valid keys. | `any` | `{}` | no |
| worker\_security\_group\_id | If provided, all workers will be attached to this security group. If not given, a security group will be created with necessary ingress/egress to work with the EKS cluster. | `string` | `""` | no |
| worker\_sg\_ingress\_from\_port | Minimum port number from which pods will accept communication. Must be changed to a lower value if some pods in your cluster will expose a port lower than 1025 (e.g. 22, 80, or 443). | `number` | `1025` | no |
| worker\_security\_group\_ids | A list of security group ids to attach to worker instances | `list(string)` | `[]` | no |
| workers\_group\_defaults | Workers group defaults from parent | `any` | n/a | yes |

## Outputs
Expand All @@ -68,6 +62,5 @@ No requirements.
|------|-------------|
| aws\_auth\_roles | Roles for use in aws-auth ConfigMap |
| worker\_groups | Outputs from EKS worker groups. Map of maps, keyed by `var.worker_groups` keys. |
| worker\_security\_group\_id | Security group ID attached to the EKS workers. |

<!-- END OF PRE-COMMIT-TERRAFORM DOCS HOOK -->
2 changes: 0 additions & 2 deletions modules/worker_groups/local.tf
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,4 @@ locals {
worker_ami_name_filter_windows = (var.worker_ami_name_filter_windows != "" ?
var.worker_ami_name_filter_windows : "Windows_Server-2019-English-Core-EKS_Optimized-${tonumber(var.cluster_version) >= 1.14 ? var.cluster_version : 1.14}-*"
)

worker_security_group_id = var.worker_create_security_group ? join("", aws_security_group.workers.*.id) : var.worker_security_group_id
}
12 changes: 1 addition & 11 deletions modules/worker_groups/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,7 @@ resource "aws_launch_template" "workers" {
delete_on_termination = each.value["eni_delete"]

security_groups = flatten([
local.worker_security_group_id,
var.worker_additional_security_group_ids,
var.worker_security_group_ids,
each.value["additional_security_group_ids"]
])
}
Expand Down Expand Up @@ -237,16 +236,7 @@ resource "aws_launch_template" "workers" {
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,
var.ng_depends_on,
]
}
Expand Down
5 changes: 0 additions & 5 deletions modules/worker_groups/outputs.tf
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,3 @@ output "worker_groups" {
description = "Outputs from EKS worker groups. Map of maps, keyed by `var.worker_groups` keys."
value = aws_autoscaling_group.workers
}

output "worker_security_group_id" {
description = "Security group ID attached to the EKS workers."
value = local.worker_security_group_id
}
38 changes: 2 additions & 36 deletions modules/worker_groups/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -73,42 +73,8 @@ variable "subnets" {
type = list(string)
}

variable "cluster_security_group_id" {
description = "EKS cluster security group id."
type = string
}

variable "cluster_primary_security_group_id" {
description = "EKS cluster primary security group id."
type = string
}

variable "worker_create_cluster_primary_security_group_rules" {
description = "Whether to create security group rules to allow communication between pods on workers and pods using the primary cluster security group."
type = bool
default = false
}

variable "worker_sg_ingress_from_port" {
description = "Minimum port number from which pods will accept communication. Must be changed to a lower value if some pods in your cluster will expose a port lower than 1025 (e.g. 22, 80, or 443)."
type = number
default = 1025
}

variable "worker_create_security_group" {
description = "Whether to create a security group for the workers or attach the workers to `worker_security_group_id`."
type = bool
default = true
}

variable "worker_security_group_id" {
description = "If provided, all workers will be attached to this security group. If not given, a security group will be created with necessary ingress/egress to work with the EKS cluster."
type = string
default = ""
}

variable "worker_additional_security_group_ids" {
description = "A list of additional security group ids to attach to worker instances"
variable "worker_security_group_ids" {
description = "A list of security group ids to attach to worker instances"
type = list(string)
default = []
}
Expand Down
2 changes: 1 addition & 1 deletion outputs.tf
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ output "oidc_provider_arn" {

output "worker_security_group_id" {
description = "Security group ID attached to the EKS workers."
value = module.worker_groups.worker_security_group_id
value = local.worker_security_group_id
}

output "node_groups" {
Expand Down
12 changes: 5 additions & 7 deletions worker_groups.tf
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module "worker_groups" {
cluster_version = var.cluster_version
cluster_name = var.cluster_name
cluster_endpoint = coalescelist(aws_eks_cluster.this[*].endpoint, [""])[0]
cluster_auth_base64 = aws_eks_cluster.this[0].certificate_authority[0].data
cluster_auth_base64 = flatten(concat(aws_eks_cluster.this[*].certificate_authority[*].data, [""]))[0]

default_iam_role_id = coalescelist(aws_iam_role.workers[*].id, [""])[0]

Expand All @@ -25,12 +25,10 @@ module "worker_groups" {
worker_ami_owner_id = var.worker_ami_owner_id
worker_ami_owner_id_windows = var.worker_ami_owner_id_windows

cluster_security_group_id = local.cluster_security_group_id
cluster_primary_security_group_id = local.cluster_primary_security_group_id
worker_create_security_group = var.worker_create_security_group
worker_security_group_id = var.worker_security_group_id
worker_sg_ingress_from_port = var.worker_sg_ingress_from_port
worker_additional_security_group_ids = var.worker_additional_security_group_ids
worker_security_group_ids = flatten([
local.worker_security_group_id,
var.worker_additional_security_group_ids
])

tags = var.tags

Expand Down
62 changes: 49 additions & 13 deletions modules/worker_groups/sg.tf → worker_groups_support.tf
Original file line number Diff line number Diff line change
@@ -1,9 +1,45 @@
resource "aws_iam_role" "workers" {
count = var.manage_worker_iam_resources && var.create_eks ? 1 : 0
name_prefix = var.workers_role_name != "" ? null : aws_eks_cluster.this[0].name
name = var.workers_role_name != "" ? var.workers_role_name : null
assume_role_policy = data.aws_iam_policy_document.workers_assume_role_policy.json
permissions_boundary = var.permissions_boundary
path = var.iam_path
force_detach_policies = true
tags = var.tags
}

resource "aws_iam_role_policy_attachment" "workers_AmazonEKSWorkerNodePolicy" {
count = var.manage_worker_iam_resources && var.create_eks ? 1 : 0
policy_arn = "${local.policy_arn_prefix}/AmazonEKSWorkerNodePolicy"
role = aws_iam_role.workers[0].name
}

resource "aws_iam_role_policy_attachment" "workers_AmazonEKS_CNI_Policy" {
count = var.manage_worker_iam_resources && var.attach_worker_cni_policy && var.create_eks ? 1 : 0
policy_arn = "${local.policy_arn_prefix}/AmazonEKS_CNI_Policy"
role = aws_iam_role.workers[0].name
}

resource "aws_iam_role_policy_attachment" "workers_AmazonEC2ContainerRegistryReadOnly" {
count = var.manage_worker_iam_resources && var.create_eks ? 1 : 0
policy_arn = "${local.policy_arn_prefix}/AmazonEC2ContainerRegistryReadOnly"
role = aws_iam_role.workers[0].name
}

resource "aws_iam_role_policy_attachment" "workers_additional_policies" {
count = var.manage_worker_iam_resources && var.create_eks ? length(var.workers_additional_policies) : 0
role = aws_iam_role.workers[0].name
policy_arn = var.workers_additional_policies[count.index]
}

resource "aws_security_group" "workers" {
count = var.worker_create_security_group && var.create_workers ? 1 : 0
count = var.worker_create_security_group && var.create_eks ? 1 : 0

name_prefix = var.cluster_name
description = "Security group for all nodes in the cluster."
vpc_id = var.vpc_id

tags = merge(
var.tags,
{
Expand All @@ -14,7 +50,7 @@ resource "aws_security_group" "workers" {
}

resource "aws_security_group_rule" "workers_egress_internet" {
count = var.worker_create_security_group && var.create_workers ? 1 : 0
count = var.worker_create_security_group && var.create_eks ? 1 : 0
description = "Allow nodes all egress to the Internet."
protocol = "-1"
security_group_id = local.worker_security_group_id
Expand All @@ -25,7 +61,7 @@ resource "aws_security_group_rule" "workers_egress_internet" {
}

resource "aws_security_group_rule" "workers_ingress_self" {
count = var.worker_create_security_group && var.create_workers ? 1 : 0
count = var.worker_create_security_group && var.create_eks ? 1 : 0
description = "Allow node to communicate with each other."
protocol = "-1"
security_group_id = local.worker_security_group_id
Expand All @@ -36,54 +72,54 @@ resource "aws_security_group_rule" "workers_ingress_self" {
}

resource "aws_security_group_rule" "workers_ingress_cluster" {
count = var.worker_create_security_group && var.create_workers ? 1 : 0
count = var.worker_create_security_group && var.create_eks ? 1 : 0
description = "Allow workers pods to receive communication from the cluster control plane."
protocol = "tcp"
security_group_id = local.worker_security_group_id
source_security_group_id = var.cluster_security_group_id
source_security_group_id = local.cluster_security_group_id
from_port = var.worker_sg_ingress_from_port
to_port = 65535
type = "ingress"
}

resource "aws_security_group_rule" "workers_ingress_cluster_kubelet" {
count = var.worker_create_security_group && var.create_workers ? var.worker_sg_ingress_from_port > 10250 ? 1 : 0 : 0
count = var.worker_create_security_group && var.create_eks ? var.worker_sg_ingress_from_port > 10250 ? 1 : 0 : 0
description = "Allow workers Kubelets to receive communication from the cluster control plane."
protocol = "tcp"
security_group_id = local.worker_security_group_id
source_security_group_id = var.cluster_security_group_id
source_security_group_id = local.cluster_security_group_id
from_port = 10250
to_port = 10250
type = "ingress"
}

resource "aws_security_group_rule" "workers_ingress_cluster_https" {
count = var.worker_create_security_group && var.create_workers ? 1 : 0
count = var.worker_create_security_group && var.create_eks ? 1 : 0
description = "Allow pods running extension API servers on port 443 to receive communication from cluster control plane."
protocol = "tcp"
security_group_id = local.worker_security_group_id
source_security_group_id = var.cluster_security_group_id
source_security_group_id = local.cluster_security_group_id
from_port = 443
to_port = 443
type = "ingress"
}

resource "aws_security_group_rule" "workers_ingress_cluster_primary" {
count = var.worker_create_security_group && var.worker_create_cluster_primary_security_group_rules && var.cluster_version >= 1.14 && var.create_workers ? 1 : 0
count = var.worker_create_security_group && var.worker_create_cluster_primary_security_group_rules && var.cluster_version >= 1.14 && var.create_eks ? 1 : 0
description = "Allow pods running on workers to receive communication from cluster primary security group (e.g. Fargate pods)."
protocol = "all"
security_group_id = local.worker_security_group_id
source_security_group_id = var.cluster_primary_security_group_id
source_security_group_id = local.cluster_primary_security_group_id
from_port = 0
to_port = 65535
type = "ingress"
}

resource "aws_security_group_rule" "cluster_primary_ingress_workers" {
count = var.worker_create_security_group && var.worker_create_cluster_primary_security_group_rules && var.cluster_version >= 1.14 && var.create_workers ? 1 : 0
count = var.worker_create_security_group && var.worker_create_cluster_primary_security_group_rules && var.cluster_version >= 1.14 && var.create_eks ? 1 : 0
description = "Allow pods running on workers to send communication to cluster primary security group (e.g. Fargate pods)."
protocol = "all"
security_group_id = var.cluster_primary_security_group_id
security_group_id = local.cluster_primary_security_group_id
source_security_group_id = local.worker_security_group_id
from_port = 0
to_port = 65535
Expand Down
34 changes: 0 additions & 34 deletions workers_iam.tf

This file was deleted.

0 comments on commit da785fc

Please sign in to comment.