Allow auto_auth with templates without specifying a sink

[kula]

For situations where you want the Vault agent to handle one or
more templates but do not require the acquired credentials
elsewhere.

Modify the logic in SyncServer so that if there are no sinks,
ignore any new credentials. Since SyncServer is responsible
for shutting down the agent, make sure it still properly
shuts down in this new situation.

Updated documentation.

Solves #7988

[ncabatoff]

Hi @kula,

Thanks for the PR! It's been a while since I dug into Agent - since before it supported templates in fact. You say " if there are no sinks, ignore any new credentials". Wouldn't that prevent re-rendering of templates post-reauth?

[kula]

When template support was added a template server (analogous to the sink server) was added, and command/agent/auth/auth.go was updated so that the new credential is sent to both the sink server and the template server (https://github.com/hashicorp/vault/blob/master/command/agent/auth/auth.go#L177-L181 or https://github.com/hashicorp/vault/blob/master/command/agent/auth/auth.go#L207-L210 depending on if it is wrapped or not).

I've been using this since shortly after I updated it to keep a token refreshed and using it to update templates with short lived IAM credentials and it's been working for me, at least.

[ncabatoff]

Thanks for clarifying. The change looks good but I'd like to see more tests. You could remove the sink from the config in TestAgent_Template_ExitCounter, since it's not using it anyway. For completeness I would also add a fixture-based test here since it's nice to have one place where we can look and see that that various supported configurations are validated.

[kula]

I think we can remove the sink config from all the TestAgent_Template_* tests, right? As far as I can tell none of them are using it other than previously having to have at least one sink around.

[kula]

With this I've reverted the unnecessary documentation line break removal, as well as changed and added some tests.

[ncabatoff]

I think we can remove the sink config from all the TestAgent_Template_* tests, right? As far as I can tell none of them are using it other than previously having to have at least one sink around.

It's fine with me, I just picked an example of a test I found that seemed to be exercising the desired behaviour and that wouldn't suffer from removing sinks.

The changes look good, please merge in master to resolve conflicts and I'll approve.

[kula]

I believe I've merged everything properly. Thanks!

[kula]

I have some test errors from the previous commit as well, I'm resolving those now.

[ncabatoff]

TestAgent_Template_Basic is now failing with timeout, it seems to block when we write to cmd.ShutdownCh on line 881.

[kula]

I did mess with the SinkServer, which is responsible for shutting down, I wonder if that had anything to do with it. Looking now.

[kula]

Okay, I think I have it narrowed down to two issues.

The first is with the test framework. We start the tests in https://github.com/hashicorp/vault/blob/master/command/agent_test.go#L855-L863 by launching a goroutine to run the test. Then in https://github.com/hashicorp/vault/blob/master/command/agent_test.go#L874, if it is a test that didn't set exit_after_auth we send the shutdown down the shutdown channel and verify the results of the test.

However, if the test we started had errors starting (https://github.com/hashicorp/vault/blob/master/command/agent_test.go#L874) by the time we get down to line 874 there's nothing to receive the shutdown notification, and it will hang. I think we need a way for places that we do that to communicate back out that the routine has already gone away, don't try to send it a shutdown notification.

The second part is two tests themselves, zero and zero-with-exit at https://github.com/hashicorp/vault/blob/master/command/agent_test.go#L763-L766. They, given the changes done in b08b137 above, return errors like:

--- FAIL: TestAgent_Template_Basic (11.28s)
    --- FAIL: TestAgent_Template_Basic/zero-with-exit (5.00s)
        agent_test.go:845: non-zero return code when running agent: 1
        agent_test.go:846: STDOUT from agent:
        agent_test.go:847: STDERR from agent:
            Error loading configuration from /tmp/config.hcl628482817: auto_auth requires at least one sink or at least one template or cache.use_auto_auth_token=true
        agent_test.go:855: timeout
FAIL
exit status 1
FAIL    github.com/hashicorp/vault/command      11.346s

with a config of

vault {
  address = "https://127.0.0.1:32897"
        tls_skip_verify = true
}

auto_auth {
    method "approle" {
        mount_path = "auth/approle"
        config = {
            role_id_file_path = "/tmp/role_id.txt378033121"
            secret_id_file_path = "/tmp/secret_id.txt195035596"
                                                remove_secret_id_file_after_reading = false
        }
    }
}



exit_after_auth = true

and

--- FAIL: TestAgent_Template_Basic (8.77s)
    --- FAIL: TestAgent_Template_Basic/zero (5.00s)
        agent_test.go:845: non-zero return code when running agent: 1
        agent_test.go:846: STDOUT from agent:
        agent_test.go:847: STDERR from agent:
            Error loading configuration from /tmp/config.hcl089484230: auto_auth requires at least one sink or at least one template or cache.use_auto_auth_token=true
        agent_test.go:855: timeout
FAIL
exit status 1
FAIL    github.com/hashicorp/vault/command      8.841s

with the config

vault {
  address = "https://127.0.0.1:39031"
        tls_skip_verify = true
}

auto_auth {
    method "approle" {
        mount_path = "auth/approle"
        config = {
            role_id_file_path = "/tmp/role_id.txt525799026"
            secret_id_file_path = "/tmp/secret_id.txt511415081"
                                                remove_secret_id_file_after_reading = false
        }
    }
}

which makes sense, according to what we wanted to do in b08b137 --- an agent with no sinks, no templates and not using the token acquired to be a cache will just do nothing. It should error, and those two tests, given the context of these changes here, will never do anything and could be removed.

However, I don't understand how those two tests ever passed. The check first removed in this PR, and which was then extended in b08b137, would error in the case if len(result.AutoAuth.Sinks) == 0 && (result.Cache == nil || !result.Cache.UseAutoAuthToken) {, and neither of the zero tests would have passed it, right? That's where I'm looking now.

[kula]

Okay, now I'm confused, because current master branch of upstream Vault has a sink config at https://github.com/hashicorp/vault/blob/master/command/agent_test.go#L823-L827 --- did I just take that out for some reason when I merged back in master? I'm going to try to clean up things and try again.

[kula]

I did, in 1bc7575, because in testing templates we no longer need to make have a sink just to render a template. And, in that case, running a test to verify template output when you have no sinks, no templates and no cache using auto auth tokens doesn't make much sense, so the zero tests likely should be removed.

[ncabatoff]

Looks good, thank you!