Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: Support default_tags in aws_autoscaling_group and launch_template #1968

Closed
wants to merge 3 commits into from
Closed

feat: Support default_tags in aws_autoscaling_group and launch_template #1968

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d50d034
feat: Support default_tags in aws_autoscaling_group and launch_template
Mar 25, 2022
43ae7f0
fix ternary logic & clean up var naming/docs
Mar 28, 2022
dfdefee
update complete example var names
Mar 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions examples/complete/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
provider "aws" {
region = local.region

default_tags {
tags = {
ExampleDefaultTag = "ExampleDefaultValue"
}
}
}

locals {
Expand Down Expand Up @@ -111,6 +117,7 @@ module "eks" {
instance_types = ["m6i.large", "m5.large", "m5n.large", "m5zn.large"]

attach_cluster_primary_security_group = true
launch_template_use_default_tags = true
vpc_security_group_ids = [aws_security_group.additional.id]
}

Expand Down Expand Up @@ -195,6 +202,8 @@ module "eks_managed_node_group" {
module.eks.cluster_security_group_id,
]

launch_template_use_default_tags = true

tags = merge(local.tags, { Separate = "eks-managed-node-group" })
}

Expand All @@ -216,6 +225,8 @@ module "self_managed_node_group" {
module.eks.cluster_security_group_id,
]

use_default_tags = true

tags = merge(local.tags, { Separate = "self-managed-node-group" })
}

