Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add info about image resolution with content trust #1065

Merged
merged 1 commit into from
Jan 12, 2017
Merged

Add info about image resolution with content trust #1065

merged 1 commit into from
Jan 12, 2017

Conversation

mdlinville
Copy link

@mdlinville mdlinville commented Jan 9, 2017

Proposed changes

Added info about how image resolution works when content trust is enabled

Unreleased project version (optional)

Engine 1.13

Fixes #1004

@mdlinville mdlinville added this to the engine/1.13.0 milestone Jan 9, 2017
@cyli
Copy link
Contributor

cyli commented Jan 9, 2017

This LGTM - cc @aaronlehmann who added this fix.

or your registry. You can force the service to use a specific version of
the image in a few different ways, depending on your desired outcome.
or your registry, or using Notary if content trust is enabled. See the
[note about content trust](#image_resolution_with_trust).You can force the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the sentence about creating a service is becoming long, and mixes different concepts.

The original sentence was saying that if you omit a tag, it will be treated as <image>:latest. Instead of saying "the latest version", I'd change this to say "the latest tag", since :latest may not actually be the most recent version.

We also want to say that if content trust is enabled, Notary is used to authenticate the tags. This happens whether you specify a custom tag, or use the default of latest.

There's a missing period after the link to the note.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed the feedback by removing the links to the note, and just having the note at the bottom. I think it's more clear that way. Let me know what you think.

- If you specify a tag, the manager resolves that tag to a digest, and directs
workers to uses that version.
- If you specify a tag, the manager resolves that tag to a digest. If you are
using [content trust](#image_resolution_with_trust), Notary verifies that the
Copy link
Contributor

@cyli cyli Jan 11, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should say "the docker client" instead of "Notary"? Or "the docker client uses notary to verify..."? It both verifies that it is signed and also resolves the tag to a digest. The manager does not resolve the tag in this case (if content trust is enabled). This is mentioned in the note already, though, so not sure if that would be repetitive and/or we should just mention it here, rather than the note.

Also, wondering if we should link to https://docs.docker.com/engine/security/trust/content_trust/ when saying "signed" so users can look up more detail about how that works?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update: Ah right, sorry, that note goes with https://github.com/docker/docker.github.io/pull/1065/files#diff-77d91a1ccd7779ca36fd0f5a1cfd5353R127.

Hm... maybe that sentence can say something like: "When you create a service, if you don't specify a digest directly, the image's tag gets resolved to the specific digest it points to at the time of service creation..." ? Thus avoiding the manager vs client resolution in that sentence.

Copy link
Contributor

@nishanttotla nishanttotla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mdlinville
Copy link
Author

@cyli can you give this another look-over? I think I addressed your feedback.

@mdlinville mdlinville merged commit ab6dea6 into docker:vnext-engine Jan 12, 2017
@mdlinville mdlinville deleted the service_create_with_content_trust branch January 12, 2017 23:47
bermudezmt added a commit that referenced this pull request May 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants