"cdk migrate" - feedback on the first release

[evgenyka]

This feature enables users to migrate AWS CloudFormation templates, previously deployed CloudFormation stacks, or resources created outside of Infrastructure as Code (IaC) into a CDK application.

https://aws.amazon.com/blogs/devops/announcing-cdk-migrate-a-single-command-to-migrate-to-the-aws-cdk/

[mrpackethead]

This is a pretty useful bit of tooling.

For a first use case, I am using it to convert some 'vendor' supplied cloudformation templates to cdk. Mostly it did a good job, but it did'nt produce a working cdk app.

Some initial observations;

(1) This parameter ( and any number type parameters ) did not translate to a number type, they were strings.

for example

"InstanceDriveSize": {
   "Type": "Number",
   "Default": 100,
   "ConstraintDescription": "Must be between 100 and 1024 GB.",
   "Description": "Provide size of the root Drive in GB. Default is 100 GB. Minimum is 100 GB. Maximum is 1024 GB.",
   "MaxValue": 1024,
   "MinValue": 100
  },

What I got was a property in the StackProps that looked like this.

readonly instanceDriveSize?: string;

(2) The way it creates Propertys for the stack, appears to be by picking out any Outputs it saw in the cloudformation. I guess thats a good starting point.

(3) The way it applies the default values, using the spread operator, while 'correct' and functional, might be quite confusing, its not something that you often see. It seems to be the general convention in cdk that the default would be set at the resource..

(4) Conditions are confusing.. 'Native' Cdk doestn really have a special set of parameters called conditions.. Tyically, we'd use an if statement, or if you want to be compact the ? : While it often results in more lines of code if statements are easier to follow than ? :

(5) .addDepedency often had the wrong types passed. for example

Argument of type 'CfnWaitCondition | undefined' is not assignable to parameter of type 'CfnResource'. Type 'undefined' is not assignable to type 'CfnResource'.ts(2345)

 

[TheRealAmazonKendra]

(1) Definitely a bug. We will investigate this.
(2) This is correct. Just out of curiosity is there a different set of values you'd like to see also converted into these fields?
(3 and 4) I definitely don't disagree with you here. We went with the option that would create the most functioning apps straight out of the box, but it's not all that pretty. I'm investigating seeing if we have enough information via the schema and provided template to refine these a little more.
(5) This is an area where we do a best guess for type, though, it looks like we didn't take into account that the resource in question might be undefined. We should probably add an assertion that this must be defined.

Thank you for the feedback here. We'll add these items to our follow up tasks.

[mrpackethead]

Another quirky one.. with conditonal things... Its adding empty code

if (configureElbAssociation != null) {
    }

[iph]

😱 -- Do you have a sample template that this code is produced from?

[mrpackethead]

try this

https://arcgisstore.s3.amazonaws.com/1120/templates/arcgis-server-ha.template.json

[TheRealAmazonKendra]

Do you mind if I add this template as a testcase in our codebase when I add the fix?

[mrpackethead]

This template produces some code that has a casing problem. AutoscalingReplacingUpdate shoudl be in camel case, its in Pascal, and again with the property WillReplace.

Also noticed that the value for WillReplace is text 'true', but it Should be boolean,

"AutoScalingGroup": {
   "Type": "AWS::AutoScaling::AutoScalingGroup",
   "Properties": {
    "MaxSize": {
     "Ref": "ServerInstances"
    },
    "MinSize": {
     "Ref": "ServerInstances"
    },
    "Cooldown": "300",
    "HealthCheckGracePeriod": 3600,
    "HealthCheckType": "EC2",
    "LaunchTemplate": {
     "LaunchTemplateId": {
      "Ref": "EC2InstanceLaunchTemplate"
     },
     "Version": {
      "Fn::GetAtt": [
       "EC2InstanceLaunchTemplate",
       "LatestVersionNumber"
      ]
     }
    },
    "Tags": [
     {
      "Key": "arcgisenterprise:cloudformation:logical-id",
      "PropagateAtLaunch": true,
      "Value": "AutoScalingGroup"
     },
     {
      "Key": "arcgisenterprise:cloudformation:stack-id",
      "PropagateAtLaunch": true,
      "Value": {
       "Ref": "AWS::StackId"
      }
     },
     {
      "Key": "arcgisenterprise:cloudformation:stack-name",
      "PropagateAtLaunch": true,
      "Value": {
       "Ref": "AWS::StackName"
      }
     },
     {
      "Key": "arcgisenterprise:cloudformation:template-name",
      "PropagateAtLaunch": true,
      "Value": "arcgis-server-ha.template.json"
     },
     {
      "Key": "arcgisenterprise:cloudformation:template-provider",
      "PropagateAtLaunch": true,
      "Value": "Esri"
     },
     {
      "Key": "Name",
      "PropagateAtLaunch": true,
      "Value": {
       "Fn::Join": [
        "",
        [
         {
          "Ref": "AWS::StackName"
         },
         "-Server-Node"
        ]
       ]
      }
     }
    ],
    "VPCZoneIdentifier": [
     {
      "Ref": "Subnet1"
     },
     {
      "Ref": "Subnet2"
     }
    ]
   },
   "UpdatePolicy": {
    "AutoScalingReplacingUpdate": {
     "WillReplace": "true"
    }
   }
  },

this produces

const autoScalingGroup = new autoscaling.CfnAutoScalingGroup(this, 'AutoScalingGroup', {
      maxSize: String(props.serverInstances!),
      minSize: String(props.serverInstances!),
      cooldown: '300',
      healthCheckGracePeriod: 3600,
      healthCheckType: 'EC2',
      launchTemplate: {
        launchTemplateId: ec2InstanceLaunchTemplate.ref,
        version: ec2InstanceLaunchTemplate.attrLatestVersionNumber,
      },
      vpcZoneIdentifier: [
        props.subnet1!,
        props.subnet2!,
      ],`
    });
    autoScalingGroup.cfnOptions.updatePolicy = {
      AutoScalingReplacingUpdate: {
        WillReplace: 'true',
      },
    };

[TheRealAmazonKendra]

This one is on our radar for a fix. I'm hoping to get it out by next week's release.