consul-template: revert function_denylist logic (#12071)

* consul-template: replace config rather than append
Co-authored-by: Seth Hoenig <seth.a.hoenig@gmail.com>

.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
+```

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

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

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) {

command/agent/test-resources/client_with_empty_template.hcl

@@ -0,0 +1,6 @@
+client {
+  enabled = true
+
+  template {
+  }
+}

command/agent/test-resources/client_with_function_denylist.hcl

@@ -0,0 +1,7 @@
+client {
+  enabled = true
+
+  template {
+    function_denylist = ["foo"]
+  }
+}

command/agent/test-resources/client_with_function_denylist_empty.hcl

@@ -0,0 +1,7 @@
+client {
+  enabled = true
+
+  template {
+    function_denylist = []
+  }
+}

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    = [""]
+  }
+}

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": [
+        ""
+      ]
+    }
+  }
+}

command/agent/test-resources/client_with_function_denylist_nil.hcl

@@ -0,0 +1,7 @@
+client {
+  enabled = true
+
+  template {
+    disable_file_sandbox = true
+  }
+}