Skip to content

Commit

Permalink
Iterate through all repo digests to find one matching the image
Browse files Browse the repository at this point in the history
  • Loading branch information
amogh09 committed Apr 25, 2024
1 parent e0d941b commit 8f8e50d
Show file tree
Hide file tree
Showing 4 changed files with 144 additions and 8 deletions.
11 changes: 6 additions & 5 deletions agent/engine/docker_task_engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -1285,17 +1285,18 @@ func (engine *DockerTaskEngine) pullContainerManifest(
})
return dockerapi.DockerContainerMetadata{}
}
parsedDigest := referenceutil.GetDigestFromImageRef(imgInspect.RepoDigests[0])
if parsedDigest == "" {
logger.Error("Failed to parse repo digest", logger.Fields{
parsedDigest, err := referenceutil.GetDigestFromRepoDigests(
imgInspect.RepoDigests, container.Image)
if err != nil {
logger.Error("Failed to find a repo digest matching the image", logger.Fields{
field.TaskARN: task.Arn,
field.ContainerName: container.Name,
field.Image: container.Image,
field.ImageDigest: imgInspect.RepoDigests[0],
"repoDigests": imgInspect.RepoDigests,
})
return dockerapi.DockerContainerMetadata{
Error: dockerapi.CannotPullImageManifestError{
FromError: fmt.Errorf("failed to parse repo digest from '%s'", imgInspect.RepoDigests[0]),
FromError: fmt.Errorf("failed to find a repo digest matching '%s'", container.Image),
},
}
}
Expand Down
6 changes: 3 additions & 3 deletions agent/engine/docker_task_engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3961,7 +3961,7 @@ func TestPullContainerManifest(t *testing.T) {
},
},
{
name: "image pull not required - inspected image has no repo digest",
name: "image pull not required - inspected image has no repo digests",
image: "myimage",
imagePullBehavior: config.ImagePullPreferCachedBehavior,
setDockerClientExpectations: func(c *gomock.Controller, d *mock_dockerapi.MockDockerClient) {
Expand All @@ -3980,7 +3980,7 @@ func TestPullContainerManifest(t *testing.T) {
},
expectedResult: dockerapi.DockerContainerMetadata{
Error: dockerapi.CannotPullImageManifestError{
FromError: errors.New("failed to parse repo digest from 'invalid'"),
FromError: errors.New("failed to find a repo digest matching 'myimage'"),
},
},
},
Expand All @@ -3990,7 +3990,7 @@ func TestPullContainerManifest(t *testing.T) {
imagePullBehavior: config.ImagePullPreferCachedBehavior,
setDockerClientExpectations: func(c *gomock.Controller, d *mock_dockerapi.MockDockerClient) {
inspectResult := &types.ImageInspect{
RepoDigests: []string{"alpine@" + testDigest.String()},
RepoDigests: []string{"myimage@" + testDigest.String()},
}
d.EXPECT().InspectImage("myimage").Times(2).Return(inspectResult, nil)
},
Expand Down
45 changes: 45 additions & 0 deletions agent/utils/reference/reference.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@
package reference

import (
"fmt"

"github.com/aws/amazon-ecs-agent/ecs-agent/logger"
"github.com/aws/amazon-ecs-agent/ecs-agent/logger/field"
"github.com/docker/distribution/reference"
"github.com/opencontainers/go-digest"
)
Expand All @@ -33,3 +37,44 @@ func GetDigestFromImageRef(imageRef string) digest.Digest {
return ""
}
}

// Finds a repo digest matching the provided image reference from a list of repo digests
// and returns the repo digest's digest.
func GetDigestFromRepoDigests(repoDigests []string, imageRef string) (digest.Digest, error) {
// Parse image reference
ref, err := reference.Parse(imageRef)
if err != nil {
return "", fmt.Errorf("failed to parse image reference '%s': %w", imageRef, err)
}
namedRef, ok := ref.(reference.Named)
if !ok {
return "", fmt.Errorf(
"failed to parse image reference '%s' as a named reference, it was parsed as '%v'",
imageRef, ref)
}

// Find a repo digest matching imageRef and return its digest
for _, repoDigest := range repoDigests {
repoDigestRef, err := reference.Parse(repoDigest)
if err != nil {
logger.Error("Error in parsing repo digest. Skipping it.", logger.Fields{
"repoDigest": repoDigest,
field.Error: err,
})
continue
}
repoDigestCanonicalRef, ok := repoDigestRef.(reference.Canonical)
if !ok {
logger.Warn("Parsed repo digest is not in canonical form. Skipping it.", logger.Fields{
"repoDigest": repoDigest,
"parsedRepoDigest": repoDigestRef.String(),
})
continue
}
if repoDigestCanonicalRef.Name() == namedRef.Name() {
return repoDigestCanonicalRef.Digest(), nil
}
}

return "", fmt.Errorf("found no repo digest matching '%s'", imageRef)
}
90 changes: 90 additions & 0 deletions agent/utils/reference/reference_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,93 @@ func TestImageFromDigest(t *testing.T) {
})
}
}

