From bf93cbd0afa7f52154cb0877946d6bde06145628 Mon Sep 17 00:00:00 2001 From: DerekStrickland Date: Tue, 1 Mar 2022 13:56:03 -0500 Subject: [PATCH 1/3] consul-template: replace config rather than append --- .changelog/12071.txt | 4 + client/config/config.go | 72 ------------ command/agent/config.go | 7 +- command/agent/config_test.go | 104 ++++++++++++------ .../client_with_empty_template.hcl | 6 + .../client_with_function_denylist.hcl | 7 ++ .../client_with_function_denylist_empty.hcl | 7 ++ ...nt_with_function_denylist_empty_string.hcl | 8 ++ ...t_with_function_denylist_empty_string.json | 11 ++ .../client_with_function_denylist_nil.hcl | 7 ++ 10 files changed, 122 insertions(+), 111 deletions(-) create mode 100644 .changelog/12071.txt create mode 100644 command/agent/test-resources/client_with_empty_template.hcl create mode 100644 command/agent/test-resources/client_with_function_denylist.hcl create mode 100644 command/agent/test-resources/client_with_function_denylist_empty.hcl create mode 100644 command/agent/test-resources/client_with_function_denylist_empty_string.hcl create mode 100644 command/agent/test-resources/client_with_function_denylist_empty_string.json create mode 100644 command/agent/test-resources/client_with_function_denylist_nil.hcl diff --git a/.changelog/12071.txt b/.changelog/12071.txt new file mode 100644 index 000000000000..93ff8e05ad2b --- /dev/null +++ b/.changelog/12071.txt @@ -0,0 +1,4 @@ +```release-note:bug +template: Fixes a bug in config parsing that resulted in the default `function_denylist` +always being appended to whatever the user specified. +``` diff --git a/client/config/config.go b/client/config/config.go index c69ddfa1dd1c..c1d59ea6b9e2 100644 --- a/client/config/config.go +++ b/client/config/config.go @@ -396,78 +396,6 @@ func (c *ClientTemplateConfig) Copy() *ClientTemplateConfig { return nc } -// Merge merges the values of two ClientTemplateConfigs. If first copies the receiver -// instance, and then overrides those values with the instance to merge with. -func (c *ClientTemplateConfig) Merge(b *ClientTemplateConfig) *ClientTemplateConfig { - if c == nil { - return b - } - - result := *c - - if b == nil { - return &result - } - - if b.BlockQueryWaitTime != nil { - result.BlockQueryWaitTime = b.BlockQueryWaitTime - } - if b.BlockQueryWaitTimeHCL != "" { - result.BlockQueryWaitTimeHCL = b.BlockQueryWaitTimeHCL - } - - if b.ConsulRetry != nil { - result.ConsulRetry = result.ConsulRetry.Merge(b.ConsulRetry) - } - - result.DisableSandbox = b.DisableSandbox - - // Maintain backward compatibility for older clients - if len(b.FunctionBlacklist) > 0 { - for _, fn := range b.FunctionBlacklist { - if !helper.SliceStringContains(result.FunctionBlacklist, fn) { - result.FunctionBlacklist = append(result.FunctionBlacklist, fn) - } - } - } else if b.FunctionBlacklist != nil { - // No funcs denied - result.FunctionBlacklist = []string{} - } - - if len(b.FunctionDenylist) > 0 { - for _, fn := range b.FunctionDenylist { - if !helper.SliceStringContains(result.FunctionDenylist, fn) { - result.FunctionDenylist = append(result.FunctionDenylist, fn) - } - } - } else if b.FunctionDenylist != nil { - // No funcs denied - result.FunctionDenylist = []string{} - } - - if b.MaxStale != nil { - result.MaxStale = b.MaxStale - } - - if b.MaxStaleHCL != "" { - result.MaxStaleHCL = b.MaxStaleHCL - } - - if b.Wait != nil { - result.Wait = result.Wait.Merge(b.Wait) - } - - if b.WaitBounds != nil { - result.WaitBounds = result.WaitBounds.Merge(b.WaitBounds) - } - - if b.VaultRetry != nil { - result.VaultRetry = result.VaultRetry.Merge(b.VaultRetry) - } - - return &result -} - func (c *ClientTemplateConfig) IsEmpty() bool { if c == nil { return true diff --git a/command/agent/config.go b/command/agent/config.go index e1d3aeaa9dde..d233c1b76b83 100644 --- a/command/agent/config.go +++ b/command/agent/config.go @@ -1705,11 +1705,8 @@ func (a *ClientConfig) Merge(b *ClientConfig) *ClientConfig { result.DisableRemoteExec = b.DisableRemoteExec } - if result.TemplateConfig == nil && b.TemplateConfig != nil { - templateConfig := *b.TemplateConfig - result.TemplateConfig = &templateConfig - } else if b.TemplateConfig != nil { - result.TemplateConfig = result.TemplateConfig.Merge(b.TemplateConfig) + if b.TemplateConfig != nil { + result.TemplateConfig = b.TemplateConfig } // Add the servers diff --git a/command/agent/config_test.go b/command/agent/config_test.go index f66146a6b535..a0ffb0284dc4 100644 --- a/command/agent/config_test.go +++ b/command/agent/config_test.go @@ -1454,43 +1454,79 @@ func TestConfig_LoadConsulTemplateConfig(t *testing.T) { require.Equal(t, 20*time.Second, *templateConfig.VaultRetry.MaxBackoff) } -func TestConfig_LoadConsulTemplateBasic(t *testing.T) { - ci.Parallel(t) - - defaultConfig := DefaultConfig() - - // hcl - agentConfig, err := LoadConfig("test-resources/client_with_basic_template.hcl") - require.NoError(t, err) - require.NotNil(t, agentConfig.Client.TemplateConfig) - - agentConfig = defaultConfig.Merge(agentConfig) - require.Len(t, agentConfig.Client.TemplateConfig.FunctionDenylist, 0) - require.NotNil(t, agentConfig.Client.TemplateConfig.FunctionDenylist) - - clientAgent := Agent{config: agentConfig} - clientConfig, err := clientAgent.clientConfig() - require.NoError(t, err) - - templateConfig := clientConfig.TemplateConfig - require.NotNil(t, templateConfig) - require.True(t, templateConfig.DisableSandbox) - require.Len(t, templateConfig.FunctionDenylist, 0) - - // json - agentConfig, err = LoadConfig("test-resources/client_with_basic_template.json") - require.NoError(t, err) +func TestConfig_LoadConsulTemplate_FunctionDenylist(t *testing.T) { + cases := []struct { + File string + Expected *client.ClientTemplateConfig + }{ + { + "test-resources/minimal_client.hcl", + nil, + }, + { + "test-resources/client_with_basic_template.json", + &client.ClientTemplateConfig{ + DisableSandbox: true, + FunctionDenylist: []string{}, + }, + }, + { + "test-resources/client_with_basic_template.hcl", + &client.ClientTemplateConfig{ + DisableSandbox: true, + FunctionDenylist: []string{}, + }, + }, + { + "test-resources/client_with_function_denylist.hcl", + &client.ClientTemplateConfig{ + DisableSandbox: false, + FunctionDenylist: []string{"foo"}, + }, + }, + { + "test-resources/client_with_function_denylist_empty.hcl", + &client.ClientTemplateConfig{ + DisableSandbox: false, + FunctionDenylist: []string{}, + }, + }, + { + "test-resources/client_with_function_denylist_empty_string.hcl", + &client.ClientTemplateConfig{ + DisableSandbox: true, + FunctionDenylist: []string{""}, + }, + }, + { + "test-resources/client_with_function_denylist_empty_string.json", + &client.ClientTemplateConfig{ + DisableSandbox: true, + FunctionDenylist: []string{""}, + }, + }, + { + "test-resources/client_with_function_denylist_nil.hcl", + &client.ClientTemplateConfig{ + DisableSandbox: true, + }, + }, + { + "test-resources/client_with_empty_template.hcl", + nil, + }, + } - agentConfig = defaultConfig.Merge(agentConfig) + for _, tc := range cases { + t.Run(tc.File, func(t *testing.T) { + agentConfig, err := LoadConfig(tc.File) - clientAgent = Agent{config: agentConfig} - clientConfig, err = clientAgent.clientConfig() - require.NoError(t, err) + require.NoError(t, err) - templateConfig = clientConfig.TemplateConfig - require.NotNil(t, templateConfig) - require.True(t, templateConfig.DisableSandbox) - require.Len(t, templateConfig.FunctionDenylist, 0) + templateConfig := agentConfig.Client.TemplateConfig + require.Equal(t, tc.Expected, templateConfig) + }) + } } func TestParseMultipleIPTemplates(t *testing.T) { diff --git a/command/agent/test-resources/client_with_empty_template.hcl b/command/agent/test-resources/client_with_empty_template.hcl new file mode 100644 index 000000000000..7d0eeec11297 --- /dev/null +++ b/command/agent/test-resources/client_with_empty_template.hcl @@ -0,0 +1,6 @@ +client { + enabled = true + + template { + } +} diff --git a/command/agent/test-resources/client_with_function_denylist.hcl b/command/agent/test-resources/client_with_function_denylist.hcl new file mode 100644 index 000000000000..f1f60f4ed493 --- /dev/null +++ b/command/agent/test-resources/client_with_function_denylist.hcl @@ -0,0 +1,7 @@ +client { + enabled = true + + template { + function_denylist = ["foo"] + } +} diff --git a/command/agent/test-resources/client_with_function_denylist_empty.hcl b/command/agent/test-resources/client_with_function_denylist_empty.hcl new file mode 100644 index 000000000000..17ea0f42b08a --- /dev/null +++ b/command/agent/test-resources/client_with_function_denylist_empty.hcl @@ -0,0 +1,7 @@ +client { + enabled = true + + template { + function_denylist = [] + } +} diff --git a/command/agent/test-resources/client_with_function_denylist_empty_string.hcl b/command/agent/test-resources/client_with_function_denylist_empty_string.hcl new file mode 100644 index 000000000000..91f3b3910d5f --- /dev/null +++ b/command/agent/test-resources/client_with_function_denylist_empty_string.hcl @@ -0,0 +1,8 @@ +client { + enabled = true + + template { + disable_file_sandbox = true + function_denylist = [""] + } +} diff --git a/command/agent/test-resources/client_with_function_denylist_empty_string.json b/command/agent/test-resources/client_with_function_denylist_empty_string.json new file mode 100644 index 000000000000..cbc0ca71cf69 --- /dev/null +++ b/command/agent/test-resources/client_with_function_denylist_empty_string.json @@ -0,0 +1,11 @@ +{ + "client": { + "enabled": true, + "template": { + "disable_file_sandbox": true, + "function_denylist": [ + "" + ] + } + } +} diff --git a/command/agent/test-resources/client_with_function_denylist_nil.hcl b/command/agent/test-resources/client_with_function_denylist_nil.hcl new file mode 100644 index 000000000000..15f090bb7a55 --- /dev/null +++ b/command/agent/test-resources/client_with_function_denylist_nil.hcl @@ -0,0 +1,7 @@ +client { + enabled = true + + template { + disable_file_sandbox = true + } +} From 5abbf9c6f79976a157b81f40aa1cf42e10c526a0 Mon Sep 17 00:00:00 2001 From: Derek Strickland <1111455+DerekStrickland@users.noreply.github.com> Date: Sat, 12 Mar 2022 13:03:27 -0500 Subject: [PATCH 2/3] Update changelog verbiage --- .changelog/12071.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changelog/12071.txt b/.changelog/12071.txt index 93ff8e05ad2b..8a97d10b22fd 100644 --- a/.changelog/12071.txt +++ b/.changelog/12071.txt @@ -1,4 +1,4 @@ ```release-note:bug -template: Fixes a bug in config parsing that resulted in the default `function_denylist` +template: Fixes a bug in config loading that resulted in the default `function_denylist` always being appended to whatever the user specified. ``` From f8518ffb2e91cf2017462e651b51f7729c71d52b Mon Sep 17 00:00:00 2001 From: Derek Strickland <1111455+DerekStrickland@users.noreply.github.com> Date: Fri, 15 Apr 2022 10:48:27 -0400 Subject: [PATCH 3/3] Update .changelog/12071.txt Co-authored-by: Seth Hoenig --- .changelog/12071.txt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.changelog/12071.txt b/.changelog/12071.txt index 8a97d10b22fd..75b072948153 100644 --- a/.changelog/12071.txt +++ b/.changelog/12071.txt @@ -1,4 +1,3 @@ ```release-note:bug -template: Fixes a bug in config loading that resulted in the default `function_denylist` -always being appended to whatever the user specified. +template: Fixed a bug where the default `function_denylist` would be appended to a specified list ```