refactor: Encapsulate overriding of logicalId in a single place

[akash1810]

What does this change?

A resource's logicalId maps to the name of the created resource.

For example, take this CloudFormation YAML for an ASG:

DataNodeAutoScalingGroup:
  Type: AWS::AutoScaling::AutoScalingGroup
  Properties:
    AvailabilityZones: !GetAZs ""
    LaunchConfigurationName: !Ref DataNodeLaunchConfig
    MinSize: 3
    MaxSize: 4
    Cooldown: 180
    HealthCheckType: EC2
    HealthCheckGracePeriod: 300
    VPCZoneIdentifier: !Ref PrivateVpcSubnets
    Tags:
    - { Key: Stack, Value: !Ref Stack, PropagateAtLaunch: true }
    - { Key: App, Value: "elk-es-data", PropagateAtLaunch: true }
    - { Key: Stage, Value: !Ref Stage, PropagateAtLaunch: true }
    - { Key: ElasticSearchCluster, Value: !Ref ClusterName, PropagateAtLaunch: true }

This creates an ASG called DataNodeAutoScalingGroup. This pattern is widely used in YAML definitions of CloudFormation as YAML files are typically hand-written.

CDK changes this as it auto-generates the logicalIds, as there one will gain more understanding about the resource from looking at the CDK template.

In order to enable a no-op migration between a YAML stack a CDK stack, we allow the logicalId to be overridden.
Our constructs seem to follow different rules regarding when to override the logicalId of a CloudFormation resource.

For example, we have this:

if (props.overrideId || (scope.migratedFromCloudFormation && props.overrideId !== false))

And this:

if (props.overrideId) {

This change aims to bring consistency and simplicity across our constructs by applying a universal rule that,
migratedFromCloudFormation on the GuStack must be true and existingLogicalId to be set for the logicalId to be overridden.

If these conditions are not met, we let CDK auto-generate the logicalId.

It's worth noting that this doesn't implement the change on the constructs, it's simply to define the behaviour.
This is mainly to keep the diff down - we'll migrate the constructs in follow-up PRs.

Usage will roughly look like this:

const MyComponentProps extends GuMigratingResource { }

class MyComponent extends SomeGuConstruct {
  constructor(scope: GuStack, id: string, props: MyComponentProps) {
    super(scope, id, props);
    GuMigratingResource.setLogicalId(this, scope, props);
  }
}

Does this change require changes to existing projects or CDK CLI?

No.

How to test

See added tests.

How can we measure success?

Consistency in behaviour across constructs.

Have we considered potential risks?

n/a

[jacobwinch]

Looks good to me 👍

more changes have been made

[akash1810]

@stephengeller @jacobwinch I've re-worked and re-worded this to make the logic more assertive, principally migratedFromCloudFormation has to be set - the function is greatly simplified and is more readable as a result. I'm hoping the commit makes sense.

A review would be welcomed.

[jacobwinch]

Much clearer 👍

[jacobwinch]

Great work - thanks for taking the time to document and simplify this 💯

[akash1810]

Now requires #388.