diff --git a/.changelog/12071.txt b/.changelog/12071.txt new file mode 100644 index 00000000000..75b07294815 --- /dev/null +++ b/.changelog/12071.txt @@ -0,0 +1,3 @@ +```release-note:bug +template: Fixed a bug where the default `function_denylist` would be appended to a specified list +``` diff --git a/client/config/config.go b/client/config/config.go index c69ddfa1dd1..c1d59ea6b9e 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 e1d3aeaa9dd..d233c1b76b8 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 f66146a6b53..a0ffb0284dc 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 00000000000..7d0eeec1129 --- /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 00000000000..f1f60f4ed49 --- /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 00000000000..17ea0f42b08 --- /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 00000000000..91f3b3910d5 --- /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 00000000000..cbc0ca71cf6 --- /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 00000000000..15f090bb7a5 --- /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 + } +}