terraform-cdk icon indicating copy to clipboard operation
terraform-cdk copied to clipboard

lifecycle: replaceTriggeredBy should not be a string interpolation

Open wayneslabs opened this issue 11 months ago • 3 comments

Expected Behavior

When a value is provided for lifecycle.replaceTriggeredBy, it should not use string interpolation in the final terraform code.

For example:

"lifecycle": {
    "replace_triggered_by": [
      "google_firebaserules_ruleset.MainRuleSet"
    ]
  }

Actual Behavior

Currently, it creates a string interpolation

For example:

"lifecycle": {
    "replace_triggered_by": [
      "${google_firebaserules_ruleset.MainRuleSet}"
    ]
  }

This throws an error:

╷
│ Error: Invalid character
│
│   on  line 33:
│   (source code not available)
│
│ This character is not used within the language.
╵

╷
│ Error: Invalid expression
│
│   on  line 33:
│   (source code not available)
│
│ Expected the start of an expression, but found an invalid expression token.

Steps to Reproduce

Create a resource with replaceTriggeredBy

lifecycle: { replaceTriggeredBy: [mainRuleSet] }

I got the error while following the steps for creating firebase rules, which requires replaceTriggeredBy: docs

Versions

language: typescript cdktf-cli: 0.20.4 node: v20.10.0 cdktf: 0.20.4 constructs: 10.3.0 jsii: null terraform: 1.7.4 arch: x64 os: MacOS Monterey

Providers

No response

Gist

No response

Possible Solutions

No response

Workarounds

Currently, it can be solved by manually constructing it, as noted at: issue comment

release.addOverride("lifecycle.replace_triggered_by", [
  mainRuleSet.terraformResourceType +
    "." +
    mainRuleSet.friendlyUniqueId,
]);

Anything Else?

No response

References

  • #3196
  • #2082
  • https://github.com/hashicorp/terraform-cdk/pull/3322

Help Wanted

  • [ ] I'm interested in contributing a fix myself

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

wayneslabs avatar Mar 03 '24 16:03 wayneslabs

replaceTriggeredBy should behave the same way like dependsOn: https://github.com/hashicorp/terraform-cdk/blob/4fd0a65ae698e999d3ba2590257fd770793dcf36/packages/cdktf/lib/terraform-resource.ts#L160-L164

vs. https://github.com/hashicorp/terraform-cdk/blob/4fd0a65ae698e999d3ba2590257fd770793dcf36/packages/cdktf/lib/terraform-resource.ts#L78-L88

So would changing from return x.fqdn; to return dependable(x); fix the issue? As a workaround, I'm doing it like this in my code:

import { TerraformResource, dependable } from 'cdktf';

const monitoredResources: TerraformResource = [];

// Add relevant resources to array

new ApiGatewayDeployment(this, 'service_api_deployment', {
  restApiId: apiGateway.id,
  lifecycle: {
    replaceTriggeredBy: monitoredResources.map(dependable),
  },
});

DJAlPee avatar Jun 26 '24 08:06 DJAlPee

I've confirmed that using dependable does work. It's also cleaner than the previous workaround I mentioned in the description above.

wayneslabs avatar Jul 23 '24 03:07 wayneslabs

With Python, i've used Token.as_any(<my_resource_instance>) -- is this intended as the drop in since dependable is not available in python from what I can tell?

kharigardner avatar Oct 02 '24 16:10 kharigardner