func TestGetDigestFromRepoDigests(t *testing.T) {
testDigest1 := "sha256:c5b1261d6d3e43071626931fc004f70149baeba2c8ec672bd4f27761f8e1ad6b"
testDigest2 := "sha256:c3839dd800b9eb7603340509769c43e146a74c63dca3045a8e7dc8ee07e53966"
tcs := []struct {
name string
repoDigests []string
imageRef string
expectedDigest string
expectedError string
}{
{
name: "repo digest matching imageRef is used - dockerhub",
repoDigests: []string{
"public.ecr.aws/library/alpine@" + testDigest1,
"alpine@" + testDigest2,
},
imageRef: "alpine",
expectedDigest: testDigest2,
},
{
name: "repo digest matching imageRef is used - ecr",
repoDigests: []string{
"public.ecr.aws/library/alpine@" + testDigest1,
"alpine@" + testDigest2,
},
imageRef: "public.ecr.aws/library/alpine",
expectedDigest: testDigest1,
},
{
name: "tags in image ref are ignored - dockerhub",
repoDigests: []string{
"public.ecr.aws/library/alpine@" + testDigest1,
"alpine@" + testDigest2,
},
imageRef: "alpine:edge",
expectedDigest: testDigest2,
},
{
name: "tags in image ref are ignored - ecr",
repoDigests: []string{
"public.ecr.aws/library/alpine@" + testDigest1,
"alpine@" + testDigest2,
},
imageRef: "public.ecr.aws/library/alpine:edge",
expectedDigest: testDigest1,
},
{
name: "invalid image ref",
imageRef: "invalid format",
expectedError: "failed to parse image reference 'invalid format': invalid reference format",
},
{
name: "image ref not named",
imageRef: "",
expectedError: "failed to parse image reference '': repository name must have at least one component",
},
{
name: "invalid repo digests are skipped",
imageRef: "alpine",
repoDigests: []string{"invalid format", "alpine@" + testDigest1},
expectedDigest: testDigest1,
},
{
name: "no repo digests match image ref",
imageRef: "public.ecr.aws/library/alpine",
repoDigests: []string{"alpine@" + testDigest1},
expectedError: "found no repo digest matching 'public.ecr.aws/library/alpine'",
},
{
name: "image reference can be canonical",
repoDigests: []string{"alpine@" + testDigest1},
imageRef: "alpine@" + testDigest2,
expectedDigest: testDigest1,
},
}
for _, tc := range tcs {
t.Run(tc.name, func(t *testing.T) {
dgst, err := GetDigestFromRepoDigests(tc.repoDigests, tc.imageRef)
if tc.expectedError == "" {
require.NoError(t, err)
expected, err := digest.Parse(tc.expectedDigest)
require.NoError(t, err)
assert.Equal(t, expected, dgst)
} else {
require.EqualError(t, err, tc.expectedError)
}
})
}
}

0 comments on commit 8f8e50d

Please sign in to comment.