ASM module not deferring data source read to apply time

[bharathkkb]

TL;DR

When GKE cluster and ASM module are created in the same config and GKE module inputs are passed to ASM module, the data source read for cluster is not deferred to apply time as project_id, cluster name and location are known at plan time. This results in an error as cluster does not exist yet.

 Error: Invalid template interpolation value
│ 
│   on ../../../modules/asm/hub.tf line 21, in resource "google_gke_hub_membership" "membership":
│   21:   membership_id = "${data.google_container_cluster.asm.name}-membership"
│     ├────────────────
│     │ data.google_container_cluster.asm.name is null
│ 
│ The expression result is null. Cannot include a null value in a string template.
╵
╷
│ Error: Invalid template interpolation value
│ 
│   on ../../../modules/asm/hub.tf line 24, in resource "google_gke_hub_membership" "membership":
│   24:       resource_link = "//container.googleapis.com/${data.google_container_cluster.asm.id}"
│     ├────────────────
│     │ data.google_container_cluster.asm.id is null
│ 
│ The expression result is null. Cannot include a null value in a string template.

Our current tests did not catch this as a random_string was passed as cluster name deferring the read to apply time

terraform-google-kubernetes-engine/test/fixtures/simple_zonal_with_asm/example.tf

Line 25 in 774fc12

cluster_name_suffix = "-${random_string.suffix.result}"

Module depends on also throws an error due to gcloud module

│ Error: Invalid count argument
│ 
│   on .terraform/modules/example.asm.cpr/main.tf line 57, in resource "random_id" "cache":
│   57:   count = (!local.skip_download) ? 1 : 0

We can look into using a computed attribute to defer this to apply time.

Expected behavior

No response

Observed behavior

No response

Terraform Configuration

https://github.com/terraform-google-modules/terraform-google-kubernetes-engine/tree/master/examples/simple_zonal_with_asm with a static cluster name

Terraform Version

v1.0.5

Additional information

No response

[Monkeyanator]

Just chatted offline with @bharathkkb. First step will be to document the workaround using a computed attribute for the cluster name to defer evaluating the data source until apply time. I'll put a note in the module docs but an example would be to use:

locals {
  cluster_name = element(split("/", module.gke.cluster_id), length(split("/", module.gke.cluster_id)) - 1)
}

and then pass that to the ASM module as the cluster name.

Longer term there are a few options. One would be for the ASM module to take in the cluster ID instead of cluster name (a breaking change to the ASM module). Another option would be to change the GKE module name output to be a computed attribute (which would make the ASM module data source wait until apply time).

[morgante]

We should just change the GKE module name output to be a computed attribute. It's an anti-pattern for outputs to just pass variables back in.