Skip to content

Commit

Permalink
feat: worker launch templates and configurations depend on security g…
Browse files Browse the repository at this point in the history
…roup rules and IAM policies (#933)

In order to ensure proper ordering when running terraform destroy. This will block Terraform from removing up security group rules before the cluster has finished its clean up chores.
  • Loading branch information
mvaal authored Jul 12, 2020
1 parent 9a0e548 commit db9bb0b
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 3 deletions.
2 changes: 2 additions & 0 deletions cluster.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 19 additions & 3 deletions workers.tf
Original file line number Diff line number Diff line change
Expand Up @@ -256,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" {
Expand All @@ -271,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"
},
)
}
Expand Down
16 changes: 16 additions & 0 deletions workers_launch_template.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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" {
Expand Down

3 comments on commit db9bb0b

@jurgenweber
Copy link
Contributor

Choose a reason for hiding this comment

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

this seems to have broken destroying?

Error: Cycle: module.cluster["0"].module.eks.aws_eks_cluster.this[0], module.cluster["0"].module.eks.aws_security_group_rule.cluster_https_worker_ingress[0], module.cluster["0"].module.eks.aws_security_group.workers[0]

@dpiddockcmp
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a terraform "feature". See #954 for more discussion.

@jurgenweber
Copy link
Contributor

Choose a reason for hiding this comment

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

I will admit, I am not following but ok.

Please sign in to comment.