Add registry support #1094

jesusbv · 2024-02-19T15:02:27Z

Description

Add registry support to RMT

Change Type

Please select the correct option.

Bug Fix (a non-breaking change which fixes an issue)
New Feature (a non-breaking change which adds new functionality)
Documentation Update (a change which only updates documentation)

Checklist

Please check off each item if the requirement is met.

I have verified that my code follows RMT's coding standards with rubocop.
I have reviewed my own code and believe that it's ready for an external review.
I have provided comments for any hard-to-understand code.
I have documented the MANUAL.md file with any changes to the user experience.
RMT's test coverage remains at 100%.
If my changes are non-trivial, I have added a changelog entry to notify users at package/obs/rmt-server.changes.

Other Notes

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

Add registry engine Add login support: auth based on SCC credentials -> username + password

Currently, allow any access

Same patterns as other engines for gitignore

engines/registry/README.md

engines/registry/app/controllers/registry/registry_controller.rb

- Add cert in config/application.rb Must match the cert of the registry

- Fix AccessScope class - Fix token key

config/application.rb

- Add the path to the config

jesusbv · 2024-03-21T15:23:43Z

config/application.rb

+      File.read('/etc/rmt/ssl/rmt-server.key')
+      )
+    config.registry_public_key = config.registry_private_key.public_key
+    config.access_policies = '/etc/rmt/access_policies.yml'


The other option is to add the path to /etc/rmt.conf

Add a check for the requested path, granting pull access if said path is allowed or denying any access it otherwise

config/application.rb

engines/registry/app/controllers/registry/registry_controller.rb

engines/registry/app/models/access_token.rb

- Add nginx redirect for /v2/_catalog - Add catalog check

- Check activated products with the access policy - Add free products (free products were added recently to the yaml file) - Add rake task to refresh cache for repositories from catalog - Return full path from catalog

bear454 · 2024-04-12T15:14:35Z

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 · 2024-04-12T16:45:58Z

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 · 2024-04-12T18:07:29Z

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 · 2024-04-12T18:12:22Z

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 · 2024-04-15T12:55:41Z

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.

package/files/nginx-pubcloud/nginx-registry-https.conf

- Redirect to RMT server on NGINX conf - Update config routes on RMT for Registry engine

- Free repos are not shown for registry-ec2

config/application.rb

- Check that the engine is defined

Fix Rubocop offenses too

rjschwei · 2024-04-16T19:49:18Z

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 · 2024-04-17T14:02:56Z

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.

bear454

There are still some magic strings in the code, but we can work on moving them out in subsequent PRs. That this is working, has a reliable config, and test coverage is good enough. Let's get rolling.

bear454 · 2024-04-17T14:18:28Z

engines/registry/app/models/access_token.rb

+      hash['iss']    = 'RMT' # "matching issuer in registry auth token config"
+      hash['sub']    = @account
+      hash['aud']    = @service
+      hash['exp']    = Time.now.getlocal.to_i + (5 * 60) # expires at


Per the spec, this needs to default to 8 hours, and be configurable server side. We'll add that in a subsequent PR.

@cwh42 sounds like we should implement the 8h default in SCC, too then?

jesusbv · 2024-04-17T14:39:53Z

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

jesusbv added 3 commits February 19, 2024 14:48

Add registry engine

373c9ec

Add registry engine Add login support: auth based on SCC credentials -> username + password

Add access to pull images

2275f84

Currently, allow any access

Update README and gitignore

3677601

Same patterns as other engines for gitignore

jesusbv requested review from bear454, digitaltom and rjschwei February 19, 2024 15:02

jesusbv self-assigned this Feb 19, 2024

Update issuer

6e33cb3

rjschwei requested changes Feb 23, 2024

View reviewed changes

engines/registry/README.md Outdated Show resolved Hide resolved

engines/registry/app/controllers/registry/registry_controller.rb Outdated Show resolved Hide resolved

jesusbv added 4 commits February 26, 2024 11:50

Update README

6a9bda3

Change registry service name

f82ed36

Better structure

a0039ae

- Add cert in config/application.rb Must match the cert of the registry

Fixes

463d5e8

- Fix AccessScope class - Fix token key

bear454 reviewed Mar 15, 2024

View reviewed changes

config/application.rb Outdated Show resolved Hide resolved

jesusbv and others added 5 commits March 20, 2024 16:20

Use RMT server private key

20fce2f

Add missing 'default_server' statement in PC nginx conf

138cef7

Add PC nginx config for registry

7211b02

Set proper engine route

ab0dd63

Set access policies yaml file path

ecd912a

- Add the path to the config

