Add 'aws_ecr_image' datasource #8403

alexrudd · 2019-04-22T16:20:36Z

Community Note

Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

Changes proposed in this pull request:

Add a new datasource aws_ecr_image that maps to the DescribeImages API

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccAWSEcrDataSource_ecrImage'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./... -v -parallel 20 -run=TestAccAWSEcrDataSource_ecrImage -timeout 120m
?       github.com/terraform-providers/terraform-provider-aws   [no test files]
=== RUN   TestAccAWSEcrDataSource_ecrImage
=== PAUSE TestAccAWSEcrDataSource_ecrImage
=== CONT  TestAccAWSEcrDataSource_ecrImage
--- PASS: TestAccAWSEcrDataSource_ecrImage (20.28s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       20.330s

Hello!

I've written a datasource for ECR images, but i'm not sure how to write acceptance tests for it. To test it works I'd need to create an ecr repository (fine), then docker push an image to it (???), and then test my data source.

Does anyone have any suggestions for how I should do this?

Thanks.

EDIT:

Have added tests in which the new data source is used twice: once to lookup the public "amazonlinux" image by the "latest" tag; and a second time to look up the same image but by the digest returned from the first lookup.

Mostly the checks are just to see if the attributes are set, except for the image_tags attribute which is checked for the presence of the "latest" tag.

bflad · 2019-04-25T12:43:14Z

Hi @alexrudd 👋 Thanks for submitting this. For the testing you might find its easier to instead reference an image that is generally available across regions, like the Amazon Linux Container Images instead of attempting to create an ECR repository and upload an image for testing. In my brief testing, the Amazon Linux image appears to come from registry ID 137112412989 in both us-west-2 (acceptance testing default) and us-east-2 (just to check another region):

$ aws ecr list-images --region us-west-2 --registry-id 137112412989 --repository-name amazonlinux | jq '.imageIds | length'
46
$ aws ecr list-images --region us-east-2 --registry-id 137112412989 --repository-name amazonlinux | jq '.imageIds | length'
46

alexrudd · 2019-04-27T09:51:31Z

For the testing you might find its easier to instead reference an image that is generally available across regions, like the Amazon Linux Container Images

Awesome, thanks! I didn't know there were public images like that. Will give this a go.

soleares · 2019-05-02T01:34:03Z

@alexrudd This is great. Would it be possible to add the image sha to the attributes as well. That's the use case I'm looking at this for.

alexrudd · 2019-05-02T07:32:43Z

@alexrudd This is great. Would it be possible to add the image sha to the attributes as well. That's the use case I'm looking at this for.

Hi @soleares, isn't the image sha the same as the image digest? ie: sha256:8c27f34545fd8d4e720b050824aca4b169ec4d5df395a5482a1c5d1571f37c64. In which case you can look such an image up by using the image_digest attribute.

soleares · 2019-05-02T20:20:22Z

@alexrudd yes, my mistake. That's what I needed. Thanks!

bflad

Thanks for the updates, @alexrudd! This looks pretty good so pulling this in. I left two notes about potential improvements for the future (nothing preventing an initial release of the data source) and covering the two conversion function related items on merge just to prevent some potential Go panics. Thanks again! 🚀

Output from acceptance testing:

--- PASS: TestAccAWSEcrDataSource_ecrImage (14.26s)
--- PASS: TestAccAWSEcrDataSource_ecrRepository (17.23s)

bflad · 2019-05-13T16:40:39Z

aws/data_source_aws_ecr_image.go

+				Type:         schema.TypeString,
+				Optional:     true,
+				Computed:     true,
+				ValidateFunc: validation.NoZeroValues,


In a future update of this data source: we should likely remove this validation since in Terraform 0.11 it may be helpful to define this data source as part of a Terraform module that uses an optional variable to override image lookup and the below d.GetOk() usage will skip "" automatically.

variable "registry_id" { default = "" } data "aws_ecr_image" "example" { registry_id = "${var.registry_id}" repostiory_name = "..." }

bflad · 2019-05-13T16:43:51Z

aws/data_source_aws_ecr_image.go

+	}
+
+	if imgId.ImageDigest == nil && imgId.ImageTag == nil {
+		return fmt.Errorf("At least one of either image_digest or image_tag must be defined")


In a future update of this data souce: instead of returning an error in this situation, we should likely just perform a latest image tag lookup instead, which matches the behavior of other tooling. The code and documentation should be updated to reflect this change. 👍

bflad · 2019-05-13T16:45:40Z

aws/data_source_aws_ecr_image.go

+	image := imageDetails[0]
+
+	d.SetId(time.Now().UTC().String())
+	if err = d.Set("registry_id", *image.RegistryId); err != nil {


To prevent potential panics, we should prefer to skip direct * dereferences with d.Set(), which handles pointers automatically, or use the AWS Go SDK provided conversion functions (e.g. aws.StringValue(image.RegistryId)) here and below

Suggested change

if err = d.Set("registry_id", *image.RegistryId); err != nil {

if err = d.Set("registry_id", image.RegistryId); err != nil {

bflad · 2019-05-13T16:48:57Z

aws/data_source_aws_ecr_image.go

+	for _, t := range image.ImageTags {
+		tags = append(tags, *t)
+	}
+	if err := d.Set("image_tags", tags); err != nil {


This can be simplified with the AWS Go SDK provided conversion function, aws.StringValueSlice()

Suggested change

if err := d.Set("image_tags", tags); err != nil {

if err := d.Set("image_tags", aws.StringValueSlice(image.ImageTags)); err != nil {

…ns to prevent panics Reference: #8403 Output from acceptance testing: ``` --- PASS: TestAccAWSEcrDataSource_ecrImage (14.26s) --- PASS: TestAccAWSEcrDataSource_ecrRepository (17.23s) ```

bflad · 2019-05-17T13:39:00Z

This has been released in version 2.11.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

ghost · 2020-03-30T17:16:23Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

ghost added size/L Managed by automation to categorize the size of a PR. documentation Introduces or discusses updates to documentation. provider Pertains to the provider itself, rather than any interaction with AWS. service/ecr Issues and PRs that pertain to the ecr service. labels Apr 22, 2019

bflad added the new-data-source Introduces a new data source. label Apr 25, 2019

ghost added the tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. label Apr 27, 2019

alexrudd marked this pull request as ready for review April 27, 2019 14:56

alexrudd changed the title ~~[WIP] Add 'aws_ecr_image' datasource~~ Add 'aws_ecr_image' datasource Apr 27, 2019

Add 'aws_ecr_image' datasource

83d4829

bflad self-assigned this May 13, 2019

bflad approved these changes May 13, 2019

View reviewed changes

bflad added this to the v2.11.0 milestone May 13, 2019

bflad merged commit 61a591a into hashicorp:master May 13, 2019

bflad added a commit that referenced this pull request May 13, 2019

Update CHANGELOG for #8403

b504ba1

ghost locked and limited conversation to collaborators Mar 30, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add 'aws_ecr_image' datasource #8403

Add 'aws_ecr_image' datasource #8403

alexrudd commented Apr 22, 2019 •

edited

Loading

bflad commented Apr 25, 2019

alexrudd commented Apr 27, 2019

soleares commented May 2, 2019

alexrudd commented May 2, 2019

soleares commented May 2, 2019

bflad left a comment

bflad May 13, 2019

bflad May 13, 2019

bflad May 13, 2019

bflad May 13, 2019

bflad commented May 17, 2019

ghost commented Mar 30, 2020

	if err = d.Set("registry_id", *image.RegistryId); err != nil {
	if err = d.Set("registry_id", image.RegistryId); err != nil {

	if err := d.Set("image_tags", tags); err != nil {
	if err := d.Set("image_tags", aws.StringValueSlice(image.ImageTags)); err != nil {

Add 'aws_ecr_image' datasource #8403

Add 'aws_ecr_image' datasource #8403

Conversation

alexrudd commented Apr 22, 2019 • edited Loading

Community Note

bflad commented Apr 25, 2019

alexrudd commented Apr 27, 2019

soleares commented May 2, 2019

alexrudd commented May 2, 2019

soleares commented May 2, 2019

bflad left a comment

Choose a reason for hiding this comment

bflad May 13, 2019

Choose a reason for hiding this comment

bflad May 13, 2019

Choose a reason for hiding this comment

bflad May 13, 2019

Choose a reason for hiding this comment

bflad May 13, 2019

Choose a reason for hiding this comment

bflad commented May 17, 2019

ghost commented Mar 30, 2020

alexrudd commented Apr 22, 2019 •

edited

Loading