Set a default User-Agent if unset

[jonjohnsonjr]

Currently, the User-Agent defaults to a golang default of either:

Go-http-client/1.1
Go-http-client/2.0

This makes it rather difficult to distinguish this client from any other
random golang program in registry logs.

Instead, set the default User-Agent to "containers/$VERSION (github.com/containers/image)".

Context: containers/skopeo#1109

Signed-off-by: Jon Johnson [email protected]

[mtrmac]

Thanks! This captures the most important part of containers/skopeo#1109 .

LGTM, hoping for a second pair of eyes to review for possible risks we might have missed.

[mtrmac]

I didn't see an obvious place to add tests for this. I'm personally happy not to write any tests for a small change like this, but I'm also more than willing to if you can point me in the right direction 😄

Yeah. I wouldn’t mind a test for newDockerClient, even if it tests only the small aspect of setting the userAgent field based on input; the c/image/docker package has few unit tests and we have to start somewhere.

OTOH the cost/benefit ratio of writing tests for this really is pretty bad — if this will ever be broken, it will probably not be individually broken, but just moved or bypassed in a way that a unit test would not detect. It would make more sense to incorporate a test into something a bit more end-to-end, at least like #254, but that has proven to be a nightmare to maintain and is probably not going to happen in this repo.

[jonjohnsonjr]

I've added the simplest test I can imagine (that actually involves sending an http request).

[rhatdan]

LGTM
@mtrmac @vrothberg PTAL

[vrothberg]

Any chance we can do something like "github.com/containers/image"? Not sure that's allowed but the source would be more explicit.

[jonjohnsonjr]

Any chance we can do something like "github.com/containers/image"? Not sure that's allowed but the source would be more explicit.

I believe "something like", yes, but not exactly:

       User-Agent     = "User-Agent" ":" 1*( product | comment )

       product         = token ["/" product-version]

       product-version = token

       token          = 1*<any CHAR except CTLs or tspecials>

       tspecials      = "(" | ")" | "<" | ">" | "@"
                      | "," | ";" | ":" | "\" | <">
                      | "/" | "[" | "]" | "?" | "="
                      | "{" | "}" | SP | HT

       comment        = "(" *( ctext | comment ) ")"

       ctext          = <any TEXT excluding "(" and ")">

       TEXT           = <any OCTET except CTLs,
                        but including LWS>

From https://tools.ietf.org/html/rfc1945#section-2.2 and https://tools.ietf.org/html/rfc1945#section-3.7 and https://tools.ietf.org/html/rfc1945#section-10.15

token forbids /, but we could add it to the comment, which can include anything other than (, ), control characters, and newlines.

How about this?

User-Agent: containers/$VERSION (github.com/containers/image)

[vrothberg]

Thanks for looking into it. I really like it :)

@rhatdan @mtrmac PTAL

[jonjohnsonjr]

Updated the PR description and pushed the suggestion of containers/$VERSION (github.com/containers/image).

Let me know if you'd prefer I squash the commits so that the commit message makes more sense.

[rhatdan]

LGTM