jesusbv commented Mar 21, 2024

View reviewed changes

Check access policies

9ad9503

Add a check for the requested path, granting pull access if said path is allowed or denying any access it otherwise

jesusbv force-pushed the add-registry-support branch from 0c95bee to 9ad9503 Compare March 21, 2024 15:31

Update authorization and credentials checking handling

ad95321

rjschwei reviewed Mar 21, 2024

View reviewed changes

config/application.rb Outdated Show resolved Hide resolved

rjschwei reviewed Mar 21, 2024

View reviewed changes

engines/registry/app/controllers/registry/registry_controller.rb Show resolved Hide resolved

rjschwei reviewed Mar 21, 2024

View reviewed changes

engines/registry/app/controllers/registry/registry_controller.rb Outdated Show resolved Hide resolved

rjschwei reviewed Mar 21, 2024

View reviewed changes

engines/registry/app/models/access_token.rb Outdated Show resolved Hide resolved

jesusbv added 3 commits March 22, 2024 13:24

Add catalog access

7d1cebe

- Add nginx redirect for /v2/_catalog - Add catalog check

Update method to use systems

77bbcce

Add access policies

a7f067e

- Check activated products with the access policy - Add free products (free products were added recently to the yaml file) - Add rake task to refresh cache for repositories from catalog - Return full path from catalog

Remove unnecessary variable

6f50005

bear454 reviewed Apr 15, 2024

View reviewed changes

package/files/nginx-pubcloud/nginx-registry-https.conf Outdated Show resolved Hide resolved

One single file for nginx conf of RMT server and registry

e616e17

- Redirect to RMT server on NGINX conf - Update config routes on RMT for Registry engine

jesusbv force-pushed the add-registry-support branch from 3c9d52c to e616e17 Compare April 15, 2024 19:11

jesusbv and others added 4 commits April 15, 2024 20:16

Remove unnecessary route for Registry engine

a93bd7b

Remove free repos from search

f271020

- Free repos are not shown for registry-ec2

Merge branch 'master' into add-registry-support

bf813ea

Remove another registry reference on spec file

8cf221d

felixsch reviewed Apr 16, 2024

View reviewed changes

config/application.rb Outdated Show resolved Hide resolved

jesusbv added 2 commits April 16, 2024 14:00

Add Registry engine files to ci configuration

46ea088

Add Public Cloud check

73721aa

- Check that the engine is defined

jesusbv force-pushed the add-registry-support branch from 5b18dca to 73721aa Compare April 16, 2024 13:07

jesusbv added 2 commits April 16, 2024 14:46

Fix typo

1faec62

Update tests for coverage

7c96808

Fix Rubocop offenses too

jesusbv force-pushed the add-registry-support branch from 8b67f2b to 7c96808 Compare April 16, 2024 13:54

rjschwei approved these changes Apr 16, 2024

View reviewed changes

bear454 approved these changes Apr 17, 2024

View reviewed changes

bear454 reviewed Apr 17, 2024

View reviewed changes

Merge branch 'master' into add-registry-support

7d5384c

jesusbv merged commit cd4559e into master Apr 17, 2024
3 checks passed

jesusbv deleted the add-registry-support branch April 17, 2024 14:52

ngetahun mentioned this pull request Apr 18, 2024

Revert "Add registry support" #1126

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add registry support #1094

Add registry support #1094

jesusbv commented Feb 19, 2024

jesusbv Mar 21, 2024

bear454 commented Apr 12, 2024

rjschwei commented Apr 12, 2024

bear454 commented Apr 12, 2024

bear454 commented Apr 12, 2024

rjschwei commented Apr 15, 2024

rjschwei commented Apr 16, 2024

bear454 commented Apr 17, 2024

bear454 left a comment

bear454 Apr 17, 2024

digitaltom Apr 17, 2024

jesusbv commented Apr 17, 2024

Add registry support #1094

Add registry support #1094

Conversation

jesusbv commented Feb 19, 2024

Description

Change Type

Checklist

Other Notes

jesusbv Mar 21, 2024

Choose a reason for hiding this comment

bear454 commented Apr 12, 2024

rjschwei commented Apr 12, 2024

bear454 commented Apr 12, 2024

bear454 commented Apr 12, 2024

rjschwei commented Apr 15, 2024

rjschwei commented Apr 16, 2024

bear454 commented Apr 17, 2024

bear454 left a comment

Choose a reason for hiding this comment

bear454 Apr 17, 2024

Choose a reason for hiding this comment

digitaltom Apr 17, 2024

Choose a reason for hiding this comment

jesusbv commented Apr 17, 2024