From e9535bda2f2acce8f50076296c26f1d9651cfb42 Mon Sep 17 00:00:00 2001 From: Jason O'Donnell <2160810+jasonodonnell@users.noreply.github.com> Date: Fri, 29 Apr 2022 12:31:32 -0400 Subject: [PATCH] agent/auto-auth: Add `min_backoff` to set first backoff value (#15204) * Add initial_backoff to auto-auth method * Disable retries in client * Fix bug * Thread initial backoff to CT * Add comment * Change to min_backoff * changelog * remove initial references, review * fix test * Thread max_backoff through * Add doc note for max_backoff/templating --- changelog/15204.txt | 3 + command/agent.go | 11 +++- command/agent/auth/auth.go | 31 ++++++++-- command/agent/auth/auth_test.go | 57 ++++++++++++++++++- command/agent/config/config.go | 10 ++++ command/agent/config/config_test.go | 43 ++++++++++++++ .../config-method-initial-backoff.hcl | 20 +++++++ command/agent/template/template.go | 13 +++++ website/content/docs/agent/autoauth/index.mdx | 10 +++- 9 files changed, 188 insertions(+), 10 deletions(-) create mode 100644 changelog/15204.txt create mode 100644 command/agent/config/test-fixtures/config-method-initial-backoff.hcl diff --git a/changelog/15204.txt b/changelog/15204.txt new file mode 100644 index 000000000..f4c5b8cc4 --- /dev/null +++ b/changelog/15204.txt @@ -0,0 +1,3 @@ +```release-note:improvement +agent/auto-auth: Add `min_backoff` to the method stanza for configuring initial backoff duration. +``` diff --git a/command/agent.go b/command/agent.go index 6bafd4cb0..2b8486515 100644 --- a/command/agent.go +++ b/command/agent.go @@ -790,10 +790,19 @@ func (c *AgentCommand) Run(args []string) int { // Start auto-auth and sink servers if method != nil { enableTokenCh := len(config.Templates) > 0 + + // Auth Handler is going to set its own retry values, so we want to + // work on a copy of the client to not affect other subsystems. + clonedClient, err := c.client.Clone() + if err != nil { + c.UI.Error(fmt.Sprintf("Error cloning client for auth handler: %v", err)) + return 1 + } ah := auth.NewAuthHandler(&auth.AuthHandlerConfig{ Logger: c.logger.Named("auth.handler"), - Client: c.client, + Client: clonedClient, WrapTTL: config.AutoAuth.Method.WrapTTL, + MinBackoff: config.AutoAuth.Method.MinBackoff, MaxBackoff: config.AutoAuth.Method.MaxBackoff, EnableReauthOnNewCredentials: config.AutoAuth.EnableReauthOnNewCredentials, EnableTemplateTokenCh: enableTokenCh, diff --git a/command/agent/auth/auth.go b/command/agent/auth/auth.go index c00028608..7406d7c8c 100644 --- a/command/agent/auth/auth.go +++ b/command/agent/auth/auth.go @@ -15,7 +15,7 @@ import ( ) const ( - initialBackoff = 1 * time.Second + defaultMinBackoff = 1 * time.Second defaultMaxBackoff = 5 * time.Minute ) @@ -55,6 +55,7 @@ type AuthHandler struct { random *rand.Rand wrapTTL time.Duration maxBackoff time.Duration + minBackoff time.Duration enableReauthOnNewCredentials bool enableTemplateTokenCh bool } @@ -64,6 +65,7 @@ type AuthHandlerConfig struct { Client *api.Client WrapTTL time.Duration MaxBackoff time.Duration + MinBackoff time.Duration Token string EnableReauthOnNewCredentials bool EnableTemplateTokenCh bool @@ -80,6 +82,7 @@ func NewAuthHandler(conf *AuthHandlerConfig) *AuthHandler { client: conf.Client, random: rand.New(rand.NewSource(int64(time.Now().Nanosecond()))), wrapTTL: conf.WrapTTL, + minBackoff: conf.MinBackoff, maxBackoff: conf.MaxBackoff, enableReauthOnNewCredentials: conf.EnableReauthOnNewCredentials, enableTemplateTokenCh: conf.EnableTemplateTokenCh, @@ -104,7 +107,15 @@ func (ah *AuthHandler) Run(ctx context.Context, am AuthMethod) error { return errors.New("auth handler: nil auth method") } - backoff := newAgentBackoff(ah.maxBackoff) + if ah.minBackoff <= 0 { + ah.minBackoff = defaultMinBackoff + } + + backoff := newAgentBackoff(ah.minBackoff, ah.maxBackoff) + + if backoff.min >= backoff.max { + return errors.New("auth handler: min_backoff cannot be greater than max_backoff") + } ah.logger.Info("starting auth handler") defer func() { @@ -164,6 +175,10 @@ func (ah *AuthHandler) Run(ctx context.Context, am AuthMethod) error { clientToUse = ah.client } + // Disable retry on the client to ensure our backoffOrQuit function is + // the only source of retry/backoff. + clientToUse.SetMaxRetries(0) + var secret *api.Secret = new(api.Secret) if first && ah.token != "" { ah.logger.Debug("using preloaded token") @@ -342,18 +357,24 @@ func (ah *AuthHandler) Run(ctx context.Context, am AuthMethod) error { // agentBackoff tracks exponential backoff state. type agentBackoff struct { + min time.Duration max time.Duration current time.Duration } -func newAgentBackoff(max time.Duration) *agentBackoff { +func newAgentBackoff(min, max time.Duration) *agentBackoff { if max <= 0 { max = defaultMaxBackoff } + if min <= 0 { + min = defaultMinBackoff + } + return &agentBackoff{ + current: min, max: max, - current: initialBackoff, + min: min, } } @@ -372,7 +393,7 @@ func (b *agentBackoff) next() { } func (b *agentBackoff) reset() { - b.current = initialBackoff + b.current = b.min } func (b agentBackoff) String() string { diff --git a/command/agent/auth/auth_test.go b/command/agent/auth/auth_test.go index 05c24fe1f..e0c2442d3 100644 --- a/command/agent/auth/auth_test.go +++ b/command/agent/auth/auth_test.go @@ -109,10 +109,10 @@ consumption: func TestAgentBackoff(t *testing.T) { max := 1024 * time.Second - backoff := newAgentBackoff(max) + backoff := newAgentBackoff(defaultMinBackoff, max) // Test initial value - if backoff.current != initialBackoff { + if backoff.current != defaultMinBackoff { t.Fatalf("expected 1s initial backoff, got: %v", backoff.current) } @@ -139,7 +139,58 @@ func TestAgentBackoff(t *testing.T) { // Test reset backoff.reset() - if backoff.current != initialBackoff { + if backoff.current != defaultMinBackoff { t.Fatalf("expected 1s backoff after reset, got: %v", backoff.current) } } + +func TestAgentMinBackoffCustom(t *testing.T) { + type test struct { + minBackoff time.Duration + want time.Duration + } + + tests := []test{ + {minBackoff: 0 * time.Second, want: 1 * time.Second}, + {minBackoff: 1 * time.Second, want: 1 * time.Second}, + {minBackoff: 5 * time.Second, want: 5 * time.Second}, + {minBackoff: 10 * time.Second, want: 10 * time.Second}, + } + + for _, test := range tests { + max := 1024 * time.Second + backoff := newAgentBackoff(test.minBackoff, max) + + // Test initial value + if backoff.current != test.want { + t.Fatalf("expected %d initial backoff, got: %v", test.want, backoff.current) + } + + // Test that backoff values are in expected range (75-100% of 2*previous) + for i := 0; i < 5; i++ { + old := backoff.current + backoff.next() + + expMax := 2 * old + expMin := 3 * expMax / 4 + + if backoff.current < expMin || backoff.current > expMax { + t.Fatalf("expected backoff in range %v to %v, got: %v", expMin, expMax, backoff) + } + } + + // Test that backoff is capped + for i := 0; i < 100; i++ { + backoff.next() + if backoff.current > max { + t.Fatalf("backoff exceeded max of 100s: %v", backoff) + } + } + + // Test reset + backoff.reset() + if backoff.current != test.want { + t.Fatalf("expected %d backoff after reset, got: %v", test.want, backoff.current) + } + } +} diff --git a/command/agent/config/config.go b/command/agent/config/config.go index 7a105573d..e68af26f6 100644 --- a/command/agent/config/config.go +++ b/command/agent/config/config.go @@ -112,6 +112,8 @@ type Method struct { MountPath string `hcl:"mount_path"` WrapTTLRaw interface{} `hcl:"wrap_ttl"` WrapTTL time.Duration `hcl:"-"` + MinBackoffRaw interface{} `hcl:"min_backoff"` + MinBackoff time.Duration `hcl:"-"` MaxBackoffRaw interface{} `hcl:"max_backoff"` MaxBackoff time.Duration `hcl:"-"` Namespace string `hcl:"namespace"` @@ -470,6 +472,14 @@ func parseAutoAuth(result *Config, list *ast.ObjectList) error { result.AutoAuth.Method.MaxBackoffRaw = nil } + if result.AutoAuth.Method.MinBackoffRaw != nil { + var err error + if result.AutoAuth.Method.MinBackoff, err = parseutil.ParseDurationSecond(result.AutoAuth.Method.MinBackoffRaw); err != nil { + return err + } + result.AutoAuth.Method.MinBackoffRaw = nil + } + return nil } diff --git a/command/agent/config/config_test.go b/command/agent/config/config_test.go index 2bb8844ba..1a1aec2a1 100644 --- a/command/agent/config/config_test.go +++ b/command/agent/config/config_test.go @@ -290,6 +290,49 @@ func TestLoadConfigFile_Method_Wrapping(t *testing.T) { } } +func TestLoadConfigFile_Method_InitialBackoff(t *testing.T) { + config, err := LoadConfig("./test-fixtures/config-method-initial-backoff.hcl") + if err != nil { + t.Fatalf("err: %s", err) + } + + expected := &Config{ + SharedConfig: &configutil.SharedConfig{ + PidFile: "./pidfile", + }, + AutoAuth: &AutoAuth{ + Method: &Method{ + Type: "aws", + MountPath: "auth/aws", + WrapTTL: 5 * time.Minute, + MinBackoff: 5 * time.Second, + MaxBackoff: 2 * time.Minute, + Config: map[string]interface{}{ + "role": "foobar", + }, + }, + Sinks: []*Sink{ + { + Type: "file", + Config: map[string]interface{}{ + "path": "/tmp/file-foo", + }, + }, + }, + }, + Vault: &Vault{ + Retry: &Retry{ + NumRetries: 12, + }, + }, + } + + config.Prune() + if diff := deep.Equal(config, expected); diff != nil { + t.Fatal(diff) + } +} + func TestLoadConfigFile_AgentCache_NoAutoAuth(t *testing.T) { config, err := LoadConfig("./test-fixtures/config-cache-no-auto_auth.hcl") if err != nil { diff --git a/command/agent/config/test-fixtures/config-method-initial-backoff.hcl b/command/agent/config/test-fixtures/config-method-initial-backoff.hcl new file mode 100644 index 000000000..6b9343aa4 --- /dev/null +++ b/command/agent/config/test-fixtures/config-method-initial-backoff.hcl @@ -0,0 +1,20 @@ +pid_file = "./pidfile" + +auto_auth { + method { + type = "aws" + wrap_ttl = 300 + config = { + role = "foobar" + } + max_backoff = "2m" + min_backoff = "5s" + } + + sink { + type = "file" + config = { + path = "/tmp/file-foo" + } + } +} diff --git a/command/agent/template/template.go b/command/agent/template/template.go index cf21944ab..9ff22fbd9 100644 --- a/command/agent/template/template.go +++ b/command/agent/template/template.go @@ -311,6 +311,19 @@ func newRunnerConfig(sc *ServerConfig, templates ctconfig.TemplateConfigs) (*ctc Enabled: &enabled, } + // Sync Consul Template's retry with user set auto-auth initial backoff value. + // This is helpful if Auto Auth cannot get a new token and CT is trying to fetch + // secrets. + if sc.AgentConfig.AutoAuth != nil && sc.AgentConfig.AutoAuth.Method != nil { + if sc.AgentConfig.AutoAuth.Method.MinBackoff > 0 { + conf.Vault.Retry.Backoff = &sc.AgentConfig.AutoAuth.Method.MinBackoff + } + + if sc.AgentConfig.AutoAuth.Method.MaxBackoff > 0 { + conf.Vault.Retry.MaxBackoff = &sc.AgentConfig.AutoAuth.Method.MaxBackoff + } + } + conf.Finalize() // setup log level from TemplateServer config diff --git a/website/content/docs/agent/autoauth/index.mdx b/website/content/docs/agent/autoauth/index.mdx index 5a87359c8..922c7427e 100644 --- a/website/content/docs/agent/autoauth/index.mdx +++ b/website/content/docs/agent/autoauth/index.mdx @@ -138,9 +138,17 @@ These are common configuration values that live within the `method` block: structure. Values can be an integer number of seconds or a stringish value like `5m`. +- `min_backoff` `(string or integer: "1s")` - The minimum backoff time Agent + will delay before retrying after a failed auth attempt. The backoff will start + at the configured value and double (with some randomness) after successive + failures, capped by `max_backoff.` If Agent templating is being used, this + value is also used as the min backoff time for the templating server. + - `max_backoff` `(string or integer: "5m")` - The maximum time Agent will delay - before retrying after a failed auth attempt. The backoff will start at 1 second + before retrying after a failed auth attempt. The backoff will start at `min_backoff` and double (with some randomness) after successive failures, capped by `max_backoff.` + If Agent templating is being used, this value is also used as the max backoff time + for the templating server. - `config` `(object: required)` - Configuration of the method itself. See the sidebar for information about each method.