aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.65k stars 3.91k forks source link

[dms] Cfn lowercases the replicationSubnetGroupIdentifier on creation but CDK passes the same string without lowercasing. #11385

Closed ahammond closed 3 years ago

ahammond commented 3 years ago

While using Cfn level constructs to generate a DMS solution, we had an issue with the replicationSubnetGroupIdentifier being implicitly lower-cased by CloudFormation but then when we referenced it elsewhere, CDK did not also lower-case the value.

Reproduction Steps

Set your stackName to something with mixed case.

    this.subnetGroup = new dms.CfnReplicationSubnetGroup(this, 'DmsSubnetGroup', {
      // If you might be mixing cases, you're going to want to force this to lowercase...
      replicationSubnetGroupIdentifier: `${scope.stackName}-subnet-group`,
      replicationSubnetGroupDescription: `${scope.stackName} Subnet Group`,
      subnetIds: scope.vpc.privateSubnets.map((s): string => {
        return s.subnetId;
      }),
    });
    new cdk.CfnOutput(this, 'TheDmsSubnetGroupArn', { value: this.subnetGroup.ref });

    this.instance = new dms.CfnReplicationInstance(this, 'Instance', {
      replicationInstanceIdentifier: `${scope.stackName}-lims-instance`,
      replicationInstanceClass: 'dms.t2.micro', // WRITEME: do we need this configurable?
      kmsKeyId: this.kmsKey.keyId,
      // Cfn will .toLowerCase this, and CDK won't do that for you.
      replicationSubnetGroupIdentifier: this.subnetGroup.replicationSubnetGroupIdentifier,
      vpcSecurityGroupIds: [sg.securityGroupId],
      publiclyAccessible: false,
    });
    this.instance.addDependsOn(this.subnetGroup);

What did you expect to happen?

I expected replicationSubnetGroupIdentifier: this.subnetGroup.replicationSubnetGroupIdentifier to provide a working reference.

What actually happened?

The SubnetGroup resource is created with all lower-case names. The Instance resource is passed a replicationSubnetGroupIdentifier with mixed case. The Instance resource then fails to create.

Environment


This is :bug: Bug Report

NetaNir commented 3 years ago

Can you add the synthesized template snippet?

ahammond commented 3 years ago
    "DmsSubnetGroup": {
      "Type": "AWS::DMS::ReplicationSubnetGroup",
      "Properties": {
        "ReplicationSubnetGroupDescription": "undefined-LimsPostgresql Subnet Group",
        "SubnetIds": [
          "subnet-07e443ebf7f21b2b8",
          "subnet-07886d168c80a5a3d",
          "subnet-06bb07f21861778e3"
        ],
        "ReplicationSubnetGroupIdentifier": "undefined-LimsPostgresql-subnet-group",
        "Tags": [
          {
            "Key": "Accounting",
            "Value": "Engineering"
          },
          {
            "Key": "Environment",
            "Value": "platform-development"
          },
          {
            "Key": "Service",
            "Value": "hodor"
          }
        ]
      },
      "Metadata": {
        "aws:cdk:path": "undefined-LimsPostgresql/undefined-LimsExtractor/DmsSubnetGroup"
      }
    },
    "Instance": {
      "Type": "AWS::DMS::ReplicationInstance",
      "Properties": {
        "ReplicationInstanceClass": "dms.t2.micro",
        "KmsKeyId": {
          "Ref": "Key961B73FD"
        },
        "PubliclyAccessible": false,
        "ReplicationInstanceIdentifier": "undefined-LimsPostgresql-lims-instance",
        "ReplicationSubnetGroupIdentifier": "undefined-LimsPostgresql-subnet-group",
        "Tags": [
          {
            "Key": "Accounting",
            "Value": "Engineering"
          },
          {
            "Key": "Environment",
            "Value": "platform-development"
          },
          {
            "Key": "Service",
            "Value": "hodor"
          }
        ],
        "VpcSecurityGroupIds": [
          {
            "Fn::GetAtt": [
              "DmsSecurityGroup6AA2E5B4",
              "GroupId"
            ]
          }
        ]
      },
      "DependsOn": [
        "DmsSubnetGroup"
      ],
      "Metadata": {
        "aws:cdk:path": "undefined-LimsPostgresql/undefined-LimsExtractor/Instance"
      }
    },

When instantiated, the actual ReplicationSubnetGroupIdentifier turns out to be undefined-limspostgresql-subnet-group.

NetaNir commented 3 years ago

Im assuming that the service is the one that creates the resource with lower case and it's not actually cloudformation, what happens when you create the same resource from the console?

Since this is an L1 there is no way to manipulate the return value, the value you provide will be returned. We can achieve this in the L2. I'll add it to the L2 tracking issue

ahammond commented 3 years ago

@NetaNir I think that it would make sense to throw a warning if there is mixed-case input at L1. That would have saved me a lot of frustration. :)

NetaNir commented 3 years ago

What do you mean a mix case input type? The value you provided to that property is the value returned.

By mix you mean that the downstream service created the resource with different casing?

ahammond commented 3 years ago

@NetaNir when I run that template, the DMS Subnet Group gets created. Something lowercases the name on creation. Then it fails to create the Instance because the instance is referencing a Subnet Group using a name with capitalization, but the actual name it should be using is all lowercase now.

NetaNir commented 3 years ago

Yes. the service does that. I created a dms subnet group from the aws console and named it using lower and upper case letters - the created subnet group name was lower cased. Since you are using the L1, which is basically using the CloudFormation resource with the niceties of code, there isn't anything we can do to address this in the L1 level, as they are auto generated. In the future L2 we can definitely add lower casting/verifications.

ahammond commented 3 years ago

@NetaNir so... should I instead be submitting a bug/feature request to the CloudFormation team?

NetaNir commented 3 years ago

What you could do is use the subnetGroup Ref:

    this.instance = new dms.CfnReplicationInstance(this, 'Instance', {
      replicationInstanceIdentifier: `${scope.stackName}-lims-instance`,
      replicationInstanceClass: 'dms.t2.micro',
      // Cfn will .toLowerCase this, and CDK won't do that for you.
      replicationSubnetGroupIdentifier: this.subnetGroup.ref,
      vpcSecurityGroupIds: [sg.securityGroupId],
      publiclyAccessible: false,
    });

The Ref is a value resolved by CloudFormation at deploy time, from DMS docs:

When you pass the logical ID of this resource to the intrinsic Ref function, Ref returns the name of the replication subnet group, such as mystack-myrepsubnetgroup-0a12bc456789de0fg.

Using Ref will also implicitly create the dependency between the subnetGroup and the instance, so you remove the line that adds it explicitly:

this.instance.addDependsOn(this.subnetGroup);
github-actions[bot] commented 3 years ago

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one. If you wish to keep having a conversation with other community members under this issue feel free to do so.