validate KicBase cache by contentDigest

[x7upLime]

Currently the main difficulty we found in moving to a less docker-centric approach,
is the the fact that the current cache-invalidation mechanism
is based upon image digest(a.k.a. ditributionDigest, a.k.a. registry reference) verification,
and that this information is not maintained in the container image itself(kicBase cache works using tarballs)
and it seems like any container engine has its own idea on how that information should be stored/retrieved.

Since this mechanism for cache-invalidation already caused quite some headache,
and the idea of dropping this mechanism for cache-invalidation has been expressed a couple of times...
#15491 (comment)
google/go-containerregistry#895 (comment)

I'd propose to change the cache invalidation mechanism first, to something based on
contentDigest(a.k.a. imageId), before trying to change the cache-access mechanism from the
kicDriver perspective. I'm sure then it'll be easier to solve a number of issues.

[+] refs:
https://windsock.io/explaining-docker-image-ids/ (contentDigest vs. distributionDigest)
https://stackoverflow.com/questions/45533005/why-digests-are-different-depend-on-registry (same as above..)
distribution/distribution#1662 (comment) -- (contentDigest is content-addressable)
google/go-containerregistry#895 (comment) (distributionDigest is not reliable)

[afbjorklund]

The kicbase cache is just an internal thing, similar to the iso cache. And minikube can do whatever with it.

gcr.io/k8s-minikube/kicbase:v0.0.36

minikube-v1.28.0-amd64.iso

It can store the digest in some side channel, it can do sha256 on the kicbase tarball. And then verify it, itself.

.minikube/cache/kic/amd64/kicbase_v0.0.36@sha256_8debc1b6a335075c5f99bfbf131b4f5566f68c6500dc5991817832e55fcc9456.tar

Opening the archive again and examining the contents, sounds like overkill ?

When the cache file was created (downloading from the registry), the digest was used.

---

For kind, the digest is needed since they include kubernetes inside the image as well as the node image.
So if you update the node image or other internals, but keep the same k8s version 1.25.3 - you bump the digest.

kindest/node:v1.25.3@sha256:f52781bc0d7a19fb6c405c2af83abfeb311f130707a0e219175677e366cc45d1

kindest/node:v1.25.3@sha256:f1de3b0670462f43280114eccceab8bf1b9576d2afe0582f8f74529da6fd0365

But for minikube, the kubernetes images are not rebuilt and they are downloaded outside the kic image.
So the kicbase can have its own version, like "0.0.36" and it is very rare to have two kic images with the same ?

Even the unstable images have their own tags.

Version = "v0.0.36-1673540226-15630"
Version = "v0.0.36-1668787669-15272"
Version = "v0.0.36"
Version = "v0.0.35-1667414058-15235"
Version = "v0.0.35-1666722858-15219"

So adding the digest to it is mostly useless...

[afbjorklund]

Now for the actual image cache, the container images used by the pods, it should use the same as kind.

docker image inspect -f '{{ .Id }}'

crictl inspecti | jq '(.status.id)'

It is not portable, so one needs to take care to convert the internal standards of the other runtimes.

        "Id": "sha256:66ba00ad3de8677a3fa4bc4ea0fc46ebca0f14db46ca365e7f60833068dd0148",
        "RepoTags": [
            "busybox:latest"
        ],
        "RepoDigests": [
            "busybox@sha256:7b3ccabffc97de872a30dfd234fd972a66d247c8cfc69b0550f276481852627c"
        ],

docker

busybox
sha256:66ba00ad3de8677a3fa4bc4ea0fc46ebca0f14db46ca365e7f60833068dd0148

cri-o

docker.io/library/busybox
66ba00ad3de8677a3fa4bc4ea0fc46ebca0f14db46ca365e7f60833068dd0148

containerd

docker.io/library/busybox
sha256:7b3ccabffc97de872a30dfd234fd972a66d247c8cfc69b0550f276481852627c

That is, to add the "sha256:" prefix for crio and to use crictl instead of ctr for containerd ?

---

I hope you were not expecting standards.

docker

REPOSITORY   TAG       IMAGE ID       CREATED        SIZE
busybox      latest    66ba00ad3de8   2 weeks ago    4.87MB

podman

REPOSITORY                 TAG         IMAGE ID      CREATED      SIZE
docker.io/library/busybox  latest      66ba00ad3de8  2 weeks ago  5.1 MB

nerdctl

REPOSITORY    TAG       IMAGE ID        CREATED          PLATFORM       SIZE       BLOB SIZE
busybox       latest    7b3ccabffc97    4 minutes ago    linux/amd64    4.8 MiB    2.5 MiB

[afbjorklund]

@x7upLime otherwise the implemenation looks correct, crane makes sure to save it in manifest.json:

[
  {
    "Config": "sha256:66ba00ad3de8677a3fa4bc4ea0fc46ebca0f14db46ca365e7f60833068dd0148",
    "RepoTags": [
      "index.docker.io/library/busybox:i-was-a-digest"
    ],
    "Layers": [
      "205dae5015e78dd8c4d302e3db4eb31576fac715b46d099fe09680ba28093a7a.tar.gz"
    ]
  }
]

