-
Notifications
You must be signed in to change notification settings - Fork 57
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
Import reference #207
Import reference #207
Conversation
Signed-off-by: Matthew Fisher <[email protected]> Signed-off-by: Colin Walters <[email protected]>
This commit performs the following changes to the Reference struct: * Get rid of the custom implementation of the `Debug` trait, this made less obvious the values of the different attributes that are part of the struct. We now rely on the derived implementation of the trait, which proves to be more useful * Replicate the behaviour of the official Docker libraries when parsing the Reference from a string. Prior to this commit the code that parsed the name of a container image name made some mistakes. For example, parsing |opensuse/leap:15.3| lead to: * Old code: Reference { registry: "opensuse", repository: "leap", tag: Some("15.3"), digest: None } * New code: Reference { registry: "docker.io", repository: "opensuse/leap", tag: Some("15.3"), digest: None } As you can see, the previous code mistakenly thought the image was not hosted on the Docker Hub. The wrong behaviour was caused by the code the splits the domain name from the repository. This commit changes the old implementation and instead relies on a Rust port of the official code used by Docker. As a side effect (which IMHO is a plus), the Reference object is now populated in a more "verbose" way. The Docker Hub is a "special place", with official images living in the "library" repository and with the domain name "docker.io" that can also be omitted. The parsing code has also been changed to ensure the `latest` tag is automatically assigned to a Reference when the input image name doesn't have neither an explicit tag nor a digest. The unit tests have been updated to reflect these changes. They are also useful to highlight what is changed by this commit: DIFF parsing |busybox| Old code: Reference { registry: "", repository: "busybox", tag: None, digest: None } New code: Reference { registry: "docker.io", repository: "library/busybox", tag: None, digest: None } DIFF parsing |test.com:tag| Old code: Reference { registry: "", repository: "test.com", tag: Some("tag"), digest: None } New code: Reference { registry: "docker.io", repository: "library/test.com", tag: Some("tag"), digest: None } DIFF parsing |test.com:5000| Old code: Reference { registry: "", repository: "test.com", tag: Some("5000"), digest: None } New code: Reference { registry: "docker.io", repository: "library/test.com", tag: Some("5000"), digest: None } DIFF parsing |lowercase:Uppercase| Old code: Reference { registry: "", repository: "lowercase", tag: Some("Uppercase"), digest: None } New code: Reference { registry: "docker.io", repository: "library/lowercase", tag: Some("Uppercase"), digest: None } DIFF parsing |foo_bar.com:8080| Old code: Reference { registry: "", repository: "foo_bar.com", tag: Some("8080"), digest: None } New code: Reference { registry: "docker.io", repository: "library/foo_bar.com", tag: Some("8080"), digest: None } DIFF parsing |foo/foo_bar.com:8080| Old code: Reference { registry: "foo", repository: "foo_bar.com", tag: Some("8080"), digest: None } New code: Reference { registry: "docker.io", repository: "foo/foo_bar.com", tag: Some("8080"), digest: None } DIFF parsing |opensuse/leap:15.3| Old code: Reference { registry: "opensuse", repository: "leap", tag: Some("15.3"), digest: None } New code: Reference { registry: "docker.io", repository: "opensuse/leap", tag: Some("15.3"), digest: None } Signed-off-by: Flavio Castelli <[email protected]> Signed-off-by: Colin Walters <[email protected]>
Address some of the warnings returned by clippy Signed-off-by: Flavio Castelli <[email protected]> Signed-off-by: Colin Walters <[email protected]>
Signed-off-by: Tom Fay <[email protected]> Signed-off-by: Colin Walters <[email protected]>
Address clippy warnings Signed-off-by: Flavio Castelli <[email protected]> Signed-off-by: Colin Walters <[email protected]>
This commit updates the default endpoint for DockerHub to be `index.docker.io`. This is the default endpoint that the Docker CLI is using. Running `docker info`, this is the endpoint returned for the registry: ``` $ docker info | grep Registry Registry: https://index.docker.io/v1 ``` This means we can now push references to `index.docker.io/user/repo:tag`. Signed-off-by: Radu Matei <[email protected]> Signed-off-by: Colin Walters <[email protected]>
Closes youki-dev#142 Signed-off-by: Taylor Thomas <[email protected]> Signed-off-by: Colin Walters <[email protected]>
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]> Signed-off-by: Colin Walters <[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]> Signed-off-by: Colin Walters <[email protected]>
Don't use a fixed digest algorithm length. Signed-off-by: Benjamin Neff <[email protected]> Signed-off-by: Colin Walters <[email protected]>
Almost all tools which operate with OCI will end up wanting to parse and use proper references. Import the code from the rust-oci-client crate. I imported the code with history with this blog which I found helpful: https://savorywatt.com/2015/01/25/move-files-and-folders-between-git-repos-using-patches/ Changes: - Move `regex.rs` code inline into this file - Switch to std `LazyLock` instead of `lazy_static` crate For more see: - youki-dev#205 - oras-project/rust-oci-client#159 Signed-off-by: Colin Walters <[email protected]>
// Digests much always be hex-encoded, ensuring that their hex portion will always be | ||
// size*2 | ||
if let Some(digest) = reference.digest() { | ||
match digest.split_once(':') { |
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.
This could also clearly use 5601907 etc. but probably best to keep code changes minimal until we've deduplicated.
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.
Yep I agree here. We just had this as a string for simplicity, but it would probably be better to have it be a concrete digest type. Definitely going to be easier to wait until this is merge though
I'm totally in favor of this PR. Once merged I'll update |
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.
@utam0k PTAL. I just have a non-blocking nit.
/// Returns the name of the registry. | ||
pub fn registry(&self) -> &str { | ||
&self.registry | ||
} | ||
|
||
/// Returns the name of the repository. | ||
pub fn repository(&self) -> &str { | ||
&self.repository | ||
} | ||
|
||
/// Returns the object's tag, if present. | ||
pub fn tag(&self) -> Option<&str> { | ||
self.tag.as_deref() | ||
} | ||
|
||
/// Returns the object's digest, if present. | ||
pub fn digest(&self) -> Option<&str> { | ||
self.digest.as_deref() | ||
} |
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.
I guess we could use getset
for those.
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.
Thanks for pulling this all in! Looking forward to using it downstream in oci-client
Reusing this PR as a communication channel...I think it'd probably make sense to add some folks who maintained/wrote the reference type as reviewers on this repository right? @flavio will you be able to watch for changes on this repo too? Any others to add? |
Fine with me. @thomastaylor312 could be interested too |
@saschagrunert ok to add those two folks to have write/reviewer access here? On a related topic I notice this repository (unlike others in the containers/ ecosystem) doesn't have branch protection rules set up which we really should have, i.e. require a PR to push to git main etc. Any objections to me also adding those? |
Yes! Having more maintainers would be awesome within this project.
Sure feel free to add it, thanks you for double ckecking! |
I have invited flavio and thomastaylor312 with write role to this repo, and I have added branch protections that:
Which I think is a standard baseline and we now have enough people to match that criteria. |
Thank you @cgwalters! 🙏 |
Related to this...as this project has grown some, does anyone have thoughts on possibly submitting an application to the CNCF ? I think it'd be a good home for a project such as this; we're a multi-vendor/organization ASL2.0 project. (And if we do I'd like to add https://github.com/containers/ocidir-rs/ too) |
@cgwalters would that project fit more specifically into the OCI? |
Seems likely yep |
Import Reference type from rust-oci-client
Almost all tools which operate with OCI will end up
wanting to parse and use proper references. Import
the code from the rust-oci-client crate.
I imported the code with history with this blog
which I found helpful:
https://savorywatt.com/2015/01/25/move-files-and-folders-between-git-repos-using-patches/
Changes:
regex.rs
code inline into this fileLazyLock
instead oflazy_static
crateFor more see:
Signed-off-by: Colin Walters [email protected]