From f74a60652d064c5c3a6ac51028b3e11b1a5fcf93 Mon Sep 17 00:00:00 2001 From: Carolyn Van Slyck Date: Thu, 1 Oct 2020 12:46:44 -0500 Subject: [PATCH 1/2] Fix invocation image digest validation Bundles pushed with cnab-to-oci will have invocation images with more than one repoDigest, which is valid. One is for the original invocation image that was pushed. The other is for the relocated invocation image that is now inside the bundle repository. https://porter.sh/distribute-bundles/#image-references-after-publishing We should check all of the repoDigests and if a match is found, then it is valid. Signed-off-by: Carolyn Van Slyck --- driver/docker/docker.go | 40 ++++++++++++++++++---------------- driver/docker/docker_test.go | 42 ++++++++++++++---------------------- 2 files changed, 38 insertions(+), 44 deletions(-) diff --git a/driver/docker/docker.go b/driver/docker/docker.go index 860ebf4c..122fedae 100644 --- a/driver/docker/docker.go +++ b/driver/docker/docker.go @@ -417,27 +417,31 @@ func (d *Driver) validateImageDigest(image bundle.InvocationImage, repoDigests [ return nil } - switch count := len(repoDigests); { - case count == 0: + if len(repoDigests) == 0 { return fmt.Errorf("image %s has no repo digests", image.Image) - case count > 1: - return fmt.Errorf("image %s has more than one repo digest", image.Image) } - // RepoDigests are of the form 'imageName@sha256:'; we parse out the digest itself for comparison - repoDigest := repoDigests[0] - ref, err := reference.ParseNormalizedNamed(repoDigest) - if err != nil { - return fmt.Errorf("unable to parse repo digest %s", repoDigest) - } - digestRef, ok := ref.(reference.Digested) - if !ok { - return fmt.Errorf("unable to parse repo digest %s", repoDigest) - } - digest := digestRef.Digest().String() + for _, repoDigest := range repoDigests { + // RepoDigests are of the form 'imageName@sha256:' or imageName: + // We only care about the ones in digest form + ref, err := reference.ParseNormalizedNamed(repoDigest) + if err != nil { + return fmt.Errorf("unable to parse repo digest %s", repoDigest) + } - if digest == image.Digest { - return nil + digestRef, ok := ref.(reference.Digested) + if !ok { + continue + } + + digest := digestRef.Digest().String() + + // image.Digest is the digest of the original invocation image defined in the bundle. + // It persists even when the bundle's invocation image has been relocated. + if digest == image.Digest { + return nil + } } - return fmt.Errorf("content digest mismatch: image %s has digest %s but the value should be %s according to the bundle file", image.Image, digest, image.Digest) + + return fmt.Errorf("content digest mismatch: invocation image %s was defined in the bundle with the digest %s but no matching repoDigest was found upon inspecting the image", image.Image, image.Digest) } diff --git a/driver/docker/docker_test.go b/driver/docker/docker_test.go index 352f8d4d..50719f0c 100644 --- a/driver/docker/docker_test.go +++ b/driver/docker/docker_test.go @@ -6,6 +6,7 @@ import ( "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/strslice" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/cnabio/cnab-go/bundle" "github.com/cnabio/cnab-go/driver" @@ -87,8 +88,12 @@ func TestDriver_GetConfigurationOptions(t *testing.T) { } func TestDriver_ValidateImageDigest(t *testing.T) { + // Mimic the digests created when a bundle is pushed with cnab-to-oci + // there is one for the original invocation image and another + // for the relocated invocation image inside the bundle repository repoDigests := []string{ - "myreg/myimg@sha256:d366a4665ab44f0648d7a00ae3fae139d55e32f9712c67accd604bb55df9d05a", + "myreg/mybun-installer:v1.0.0", + "myreg/mybun@sha256:d366a4665ab44f0648d7a00ae3fae139d55e32f9712c67accd604bb55df9d05a", } t.Run("no image digest", func(t *testing.T) { @@ -101,15 +106,15 @@ func TestDriver_ValidateImageDigest(t *testing.T) { assert.NoError(t, err) }) - t.Run("image digest exists - no match exists", func(t *testing.T) { + t.Run("image digest exists - no match found", func(t *testing.T) { d := &Driver{} image := bundle.InvocationImage{} - image.Image = "myreg/myimg" + image.Image = "myreg/mybun@sha256:d366a4665ab44f0648d7a00ae3fae139d55e32f9712c67accd604bb55df9d05a" image.Digest = "sha256:185518070891758909c9f839cf4ca393ee977ac378609f700f60a771a2dfe321" err := d.validateImageDigest(image, repoDigests) - assert.NotNil(t, err, "expected an error") + require.NotNil(t, err, "expected an error") assert.Contains(t, err.Error(), "content digest mismatch") }) @@ -117,39 +122,24 @@ func TestDriver_ValidateImageDigest(t *testing.T) { d := &Driver{} image := bundle.InvocationImage{} - image.Image = "myreg/myimg" + image.Image = "myreg/mybun@sha256:d366a4665ab44f0648d7a00ae3fae139d55e32f9712c67accd604bb55df9d05a" image.Digest = "sha256:185518070891758909c9f839cf4ca393ee977ac378609f700f60a771a2dfe321" - badRepoDigests := []string{"myreg/myimg@sha256:deadbeef"} + badRepoDigests := []string{"myreg/mybun@sha256:deadbeef"} err := d.validateImageDigest(image, badRepoDigests) - assert.NotNil(t, err, "expected an error") - assert.EqualError(t, err, "unable to parse repo digest myreg/myimg@sha256:deadbeef") + require.NotNil(t, err, "expected an error") + assert.Contains(t, err.Error(), "unable to parse repo digest") }) - t.Run("image digest exists - more than one repo digest exists", func(t *testing.T) { + t.Run("image digest exists - match found", func(t *testing.T) { d := &Driver{} image := bundle.InvocationImage{} - image.Image = "myreg/myimg" - image.Digest = "sha256:d366a4665ab44f0648d7a00ae3fae139d55e32f9712c67accd604bb55df9d05a" - - multipleRepoDigests := append(repoDigests, - "myreg/myimg@sha256:185518070891758909c9f839cf4ca393ee977ac378609f700f60a771a2dfe321") - - err := d.validateImageDigest(image, multipleRepoDigests) - assert.NotNil(t, err, "expected an error") - assert.EqualError(t, err, "image myreg/myimg has more than one repo digest") - }) - - t.Run("image digest exists - an exact match exists", func(t *testing.T) { - d := &Driver{} - - image := bundle.InvocationImage{} - image.Image = "myreg/myimg" + image.Image = "myreg/mybun@sha256:d366a4665ab44f0648d7a00ae3fae139d55e32f9712c67accd604bb55df9d05a" image.Digest = "sha256:d366a4665ab44f0648d7a00ae3fae139d55e32f9712c67accd604bb55df9d05a" err := d.validateImageDigest(image, repoDigests) - assert.NoError(t, err) + require.NoError(t, err, "validateImageDigest failed") }) } From 3e677ef0785fee4fad695ee69da2f83552b5ec36 Mon Sep 17 00:00:00 2001 From: Carolyn Van Slyck Date: Thu, 1 Oct 2020 14:41:01 -0500 Subject: [PATCH 2/2] Fix assert to account for updated error message Signed-off-by: Carolyn Van Slyck --- driver/docker/docker_integration_test.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/driver/docker/docker_integration_test.go b/driver/docker/docker_integration_test.go index f9ed32a0..4c21308a 100644 --- a/driver/docker/docker_integration_test.go +++ b/driver/docker/docker_integration_test.go @@ -9,6 +9,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/cnabio/cnab-go/bundle" "github.com/cnabio/cnab-go/bundle/definition" @@ -109,10 +110,8 @@ func TestDriver_ValidateImageDigestFail(t *testing.T) { docker := &Driver{} _, err := docker.Run(op) - assert.Error(t, err) + require.Error(t, err, "expected an error") // Not asserting actual image digests to support arbitrary integration test images assert.Contains(t, err.Error(), - fmt.Sprintf("content digest mismatch: image %s has digest", op.Image.Image)) - assert.Contains(t, err.Error(), - fmt.Sprintf("but the value should be %s according to the bundle file", badDigest)) + fmt.Sprintf("content digest mismatch: invocation image %s was defined in the bundle with the digest %s but no matching repoDigest was found upon inspecting the image", op.Image.Image, badDigest)) }