From 9b8c89ada6dc2bf8054b2db8a01452688a56c15f Mon Sep 17 00:00:00 2001 From: Clint Date: Mon, 2 Dec 2019 12:13:33 -0600 Subject: [PATCH] Agent Template: check rendering to match expectations (#7899) (#7918) * add regression test for #7883 * Add logic to count render events and match them to expected * remove the WAIT label and make some changes to remove the break statements * change the 'randomness' of the templateContents test helper method --- command/agent/template/template.go | 43 +++- command/agent_test.go | 330 +++++++++++++++++++++++++---- 2 files changed, 327 insertions(+), 46 deletions(-) diff --git a/command/agent/template/template.go b/command/agent/template/template.go index 426ea11d2eb4..008fa94cf54f 100644 --- a/command/agent/template/template.go +++ b/command/agent/template/template.go @@ -50,6 +50,11 @@ type Server struct { // Templates holds the parsed Consul Templates Templates []*ctconfig.TemplateConfig + // lookupMap is alist of templates indexed by their consul-template ID. This + // is used to ensure all Vault templates have been rendered before returning + // from the runner in the event we're using exit after auth. + lookupMap map[string][]*ctconfig.TemplateConfig + DoneCh chan struct{} logger hclog.Logger exitAfterAuth bool @@ -104,6 +109,23 @@ func (ts *Server) Run(ctx context.Context, incoming chan string, templates []*ct return } + // Build the lookup map using the id mapping from the Template runner. This is + // used to check the template rendering against the expected templates. This + // returns a map with a generated ID and a slice of templates for that id. The + // slice is determined by the source or contents of the template, so if a + // configuration has multiple templates specified, but are the same source / + // contents, they will be identified by the same key. + idMap := ts.runner.TemplateConfigMapping() + lookupMap := make(map[string][]*ctconfig.TemplateConfig, len(idMap)) + for id, ctmpls := range idMap { + for _, ctmpl := range ctmpls { + tl := lookupMap[id] + tl = append(tl, ctmpl) + lookupMap[id] = tl + } + } + ts.lookupMap = lookupMap + for { select { case <-ctx.Done(): @@ -133,7 +155,26 @@ func (ts *Server) Run(ctx context.Context, incoming chan string, templates []*ct ts.logger.Error("template server error", "error", err.Error()) return case <-ts.runner.TemplateRenderedCh(): - if ts.exitAfterAuth { + // A template has been rendered, figure out what to do + events := ts.runner.RenderEvents() + + // events are keyed by template ID, and can be matched up to the id's from + // the lookupMap + if len(events) < len(ts.lookupMap) { + // Not all templates have been rendered yet + continue + } + + // assume the renders are finished, until we find otherwise + doneRendering := true + for _, event := range events { + // This template hasn't been rendered + if event.LastWouldRender.IsZero() { + doneRendering = false + } + } + + if doneRendering && ts.exitAfterAuth { // if we want to exit after auth, go ahead and shut down the runner and // return. The deferred closing of the DoneCh will allow agent to // continue with closing down diff --git a/command/agent_test.go b/command/agent_test.go index 0ecff3a3f08b..d8dbd5cf26e9 100644 --- a/command/agent_test.go +++ b/command/agent_test.go @@ -224,7 +224,7 @@ cache { } */ -func TestExitAfterAuth(t *testing.T) { +func TestAgent_ExitAfterAuth(t *testing.T) { logger := logging.NewVaultLogger(hclog.Trace) coreConfig := &vault.CoreConfig{ Logger: logger, @@ -381,19 +381,6 @@ auto_auth { } func TestAgent_RequireRequestHeader(t *testing.T) { - - // makeTempFile creates a temp file and populates it. - makeTempFile := func(name, contents string) string { - f, err := ioutil.TempFile("", name) - if err != nil { - t.Fatal(err) - } - path := f.Name() - f.WriteString(contents) - f.Close() - return path - } - // newApiClient creates an *api.Client. newApiClient := func(addr string, includeVaultRequestHeader bool) *api.Client { conf := api.DefaultConfig() @@ -468,13 +455,13 @@ func TestAgent_RequireRequestHeader(t *testing.T) { secretID := data["secret_id"].(string) // Write the RoleID and SecretID to temp files - roleIDPath := makeTempFile("role_id.txt", roleID+"\n") - secretIDPath := makeTempFile("secret_id.txt", secretID+"\n") + roleIDPath := makeTempFile(t, "role_id.txt", roleID+"\n") + secretIDPath := makeTempFile(t, "secret_id.txt", secretID+"\n") defer os.Remove(roleIDPath) defer os.Remove(secretIDPath) // Get a temp file path we can use for the sink - sinkPath := makeTempFile("sink.txt", "") + sinkPath := makeTempFile(t, "sink.txt", "") defer os.Remove(sinkPath) // Create a config file @@ -515,7 +502,7 @@ listener "tcp" { } ` config = fmt.Sprintf(config, roleIDPath, secretIDPath, sinkPath) - configPath := makeTempFile("config.hcl", config) + configPath := makeTempFile(t, "config.hcl", config) defer os.Remove(configPath) // Start the agent @@ -598,22 +585,7 @@ listener "tcp" { } // TestAgent_Template tests rendering templates -func TestAgent_Template(t *testing.T) { - //---------------------------------------------------- - // Pre-test setup - //---------------------------------------------------- - // makeTempFile creates a temp file and populates it. - makeTempFile := func(name, contents string) string { - f, err := ioutil.TempFile("", name) - if err != nil { - t.Fatal(err) - } - path := f.Name() - f.WriteString(contents) - f.Close() - return path - } - +func TestAgent_Template_Basic(t *testing.T) { //---------------------------------------------------- // Start the server and agent //---------------------------------------------------- @@ -673,8 +645,8 @@ func TestAgent_Template(t *testing.T) { secretID := data["secret_id"].(string) // Write the RoleID and SecretID to temp files - roleIDPath := makeTempFile("role_id.txt", roleID+"\n") - secretIDPath := makeTempFile("secret_id.txt", secretID+"\n") + roleIDPath := makeTempFile(t, "role_id.txt", roleID+"\n") + secretIDPath := makeTempFile(t, "secret_id.txt", secretID+"\n") defer os.Remove(roleIDPath) defer os.Remove(secretIDPath) @@ -695,8 +667,19 @@ func TestAgent_Template(t *testing.T) { }`) request(t, serverClient, req, 200) + // populate another secret + req = serverClient.NewRequest("POST", "/v1/secret/data/otherapp") + req.BodyBytes = []byte(`{ + "data": { + "username": "barstuff", + "password": "zap", + "cert": "something" + } + }`) + request(t, serverClient, req, 200) + // Get a temp file path we can use for the sink - sinkPath := makeTempFile("sink.txt", "") + sinkPath := makeTempFile(t, "sink.txt", "") defer os.Remove(sinkPath) // make a temp directory to hold renders. Each test will create a temp dir @@ -719,15 +702,15 @@ func TestAgent_Template(t *testing.T) { "one": { templateCount: 1, }, - "one_exit": { + "one_with_exit": { templateCount: 1, exitAfterAuth: true, }, "many": { templateCount: 15, }, - "many_exit": { - templateCount: 15, + "many_with_exit": { + templateCount: 13, exitAfterAuth: true, }, } @@ -737,7 +720,7 @@ func TestAgent_Template(t *testing.T) { // make some template files var templatePaths []string for i := 0; i < tc.templateCount; i++ { - path := makeTempFile(fmt.Sprintf("render_%d", i), templateContents) + path := makeTempFile(t, fmt.Sprintf("render_%d", i), templateContents(i)) templatePaths = append(templatePaths, path) } @@ -794,7 +777,7 @@ auto_auth { templateConfig := strings.Join(templateConfigStrings, " ") config = fmt.Sprintf(config, serverClient.Address(), roleIDPath, secretIDPath, sinkPath, templateConfig, exitAfterAuth) - configPath := makeTempFile("config.hcl", config) + configPath := makeTempFile(t, "config.hcl", config) defer os.Remove(configPath) // Start the agent @@ -848,13 +831,256 @@ auto_auth { } } -var templateContents = `{{ with secret "secret/myapp"}} +// TestAgent_Template_ExitCounter tests that Vault Agent correctly renders all +// templates before exiting when the configuration uses exit_after_auth. This is +// similar to TestAgent_Template_Basic, but differs by using a consistent number +// of secrets from multiple sources, where as the basic test could possibly +// generate a random number of secrets, but all using the same source. This test +// reproduces https://github.com/hashicorp/vault/issues/7883 +func TestAgent_Template_ExitCounter(t *testing.T) { + //---------------------------------------------------- + // Start the server and agent + //---------------------------------------------------- + logger := logging.NewVaultLogger(hclog.Trace) + cluster := vault.NewTestCluster(t, + &vault.CoreConfig{ + Logger: logger, + CredentialBackends: map[string]logical.Factory{ + "approle": credAppRole.Factory, + }, + LogicalBackends: map[string]logical.Factory{ + "kv": logicalKv.Factory, + }, + }, + &vault.TestClusterOptions{ + HandlerFunc: vaulthttp.Handler, + }) + cluster.Start() + defer cluster.Cleanup() + + vault.TestWaitActive(t, cluster.Cores[0].Core) + serverClient := cluster.Cores[0].Client + + // Enable the approle auth method + req := serverClient.NewRequest("POST", "/v1/sys/auth/approle") + req.BodyBytes = []byte(`{ + "type": "approle" + }`) + request(t, serverClient, req, 204) + + // give test-role permissions to read the kv secret + req = serverClient.NewRequest("PUT", "/v1/sys/policy/myapp-read") + req.BodyBytes = []byte(`{ + "policy": "path \"secret/*\" { capabilities = [\"read\", \"list\"] }" + }`) + request(t, serverClient, req, 204) + + // Create a named role + req = serverClient.NewRequest("PUT", "/v1/auth/approle/role/test-role") + req.BodyBytes = []byte(`{ + "token_ttl": "5m", + "token_policies":"default,myapp-read", + "policies":"default,myapp-read" + }`) + request(t, serverClient, req, 204) + + // Fetch the RoleID of the named role + req = serverClient.NewRequest("GET", "/v1/auth/approle/role/test-role/role-id") + body := request(t, serverClient, req, 200) + data := body["data"].(map[string]interface{}) + roleID := data["role_id"].(string) + + // Get a SecretID issued against the named role + req = serverClient.NewRequest("PUT", "/v1/auth/approle/role/test-role/secret-id") + body = request(t, serverClient, req, 200) + data = body["data"].(map[string]interface{}) + secretID := data["secret_id"].(string) + + // Write the RoleID and SecretID to temp files + roleIDPath := makeTempFile(t, "role_id.txt", roleID+"\n") + secretIDPath := makeTempFile(t, "secret_id.txt", secretID+"\n") + defer os.Remove(roleIDPath) + defer os.Remove(secretIDPath) + + // setup the kv secrets + req = serverClient.NewRequest("POST", "/v1/sys/mounts/secret/tune") + req.BodyBytes = []byte(`{ + "options": {"version": "2"} + }`) + request(t, serverClient, req, 200) + + // populate a secret + req = serverClient.NewRequest("POST", "/v1/secret/data/myapp") + req.BodyBytes = []byte(`{ + "data": { + "username": "bar", + "password": "zap" + } + }`) + request(t, serverClient, req, 200) + + // populate another secret + req = serverClient.NewRequest("POST", "/v1/secret/data/myapp2") + req.BodyBytes = []byte(`{ + "data": { + "username": "barstuff", + "password": "zap" + } + }`) + request(t, serverClient, req, 200) + + // populate another, another secret + req = serverClient.NewRequest("POST", "/v1/secret/data/otherapp") + req.BodyBytes = []byte(`{ + "data": { + "username": "barstuff", + "password": "zap", + "cert": "something" + } + }`) + request(t, serverClient, req, 200) + + // Get a temp file path we can use for the sink + sinkPath := makeTempFile(t, "sink.txt", "") + defer os.Remove(sinkPath) + + // make a temp directory to hold renders. Each test will create a temp dir + // inside this one + tmpDirRoot, err := ioutil.TempDir("", "agent-test-renders") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tmpDirRoot) + + // create temp dir for this test run + tmpDir, err := ioutil.TempDir(tmpDirRoot, "agent-test") + if err != nil { + t.Fatal(err) + } + + // Create a config file + config := ` +vault { + address = "%s" + tls_skip_verify = true +} + +auto_auth { + method "approle" { + mount_path = "auth/approle" + config = { + role_id_file_path = "%s" + secret_id_file_path = "%s" + remove_secret_id_file_after_reading = false + } + } + + sink "file" { + config = { + path = "%s" + } + } +} + +template { + contents = "{{ with secret \"secret/myapp\" }}{{ range $k, $v := .Data.data }}{{ $v }}{{ end }}{{ end }}" + destination = "%s/render-pass.txt" +} + +template { + contents = "{{ with secret \"secret/myapp2\" }}{{ .Data.data.username}}{{ end }}" + destination = "%s/render-user.txt" +} + +template { + contents = <