-
Notifications
You must be signed in to change notification settings - Fork 18.6k
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
experimental: always use trust for official images #20844
Conversation
// true if trust is enabled and we are not already doing a pull by digest, | ||
// and also returns true if it is an official image being that is not | ||
// already being pulled by digest | ||
func trust_should_lookup(byDigest, official bool) bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why trust_should_lookup
instead of trustShouldLookup
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ugh, yeah, was naming the files with Docker's existing naming scheme and fell back into Python syntax. Will fix now
👍 on design. I think using content trust for official images in experimental is a good step. |
6081a17
to
5d3fea5
Compare
Since the signing of the official images is currently a separate and manual process that is not managed by the team in charge of building and pushing the official images (and thus they might be outdated depending), I'm honestly a bit -1 on this idea until/unless that obstacle can be overcome. 😕 |
@tianon there is active work being done on auto-signing based on registry notifications for official images. Hence experimental. |
While I appreciate the idea of (and work going into) auto-signing, why aren't we working on signing them directly from the source before they hit the Hub? What're the technical or organizational challenges with doing so and how can we overcome them? |
@icecrime pushed this so the tests could run while I got on with other stuff. Marking as WIP while I fix the tests. The failures were expected but I wasn't sure exactly what they would look like. |
👍 @tianon, it would definitely garner more trust if the images are actually signed where they are built rather than after a push and pull. |
@tianon / @yosifkit - what if you guys sign all the That way even if it's a manual process until |
It should not be necessary to need a second signing. From all of the documentation I have read, there are two related avenues of approach that would keep the root keys safe while allow the build server to directly sign images.
|
@yosifkit @toli @tianon This is a good discussion but seems a bit orthogonal to this PR. I opened an issue (docker-library/official-images#1516) to discuss and (eventually) document this process. |
@NathanMcCauley I agree with the discussion being orthogonal, but I still feel that it would be foolish to consider this PR while the signing process for the official images is still so far from ideal, which is what I was trying to convey; thanks for filing that issue -- my own complaints for this PR as-is are now noted and I'm happy to continue the rest of this particular discussion over there 👍 |
7abbf5a
to
a1f0c13
Compare
48c7a86
to
148f0c5
Compare
The windows test is now failing for different reasons each build, all of them seemingly related to the environment in which the tests are running, rather than anything related to my changes... |
Signed-off-by: David Lawrence <david.lawrence@docker.com> (github: endophage)
bc92f91
to
08a8f54
Compare
@thaJeztah can I call on your powers of closure to help get this through now the windows tests have finally managed a clean run and before I'm forced to rebase it again. |
@endophage I'll see if I can get it moving. First step IMO is a 👍 from @tianon, who is both a maintainer here, and involved in all the official images |
@tianon I know there have been discussions going on elsewhere. Do you feel we can get this into experimental around the docker 1.11 release and resolve those discussions in the docker 1.12 timeframe? |
Just a reminder that experimental is not tied to any release. I think we should moved this forward, but I'll wait for @tianon to comment. I also agree that we should work in parallel to have signing of official images in the source, letting the official images maintainers to handle it. |
There's some work in progress to get our signing situation closer to something sane (instead of a manual process), but we're not quite there yet (still in the discussion phases, and there's some development going on currently to automate the existing process); IMO, automating the current signing process is still a strong prerequisite to claiming that the "signed" official images are actually recommended for use. |
@calavera yep, we just want to get ~1 full release cycle of testing so that docker 1.12 becomes a possible target for moving it to stable. Hence I'm bugging people in an effort to get it into experimental around the 1.11 release. |
Looks like @tianon is ok with the current approach (docker-library/official-images#1516 (comment)), so moving to code review |
(but feel free to comment if we interpreted incorrectly @tianon ❤️) |
I'm OK with the plan that's in progress to get us there, but we aren't
there yet.
|
Don't get me wrong I am all for this idea... But it cant be a worse experience than normal |
Ok but seriously on master w DCT enabled I cant even build images:
Is anyone actually setting this on their computer and if so I'm wondering what impressions they have Then if I build something locally I cannot run it. |
I cannot even run images that I have not pulled:
|
so I understand that this patch is not enabling all of it, but a lot of these walls are going to be hit with it and this cannot be the behavior |
+1 @jfrazelle Absolutely need to get these kinks worked out. Thanks for the feedback. |
ping @endophage needs a rebase I think the authentication issues that @jfrazelle mentioned have been resolved in the meantime? |
The problems with build and run not working at all were due to a bug unrelated to this PR (but related to trust). That has been fixed. Other issues related to how build and run operate with trust are being discussed at the moment. I'll get around to rebasing shortly :-) |
i think just the none thing still needs to be fixed, and sometimes official images are up and not signed which will cause a lot of people to break if its default |
ping @endophage what's the status on this one? |
@endophage I'll close this for now, please ping me when you come around to updating it, then I'll reopen |
Yep, that's fair enough. I don't think just fixing the thing is enough. We need to work out something for build and run too. |
Should all these be made separate issues so they know to be fixed? |
We would like to enable content trust for all official images. For now we just want to put it in experimental so we can try and get a little more activity on the trust servers and work on real time signing.
Because a number of the pull tests inspect the output for certain strings, I had to move those tests into files with the
!experimental
build tag. I then created copies of those tests that look for the modified, trust related, output and put them in files with theexperimental
build tag.I gave the pairs of test files similar names differing only in
stable
orexperimental
in the filename. Happy to change thestable
for something else,experimental
(in the filename) and the use of build tags already seemed to be in use.Signed-off-by: David Lawrence david.lawrence@docker.com (github: endophage)