return reference.WithDigest(ref, dgst).String(), nil

Fixed.

I'd suggest ignoring confirmedV2 here and attempting the Tags(ctx).Get() anyway. Worst case, it fails.

Sounds fair. The check doesn't provide any particular advantages anyway.

Return the Digest object rather than the stringified version.

The if is not necessary - you can just do ref = reference.WithDefaultTag(ref) since WithDefaultTag is defined as follows:

// WithDefaultTag adds a default tag to a reference if it only has a repo name.
func WithDefaultTag(ref Named) Named {
        if IsNameOnly(ref) {
                ref, _ = WithTag(ref, DefaultTag)
        }
        return ref
}

You're right. I think I missed this after some refactoring.

namedTaggedRef, ok := ref.(reference.NamedTagged)

Return an error if ok is false (should not be possible currently, but the reference package could change in the future).

This should probably be a Debugf, and ideally should provide more context about what was pinned. I'd suggest creating a logger at the top of this function with logrus.WithFields. Then if you use that logger for the log messages, they can have the service name, image reference, and so on filled in.

I moved these log messages outside this function, which now only returns an error, and the caller decides what to do with it.

Move this above the if encodedAuth != "" { so cntr can be reused there.

This would log an error message if the user already provided a digest. How about making imageWithDigestString return image, nil if the image already has a digest specified, and then this code can only log "pinning image..." if digestImage != newCntr.Image?

Don't make the error path in the main path. Get in the habit of making an early return.

Why wouldn't we return the canonical reference if it is already canonical?

if canonical, ok := ref.(reference.Canonical); ok {
  return canonical, nil
}

encodedAuth should really be cross-cutting. Perhaps, store in the backed or preconfigure an object with the auth before calling this method.

@stevvooe if I understand correctly, passing the types.AuthConfig struct to this function would be acceptable?

That is probably a first step. It is hard to tell, since it is not actually being used. I have no clue why we would bury an unencoded value so deeply in the call stack. The auth should probably be decoded as soon as the request is received. Then, I would put it in the context and access as needed, without passing it down the entire call chain.

Rather than add yet another method to daemon, why not export access to the configured repository? As it is, this method is very narrow and not very flexible. Ideally, we give it an image reference and get back fully configured repository.

Will make this function return a fully configured repository to expand its usability.

if err != nil {
log warning
} else if digestImage != ctnr.Image {
log debug(pinning)
ctnr.Image = digestImage
}

is this expected not to error out?

Any operations related to pinning by digest aren't supposed to error out right now, because some people use local images in dev, and this would break that workflow.

This should return lastError instead of nil, it is possible that the last endpoint returned an error and caused repository to be nil.

My bad, good catch @dmcgowan