From b93e8e817caf1fdc0f021dc6cb9df28cf2d42fd7 Mon Sep 17 00:00:00 2001 From: Jenny Deng Date: Mon, 24 Feb 2025 10:56:03 -0800 Subject: [PATCH] Fix bug with merging vault agent configs that set template_config (#29680) * fix bug with merging configs with TemplateConfig and add test * add changelog --- changelog/29680.txt | 4 ++ command/agent/config/config.go | 13 +++-- command/agent/config/config_test.go | 49 +++++++++++++++++++ .../config-template-min-no-auth.hcl | 7 +++ 4 files changed, 69 insertions(+), 4 deletions(-) create mode 100644 changelog/29680.txt create mode 100644 command/agent/config/test-fixtures/config-template-min-no-auth.hcl diff --git a/changelog/29680.txt b/changelog/29680.txt new file mode 100644 index 0000000000..7180684e79 --- /dev/null +++ b/changelog/29680.txt @@ -0,0 +1,4 @@ +```release-note:bug +agent: Fixed an issue where giving the agent multiple config files could cause the merged config to be incorrect +when `template_config` is set in one of the config files. +``` \ No newline at end of file diff --git a/command/agent/config/config.go b/command/agent/config/config.go index 7b9a942824..aeaba36b51 100644 --- a/command/agent/config/config.go +++ b/command/agent/config/config.go @@ -181,6 +181,9 @@ type ExecConfig struct { func NewConfig() *Config { return &Config{ SharedConfig: new(configutil.SharedConfig), + TemplateConfig: &TemplateConfig{ + MaxConnectionsPerHost: DefaultTemplateConfigMaxConnsPerHost, + }, } } @@ -248,8 +251,13 @@ func (c *Config) Merge(c2 *Config) *Config { result.DisableKeepAlivesTemplating = c2.DisableKeepAlivesTemplating } + // Instead of checking if TemplateConfig is not nil, we compare against the default value + // that is set in NewConfig to determine if a TemplateConfig was specified in the config + defaultConfig := TemplateConfig{ + MaxConnectionsPerHost: DefaultTemplateConfigMaxConnsPerHost, + } result.TemplateConfig = c.TemplateConfig - if c2.TemplateConfig != nil { + if *c2.TemplateConfig != defaultConfig { result.TemplateConfig = c2.TemplateConfig } @@ -1104,9 +1112,6 @@ func parseTemplateConfig(result *Config, list *ast.ObjectList) error { templateConfigList := list.Filter(name) if len(templateConfigList.Items) == 0 { - result.TemplateConfig = &TemplateConfig{ - MaxConnectionsPerHost: DefaultTemplateConfigMaxConnsPerHost, - } return nil } diff --git a/command/agent/config/config_test.go b/command/agent/config/config_test.go index 6c12ebe5de..df34e0db49 100644 --- a/command/agent/config/config_test.go +++ b/command/agent/config/config_test.go @@ -1100,6 +1100,55 @@ func TestLoadConfigFile_TemplateConfig(t *testing.T) { } } +// TestLoadConfigFile_TemplateConfig_MergeConfigs tests that in the case of multiple config files, +// a provided TemplateConfig in one file is properly preserved in the merged config +func TestLoadConfigFile_TemplateConfig_MergeConfigs(t *testing.T) { + configPaths := []string{ + "./test-fixtures/config-template_config.hcl", + "./test-fixtures/config-template-min-no-auth.hcl", + } + + expected := &Config{ + SharedConfig: &configutil.SharedConfig{}, + Vault: &Vault{ + Address: "http://127.0.0.1:1111", + Retry: &Retry{ + NumRetries: 5, + }, + }, + TemplateConfig: &TemplateConfig{ + ExitOnRetryFailure: true, + StaticSecretRenderInt: 1 * time.Minute, + MaxConnectionsPerHost: 100, + LeaseRenewalThreshold: FloatPtr(0.8), + }, + Templates: []*ctconfig.TemplateConfig{ + { + Source: pointerutil.StringPtr("/path/on/disk/to/template.ctmpl"), + Destination: pointerutil.StringPtr("/path/on/disk/where/template/will/render.txt"), + }, + { + Source: pointerutil.StringPtr("/path/on/disk/to/template2.ctmpl"), + Destination: pointerutil.StringPtr("/path/on/disk/where/template/will/render2.txt"), + }, + }, + } + + cfg := NewConfig() + for _, path := range configPaths { + configFromPath, err := LoadConfigFile(path) + if err != nil { + t.Fatal(err) + } + cfg = cfg.Merge(configFromPath) + } + + cfg.Prune() + if diff := deep.Equal(cfg, 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-min-no-auth.hcl b/command/agent/config/test-fixtures/config-template-min-no-auth.hcl new file mode 100644 index 0000000000..ec96c9a9f7 --- /dev/null +++ b/command/agent/config/test-fixtures/config-template-min-no-auth.hcl @@ -0,0 +1,7 @@ +# Copyright (c) HashiCorp, Inc. +# SPDX-License-Identifier: BUSL-1.1 + +template { + source = "/path/on/disk/to/template2.ctmpl" + destination = "/path/on/disk/where/template/will/render2.txt" +}