Skip to content
This repository has been archived by the owner on May 3, 2022. It is now read-only.

feat(*): add digest to invocation image on build #691

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

michelleN
Copy link
Contributor

resolves #690

@carolynvs
Copy link
Contributor

Does this digest replace having to push to a registry?

@michelleN
Copy link
Contributor Author

Yea, this implementation of digest is decoupled from the registry. You can still push to registry and update the image tag with the registry digest if you want to validate the digest in the image manifest every time you do a docker pull though. @carolynvs

@jeremyrickard jeremyrickard self-requested a review April 1, 2019 21:46
@radu-matei
Copy link
Member

FYI, the builder test seems to be failing:

pkg/builder/builder_test.go:74:13: cannot use testImage literal (type *testImage) as type imagebuilder.ImageBuilder in array or slice literal:
	*testImage does not implement imagebuilder.ImageBuilder (wrong type for Build method)
		have Build(context.Context, io.WriteCloser) error
		want Build(context.Context, io.WriteCloser) (string, error)

@michelleN michelleN force-pushed the build-digest branch 2 times, most recently from 8ddfcf6 to 140e400 Compare April 5, 2019 00:33
@michelleN
Copy link
Contributor Author

Fixed those tests and rebased for the updated linter issues so we should be all good now!

@simonferquel
Copy link
Contributor

The problem here, is that this digesting algorithm is not the one used by a registry. We can't build a digested reference to the image in the registry out of this :(.
There is no way with the Docker API to know in advance the digest that a particular image will have once pushed to the registry. It might be doable but in a very brittle way (it involves gzipping in exactly the same way as the daemon every layer, creating a manifest in exactly the same way the daemon does that referencing the config+layers of the image, and digesting it). The layer compression algorithm used by the daemon is undocumented, and thus subject to changes.

I don't see any reasonable solution for that, other than by not using the daemon for pushing the invocation image.
What we do with docker-app, is that when in dev mode, all our images are undigested, and we only fix the digests on push.

@michelleN
Copy link
Contributor Author

@simonferquel I did quite a bit of digging into how to mimic the digest that the registry uses but came to the same conclusion as you in that it would be a brittle solution and very complex.

The solution in the PR however is not tied to the registry at all. It simply creates a sha256 digest of the of the image tar (replicated locally with docker save <image> -o <file>.tar then shasum -a 256 <file>.tar). If I push this image to a remote registry (DockerHub, Quay.io), delete the image from my local docker store and pull it back down, then I will still compute the same exact same digest on the image I just pulled down using docker save and shasum -a 256

I think we're on the same page in that when a remote registry is involved that the push logic for a bundle should push the images and update the image digest. However, it makes most sense to me to make this adjustment in the image reference itself. So update the image for example from something like deislabs/helloworld-cnab:0.1.0 to deislabs/helloworld-cnab@sha256:326f38ad43b35f081e9470e5941113cdab4eece4a9bf24925995bd50a6be53fd because

  1. we'll have a content addressable tag vs. a mutable tag that can be overwritten easily
  2. the docker pull will also calculate the digest of the thing we pulled and verify it matches that content addressable tag.

Putting the digest that gets computed by the registry in the digest field on the image in the bundle descriptor seems redundant to me. That's why I was looking for a digest that is not tied to the registry at all to also help validate bundles that are developed and packaged locally and moved around without any registry involvement at all. For example, if you build a bundle locally using duffle build and then export that bundle and import it on to another machine via duffle export and duffle import.

In that scenario, there should be no need for a registry component but there is a need to
1, validate that the invocation image you're about to use for duffle install is in fact the image you mean to use. install will use the image referenced in the bundle manifest. Need some way to validate the contents of the image bc the tag is mutable
2. that the image you export is the one you intend to export. Need some way to validate that we're grabbing the right image from your local store. Using the image reference via repo:tag is not enough bc that can easily be overwritten with a docker build on the command line.
3. that the image(s) you import are the ones you mean to import (same mutable tag story as export)

Given that context, is there something I am missing or mis-understanding about docker save and registries that you know?

@michelleN
Copy link
Contributor Author

I can look into implementing digest with the containerd libraries you pointed out to me to attain a digest that doesn't change when changing image repo/name

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
WIP work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

duffle push should add invocation image digest to bundle.json if it doesn't exist
4 participants