Confusing Deprecations: IDeploymentEnvironment / Stack.addDockerImageAsset / DockerImageAsset.repositoryName

[rrrix]

DockerImageAssetOptions.repositoryName is deprecated, instructing to override Stack.addDockerImageAsset.

aws-cdk/packages/@aws-cdk/aws-ecr-assets/lib/image-asset.ts

Lines 20 to 22 in de5e406

	* @deprecated to control the location of docker image assets, please override
	* `Stack.addDockerImageAsset`. this feature will be removed in future
	* releases.

Interface property DockerImageAssetSource.repositoryName is also deprecated with a confusing message.

aws-cdk/packages/@aws-cdk/core/lib/assets.ts

Line 153 in de5e406

* @deprecated repository name should be specified at the environment-level and not at the image level

However Stack.addDockerImageAsset() is also deprecated with a confusing deprecation warning:

aws-cdk/packages/@aws-cdk/core/lib/stack.ts

Lines 605 to 606 in de5e406

	* @deprecated Use `stack.synthesizer.addDockerImageAsset()` if you are calling,
	* and a different `IDeploymentEnvironment` class if you are implementing.

I assume IDeploymentEnvironment is an interface I need to implement. However I cannot find this interface in the CDK source code anywhere and can only find references to it in the JSDoc's mentioning it in the @aws-cdk/core/lib/stack.ts file.

I did find one other mention in an (Outdated!) PR: #6366 (comment)

From reviewing reviewing #6366 it seems that that IDeploymentEnvironment was renamed to IStackSynthesizer.

So... that means if I want to change what my docker image repository name is, I have to implement an entire stack synthesizer class? I just want a custom repository name for each image... :)

Reproduction Steps

import { Repository } from '@aws-cdk/aws-ecr';
import { DockerImageAsset } from '@aws-cdk/aws-ecr-assets';
import { Construct, DockerImageAssetLocation, DockerImageAssetSource, Stack, StackProps } from '@aws-cdk/core';

class MyStack extends Stack {
  // deprecated
  addDockerImageAsset(asset: DockerImageAssetSource): DockerImageAssetLocation {
    return super.addDockerImageAsset(asset);
  }
  constructor(scope: Construct, id: string, props?: StackProps) {
    super(scope, id, props);

    const repo = new Repository(this, 'repo', {
      repositoryName: 'my-repo',
    });
    new DockerImageAsset(this, 'containerImageAsset', {
      directory: './containerImageSrc',
      // deprecated
      repositoryName: repo.repositoryName,
    });
  }
}

Error Log

[Warning at /ecr/apiAsset] DockerImageAsset.repositoryName is deprecated. Override "core.Stack.addDockerImageAsset" to control asset locations

Environment

• CLI Version : 1.45.0 (build 0cfab15)
• Framework Version: 1.45.0
• Node.js Version: v14.4.0
• OS : MacOS 10.15.5
• Language (Version): TypeScript (3.9.3)

Other

I need to create multiple ECR repositories, one repo per "application image", ideally with a custom (human friendly) repository name. I need this for several reasons:

• Security (access control via IAM and Resource Policies)
• User access (docker pull repo-url/image:tag)
• External (non-CDK) tools access to image URI's
• Ability to use the latest tag for each of my images & repositories (docker pull repo-url/image:latest)
• Managing individual Repository Tags for Billing and IAM Security

We have many different images for various applications, and pushing them all into aws-cdk/assets makes it impossible to distinguish one application image from another. This also means that I cannot use the latest tag (since only one image can have a unique tag in a repository).

I need to be able to reference / find my images from outside of my CDK Application / Constructs, so that users and external (non-CDK) tools can easily find and pull these images.

With a single ECR repository, I can no longer effectively manage security on my individual images, since that's typically done per-repository, not per individual image hash/tag. If all of my images for different containers are in aws-cdk/assets, that makes granting access all-or-nothing.

Regarding the new Stack Synthesizer changes, why should my Docker Image Repository Name be specified at the environment-level and not at the image level? I think specifying a Container Registry at the environment level makes more sense (but not an image repository.)

