Skip to content

Allow overriding of prefix separator in self-managed node groups #3600

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

Closed
james-menzies wants to merge 4 commits into from
Closed

Allow overriding of prefix separator in self-managed node groups #3600

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e305873
allow overriding of prefix separator in self-managed node groups
james-menzies Nov 25, 2025
ac9756e
create separate input for prefix_separator
james-menzies Nov 25, 2025
fe9310c
update README
james-menzies Nov 25, 2025
24a2c0c
fix prefix separator propagation logic
james-menzies Nov 26, 2025
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
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop

Large diffs are not rendered by default.

1 change: 1 addition & 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 @@ -172,6 +172,7 @@ module "self_managed_node_group" {
| <a name="input_placement_group"></a> [placement\_group](#input\_placement\_group) | The name of the placement group into which you'll launch your instances | `string` | `null` | no |
| <a name="input_post_bootstrap_user_data"></a> [post\_bootstrap\_user\_data](#input\_post\_bootstrap\_user\_data) | User data that is appended to the user data script after of the EKS bootstrap script. Not used when `ami_type` = `BOTTLEROCKET_*` | `string` | `null` | no |
| <a name="input_pre_bootstrap_user_data"></a> [pre\_bootstrap\_user\_data](#input\_pre\_bootstrap\_user\_data) | User data that is injected into the user data script ahead of the EKS bootstrap script. Not used when `ami_type` = `BOTTLEROCKET_*` | `string` | `null` | no |
| <a name="input_prefix_separator"></a> [prefix\_separator](#input\_prefix\_separator) | The separator to use between the prefix and the generated timestamp for resource names | `string` | `"-"` | no |
| <a name="input_private_dns_name_options"></a> [private\_dns\_name\_options](#input\_private\_dns\_name\_options) | The options for the instance hostname. The default values are inherited from the subnet | <pre>object({<br/> enable_resource_name_dns_aaaa_record = optional(bool)<br/> enable_resource_name_dns_a_record = optional(bool)<br/> hostname_type = optional(string)<br/> })</pre> | `null` | no |
| <a name="input_protect_from_scale_in"></a> [protect\_from\_scale\_in](#input\_protect\_from\_scale\_in) | Allows setting instance protection. The autoscaling group will not select instances with this setting for termination during scale in events | `bool` | `false` | no |
| <a name="input_ram_disk_id"></a> [ram\_disk\_id](#input\_ram\_disk\_id) | The ID of the ram disk | `string` | `null` | no |
Expand Down
12 changes: 6 additions & 6 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 @@ -380,7 +380,7 @@ resource "aws_launch_template" "this" {
}

name = var.launch_template_use_name_prefix ? null : local.launch_template_name
name_prefix = var.launch_template_use_name_prefix ? "${local.launch_template_name}-" : null
name_prefix = var.launch_template_use_name_prefix ? "${local.launch_template_name}${var.prefix_separator}" : null

dynamic "network_interfaces" {
for_each = length(local.network_interfaces) > 0 ? local.network_interfaces : []
Expand Down Expand Up @@ -749,7 +749,7 @@ resource "aws_autoscaling_group" "this" {
}

name = var.use_name_prefix ? null : var.name
name_prefix = var.use_name_prefix ? "${var.name}-" : null
name_prefix = var.use_name_prefix ? "${var.name}${var.prefix_separator}" : null
placement_group = var.placement_group
protect_from_scale_in = var.protect_from_scale_in
suspended_processes = var.suspended_processes
Expand Down Expand Up @@ -836,7 +836,7 @@ resource "aws_iam_role" "this" {
count = local.create_iam_instance_profile ? 1 : 0

name = var.iam_role_use_name_prefix ? null : local.iam_role_name
name_prefix = var.iam_role_use_name_prefix ? "${local.iam_role_name}-" : null
name_prefix = var.iam_role_use_name_prefix ? "${local.iam_role_name}${var.prefix_separator}" : null
Copy link
Author

@james-menzies james-menzies Nov 26, 2025

Choose a reason for hiding this comment

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

I replaced all instances where I felt the separator was hard-coded in this file for consistency, but am happy to take some out if appropriate.

path = var.iam_role_path
description = var.iam_role_description

Expand Down Expand Up @@ -875,7 +875,7 @@ resource "aws_iam_instance_profile" "this" {
role = aws_iam_role.this[0].name

name = var.iam_role_use_name_prefix ? null : local.iam_role_name
name_prefix = var.iam_role_use_name_prefix ? "${local.iam_role_name}-" : null
name_prefix = var.iam_role_use_name_prefix ? "${local.iam_role_name}${var.prefix_separator}" : null
path = var.iam_role_path

tags = merge(var.tags, var.iam_role_tags)
Expand Down Expand Up @@ -942,7 +942,7 @@ resource "aws_iam_role_policy" "this" {
count = local.create_iam_role_policy ? 1 : 0

name = var.iam_role_use_name_prefix ? null : local.iam_role_name
name_prefix = var.iam_role_use_name_prefix ? "${local.iam_role_name}-" : null
name_prefix = var.iam_role_use_name_prefix ? "${local.iam_role_name}${var.prefix_separator}" : null
policy = data.aws_iam_policy_document.role[0].json
role = aws_iam_role.this[0].id
}
Expand Down Expand Up @@ -1044,7 +1044,7 @@ resource "aws_security_group" "this" {
region = var.region

name = var.security_group_use_name_prefix ? null : local.security_group_name
name_prefix = var.security_group_use_name_prefix ? "${local.security_group_name}-" : null
name_prefix = var.security_group_use_name_prefix ? "${local.security_group_name}${var.prefix_separator}" : null
description = var.security_group_description
vpc_id = data.aws_subnet.this[0].vpc_id

Expand Down
6 changes: 6 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 @@ -5,6 +5,12 @@ variable "create" {
nullable = false
}

variable "prefix_separator" {
description = "The separator to use between the prefix and the generated timestamp for resource names"
type = string
default = "-"
}

variable "tags" {
description = "A map of tags to add to all resources"
type = map(string)
Expand Down
5 changes: 3 additions & 2 deletions node_groups.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -415,8 +415,9 @@ module "self_managed_node_group" {
# Autoscaling Group
create_autoscaling_group = each.value.create_autoscaling_group

name = coalesce(each.value.name, each.key)
use_name_prefix = each.value.use_name_prefix
name = coalesce(each.value.name, each.key)
use_name_prefix = each.value.use_name_prefix
prefix_separator = each.value.prefix_separator != null ? each.value.prefix_separator : "-"
Copy link
Author

@james-menzies james-menzies Nov 26, 2025

Choose a reason for hiding this comment

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

I opted for this strategy rather than coalesce so that an empty string is not clobbered by the default.


availability_zones = each.value.availability_zones
subnet_ids = coalesce(each.value.subnet_ids, var.subnet_ids)
Expand Down
1 change: 1 addition & 0 deletions variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -808,6 +808,7 @@ variable "self_managed_node_groups" {
create_autoscaling_group = optional(bool)
name = optional(string) # Will fall back to map key
use_name_prefix = optional(bool)
prefix_separator = optional(string)
availability_zones = optional(list(string))
subnet_ids = optional(list(string))
min_size = optional(number)
Expand Down
Loading