VAULT-12940 Vault Agent uses Vault Agent specific User-Agent header when issuing requests

[VioletHynes]

Vault Agent now uses the following User-Agent headers in the following situations (with version information, in all cases):

• When reporting metrics: Vault Agent
• When proxying: Vault Agent API Proxy (with the original User-Agent preserved at the end)
• When a request comes from the proxy subsystem: Vault Agent API Proxy
• When it's performing auto-auth functionalities: Vault Agent Auto-Auth
• When it's rendering templates: Vault Agent Templating

For example, Vault Agent Auto-Auth/1.14.0-beta1 (+https://www.vaultproject.io/; go1.20.1) would be a full, complete string.

There is a separate project to report User-Agent strings in the audit logs, making it easy for customers to determine which requests Vault received came from Agent, and in particular, which subsystem.

I've added tests that validate that Vault receives the correct User-Agent in all of these circumstances.

Sorry this is a big PR, but I think most of the size is in agent_test, so it isn't as intimidating as hopefully it first seems.

[akshya96]

I went through this test but I am confused as to where we are validating User-Agent header here? Sorry, if I missed something here.

[VioletHynes]

No worries, this is admittedly a bit of a weird test, but this seemed to be the best way to test it.

These tests use a custom handler func:

			HandlerFunc: vaulthttp.HandlerFunc(
				func(properties *vault.HandlerProperties) http.Handler {
					h.props = properties
					h.userAgentToCheckFor = useragent.AgentTemplatingString()
					h.pathToCheck = "/v1/secret/data"
					h.requestMethodToCheck = "GET"
					h.t = t
					return &h
				}),

Using the userAgentHandler in the same file.

In particular, when Vault receives a request that matches the paths/method, it can check the request's user agent.

[VioletHynes]

I'll improve the comment to point this out :)

[peteski22]

I added a few comments for you, feel free to take a spy and see if anything is useful?

[peteski22]

This might feel like overkill, but I'm wondering now that we've got lots of functions that all create user agent strings that follow the same rules:

Is it worth introducing a single place they're 'built' for example:

func generateUserAgentString(product string, productVersion string, extension string) string {
	userAgent := product
	if productVersion != "" {
		userAgent += fmt.Sprintf(" %s", productVersion)
	}

	userAgent += fmt.Sprintf("/%s (+%s; %s)", versionFunc(), projectURL, rt)

	if extension != "" {
		userAgent += fmt.Sprintf("; %s", extension)
	}

	return userAgent
}

Then pulling out some const values for 'product' e.g.Vault Vault Agent, and product versions 'API Proxy', 'Auto-Auth', 'Templating'. It felt to me like that would reduce the chance of a formatting error in the strings for each function and group the various supported versions in a single place.

Just a suggestion/thought, as I appreciate we've got the tests in place now and it's not likely the user agents are going to change often.

[VioletHynes]

I did consider this approach, but I thought there was value to the way it is now - it could just be my perception, but I find that this makes it clearer that you shouldn't be going around making custom user agents - and that the set we have today is an intentional set that could theoretically diverge in the future. Open to other opinions, but unless you feel strongly, I think I'll leave it as-is!

[peteski22]

I think the thing that brought it to mind was the copy/paste stuff that caught us out in the comments, as strings are the prime target for things like that to slip in. So having params which were constants for the most part (and could be re-used) felt like a safer bet. But sure, leave it as-is, as I said .. there's tests in place for the output anyway.

[peteski22]

I was struggling to follow along with some of the tests, I think mainly it seems quite hard to be able to set things up (Vault + Agent + all the config/state).

This is just an observation, and I'm guessing you've already tried to streamline the test code as much as possible, I'm just not the most familiar with it so it was a bit of a slog :)

[VioletHynes]

Awaiting a fix to the pre-commit hooks before I can fix the branch's merge conflicts :)