From 9fda5753a22698be313b1282ff820a0b38c349c7 Mon Sep 17 00:00:00 2001 From: Jason O'Donnell <2160810+jasonodonnell@users.noreply.github.com> Date: Tue, 21 Jun 2022 13:04:47 -0400 Subject: [PATCH] agent: add disable_idle_connections configurable (#15986) (#16084) * agent: add disable_keep_alives configurable * Add empty test * Add website doc * Change to disable_idle_connections * Update tests and doc * Add note about env * Changelog * Change to slice * Remove unused disable keep alive methods * Add invalid value test --- api/client.go | 18 + changelog/15986.txt | 2 + command/agent.go | 38 ++- command/agent/config/config.go | 40 ++- command/agent/config/config_test.go | 307 ++++++++++++++++++ .../bad-config-disable-idle-connections.hcl | 27 ++ .../config-disable-idle-connections-all.hcl | 27 ++ ...fig-disable-idle-connections-auto-auth.hcl | 27 ++ ...onfig-disable-idle-connections-caching.hcl | 27 ++ .../config-disable-idle-connections-empty.hcl | 27 ++ ...ig-disable-idle-connections-templating.hcl | 27 ++ command/agent/template/template.go | 6 + website/content/docs/agent/index.mdx | 4 + 13 files changed, 565 insertions(+), 12 deletions(-) create mode 100644 changelog/15986.txt create mode 100644 command/agent/config/test-fixtures/bad-config-disable-idle-connections.hcl create mode 100644 command/agent/config/test-fixtures/config-disable-idle-connections-all.hcl create mode 100644 command/agent/config/test-fixtures/config-disable-idle-connections-auto-auth.hcl create mode 100644 command/agent/config/test-fixtures/config-disable-idle-connections-caching.hcl create mode 100644 command/agent/config/test-fixtures/config-disable-idle-connections-empty.hcl create mode 100644 command/agent/config/test-fixtures/config-disable-idle-connections-templating.hcl diff --git a/api/client.go b/api/client.go index b5f7e9bb8265..957ba5d82498 100644 --- a/api/client.go +++ b/api/client.go @@ -720,6 +720,24 @@ func (c *Client) SetMaxRetries(retries int) { c.config.MaxRetries = retries } +func (c *Client) SetMaxIdleConnections(idle int) { + c.modifyLock.RLock() + defer c.modifyLock.RUnlock() + c.config.modifyLock.Lock() + defer c.config.modifyLock.Unlock() + + c.config.HttpClient.Transport.(*http.Transport).MaxIdleConns = idle +} + +func (c *Client) MaxIdleConnections() int { + c.modifyLock.RLock() + defer c.modifyLock.RUnlock() + c.config.modifyLock.Lock() + defer c.config.modifyLock.Unlock() + + return c.config.HttpClient.Transport.(*http.Transport).MaxIdleConns +} + func (c *Client) MaxRetries() int { c.modifyLock.RLock() defer c.modifyLock.RUnlock() diff --git a/changelog/15986.txt b/changelog/15986.txt new file mode 100644 index 000000000000..663ccc8c9b41 --- /dev/null +++ b/changelog/15986.txt @@ -0,0 +1,2 @@ +```release-note:improvement +agent: Added `disable_idle_connections` configuration to disable leaving idle connections open in auto-auth, caching and templating. \ No newline at end of file diff --git a/command/agent.go b/command/agent.go index 883944da756c..16fa9fa38e18 100644 --- a/command/agent.go +++ b/command/agent.go @@ -368,13 +368,24 @@ func (c *AgentCommand) Run(args []string) int { client.SetNamespace(config.AutoAuth.Method.Namespace) } templateNamespace = client.Headers().Get(consts.NamespaceHeaderName) + + sinkClient, err := client.CloneWithHeaders() + if err != nil { + c.UI.Error(fmt.Sprintf("Error cloning client for file sink: %v", err)) + return 1 + } + + if config.DisableIdleConnsAutoAuth { + sinkClient.SetMaxIdleConnections(-1) + } + for _, sc := range config.AutoAuth.Sinks { switch sc.Type { case "file": config := &sink.SinkConfig{ Logger: c.logger.Named("sink.file"), Config: sc.Config, - Client: client, + Client: sinkClient, WrapTTL: sc.WrapTTL, DHType: sc.DHType, DeriveKey: sc.DeriveKey, @@ -490,9 +501,19 @@ func (c *AgentCommand) Run(args []string) int { if config.Cache != nil { cacheLogger := c.logger.Named("cache") + proxyClient, err := client.CloneWithHeaders() + if err != nil { + c.UI.Error(fmt.Sprintf("Error cloning client for caching: %v", err)) + return 1 + } + + if config.DisableIdleConnsAutoAuth { + proxyClient.SetMaxIdleConnections(-1) + } + // Create the API proxier apiProxy, err := cache.NewAPIProxy(&cache.APIProxyConfig{ - Client: client, + Client: proxyClient, Logger: cacheLogger.Named("apiproxy"), EnforceConsistency: enforceConsistency, WhenInconsistentAction: whenInconsistent, @@ -505,7 +526,7 @@ func (c *AgentCommand) Run(args []string) int { // Create the lease cache proxier and set its underlying proxier to // the API proxier. leaseCache, err = cache.NewLeaseCache(&cache.LeaseCacheConfig{ - Client: client, + Client: proxyClient, BaseContext: ctx, Proxier: apiProxy, Logger: cacheLogger.Named("leasecache"), @@ -793,14 +814,19 @@ func (c *AgentCommand) Run(args []string) int { // 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.CloneWithHeaders() + ahClient, err := c.client.CloneWithHeaders() if err != nil { c.UI.Error(fmt.Sprintf("Error cloning client for auth handler: %v", err)) return 1 } + + if config.DisableIdleConnsAutoAuth { + ahClient.SetMaxIdleConnections(-1) + } + ah := auth.NewAuthHandler(&auth.AuthHandlerConfig{ Logger: c.logger.Named("auth.handler"), - Client: clonedClient, + Client: ahClient, WrapTTL: config.AutoAuth.Method.WrapTTL, MinBackoff: config.AutoAuth.Method.MinBackoff, MaxBackoff: config.AutoAuth.Method.MaxBackoff, @@ -811,7 +837,7 @@ func (c *AgentCommand) Run(args []string) int { ss := sink.NewSinkServer(&sink.SinkServerConfig{ Logger: c.logger.Named("sink.server"), - Client: client, + Client: ahClient, ExitAfterAuth: exitAfterAuth, }) diff --git a/command/agent/config/config.go b/command/agent/config/config.go index e68af26f644f..8a28dcf63152 100644 --- a/command/agent/config/config.go +++ b/command/agent/config/config.go @@ -24,14 +24,20 @@ 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"` - TemplateConfig *TemplateConfig `hcl:"template_config"` - 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"` + DisableIdleConns []string `hcl:"disable_idle_connections"` + DisableIdleConnsCaching bool `hcl:"-"` + DisableIdleConnsTemplating bool `hcl:"-"` + DisableIdleConnsAutoAuth bool `hcl:"-"` } +const DisableIdleConnsEnv = "VAULT_AGENT_DISABLE_IDLE_CONNECTIONS" + func (c *Config) Prune() { for _, l := range c.Listeners { l.RawConfig = nil @@ -260,6 +266,28 @@ func LoadConfig(path string) (*Config, error) { result.Vault.Retry.NumRetries = 0 } + if disableIdleConnsEnv := os.Getenv(DisableIdleConnsEnv); disableIdleConnsEnv != "" { + result.DisableIdleConns, err = parseutil.ParseCommaStringSlice(strings.ToLower(disableIdleConnsEnv)) + if err != nil { + return nil, fmt.Errorf("error parsing environment variable %s: %v", DisableIdleConnsEnv, err) + } + } + + for _, subsystem := range result.DisableIdleConns { + switch subsystem { + case "auto-auth": + result.DisableIdleConnsAutoAuth = true + case "caching": + result.DisableIdleConnsCaching = true + case "templating": + result.DisableIdleConnsTemplating = true + case "": + continue + default: + return nil, fmt.Errorf("unknown disable_idle_connections value: %s", subsystem) + } + } + return result, nil } diff --git a/command/agent/config/config_test.go b/command/agent/config/config_test.go index 1a1aec2a14d1..c9728543c3b4 100644 --- a/command/agent/config/config_test.go +++ b/command/agent/config/config_test.go @@ -1033,3 +1033,310 @@ func TestLoadConfigFile_EnforceConsistency(t *testing.T) { t.Fatal(diff) } } + +func TestLoadConfigFile_Disable_Idle_Conns_All(t *testing.T) { + config, err := LoadConfig("./test-fixtures/config-disable-idle-connections-all.hcl") + if err != nil { + t.Fatal(err) + } + + expected := &Config{ + SharedConfig: &configutil.SharedConfig{ + PidFile: "./pidfile", + }, + DisableIdleConns: []string{"auto-auth", "caching", "templating"}, + DisableIdleConnsCaching: true, + DisableIdleConnsAutoAuth: true, + DisableIdleConnsTemplating: true, + AutoAuth: &AutoAuth{ + Method: &Method{ + Type: "aws", + MountPath: "auth/aws", + Namespace: "my-namespace/", + Config: map[string]interface{}{ + "role": "foobar", + }, + }, + Sinks: []*Sink{ + { + Type: "file", + DHType: "curve25519", + DHPath: "/tmp/file-foo-dhpath", + AAD: "foobar", + Config: map[string]interface{}{ + "path": "/tmp/file-foo", + }, + }, + }, + }, + Vault: &Vault{ + Address: "http://127.0.0.1:1111", + Retry: &Retry{ + ctconfig.DefaultRetryAttempts, + }, + }, + } + + config.Prune() + if diff := deep.Equal(config, expected); diff != nil { + t.Fatal(diff) + } +} + +func TestLoadConfigFile_Disable_Idle_Conns_Auto_Auth(t *testing.T) { + config, err := LoadConfig("./test-fixtures/config-disable-idle-connections-auto-auth.hcl") + if err != nil { + t.Fatal(err) + } + + expected := &Config{ + SharedConfig: &configutil.SharedConfig{ + PidFile: "./pidfile", + }, + DisableIdleConns: []string{"auto-auth"}, + DisableIdleConnsCaching: false, + DisableIdleConnsAutoAuth: true, + DisableIdleConnsTemplating: false, + AutoAuth: &AutoAuth{ + Method: &Method{ + Type: "aws", + MountPath: "auth/aws", + Namespace: "my-namespace/", + Config: map[string]interface{}{ + "role": "foobar", + }, + }, + Sinks: []*Sink{ + { + Type: "file", + DHType: "curve25519", + DHPath: "/tmp/file-foo-dhpath", + AAD: "foobar", + Config: map[string]interface{}{ + "path": "/tmp/file-foo", + }, + }, + }, + }, + Vault: &Vault{ + Address: "http://127.0.0.1:1111", + Retry: &Retry{ + ctconfig.DefaultRetryAttempts, + }, + }, + } + + config.Prune() + if diff := deep.Equal(config, expected); diff != nil { + t.Fatal(diff) + } +} + +func TestLoadConfigFile_Disable_Idle_Conns_Templating(t *testing.T) { + config, err := LoadConfig("./test-fixtures/config-disable-idle-connections-templating.hcl") + if err != nil { + t.Fatal(err) + } + + expected := &Config{ + SharedConfig: &configutil.SharedConfig{ + PidFile: "./pidfile", + }, + DisableIdleConns: []string{"templating"}, + DisableIdleConnsCaching: false, + DisableIdleConnsAutoAuth: false, + DisableIdleConnsTemplating: true, + AutoAuth: &AutoAuth{ + Method: &Method{ + Type: "aws", + MountPath: "auth/aws", + Namespace: "my-namespace/", + Config: map[string]interface{}{ + "role": "foobar", + }, + }, + Sinks: []*Sink{ + { + Type: "file", + DHType: "curve25519", + DHPath: "/tmp/file-foo-dhpath", + AAD: "foobar", + Config: map[string]interface{}{ + "path": "/tmp/file-foo", + }, + }, + }, + }, + Vault: &Vault{ + Address: "http://127.0.0.1:1111", + Retry: &Retry{ + ctconfig.DefaultRetryAttempts, + }, + }, + } + + config.Prune() + if diff := deep.Equal(config, expected); diff != nil { + t.Fatal(diff) + } +} + +func TestLoadConfigFile_Disable_Idle_Conns_Caching(t *testing.T) { + config, err := LoadConfig("./test-fixtures/config-disable-idle-connections-caching.hcl") + if err != nil { + t.Fatal(err) + } + + expected := &Config{ + SharedConfig: &configutil.SharedConfig{ + PidFile: "./pidfile", + }, + DisableIdleConns: []string{"caching"}, + DisableIdleConnsCaching: true, + DisableIdleConnsAutoAuth: false, + DisableIdleConnsTemplating: false, + AutoAuth: &AutoAuth{ + Method: &Method{ + Type: "aws", + MountPath: "auth/aws", + Namespace: "my-namespace/", + Config: map[string]interface{}{ + "role": "foobar", + }, + }, + Sinks: []*Sink{ + { + Type: "file", + DHType: "curve25519", + DHPath: "/tmp/file-foo-dhpath", + AAD: "foobar", + Config: map[string]interface{}{ + "path": "/tmp/file-foo", + }, + }, + }, + }, + Vault: &Vault{ + Address: "http://127.0.0.1:1111", + Retry: &Retry{ + ctconfig.DefaultRetryAttempts, + }, + }, + } + + config.Prune() + if diff := deep.Equal(config, expected); diff != nil { + t.Fatal(diff) + } +} + +func TestLoadConfigFile_Disable_Idle_Conns_Empty(t *testing.T) { + config, err := LoadConfig("./test-fixtures/config-disable-idle-connections-empty.hcl") + if err != nil { + t.Fatal(err) + } + + expected := &Config{ + SharedConfig: &configutil.SharedConfig{ + PidFile: "./pidfile", + }, + DisableIdleConns: []string{}, + DisableIdleConnsCaching: false, + DisableIdleConnsAutoAuth: false, + DisableIdleConnsTemplating: false, + AutoAuth: &AutoAuth{ + Method: &Method{ + Type: "aws", + MountPath: "auth/aws", + Namespace: "my-namespace/", + Config: map[string]interface{}{ + "role": "foobar", + }, + }, + Sinks: []*Sink{ + { + Type: "file", + DHType: "curve25519", + DHPath: "/tmp/file-foo-dhpath", + AAD: "foobar", + Config: map[string]interface{}{ + "path": "/tmp/file-foo", + }, + }, + }, + }, + Vault: &Vault{ + Address: "http://127.0.0.1:1111", + Retry: &Retry{ + ctconfig.DefaultRetryAttempts, + }, + }, + } + + config.Prune() + if diff := deep.Equal(config, expected); diff != nil { + t.Fatal(diff) + } +} + +func TestLoadConfigFile_Disable_Idle_Conns_Env(t *testing.T) { + err := os.Setenv(DisableIdleConnsEnv, "auto-auth,caching,templating") + defer os.Unsetenv(DisableIdleConnsEnv) + + if err != nil { + t.Fatal(err) + } + config, err := LoadConfig("./test-fixtures/config-disable-idle-connections-empty.hcl") + if err != nil { + t.Fatal(err) + } + + expected := &Config{ + SharedConfig: &configutil.SharedConfig{ + PidFile: "./pidfile", + }, + DisableIdleConns: []string{"auto-auth", "caching", "templating"}, + DisableIdleConnsCaching: true, + DisableIdleConnsAutoAuth: true, + DisableIdleConnsTemplating: true, + AutoAuth: &AutoAuth{ + Method: &Method{ + Type: "aws", + MountPath: "auth/aws", + Namespace: "my-namespace/", + Config: map[string]interface{}{ + "role": "foobar", + }, + }, + Sinks: []*Sink{ + { + Type: "file", + DHType: "curve25519", + DHPath: "/tmp/file-foo-dhpath", + AAD: "foobar", + Config: map[string]interface{}{ + "path": "/tmp/file-foo", + }, + }, + }, + }, + Vault: &Vault{ + Address: "http://127.0.0.1:1111", + Retry: &Retry{ + ctconfig.DefaultRetryAttempts, + }, + }, + } + + config.Prune() + if diff := deep.Equal(config, expected); diff != nil { + t.Fatal(diff) + } +} + +func TestLoadConfigFile_Bad_Value_Disable_Idle_Conns(t *testing.T) { + _, err := LoadConfig("./test-fixtures/bad-config-disable-idle-connections.hcl") + if err == nil { + t.Fatal("should have error, it didn't") + } +} diff --git a/command/agent/config/test-fixtures/bad-config-disable-idle-connections.hcl b/command/agent/config/test-fixtures/bad-config-disable-idle-connections.hcl new file mode 100644 index 000000000000..c13c82520ee6 --- /dev/null +++ b/command/agent/config/test-fixtures/bad-config-disable-idle-connections.hcl @@ -0,0 +1,27 @@ +pid_file = "./pidfile" +disable_idle_connections = ["foo","caching","templating"] + +auto_auth { + method { + type = "aws" + namespace = "my-namespace/" + + config = { + role = "foobar" + } + } + + sink { + type = "file" + config = { + path = "/tmp/file-foo" + } + aad = "foobar" + dh_type = "curve25519" + dh_path = "/tmp/file-foo-dhpath" + } +} + +vault { + address = "http://127.0.0.1:1111" +} diff --git a/command/agent/config/test-fixtures/config-disable-idle-connections-all.hcl b/command/agent/config/test-fixtures/config-disable-idle-connections-all.hcl new file mode 100644 index 000000000000..69ff548f5561 --- /dev/null +++ b/command/agent/config/test-fixtures/config-disable-idle-connections-all.hcl @@ -0,0 +1,27 @@ +pid_file = "./pidfile" +disable_idle_connections = ["auto-auth","caching","templating"] + +auto_auth { + method { + type = "aws" + namespace = "my-namespace/" + + config = { + role = "foobar" + } + } + + sink { + type = "file" + config = { + path = "/tmp/file-foo" + } + aad = "foobar" + dh_type = "curve25519" + dh_path = "/tmp/file-foo-dhpath" + } +} + +vault { + address = "http://127.0.0.1:1111" +} diff --git a/command/agent/config/test-fixtures/config-disable-idle-connections-auto-auth.hcl b/command/agent/config/test-fixtures/config-disable-idle-connections-auto-auth.hcl new file mode 100644 index 000000000000..1a63b20480d4 --- /dev/null +++ b/command/agent/config/test-fixtures/config-disable-idle-connections-auto-auth.hcl @@ -0,0 +1,27 @@ +pid_file = "./pidfile" +disable_idle_connections = ["auto-auth"] + +auto_auth { + method { + type = "aws" + namespace = "my-namespace/" + + config = { + role = "foobar" + } + } + + sink { + type = "file" + config = { + path = "/tmp/file-foo" + } + aad = "foobar" + dh_type = "curve25519" + dh_path = "/tmp/file-foo-dhpath" + } +} + +vault { + address = "http://127.0.0.1:1111" +} diff --git a/command/agent/config/test-fixtures/config-disable-idle-connections-caching.hcl b/command/agent/config/test-fixtures/config-disable-idle-connections-caching.hcl new file mode 100644 index 000000000000..30d0806c0337 --- /dev/null +++ b/command/agent/config/test-fixtures/config-disable-idle-connections-caching.hcl @@ -0,0 +1,27 @@ +pid_file = "./pidfile" +disable_idle_connections = ["caching"] + +auto_auth { + method { + type = "aws" + namespace = "my-namespace/" + + config = { + role = "foobar" + } + } + + sink { + type = "file" + config = { + path = "/tmp/file-foo" + } + aad = "foobar" + dh_type = "curve25519" + dh_path = "/tmp/file-foo-dhpath" + } +} + +vault { + address = "http://127.0.0.1:1111" +} diff --git a/command/agent/config/test-fixtures/config-disable-idle-connections-empty.hcl b/command/agent/config/test-fixtures/config-disable-idle-connections-empty.hcl new file mode 100644 index 000000000000..eb95310cedff --- /dev/null +++ b/command/agent/config/test-fixtures/config-disable-idle-connections-empty.hcl @@ -0,0 +1,27 @@ +pid_file = "./pidfile" +disable_idle_connections = [] + +auto_auth { + method { + type = "aws" + namespace = "my-namespace/" + + config = { + role = "foobar" + } + } + + sink { + type = "file" + config = { + path = "/tmp/file-foo" + } + aad = "foobar" + dh_type = "curve25519" + dh_path = "/tmp/file-foo-dhpath" + } +} + +vault { + address = "http://127.0.0.1:1111" +} diff --git a/command/agent/config/test-fixtures/config-disable-idle-connections-templating.hcl b/command/agent/config/test-fixtures/config-disable-idle-connections-templating.hcl new file mode 100644 index 000000000000..922377fc82a9 --- /dev/null +++ b/command/agent/config/test-fixtures/config-disable-idle-connections-templating.hcl @@ -0,0 +1,27 @@ +pid_file = "./pidfile" +disable_idle_connections = ["templating"] + +auto_auth { + method { + type = "aws" + namespace = "my-namespace/" + + config = { + role = "foobar" + } + } + + sink { + type = "file" + config = { + path = "/tmp/file-foo" + } + aad = "foobar" + dh_type = "curve25519" + dh_path = "/tmp/file-foo-dhpath" + } +} + +vault { + address = "http://127.0.0.1:1111" +} diff --git a/command/agent/template/template.go b/command/agent/template/template.go index 9ff22fbd9b25..0fa1e9a0d273 100644 --- a/command/agent/template/template.go +++ b/command/agent/template/template.go @@ -107,6 +107,7 @@ func (ts *Server) Run(ctx context.Context, incoming chan string, templates []*ct // configuration var runnerConfig *ctconfig.Config var runnerConfigErr error + if runnerConfig, runnerConfigErr = newRunnerConfig(ts.config, templates); runnerConfigErr != nil { return fmt.Errorf("template server failed to runner generate config: %w", runnerConfigErr) } @@ -244,6 +245,11 @@ func newRunnerConfig(sc *ServerConfig, templates ctconfig.TemplateConfigs) (*ctc conf.Vault.DefaultLeaseDuration = &sc.AgentConfig.TemplateConfig.StaticSecretRenderInt } + if sc.AgentConfig.DisableIdleConnsTemplating { + idleConns := -1 + conf.Vault.Transport.MaxIdleConns = &idleConns + } + conf.Vault.SSL = &ctconfig.SSLConfig{ Enabled: pointerutil.BoolPtr(false), Verify: pointerutil.BoolPtr(false), diff --git a/website/content/docs/agent/index.mdx b/website/content/docs/agent/index.mdx index a7de23df1445..6f7875f2f3db 100644 --- a/website/content/docs/agent/index.mdx +++ b/website/content/docs/agent/index.mdx @@ -144,6 +144,10 @@ These are the currently-available general configuration option: with code `0` after a single successful auth, where success means that a token was retrieved and all sinks successfully wrote it +- `disable_idle_connections` `(string array: [])` - A list of strings that disables idle connections for various features in Vault Agent. + Valid values include: `auto-auth`, `caching` and `templating`. Can also be configured by setting the `VAULT_AGENT_DISABLE_IDLE_CONNECTIONS` + environment variable as a comma separated string. This environment variable will override any values found in a configuration file. + - `template` ([template][template]: ) - Specifies options used for templating Vault secrets to files. - `template_config` ([template_config][template-config]: ) - Specifies templating engine behavior.