This repository has been archived by the owner on Apr 17, 2023. It is now read-only.
Fixed the registry client #259
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Up until now, the portus "user" was handled in a special way, by adding some code that worked around the whole authentication mechanism. However, we recently discovered that this was basically broken. The solution to this situation is to effectively create the "portus" user, as any other user in the system. Because of this, I've added a new secret, which is the password of the "portus" user, and I have added a rake task to help with the migration. Moreover, the seeds file already takes care of that. With this commit, the `manifest` method from the RegisterClient works again, and the authentication mechanism has been greatly simplified. There are still some small issues that need to be addressed. This will be done in later commits.
This method returns all the repositories (with their respective tags) that can be found through the new Catalog API from Docker's Distribution v2.
… portus The portus user has to be handled transparently behind the scenes. This means that it must not be listed in the users list for admins, and it has to be hidden when dealing with authentication. Moreover, the admin will have a notice in the dashboard if the Portus user is not there. This should never happen since the portus user is installed always by seeding it. This would mean that there's something really wrong with the DB (some manual update gone wrong).
|
Some observations:
|
| bundle exec rake db:create | ||
| bundle exec rake db:migrate | ||
| bundle exec rake portus:create |
There was a problem hiding this comment.
Shouldn't we run `db:seed instead? I foresee one day we will remove this special task.
| # Returns an array containing the scopes available for this registry object. | ||
| # It returns a ["*"] if it's an invalid scope, because this will not be | ||
| # implemented as a method. | ||
| def scopes |
There was a problem hiding this comment.
Can you provide more details about the two scopes, like when they are used? This is not so clear right now as it is for the namespace scope (where pull and push are pretty obvious). The goal is to avoid someone else to dig into the distribution code later on ;)
|
You forgot to remove the OTP gem library from the Gemfile. |
|
The code looks good. Thanks also for all the documentation you added. I think this obsoletes part of this #241 pull request. |
|
The documentation on the Registry::AuthScope has been moved to the new class Portus::AuthScope. In this same class I also added documentation on how scopes & policies interact with each other. |
|
LGTM |
1 task
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In order to implement both the Manifest API (#200) and the Catalog API (#199), the register client (and its associated classes) needed some heavy refactoring. This PR brings this refactoring + the test suite for it.