New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Drop docker prefix from storage driver API user agent #3979
Conversation
milosgajdos
commented
Aug 13, 2023
•
edited by thaJeztah
edited by thaJeztah
- Closes User-Agent for storage requests is still docker-distribution #3832
Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
Codecov ReportPatch coverage:
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. Additional details and impacted files@@ Coverage Diff @@
## main #3979 +/- ##
=======================================
Coverage 55.91% 55.91%
=======================================
Files 110 110
Lines 11057 11057
=======================================
Hits 6183 6183
Misses 4185 4185
Partials 689 689
☔ View full report in Codecov by Sentry. |
@@ -118,7 +118,7 @@ func NewApp(ctx context.Context, config *configuration.Configuration) *App { | |||
if storageParams == nil { | |||
storageParams = make(configuration.Parameters) | |||
} | |||
storageParams["useragent"] = fmt.Sprintf("docker-distribution/%s %s", version.Version, runtime.Version()) | |||
storageParams["useragent"] = fmt.Sprintf("distribution/%s %s", version.Version, runtime.Version()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess cncf-distribution
could be an alternative.
That said; it looks like this unconditionally overwrites the option (if present) from storageParams
, which may be intentional (if it should not be configurable through the config-file), but wondering if this should be passed as argument: if this code is also used as library code in other products, it should probably be configurable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm