From 2609f7c4e997666c689ec967dd0f19c045043f4a Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang <1883212+calvn@users.noreply.github.com> Date: Fri, 4 Jun 2021 12:05:00 -0700 Subject: [PATCH 01/25] agent: restart template runner on retry for unlimited retries --- command/agent/cache/api_proxy.go | 4 ++++ command/agent/config/config.go | 3 ++- command/agent/template/template.go | 15 ++++++++++++++- 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/command/agent/cache/api_proxy.go b/command/agent/cache/api_proxy.go index 5ba7076d12a5..a4793239ca14 100644 --- a/command/agent/cache/api_proxy.go +++ b/command/agent/cache/api_proxy.go @@ -119,6 +119,10 @@ func (ap *APIProxy) Send(ctx context.Context, req *SendRequest) (*SendResponse, } client.SetToken(req.Token) + // Derive and set a logger for the client + clientLogger := ap.logger.Named("client") + client.SetLogger(clientLogger) + // http.Transport will transparently request gzip and decompress the response, but only if // the client doesn't manually set the header. Removing any Accept-Encoding header allows the // transparent compression to occur. diff --git a/command/agent/config/config.go b/command/agent/config/config.go index aa0647bd663d..667725914a2a 100644 --- a/command/agent/config/config.go +++ b/command/agent/config/config.go @@ -44,7 +44,8 @@ func (c *Config) Prune() { } type Retry struct { - NumRetries int `hcl:"num_retries"` + NumRetries int `hcl:"num_retries"` + TemplateUnlimitedRetries bool `hcl:"template_unlimited_retries"` } // Vault contains configuration for connecting to Vault servers diff --git a/command/agent/template/template.go b/command/agent/template/template.go index 9396d1f82116..a5af0e1d8fcc 100644 --- a/command/agent/template/template.go +++ b/command/agent/template/template.go @@ -173,7 +173,20 @@ func (ts *Server) Run(ctx context.Context, incoming chan string, templates []*ct case err := <-ts.runner.ErrCh: ts.runner.StopImmediately() - return fmt.Errorf("template server: %w", err) + + // If unlimited retry was not specified, then we return after + // stopping the runner + if !ts.config.AgentConfig.Vault.Retry.TemplateUnlimitedRetries { + return fmt.Errorf("template server: %w", err) + } + + ts.logger.Error("template server error", "error", err.Error()) + + ts.runner, err = manager.NewRunner(runnerConfig, false) + if err != nil { + return fmt.Errorf("template server failed to create: %w", err) + } + go ts.runner.Start() case <-ts.runner.TemplateRenderedCh(): // A template has been rendered, figure out what to do From a521f1d2885c457823bbd3557ae66f2b7f3e95d2 Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang <1883212+calvn@users.noreply.github.com> Date: Fri, 4 Jun 2021 13:59:26 -0700 Subject: [PATCH 02/25] template: log error message early --- command/agent/template/template.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/command/agent/template/template.go b/command/agent/template/template.go index a5af0e1d8fcc..f59508097dd9 100644 --- a/command/agent/template/template.go +++ b/command/agent/template/template.go @@ -172,6 +172,7 @@ func (ts *Server) Run(ctx context.Context, incoming chan string, templates []*ct } case err := <-ts.runner.ErrCh: + ts.logger.Error("template server error", "error", err.Error()) ts.runner.StopImmediately() // If unlimited retry was not specified, then we return after @@ -180,8 +181,6 @@ func (ts *Server) Run(ctx context.Context, incoming chan string, templates []*ct return fmt.Errorf("template server: %w", err) } - ts.logger.Error("template server error", "error", err.Error()) - ts.runner, err = manager.NewRunner(runnerConfig, false) if err != nil { return fmt.Errorf("template server failed to create: %w", err) From 83e2d94b3a280e61642811a228be94bf1760c045 Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang <1883212+calvn@users.noreply.github.com> Date: Fri, 4 Jun 2021 14:00:13 -0700 Subject: [PATCH 03/25] template: delegate retries back to template if param is set to true --- command/agent/template/template.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/command/agent/template/template.go b/command/agent/template/template.go index f59508097dd9..8a95af1e0e86 100644 --- a/command/agent/template/template.go +++ b/command/agent/template/template.go @@ -265,6 +265,13 @@ func newRunnerConfig(sc *ServerConfig, templates ctconfig.TemplateConfigs) (*ctc // that need to be fixed for 1.7x/1.8 if sc.AgentConfig.Cache != nil && sc.AgentConfig.Cache.Persist != nil && len(sc.AgentConfig.Listeners) != 0 { attempts = 0 + + // If unlimited template retries is specified, let consul-template + // handle retry and backoff logic + if sc.AgentConfig.Vault.Retry.TemplateUnlimitedRetries { + attempts = ctconfig.DefaultRetryAttempts + } + scheme := "unix://" if sc.AgentConfig.Listeners[0].Type == "tcp" { scheme = "https://" From 20af4853eff955cf96c44b3eefce00ae4421d2bd Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang <1883212+calvn@users.noreply.github.com> Date: Wed, 9 Jun 2021 15:33:03 -0700 Subject: [PATCH 04/25] agent: add and use the new template config stanza --- command/agent/config/config.go | 48 ++++++++++++++--- command/agent/config/config_test.go | 53 +++++++++++++++++++ .../config-template_config-empty.hcl | 13 +++++ .../test-fixtures/config-template_config.hcl | 15 ++++++ command/agent/template/template.go | 12 ++--- 5 files changed, 128 insertions(+), 13 deletions(-) create mode 100644 command/agent/config/test-fixtures/config-template_config-empty.hcl create mode 100644 command/agent/config/test-fixtures/config-template_config.hcl diff --git a/command/agent/config/config.go b/command/agent/config/config.go index 667725914a2a..13788f11398f 100644 --- a/command/agent/config/config.go +++ b/command/agent/config/config.go @@ -22,11 +22,12 @@ import ( type Config struct { *configutil.SharedConfig `hcl:"-"` - AutoAuth *AutoAuth `hcl:"auto_auth"` - ExitAfterAuth bool `hcl:"exit_after_auth"` - Cache *Cache `hcl:"cache"` - Vault *Vault `hcl:"vault"` - Templates []*ctconfig.TemplateConfig `hcl:"templates"` + AutoAuth *AutoAuth `hcl:"auto_auth"` + ExitAfterAuth bool `hcl:"exit_after_auth"` + Cache *Cache `hcl:"cache"` + Vault *Vault `hcl:"vault"` + TemplateConfig *TemplateConfig `hcl:"template_config"` + Templates []*ctconfig.TemplateConfig `hcl:"templates"` } func (c *Config) Prune() { @@ -44,8 +45,7 @@ func (c *Config) Prune() { } type Retry struct { - NumRetries int `hcl:"num_retries"` - TemplateUnlimitedRetries bool `hcl:"template_unlimited_retries"` + NumRetries int `hcl:"num_retries"` } // Vault contains configuration for connecting to Vault servers @@ -115,6 +115,11 @@ type Sink struct { Config map[string]interface{} } +// TemplateConfig defines global behaviors around template +type TemplateConfig struct { + ExitOnRetryFailure bool `hcl:"exit_on_retry_failure"` +} + func NewConfig() *Config { return &Config{ SharedConfig: new(configutil.SharedConfig), @@ -178,6 +183,10 @@ func LoadConfig(path string) (*Config, error) { return nil, fmt.Errorf("error parsing 'cache':%w", err) } + if err := parseTemplateConfig(result, list); err != nil { + return nil, fmt.Errorf("error parsing 'template_config': %w", err) + } + if err := parseTemplates(result, list); err != nil { return nil, fmt.Errorf("error parsing 'template': %w", err) } @@ -552,6 +561,31 @@ func parseSinks(result *Config, list *ast.ObjectList) error { return nil } +func parseTemplateConfig(result *Config, list *ast.ObjectList) error { + name := "template_config" + + templateConfigList := list.Filter(name) + if len(templateConfigList.Items) == 0 { + return nil + } + + if len(templateConfigList.Items) > 1 { + return fmt.Errorf("at most one %q block is allowed", name) + } + + // Get our item + item := templateConfigList.Items[0] + + var cfg TemplateConfig + if err := hcl.DecodeObject(&cfg, item.Val); err != nil { + return err + } + + result.TemplateConfig = &cfg + + return nil +} + func parseTemplates(result *Config, list *ast.ObjectList) error { name := "template" diff --git a/command/agent/config/config_test.go b/command/agent/config/config_test.go index ac14d2965eab..005db39d6344 100644 --- a/command/agent/config/config_test.go +++ b/command/agent/config/config_test.go @@ -535,6 +535,59 @@ func TestLoadConfigFile_AgentCache_PersistMissingType(t *testing.T) { } } +func TestLoadConfigFile_TemplateConfig(t *testing.T) { + + testCases := map[string]struct { + fixturePath string + expectedTemplateConfig TemplateConfig + }{ + "set-true": { + "./test-fixtures/config-template_config.hcl", + TemplateConfig{ + ExitOnRetryFailure: true, + }, + }, + "empty": { + "./test-fixtures/config-template_config-empty.hcl", + TemplateConfig{ + ExitOnRetryFailure: false, + }, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + config, err := LoadConfig(tc.fixturePath) + if err != nil { + t.Fatal(err) + } + + expected := &Config{ + SharedConfig: &configutil.SharedConfig{}, + Vault: &Vault{ + Address: "http://127.0.0.1:1111", + Retry: &Retry{ + NumRetries: 5, + }, + }, + TemplateConfig: &tc.expectedTemplateConfig, + Templates: []*ctconfig.TemplateConfig{ + { + Source: pointerutil.StringPtr("/path/on/disk/to/template.ctmpl"), + Destination: pointerutil.StringPtr("/path/on/disk/where/template/will/render.txt"), + }, + }, + } + + config.Prune() + if diff := deep.Equal(config, expected); diff != nil { + t.Fatal(diff) + } + }) + } + +} + // TestLoadConfigFile_Template tests template definitions in Vault Agent func TestLoadConfigFile_Template(t *testing.T) { testCases := map[string]struct { diff --git a/command/agent/config/test-fixtures/config-template_config-empty.hcl b/command/agent/config/test-fixtures/config-template_config-empty.hcl new file mode 100644 index 000000000000..a4f5b3a0938f --- /dev/null +++ b/command/agent/config/test-fixtures/config-template_config-empty.hcl @@ -0,0 +1,13 @@ +vault { + address = "http://127.0.0.1:1111" + retry { + num_retries = 5 + } +} + +template_config {} + +template { + source = "/path/on/disk/to/template.ctmpl" + destination = "/path/on/disk/where/template/will/render.txt" +} \ No newline at end of file diff --git a/command/agent/config/test-fixtures/config-template_config.hcl b/command/agent/config/test-fixtures/config-template_config.hcl new file mode 100644 index 000000000000..c2dfea20ae28 --- /dev/null +++ b/command/agent/config/test-fixtures/config-template_config.hcl @@ -0,0 +1,15 @@ +vault { + address = "http://127.0.0.1:1111" + retry { + num_retries = 5 + } +} + +template_config { + exit_on_retry_failure = true +} + +template { + source = "/path/on/disk/to/template.ctmpl" + destination = "/path/on/disk/where/template/will/render.txt" +} \ No newline at end of file diff --git a/command/agent/template/template.go b/command/agent/template/template.go index 8a95af1e0e86..962fc1ffa75f 100644 --- a/command/agent/template/template.go +++ b/command/agent/template/template.go @@ -175,9 +175,9 @@ func (ts *Server) Run(ctx context.Context, incoming chan string, templates []*ct ts.logger.Error("template server error", "error", err.Error()) ts.runner.StopImmediately() - // If unlimited retry was not specified, then we return after - // stopping the runner - if !ts.config.AgentConfig.Vault.Retry.TemplateUnlimitedRetries { + // Return after stopping the runner if exit on retry failure was + // specified + if ts.config.AgentConfig.TemplateConfig.ExitOnRetryFailure { return fmt.Errorf("template server: %w", err) } @@ -266,9 +266,9 @@ func newRunnerConfig(sc *ServerConfig, templates ctconfig.TemplateConfigs) (*ctc if sc.AgentConfig.Cache != nil && sc.AgentConfig.Cache.Persist != nil && len(sc.AgentConfig.Listeners) != 0 { attempts = 0 - // If unlimited template retries is specified, let consul-template - // handle retry and backoff logic - if sc.AgentConfig.Vault.Retry.TemplateUnlimitedRetries { + // If we don't want exit on template retry failure (i.e. unlimited + // retries), let consul-template handle retry and backoff logic + if sc.AgentConfig.TemplateConfig != nil && !sc.AgentConfig.TemplateConfig.ExitOnRetryFailure { attempts = ctconfig.DefaultRetryAttempts } From 4c23bad34b07e8a35f34f2acee6f250856125a29 Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang <1883212+calvn@users.noreply.github.com> Date: Thu, 10 Jun 2021 14:34:56 -0700 Subject: [PATCH 05/25] agent: fix panic, fix existing tests --- command/agent/template/template.go | 2 +- command/agent/template/template_test.go | 23 ++++++++++++++++------- command/agent_test.go | 3 +++ 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/command/agent/template/template.go b/command/agent/template/template.go index 962fc1ffa75f..6bdb537ced00 100644 --- a/command/agent/template/template.go +++ b/command/agent/template/template.go @@ -177,7 +177,7 @@ func (ts *Server) Run(ctx context.Context, incoming chan string, templates []*ct // Return after stopping the runner if exit on retry failure was // specified - if ts.config.AgentConfig.TemplateConfig.ExitOnRetryFailure { + if ts.config.AgentConfig.TemplateConfig != nil && ts.config.AgentConfig.TemplateConfig.ExitOnRetryFailure { return fmt.Errorf("template server: %w", err) } diff --git a/command/agent/template/template_test.go b/command/agent/template/template_test.go index 5efa76900eb2..af26f15e3f54 100644 --- a/command/agent/template/template_test.go +++ b/command/agent/template/template_test.go @@ -352,8 +352,9 @@ func TestServerRun(t *testing.T) { } testCases := map[string]struct { - templateMap map[string]*templateTest - expectError bool + templateMap map[string]*templateTest + expectError bool + exitOnRetryFailure bool }{ "simple": { templateMap: map[string]*templateTest{ @@ -363,7 +364,8 @@ func TestServerRun(t *testing.T) { }, }, }, - expectError: false, + expectError: false, + exitOnRetryFailure: false, }, "multiple": { templateMap: map[string]*templateTest{ @@ -403,7 +405,8 @@ func TestServerRun(t *testing.T) { }, }, }, - expectError: false, + expectError: false, + exitOnRetryFailure: false, }, "bad secret": { templateMap: map[string]*templateTest{ @@ -413,7 +416,8 @@ func TestServerRun(t *testing.T) { }, }, }, - expectError: true, + expectError: true, + exitOnRetryFailure: true, }, "missing key": { templateMap: map[string]*templateTest{ @@ -424,7 +428,8 @@ func TestServerRun(t *testing.T) { }, }, }, - expectError: true, + expectError: true, + exitOnRetryFailure: true, }, "permission denied": { templateMap: map[string]*templateTest{ @@ -434,7 +439,8 @@ func TestServerRun(t *testing.T) { }, }, }, - expectError: true, + expectError: true, + exitOnRetryFailure: true, }, } @@ -458,6 +464,9 @@ func TestServerRun(t *testing.T) { NumRetries: 3, }, }, + TemplateConfig: &config.TemplateConfig{ + ExitOnRetryFailure: tc.exitOnRetryFailure, + }, }, LogLevel: hclog.Trace, LogWriter: hclog.DefaultOutput, diff --git a/command/agent_test.go b/command/agent_test.go index 11440795f560..b7566aa31aa2 100644 --- a/command/agent_test.go +++ b/command/agent_test.go @@ -1387,6 +1387,9 @@ vault { tls_skip_verify = true } %s +template_config { + exit_on_retry_failure = true +} `, methodConf, serverClient.Address(), retryConf, templateConfig) configPath := makeTempFile(t, "config.hcl", config) From b346dd81012de0dd044916b09b8c0ab42b993b65 Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang <1883212+calvn@users.noreply.github.com> Date: Thu, 10 Jun 2021 14:59:06 -0700 Subject: [PATCH 06/25] changelog: add changelog entry --- changelog/11775.txt | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 changelog/11775.txt diff --git a/changelog/11775.txt b/changelog/11775.txt new file mode 100644 index 000000000000..0aa4359c0b60 --- /dev/null +++ b/changelog/11775.txt @@ -0,0 +1,9 @@ +```release-note:change +agent: The template engine errors will no longer cause agent to exit unless +explicitly defined to do so. A new configuration parameter, +`exit_on_retry_failure`, within the new top-level stanza, `template_config`, can +be set to `true` in order to cause agent to exit. Note that for agent to exit if +`template.error_on_missing_key` is set to `true`, `exit_on_retry_failure` must +be also set to `true`. Otherwise, the template engine will log an error but then +restart its internal runner. +``` \ No newline at end of file From 89fa90d67a781ae893baee84124fca7f33a89145 Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang <1883212+calvn@users.noreply.github.com> Date: Fri, 11 Jun 2021 15:24:44 -0700 Subject: [PATCH 07/25] agent: add tests for exit_on_retry_failure --- command/agent_test.go | 249 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 249 insertions(+) diff --git a/command/agent_test.go b/command/agent_test.go index b7566aa31aa2..09de8a782995 100644 --- a/command/agent_test.go +++ b/command/agent_test.go @@ -22,6 +22,7 @@ import ( vaulthttp "github.com/hashicorp/vault/http" "github.com/hashicorp/vault/sdk/helper/consts" "github.com/hashicorp/vault/sdk/helper/logging" + "github.com/hashicorp/vault/sdk/helper/pointerutil" "github.com/hashicorp/vault/sdk/logical" "github.com/hashicorp/vault/vault" "github.com/mitchellh/cli" @@ -1694,3 +1695,251 @@ vault { }) } } + +func TestAgent_TemplateConfig_ExitOnRetryFailure(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{ + NumCores: 1, + HandlerFunc: vaulthttp.Handler, + }) + cluster.Start() + defer cluster.Cleanup() + + vault.TestWaitActive(t, cluster.Cores[0].Core) + serverClient := cluster.Cores[0].Client + + // Unset the environment variable so that agent picks up the right test + // cluster address + defer os.Setenv(api.EnvVaultAddress, os.Getenv(api.EnvVaultAddress)) + os.Unsetenv(api.EnvVaultAddress) + + autoAuthConfig, cleanup := prepAgentApproleKV(t, serverClient) + defer cleanup() + + err := serverClient.Sys().TuneMount("secret", api.MountConfigInput{ + Options: map[string]string{ + "version": "2", + }, + }) + if err != nil { + t.Fatal(err) + } + + _, err = serverClient.Logical().Write("secret/data/otherapp", map[string]interface{}{ + "data": map[string]interface{}{ + "username": "barstuff", + "password": "zap", + "cert": "something", + }, + }) + if err != nil { + t.Fatal(err) + } + + // 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) + + missingKeyTemplateContent := `{{- with secret "secret/non-existent"}}{"secret": "other", +{{- if .Data.data.foo}}"foo":"{{ .Data.data.foo}}",{{- end }} +{{- end }}` + + testCases := map[string]struct { + exitOnRetryFailure *bool + templateContents string + templateErrorOnMissingKey bool + expectError bool + }{ + "true, no template error": { + exitOnRetryFailure: pointerutil.BoolPtr(true), + templateContents: templateContents(0), + templateErrorOnMissingKey: false, + expectError: false, + }, + "true, with template error": { + exitOnRetryFailure: pointerutil.BoolPtr(true), + templateContents: missingKeyTemplateContent, + templateErrorOnMissingKey: false, + expectError: true, + }, + "true, with template error, with error_on_missing_key": { + exitOnRetryFailure: pointerutil.BoolPtr(true), + templateContents: missingKeyTemplateContent, + templateErrorOnMissingKey: true, + expectError: true, + }, + "false, no template error": { + exitOnRetryFailure: pointerutil.BoolPtr(false), + templateContents: templateContents(0), + templateErrorOnMissingKey: false, + expectError: false, + }, + "false, with template error": { + exitOnRetryFailure: pointerutil.BoolPtr(false), + templateContents: missingKeyTemplateContent, + templateErrorOnMissingKey: false, + expectError: true, + }, + "false, with template error, with error_on_missing_key": { + exitOnRetryFailure: pointerutil.BoolPtr(false), + templateContents: missingKeyTemplateContent, + templateErrorOnMissingKey: true, + expectError: true, + }, + "missing": { + exitOnRetryFailure: nil, + templateContents: templateContents(0), + templateErrorOnMissingKey: false, + expectError: false, + }, + } + + for tcName, tc := range testCases { + t.Run(tcName, func(t *testing.T) { + // create temp dir for this test run + tmpDir, err := ioutil.TempDir(tmpDirRoot, tcName) + if err != nil { + t.Fatal(err) + } + + listenAddr := "127.0.0.1:18123" + listenConfig := fmt.Sprintf(` +listener "tcp" { + address = "%s" + tls_disable = true +} +`, listenAddr) + + var exitOnRetryFailure string + if tc.exitOnRetryFailure != nil { + exitOnRetryFailure = fmt.Sprintf("exit_on_retry_failure = %t", *tc.exitOnRetryFailure) + } + templateConfig := fmt.Sprintf(` +template_config = { + %s +} +`, exitOnRetryFailure) + + template := fmt.Sprintf(` +template { + contents = < Date: Fri, 11 Jun 2021 16:46:12 -0700 Subject: [PATCH 08/25] agent: properly check on agent exit cases, add separate tests for missing key vs missing secrets --- command/agent_test.go | 69 ++++++++++++++++++++++++++++++++++++------- 1 file changed, 59 insertions(+), 10 deletions(-) diff --git a/command/agent_test.go b/command/agent_test.go index 09de8a782995..b5b3de7685d7 100644 --- a/command/agent_test.go +++ b/command/agent_test.go @@ -1757,56 +1757,93 @@ func TestAgent_TemplateConfig_ExitOnRetryFailure(t *testing.T) { } defer os.RemoveAll(tmpDirRoot) - missingKeyTemplateContent := `{{- with secret "secret/non-existent"}}{"secret": "other", -{{- if .Data.data.foo}}"foo":"{{ .Data.data.foo}}",{{- end }} + missingKeyTemplateContent := `{{- with secret "secret/otherapp"}}{"secret": "other", +{{- if .Data.data.foo}}"foo":"{{ .Data.data.foo}}"{{- end }}} +{{- end }}` + missingKeyTemplateRender := `{"secret": "other",}` + + badTemplateContent := `{{- with secret "secret/non-existent"}}{"secret": "other", +{{- if .Data.data.foo}}"foo":"{{ .Data.data.foo}}"{{- end }}} {{- end }}` testCases := map[string]struct { exitOnRetryFailure *bool templateContents string + expectTemplateRender string templateErrorOnMissingKey bool + expectExit bool expectError bool }{ "true, no template error": { exitOnRetryFailure: pointerutil.BoolPtr(true), templateContents: templateContents(0), + expectTemplateRender: templateRendered(0), templateErrorOnMissingKey: false, + expectExit: true, expectError: false, }, - "true, with template error": { + "true, with non-existent secret": { exitOnRetryFailure: pointerutil.BoolPtr(true), - templateContents: missingKeyTemplateContent, + templateContents: badTemplateContent, + expectTemplateRender: "", templateErrorOnMissingKey: false, + expectExit: true, expectError: true, }, - "true, with template error, with error_on_missing_key": { + "true, with missing key": { exitOnRetryFailure: pointerutil.BoolPtr(true), templateContents: missingKeyTemplateContent, + expectTemplateRender: missingKeyTemplateRender, + templateErrorOnMissingKey: false, + expectExit: true, + expectError: false, + }, + "true, with missing key, with error_on_missing_key": { + exitOnRetryFailure: pointerutil.BoolPtr(true), + templateContents: missingKeyTemplateContent, + expectTemplateRender: "", templateErrorOnMissingKey: true, + expectExit: true, expectError: true, }, "false, no template error": { exitOnRetryFailure: pointerutil.BoolPtr(false), templateContents: templateContents(0), + expectTemplateRender: templateRendered(0), templateErrorOnMissingKey: false, + expectExit: false, expectError: false, }, - "false, with template error": { + "false, with non-existent secret": { exitOnRetryFailure: pointerutil.BoolPtr(false), - templateContents: missingKeyTemplateContent, + templateContents: badTemplateContent, + expectTemplateRender: "", templateErrorOnMissingKey: false, + expectExit: false, expectError: true, }, - "false, with template error, with error_on_missing_key": { + "false, with missing key": { exitOnRetryFailure: pointerutil.BoolPtr(false), templateContents: missingKeyTemplateContent, + expectTemplateRender: missingKeyTemplateRender, + templateErrorOnMissingKey: false, + expectExit: false, + expectError: false, + }, + "false, with missing key, with error_on_missing_key": { + exitOnRetryFailure: pointerutil.BoolPtr(false), + templateContents: missingKeyTemplateContent, + expectTemplateRender: missingKeyTemplateRender, templateErrorOnMissingKey: true, + expectExit: false, expectError: true, }, "missing": { exitOnRetryFailure: nil, templateContents: templateContents(0), + expectTemplateRender: templateRendered(0), templateErrorOnMissingKey: false, + expectExit: false, expectError: false, }, } @@ -1876,11 +1913,17 @@ vault { ui, cmd := testAgentCommand(t, logger) cmd.startedCh = make(chan struct{}) + // Channel to let verify() know to stop early if agent + // has exited + cmdRunDoneCh := make(chan struct{}) + var exited bool + wg := &sync.WaitGroup{} wg.Add(1) var code int go func() { code = cmd.Run([]string{"-config", configPath}) + close(cmdRunDoneCh) wg.Done() }() @@ -1895,6 +1938,9 @@ vault { var err error for { select { + case <-cmdRunDoneCh: + exited = true + return nil case <-timeout: return fmt.Errorf("timed out waiting for templates to render, last error: %w", err) case <-tick: @@ -1918,8 +1964,8 @@ vault { if err != nil { continue } - if strings.TrimSpace(string(c)) != templateRendered(0) { - err = fmt.Errorf("expected='%s', got='%s'", templateRendered(0), string(c)) + if strings.TrimSpace(string(c)) != tc.expectTemplateRender { + err = fmt.Errorf("expected='%s', got='%s'", tc.expectTemplateRender, strings.TrimSpace(string(c))) continue } return nil @@ -1932,6 +1978,9 @@ vault { switch { case (code != 0 || err != nil) && tc.expectError: + if exited != tc.expectExit { + t.Fatalf("expected program exit to be '%t', got '%t'", tc.expectExit, exited) + } case code == 0 && err == nil && !tc.expectError: default: if code != 0 { From 27df22ba2963411b04f15fbcec6bad500fdb6234 Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang <1883212+calvn@users.noreply.github.com> Date: Fri, 11 Jun 2021 16:53:44 -0700 Subject: [PATCH 09/25] agent: add note on difference between missing key vs missing secret --- command/agent_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/command/agent_test.go b/command/agent_test.go index b5b3de7685d7..b97ccb2f9321 100644 --- a/command/agent_test.go +++ b/command/agent_test.go @@ -1757,6 +1757,10 @@ func TestAgent_TemplateConfig_ExitOnRetryFailure(t *testing.T) { } defer os.RemoveAll(tmpDirRoot) + // Note that missing key is different from a non-existent secret. A missing + // key (2xx response with missing keys in the response map) can still yield + // a successful render unless error_on_missing_key is specified, whereas a + // missing secret (4xx response) always results in an error. missingKeyTemplateContent := `{{- with secret "secret/otherapp"}}{"secret": "other", {{- if .Data.data.foo}}"foo":"{{ .Data.data.foo}}"{{- end }}} {{- end }}` From 300dc8c49627b24d9f49594b180d772ba1cd00c4 Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang <1883212+calvn@users.noreply.github.com> Date: Fri, 11 Jun 2021 17:37:54 -0700 Subject: [PATCH 10/25] docs: add docs for template_config --- website/content/docs/agent/index.mdx | 3 ++ .../content/docs/agent/template-config.mdx | 48 +++++++++++++++++++ website/data/docs-nav-data.json | 4 ++ 3 files changed, 55 insertions(+) create mode 100644 website/content/docs/agent/template-config.mdx diff --git a/website/content/docs/agent/index.mdx b/website/content/docs/agent/index.mdx index 71ca95acd3d0..c6335a808b16 100644 --- a/website/content/docs/agent/index.mdx +++ b/website/content/docs/agent/index.mdx @@ -55,6 +55,8 @@ These are the currently-available general configuration option: - `template` ([template][template]: ) - Specifies options used for templating Vault secrets to files. +- `template_config` ([template_config][template-config]: ) - Specifies templating engine behavior. + ### vault Stanza There can at most be one top level `vault` block and it has the following @@ -199,6 +201,7 @@ template { [autoauth]: /docs/agent/autoauth [caching]: /docs/agent/caching [template]: /docs/agent/template +[template-config]: /docs/agent/template-config [listener]: /docs/agent#listener-stanza [listener_main]: /docs/configuration/listener/tcp [winsvc]: /docs/agent/winsvc diff --git a/website/content/docs/agent/template-config.mdx b/website/content/docs/agent/template-config.mdx new file mode 100644 index 000000000000..1d499c16b4d4 --- /dev/null +++ b/website/content/docs/agent/template-config.mdx @@ -0,0 +1,48 @@ +--- +layout: docs +page_title: Vault Agent Template Config +description: |- + Vault Agent's Template Config to set Templating Engine behavior +--- + +# Vault Agent Template Config + +Template Config allows users of Vault Agent to determine the behavior that +pertains to the templating engine. + +## Functionality + +The `template_config` stanza. Note that `template_config` can only be defined +once, and is different from the `template` stanza. Unlike `template` which +focuses on where and how a specific secret is rendered, `template_config` +contains parameters affects how the templating engine as a whole behaves and its +interaction with the rest of Agent. This includes, but is not limited to, +program exit behavior. Other parameters that applies to the templating engine as +a whole may be added over time. + +### Interaction with `error_on_missing_key` + +The parameter `error_on_missing_key` can be specified within the `template` +stanza which determines if a template should ignore rendering a missing key +within the response of a secret or error out. When `error_on_missing_key` is +not specified or set to `false` and the key to render is not in the secret's +response, the templating engine will ignore it (or render `""`) +and continue on with its rendering. + +If the desired is to have Agent fail and exit on a missing key, both +`template.error_on_missing_key` and `template_config.exit_on_retry_failure` must +be set to true. Otherwise, the templating engine will error and render to its +destination, but agent will not exit and retry until it's able to do so or until +the process is terminated. + +Note that a missing key from a secret's response is different from a missing or +non-existent secret. The former's behavior can be toggled through +`error_on_missing_key`, while the latter always results in an error returned by +the templating engine. + +## Configuration + +The top level `template_config` block has the following configuration entries: + +- `exit_on_retry_failure` `(bool: false)` - This option tells Vault Agent to create + the parent directories of the destination path if they do not exist. diff --git a/website/data/docs-nav-data.json b/website/data/docs-nav-data.json index 4f63db3c20b0..b182f53051b4 100644 --- a/website/data/docs-nav-data.json +++ b/website/data/docs-nav-data.json @@ -808,6 +808,10 @@ "title": "Templates", "path": "agent/template" }, + { + "title": "Template Config", + "path": "agent/template-config" + }, { "title": "Windows service", "path": "agent/winsvc" From 3cf2e48f97dfcbdf5387afe0f8fa8117bb666a9a Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang <1883212+calvn@users.noreply.github.com> Date: Mon, 14 Jun 2021 11:49:24 -0700 Subject: [PATCH 11/25] Update website/content/docs/agent/template-config.mdx Co-authored-by: Jason O'Donnell <2160810+jasonodonnell@users.noreply.github.com> --- website/content/docs/agent/template-config.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/content/docs/agent/template-config.mdx b/website/content/docs/agent/template-config.mdx index 1d499c16b4d4..31638543e99a 100644 --- a/website/content/docs/agent/template-config.mdx +++ b/website/content/docs/agent/template-config.mdx @@ -29,7 +29,7 @@ not specified or set to `false` and the key to render is not in the secret's response, the templating engine will ignore it (or render `""`) and continue on with its rendering. -If the desired is to have Agent fail and exit on a missing key, both +If the desire is to have Agent fail and exit on a missing key, both `template.error_on_missing_key` and `template_config.exit_on_retry_failure` must be set to true. Otherwise, the templating engine will error and render to its destination, but agent will not exit and retry until it's able to do so or until From 310f564d079362ab56a3428062e3131b0e0c1cd2 Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang <1883212+calvn@users.noreply.github.com> Date: Mon, 14 Jun 2021 11:49:36 -0700 Subject: [PATCH 12/25] Update website/content/docs/agent/template-config.mdx Co-authored-by: Jason O'Donnell <2160810+jasonodonnell@users.noreply.github.com> --- website/content/docs/agent/template-config.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/content/docs/agent/template-config.mdx b/website/content/docs/agent/template-config.mdx index 31638543e99a..700002065ff7 100644 --- a/website/content/docs/agent/template-config.mdx +++ b/website/content/docs/agent/template-config.mdx @@ -44,5 +44,5 @@ the templating engine. The top level `template_config` block has the following configuration entries: -- `exit_on_retry_failure` `(bool: false)` - This option tells Vault Agent to create +- `exit_on_retry_failure` `(bool: false)` - This option configures Vault Agent to create the parent directories of the destination path if they do not exist. From d7b42ac28e4294bcc27df8840d44ff65fb1ebf13 Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang <1883212+calvn@users.noreply.github.com> Date: Wed, 16 Jun 2021 12:51:17 -0700 Subject: [PATCH 13/25] Update website/content/docs/agent/template-config.mdx Co-authored-by: Tom Proctor --- website/content/docs/agent/template-config.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/content/docs/agent/template-config.mdx b/website/content/docs/agent/template-config.mdx index 700002065ff7..ec4cd80ea10a 100644 --- a/website/content/docs/agent/template-config.mdx +++ b/website/content/docs/agent/template-config.mdx @@ -22,7 +22,7 @@ a whole may be added over time. ### Interaction with `error_on_missing_key` -The parameter `error_on_missing_key` can be specified within the `template` +The parameter [`error_on_missing_key`](/docs/agent/template#error_on_missing_key) can be specified within the `template` stanza which determines if a template should ignore rendering a missing key within the response of a secret or error out. When `error_on_missing_key` is not specified or set to `false` and the key to render is not in the secret's From 4f673d8133596f5132b588b1a2daaf22884f9ad0 Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang <1883212+calvn@users.noreply.github.com> Date: Wed, 16 Jun 2021 13:03:19 -0700 Subject: [PATCH 14/25] Update website/content/docs/agent/template-config.mdx Co-authored-by: Tom Proctor --- website/content/docs/agent/template-config.mdx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/website/content/docs/agent/template-config.mdx b/website/content/docs/agent/template-config.mdx index ec4cd80ea10a..76c6d7f5abac 100644 --- a/website/content/docs/agent/template-config.mdx +++ b/website/content/docs/agent/template-config.mdx @@ -23,8 +23,8 @@ a whole may be added over time. ### Interaction with `error_on_missing_key` The parameter [`error_on_missing_key`](/docs/agent/template#error_on_missing_key) can be specified within the `template` -stanza which determines if a template should ignore rendering a missing key -within the response of a secret or error out. When `error_on_missing_key` is +stanza which determines if a template should error when a key is missing +in the secret. When `error_on_missing_key` is not specified or set to `false` and the key to render is not in the secret's response, the templating engine will ignore it (or render `""`) and continue on with its rendering. From ddccb43ce46b51d3f6ac5b868b8726dd975c8eb9 Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang <1883212+calvn@users.noreply.github.com> Date: Wed, 16 Jun 2021 13:54:16 -0700 Subject: [PATCH 15/25] Update website/content/docs/agent/template-config.mdx Co-authored-by: Tom Proctor --- website/content/docs/agent/template-config.mdx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/website/content/docs/agent/template-config.mdx b/website/content/docs/agent/template-config.mdx index 76c6d7f5abac..e88c773014dc 100644 --- a/website/content/docs/agent/template-config.mdx +++ b/website/content/docs/agent/template-config.mdx @@ -36,9 +36,7 @@ destination, but agent will not exit and retry until it's able to do so or until the process is terminated. Note that a missing key from a secret's response is different from a missing or -non-existent secret. The former's behavior can be toggled through -`error_on_missing_key`, while the latter always results in an error returned by -the templating engine. +non-existent secret. The templating engine will always error if a secret is missing, but will only error for a missing key if `error_on_missing_key` is set. Whether Vault Agent will exit when the templating engine errors depends on the value of `exit_on_retry_failure`. ## Configuration From b7ecf45b40bc3fb28c05d3ad2cc67074cafcd0ec Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang <1883212+calvn@users.noreply.github.com> Date: Wed, 16 Jun 2021 14:36:58 -0700 Subject: [PATCH 16/25] docs: fix exit_on_retry_failure, fix Functionality section --- .../content/docs/agent/template-config.mdx | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/website/content/docs/agent/template-config.mdx b/website/content/docs/agent/template-config.mdx index e88c773014dc..416cc6341146 100644 --- a/website/content/docs/agent/template-config.mdx +++ b/website/content/docs/agent/template-config.mdx @@ -12,13 +12,14 @@ pertains to the templating engine. ## Functionality -The `template_config` stanza. Note that `template_config` can only be defined -once, and is different from the `template` stanza. Unlike `template` which -focuses on where and how a specific secret is rendered, `template_config` -contains parameters affects how the templating engine as a whole behaves and its -interaction with the rest of Agent. This includes, but is not limited to, -program exit behavior. Other parameters that applies to the templating engine as -a whole may be added over time. +The `template_config` stanza configures overall default behavior for the +templating engine. Note that `template_config` can only be defined once, and is +different from the `template` stanza. Unlike `template` which focuses on where +and how a specific secret is rendered, `template_config` contains parameters +affects how the templating engine as a whole behaves and its interaction with +the rest of Agent. This includes, but is not limited to, program exit behavior. +Other parameters that applies to the templating engine as a whole may be added +over time. ### Interaction with `error_on_missing_key` @@ -36,11 +37,15 @@ destination, but agent will not exit and retry until it's able to do so or until the process is terminated. Note that a missing key from a secret's response is different from a missing or -non-existent secret. The templating engine will always error if a secret is missing, but will only error for a missing key if `error_on_missing_key` is set. Whether Vault Agent will exit when the templating engine errors depends on the value of `exit_on_retry_failure`. +non-existent secret. The templating engine will always error if a secret is +missing, but will only error for a missing key if `error_on_missing_key` is set. +Whether Vault Agent will exit when the templating engine errors depends on the +value of `exit_on_retry_failure`. ## Configuration The top level `template_config` block has the following configuration entries: -- `exit_on_retry_failure` `(bool: false)` - This option configures Vault Agent to create - the parent directories of the destination path if they do not exist. +- `exit_on_retry_failure` `(bool: false)` - This option configures Vault Agent +to exit after it has exhausted its number of template retry attempts due to +failures. From 3657ea638dc7510fefb4e1e4adbec9d032cbf9a7 Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang <1883212+calvn@users.noreply.github.com> Date: Wed, 16 Jun 2021 17:15:53 -0700 Subject: [PATCH 17/25] docs: update interaction title --- website/content/docs/agent/template-config.mdx | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/website/content/docs/agent/template-config.mdx b/website/content/docs/agent/template-config.mdx index 416cc6341146..706ad44b598e 100644 --- a/website/content/docs/agent/template-config.mdx +++ b/website/content/docs/agent/template-config.mdx @@ -21,14 +21,15 @@ the rest of Agent. This includes, but is not limited to, program exit behavior. Other parameters that applies to the templating engine as a whole may be added over time. -### Interaction with `error_on_missing_key` - -The parameter [`error_on_missing_key`](/docs/agent/template#error_on_missing_key) can be specified within the `template` -stanza which determines if a template should error when a key is missing -in the secret. When `error_on_missing_key` is -not specified or set to `false` and the key to render is not in the secret's -response, the templating engine will ignore it (or render `""`) -and continue on with its rendering. +### Interaction between `exit_on_retry_failure` and `error_on_missing_key` + +The parameter +[`error_on_missing_key`](/docs/agent/template#error_on_missing_key) can be +specified within the `template` stanza which determines if a template should +error when a key is missing in the secret. When `error_on_missing_key` is not +specified or set to `false` and the key to render is not in the secret's +response, the templating engine will ignore it (or render `""`) and +continue on with its rendering. If the desire is to have Agent fail and exit on a missing key, both `template.error_on_missing_key` and `template_config.exit_on_retry_failure` must From ffe15f52ee4fd0495dfef082f45d437dcb749d02 Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang <1883212+calvn@users.noreply.github.com> Date: Wed, 16 Jun 2021 17:23:50 -0700 Subject: [PATCH 18/25] template: add internal note on behavior for persist case --- command/agent/template/template.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/command/agent/template/template.go b/command/agent/template/template.go index 6bdb537ced00..0b9c1e007c56 100644 --- a/command/agent/template/template.go +++ b/command/agent/template/template.go @@ -267,7 +267,14 @@ func newRunnerConfig(sc *ServerConfig, templates ctconfig.TemplateConfigs) (*ctc attempts = 0 // If we don't want exit on template retry failure (i.e. unlimited - // retries), let consul-template handle retry and backoff logic + // retries), let consul-template handle retry and backoff logic. + // + // Note: This is a fixed value (12) that ends up being a multiplier to + // retry.num_retires (i.e. 12 * N total retries per runner restart). + // Since we are performing retries indefinitely this base number helps + // prevent agent from spamming Vault if retry.num_retries is set to a + // low value by forcing exponential backoff to be high towards the end + // of retries during the process. if sc.AgentConfig.TemplateConfig != nil && !sc.AgentConfig.TemplateConfig.ExitOnRetryFailure { attempts = ctconfig.DefaultRetryAttempts } From 1440475a03b9b860727733b4eae17001d0a0db2e Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang <1883212+calvn@users.noreply.github.com> Date: Fri, 18 Jun 2021 11:35:30 -0700 Subject: [PATCH 19/25] docs: update agent, template, and template-config docs --- website/content/docs/agent/index.mdx | 9 +++++++-- website/content/docs/agent/template-config.mdx | 7 +++++-- website/content/docs/agent/template.mdx | 17 ++++++++++------- 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/website/content/docs/agent/index.mdx b/website/content/docs/agent/index.mdx index c6335a808b16..c59a7ec78f34 100644 --- a/website/content/docs/agent/index.mdx +++ b/website/content/docs/agent/index.mdx @@ -96,12 +96,17 @@ configuration entries: #### retry Stanza -The `vault` stanza may contain a `retry` stanza that controls how failing -Vault requests are handled, whether these requests are issued in order to render +The `vault` stanza may contain a `retry` stanza that controls how failing Vault +requests are handled, whether these requests are issued in order to render templates, or are proxied requests coming from the proxy cache subsystem. Auto-auth, however, has its own notion of retrying and is not affected by this section. +For requests from the templating engine, Agent will restart its internal runner +and perform retries again once all retries are exhausted unless +`exit_after_retry_failure` from the +[`template_config`](/docs/agent/template-config) stanza is set to `true`. + Here are the options for the `retry` stanza: - `num_retries` `(int: 12)` - Specify how many times a failing request will diff --git a/website/content/docs/agent/template-config.mdx b/website/content/docs/agent/template-config.mdx index 706ad44b598e..04c087f6c13b 100644 --- a/website/content/docs/agent/template-config.mdx +++ b/website/content/docs/agent/template-config.mdx @@ -7,8 +7,11 @@ description: |- # Vault Agent Template Config -Template Config allows users of Vault Agent to determine the behavior that -pertains to the templating engine. +Template Config allows users to determine the behavior that +pertains to the templating engine within Vault Agent. + +For template-specific rendering configuration, refer to the parameters within the +[`template`](/docs/agent/template) stanza. ## Functionality diff --git a/website/content/docs/agent/template.mdx b/website/content/docs/agent/template.mdx index e2d3504221e0..8f93d91014b1 100644 --- a/website/content/docs/agent/template.mdx +++ b/website/content/docs/agent/template.mdx @@ -13,17 +13,20 @@ description: >- Vault Agent's Template functionality allows Vault secrets to be rendered to files using [Consul Template markup](https://github.com/hashicorp/consul-template/blob/master/docs/templating-language.md). +For globally applicable templating engine configuration, refer to the parameters +within the [`template_config`](/docs/agent/template-config) stanza. + ## Functionality -The `template` stanza configures the templating engine in the Vault agent for rendering -secrets to files using Consul Template markup language. Multiple `template` stanzas -can be defined to render multiple files. +The `template` stanza configures the Vault agent for rendering secrets to files +using Consul Template markup language. Multiple `template` stanzas can be +defined to render multiple files. When the agent is started with templating enabled, it will attempt to acquire a -Vault token using the configured Method. On failure, it will back off for a short -while (including some randomness to help prevent thundering herd scenarios) and -retry. On success, secrets defined in the templates will be retrieved from Vault and -rendered locally. +Vault token using the configured auto-auth Method. On failure, it will back off +for a short while (including some randomness to help prevent thundering herd +scenarios) and retry. On success, secrets defined in the templates will be +retrieved from Vault and rendered locally. ## Configuration From 9a73bb5b429dcf3f0d7496055733a556f336d03e Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang <1883212+calvn@users.noreply.github.com> Date: Fri, 18 Jun 2021 12:32:11 -0700 Subject: [PATCH 20/25] docs: update agent docs on retry stanza --- website/content/docs/agent/index.mdx | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/website/content/docs/agent/index.mdx b/website/content/docs/agent/index.mdx index c59a7ec78f34..dec1145cb733 100644 --- a/website/content/docs/agent/index.mdx +++ b/website/content/docs/agent/index.mdx @@ -102,10 +102,10 @@ templates, or are proxied requests coming from the proxy cache subsystem. Auto-auth, however, has its own notion of retrying and is not affected by this section. -For requests from the templating engine, Agent will restart its internal runner -and perform retries again once all retries are exhausted unless -`exit_after_retry_failure` from the -[`template_config`](/docs/agent/template-config) stanza is set to `true`. +For requests from the templating engine, Agent will reset its retry counter and +perform retries again once all retries are exhausted. This means that templating +will retry on failures indefinitely unless `exit_after_retry_failure` from the +[`template_config`][template-config] stanza is set to `true`. Here are the options for the `retry` stanza: @@ -121,7 +121,13 @@ result codes: any 50x code except 501 ("not implemented"), as well as 412 stale read due to eventual consistency. Requests coming from the template subsystem are retried regardless of the failure. -Second, the backoff algorithm used to set the time between retries differs for +Second, templating retries may be performed by both the templating engine _and_ +the cache proxy if Agent [persistent +cache][persitent-cache] is enabled. This is due to the +fact that templating requests go through the cache proxy when persistence is +enabled. + +Third, the backoff algorithm used to set the time between retries differs for the template and cache subsystems. This is a technical limitation we hope to address in the future. @@ -205,6 +211,7 @@ template { [vault]: /docs/agent#vault-stanza [autoauth]: /docs/agent/autoauth [caching]: /docs/agent/caching +[persitent-cache]: /docs/agent/caching/persistent-caches [template]: /docs/agent/template [template-config]: /docs/agent/template-config [listener]: /docs/agent#listener-stanza From 69b3204051dd8c92c2ded54c1fc99b5b216de104 Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang <1883212+calvn@users.noreply.github.com> Date: Fri, 18 Jun 2021 15:41:22 -0700 Subject: [PATCH 21/25] Apply suggestions from code review Co-authored-by: Jim Kalafut Co-authored-by: Theron Voran --- website/content/docs/agent/index.mdx | 4 ++-- website/content/docs/agent/template-config.mdx | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/website/content/docs/agent/index.mdx b/website/content/docs/agent/index.mdx index dec1145cb733..726a47bceab1 100644 --- a/website/content/docs/agent/index.mdx +++ b/website/content/docs/agent/index.mdx @@ -123,7 +123,7 @@ subsystem are retried regardless of the failure. Second, templating retries may be performed by both the templating engine _and_ the cache proxy if Agent [persistent -cache][persitent-cache] is enabled. This is due to the +cache][persistent-cache] is enabled. This is due to the fact that templating requests go through the cache proxy when persistence is enabled. @@ -211,7 +211,7 @@ template { [vault]: /docs/agent#vault-stanza [autoauth]: /docs/agent/autoauth [caching]: /docs/agent/caching -[persitent-cache]: /docs/agent/caching/persistent-caches +[persistent-cache]: /docs/agent/caching/persistent-caches [template]: /docs/agent/template [template-config]: /docs/agent/template-config [listener]: /docs/agent#listener-stanza diff --git a/website/content/docs/agent/template-config.mdx b/website/content/docs/agent/template-config.mdx index 04c087f6c13b..be13d1cae972 100644 --- a/website/content/docs/agent/template-config.mdx +++ b/website/content/docs/agent/template-config.mdx @@ -19,9 +19,9 @@ The `template_config` stanza configures overall default behavior for the templating engine. Note that `template_config` can only be defined once, and is different from the `template` stanza. Unlike `template` which focuses on where and how a specific secret is rendered, `template_config` contains parameters -affects how the templating engine as a whole behaves and its interaction with +affecting how the templating engine as a whole behaves and its interaction with the rest of Agent. This includes, but is not limited to, program exit behavior. -Other parameters that applies to the templating engine as a whole may be added +Other parameters that apply to the templating engine as a whole may be added over time. ### Interaction between `exit_on_retry_failure` and `error_on_missing_key` @@ -37,7 +37,7 @@ continue on with its rendering. If the desire is to have Agent fail and exit on a missing key, both `template.error_on_missing_key` and `template_config.exit_on_retry_failure` must be set to true. Otherwise, the templating engine will error and render to its -destination, but agent will not exit and retry until it's able to do so or until +destination, but agent will not exit and will retry until the key exists or until the process is terminated. Note that a missing key from a secret's response is different from a missing or From 5d41beee66fe753765ca3f9df2d7e7bf1c2f770f Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang <1883212+calvn@users.noreply.github.com> Date: Fri, 18 Jun 2021 16:29:03 -0700 Subject: [PATCH 22/25] Update changelog/11775.txt Co-authored-by: Brian Kassouf --- changelog/11775.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/changelog/11775.txt b/changelog/11775.txt index 0aa4359c0b60..be3735521a84 100644 --- a/changelog/11775.txt +++ b/changelog/11775.txt @@ -1,9 +1,9 @@ ```release-note:change -agent: The template engine errors will no longer cause agent to exit unless +agent: Errors in the template engine will no longer cause agent to exit unless explicitly defined to do so. A new configuration parameter, `exit_on_retry_failure`, within the new top-level stanza, `template_config`, can be set to `true` in order to cause agent to exit. Note that for agent to exit if `template.error_on_missing_key` is set to `true`, `exit_on_retry_failure` must be also set to `true`. Otherwise, the template engine will log an error but then restart its internal runner. -``` \ No newline at end of file +``` From 8e6be32ef1db75e851a8978692d2d9b964118e05 Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang <1883212+calvn@users.noreply.github.com> Date: Fri, 18 Jun 2021 16:34:33 -0700 Subject: [PATCH 23/25] agent/test: rename expectExit to expectExitFromError --- command/agent_test.go | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/command/agent_test.go b/command/agent_test.go index b97ccb2f9321..a557b2aafafe 100644 --- a/command/agent_test.go +++ b/command/agent_test.go @@ -1775,80 +1775,80 @@ func TestAgent_TemplateConfig_ExitOnRetryFailure(t *testing.T) { templateContents string expectTemplateRender string templateErrorOnMissingKey bool - expectExit bool expectError bool + expectExitFromError bool }{ "true, no template error": { exitOnRetryFailure: pointerutil.BoolPtr(true), templateContents: templateContents(0), expectTemplateRender: templateRendered(0), templateErrorOnMissingKey: false, - expectExit: true, expectError: false, + expectExitFromError: true, }, "true, with non-existent secret": { exitOnRetryFailure: pointerutil.BoolPtr(true), templateContents: badTemplateContent, expectTemplateRender: "", templateErrorOnMissingKey: false, - expectExit: true, expectError: true, + expectExitFromError: true, }, "true, with missing key": { exitOnRetryFailure: pointerutil.BoolPtr(true), templateContents: missingKeyTemplateContent, expectTemplateRender: missingKeyTemplateRender, templateErrorOnMissingKey: false, - expectExit: true, expectError: false, + expectExitFromError: true, }, "true, with missing key, with error_on_missing_key": { exitOnRetryFailure: pointerutil.BoolPtr(true), templateContents: missingKeyTemplateContent, expectTemplateRender: "", templateErrorOnMissingKey: true, - expectExit: true, expectError: true, + expectExitFromError: true, }, "false, no template error": { exitOnRetryFailure: pointerutil.BoolPtr(false), templateContents: templateContents(0), expectTemplateRender: templateRendered(0), templateErrorOnMissingKey: false, - expectExit: false, expectError: false, + expectExitFromError: false, }, "false, with non-existent secret": { exitOnRetryFailure: pointerutil.BoolPtr(false), templateContents: badTemplateContent, expectTemplateRender: "", templateErrorOnMissingKey: false, - expectExit: false, expectError: true, + expectExitFromError: false, }, "false, with missing key": { exitOnRetryFailure: pointerutil.BoolPtr(false), templateContents: missingKeyTemplateContent, expectTemplateRender: missingKeyTemplateRender, templateErrorOnMissingKey: false, - expectExit: false, expectError: false, + expectExitFromError: false, }, "false, with missing key, with error_on_missing_key": { exitOnRetryFailure: pointerutil.BoolPtr(false), templateContents: missingKeyTemplateContent, expectTemplateRender: missingKeyTemplateRender, templateErrorOnMissingKey: true, - expectExit: false, expectError: true, + expectExitFromError: false, }, "missing": { exitOnRetryFailure: nil, templateContents: templateContents(0), expectTemplateRender: templateRendered(0), templateErrorOnMissingKey: false, - expectExit: false, expectError: false, + expectExitFromError: false, }, } @@ -1982,8 +1982,8 @@ vault { switch { case (code != 0 || err != nil) && tc.expectError: - if exited != tc.expectExit { - t.Fatalf("expected program exit to be '%t', got '%t'", tc.expectExit, exited) + if exited != tc.expectExitFromError { + t.Fatalf("expected program exit to be '%t', got '%t'", tc.expectExitFromError, exited) } case code == 0 && err == nil && !tc.expectError: default: From b3dea46d5278db4781f9b2ac25465052cb12b1bc Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang <1883212+calvn@users.noreply.github.com> Date: Fri, 18 Jun 2021 17:02:41 -0700 Subject: [PATCH 24/25] agent/test: add check on early exits on the happy path --- command/agent_test.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/command/agent_test.go b/command/agent_test.go index a557b2aafafe..9bcb29713eb3 100644 --- a/command/agent_test.go +++ b/command/agent_test.go @@ -1784,7 +1784,7 @@ func TestAgent_TemplateConfig_ExitOnRetryFailure(t *testing.T) { expectTemplateRender: templateRendered(0), templateErrorOnMissingKey: false, expectError: false, - expectExitFromError: true, + expectExitFromError: false, }, "true, with non-existent secret": { exitOnRetryFailure: pointerutil.BoolPtr(true), @@ -1800,7 +1800,7 @@ func TestAgent_TemplateConfig_ExitOnRetryFailure(t *testing.T) { expectTemplateRender: missingKeyTemplateRender, templateErrorOnMissingKey: false, expectError: false, - expectExitFromError: true, + expectExitFromError: false, }, "true, with missing key, with error_on_missing_key": { exitOnRetryFailure: pointerutil.BoolPtr(true), @@ -1920,7 +1920,7 @@ vault { // Channel to let verify() know to stop early if agent // has exited cmdRunDoneCh := make(chan struct{}) - var exited bool + var exitedEarly bool wg := &sync.WaitGroup{} wg.Add(1) @@ -1943,7 +1943,7 @@ vault { for { select { case <-cmdRunDoneCh: - exited = true + exitedEarly = true return nil case <-timeout: return fmt.Errorf("timed out waiting for templates to render, last error: %w", err) @@ -1982,10 +1982,13 @@ vault { switch { case (code != 0 || err != nil) && tc.expectError: - if exited != tc.expectExitFromError { - t.Fatalf("expected program exit to be '%t', got '%t'", tc.expectExitFromError, exited) + if exitedEarly != tc.expectExitFromError { + t.Fatalf("expected program exit due to error to be '%t', got '%t'", tc.expectExitFromError, exitedEarly) } case code == 0 && err == nil && !tc.expectError: + if exitedEarly { + t.Fatalf("did not expect program to exit before verify completes") + } default: if code != 0 { t.Logf("output from agent:\n%s", ui.OutputWriter.String()) From 08be22de9a24816db7d69ccee4ce8e3984c03a10 Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang <1883212+calvn@users.noreply.github.com> Date: Mon, 21 Jun 2021 10:49:38 -0700 Subject: [PATCH 25/25] Update website/content/docs/agent/template-config.mdx Co-authored-by: Tom Proctor --- website/content/docs/agent/template-config.mdx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/website/content/docs/agent/template-config.mdx b/website/content/docs/agent/template-config.mdx index be13d1cae972..645a847dc834 100644 --- a/website/content/docs/agent/template-config.mdx +++ b/website/content/docs/agent/template-config.mdx @@ -7,8 +7,7 @@ description: |- # Vault Agent Template Config -Template Config allows users to determine the behavior that -pertains to the templating engine within Vault Agent. +Template Config configures Vault Agent behavior common to all `template` stanzas. For template-specific rendering configuration, refer to the parameters within the [`template`](/docs/agent/template) stanza.