Cannot reference EcrImage by digest instead of tag

[statik]

ecs.ContainerImage.fromEcrRepository() does not allow constructing an image reference by digest instead of tag (although the comments reference the ECR support for specifying an image by digest).

Reproduction Steps

import cdk = require("@aws-cdk/core");
import * as ecr from "@aws-cdk/aws-ecr";
import * as ecs from "@aws-cdk/aws-ecs";
const myApp = new cdk.App();
export class DemoStack extends cdk.Stack {
  constructor(scope: cdk.Construct, id: string, props: cdk.StackProps) {
    super(scope, id, props);

    const ecrRepoName = "test-repo";
    const ecrRepo = ecr.Repository.fromRepositoryAttributes(
      stack,
      ecrRepoName,
      {
        repositoryArn: `arn:aws:ecr:us-west-2:0123456789:repository/${ecrRepoName}`,
        repositoryName: ecrRepoName
      }
    );
    const image = ecs.ContainerImage.fromEcrRepository(
      ecrRepo,
      "sha256:cb8444cafe12a57bff212d1aaf1ab9063233f701760dcf5c08d5e98f4933a04"
    );
    console.log(image.imageName);
    // Got:  0123456789.dkr.ecr.us-west-2.${Token[AWS::URLSuffix.1]}/test-repo:sha256:cb8444cafe12a57bff212d1aaf1ab9063233f701760dcf5c08d5e98f4933a04
    // Need: 0123456789.dkr.ecr.us-west-2.${Token[AWS::URLSuffix.1]}/test-repo@sha256:cb8444cafe12a57bff212d1aaf1ab9063233f701760dcf5c08d5e98f4933a04
  }
}
new DemoStack(myApp, "demostack", {});

Error Log

The printed image name contains a : separating the image name and the digest. The current implementation of EcrImage only support using tags, while ECR allows specifying either a tag or an image digest. See

aws-cdk/packages/@aws-cdk/aws-ecs/lib/images/ecr.ts

Lines 11 to 16 in 7a3a3b1

	* The image name. Images in Amazon ECR repositories can be specified by either using the full registry/repository:tag or
	* registry/repository@digest.
	*
	* For example, 012345678910.dkr.ecr.<region-name>.amazonaws.com/<repository-name>:latest or
	* 012345678910.dkr.ecr.<region-name>.amazonaws.com/<repository-name>@sha256:94afd1f2e64d908bc90dbca0035a5b567EXAMPLE.
	*/

for context.

Environment

• CLI Version : 1.16.3
• Framework Version: 1.16.3
• OS : MacOS Mojave 10.14.6
• Language : Typescript

Other

I'd like to be able to specify a digest instead of a tag when calling ecs.ContainerImage.fromEcrRepository.

I'm using a workaround of defining my own image class, but this seems like something that we should fix in aws-cdk itself. I'm not sure what the API should look like for allowing the caller to specify either a tag or a digest, so filing this bug first. I'd be happy to prepare a PR for this if there is agreement on what the API should be.

Workaround

import * as cdk from "@aws-cdk/core";
import * as ecs from "@aws-cdk/aws-ecs";
import * as ecr from "@aws-cdk/aws-ecr";
export class DigestibleEcrImage extends ecs.ContainerImage {
  /**
   * The image name. Images in Amazon ECR repositories can be specified by either using the full registry/repository:tag or
   * registry/repository@digest.
   *
   * 012345678910.dkr.ecr.<region-name>.amazonaws.com/<repository-name>@sha256:94afd1f2e64d908bc90dbca0035a5b567EXAMPLE.
   */
  public readonly imageName: string;

  constructor(
    private readonly repository: ecr.IRepository,
    private readonly digest: string
  ) {
    super();

    this.imageName = `${this.repository.repositoryUriForTag()}@${digest}`;
  }

  public bind(
    _scope: cdk.Construct,
    containerDefinition: ecs.ContainerDefinition
  ): ecs.ContainerImageConfig {
    this.repository.grantPull(
      containerDefinition.taskDefinition.obtainExecutionRole()
    );

    return {
      imageName: this.imageName
    };
  }
}

---

This is 🐛 Bug Report

[rix0rrr]

We can detect whether the supposed "tag" starts with "sha256:" and then switch to digest mode. There is no point in supporting both methods, so I don't see why this wouldn't work.

Then the people will arrive that want to use a digest from a CfnParameter, so we'll have to add an optional 3rd enum parameter that indicates the type of identifier (TAG or DIGEST). If specified, we'll use that to determine : or @. If unspecified AND the parameter is a token, we'll have to fail. If unspecified and the parameter is not a Token, we can autodetect.

This seems like a low-complexity change that mostly does "what you want it to do" out of the box, and I think it won't cause any problems in the future.

[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.