Add namespace query parameter for registry mirroring/proxying #148
Conversation
SuperTux88
requested review from
thomastaylor312,
bacongobbler and
flavio
as code owners
❤️ |
| /// | ||
| /// The mirror registry will be used to resolve the image, the original registry | ||
| /// is available via the [`Reference::ns`] function. | ||
| pub fn set_mirror_registry(&mut self, registry: String) { |
There was a problem hiding this comment.
We should probably have this and the ns function be #[doc(hidden)] with information in their doc strings about this not being stable yet (and as such, is exempt from semver backwards compat guarantees). I am perfectly fine with it being in here if things like containerd are already using it, but I want to make it clear that this isn't fully supported yet. The other option would be to have something like a ClientExp trait that the client could implement for experimental features, but that also feels a bit extra.
Like you mentioned, it looks like it will land, but things can take a while in the OCI spec, so I want to be extra careful here
There was a problem hiding this comment.
Sure, I added the #[doc(hidden)] and added a note about it not being stable yet, and also added a note that the ns query parameter might not work with all registries yet.
SuperTux88
force-pushed
the
ns-parameter
branch
from
b975ad5 to
53e275b
Compare
You can pull images via a pull-through proxy, this will change the resolved registry for the actual request, and the original registry of the image gets forwarded as a hint for the upstream registry in the "ns" query parameter. Signed-off-by: Benjamin Neff <[email protected]>
This allows to create a copy of the reference but setting a new digest. This also copies the registry_mirror, so the digest also gets pulled from the same mirror. Signed-off-by: Benjamin Neff <[email protected]>
Don't use a fixed digest algorithm length. Signed-off-by: Benjamin Neff <[email protected]>
SuperTux88
force-pushed
the
ns-parameter
branch
from
53e275b to
1ec27cc
Compare
This adds the ability to set a mirror to the
Referenceinstance, which is then used to pull the image, instead of the registry that was parsed from the image name. And it then forwards the original registry with thensquery parameter to the mirror registry, so the mirror registry can use this information to decide from which registry you wanted to pull something.This is currently not an officially documented standard feature yet, but it's probably going to be eventually, it's just taking a really long time (opencontainers/distribution-spec#66). But containerd is already using the
nsquery parameter and I think I read somewhere that nexus is also already using the parameter when used as pull through cache (but I haven't verified that). But I think it's a really useful feature (when working with mirrors/proxies), even if it's not officially part of the spec yet, so I also implemented it to the library here.Some additional notes:
Referenceinstance also for pushing the same image again (would that even make sense if you just pulled it). So I decided to not add additional complexity for that use-case in the code for now. For people who don't set a mirror registry, everything works the same as before and pushing works. And if it's really needed, this logic can always be added later, but I wanted to keep the code simpler. But I'm open to feedback.set_mirror_registry()function anOption<String>, so you can also unset the mirror again, but for my use-case I don't need it to be unset. But let me know what you think and I can change it.cargo fmtformats therstestat thetest_mirror_registrylike that, but doesn't do it for the other tests 🤷♂️Also: The last commit doesn't have anything to do with this feature, but when scrolling through the code while adding this feature, I saw the
FIXMEand thought "why not just fix that as well?" 😃