Use ORAS v2 for listing repositories from ghcr.io

[gfichtenholt]

per
#4932 (comment)
Pending oras-project/oras-go#196

Thank you, Michael, for finding and pointing out this code I can re-use.

plus made further progress with OCI support (redis cache)

[netlify]

✅ Deploy Preview for kubeapps-dev ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	b554af3
🔍 Latest deploy log	https://app.netlify.com/sites/kubeapps-dev/deploys/62bbe08341b1b300096ca23d
😎 Deploy Preview	https://deploy-preview-4947--kubeapps-dev.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[absoludity]

Sorry for the delay Greg.

See my main question inline. Thanks!

[absoludity]

Ah - so it was just updating to v2? Which OCI registries support this?

[gfichtenholt]

Ah - so it was just updating to v2? Which OCI registries support this?

ghcr.io (GitHub). Have not tested others

[absoludity]

Where is this called? I may have missed it and can't grep in a browser.

[gfichtenholt]

Where is this called? I may have missed it and can't grep in a browser.

nowhere. Its a debug thing I found very useful and left it there to use in the future. If you feel strongly about it, I'll get rid of it

[absoludity]

It's not clear to me, looking at the above two links, which part is relevant?

[gfichtenholt]

It's not clear to me, looking at the above two links, which part is relevant?

both actually. I will change the first one to
// 1. https://github.com/oras-project/oras-go/blob/14422086e41897a44cb706726e687d39dc728805/registry/remote/registry.go#L105

First link implements .Registries() API. Second link
// 2. https://github.com/oras-project/oras-go/blob/14422086e41897a44cb706726e687d39dc728805/registry/remote/url.go#L43
is how the catalog URL is constructed which was very relevant too when debugging

[absoludity]

Again here: it looks like you're using your done error as a sentinel object to verify that there was really no error, but I don't think that's what this code should be doing (or don't see the evidence for why?)

But then, nothing happens below this line either, other than still returning the hard-coded string slice. Based on what I said above, I"d expect that here we'd see our own string slice populated (by the fn) and be able to return it?

[gfichtenholt]

Again here: it looks like you're using your done error as a sentinel object to verify that there was really no error, but I don't think that's what this code should be doing (or don't see the evidence for why?)

But then, nothing happens below this line either, other than still returning the hard-coded string slice. Based on what I said above, I"d expect that here we'd see our own string slice populated (by the fn) and be able to return it?

This is work in progress. There is a TODO comment right above (or below, depends on which way you are scrolling) it with a ref to an open issue in ORAS v2.
oras-project/oras-go#196
In other words you don't need to review that part (yet)

[absoludity]

I'm not following exactly what you're doing here: it looks like you're creating a function that always returns this error here - which effectively means that your function will only ever be called once with the first page of results. But reading the code at the above link, it looks to me like we're meant to provide a function that takes a string slice and can return an error, but is intended to process/store that page of results. That is, what I expected to see here was something like:

// A var outside the closure fn that we can reference inside:
repositories := []string{}

fn := func(repos []string) error {
	repositories = append(repositories, repos)
	return nil
}

so that if we paginate through the repos, they get appended to our slice each time (or the function could do other things, such as filtering, sending off, etc., which is why I assume it can return an error). But even that doesn't seem to be implemented in the oras library, even though it appears to be intended: the repositories function comment says:

https://github.com/oras-project/oras-go/blob/14422086e41897a44cb706726e687d39dc728805/registry/remote/registry.go#L121-L122

and yet it is only ever called once, with the returned url (for the next page of results) ignored by the caller:

https://github.com/oras-project/oras-go/blob/2dd719ebdf99d70a1f1abd81204e296beb4076c9/registry/remote/registry.go#L105-L119

That said, if you wanted to use it in its current (non-paginated) form, I'm OK with that, but still don't see why we're not defining the fn to copy the data to return?

[gfichtenholt]

see above. Please ignore that part

[absoludity]

Have +1'd on that advice. Personally prefer to not include WIP stuff in a PR that is landing, and instead just have stubs (as you had already with the hard-coded value), just so new functionality is added as a unit in context, rather than piecemeal WIP parts over time. But if you find it easier this way, that's fine :)

[gfichtenholt]

thanks Michael

[absoludity]

Approving based on you mentioning that I don't yet need to review the WIP bit that I highlighted. Thanks.

[FeynmanZhou]

Hi @gfichtenholt ,

We are glad to see you leveraged ORAS-go to list repos in KubeApps. ORAS-go just release v2.0.0-rc.1 with some new APIs. Have a try and let us know if you have any suggestions.