feat(agent): add retry configuration for vault agent #10644

RXC001 · 2021-01-04T22:32:53Z

Adds retry configuration for Vault Agent. Structure is similar to the retry blocks found in consul-template

hashicorp-cla · 2021-01-04T22:32:56Z

All committers have signed the CLA.

HridoyRoy

👍 LGTM, but I'd like another pair of eyes on this in case I missed something. @calvn could you take a look?

calvn · 2021-01-12T22:55:48Z

command/agent/config/config.go

@@ -40,6 +40,7 @@ type Vault struct {
 	ClientCert       string      `hcl:"client_cert"`
 	ClientKey        string      `hcl:"client_key"`
 	TLSServerName    string      `hcl:"tls_server_name"`
+	Retry            *Retry      `hcl:"retry"`


The retry behavior in here is specifically for the template server (at least for now) so I'm not entirely convinced that it should live in the vault stanza since this doesn't control retry logic when agent communicates with the server for auto-auth and/or caching.

@HridoyRoy thoughts on making this be a top-level template_retry stanza instead?

We did think about that, but the existing retry context is under ctv.Vault.Retry = &ctconfig.RetryConfig{Attempts: &ts.testingLimitRetry} and consul-template puts it under the vault section so we kept the same structure location.

As for the auto-auth retry that is a feature we would like to add as well as our use case of vault needs to be fault tolerant of network disconnects both for getting new tokens and template rendering, this MR was the first step.

Hm, I do agree that "Retry" in the Vault section shouldn't be a configuration parameter that is limited to the consul template use-case. Let me dig into this a bit -- I'll circle back soon.

Hi @ibspoof @RXC001 , sorry for the delay in circling back!

We'd like to move the Retry stanza into as a top level config named template_retry. ctv is the consul template, so ctv.Vault.Retry can be set to values that exist in a structurally different location in the Vault config. Since this retry functionality is currently limited in scope to the consul-template, we'd want to make that clear with the name and location of the stanza, instead of keeping structural similarity between the consul-template config and the vault config.

Thanks so much!

Makes sense we had the same discussion in our team. Will get that changed and pushed.

HridoyRoy · 2021-01-15T18:18:35Z

command/agent/config/config.go

@@ -40,6 +40,7 @@ type Vault struct {
 	ClientCert       string      `hcl:"client_cert"`
 	ClientKey        string      `hcl:"client_key"`
 	TLSServerName    string      `hcl:"tls_server_name"`
+	Retry            *Retry      `hcl:"retry"`


Hi @ibspoof @RXC001 , sorry for the delay in circling back!

We'd like to move the Retry stanza into as a top level config named template_retry. ctv is the consul template, so ctv.Vault.Retry can be set to values that exist in a structurally different location in the Vault config. Since this retry functionality is currently limited in scope to the consul-template, we'd want to make that clear with the name and location of the stanza, instead of keeping structural similarity between the consul-template config and the vault config.

Thanks so much!

RXC001 · 2021-01-15T20:03:52Z

Moving the Retry stanza to the top level makes sense. I can make that happen.

HridoyRoy

lgtm!

hashicorp#10750) * Move the declaration to a OSS build tag file to not have it collide with ent declarations * Add comment * Remove comment to trigger ci

HridoyRoy · 2021-01-25T17:22:51Z

Hi @RXC001 , I think this needs to merge from HashiCorp/master for the tests to pass (seems to be missing #10749 ).
Thanks, and thank you for your patience.

…ashicorp#10742)

…om/RXC001/vault into feat/agent/add-retry-configuration

vercel · 2021-01-25T18:40:07Z

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.

vault-storybook – ./ui

🔍 Inspect: https://vercel.com/hashicorp/vault-storybook/ot1pw5ox3
✅ Preview: Canceled

…those from consul-template configuration. Also fix some existing config docs that weren't adhering to our conventions.

…those from consul-template configuration. Also fix some existing config docs that weren't adhering to our conventions. (#10911)

* master: Adds API docs for max_age role parameter of JWT/OIDC auth method (#10916) UI/Database Secrets Engine cleanup (#10949) helper/metricsutil: Prevent potential Ticker leak (#10913) core/expiration: Add backoff jitter to the expiration retries (#10937) Revert "Vault Dependency Upgrades [VAULT-871] (#10903)" (#10939) Vault Dependency Upgrades [VAULT-871] (#10903) Add docs for Agent's template_retry option added in #10644, based on those from consul-template configuration. Also fix some existing config docs that weren't adhering to our conventions. (#10911) UI Database Secrets Engine (MongoDB) (#10655) OpenAPI - Don't panic if field isn't found (#10929) Vault-1403 Switch Expiration Manager to use Fairsharing Backpressure (#1709) (#10932) Update KV Secrets Engine index (#10933)

other-ryan · 2021-02-23T00:00:17Z

Thanks for this feature. This is awesome!

It appears this didn't make it into the 1.6.2 release. Any timeframe of when we can expect this to be part of the GA build?

ncabatoff · 2021-02-23T16:21:42Z

Thanks for this feature. This is awesome!

It appears this didn't make it into the 1.6.2 release. Any timeframe of when we can expect this to be part of the GA build?

It should be included in the upcoming 1.7 release. We don't have an official timeframe that I know of, but if you look at the past cadence of Vault major releases you'll see that we're pretty regular and that 1.7 is likely not far off.

other-ryan · 2021-02-23T18:11:26Z

@ncabatoff thanks for the info. Just a heads up that the documentation for these configs are live at https://www.vaultproject.io/docs/agent/template and may cause some confusion for folks trying to figure out why the configs aren't working.

ncabatoff · 2021-02-23T18:36:53Z

Unfortunately this has been an issue for the longest time. Our docs reflect not the latest available version, but what's in the master branch. There is work ongoing to provide a versioned view of documentation which should fix this, but I don't know when that will be released. Sorry about the confusion in the meantime.

kalafut · 2021-02-26T06:29:49Z

@ncabatoff The docs for this are showing up because the docs-cherrypick label was added in #10911. As far as I can tell we're rendering stable-website as expected. I reverted the docs commit there.

…ased on those from consul-template configuration. Also fix some existing config docs that weren't adhering to our conventions. (#10911)" 8e0c00c

ncabatoff · 2021-03-18T18:11:30Z

Hi folks,

I'm afraid I have some potentially bad news: I'm reverting this change. In its place, #11113 is adding similar functionality that will apply not just to templating, but also to requests that go through the proxy/cache. I say this is potentially bad news because the new configuration only allows you to specify the number of retries, there's no provision like in template_retry for specifying backoff or max_backoff.

Some context for this decision:

we needed the ability to configure retries for proxy requests for another 1.7 feature
yet another 1.7 feature allows template requests to go through the cache, and having independent configuration for proxy requests and template requests would be problematic in that scenario
templating uses a different backoff algorithm than proxy requests, so providing backoff configuration that has very different meanings for the two systems would've created confusion
we're not persuaded that being able to configure backoff intervals is strictly necessary

In the future we plan to do further work to better unify templating with the rest of Agent, such that we won't have multiple backoff algorithms in use, and we can revisit the decision as to whether to have configuration for backoff intervals.

ibspoof mentioned this pull request Jan 4, 2021

Agent: Allow setting vault retry params for rendering templates #10646

Closed

RXC001 marked this pull request as ready for review January 4, 2021 23:48

HridoyRoy reviewed Jan 12, 2021

View reviewed changes

HridoyRoy requested a review from calvn January 12, 2021 18:47

calvn reviewed Jan 12, 2021

View reviewed changes

HridoyRoy suggested changes Jan 15, 2021

View reviewed changes

RXC001 added 3 commits January 15, 2021 17:38

feat(agent): add retry configuration for vault agent

7aadd2f

feat(agent): add test fixtures for retry

c3ebb69

fix(retry): move retry stanza to top level as template_retry

f46580e

vercel bot temporarily deployed to Preview January 16, 2021 01:41 Inactive

vercel bot temporarily deployed to Preview January 16, 2021 01:51 Inactive

RXC001 force-pushed the feat/agent/add-retry-configuration branch from 6edb667 to f46580e Compare January 16, 2021 01:59

vercel bot temporarily deployed to Preview January 16, 2021 02:00 Inactive

fix(retry): add retry config to ServerConfig struct

910f086

vercel bot temporarily deployed to Preview January 16, 2021 02:15 Inactive

fix(retry): point config parser to parse template_retry instead of retry

f8dd06c

vercel bot temporarily deployed to Preview January 16, 2021 02:25 Inactive

Merge branch 'master' into feat/agent/add-retry-configuration

ae995b1

vercel bot temporarily deployed to Preview January 19, 2021 19:13 Inactive

Merge branch 'master' into feat/agent/add-retry-configuration

1add6d7

vercel bot temporarily deployed to Preview January 19, 2021 20:58 Inactive

HridoyRoy approved these changes Jan 19, 2021

View reviewed changes

HridoyRoy mentioned this pull request Jan 19, 2021

docs PR for template-retry stanza #10727

Closed

petems mentioned this pull request Jan 21, 2021

vault agent retry maximum #6001

Closed

Merge branch 'master' into feat/agent/add-retry-configuration

0922030

vercel bot deployed to Preview January 22, 2021 17:18 View deployment

jescalan and others added 2 commits January 22, 2021 15:16

remove netlify config (hashicorp#10711)

1fa248e

Fix build (hashicorp#10749)

78022fc

Move the declaration to a OSS build tag file to not have it collide w… (

a8519a8

hashicorp#10750) * Move the declaration to a OSS build tag file to not have it collide with ent declarations * Add comment * Remove comment to trigger ci

Mark Gritter and others added 7 commits January 25, 2021 11:25

Unconditionally use the root namespace when calling sys/seal-status. (h…

f61280b

…ashicorp#10742)

feat(agent): add retry configuration for vault agent

8d46cb0

feat(agent): add test fixtures for retry

e232a43

fix(retry): move retry stanza to top level as template_retry

fb13b72

fix(retry): add retry config to ServerConfig struct

6009c26

fix(retry): point config parser to parse template_retry instead of retry

af27b63

Merge branch 'feat/agent/add-retry-configuration' of https://github.c…

4b43d47

…om/RXC001/vault into feat/agent/add-retry-configuration

vercel bot temporarily deployed to Preview – vault-storybook January 25, 2021 18:40 Inactive

vercel bot temporarily deployed to Preview – vault January 25, 2021 18:40 Inactive

HridoyRoy merged commit 8c304ed into hashicorp:master Jan 25, 2021

HridoyRoy added this to the 1.7 milestone Jan 25, 2021

ncabatoff added a commit that referenced this pull request Feb 12, 2021

Add docs for Agent's template_retry option added in #10644, based on …

14fa4ce

…those from consul-template configuration. Also fix some existing config docs that weren't adhering to our conventions.

HridoyRoy pushed a commit that referenced this pull request Feb 18, 2021

Add docs for Agent's template_retry option added in #10644, based on …

3cc659a

…those from consul-template configuration. Also fix some existing config docs that weren't adhering to our conventions. (#10911)

github-actions bot pushed a commit that referenced this pull request Feb 18, 2021

Add docs for Agent's template_retry option added in #10644, based on …

8e0c00c

…those from consul-template configuration. Also fix some existing config docs that weren't adhering to our conventions. (#10911)

HridoyRoy mentioned this pull request Feb 23, 2021

Changelog: Agent Retry Stanza #10981

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(agent): add retry configuration for vault agent #10644

feat(agent): add retry configuration for vault agent #10644

RXC001 commented Jan 4, 2021

hashicorp-cla commented Jan 4, 2021 •

edited

Loading

HridoyRoy left a comment

calvn Jan 12, 2021

ibspoof Jan 13, 2021

HridoyRoy Jan 13, 2021

HridoyRoy Jan 15, 2021

ibspoof Jan 15, 2021

HridoyRoy Jan 15, 2021

RXC001 commented Jan 15, 2021

HridoyRoy left a comment

HridoyRoy commented Jan 25, 2021 •

edited

Loading

vercel bot commented Jan 25, 2021 •

edited

Loading

other-ryan commented Feb 23, 2021

ncabatoff commented Feb 23, 2021

other-ryan commented Feb 23, 2021

ncabatoff commented Feb 23, 2021

kalafut commented Feb 26, 2021 •

edited

Loading

ncabatoff commented Mar 18, 2021

feat(agent): add retry configuration for vault agent #10644

feat(agent): add retry configuration for vault agent #10644

Conversation

RXC001 commented Jan 4, 2021

hashicorp-cla commented Jan 4, 2021 • edited Loading

HridoyRoy left a comment

Choose a reason for hiding this comment

calvn Jan 12, 2021

Choose a reason for hiding this comment

ibspoof Jan 13, 2021

Choose a reason for hiding this comment

HridoyRoy Jan 13, 2021

Choose a reason for hiding this comment

HridoyRoy Jan 15, 2021

Choose a reason for hiding this comment

ibspoof Jan 15, 2021

Choose a reason for hiding this comment

HridoyRoy Jan 15, 2021

Choose a reason for hiding this comment

RXC001 commented Jan 15, 2021

HridoyRoy left a comment

Choose a reason for hiding this comment

HridoyRoy commented Jan 25, 2021 • edited Loading

vercel bot commented Jan 25, 2021 • edited Loading

vault-storybook – ./ui

other-ryan commented Feb 23, 2021

ncabatoff commented Feb 23, 2021

other-ryan commented Feb 23, 2021

ncabatoff commented Feb 23, 2021

kalafut commented Feb 26, 2021 • edited Loading

ncabatoff commented Mar 18, 2021

hashicorp-cla commented Jan 4, 2021 •

edited

Loading

HridoyRoy commented Jan 25, 2021 •

edited

Loading

vercel bot commented Jan 25, 2021 •

edited

Loading

kalafut commented Feb 26, 2021 •

edited

Loading