Lastly, how do I properly override the repository name at the environment level? There's no documentation to do this AFAIK, although from reading through @aws-cdk/core/lib/stack-synthesizers/legacy.ts I think I can set the context key assets-ecr-repository-name (which defaults to aws-cdk/assets) to a custom value. I think I could get this to sufficiently solve my use case, with an per-stack context value and one image/repository per stack.

I feel like this whole thing might all be a CDK-specific anti-pattern (even though it's the norm for most container workflows). Should I avoid using the CDK and exclusively manage my container images completely outside of the CDK for this use case?

---

This is 🐛 Bug Report and a 📕 documentation issue

[eladb]

Transferring to @rix0rrr

[rix0rrr]

At the moment the CDK comes with 2 asset systems:

• The legacy one (currently still the default), where you get to specify a repositoryName per asset, and the CLI will create and push to whatever ECR repository you name.

• The new one (will become the default in the future), where a single ECR repository will be created by doing cdk bootstrap and all images will be pushed into it. The CLI will not create the repository any more, it must already exist. IIRC this was done to limit the permissions required for deployments. @eladb, can you help me remember why we chose to do it this way?

If you want to deviate from this pattern then yes, you will have to implement your own subclass of IStackSynthesizer and use that.

[rrrix]

Thanks @rix0rrr for the explanation. I'm also curious as to the background / reasoning of this mechanism (given my requirements above).

Is the CDK Context Key assets-ecr-repository-name also deprecated? Or is this an undocumented "internal-use-only" Context Key that end-users shouldn't be using anyways?

aws-cdk/packages/@aws-cdk/core/lib/stack-synthesizers/legacy.ts

Lines 18 to 24 in b0286e9

	/**
	* This allows users to work around the fact that the ECR repository is
	* (currently) not configurable by setting this context key to their desired
	* repository name. The CLI will auto-create this ECR repository if it's not
	* already created.
	*/
	const ASSETS_ECR_REPOSITORY_NAME_OVERRIDE_CONTEXT_KEY = 'assets-ecr-repository-name';

Overriding the repository name with a simple context key would be much more convenient than having to subclass IStackSynthesizer and all of the dragons that go with building and maintaining such a thing. I may be overestimating the effort needed to subclass the new stack synthesizer though.

For today, until it's clear how to do this the "right way" and/or the new stack synthesizer is turned on by default, I temporarily have moved away from using CDK ECR Image Assets and setup a simple multi-step script to:

1. Create my ECR Repositories (using @aws-cdk/aws-ecr) via cdk deploy ecr-stack1 ecr-stack2
2. Simple shell script to several docker build -t my-image:latest . && docker push 123456789012.dkr.ecr.region.amazonaws.com/my-image:latest
3. Continue with the rest of the cdk deploy stack1 stack 2 ...

The ugly downside is that it's no longer a single command to deploy everything, and I have to manage my docker builds outside of my CDK App.

[rix0rrr]

As I mentioned in the other issue, I think a better way to go would be to replicate the container from a "temporary" repository to a controlled repository using a custom resource.

I get that this might feel a little wasteful, but given the use cases we're targeting with the provided built-ins that is probably the best you can do for now.

[gregorypierce]

If one already has a repository in the stack, wouldn't it be agreeable to allow that to be passed to the DockerImageAsset? In many cases I just need that asset to land in an already created repository. In my case I have an environment that created its own ECR Repository and I want those images to go there and NOT into a default cdk/asset repository as the Repository that I want those images to land in will have other ways of scanning images and such.

If those images go elsewhere, a project's resources bleed out into a general location where they really shouldn't be. Is it at least agreeable to let one pass in the repository where those resources should land? I don't need CDK to create it, but if I have to later fish the resources out of cdk/assets - that has a bunch of other implications.

        var dockerImageAsset = new ecr_assets.DockerImageAsset( this, props.lifecycle.name + "_docker_image", {
            directory: props.dockerFilePath,
            repositoryName: props.lifecycle.deploymentEnvironment.repository.repositoryName
        });

could then easily become

        var dockerImageAsset = new ecr_assets.DockerImageAsset( this, props.lifecycle.name + "_docker_image", {
            directory: props.dockerFilePath,
            repository: props.lifecycle.deploymentEnvironment.repository
        });

and this would be no different than how we pass Vpcs and other resources around today across stacks and such.

[p8a]

I second this, if you have multiple applications/stacks (like in a "dev" environment for example) everything will be merged into one place and since the original image name and tags are not kept it's hard to keep track of what's obsolete and needs to go vs what is still needed.

[okonon]

@rix0rrr @SomayaB any feedback on @gregorypierce comments? I am running into same issue were i would like to use ECR created in my stack for lambda deployments. We have multiple developers working on a lot of separate lambda microservices projects and do not want to bundle all 20+ of them in a single CDK project and rather manage them in their own sub project so to speak and deploy using cli.

[rix0rrr]

We consciously decided not to do this, and this is the wrong ticket for it. You can reopen a new feature request and we may consider it given enough upvotes, but right now this is not really on the docket.

We recommend you don't care about ECR repositories the same way you don't care about the file asset S3 bucket.

---

If you need dev and prod resources in the same account separated (keeping in mind mixing dev and prod resources in a single account is strongly disrecommended!), you can bootstrap multiple sets of resources with different qualifiers to keep them separate.

[github-actions]

⚠️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.

[luxaritas]

@rix0rrr Before I open a new issue that doesn't make sense: While I understand this, does this leave any recommended way of cleaning up unused resources? It's seems like currently there is no way to even know what each image even is (there's no human readable name or tag). I guess #6692 and/or #2663 is needed, and hopefully that is resolved before repositoryName is finally removed? Or am I missing something and completely off base? Just want to make sure I understand the current situation.

[farajfarook]

Readable names and tags are required to maintain a clean solution. Call me OCD, but cleaner solution means one can login to the console and figure out issues or roll back stuff faster when sh*t hits the fan. It's stupid to have all the images pushed into a single ECR repo with tags being some hash which is not human readable. Seriously who takes these decisions. At least can someone please explain the reason behind this. What am I missing here?
Also I feel like I'm using an Apple product here. :D

[wchaws]

@gregorypierce @okonon I wrote a cdk-ecr-deployment https://github.com/wchaws/cdk-ecr-deployment. See if this can help you.
With this you can replicate your ECR to another location

const image = new DockerImageAsset(this, 'CDKDockerImage', {
  directory: path.join(__dirname, 'docker'),
});

new ecrDeploy.ECRDeployment(this, 'DeployDockerImage', {
  src: new ecrDeploy.DockerImageName(image.imageUri),
  dest: new ecrDeploy.DockerImageName(`${cdk.Aws.ACCOUNT_ID}.dkr.ecr.us-west-2.amazonaws.com/test:nginx`),
});

[JessieWadman]

This is a bit of an ill thought through design decision, because "We recommend you don't care about ECR repositories the same way you don't care about the file asset S3 bucket" is problematic. You -do- need to care about them in a different way.

Consider when you have Lambdas that use Docker images, and you are deploying two stacks of such to the same account.

Eventually you will run out of storage in your ECR, so you apply a lifecycle policy to remove old images.
Now, you deploy endlessly to stack one, but not to stack two. Old images start being evicted, and eventually also those that stack two is CURRENTLY USING. Now you are in a bad place. I know this because I'm currently dealing with it :')
This is because the lifecycle policies of ECR do not care if the image is in use or not.

The solution (in case anyone else runs into this) is to docker pull hello-world, tag it with the missing images and manually push to ecr just to be able complete stack deployments and rollbacks, like so:
docker pull hello-world
docker tag hello-world {account}.dkr.ecr.{region}.amazonaws.com/aws-cdk/assets:{missing-tag}
docker push {account}.dkr.ecr.{region}.amazonaws.com/aws-cdk/assets:{missing-tag}

For each image that was evicted but still needed. Now you can do your rollback/deployment or whatever again 🥇
If you are deploying multiple stacks to the same account, and using lifecycle policy in ECR, I would recommend scheduling a deployment once a month (or so) for each stack, just to make sure the images that are in use do not get evicted due to age or count.

Being able to (at the very least) specify repository for each stack, would eliminate the problem.