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

feat: Require users to supply OS via ami_type and not via platform which is unable to distinquish between the number of variants supported today #3068

Merged
merged 1 commit into from
Jun 13, 2024
Merged

feat: Require users to supply OS via ami_type and not via platform which is unable to distinquish between the number of variants supported today #3068

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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 .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
repos:
- repo: https://github.com/antonbabenko/pre-commit-terraform
rev: v1.90.0
rev: v1.91.0
hooks:
- id: terraform_fmt
- id: terraform_docs
Expand Down
2 changes: 1 addition & 1 deletion modules/self-managed-node-group/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ module "self_managed_node_group" {
| <a name="input_network_interfaces"></a> [network\_interfaces](#input\_network\_interfaces) | Customize network interfaces to be attached at instance boot time | `list(any)` | `[]` | no |
| <a name="input_placement"></a> [placement](#input\_placement) | The placement of the instance | `map(string)` | `{}` | no |
| <a name="input_placement_group"></a> [placement\_group](#input\_placement\_group) | The name of the placement group into which you'll launch your instances, if any | `string` | `null` | no |
| <a name="input_platform"></a> [platform](#input\_platform) | [DEPRECATED - use `ami_type` instead. Will be removed in `v21.0`] Identifies the OS platform as `bottlerocket`, `linux` (AL2), `al2023`, or `windows` | `string` | `"linux"` | no |
| <a name="input_platform"></a> [platform](#input\_platform) | [DEPRECATED - must use `ami_type` instead. Will be removed in `v21.0`] | `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` | `""` | 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` | `""` | 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 | `map(string)` | `{}` | no |
Expand Down
5 changes: 2 additions & 3 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 @@ -25,9 +25,8 @@ locals {
AL2023_x86_64_STANDARD = "al2023"
AL2023_ARM_64_STANDARD = "al2023"
}
# Try to use `ami_type` first, but fall back to current, default behavior
# TODO - will be removed in v21.0
user_data_type = try(local.ami_type_to_user_data_type[var.ami_type], var.platform)

user_data_type = local.ami_type_to_user_data_type[var.ami_type]
Copy link
Member

Choose a reason for hiding this comment

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

LGTM, but this change is probably "feat" not "fix".


# Map the AMI type to the respective SSM param path
ami_type_to_ssm_param = {
Expand Down
10 changes: 8 additions & 2 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 @@ -10,10 +10,16 @@ variable "tags" {
default = {}
}

# tflint-ignore: terraform_unused_declarations
variable "platform" {
description = "[DEPRECATED - use `ami_type` instead. Will be removed in `v21.0`] Identifies the OS platform as `bottlerocket`, `linux` (AL2), `al2023`, or `windows`"
description = "[DEPRECATED - must use `ami_type` instead. Will be removed in `v21.0`]"
type = string
default = "linux"
default = null

validation {
condition = var.platform == null
error_message = "`platform` is no longer valid due to the number of OS choices. Please provide an [`ami_type`](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-eks-nodegroup.html#cfn-eks-nodegroup-amitype) instead."
}
}

################################################################################
Expand Down
2 changes: 1 addition & 1 deletion node_groups.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ module "self_managed_node_group" {
autoscaling_group_tags = try(each.value.autoscaling_group_tags, var.self_managed_node_group_defaults.autoscaling_group_tags, {})

# User data
platform = try(each.value.platform, var.self_managed_node_group_defaults.platform, "linux")
platform = try(each.value.platform, var.self_managed_node_group_defaults.platform, null)
# TODO - update this when `var.platform` is removed in v21.0
ami_type = try(each.value.ami_type, var.self_managed_node_group_defaults.ami_type, "AL2_x86_64")
cluster_endpoint = try(time_sleep.this[0].triggers["cluster_endpoint"], "")
Expand Down