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

Converting dynamic block with local array causes problems #2637

Closed
mutahhir opened this issue Feb 14, 2023 · 6 comments
Closed

Converting dynamic block with local array causes problems #2637

mutahhir opened this issue Feb 14, 2023 · 6 comments
Assignees
Labels
bug Something isn't working feature/convert priority/important-soon High priority, to be worked on as part of our current release or the following one.
Milestone

Comments

@mutahhir
Copy link
Member

Source

# Summary: Uses 'dynamic' to create multiple 'ingress' blocks within an 'aws_security_group' resource.

# Documentation: https://www.terraform.io/docs/language/settings/index.html
terraform {
  required_version = ">= 1.0.0"
  required_providers {
    aws = {
      source  = "hashicorp/aws"
      version = "~> 3.38"
    }
  }
}

# Documentation: https://www.terraform.io/docs/language/providers/requirements.html
provider "aws" {
  region = "us-east-1"
  default_tags {
    tags = {
      cs_terraform_examples = "aws_security_group/dynamic"
    }
  }
}

# Documentation: https://www.terraform.io/docs/language/values/locals.html
locals {
  ports = [80, 443, 8080]
}

# Documentation: https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group
resource "aws_security_group" "changeme_aws_security_group_dynamic" {
  name = "changeme-aws-security-group-dynamic"

  # Documentation: https://www.terraform.io/docs/language/expressions/dynamic-blocks.html
  dynamic "ingress" {
    for_each = local.ports
    content {
      description = "changeme-aws-security-group-dynamic-ingress-${ingress.key}"
      from_port   = ingress.value
      to_port     = ingress.value
      protocol    = "tcp"
      cidr_blocks = ["0.0.0.0/0"]
    }
  }
}

Converted

The errors are included in the output, so I'm pasting them here too.

// �[91m[2023-01-26T12:19:50.402] [ERROR] default - �[39mFound a reference that is unknown: changeme-aws-security-group-dynamic-ingress-${ingress.key} has reference "ingress.key". The id was not found in ["aws","local.ports","aws_security_group.changeme_aws_security_group_dynamic","ingress"] with temporary values [].
//         Please leave a comment at https://cdk.tf/bugs/convert-expressions if you run into this issue.
// �[91m[2023-01-26T12:19:50.403] [ERROR] default - �[39mFound a reference that is unknown: ${ingress.value} has reference "ingress.value". The id was not found in ["aws","local.ports","aws_security_group.changeme_aws_security_group_dynamic","ingress"] with temporary values [].
//         Please leave a comment at https://cdk.tf/bugs/convert-expressions if you run into this issue.
// �[91m[2023-01-26T12:19:50.403] [ERROR] default - �[39mFound a reference that is unknown: ${ingress.value} has reference "ingress.value". The id was not found in ["aws","local.ports","aws_security_group.changeme_aws_security_group_dynamic","ingress"] with temporary values [].
//         Please leave a comment at https://cdk.tf/bugs/convert-expressions if you run into this issue.
// �[91m[2023-01-26T12:19:50.408] [ERROR] default - �[39mFound a reference that is unknown: changeme-aws-security-group-dynamic-ingress-${ingress.key} has reference "ingress.key". The id was not found in ["aws","local.ports","aws_security_group.changeme_aws_security_group_dynamic"] with temporary values ["ingress"].
//         Please leave a comment at https://cdk.tf/bugs/convert-expressions if you run into this issue.
// �[91m[2023-01-26T12:19:50.408] [ERROR] default - �[39mFound a reference that is unknown: ${ingress.value} has reference "ingress.value". The id was not found in ["aws","local.ports","aws_security_group.changeme_aws_security_group_dynamic"] with temporary values ["ingress"].
//         Please leave a comment at https://cdk.tf/bugs/convert-expressions if you run into this issue.
// �[91m[2023-01-26T12:19:50.408] [ERROR] default - �[39mFound a reference that is unknown: ${ingress.value} has reference "ingress.value". The id was not found in ["aws","local.ports","aws_security_group.changeme_aws_security_group_dynamic"] with temporary values ["ingress"].
//         Please leave a comment at https://cdk.tf/bugs/convert-expressions if you run into this issue.
import { Construct } from "constructs";
import { TerraformStack } from "cdktf";
/*Provider bindings are generated by running cdktf get.
See https://cdk.tf/provider-generation for more details.*/
import * as aws from "../../.gen/providers/aws";

