From 95b5449e8ad704b5d9627b29e76cf087a2ff4722 Mon Sep 17 00:00:00 2001 From: Violet Hynes Date: Tue, 6 Sep 2022 10:35:54 -0400 Subject: [PATCH] VAULT-6575 Vault agent respects retry config even with caching set (#16970) * VAULT-6575 Vault agent respects retry config even with caching set * VAULT-6575 Add changelog * VAULT-6575 Change log levels --- changelog/16970.txt | 3 +++ command/agent/cache/api_proxy.go | 2 +- command/agent/cache/handler.go | 2 +- command/agent/cache/lease_cache.go | 2 +- command/agent/template/template.go | 22 ++-------------------- 5 files changed, 8 insertions(+), 23 deletions(-) create mode 100644 changelog/16970.txt diff --git a/changelog/16970.txt b/changelog/16970.txt new file mode 100644 index 000000000..0f0a9f8d6 --- /dev/null +++ b/changelog/16970.txt @@ -0,0 +1,3 @@ +```release-note:bug +agent: Agent will now respect `max_retries` retry configuration even when caching is set. +``` diff --git a/command/agent/cache/api_proxy.go b/command/agent/cache/api_proxy.go index 9523d310b..1a754e064 100644 --- a/command/agent/cache/api_proxy.go +++ b/command/agent/cache/api_proxy.go @@ -112,7 +112,7 @@ func (ap *APIProxy) Send(ctx context.Context, req *SendRequest) (*SendResponse, } // Make the request to Vault and get the response - ap.logger.Info("forwarding request", "method", req.Request.Method, "path", req.Request.URL.Path) + ap.logger.Info("forwarding request to Vault", "method", req.Request.Method, "path", req.Request.URL.Path) resp, err := client.RawRequestWithContext(ctx, fwReq) if resp == nil && err != nil { diff --git a/command/agent/cache/handler.go b/command/agent/cache/handler.go index 2beea6cc2..60f9e046a 100644 --- a/command/agent/cache/handler.go +++ b/command/agent/cache/handler.go @@ -54,7 +54,7 @@ func Handler(ctx context.Context, logger hclog.Logger, proxier Proxier, inmemSin resp, err := proxier.Send(ctx, req) if err != nil { - // If this is a api.Response error, don't wrap the response. + // If this is an api.Response error, don't wrap the response. if resp != nil && resp.Response.Error() != nil { copyHeader(w.Header(), resp.Response.Header) w.WriteHeader(resp.Response.StatusCode) diff --git a/command/agent/cache/lease_cache.go b/command/agent/cache/lease_cache.go index 1f7224691..9bc79d4a2 100644 --- a/command/agent/cache/lease_cache.go +++ b/command/agent/cache/lease_cache.go @@ -274,7 +274,7 @@ func (c *LeaseCache) Send(ctx context.Context, req *SendRequest) (*SendResponse, return cachedResp, nil } - c.logger.Debug("forwarding request", "method", req.Request.Method, "path", req.Request.URL.Path) + c.logger.Debug("forwarding request from cache", "method", req.Request.Method, "path", req.Request.URL.Path) // Pass the request down and get a response resp, err := c.proxier.Send(ctx, req) diff --git a/command/agent/template/template.go b/command/agent/template/template.go index 1dbfc8e61..32e9022bb 100644 --- a/command/agent/template/template.go +++ b/command/agent/template/template.go @@ -264,10 +264,8 @@ func newRunnerConfig(sc *ServerConfig, templates ctconfig.TemplateConfigs) (*ctc ServerName: pointerutil.StringPtr(""), } - // The cache does its own retry management based on sc.AgentConfig.Retry, - // so we only want to set this up for templating if we're not routing - // templating through the cache. We do need to assign something to Retry - // though or it will use its default of 12 retries. + // We need to assign something to Vault.Retry or it will use its default of 12 retries. + // This retry value will be respected regardless of if we use the cache. var attempts int if sc.AgentConfig.Vault != nil && sc.AgentConfig.Vault.Retry != nil { attempts = sc.AgentConfig.Vault.Retry.NumRetries @@ -275,21 +273,6 @@ func newRunnerConfig(sc *ServerConfig, templates ctconfig.TemplateConfigs) (*ctc // Use the cache if available or fallback to the Vault server values. if sc.AgentConfig.Cache != nil { - attempts = 0 - - // If we don't want exit on template retry failure (i.e. unlimited - // 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 - } - if sc.AgentConfig.Cache.InProcDialer == nil { return nil, fmt.Errorf("missing in-process dialer configuration") } @@ -301,7 +284,6 @@ func newRunnerConfig(sc *ServerConfig, templates ctconfig.TemplateConfigs) (*ctc // setting it here to override the setting at the top of this function, // and to prevent the vault/http client from defaulting to https. conf.Vault.Address = pointerutil.StringPtr("http://127.0.0.1:8200") - } else if strings.HasPrefix(sc.AgentConfig.Vault.Address, "https") || sc.AgentConfig.Vault.CACert != "" { skipVerify := sc.AgentConfig.Vault.TLSSkipVerify verify := !skipVerify