Non-blocking: It looks likeWithDefaultTag checks IsNameOnly before adding the default tag, so possibly this extra reference.IsNameOnly(ref) check is not necessary? (Update: apologies I forgot to delete the first half of this comment - I originally thought this was in distribution's code)

Fixed

Should service create fail if the image fails to resolve the reference against DCT (similarly to docker run?)

Hmm. That's a tough one. I can see the security argument for doing it, since otherwise a denial of service can cause a downgrade to non-trusted resolution (by making it happen on the daemon). However, there are valid use cases that involve specifying images that have never been pushed to a registry, and the client won't be able to know if the images already exist on the worker nodes.

Should we require content trust to be turned off to support local image use cases?

That's true - and it's not a great experience on non-swarm docker atm, where you can't run images that are are built but not pushed. I lean towards requiring content trust to be turned off, just because it seems like it'd be easier to go from tighter security down, and also because it'd be consistent with docker run behavior, and we probably need to work out a better model for the local image case anyway.

Out of curiosity, docker build happens on a single worker node at a time, right? Does scheduling take into account which nodes have the image locally, or replicate the local images if the service needs to scale?

Out of curiosity, docker build happens on a single worker node at a time, right? Does scheduling take into account which nodes have the image locally, or replicate the local images if the service needs to scale?

No, none of this is automatic. Typically a scenario using an image that's not pushed to a registry would be a single node swarm, or a few nodes where the image was manually copied in advance.

Changed to fail the "service create" or "service update" command if digest resolution fails when using content trust.

Should we leave this as it is? Maybe it is worthwhile to have a log message specifying which digest the service was pinned to upon creation, even if the client provided a digest to begin with.

I noticed this message while testing and found it quite strange and misleading. It's of the form:

pinning image busybox:latest@sha256:29f5d56d12684887bdfa50dcd29fc31eea4aaf4ad3bec43daf19026a7ce69912 by digest: busybox:latest@sha256:29f5d56d12684887bdfa50dcd29fc31eea4aaf4ad3bec43daf19026a7ce69912

I think it would be okay to have a debug log message noting that pinning by digest is being skipped because the image reference already has a digest. But saying that we're doing digest pinning when we're actually not touching the image reference is misleading.

"service update" already skips this log message when there is no change to the reference. I thought it was an oversight that "service create" logs it anyway.

I see your point. I had deliberately kept the log message on create but didn't consider how it can look misleading.

Perhaps it's reasonable to add a separate message when no pinning happens, such as creating service with image digest busybox:latest@sha256:29f5d56d12684887bdfa50dcd29fc31eea4aaf4ad3bec43daf19026a7ce69912, and leave the existing message for the default case.

Either way, I don't feel too strongly about it.

Perhaps it's reasonable to add a separate message when no pinning happens

Added a debug log message in this case for create and update.

... but it's not showing up because GitHub is broken

remote: Unexpected system error after push was received.
remote: These changes may not be reflected on github.com!
remote: Your unique error code: ...

Resolved the GitHub issue by force-pushing.

Non-blocking question regarding this comment: by image ID do you mean image name? If not, what is the syntax for passing the ID?

It's what you see under Id in docker inspect. An example is:

sha256:a95d886ecdf2cda6a61eef160ca61be49c56d005c7b5fec048706e4a12e53eb7

In other words, if the digest package validates it as a digest, we treat it as a content-addressable image ID. Since neither notary nor registry can fetch images by their ID, this avoids talking to notary/registry to query it. Images referenced in this way will only work if the image already exists locally.

Ah ok, so not the truncated hex displayed by docker images. I had interpreted "digest reference" to mean something like sha256:a95d886ecdf2cda6a61eef160ca61be49c56d005c7b5fec048706e4a12e53eb7, so I though image ID was something else.

Does "digest reference" instead mean distribution/reference.Named, which can have a digest?

Does "digest reference" instead mean distribution/reference.Named, which can have a digest?

Correct. An example is busybox@sha256:29f5d56d12684887bdfa50dcd29fc31eea4aaf4ad3bec43daf19026a7ce69912

Thanks for the explanation!