Error if `null` value assigned to `cluster_config_mode`, or if `cluster_config_mode.enabled = false`

[lorengordon]

Description

There is one logic error around the cluster_config_mode, and another error around the API interaction.

The logic error is that if a user assigns a null value to cluster_config_mode, then the length() function throws an error, here:

terraform-aws-eks/main.tf

Line 56 in 1bfc10a

for_each = length(var.cluster_compute_config) > 0 ? [var.cluster_compute_config] : []

╷
│ Error: Invalid function argument
│
│   on .terraform/modules/eks.cluster/main.tf line 56, in resource "aws_eks_cluster" "this":
│   56:     for_each = length(var.cluster_compute_config) > 0 ? [var.cluster_compute_config] : []
│     ├────────────────
│     │ while calling length(value)
│     │ var.cluster_compute_config is null
│
│ Invalid value for "value" parameter: argument must not be null.
╵

Since the default value is already an empty map, that error is most easily fixed by adding nullable = false to the variable definition.

---

The second error, around the API interaction, is that if you only set cluster_config_mode.enabled = false, then the cluster will plan fine, but then fail to create with an error reported by the API during the apply:

│ Error: creating EKS Cluster (test-minimum-inputs-kuneic): operation error EKS: CreateCluster, https response error StatusCode: 400, RequestID: 573c664d-68c6-463c-b117-ba12722ebe96, InvalidParameterException: For EKS Auto Mode, please ensure that all required configs, including computeConfig, kubernetesNetworkConfig, and blockStorage are all either fully enabled or fully disabled.

In one sense, I think the API is doing the wrong thing, since we're disabling auto mode and the module already empties all the other mentioned configs when disabling the compute config. But probably the easiest way to resolve the problem is to change the condition on the dynamic block to just be local.auto_mode_enabled, instead of testing the length() of the input...

Note that this was previously reported, but the issue was closed without resolution due to lack of details, #3268. Hopefully I've provided enough to pursue/accept a fix.

I'll have a PR ready shortly to resolve this report...

• ✋ I have searched the open/closed issues and my issue is not listed.

Versions

• Module version [Required]: 20.33.1

• Terraform version: 1.10.5

• Provider version(s):

+ provider registry.terraform.io/hashicorp/aws v5.87.0
+ provider registry.terraform.io/hashicorp/cloudinit v2.3.5
+ provider registry.terraform.io/hashicorp/helm v2.17.0
+ provider registry.terraform.io/hashicorp/kubernetes v2.35.1
+ provider registry.terraform.io/hashicorp/null v3.2.3
+ provider registry.terraform.io/hashicorp/random v3.6.3
+ provider registry.terraform.io/hashicorp/time v0.12.1
+ provider registry.terraform.io/hashicorp/tls v4.0.6

Reproduction Code [Required]

  cluster_compute_config = null

  cluster_compute_config = {
    enabled = false
  }

Expected behavior

1. Expected to be able to pass a null value to cluster_compute_config to get the default value.

2. Expected to be able to disable Auto Mode by setting cluster_compute_config.enabled = false.

Actual behavior

Both scenarios fail, for reasons detailed above.

[bryantbiggs]

null is not a valid value for cluster_compute_config. if you want to enable auto mode, you provide values via cluster_compute_config, if you don't, you don't provide anything for cluster_compute_config

[lorengordon]

Yes, I get that. Modern Terraform typically uses null to represent "no config". When writing a module around this module, adding typing to improve user feedback, encompassing some business logic, letting the user choose whether to enable auto mode or not, etc, then we run into typing constraints due to the use of {} as the default value in this module.

  cluster_compute_config = var.cluster_compute_config != null ? var.cluster_compute_config : {}

That will error because the var.cluster_compute_config is an object type and object types can't be converted to an empty map.

As far as I can tell, and I tried a lot of variations in the wrapping module unsuccessfully, the only way to fix it is actually here in the upstream module, so it supports a null value correctly. Then we can simply pass the variable object through without extra conditions.

  cluster_compute_config = var.cluster_compute_config

[lorengordon]

I'd be happy to submit a separate PR that fixes just that one bug.

Even better, I'd love to actually add typing to the variable here upstream, that would be amazing, and it would also simplify other logic in this module. Requires less of that try() logic, since the type ensures the attributes are present (and may even set a default value).

[lorengordon]

I'd be happy to submit a separate PR that fixes just that one bug.

Even better, I'd love to actually add typing to the variable here upstream, that would be amazing, and it would also simplify other logic in this module. Requires less of that try() logic, since the type ensures the attributes are present (and may even set a default value).

I went ahead and submitted a PR adding the type info, just in case, #3309

[github-actions]

This issue has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this issue will be closed in 10 days

[lorengordon]

Not stale

[bryantbiggs]

going to close as duplicate of #3273 - we can revisit once it and the upstream issues are resolved