cri-o: Digest could be invalid when using multiple registries

What happened?

When image is pulled from multiple registries RepoDigest is incorrectly set. It includes cross product of image name and repository digest. This lead to situation when having images available at A:1 and B:2 we end up with A:1, A:2, B:1, B:2 which is incorrect as repo digest is valid only for repo it was obtained from as it’s sha of repository output not manifest or layers (e.g. output could be compressed with different algorithm).

What did you expect to happen?

RepoDigest, ID and Digest should be valid pullable from registry.

How can we reproduce it (as minimally and precisely as possible)?

package storage

import (
	"github.com/containers/storage"
	. "github.com/onsi/ginkgo/v2"
	. "github.com/onsi/gomega"
	"github.com/opencontainers/go-digest"
	"testing"
)

func TestBooks(t *testing.T) {
	RegisterFailHandler(Fail)
	RunSpecs(t, "Image Suite")
}

var _ = Describe("MakeImageDigest", func() {
	It("multiple repo digests", func() {
		sut := imageService{}
		knownRepoDigests := []string{
			"image-registry.openshift-image-registry.svc:5000/12400-repro/nginx@sha256:3333333333333333333333333333333333333333333333333333333333333333",
			"quay.io/dhaus/repro@sha256:5555555555555555555555555555555555555555555555555555555555555555",
		}
		tags := []string{
			"image-registry.openshift-image-registry.svc:5000/12400-repro/nginx:1.23",
			"quay.io/dhaus/repro:1.23",
		}
		img := storage.Image{
			ID:     "sha256:2222222222222222222222222222222222222222222222222222222222222222",
			Digest: "sha256:1111111111111111111111111111111111111111111111111111111111111111",
			Digests: []digest.Digest{
				"sha256:3333333333333333333333333333333333333333333333333333333333333333",
			},
		}
		gotImageDigest, gotRepoDigests := sut.makeRepoDigests(knownRepoDigests, tags, &img)
		Expect(gotImageDigest).To(Equal(digest.Digest("sha256:1111111111111111111111111111111111111111111111111111111111111111")))
		Expect(gotRepoDigests).To(Equal([]string{
			"image-registry.openshift-image-registry.svc:5000/12400-repro/nginx@sha256:3333333333333333333333333333333333333333333333333333333333333333",
			"quay.io/dhaus/repro@sha256:5555555555555555555555555555555555555555555555555555555555555555",
			"quay.io/dhaus/repro@sha256:1111111111111111111111111111111111111111111111111111111111111111",
			"image-registry.openshift-image-registry.svc:5000/12400-repro/nginx@sha256:1111111111111111111111111111111111111111111111111111111111111111",
		}))
	})
})

Anything else we need to know?

  • Pod deployed with an image in the internal registry of OpenShift which is a re-tagged nginx image from quay
apiVersion: v1
kind: Pod
metadata:
  labels:
    run: internal
  name: internal
  namespace: internal
spec:
  containers:
  - image: image-registry.openshift-image-registry.svc:5000/internal/nginx:1.23 # re-tagged quay.io/nginx:1.23
    name: internal
  dnsPolicy: ClusterFirst
  restartPolicy: Always
  nodeName: worker-1
  • Pod deployed with an image from quay
apiVersion: v1
kind: Pod
metadata:
  labels:
    run: nginx
  name: nginx
  namespace: internal
spec:
  containers:
  - image: quay.io/nginx:1.23
    name: nginx
  dnsPolicy: ClusterFirst
  restartPolicy: Always
  nodeName: worker-1
  • Describe of the pod without the re-tagged image being present on the node
oc describe pod internal                                                                                                                                                                                                 ─╯
Name:         internal
Namespace:    internal
Containers:
  rox-12400-internal:
    Container ID:   cri-o://69b40bcc6a57b281080c448a5e4ae39fb25756ab2643509535035901d58e6b9d
    Image:          image-registry.openshift-image-registry.svc:5000/internal/nginx:1.23
    Image ID:       image-registry.openshift-image-registry.svc:5000/internal/nginx@sha256:f8fe7d0e6be40a7febbdec9aaf820eed7dbe62cb2afff00f67610666f7a9bd97
  • Describe of the pod after the pod with the re-tagged image was deployed on the same node
oc describe pod internal                                                                                                                                                                                                    ─╯
Name:         internal
Namespace:    internal
Containers:
  internal:
    Container ID:   cri-o://e6f39f86dac6542ff2352a13e9b8e9eb931db207b649b14869fe449439fcf4aa
    Image:          image-registry.openshift-image-registry.svc:5000/internal/nginx:1.23
    Image ID:       image-registry.openshift-image-registry.svc:5000/internal/nginx@sha256:89020cd33be2767f3f894484b8dd77bc2e5a1ccc864350b92c53262213257dfc  

CRI-O and Kubernetes version

$ crio --version
crio version 1.24.2-4.rhaos4.11.gitd6283df.el8
Version:          1.24.2-4.rhaos4.11.gitd6283df.el8
GoVersion:        go1.18.4
Compiler:         gc
Platform:         linux/amd64
Linkmode:         dynamic
BuildTags:        exclude_graphdriver_devicemapper, containers_image_ostree_stub, seccomp, selinux
SeccompEnabled:   true
AppArmorEnabled:  false
$ oc version
Client Version: 4.11.0-202208110436.p0.gfcf512e.assembly.stream-fcf512e
Kustomize Version: v4.5.4
Server Version: 4.11.1
Kubernetes Version: v1.24.0+4f0dd4d

OS version

N/A

Additional environment details (AWS, VirtualBox, physical, etc.)

N/A

About this issue

  • Original URL
  • State: open
  • Created 2 years ago
  • Comments: 17 (11 by maintainers)

Commits related to this issue

Most upvoted comments

A friendly reminder that this issue had no activity for 30 days.

Unfortunately I am still not very clear on under what circumstances Names contains a tag only and not a repo digest, but I will keep digging.

IIRC every pull by tag only adds the tag to Names, not the repoDigest combination.

Yes, I think I understand it slightly better now. I think https://github.com/misberner/cri-o/commit/0d745dbc45b252d262de5fa8cc4ff86f467d0091 (illustrative purposes only, unit tests probably need fixing) would be a “better” way to compute the cross-product that might alleviate issues in some situations. However, it seems that the second portion

Then modify c/image to always include a repo@digest value in Names.

is actually the more important/more time-sensitive one, because it ensures that we have the ground-truth data available that makes the first part work. Unfortunately I am still not very clear on under what circumstances Names contains a tag only and not a repo digest, but I will keep digging.