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

Add support for the signatures registry API extension #97

Open
mattarnoatibm opened this issue Oct 10, 2023 · 9 comments
Open

Add support for the signatures registry API extension #97

mattarnoatibm opened this issue Oct 10, 2023 · 9 comments

Comments

@mattarnoatibm
Copy link
Contributor

mattarnoatibm commented Oct 10, 2023

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
Copy link
Contributor

thomastaylor312 commented Oct 20, 2023

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
Copy link
Contributor

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/[email protected]+incompatible/distribution, so I am not quite certain how this is done in other clients

@flavio
Copy link
Contributor

flavio commented Oct 25, 2023

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?

@flavio flavio mentioned this issue Oct 25, 2023
@thomastaylor312
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor

flavio commented Nov 15, 2023

Sorry for the late response 😞

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@flavio
Copy link
Contributor

flavio commented Nov 23, 2023

@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

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

No branches or pull requests

3 participants