Create latest_container resolver

[patrobinson]

While we generally don't recommend deploying containers with CloudFormation, see #237, it can sometimes be useful.

This PR creates a parameter resolver that finds the latest image in an ECR repository and returns the full image path to that image.

[mipearson]

LGTM. Appreciate the spec for the semi-edge-case of multiple tags.

[mipearson]

That sounds like a fun thing to find out the hard way.

[viraptor]

Is the tag safe to use here? Wouldn't we be better with image_digest?

[patrobinson]

This is only an issue if you use mutable tags like "production". I don't think that's good practice so while I wouldn't object to such a change I don't think we should prioritise it

[patrobinson]

Hmm, recent events have made me question this view.
It seems as though we use our repositories not just to store the latest images but also as a cache for our CI pipelines. It's quite common for pipelines to push up images from a test environment. So finding the latest image is probably a terrible idea and what we should be doing instead is tagging latest (or production or staging), but fetching the sha for that tag and returning it.

[stevehodgkiss]

Nitpick: begin/end/while is an alternative.

begin
  # ..
end while next_token.nil?

[patrobinson]

This breaks one of my tests... and not the pagination test I just added.

[stevehodgkiss]

Ah, looks like it should be !next_token.nil?

[stevehodgkiss]

It looks like this will create an array of arrays containing image_details responses https://docs.aws.amazon.com/sdkforruby/api/Aws/ECR/Client.html#describe_images-instance_method

Maybe images += resp.image_details?

[patrobinson]

I flatten it later but that's a better way

[patrobinson]

@stevehodgkiss @mipearson @viraptor sorry I changed the interface and code a bit based on my renewed understanding of my use case. We now allow specifying a tag parameter and returning the sha of that tag, to avoid having to version tags and store the latest tag version somewhere.

[viraptor]

Thanks for changing this to digests!

[mipearson]

minor nitpicky changes requested only

[mipearson]

Unnecessary return

[mipearson]

Maybe space this function out a bit to make it easier to understand logical groupings, eg:

      def resolve(parameters)
        if parameters['repository_name'].nil?
          raise ArgumentError, "repository_name parameter is required but was not supplied"
        end

        @region = parameters['region'] || @stack_definition.region
        ecr_client = Aws::ECR::Client.new(region: @region)

        images = fetch_images(parameters['repository_name'], parameters['registry_id'], ecr_client)
        return nil if images.empty?

        if !parameters['tag'].nil?
          images.select! { |image| image.image_tags.any? { |tag| tag == parameters['tag'] } }
        end
        images.sort! { |image_x, image_y| image_y.image_pushed_at <=> image_x.image_pushed_at }
        latest_image = images.first

        # aws_account_id.dkr.ecr.region.amazonaws.com/repository@sha256:digest
        "#{latest_image.registry_id}.dkr.ecr.#{@region}.amazonaws.com/#{parameters['repository_name']}@#{latest_image.image_digest}"
      end

[andrewjhumphrey]

Is registry_id also a required parameter?

[patrobinson]

No, as per the documentation it's only needed if you're fetching a container from a different account

[andrewjhumphrey]

NM, I see the default now.

[andrewjhumphrey]

Actually, no I don't, the docs say it should default but I don't see the defaulting happening.
And no test for what happens when it's not...

[patrobinson]

The API handles this for us. If we don't specify a registry ID, it defaults to the current account

[andrewjhumphrey]

Perfect, no need for tests then either :-)