Expand Down
2 changes: 2 additions & 0 deletions modules/eks-managed-node-group/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ module "eks_managed_node_group" {
| [aws_security_group.this](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group) | resource |
| [aws_security_group_rule.this](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group_rule) | resource |
| [aws_caller_identity.current](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/caller_identity) | data source |
| [aws_default_tags.current](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/default_tags) | data source |
| [aws_iam_policy_document.assume_role_policy](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source |
| [aws_partition.current](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/partition) | data source |

Expand Down Expand Up @@ -130,6 +131,7 @@ module "eks_managed_node_group" {
| <a name="input_launch_template_description"></a> [launch\_template\_description](#input\_launch\_template\_description) | Description of the launch template | `string` | `null` | no |
| <a name="input_launch_template_name"></a> [launch\_template\_name](#input\_launch\_template\_name) | Launch template name - either to be created (`var.create_launch_template` = `true`) or existing (`var.create_launch_template` = `false`) | `string` | `""` | no |
| <a name="input_launch_template_tags"></a> [launch\_template\_tags](#input\_launch\_template\_tags) | A map of additional tags to add to the tag\_specifications of launch template created | `map(string)` | `{}` | no |
| <a name="input_launch_template_use_default_tags"></a> [launch\_template\_use\_default\_tags](#input\_launch\_template\_use\_default\_tags) | Enables/disables the use of provider default tags in the tag\_specifications of the launch template created | `bool` | `false` | no |
| <a name="input_launch_template_use_name_prefix"></a> [launch\_template\_use\_name\_prefix](#input\_launch\_template\_use\_name\_prefix) | Determines whether to use `launch_template_name` as is or create a unique name beginning with the `launch_template_name` as the prefix | `bool` | `true` | no |
| <a name="input_launch_template_version"></a> [launch\_template\_version](#input\_launch\_template\_version) | Launch template version number. The default is `$Default` | `string` | `null` | no |
| <a name="input_license_specifications"></a> [license\_specifications](#input\_license\_specifications) | A list of license specifications to associate with | `map(string)` | `null` | no |
Expand Down
6 changes: 5 additions & 1 deletion modules/eks-managed-node-group/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ data "aws_partition" "current" {}

data "aws_caller_identity" "current" {}

data "aws_default_tags" "current" {}

################################################################################
# User Data
################################################################################
Expand Down Expand Up @@ -38,6 +40,8 @@ locals {
use_custom_launch_template = var.create_launch_template || var.launch_template_name != ""

launch_template_name_int = coalesce(var.launch_template_name, "${var.name}-eks-node-group")

launch_template_default_tags = var.launch_template_use_default_tags ? merge(data.aws_default_tags.current.tags, var.tags) : var.tags
}

resource "aws_launch_template" "this" {
Expand Down Expand Up @@ -238,7 +242,7 @@ resource "aws_launch_template" "this" {
for_each = toset(["instance", "volume", "network-interface"])
content {
resource_type = tag_specifications.key
tags = merge(var.tags, { Name = var.name }, var.launch_template_tags)
tags = merge(local.launch_template_default_tags, { Name = var.name }, var.launch_template_tags)
Copy link
Member

Choose a reason for hiding this comment

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

why do we need to set this here? from my understanding, only autoscaling groups behave differently when it comes to default tags

Copy link
Contributor Author

@raizyr raizyr Mar 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I double checked what I'd been seeing on Friday, and while the provider default tags do get automatically included in the launch_template resource's tags, they do not get included in the tag_specifications of the launch_template. Those instead act much like the ASG tag specifications. Adding the ability to include the default tags to the launch_template tag specifications ensures that any instances created from the launch_template will also have the default tags set on the instance itself.

Edit to fix the misleading resource tags comment. The launch_template resource has both a tags and tag_specifications input attribute. The default_tags do get added to the tags_all attribute of the state, however they do not get added to the tag_specifications that propagate down to the instance, network-interface, and volume.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would a different variable name make sense to make it clearer? Maybe tag_specification_use_default_tags or something similar?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I found a relevant issue in the provider repo. hashicorp/terraform-provider-aws#19387

Copy link
Member

Choose a reason for hiding this comment

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

I think for now we leave off the launch template changes - this matches what we have in the autoscaling module (which at some point will get folded into this module) and just add to the autoscaling group

Copy link
Contributor Author

@raizyr raizyr Mar 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I'm very confused. The module as it exists right now is not able to correctly set provider default tags for resources created by neither an autoscaling group nor a launch template. Are you saying that only the autoscaling group use case should be fixed?

Copy link
Member

Choose a reason for hiding this comment

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

  1. We cannot control the autoscaling groups created by the EKS managed node group service so there is no solution for default tags on that resource currently other than using the external autoscaling group tags resource (has to be external to the module Add Tags to ASG when using node groups #1558 (comment))
  2. We should mirror what we did in the autoscaling group module for the self-managed node group module https://github.com/terraform-aws-modules/terraform-aws-autoscaling/blob/3c928b8cfb2be46ff756be350fb26c33ee1a640a/main.tf#L10-L15 which is the same as what the Terraform docs specify. We only add the default tags to the autoscaling group resource

Copy link
Member

Choose a reason for hiding this comment

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

I understand there are many issues related to default tags right now which is why I am cautious regarding what gets introduced. The Terraform docs only offer the caveat for autoscaling groups https://www.hashicorp.com/blog/default-tags-in-the-terraform-aws-provider and not launch templates and this is what we have mimicked in the autoscaling group module without issue. So for now, I think that is a safe path if we want to introduce that change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, unfortunately I need to maintain my company fork with the launch template changes, since that is the issue I needed to solve when I stumbled across the open issue about the asg. I'll try to get a moment to pull just the asg changes over to a fork on my personal github and submit a new PR.

}
}

Expand Down
6 changes: 6 additions & 0 deletions modules/eks-managed-node-group/variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,12 @@ variable "launch_template_tags" {
default = {}
}

variable "launch_template_use_default_tags" {
description = "Enables/disables the use of provider default tags in the tag_specifications of the launch template created"
type = bool
default = false
}

################################################################################
# EKS Managed Node Group
################################################################################
Expand Down
3 changes: 3 additions & 0 deletions modules/self-managed-node-group/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ module "self_managed_node_group" {
| [aws_security_group_rule.this](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group_rule) | resource |
| [aws_ami.eks_default](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/ami) | data source |
| [aws_caller_identity.current](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/caller_identity) | data source |
| [aws_default_tags.current](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/default_tags) | data source |
| [aws_iam_policy_document.assume_role_policy](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source |
| [aws_partition.current](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/partition) | data source |

Expand Down Expand Up @@ -129,6 +130,7 @@ module "self_managed_node_group" {
| <a name="input_launch_template_description"></a> [launch\_template\_description](#input\_launch\_template\_description) | Description of the launch template | `string` | `null` | no |
| <a name="input_launch_template_name"></a> [launch\_template\_name](#input\_launch\_template\_name) | Launch template name - either to be created (`var.create_launch_template` = `true`) or existing (`var.create_launch_template` = `false`) | `string` | `null` | no |
| <a name="input_launch_template_tags"></a> [launch\_template\_tags](#input\_launch\_template\_tags) | A map of additional tags to add to the tag\_specifications of launch template created | `map(string)` | `{}` | no |
| <a name="input_launch_template_use_default_tags"></a> [launch\_template\_use\_default\_tags](#input\_launch\_template\_use\_default\_tags) | Enables/disables the use of provider default tags in the tag\_specifications of launch template created | `bool` | `false` | no |
| <a name="input_launch_template_use_name_prefix"></a> [launch\_template\_use\_name\_prefix](#input\_launch\_template\_use\_name\_prefix) | Determines whether to use `launch_template_name` as is or create a unique name beginning with the `launch_template_name` as the prefix | `bool` | `true` | no |
| <a name="input_launch_template_version"></a> [launch\_template\_version](#input\_launch\_template\_version) | Launch template version. Can be version number, `$Latest`, or `$Default` | `string` | `null` | no |
| <a name="input_license_specifications"></a> [license\_specifications](#input\_license\_specifications) | A list of license specifications to associate with | `map(string)` | `null` | no |
Expand Down Expand Up @@ -161,6 +163,7 @@ module "self_managed_node_group" {
| <a name="input_target_group_arns"></a> [target\_group\_arns](#input\_target\_group\_arns) | A set of `aws_alb_target_group` ARNs, for use with Application or Network Load Balancing | `list(string)` | `[]` | no |
| <a name="input_termination_policies"></a> [termination\_policies](#input\_termination\_policies) | A list of policies to decide how the instances in the Auto Scaling Group should be terminated. The allowed values are `OldestInstance`, `NewestInstance`, `OldestLaunchConfiguration`, `ClosestToNextInstanceHour`, `OldestLaunchTemplate`, `AllocationStrategy`, `Default` | `list(string)` | `null` | no |
| <a name="input_update_launch_template_default_version"></a> [update\_launch\_template\_default\_version](#input\_update\_launch\_template\_default\_version) | Whether to update Default Version each update. Conflicts with `launch_template_default_version` | `bool` | `true` | no |
| <a name="input_use_default_tags"></a> [use\_default\_tags](#input\_use\_default\_tags) | Enables/disables the use of provider default tags in the tag\_specifications of the Auto Scaling group | `bool` | `false` | no |
| <a name="input_use_mixed_instances_policy"></a> [use\_mixed\_instances\_policy](#input\_use\_mixed\_instances\_policy) | Determines whether to use a mixed instances policy in the autoscaling group or not | `bool` | `false` | no |
| <a name="input_use_name_prefix"></a> [use\_name\_prefix](#input\_use\_name\_prefix) | Determines whether to use `name` as is or create a unique name beginning with the `name` as the prefix | `bool` | `true` | no |
| <a name="input_user_data_template_path"></a> [user\_data\_template\_path](#input\_user\_data\_template\_path) | Path to a local, custom user data template file to use when rendering user data | `string` | `""` | no |
Expand Down
10 changes: 8 additions & 2 deletions modules/self-managed-node-group/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ data "aws_partition" "current" {}

data "aws_caller_identity" "current" {}

data "aws_default_tags" "current" {}

data "aws_ami" "eks_default" {
count = var.create ? 1 : 0

Expand Down Expand Up @@ -42,6 +44,8 @@ module "user_data" {

locals {
launch_template_name_int = coalesce(var.launch_template_name, "${var.name}-node-group")

launch_template_default_tags = var.launch_template_use_default_tags ? merge(data.aws_default_tags.current.tags, var.tags) : var.tags
}

resource "aws_launch_template" "this" {
Expand Down Expand Up @@ -229,7 +233,7 @@ resource "aws_launch_template" "this" {
for_each = toset(["instance", "volume", "network-interface"])
content {
resource_type = tag_specifications.key
tags = merge(var.tags, { Name = var.name }, var.launch_template_tags)
tags = merge(local.launch_template_default_tags, { Name = var.name }, var.launch_template_tags)
Copy link
Member

Choose a reason for hiding this comment

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

same comment as above - why set on launch template explicitly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

see above

}
}

Expand All @@ -255,6 +259,8 @@ locals {
launch_template_name = try(aws_launch_template.this[0].name, var.launch_template_name)
# Change order to allow users to set version priority before using defaults
launch_template_version = coalesce(var.launch_template_version, try(aws_launch_template.this[0].default_version, "$Default"))

asg_default_tags = var.use_default_tags ? merge(data.aws_default_tags.current.tags, var.tags) : var.tags
}

resource "aws_autoscaling_group" "this" {
Expand Down Expand Up @@ -385,7 +391,7 @@ resource "aws_autoscaling_group" "this" {
"kubernetes.io/cluster/${var.cluster_name}" = "owned"
"k8s.io/cluster/${var.cluster_name}" = "owned"
},
var.tags,
local.asg_default_tags,
)

content {
Expand Down
12 changes: 12 additions & 0 deletions modules/self-managed-node-group/variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,12 @@ variable "launch_template_tags" {
default = {}
}

variable "launch_template_use_default_tags" {
description = "Enables/disables the use of provider default tags in the tag_specifications of launch template created"
type = bool
default = false
}

################################################################################
# Autoscaling group
################################################################################
Expand Down Expand Up @@ -452,6 +458,12 @@ variable "delete_timeout" {
default = null
}

variable "use_default_tags" {
description = "Enables/disables the use of provider default tags in the tag_specifications of the Auto Scaling group"
type = bool
default = false
}

################################################################################
# Autoscaling group schedule
################################################################################
Expand Down
29 changes: 16 additions & 13 deletions node_groups.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,12 +272,13 @@ module "eks_managed_node_group" {
user_data_template_path = try(each.value.user_data_template_path, var.eks_managed_node_group_defaults.user_data_template_path, "")

# Launch Template
create_launch_template = try(each.value.create_launch_template, var.eks_managed_node_group_defaults.create_launch_template, true)
launch_template_name = try(each.value.launch_template_name, var.eks_managed_node_group_defaults.launch_template_name, each.key)
launch_template_use_name_prefix = try(each.value.launch_template_use_name_prefix, var.eks_managed_node_group_defaults.launch_template_use_name_prefix, true)
launch_template_version = try(each.value.launch_template_version, var.eks_managed_node_group_defaults.launch_template_version, null)
launch_template_description = try(each.value.launch_template_description, var.eks_managed_node_group_defaults.launch_template_description, "Custom launch template for ${try(each.value.name, each.key)} EKS managed node group")
launch_template_tags = try(each.value.launch_template_tags, var.eks_managed_node_group_defaults.launch_template_tags, {})
create_launch_template = try(each.value.create_launch_template, var.eks_managed_node_group_defaults.create_launch_template, true)
launch_template_name = try(each.value.launch_template_name, var.eks_managed_node_group_defaults.launch_template_name, each.key)
launch_template_use_name_prefix = try(each.value.launch_template_use_name_prefix, var.eks_managed_node_group_defaults.launch_template_use_name_prefix, true)
launch_template_version = try(each.value.launch_template_version, var.eks_managed_node_group_defaults.launch_template_version, null)
launch_template_description = try(each.value.launch_template_description, var.eks_managed_node_group_defaults.launch_template_description, "Custom launch template for ${try(each.value.name, each.key)} EKS managed node group")
launch_template_tags = try(each.value.launch_template_tags, var.eks_managed_node_group_defaults.launch_template_tags, {})
launch_template_use_default_tags = try(each.value.launch_template_use_default_tags, var.eks_managed_node_group_defaults.launch_template_use_default_tags, true)

ebs_optimized = try(each.value.ebs_optimized, var.eks_managed_node_group_defaults.ebs_optimized, null)
key_name = try(each.value.key_name, var.eks_managed_node_group_defaults.key_name, null)
Expand Down Expand Up @@ -381,7 +382,8 @@ module "self_managed_node_group" {
create_schedule = try(each.value.create_schedule, var.self_managed_node_group_defaults.create_schedule, false)
schedules = try(each.value.schedules, var.self_managed_node_group_defaults.schedules, null)

delete_timeout = try(each.value.delete_timeout, var.self_managed_node_group_defaults.delete_timeout, null)
delete_timeout = try(each.value.delete_timeout, var.self_managed_node_group_defaults.delete_timeout, null)
use_default_tags = try(each.value.use_default_tags, var.self_managed_node_group_defaults.use_default_tags, false)

# User data
platform = try(each.value.platform, var.self_managed_node_group_defaults.platform, "linux")
Expand All @@ -393,12 +395,13 @@ module "self_managed_node_group" {
user_data_template_path = try(each.value.user_data_template_path, var.self_managed_node_group_defaults.user_data_template_path, "")

# Launch Template
create_launch_template = try(each.value.create_launch_template, var.self_managed_node_group_defaults.create_launch_template, true)
launch_template_name = try(each.value.launch_template_name, var.self_managed_node_group_defaults.launch_template_name, each.key)
launch_template_use_name_prefix = try(each.value.launch_template_use_name_prefix, var.self_managed_node_group_defaults.launch_template_use_name_prefix, true)
launch_template_version = try(each.value.launch_template_version, var.self_managed_node_group_defaults.launch_template_version, null)
launch_template_description = try(each.value.launch_template_description, var.self_managed_node_group_defaults.launch_template_description, "Custom launch template for ${try(each.value.name, each.key)} self managed node group")
launch_template_tags = try(each.value.launch_template_tags, var.self_managed_node_group_defaults.launch_template_tags, {})
create_launch_template = try(each.value.create_launch_template, var.self_managed_node_group_defaults.create_launch_template, true)
launch_template_name = try(each.value.launch_template_name, var.self_managed_node_group_defaults.launch_template_name, each.key)
launch_template_use_name_prefix = try(each.value.launch_template_use_name_prefix, var.self_managed_node_group_defaults.launch_template_use_name_prefix, true)
launch_template_version = try(each.value.launch_template_version, var.self_managed_node_group_defaults.launch_template_version, null)
launch_template_description = try(each.value.launch_template_description, var.self_managed_node_group_defaults.launch_template_description, "Custom launch template for ${try(each.value.name, each.key)} self managed node group")
launch_template_tags = try(each.value.launch_template_tags, var.self_managed_node_group_defaults.launch_template_tags, {})
launch_template_use_default_tags = try(each.value.launch_template_use_default_tags, var.eks_managed_node_group_defaults.launch_template_use_default_tags, true)

ebs_optimized = try(each.value.ebs_optimized, var.self_managed_node_group_defaults.ebs_optimized, null)
ami_id = try(each.value.ami_id, var.self_managed_node_group_defaults.ami_id, "")
Expand Down