fix: don't override storage driver useragent if it's set #4195
Conversation
Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
milosgajdos
requested review from
Jamstah,
joaodrp,
thaJeztah and
deleteriousEffect
| if storageParams["useragent"] == "" { | ||
| storageParams["useragent"] = fmt.Sprintf("distribution/%s %s", version.Version, runtime.Version()) | ||
| } |
There was a problem hiding this comment.
I saw those PRs: my takeaway was it wasnt intended for debugging purposes, but that doesn't mean you shouldn't be able to set your own should you wish to do so.
This PR sets it to the "default" value unless the user explicitly requires a specific User-agent.
Say you run a commercial registry offering and want to distinguish your UA from the default one. How do you imagine doing that? (NOTE: I am not running any commercial service, just imagining a legitimate usecase)
There was a problem hiding this comment.
Ah, sorry, I should've looked more in-depth. I wasn't sure where storageParams["useragent"] could originate from, and if it could be initialised by the storage-driver itself (i.e., if the storage-driver itself would propagate it with (say) a blanket Amazon s3 client SDK). If that's not the case, then 💯 on not overriding it if it's set.
I'm curious though; is there a global config for this? It looks like it currently is required to be set for the selected storage driver (config.Storage.Parameters() returns config options for each driver, which feels ... odd (I would consider the application as a whole to use a specific user-agent)
There was a problem hiding this comment.
To summarise the conversation I had with Seb offline:
He is suggesting creating a global configuration for useragent which can then be passed to individual storage drivers and used anywhere it might be needed.
That's a reasonable suggestion and I've made a new issue for it here: #4202
We are overriding whatever value is set in the storage driver
useragentconfig.Let's.....NOT do that and only override it if it's not set.
PTAL @Jamstah @thaJeztah