[bugfix]random_pet uses kubernetes_version as a string

[miguelzenteno]

what

• Fixing a bug for random_pet keepers
• Update required terraform version to handle strcontains

why

• random_pet uses kubernetes_version as a string, but the variable is a list of strings, so extracting the first element is enough.

references

• closes Required terraform version #188
• https://github.com/hashicorp/terraform/blob/v1.5/CHANGELOG.md#150-june-12-2023

[Gowiem]

Couple questions here -- Thanks!

[Gowiem]

@miguelzenteno you mention the following:

Update required terraform version to handle strcontains

But you don't use strcontains anywhere in the code. Why is that necessary? Is that used elsewhere?

This would require a major version bump of this module and I don't see why that would be necessary here. Please expand on why this is necessary.

[miguelzenteno]

@Gowiem here it is:
https://github.com/cloudposse/terraform-aws-eks-node-group/blob/main/userdata.tf#L62
error can be seen in the issue here: #188

This should have been caught in the bump from v2 to v3, since the code was introduced during that major version bump. that's why I'm thinking a minor would be enough.

[Gowiem]

@miguelzenteno I'm surprised this isn't a wider issue if this is necessary. Can you explain what was happening before that requires this change? Was it an error? Or was the random_pet not re-creating as you changed the value of var.kubernetes_version? Apologies, but I want to understand why we haven't seen this crop up until now.

[miguelzenteno]

well, I guess no one is setting replace_node_group_on_version_update to true but us.

replace_node_group_on_version_update is set to false by default.

other than that, version should be a string, and kubernetes_version is a list(string), so you get this error:

│ Error: Incorrect attribute value type
│ 
│   on .terraform/modules/eks_node_group_3.eks_node_group/main.tf line 123, in resource "random_pet" "cbd":
│  123:   keepers = merge(
│  124:     {
│  125:       node_role_arn  = local.ng.node_role_arn
│  126:       subnet_ids     = join(",", local.ng.subnet_ids)
│  127:       instance_types = join(",", local.ng.instance_types)
│  128:       ami_type       = local.ng.ami_type
│  129:       capacity_type  = local.ng.capacity_type
│  130:       launch_template_id = local.launch_template_configured || !local.immediately_apply_lt_changes ? local.launch_template_id : (
│  131:         # If we want changes to the generated launch template to be applied immediately, keep the settings
│  132:         jsonencode(local.launch_template_config)
│  133:       )
│  134:     },
│  135:     # If `var.replace_node_group_on_version_update` is set to `true`, the Node Groups will be replaced instead of updated in-place
│  136:     var.replace_node_group_on_version_update ?
│  137:     {
│  138:       version = var.kubernetes_version
│  139:     } : {}
│  140:   )
│     ├────────────────
│     │ local.immediately_apply_lt_changes is true
│     │ local.launch_template_config is object with 13 attributes
│     │ local.launch_template_configured is false
│     │ local.launch_template_id is "lt-xxxxxxx"
│     │ local.ng.ami_type is "AL2_x86_64"
│     │ local.ng.capacity_type is "ON_DEMAND"
│     │ local.ng.instance_types is list of string with 1 element
│     │ local.ng.node_role_arn is "arn:aws:iam::xxxxx:role/xxxx"
│     │ local.ng.subnet_ids is list of string with 1 element
│     │ var.kubernetes_version is list of string with 1 element
│     │ var.replace_node_group_on_version_update is true
│ 
│ Inappropriate value for attribute "keepers": element "version": string required.

I have test the change and it doesn't affect the rest of the module, it's just that part.

Jeremy (@Nuru) has the same usage here: https://github.com/cloudposse/terraform-aws-eks-node-group/blob/main/main.tf#L9

so I think it's a simple bug.

[Nuru]

@miguelzenteno Thank you for this PR.

@Gowiem Thank you for your review.

I'm asking both of you to stop work on this for the moment. I am working on a superseding PR that will fix a few other problems, too.

Reopen this tomorrow if I haven't released a fix by then.