From 522c8e94488660bbd314b360b28102752cadfb64 Mon Sep 17 00:00:00 2001 From: CrazyMax Date: Sat, 16 Sep 2023 18:40:53 +0200 Subject: [PATCH] handle analysis of image with tag and digest --- docs/faq.md | 14 ++++++++++++++ internal/app/job.go | 2 +- internal/provider/common.go | 4 ---- internal/provider/common_test.go | 4 ++-- pkg/registry/manifest.go | 13 ++++--------- pkg/registry/manifest_test.go | 15 ++------------- pkg/registry/ref.go | 27 ++++++++++++--------------- pkg/registry/ref_test.go | 8 ++++++-- test/dockerfile2/mount/Dockerfile | 2 +- 9 files changed, 42 insertions(+), 47 deletions(-) diff --git a/docs/faq.md b/docs/faq.md index ae887ab5..1f54e98b 100644 --- a/docs/faq.md +++ b/docs/faq.md @@ -333,3 +333,17 @@ Following profilers are available: * `mutex` enables mutex profiling * `threads` enables thread creation profiling * `block` enables block (contention) profiling + +## Image with digest and `image:tag@digest` format + +Analysis of an image with a digest but without tag will be done using `latest` +as a tag which could lead to false positives. + +For example `crazymax/diun@sha256:fa80af32a7c61128ffda667344547805b3c5e7721ecbbafd70e35bb7bb7c989f` +is referring to `crazymax/diun:4.24.0` tag, so it's not correct to assume that +we want to analyze `crazymax/diun:latest`. + +You can still pin an image to a specific digest and analyze the image if the +tag is specified using the `image:tag@digest` format. Taking the previous +example if we specify `crazymax/diun:4.24.0@sha256:fa80af32a7c61128ffda667344547805b3c5e7721ecbbafd70e35bb7bb7c989f`, +then `crazymax/diun:4.24.0` will be analyzed. diff --git a/internal/app/job.go b/internal/app/job.go index 956a9b03..f186b17d 100644 --- a/internal/app/job.go +++ b/internal/app/job.go @@ -22,7 +22,7 @@ func (di *Diun) createJob(job model.Job) { Str("image", job.Image.Name). Logger() - // Validate image + // Parse image prvImage, err = registry.ParseImage(registry.ParseImageOptions{ Name: job.Image.Name, HubTpl: job.Image.HubTpl, diff --git a/internal/provider/common.go b/internal/provider/common.go index 31687061..27830b4d 100644 --- a/internal/provider/common.go +++ b/internal/provider/common.go @@ -19,10 +19,6 @@ var ( // ValidateImage returns a standard image through Docker labels func ValidateImage(image string, metadata, labels map[string]string, watchByDef bool, imageDefaults model.Image) (img model.Image, err error) { - if i := strings.Index(image, "@sha256:"); i > 0 { - image = image[:i] - } - img = model.Image{ Name: image, } diff --git a/internal/provider/common_test.go b/internal/provider/common_test.go index d6d145e6..abaf59f7 100644 --- a/internal/provider/common_test.go +++ b/internal/provider/common_test.go @@ -21,11 +21,11 @@ func TestValidateImage(t *testing.T) { expectedErr interface{} }{ { - name: "Test strip digest", + name: "Test with digest", image: "myimg@sha256:1234567890abcdef", watchByDef: true, expectedImage: model.Image{ - Name: "myimg", + Name: "myimg@sha256:1234567890abcdef", }, expectedErr: nil, }, diff --git a/pkg/registry/manifest.go b/pkg/registry/manifest.go index 4c6b1b8c..a8314e20 100644 --- a/pkg/registry/manifest.go +++ b/pkg/registry/manifest.go @@ -35,15 +35,10 @@ func (c *Client) Manifest(image Image, dbManifest Manifest) (Manifest, bool, err return Manifest{}, false, errors.Wrap(err, "cannot parse reference") } - // Retrieve remote digest through HEAD request or get one from image reference - var rmDigest digest.Digest - if len(image.Digest) > 0 { - rmDigest = image.Digest - } else { - rmDigest, err = docker.GetDigest(ctx, c.sysCtx, rmRef) - if err != nil { - return Manifest{}, false, errors.Wrap(err, "cannot get image digest from HEAD request") - } + // Retrieve remote digest through HEAD request + rmDigest, err := docker.GetDigest(ctx, c.sysCtx, rmRef) + if err != nil { + return Manifest{}, false, errors.Wrap(err, "cannot get image digest from HEAD request") } // Digest match, returns db manifest diff --git a/pkg/registry/manifest_test.go b/pkg/registry/manifest_test.go index 180fbf54..06107763 100644 --- a/pkg/registry/manifest_test.go +++ b/pkg/registry/manifest_test.go @@ -339,7 +339,7 @@ func TestManifestTaggedDigest(t *testing.T) { assert.Equal(t, "linux/amd64", manifest.Platform) } -func TestManifestTaggedDigestDummyTag(t *testing.T) { +func TestManifestTaggedDigestUnknownTag(t *testing.T) { rc, err := New(Options{ CompareDigest: true, ImageOs: "linux", @@ -356,19 +356,8 @@ func TestManifestTaggedDigestDummyTag(t *testing.T) { t.Error(err) } - // download manifest _, _, err = rc.Manifest(img, Manifest{}) - assert.NoError(t, err) - - // check manifest - manifest, updated, err := rc.Manifest(img, manifestCrazymaxDiun4250) - assert.NoError(t, err) - assert.Equal(t, false, updated) - assert.Equal(t, "docker.io/crazymax/diun", manifest.Name) - assert.Equal(t, "latest", manifest.Tag) - assert.Equal(t, "application/vnd.oci.image.index.v1+json", manifest.MIMEType) - assert.Equal(t, "sha256:3fca3dd86c2710586208b0f92d1ec4ce25382f4cad4ae76a2275db8e8bb24031", manifest.Digest.String()) - assert.Equal(t, "linux/amd64", manifest.Platform) + assert.Error(t, err) } var manifestCrazymaxDiun4250 = Manifest{ diff --git a/pkg/registry/ref.go b/pkg/registry/ref.go index 00883bdc..194ecb68 100644 --- a/pkg/registry/ref.go +++ b/pkg/registry/ref.go @@ -39,31 +39,28 @@ func namedReference(name string) (reference.Named, error) { if err != nil { return nil, errors.Wrapf(err, "normalizing tagged digested name %q", name) } - return ref, nil - } - if _, hasDigest := ref.(reference.Digested); hasDigest { - return ref, nil + } else if _, hasDigest := ref.(reference.Digested); hasDigest { + ref = reference.TrimNamed(ref) } return reference.TagNameOnly(ref), nil } -// normalizeTaggedDigestedNamed strips the tag off the specified named -// reference if it is tagged and digested. Note that the tag is entirely -// ignored. +// normalizeTaggedDigestedNamed strips the digest off the specified named +// reference if it is tagged and digested. func normalizeTaggedDigestedNamed(named reference.Named) (reference.Named, error) { - _, isTagged := named.(reference.NamedTagged) - if !isTagged { - return named, nil - } - digested, isDigested := named.(reference.Digested) + _, isDigested := named.(reference.Digested) if !isDigested { return named, nil } - // strip off the tag + tag, isTagged := named.(reference.NamedTagged) + if !isTagged { + return named, nil + } + // strip off the tag and digest newNamed := reference.TrimNamed(named) - // re-add the digest - newNamed, err := reference.WithDigest(newNamed, digested.Digest()) + // re-add the tag + newNamed, err := reference.WithTag(newNamed, tag.Tag()) if err != nil { return named, err } diff --git a/pkg/registry/ref_test.go b/pkg/registry/ref_test.go index f70942b9..6588b8cd 100644 --- a/pkg/registry/ref_test.go +++ b/pkg/registry/ref_test.go @@ -36,11 +36,15 @@ func TestImageReference(t *testing.T) { }, { input: "busybox" + sha256digest, - expected: "docker.io/library/busybox" + sha256digest, + expected: "docker.io/library/busybox:latest", }, { input: "busybox:latest" + sha256digest, - expected: "docker.io/library/busybox" + sha256digest, + expected: "docker.io/library/busybox:latest", + }, + { + input: "busybox:v1.0.0" + sha256digest, + expected: "docker.io/library/busybox:v1.0.0", }, { input: "UPPERCASEISINVALID", diff --git a/test/dockerfile2/mount/Dockerfile b/test/dockerfile2/mount/Dockerfile index 0dadae18..ac66e9d4 100644 --- a/test/dockerfile2/mount/Dockerfile +++ b/test/dockerfile2/mount/Dockerfile @@ -6,7 +6,7 @@ FROM alpine:latest # diun.platform=linux/amd64 # diun.metadata.foo=bar RUN --mount=type=bind,target=.,rw \ - --mount=type=bind,from=crazymax/undock:0.5.0@sha256:736fdfde1268b93c2f733c53a7c45ece24e275318628fbb790bee7f89459961f,source=/usr/local/bin/undock,target=/usr/local/bin/undock \ + --mount=type=bind,from=crazymax/undock@sha256:736fdfde1268b93c2f733c53a7c45ece24e275318628fbb790bee7f89459961f,source=/usr/local/bin/undock,target=/usr/local/bin/undock \ undock --version # diun.platform=linux/amd64