oras-project / rust-oci-client

A Rust crate to interact with OCI registries
Apache License 2.0
93 stars 49 forks source link

Add support for the signatures registry API extension #97

Open mattarnoatibm opened 12 months ago

mattarnoatibm commented 12 months ago

OpenShift and IBM Cloud container registries support a signatures API extension where Red Hat Simple Signing signatures for an image digest can be stored in and fetched from the container registry.

We could add support for this extension to the OCI distribution client.

Under this issue we might also want to design how the oci-distribution Rust library should support extensions to the OCI distribution spec geerally.

Refs:

thomastaylor312 commented 11 months ago

So I am not personally familiar with the RSS (RHSS?) extension, but I am definitely not opposed to adding in registry extensions. The main thing I'd want to figure out before reviewing #98 is how we decide on which extensions to include. Given that this specific extension seems to be relatively well-known, it feels like it would fit, but I also don't want to get to a point where we include every extension anyone would ever use

@flavio I'm curious if you have any thoughts here on how we should weigh extensions for inclusion in the main crate or if there is another way we could possibly handle them

thomastaylor312 commented 11 months ago

I don't think I see extensions here https://github.com/google/go-containerregistry/tree/v0.16.1/pkg/v1 or here: https://pkg.go.dev/github.com/docker/docker@v24.0.6+incompatible/distribution, so I am not quite certain how this is done in other clients

flavio commented 11 months ago

I didn't know this mechanism of registry API extensions. From what I understand this is something specific to the registry used by OpenShift. @mattarnoatibm am I right?

I have mixed feeling about getting this code into the main crate. Right now the code from the PR is pretty small and enabled only via a new dedicated feature flag.

The PR adds support for pulling and validating the signatures, but it doesn't seem to allow the creation/push of these objects. Am I right? I wonder how big this specific code could become in the future.

As for having this feature into a separate crate. I looked at the code added by #98. I think this could be moved to a different crate. We would have to extend the Client::auth return signature to return the authentication token. All the other operations done by the PR can be performed with a vanilla reqwest.Client and the auth token.

What do you think?

thomastaylor312 commented 11 months ago

That seems to be the simplest way forward to me and avoids taking on all the code into this crate. As things evolve, we could also make this easier by creating some sort of trait that other crates could implement

mattarnoatibm commented 11 months ago

Hi Flavio, the signatures API extension is used by OpenShift container registries and IBM Cloud container registries. For API extensions to the OCI Distribution Spec in general, I think anyone can write one but I must admit I'm not aware of any extensions apart from the one for signatures.

The signatures API extension supports a PUT endpoint, where you can push a Simple Signing signature to a container registry. I think the PUT and GET (for retrieving the signatures for a digest) are the only endpoints the signatures API extension introduces.

For moving the feature to a separate crate, is there more than Client::auth returning the authentication token that should be added to the oci-distribution crate and exposed for other crates to use? For example, would it be better to use the RequestBuilderWrapper pattern from the oci-distribution crate client.rs than a vanilla reqwest.Client?

flavio commented 10 months ago

Sorry for the late response :disappointed:

For moving the feature to a separate crate, is there more than Client::auth returning the authentication token that should be added to the oci-distribution crate and exposed for other crates to use? For example, would it be better to use the RequestBuilderWrapper pattern from the oci-distribution crate client.rs than a vanilla reqwest.Client?

I think the RequestBuilderWrapper isn't doing anything fancy, you could achieve all of that with a basic reqwest.Client. Moreover, making RequestBuilderWrapper public doesn't look good to me.

I would propose to create a feature branch where we implement the changes proposed above. You can then try to consume this feature branch and see if you have all the things needed.

Once you're happy with that we can merge the feature branch into main and tag a new release of oci-distribution.

Does it sound reasonable to you?

mattarnoatibm commented 10 months ago

Hi Flavio.

Also sorry for the late response, due to vacation I've been taking.

I'd be happy to update the oci-distribution crate as you propose so Client::auth returns an authentication token. I'll try to consume that updated Client::auth function from my feature branch in the image-rs crate of the confidential-containers/guest-components repo.

mattarnoatibm commented 10 months ago

Opened a PR and new issue for Client::auth returning an auth token.

flavio commented 10 months ago

@mattarnoatibm good, if this is all you need to be unblocked I'll be happy to merge it and tag a new release of the crate