From a21e379b69916558c1771b086a4a17b0a58f7cc1 Mon Sep 17 00:00:00 2001
From: Ray Burgemeestre <rayb@nvidia.com>
Date: Thu, 21 Mar 2024 12:00:24 +0100
Subject: [PATCH] Allow sections of Plugins to be merged, and not overwritten
 as entire sections.

See for rationale the Pull Request description. Added unit test to
demonstrate the difference of this change.

Signed-off-by: Ray Burgemeestre <rayb@nvidia.com>
---
 cmd/containerd/server/config/config.go      |  4 --
 cmd/containerd/server/config/config_test.go | 73 +++++++++++++++++++++
 2 files changed, 73 insertions(+), 4 deletions(-)

diff --git a/cmd/containerd/server/config/config.go b/cmd/containerd/server/config/config.go
index c77f165e4..24687b873 100644
--- a/cmd/containerd/server/config/config.go
+++ b/cmd/containerd/server/config/config.go
@@ -412,10 +412,6 @@ func mergeConfig(to, from *Config) error {
 	}
 
 	// Replace entire sections instead of merging map's values.
-	for k, v := range from.Plugins {
-		to.Plugins[k] = v
-	}
-
 	for k, v := range from.StreamProcessors {
 		to.StreamProcessors[k] = v
 	}
diff --git a/cmd/containerd/server/config/config_test.go b/cmd/containerd/server/config/config_test.go
index 8240aad01..d76d462f0 100644
--- a/cmd/containerd/server/config/config_test.go
+++ b/cmd/containerd/server/config/config_test.go
@@ -17,12 +17,17 @@
 package config
 
 import (
+	"bytes"
 	"context"
+	"fmt"
 	"os"
 	"path/filepath"
 	"sort"
+	"strings"
 	"testing"
 
+	"github.com/pelletier/go-toml/v2"
+
 	"github.com/stretchr/testify/assert"
 
 	"github.com/containerd/containerd/v2/version"
@@ -260,3 +265,71 @@ func TestDecodePluginInV1Config(t *testing.T) {
 	assert.NoError(t, err)
 	assert.Equal(t, true, pluginConfig["shim_debug"])
 }
+
+func TestMergingPluginsWithTwoCriDropInConfigs(t *testing.T) {
+	data1 := `
+[plugins."io.containerd.grpc.v1.cri".cni]
+    bin_dir = "/cm/local/apps/kubernetes/current/bin/cni"
+`
+	data2 := `
+[plugins."io.containerd.grpc.v1.cri".registry]
+    config_path = "/cm/local/apps/containerd/var/etc/certs.d"
+`
+	expected := `
+[cni]
+  bin_dir = '/cm/local/apps/kubernetes/current/bin/cni'
+
+[registry]
+  config_path = '/cm/local/apps/containerd/var/etc/certs.d'
+`
+
+	testMergeConfig(t, []string{data1, data2}, expected, "io.containerd.grpc.v1.cri")
+	testMergeConfig(t, []string{data2, data1}, expected, "io.containerd.grpc.v1.cri")
+}
+
+func TestMergingPluginsWithTwoCriCniDropInConfigs(t *testing.T) {
+	data1 := `
+[plugins."io.containerd.grpc.v1.cri".cni]
+    bin_dir = "/cm/local/apps/kubernetes/current/bin/cni"
+`
+	data2 := `
+[plugins."io.containerd.grpc.v1.cri".cni]
+    conf_dir = "/tmp"
+`
+	expected := `
+[cni]
+  bin_dir = '/cm/local/apps/kubernetes/current/bin/cni'
+  conf_dir = '/tmp'
+`
+	testMergeConfig(t, []string{data1, data2}, expected, "io.containerd.grpc.v1.cri")
+}
+
+func testMergeConfig(t *testing.T, inputs []string, expected string, comparePlugin string) {
+	tempDir := t.TempDir()
+	var result Config
+
+	for i, data := range inputs {
+		filename := fmt.Sprintf("data%d.toml", i+1)
+		filepath := filepath.Join(tempDir, filename)
+		err := os.WriteFile(filepath, []byte(data), 0600)
+		assert.NoError(t, err)
+
+		var tempOut Config
+		err = LoadConfig(context.Background(), filepath, &tempOut)
+		assert.NoError(t, err)
+
+		if i == 0 {
+			result = tempOut
+		} else {
+			err = mergeConfig(&result, &tempOut)
+			assert.NoError(t, err)
+		}
+	}
+
+	criPlugin := result.Plugins[comparePlugin]
+	var buf bytes.Buffer
+	if err := toml.NewEncoder(&buf).SetIndentTables(true).Encode(criPlugin); err != nil {
+		panic(err)
+	}
+	assert.Equal(t, strings.TrimLeft(expected, "\n"), buf.String())
+}