Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
1 of 2 tasks
blimmer opened this issue Dec 1, 2021 · 3 comments
Open
1 of 2 tasks

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

blimmer opened this issue Dec 1, 2021 · 3 comments
Labels
@aws-cdk/core Related to core CDK functionality effort/large Large work item – several weeks of effort feature-request A feature should be added or improved. p2

Comments

@blimmer
Copy link
Contributor

blimmer commented Dec 1, 2021

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:

  1. 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: Garbage Collection for Assets aws-cdk-rfcs#64.
  2. 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

/**
* bucketPrefix to use while storing S3 Assets
*
* @default - DefaultStackSynthesizer.DEFAULT_FILE_ASSET_PREFIX
*/
readonly bucketPrefix?: string;
/**
* A prefix to use while tagging and uploading Docker images to ECR.
*
* This does not add any separators - the source hash will be appended to
* this string directly.
*
* @default - DefaultStackSynthesizer.DEFAULT_DOCKER_ASSET_PREFIX
*/
readonly dockerTagPrefix?: string;

The defaults are currently blank strings

/**
* Default file asset prefix
*/
public static readonly DEFAULT_FILE_ASSET_PREFIX = '';
/**
* Default Docker asset prefix
*/
public static readonly DEFAULT_DOCKER_ASSET_PREFIX = '';

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

  • I may be able to implement this feature request
  • This feature might incur a breaking change
@blimmer blimmer added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Dec 1, 2021
@github-actions github-actions bot added the @aws-cdk/core Related to core CDK functionality label Dec 1, 2021
@rix0rrr rix0rrr added effort/large Large work item – several weeks of effort p2 and removed needs-triage This issue or PR still needs to be triaged. labels Dec 24, 2021
@rix0rrr rix0rrr removed their assignment Dec 24, 2021
@omegayal
Copy link

omegayal commented Feb 17, 2022

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
Copy link

rehanvdm commented Mar 2, 2022

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
Copy link
Contributor

comcalvi commented Jan 25, 2023

Related: #9628

@aws aws deleted a comment from github-actions bot Jan 25, 2023
@comcalvi comcalvi reopened this Jan 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/core Related to core CDK functionality effort/large Large work item – several weeks of effort feature-request A feature should be added or improved. p2
Projects
None yet
Development

No branches or pull requests

5 participants