export class MyStack extends TerraformStack {
  constructor(scope: Construct, id: string) {
    super(scope, id);

    new aws.provider.AwsProvider(this, "aws", {
      defaultTags: [
        {
          tags: {
            csTerraformExamples: "aws_security_group/dynamic",
          },
        },
      ],
      region: "us-east-1",
    });
    const ports = [80, 443, 8080];
    const awsSecurityGroupChangemeAwsSecurityGroupDynamic =
      new aws.securityGroup.SecurityGroup(
        this,
        "changeme_aws_security_group_dynamic",
        {
          ingress: [],
          name: "changeme-aws-security-group-dynamic",
        }
      );
    /*In most cases loops should be handled in the programming language context and 
    not inside of the Terraform context. If you are looping over something external, e.g. a variable or a file input
    you should consider using a for loop. If you are looping over something only known to Terraform, e.g. a result of a data source
    you need to keep this like it is.*/
    awsSecurityGroupChangemeAwsSecurityGroupDynamic.addOverride("ingress", {
      for_each: ports,
      content: [
        {
          cidr_blocks: ["0.0.0.0/0"],
          description:
            "changeme-aws-security-group-dynamic-ingress-${ingress.key}",
          from_port: "${ingress.value}",
          protocol: "tcp",
          to_port: "${ingress.value}",
        },
      ],
    });
  }
}

Another example in Python:

