hashicorp / terraform-cdk

Define infrastructure resources using programming constructs and provision them using HashiCorp Terraform
https://www.terraform.io/cdktf
Mozilla Public License 2.0
4.79k stars 442 forks source link

Inconsistent behaviour of `dependsOn` if set via initial resource config vs. via `resource.dependsOn` #2334

Open mihok opened 1 year ago

mihok commented 1 year ago

Community Note

Description

Currently tokens are forced into strings, and dependsOn or addOverride resolves these down to templated strings, which are not allowed. Where a block of code such as this:

    const account = new ServiceAccount(scope, `${name}-account`, {
      accountId: name,
      description: "Generated by Terraform",
    });

    const key = new ServiceAccountKey(scope, `${name}-key`, {
      serviceAccountId: account.id,
      publicKeyType: "TYPE_RAW_PUBLIC_KEY",
    });

    key.dependsOn = [ account.id ];

Which results in the following error from terraform:

│ Error: Invalid expression
│ 
│   on cdk.tf.json line 102, in resource.google_service_account_key.logging-robot-key.depends_on:
│  102:           "${google_service_account.logging-robot-account.id}",
│ 
│ A single static variable reference is required: only attribute access and
│ indexing with constant keys. No calculations, function calls, template
│ expressions, etc are allowed here.

There needs to be a simple way for people to reference the object without templated strings.

Ideally, this might be as simple as having some kind of toUntemplatedString (ugly, not set on the name of the function/property)

So the above code would look something along the lines of:

    const account = new ServiceAccount(scope, `${name}-account`, {
      accountId: name,
      description: "Generated by Terraform",
    });

    const key = new ServiceAccountKey(scope, `${name}-key`, {
      serviceAccountId: account.id,
      publicKeyType: "TYPE_RAW_PUBLIC_KEY",
    });

    key.dependsOn = [ account.idRaw ];

Which result in a cdk.tf.json:


 "depends_on": [
   "google_service_account.logging-robot-account.id"
 ]

Happy to do some of the implementation if it helps!

ansgarm commented 1 year ago

Hi @mihok 👋

As you're using account.id in the key, Terraform should be able to automatically figure out the dependency between those two resources (you don't need to specify the dependency). That said, if you would need to define the dependency, usually you would pass the whole resource into the dependsOn property: key.dependsOn = [account] (like done in HCL)

mihok commented 1 year ago

Thanks for looking at this @ansgarm, I reran the codebase with the instructions you have given me but Typescript suggests the value for dependsOn must be an array of strings, which is why I created this bug

ansgarm commented 1 year ago

Ah, got it. That's something we should check then 🤔 I came up with this because of this example which passes the resource construct: https://github.com/hashicorp/terraform-cdk/blob/217b7398fcc470d618f16f1095c3a88f69d6baeb/examples/typescript/google/main.ts#L46 (we run examples in CI, so it should fail if TypeScript does not like that)

ansgarm commented 1 year ago

Okay, there seems to be an inconsistency between dependencies passed via the initial config (like in the example I posted) and setting the property later on.

This extra bit happens if dependsOn is defined in the config: https://github.com/hashicorp/terraform-cdk/blob/217b7398fcc470d618f16f1095c3a88f69d6baeb/packages/cdktf/lib/terraform-resource.ts#L95-L99

So, the proper way in your case would be to call:

import { dependable } from "cdktf";
// ...
key.dependsOn = [dependable(account)]

But I agree that this could use some more docs (at least inline docstrings on the dependsOn property; ideally some section in our docs).

jsteinich commented 1 year ago

Ideally setting after would also work with passing the entire object rather than needing to turn into a string. Can certainly improve documentation either way, but not requiring extra steps when late setting seems preferable.

edwardch-ibotta commented 2 weeks ago

is dependable something exposed in non-type script versions of the cdk?