Feat/oci registry collector

[robert-cronin]

Description of the PR

This is my attempt at progressing #1279 cc @ridhoq
I think this feature is still one we want to add long-term

Partial fix for #298, this should be merged first and then #2241 will fix the issue.

A couple of thoughts I've had along the way:

• It seems like a call to /v2/_catalog requires authentication for many registries (e.g. ghcr.io, ghr.io to name two)
• My understanding is that regclient silently fails on a 401 response from the registry and doesn't seek to reconcile it by attempting login with local creds even when we pass WithDockerCreds in. I'm not sure if that is still the case. Relevant issue: [Question] regctl repo ls with authentication regclient/regclient#748 (comment)
• Another useful issue/comment for context: [Issue] regsync to full registry using jfrog artifactory as a source not working regclient/regclient#792 (comment)
• In my drafting of testing for the feature, I could only get mcr.microsoft.com to return a list of repos (see https://mcr.microsoft.com/v2/_catalog for this list) however the call returns around 2k+ repos with some having upwards of 100+ tags, so unless we implement some kind of concurrency, I think unit testing might be infeasible. Also, it looks like mcr doesn't yet support limits (e.g. https://mcr.microsoft.com/v2/_catalog?n=5).

Any guidance on the testing or concurrency would be appreciated!

PR Checklist

• All commits have a Developer Certificate of Origin (DCO) -- they are generated using -s flag to git commit.
• All new changes are covered by tests
• If GraphQL schema is changed, make generate has been run
• If GraphQL schema is changed, GraphQL client updates/additions have been made
• If OpenAPI spec is changed, make generate has been run
• If ent schema is changed, make generate has been run
• If collectsub protobuf has been changed, make proto has been run
• All CI checks are passing (tests and formatting)
• All dependent PRs have already been merged

[robert-cronin]

This is still a WIP, but thought I should push what I have so far. A couple more thoughts:

• I think mocking the registry endpoints is probably the most feasible way to get reliable unit testing. To this end, I have adapted a httptest handler found in the regclient lib that returns mock data for the catalog, manifests and tags endpoints.
• To address the concern of potentially having all repos in a registry collected serially, there are two mechanisms that might be suitable:
  • processing the tags of an image in a worker queue which I have attempted in the latest commit; and
  • passing each new repo into CollectSub via gRPC which I have yet to attempt.

Any feedback or suggestions would be greatly appreciated!

cc @ridhoq

[robert-cronin]

I've created a basic test suite which is working for the registry collector. It is an attempt to model the Pull Workflow that starts from a call to the catalog endpoint. Below are a couple of points that might be of interest to reviewers:

• OCIRegistryCollector is essentially a wrapper over OCICollector so the collected documents will have the source labelled as OCICollector and not OCIRegistryCollector. Not sure how best to fix this without duplicating the OCICollector code but maybe it is okay.
• There is probably an easier way to test the registry collector; it was useful for my learning and I figured it might also be useful for other test suites in the future, but I am happy to find some other way to test the feature as well.
• I've added a new method: DeregisterDocumentCollector to the collector in order to facilitate testing. Without it, the original OCI test suite fails when run in tandem with the new test suite because the registry collector is still registered.

[pxp928]

@robert-cronin is this ready for review?

[robert-cronin]

@robert-cronin is this ready for review?

yes, all ready for review 👍 thank you!

Sorry, I've just noticed some TODO statements that need to be resolved. I guess at this stage any comments on the overall approach and whether I'm missing any key elements would be appreciated. I am still learning my way around the source code.

[robert-cronin]

I've now addressed the TODO comments and also tried to add the registry collector to the collect sub although I'm not confident I haven't missed anything there; I think I am still confused about how exactly the CollectSources get picked up by the respective collectors.

[robert-cronin]

just to add a bit of extra context; in my local testing I have run the collector against a local ACR registry with a mock image + referrers and that seems to be working (assuming the 404's are expected to be there, I guess they are a part of the OCI collector logging?)
Screenshot below:

could probably have added a known manifest type but I think it demonstrates that the registry collector is successfully indexing and passing the images onto the oci collector.

[pxp928]

Thanks @robert-cronin! Let me take a look first thing in the morning and I will review your PR.

[robert-cronin]

Thanks @robert-cronin! Let me take a look first thing in the morning and I will review your PR.

@pxp928 no problems, feel free to take your time. Thank you!

[lumjjb]

Awesome PR @robert-cronin , cool implementation with the recursive collectors and great testing! I have a couple comments suggesting some changes, but mostly LGTM. Can i also request we split this into the collector implementation and the CLI changes in another PR? Thanks!

[robert-cronin]

Awesome PR @robert-cronin , cool implementation with the recursive collectors and great testing! I have a couple comments suggesting some changes, but mostly LGTM. Can i also request we split this into the collector implementation and the CLI changes in another PR? Thanks!

I agree, this PR could do with some splitting in two. I've added the CLI-relevant changes to a new PR: #2241. This PR should be merged before that one. Thanks for the review @lumjjb ! back over to you for round 2 👍

[hown3d]

Really nice!
I could see having a filter for the repositories being useful. For example we have a registry with around 3000+ repos where not all of them are interesting for ingestion into GUAC.

[pxp928]

This is great! Just a very minor nit otherwise it looks great! LGTM

[robert-cronin]

Really nice! I could see having a filter for the repositories being useful. For example we have a registry with around 3000+ repos where not all of them are interesting for ingestion into GUAC.

I can see how that would be helpful! I'm wondering would it make sense then to have an option on the collector CLI that takes a regex pattern? e.g. guacone collect registry example.azurecr.io --exclude ".*-deprecated$". Perhaps we can add an issue and tackle it after this PR is merged

[robert-cronin]

This is great! Just a very minor nit otherwise it looks great! LGTM

Thank you @pxp928!