Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(lib): Add DynamicListTerraformIterator with support for lists or sets with objects that have properties that are only known at apply time #3273

Merged
merged 33 commits into from
Nov 30, 2023

Conversation

ansgarm
Copy link
Member

@ansgarm ansgarm commented Nov 17, 2023

Note! This PR is based on #3272

Problem

Terraform does not support iterating over lists without turning them into a set first. Sets that contain computed values need to be applied using -target in order for Terraform to know about these values before a plan is made. Using -target and applying changes in two passes is not desirable. However, there’s a workaround by turning them into maps which is described below.

Workaround by using Maps

A workaround for this case is to turn the set of objects into a map with keys that are based on arguments that are known during the plan stage. This is done for example in the HCL docs snippet for the aws_acm_certificate_validation resource:

image

In the snippet above, the set of objects stored in domain_validation_options is converted into a map with the domain_name of each object as the key.

If one instead tries to directly use domain_validation_options, the following error is printed (hinting at using -target, as described in the problem section above):

image

Solution

Allow the user to pass the attribute name to be used as the key in the map and convert the set/list into a map using a for expression.

Closes #2178
Related to #2001, #1393, #430 (some already closed with workarounds)

Copy link
Contributor

@DanielMSchmidt DanielMSchmidt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the approach overall and to my knowledge it looks good 👍 There are a few things missing for the overall picture from my point of view, let me know what you think:

  • Documentation section about this (detailing the use-case & usage)
  • Hcl2cdk needs to be made aware of this, ideally we would detect dynamic blocks / for_each being used from a complex list and then add the correct fromComplexList invocation. This will allow us to remove yet another escape hatch 🎉

packages/cdktf/lib/terraform-iterator.ts Outdated Show resolved Hide resolved
packages/cdktf/lib/terraform-iterator.ts Show resolved Hide resolved
test/typescript/iterators/test.ts Outdated Show resolved Hide resolved
@ansgarm ansgarm requested a review from a team as a code owner November 23, 2023 14:36
@ansgarm ansgarm changed the base branch from main to proper-chained-iterators November 23, 2023 14:48
@ansgarm ansgarm marked this pull request as draft November 23, 2023 14:49
@ansgarm
Copy link
Member Author

ansgarm commented Nov 30, 2023

Merging even though C# example fails on the pluckProperty call, which is a problem that's not directly related to this branch and something that @DanielMSchmidt is looking into.

@ansgarm ansgarm added this pull request to the merge queue Nov 30, 2023
Merged via the queue into main with commit 53bec67 Nov 30, 2023
237 checks passed
@ansgarm ansgarm deleted the complex-list-iterators branch November 30, 2023 16:15
Copy link
Contributor

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've 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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AcmCertificateValidation: support complex for expressions
3 participants