The same example seems to work for Typescript, but does not for Python (and possibly other languages)

        ports = [80, 443, 8080]
        aws_security_group_changeme_aws_security_group_dynamic =         aws.security_group.SecurityGroup(self, "changeme_aws_security_group_dynamic",
            ingress=[],
            name="changeme-aws-security-group-dynamic"
        )
        # In most cases loops should be handled in the programming language context and
        # not inside of the Terraform context. If you are looping over something external, e.g. a variable or a file input
        # you should consider using a for loop. If you are looping over something only known to Terraform, e.g. a result of a data source
        # you need to keep this like it is.
        aws_security_group_changeme_aws_security_group_dynamic.add_override("ingress",
            for_each=ports,
            content=[{
                "cidr_blocks": ["0.0.0.0/0"],
                "description": "changeme-aws-security-group-dynamic-ingress-${ingress.key}",
                "from_port": "${ingress.value}",
                "protocol": "tcp",
                "to_port": "${ingress.value}"
            }
            ]
        )
    Traceback (most recent call last):
      File "/Users/mutahhir/src/scratchpad/terraform-convert-examples/converted-langs/python/aws/main.py", line 66, in <module>
        aws_security_group_dynamic(app, "aws_security_group_dynamic")
      File "/Users/mutahhir/.local/share/virtualenvs/aws-CDeXujSt/lib/python3.10/site-packages/jsii/_runtime.py", line 111, in __call__
        inst = super().__call__(*args, **kwargs)
      File "/Users/mutahhir/src/scratchpad/terraform-convert-examples/converted-langs/python/aws/aws_security_group/dynamic/main.py", line 43, in __init__
        aws_security_group_changeme_aws_security_group_dynamic.add_override("ingress",
    TypeError: TerraformElement.add_override() got an unexpected keyword argument 'for_each'
# Documentation: https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group
resource "aws_security_group" "changeme_aws_security_group_dynamic" {
  name = "changeme-aws-security-group-dynamic"

  # Documentation: https://www.terraform.io/docs/language/expressions/dynamic-blocks.html
  dynamic "ingress" {
    for_each = local.ports
    content {
      description = "changeme-aws-security-group-dynamic-ingress-${ingress.key}"
      from_port   = ingress.value
      to_port     = ingress.value
      protocol    = "tcp"
      cidr_blocks = ["0.0.0.0/0"]
    }
  }
}

Solution

Converting the override to a map works well

        # In most cases loops should be handled in the programming language context and
        # not inside of the Terraform context. If you are looping over something external, e.g. a variable or a file input
        # you should consider using a for loop. If you are looping over something only known to Terraform, e.g. a result of a data source
        # you need to keep this like it is.
        aws_security_group_changeme_aws_security_group_dynamic.add_override("ingress", {
            "for_each": ports,
            "content": [{
                "cidr_blocks": ["0.0.0.0/0"],
                "description": "changeme-aws-security-group-dynamic-ingress-${ingress.key}",
                "from_port": "${ingress.value}",
                "protocol": "tcp",
                "to_port": "${ingress.value}"
            }]
        })
@xiehan xiehan added this to the 0.16 (tentative) milestone Feb 14, 2023
@xiehan xiehan added feature/convert bug Something isn't working priority/important-soon High priority, to be worked on as part of our current release or the following one. labels Feb 15, 2023
@ansgarm ansgarm self-assigned this Apr 12, 2023
@ansgarm
Copy link
Member

ansgarm commented Apr 12, 2023

Update

Since this issue was posted, we added support for iterators to convert. Therefore convert currently outputs the following Python (if ran in a TypeScript(!) project):

import constructs as constructs
import cdktf as cdktf
# Provider bindings are generated by running cdktf get.
# See https://cdk.tf/provider-generation for more details.
import ...gen.providers.aws as aws
class MyConvertedCode(constructs.Construct):
    def __init__(self, scope, name):
        super().__init__(scope, name)
        aws.provider.AwsProvider(self, "aws",
            default_tags=[AwsProviderDefaultTags(
                tags={
                    "cs_terraform_examples": "aws_security_group/dynamic"
                }
            )
            ],
            region="us-east-1"
        )
        ports = [80, 443, 8080]
        # In most cases loops should be handled in the programming language context and
        #     not inside of the Terraform context. If you are looping over something external, e.g. a variable or a file input
        #     you should consider using a for loop. If you are looping over something only known to Terraform, e.g. a result of a data source
        #     you need to keep this like it is.
        aws_security_group_changeme_aws_security_group_dynamic_dynamic_iterator0 =
        cdktf.TerraformIterator.from_list(cdktf.Token.as_any(ports))
        aws.security_group.SecurityGroup(self, "changeme_aws_security_group_dynamic",
            name="changeme-aws-security-group-dynamic",
            ingress=
            aws_security_group_changeme_aws_security_group_dynamic_dynamic_iterator0.dynamic({
                "cidr_blocks": ["0.0.0.0/0"],
                "description": "changeme-aws-security-group-dynamic-ingress-${" + aws_security_group_changeme_aws_security_group_dynamic_dynamic_iterator0.key + "}",
                "from_port": aws_security_group_changeme_aws_security_group_dynamic_dynamic_iterator0.value,
                "protocol": "tcp",
                "to_port": aws_security_group_changeme_aws_security_group_dynamic_dynamic_iterator0.value
            })
        )

which seems to be better, but returns a different error:

      File "/Users/ansgar/projects/playground/test-cdktf-convert/main.py", line 24
        aws_security_group_changeme_aws_security_group_dynamic_dynamic_iterator0 =
                                                                                  ^
    SyntaxError: invalid syntax

And after fixing that one (by removing the line break) and changing AwsProviderDefaultTags to aws.provider.AwsProviderDefaultTags the example fails with:

      File "/Users/ansgar/projects/playground/test-cdktf-convert/main.py", line 30, in __init__
        "description": "changeme-aws-security-group-dynamic-ingress-${" + aws_security_group_changeme_aws_security_group_dynamic_dynamic_iterator0.key + "}",
    TypeError: can only concatenate str (not "_LazyBaseProxy") to str

which might be an error we already track – will check.

@ansgarm
Copy link
Member

ansgarm commented Apr 12, 2023

So out of those three (new) problems mentioned, some are already covered by other issues as follows:

  1. The line break that produced invalid Python is depending on this upstream issue: jsii-rosetta: Multi-line assignment statement where function is on the right hand side breaks in Python aws/jsii#3968 (also tracking in our repo under at Convert breaking long lines into multiple causes compilation issues in python #2641)
  2. The AwsProviderDefaultTags missing an import / full namespace is not yet tracked tracked by test(rosetta): Extend submodule import test case for nested structs in submodules aws/jsii#4056
  3. The string concatenation issue is related to Conversion from Typescript template literals and concatenated strings to other languages is broken #2643 but not entirely covered by that issue (as that one is about template strings)

@ansgarm
Copy link
Member

ansgarm commented Apr 13, 2023

Update: I created an upstream "issue-PR" for # 2: aws/jsii#4056

@ansgarm
Copy link
Member

ansgarm commented Apr 13, 2023

I'd say we can close this issue now as we do have those three other issues tracking the other problems that now happen when converting the snippet which this issue is based on. What do you think, @mutahhir?
One thing we could still do, though, is creating an issue in our repo that tracks the upstream issue in JSII (bullet point # 2) – shall we?

@ansgarm
Copy link
Member

ansgarm commented Apr 13, 2023

Created #2800 to track the upstream JSII issue – closing this (as discussed offline with Mutahhir)

@ansgarm ansgarm closed this as completed Apr 13, 2023
@github-actions
Copy link
Contributor

I'm going to lock this issue 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 similar to this, 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 May 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working feature/convert priority/important-soon High priority, to be worked on as part of our current release or the following one.
Projects
None yet
Development

No branches or pull requests

3 participants