(core): default docker and file asset prefixes to the stack's ID

blimmer commented 2 years ago

Description

Today, when assets like Docker Images and Files are produced, the filename is a hash of the file contents. The hash behavior is great because it allows CloudFormation to skip re-uploading assets when the changes are "no-op"s. However, these assets are not very human-friendly because they're not identifiable in any way.

> aws s3 ls  s3://cdk-hnb659fds-assets-<account-number>/us-west-2/assets/
2021-12-01 11:15:51      11364 2f9df4a79dca4c4e8df01a0dc0631a475e7bb18edaaa9b2cbc294dec28947f54.json
2021-12-01 11:15:46       5669 9c59dd635d29a0401122e4e8d53e27dcd88d83c6c8d5e7a071628449e4ce96f3.zip
2021-12-01 11:15:49     671822 9fe17f7f2e00bbf5ea4262003fd602fd18fa231b9953b1d483ff8a117aa79864.zip
2021-12-01 11:15:50       8738 b120b13d9d868c7622e7db1b68bae4c0f82ffd0227b8c15f2cef38e186ff3827.zip
2021-12-01 11:15:48      21920 c691172cdeefa2c91b5a2907f9d81118e47597634943344795f1a844192dd49c.zip
... and thousands more ...

Use Case

Ther are a few distinct use-cases I can think of:

Garbage Collection Support. As a long-time CDK user, I've experienced the S3 Bucket and ECR repos created by S3 become extremely unmanageable. Because the files are not identifiable, I can't really go in and easily clean up (manually or via Lifecycle Rules) the assets. There's an RFC right now to support this, so this change might also make it easier to implement gc in the future: https://github.com/aws/aws-cdk-rfcs/issues/64.
Human-Readable Assets. By making assets more human-readable, it's easier to spot-check/validate that what CDK is doing is "correct". For instance, consider an asset called c83185b9d9d867148abd3f2c7c1f2d8aa3f39effb705b56ae99d114e015bf729.py today. By just looking at this, I have no idea what it could be. However, if the Stack ID was prepended, I would automatically have a better idea of what that file is MyStackId-c83185b9d9d867148abd3f2c7c1f2d8aa3f39effb705b56ae99d114e015bf729.py

Proposed Solution

Currently, there are two properties exposed by the DefaultStackSynthesizer that make this change fairly straightforward, bucketPrefix and dockerTagPrefix https://github.com/aws/aws-cdk/blob/534babde886646b6cd01f8799e3b4ed6db221263/packages/%40aws-cdk/core/lib/stack-synthesizers/default-synthesizer.ts#L168-L183

The defaults are currently blank strings https://github.com/aws/aws-cdk/blob/534babde886646b6cd01f8799e3b4ed6db221263/packages/%40aws-cdk/core/lib/stack-synthesizers/default-synthesizer.ts#L250-L257

I propose that, instead of blank strings, we use the stack name and some separator (e.g., - or _) as the default prefix for Docker Images and File assets.

You can get this behavior today by overriding these values like so

  constructor(scope: cdk.Construct, id: string, props?: cdk.StackProps = {}) {
    super(scope, id, {
      synthesizer: new DefaultStackSynthesizer({
        dockerTagPrefix: `${id}-`,
        bucketPrefix: `${id}-`,
      }),
      ...props,
    });
  }

but I think there's an argument to make this the default in CDK.

Other information

There are a number of other issues that relate to this idea:

Though this wouldn't solve these issues directly, it could make it easier to implement these types of improvements in the future.

Acknowledge

[X] I may be able to implement this feature request
[ ] This feature might incur a breaking change

omegayal commented 2 years ago

Is it possible to change dockerTagPrefix after the stack instance is created? The use case is: Our stack will create two container images (one for arm64, the other for x86_64). However, we can't find a suitable ECR lifecycle policy to guarantee that the latest version of both images will be retained if they share the same prefix. So, it will be useful if the dockerTagPrefix can be changed (maybe by a dedicated method?) after the stack instance is created. For example:

  constructor(scope: cdk.Construct, id: string, props?: cdk.StackProps = {}) {
    // init the stack instance with prefix 'arm64-'
    super(scope, id, {
      synthesizer: new DefaultStackSynthesizer({
        dockerTagPrefix: `arm64-`
      }),
      ...props,
    });

    //Some logic to create the arm64 image. The image uploaded to ECR should have prefix 'arm64-'
    ...

    //Change the dockerTagPrefix to 'x86-' with dedicated method, say, updateDockerTagPrefix
    updateDockerTagPrefix('x86-');

    //Some logic to create the x86 image. The image uploaded to ECR should have prefix 'x86-'
    ...

  }

rehanvdm commented 2 years ago

Agree, the prefix must be on a container image basis, not on the stack. An alternative would be to add the prefix

const container = taskDefinition.addContainer(containerName, {
        containerName: containerName,
        image: ecs.ContainerImage.fromAsset("../", {
            dockerTagPrefix: containerName
            file: "server.dockerfile",
            buildArgs: {  }
        }),

comcalvi commented 1 year ago

Related: #9628

aws / aws-cdk