Add Container Registry credential extension#5993
Add Container Registry credential extension#5993qwerty287 wants to merge 26 commits intowoodpecker-ci:mainfrom
Conversation
|
Surge PR preview deployment succeeded. View it at https://woodpecker-ci-woodpecker-pr-5993.surge.sh |
|
|
||
| // NewWithExtension returns a registry service that combines a base service with an HTTP extension. | ||
| // The extension is called during RegistryListPipeline to fetch additional registry credentials. | ||
| func NewWithExtension(base Service, extension *httpExtension) Service { |
There was a problem hiding this comment.
combined should be doing the same thing
There was a problem hiding this comment.
No, because it's the other way round:
Combined uses the base service for everything, while the other services are read only. And the base one has priority.
Here it has to use the read only services first. Of course this can be implemented with some extra boolswitch, should I do this? Not sure how easy this will be in combined though.
There was a problem hiding this comment.
Couldn't you achieve this by just swapping the registries passed to combined already 🤔
There was a problem hiding this comment.
No because combined has a list of readonly services (that are not prioritized) and only one full service (that is prioritized).
Here we need one readonly that's prioritized.
Co-authored-by: Anbraten <6918444+anbraten@users.noreply.github.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #5993 +/- ##
==========================================
- Coverage 22.30% 22.23% -0.08%
==========================================
Files 433 435 +2
Lines 39472 39575 +103
==========================================
- Hits 8805 8800 -5
- Misses 29850 29958 +108
Partials 817 817 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@woodpecker-ci/maintainers could I get a review here? :) |
5 participants
From #5852
Added UI and docs.