ClientTemplateConfig: Replace rather than append FunctionDenylist during merge

Author: DerekStrickland

client/config/config.go

@@ -405,21 +405,12 @@ func (c *ClientTemplateConfig) Merge(b *ClientTemplateConfig) *ClientTemplateCon
 
 	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)
-			}
-		}
+	if b.FunctionBlacklist != nil {
+		result.FunctionBlacklist = b.FunctionBlacklist
 	}
 
-	if len(b.FunctionDenylist) > 0 {
-		for _, fn := range b.FunctionDenylist {
-			if !helper.SliceStringContains(result.FunctionDenylist, fn) {
-				result.FunctionDenylist = append(result.FunctionDenylist, fn)
-			}
-		}
+	if b.FunctionDenylist != nil {
+		result.FunctionDenylist = b.FunctionDenylist
 	}
 
 	if b.MaxStale != nil {
@@ -451,8 +442,8 @@ func (c *ClientTemplateConfig) IsEmpty() bool {
 	}
 
 	return !c.DisableSandbox &&
-		len(c.FunctionDenylist) == 0 &&
-		len(c.FunctionBlacklist) == 0 &&
+		c.FunctionDenylist == nil &&
+		c.FunctionBlacklist == nil &&
 		c.BlockQueryWaitTime == nil &&
 		c.BlockQueryWaitTimeHCL == "" &&
 		c.MaxStale == nil &&

command/agent/config.go

@@ -1697,10 +1697,7 @@ 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 {
+	if b.TemplateConfig != nil {
 		result.TemplateConfig = result.TemplateConfig.Merge(b.TemplateConfig)
 	}
 

command/agent/config_test.go

@@ -1413,39 +1413,72 @@ func TestConfig_LoadConsulTemplateConfig(t *testing.T) {
 	require.Equal(t, 20*time.Second, *templateConfig.VaultRetry.MaxBackoff)
 }
 
-func TestConfig_LoadConsulTemplateBasic(t *testing.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)
-
-	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, 1)
-
-	// 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_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, 1)
+			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_nil.hcl

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