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

RFC: Prefer X-Registry-Supports-Signatures to lookaside #385

Closed
wants to merge 0 commits into from

Conversation

mtrmac
Copy link
Collaborator

@mtrmac mtrmac commented Nov 29, 2017

In general, a single server should never have both, so the order mostly doesn't matter. The one place where the difference is important is the ability of lookside to have a global default; if lookaside is preferred, such a global default makes X-R-S-S impossible to use.

The disadvantage of this is that simply upgrading a registry to a server version which supports X-R-S-S breaks any previously configured lookaside for the server.

Fixes #384 (?)

@TomSweeneyRedHat
Copy link
Member

LGTM

@TomSweeneyRedHat
Copy link
Member

@runcom PTAL

@runcom
Copy link
Member

runcom commented Nov 30, 2017

So, this patch effectively prefers the registry extension when supported right? If that's the case, the only broken people are the one who are already using the lookaside with a pre-sig-extension registry and they'll start to use the atomic registry. It's tricky, we could look into the sigstore to see if they're using the lookaside, and if they are, prefer that... Still an hack but maybe better than totally overriding.

@mtrmac
Copy link
Collaborator Author

mtrmac commented Nov 30, 2017

We could also integrate the two systems, so that lookaside per-repo is prioritized over either X-R-S-or lookaside configured per-host (and having both could be a hard error), which is prioritized over the global default.

That seems clean as far as pure algorithms / correctness goes, but it also feels like more effort, and more importantly more complexity that would have to be documented and understood by users, than this corner case would warrant.

@runcom
Copy link
Member

runcom commented Nov 30, 2017

We could also integrate the two systems, so that lookaside per-repo is prioritized over either X-R-S-or lookaside configured per-host (and having both could be a hard error), which is prioritized over the global default.

That seems clean as far as pure algorithms / correctness goes, but it also feels like more effort, and more importantly more complexity that would have to be documented and understood by users, than this corner case would warrant.

so maybe we can just go with this? and document somewhere (TBD) what we did

@mtrmac mtrmac force-pushed the xrss-preferred branch 2 times, most recently from 1540988 to dda11f0 Compare January 18, 2018 16:05
@mtrmac mtrmac force-pushed the xrss-preferred branch 2 times, most recently from b568abf to 6244312 Compare February 26, 2018 21:58
@mtrmac mtrmac force-pushed the xrss-preferred branch 2 times, most recently from 035e757 to e1f2498 Compare March 15, 2018 15:33
@mtrmac mtrmac force-pushed the xrss-preferred branch 2 times, most recently from 93ec26c to 8db4896 Compare April 10, 2018 18:15
@mtrmac mtrmac force-pushed the xrss-preferred branch 3 times, most recently from d36d9fe to f2040a5 Compare May 7, 2018 18:21
@mtrmac mtrmac force-pushed the xrss-preferred branch 3 times, most recently from 898c38e to bbc71bc Compare May 15, 2018 19:31
@mtrmac mtrmac force-pushed the xrss-preferred branch 2 times, most recently from a33e8d8 to c14e001 Compare June 5, 2018 19:36
@mtrmac mtrmac force-pushed the xrss-preferred branch 2 times, most recently from 29914e2 to fdcaaf2 Compare June 26, 2018 14:37
@rhatdan
Copy link
Member

rhatdan commented Dec 11, 2018

This PR is over a year old, @mtrmac What should we do with it?

@rhatdan
Copy link
Member

rhatdan commented Apr 25, 2019

@mtrmac @vrothberg @TomSweeneyRedHat What should we do with this PR? Should we rebase and move forward or should we close it?

@mtrmac
Copy link
Collaborator Author

mtrmac commented Jun 1, 2019

Rebased, but #385 (comment) is still outstanding (at least to consider, if not to actually implement).

@mtrmac mtrmac force-pushed the xrss-preferred branch 3 times, most recently from 8d2cf14 to 4507206 Compare June 20, 2019 18:32
@vrothberg
Copy link
Member

@mtrmac, can we use the registry whenever it supports storing signatures in addition to a potentially configured sigstore? This way, we should remain backwards compatible and would support https://bugzilla.redhat.com/show_bug.cgi?id=1724946. We might want to compare signatures when fetching from two sources.

@mtrmac
Copy link
Collaborator Author

mtrmac commented Sep 2, 2019

Interesting… that’s clearly safe enough for reading (which is where we don’t have a look-aside default, though). It’s not obviously the right thing to do on writes, where we do, but it might be good enough.

In particular, the OpenShift implementation of X-R-S-S has a pretty low limit on the number of allowed signatures (used to be as little as 3!), so it could cause unwanted failures.

@mtrmac
Copy link
Collaborator Author

mtrmac commented Sep 2, 2019

(Generally speaking, the current default/preference is mostly just wrong, and we should switch to making the sigstore opt-in for any X-R-S-S registries. Still, the potential for breaking users who rely on the current setup is there.)

@sjhx
Copy link
Contributor

sjhx commented Apr 8, 2020

I can only offer tea and sympathy on the "breaks existing" problem however I can say that we are bringing on new users (especially via skopeo) who need to take special care to reset the registries.d settings, most remove or comment out the supplied default.yaml. Progress on prioritising XRSS would help us the experience.

@mtrmac mtrmac force-pushed the xrss-preferred branch 2 times, most recently from e74fd41 to 5986827 Compare May 4, 2020 17:03
@rhatdan
Copy link
Member

rhatdan commented Jun 3, 2020

Where are we on this?

@mtrmac
Copy link
Collaborator Author

mtrmac commented Oct 26, 2020

This was done in #1035.

@mtrmac mtrmac deleted the xrss-preferred branch October 26, 2020 18:41
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.

Try signature extensions in registry before using the docker sigstore
6 participants