While docker save leaves it as null, when you try to save a digest (instead of a tag):

[
  {
    "Config": "66ba00ad3de8677a3fa4bc4ea0fc46ebca0f14db46ca365e7f60833068dd0148.json",
    "RepoTags": null,
    "Layers": [
      "ad182516200d60412b9c0f8861a2f17c88c6f4976fbeaa46676542ab6197b082/layer.tar"
    ]
  }
]

But note that you are only checking a filename. Otherwise you would have to extract and sha256 it ?

PS: "crane" is the CLI of the google/go-containerregistry API

https://github.com/google/go-containerregistry/blob/main/cmd/crane/README.md

[x7upLime]

But note that you are only checking a filename. Otherwise you would have to extract and sha256 it ?

I'm checking the name of the config blob file of the image, which is synonym for the sha of its content.
The majority of container engines treat the content blob sha as the imageId.

The config blob's content (among other things), is a list of shas of the image's layers.
It responsibility of the container engine to validate the .tar archive, according to what the config blob provides..
So in any case, that is sha256'd by the containerEngine itself: I wouldn't do that again in minikube code..

The worst that can happen if that image is tampered with inside the cache(I think..):

1. modify one layer's content, 
2. re-tar the layer inside the image's .tar archive.. modifying its name accordingly inside the .tar, so that we wouldn't fail in verifying it from the container engine.
3. at this point we have another filename, that the containerEngine wouldn't find, because it's looking for what is provided by the config blob
4. We modify the name of the layer inside the config blob, without changing config blob's name..
5. At this point we can effectively load the image.. but it has a different imageId
...tried this on docker..

---

I hope you were not expecting standards.

Maybe containerEngines implementations output different things for the imageId we were looking at..
But since we're relying on the underlying oci pkg to do all the kic driver's stuff..
We can easily address there all this little differences.

In any case.. Config Blob sha persists across all implementations:

kinda what we're doing with tarball.WriteToFile()

$ docker pull gcr.io/k8s-minikube/kicbase:v0.0.37@sha256:8bf7a0e8a062bc5e2b71d28b35bfa9cc862d9220e234e86176b3785f685d8b15

$ docker save -o img.tar gcr.io/k8s-minikube/kicbase:v0.0.37@sha256:8bf7a0e8a062bc5e2b71d28b35bfa9cc862d9220e234e86176b3785f685d8b15

## then clean..
$ docker system prune --all

docker

$ docker load -i img.tar

$ docker images --no-trunc --format '{{.ID}}'
sha256:01c0ce65fff70ab1f019aa14679c46b23331bd108ae899438e589673efaa9c00

podman

$ podman load -i img.tar

$ podman images --no-trunc --format '{{.ID}}'
sha256:01c0ce65fff70ab1f019aa14679c46b23331bd108ae899438e589673efaa9c00

cri-o

$ sudo podman load -i img.tar --runtime $(which crio)
# see https://github.com/kubernetes-sigs/cri-tools/issues/707

$ sudo crictl images --no-trunc -o json | jq .images[0].id
"01c0ce65fff70ab1f019aa14679c46b23331bd108ae899438e589673efaa9c00"

containerd

$ sudo $(which nerdctl) load -i img.tar

$ sudo $(which nerdctl) image inspect --platform amd64 --format '{{.ID}}' $(sudo $(which nerdctl) images | tail -n +2  | awk '{print $3}') 
sha256:01c0ce65fff70ab1f019aa14679c46b23331bd108ae899438e589673efaa9c00

## it was to keep it in one line.. 
## I'm not entirely sure what containerd displays instead of config blob...
## But it's definitely possible to fetch that info from containerd as well.

[x7upLime]

As for our kicBase cache implementation.

We could use this contentDigest-based mechanism in a number of ways..
I started with something as easy as retrieve the config blob sha ourselves from the name in the .tar of the image.

It could be more complex tho,
like adding something like a repositories.json file (in a docker fashion -- /var/lib/docker/image/overlay2/repositories.json);
more generally.. add persistence to that information.
But then..
a user could remove tarballs to free disk space, and we should find a way to reflect that inside our (for instance..)file;
like some refreshCache() thing that we'd call at the beginning of pkg/minikube/node.beginDownloadKicBaseImage or even pkg/minikube/node.Provision, during minikube startup...

[x7upLime]

Implementation aside..
I think that, to disentangle ourselves from the containerEngine-implementation-specific mess..

Some kind of mechanism based on something as generic as the config blob, is definitely the way.

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle stale
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle rotten
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Reopen this issue with /reopen
• Mark this issue as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close not-planned

[k8s-ci-robot]

@k8s-triage-robot: Closing this issue, marking it as "Not Planned".

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Reopen this issue with /reopen
• Mark this issue as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close not-planned

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.