SUSE / rmt

RPM repository mirroring tool and registration proxy for SUSE Customer Center.
Other
37 stars 46 forks source link

Add registry support #1094

Closed jesusbv closed 2 months ago

jesusbv commented 4 months ago

Description

Add registry support to RMT

Change Type

Please select the correct option.

Checklist

Please check off each item if the requirement is met.

Other Notes

Please use this space to provide notes or thoughts to the team, such as tips on how to review/demo your changes.

bear454 commented 2 months ago

@jesusbv we looked at two different approaches for the nginx config, but the current state reflects half of each.

Which way did you intend to go?

jesusbv commented 2 months ago

@jesusbv we looked at two different approaches for the nginx config, but the current state reflects half of each.

* One https config for both rmt & registry, with the search endpoint of registry assigned as a location to rmt;

* Two separate configs, with the search endpoint in the registry config redirecting to rmt.

Which way did you intend to go?

The latter, split config, I think having the two configs separate is better, I updated the PR now to reflect that

Do you think one file is better ?

bear454 commented 2 months ago

The latter, split config, I think having the two configs separate is better, I updated the PR now to reflect that

Do you think one file is better ?

I think it's fine as long as it's been proven to work with both podman and docker.

rjschwei commented 2 months ago

The latter, split config, I think having the two configs separate is better, I updated the PR now to reflect that Do you think one file is better ?

I think it's fine as long as it's been proven to work with both podman and docker.

Separate config is preferable, that allows a 3rd party that doesn't want to run the registry to not install the config and then the paths are not available.

bear454 commented 2 months ago

The latter, split config, I think having the two configs separate is better, I updated the PR now to reflect that Do you think one file is better ?

I think it's fine as long as it's been proven to work with both podman and docker.

I'm withdrawing my objection here - as docker is incapable of searching via an authenticated endpoint, and only uses the unauthenticated /v1/search method. So, search of our registry will only be capable via podman.

bear454 commented 2 months ago

The latter, split config, I think having the two configs separate is better, I updated the PR now to reflect that Do you think one file is better ?

I think it's fine as long as it's been proven to work with both podman and docker.

Separate config is preferable, that allows a 3rd party that doesn't want to run the registry to not install the config and then the paths are not available.

Well, the problem is around search. Because we want to emulate the behavior of SCC, where the /v2/_catalog API endpoint is overriden, and served by rmt instead of distribution, there is an intimate linking. The two-config solution requires that the registry config introduce a 302 REDIRECT on that API endpoint, which also means the rmt FQDN needs to be specified in the config.

With one config file, the v2/_catalog API endpoint is overridden not to a 302 REDIRECT but directly served by the defined rmt application, as part of the reverse proxy configuration. The client doesn't see a redirect (that's not defined in the registry API) and get served the direct results on the original request. But yes, both rmt and registry vhosts have to be defined in the same file for that to work.

rjschwei commented 2 months ago

The latter, split config, I think having the two configs separate is better, I updated the PR now to reflect that Do you think one file is better ?

I think it's fine as long as it's been proven to work with both podman and docker.

Separate config is preferable, that allows a 3rd party that doesn't want to run the registry to not install the config and then the paths are not available.

Well, the problem is around search. Because we want to emulate the behavior of SCC, where the /v2/_catalog API endpoint is overriden, and served by rmt instead of distribution, there is an intimate linking. The two-config solution requires that the registry config introduce a 302 REDIRECT on that API endpoint, which also means the rmt FQDN needs to be specified in the config.

With one config file, the v2/_catalog API endpoint is overridden not to a 302 REDIRECT but directly served by the defined rmt application, as part of the reverse proxy configuration. The client doesn't see a redirect (that's not defined in the registry API) and get served the direct results on the original request. But yes, both rmt and registry vhosts have to be defined in the same file for that to work.

OK, thanks for the the details, then we are stuck with one config. But thta also means we should have 1 config, i.e. the existing config file should change rather than adding a new one.

rjschwei commented 2 months ago

What's missing, I think, is the cache handling. We had agreed that clients that access the registry have a longer TTL than access to the repositories. Anyway, that change should come in a separate PR. This PR is big enough as it is and we have not made the desired timely progress in getting it merged.

bear454 commented 2 months ago

What's missing, I think, is the cache handling. We had agreed that clients that access the registry have a longer TTL than access to the repositories. Anyway, that change should come in a separate PR. This PR is big enough as it is and we have not made the desired timely progress in getting it merged.

Yes, agreed, this needs to be in a separate PR.

jesusbv commented 2 months ago

What's missing, I think, is the cache handling. We had agreed that clients that access the registry have a longer TTL than access to the repositories. Anyway, that change should come in a separate PR. This PR is big enough as it is and we have not made the desired timely progress in getting it merged.

Yes, agreed, this needs to be in a separate PR.

Agree, yes, cache is not on this PR, and it should go on a different PR