Conversation
| type Client struct{} | ||
|
|
||
| func New() (*Client, error) { | ||
| return &Client{}, nil | ||
| } |
There was a problem hiding this comment.
I think we could construct a *remote.Puller here and use it for the remote operations in Tags. This will cache auth across different calls, reducing the potential number of requests made for each operation.
See:
https://github.com/google/go-containerregistry/blob/main/pkg/v1/remote/puller.go
There was a problem hiding this comment.
Yeah, I had this in mind for the future in building this out to be a fully fledged client in it's own right, but I didn't want to do it here as a first pass - mostly I try to work in incremental value, rather than hitting too many things at once.
ribbybibby
left a comment
ribbybibby
left a comment
There was a problem hiding this comment.
Left some comments.
Two other things worth mentioning:
- Auth. By default, ggcr will try to find credentials in the environment, for instance in
~/.docker/config.json. I reckon we should document this, so users are aware that they can make creds available like that. - Tests. It's pretty straightforward and powerful to use the the ggcr registry for testing. You can see an example of that in paranoia.
OCI registries don't provide a way to retrieve metadata in the same call as listing tags. This means that you have to do a separate API request for each tag if you want the metadata, which could be very slow for a large number of tags. As such, preferably use the selfhosted client as a fallback, but add a fallback fallback to the oci handler for registries that are incompatible with the selfhosted API implementation.
As we might not have retrieved a SHA (dependent on registry type), don't try to include one if it's empty.
|
I'll probably look to add tests in a separate PR - As there's not currently a comprehensive testing framework, I think bringing tests into this PR might make it difficult to review. |
davidcollom
left a comment
davidcollom
left a comment
There was a problem hiding this comment.
Now that I've had chance to review.. Looks good to me - Appreciate the approach of falling back and keeping others - I may have some time to complete the auth and caching... Will look to include this in the next minor release.
4 participants
connects #121
This isn't a full implementation of a OCI registry client at this point, and doesn't support auth etc. Only intended as a last resort fallback for now, and as such I've not wired it in to be selectable by host via command line flags.