cdktf: construct name should be unique per resource type [same as terraform]

[shinebayar-g]

Description

Currently Construct naming experience is not consistent with Terraform. Consider the following example:

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

        new GoogleProvider(this, "dev", {
            project: "foo",
        });

        const vpc = new ComputeNetwork(this, "main", {
            name: "main",
            routingMode: "REGIONAL",
            autoCreateSubnetworks: false,
        });

        new ComputeRoute(this, "main", {
            name: "egress-internet",
            description: "route through IGW to access internet",
            destRange: "0.0.0.0/0",
            network: vpc.id,
            nextHopGateway: "default-internet-gateway",
        });
    }
}

Above code will give an error:

throw new Error(`There is already a Construct with name '${childName}' in ${typeName}${name.length > 0 ? ' [' + name + ']' : ''}`);
Error: There is already a Construct with name 'main' in DevStack [dev]

Ideally resource names shouldn't include resource types. In Terraform, we just name it as google_compute_network.main and google_compute_route.main as described in https://developer.hashicorp.com/terraform/language/resources/syntax#resource-syntax

Because of this we're forced to duplicate the resource type in the name to get around the error.

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

        new GoogleProvider(this, "dev", {
            project: "foo",
        });

        const vpc = new ComputeNetwork(this, "main-vpc", {
            name: "main",
            routingMode: "REGIONAL",
            autoCreateSubnetworks: false,
        });

        new ComputeRoute(this, "main-route", {
            name: "egress-internet",
            description: "route through IGW to access internet",
            destRange: "0.0.0.0/0",
            network: vpc.id,
            nextHopGateway: "default-internet-gateway",
        });
    }
}

which ultimately leaks to the terraform resource names:

     Terraform will perform the following actions:
dev    # google_compute_network.main-vpc (main-vpc) will be created
       + resource "google_compute_network" "main-vpc" {
           + auto_create_subnetworks                   = false
           + delete_default_routes_on_create           = false
           + gateway_ipv4                              = (known after apply)
           + id                                        = (known after apply)
           ...

       # google_compute_route.main-route (main-route) will be created
       + resource "google_compute_route" "main-route" {
           + description            = "route through IGW to access internet"
           + dest_range             = "0.0.0.0/0"
           + id                     = (known after apply)
           + name                   = "egress-internet"
           + network                = (known after apply)
           ...

Resources declared inside Constructs even have random suffix attached to them which makes this constraint even more meaningless.

References

https://developer.hashicorp.com/terraform/language/resources/syntax#resource-syntax

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

[ansgarm]

Hi @shinebayar-g 👋

Thank you for raising this. I agree from a Terraform point of view – unfortunately, we can't control this, as the underlying construct package (which is used by all CDKs), makes no distinction between types of constructs and requires a unique name within each scope.

However, you can work around this by using .overrideLocalId():

new RandomProvider(this, "random");

new Password(this, "password", { length: 16 });
const pet = new Pet(this, "pet-password", {});
pet.overrideLogicalId("password");

produces:

"resource": {
    "random_password": {
      "password": {
        "//": {
          "metadata": {
            "path": "test-override/password",
            "uniqueId": "password"
          }
        },
        "length": 16
      }
    },
    "random_pet": {
      "password": {
        "//": {
          "metadata": {
            "path": "test-override/pet-password",
            "uniqueId": "password"
          }
        }
      }
    }
  },

[DanielMSchmidt]

If you feel strongly about keeping the terraform naming conventions in place in your CDKTF application you can extend the existing resources:

import { S3Bucket, S3BucketConfig } from "./.gen/providers/aws/s3-bucket"

class BetterS3Bucket extends S3Bucket {
  constructor(scope: Construct, name: string, config: S3BucketConfig) {
    super(scope, `${S3Bucket.tfResourceType}-${name}`, config);
    this.overrideLogicalId(name)
  }
}

For this class the same constraints as in Terraform would apply.

[shinebayar-g]

Thanks for the explanation and workarounds.

as the underlying construct package (which is used by all CDKs), makes no distinction between types of constructs and requires a unique name within each scope.

It looks like every resource is extended from cdktf.TerraformResource class and has tfResourceType property. Maybe using this information it might be doable. I hope this limitation will be improved in the future.

[StefanTheWiz]

If you feel strongly about keeping the terraform naming conventions in place in your CDKTF application

Err... we do feel strongly about this. CDKTF will be a tough sell in any organisation unless it saves us time writing code.

Thank you for raising this. I agree from a Terraform point of view – unfortunately, we can't control this, as the underlying construct package (which is used by all CDKs), makes no distinction between types of constructs and requires a unique name within each scope.

I don't fully agree with the "we can't control this" part.

Child classes of Construct that are defined in the cdktf package have access to the element type so one of these classes that's closest to the Construct class in terms of hierarchy could prefix the id with the element/resource type before passing it to the parent class, no?

In terms of practical solutions for people who might have wasted time looking into a nice hack for this, I just gave up and used uuid for now 😄

import { S3BucketServerSideEncryptionConfigurationA } from "@cdktf/provider-aws/lib/s3-bucket-server-side-encryption-configuration";
import { v4 as uuid } from "uuid";

new S3BucketServerSideEncryptionConfigurationA(this, uuid(), {
      bucket: "my-bucket",
      rule: [{ applyServerSideEncryptionByDefault: { sseAlgorithm: "aws:kms" } }],
    }).overrideLogicalId(id);

Code brevity is really appreciated, especially for long resource names like S3BucketServerSideEncryptionConfiguration...