fix(codepipeline): cannot trigger on all tags anymore in EcrSourceAction

[sparten11740]

The EcrSourceAction could previously be used to trigger on changes to all tags of an image. As part of the fix #13818, the imageTag was defaulted to latest if not provided. Therefore it was no longer possible to call the underlying onCloudTrailImagePushed function with an undefined imageTag to watch changes on all tags.

Reintroduce triggering on all tags by passing an empty string as the imageTag.

Fixes #13818

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

[gitpod-io]

[skinny85]

@sparten11740 thank you for the contribution, but I have to say, I'm having second thoughts on whether this change even makes sense 😜.

The CodePipeline documentation states:

ImageTag
Required: No

The tag used for the image.

Note
If a value for ImageTag is not specified, the value defaults to latest.

If that is true, then specifying '' here would trigger on a push to every tag, yes. But the image downloaded would always be for the 'latest' tag anyway! If that's the case, what's the point of triggering a deploy for a different tag, if it won't be used anyway, and 'latest' will be used (so basically, a no-op deployment)?

Can you elaborate on your usecase a little bit?

[sparten11740]

@skinny85

You’re assuming that latest is used as a tag, which I’d like to avoid.

From my understanding the EcrSourceAction does not download any images but only provides metadata (as artifact or output variables) to the successor steps. When watching changes on latest, imageTag in the output variables will always be forwarded as latest. This is pretty much useless when deploying an image to a Fargate ECS cluster as infrastructure deployment. As a workaround we have to user either the imageDigest (or the imageUri which is also possible since it is a digest- and not tag-based image URI) to update the container image in our stack.

There are two minor disadvantages with this approach. First, looking at the container definition in the ECS console to find out what’s deployed, we’d always have to go to ECR first to see what image is actually hiding behind the image url (whereas if we used the e.g. commit hash or something else meaningful as dynamic tag this wouldn’t be the case). Second, creating a container image from a digest just feels a little clumsy.

Of course this works and is not a big deal, but I’d much rather not be forced to use the latest tag at all. Does that make sense?

[skinny85]

I mostly understand, but I have a few follow-up questions 🙂.

So, the ECR source Action produces a file with a bunch of metadata about the image. Among them is tag, imageDigest, and imageUri. You don't want to use tag. First question - can you expand on why don't you want to use tag? Is it because the tag needs to be the same for all Pipeline invocations, and you want to use different tags?

Second question. If we make the CloudWatch event react to every tag, like we do in this PR, does this mean that the metadata produced by the ECR source Action will be for whatever image was pushed that caused that Event to fire? So, even if the tag in the source Action will be undefined, which implies latest, you're saying here that if you push a different tag to the repository, the CloudWatch event will fire, and that will make the ECR source Action produce the imageDigest and imageUri of the just pushed image, instead of latest?

Third question: you wrote:

As a workaround we have to user either the imageDigest (or the imageUri which is also possible since it is a digest- and not tag-based image URI) to update the container image in our stack.

Can you explain how are you achieving this?

Thanks,
Adam

[sparten11740]

First question - can you expand on why don't you want to use tag? Is it because the tag needs to be the same for all Pipeline invocations, and you want to use different tags?

That's correct. I would like to use tag but can't because it needs to be the same for all invocations.

Second question. If we make the CloudWatch event react to every tag, like we do in this PR, does this mean that the metadata produced by the ECR source Action will be for whatever image was pushed that caused that Event to fire?

If we do it like in this PR, this would be the case but only if we provide '' (empty string) as imageTag to the source action. The produced metadata will then be for the image that was pushed. The behaviour when providing undefined, latest or any other specific tag, remains unchanged.

Regarding your third question: The ECRSourceAction in our pipeline triggers on latest. The stack will be synthesized with imageDigest provided as env variable. Within the stack, imageDigest is used to instantiate a container image:

const containerImage = ContainerImage.fromDockerImageAsset({
  repository,
  imageUri: repository.repositoryUriForDigest(imageDigest),
  assetHash: imageDigest,
  node: repository.node,
});

If tag could be used, this would shrink to:

ContainerImage.fromEcrRepository(repository, imageTag);

I hope this could make it a little clearer.

[skinny85]

OK, thanks for the answers. I feel like this is all a little bit fragile - from relying on the undocumented behavior of the ECR CodePipeline source Action with regards to ignoring the tag, and just including whatever metadata of the image was just pushed, to the way you implement a DockerImageAsset inline inside the ContainerImage.fromDockerImageAsset() call. It feels like this should all be simpler than this 😜. But kudos to you for figuring all of this out.

I don't have any more philosophical objections to this PR. Let me do another round of reviews.

[skinny85]

Looks good @sparten11740, just a few minor changes!

Pull request has been modified.

[skinny85]

Awesome, thanks so much for the contribution @sparten11740!

One last tiny comment before we merge this in.

[skinny85]

Thanks for the contribution @sparten11740!

[mergify]

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
• Commit ID: da03f37
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[mergify]

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

[rix0rrr]

As reported in #20594, this change does not actually work as advertised. I looked at the ECR Source Action source code and will always substitute in the value "default" if imageTag is unspecified.