fix: Use splat syntax for cluster name to avoid (known after apply) in managed node groups

[barryib]

PR o'clock

Description

After some tests, I've noticed that all inputs of the null_data_source are already know.

  # module.eks.data.null_data_source.node_groups[0] will be read during apply
  # (config refers to values not yet known)
 <= data "null_data_source" "node_groups"  {
      + has_computed_default = (known after apply)
      + id                   = (known after apply)
      + inputs               = {
          + "aws_auth"        = "kube-system/aws-auth"
          + "cluster_name"    = "test-eks-uIVr3XSp"
          + "role_CNI_Policy" = "test-eks-uIVr3XSp20200505210731845200000005-20200505210733402400000008"
          + "role_Container"  = "test-eks-uIVr3XSp20200505210731845200000005-20200505210733399800000007"
          + "role_NodePolicy" = "test-eks-uIVr3XSp20200505210731845200000005-20200505210733441800000009"
        }
      + outputs              = (known after apply)
      + random               = (known after apply)
    }

The unknown values are computed one like random, id, etc. and those will be read only during apply unless we remove the count. In that way Terraform will always read datas before apply and avoid the (known after apply) issue. Sounds like a Terraform magic again 🤷‍♂️

Resolves #843 #862

Checklist

• CI tests are passing
• README.md has been updated after any changes to variables and outputs. See https://github.com/terraform-aws-modules/terraform-aws-eks/#doc-generation

[dpiddockcmp]

It works. And fixes the issues 👍

I didn't know that TF 0.12 supported depends_on for variables. I thought this hadn't been implemented yet and the bug traffic I found had it potentially scheduled for 0.13. I would have gone that route if I'd known it existed when first implementing this module 😉

[barryib]

It works. And fixes the issues 👍

I didn't know that TF 0.12 supported depends_on for variables. I thought this hadn't been implemented yet and the bug traffic I found had it potentially scheduled for 0.13. I would have gone that route if I'd known it existed when first implementing this module 😉

@dpiddockcmp So should we go with #867 or continue on this PR ?

[barryib]

It works. And fixes the issues 👍
I didn't know that TF 0.12 supported depends_on for variables. I thought this hadn't been implemented yet and the bug traffic I found had it potentially scheduled for 0.13. I would have gone that route if I'd known it existed when first implementing this module 😉

@dpiddockcmp So should we go with #867 or continue on this PR ?

Let's stick with this. It sounds to work without notable change.

[wwentland]

Just wanted to mention that I find the approach in #867 to be much more explicit and easier to understand. While it is true that the PR consists of more notable changes to the make-up of the module, it might be preferred as it relies on less "magic" than the approach here.

That being said, I have tested the changes in here and can confirm that they address the issues at hand, but I am unsure as to why that is exactly.

[dpiddockcmp]

This does work and with minimal changes. Ignoring the confusion over why it works and breaking my mental model of Terraform 😁

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.