Skip to content

Commit 1571301

Browse files
committed
Update server config slice merge strategy
Merge slices while checking for equal values rather than always appending. Remove setting Import to prevent migrations from setting incorrect configuration Imports. Signed-off-by: Derek McGowan <[email protected]>
1 parent cf6f439 commit 1571301

File tree

2 files changed

+44
-9
lines changed

2 files changed

+44
-9
lines changed

cmd/containerd/server/config/config.go

Lines changed: 41 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030
"io"
3131
"os"
3232
"path/filepath"
33+
"reflect"
3334
"strings"
3435

3536
"dario.cat/mergo"
@@ -310,12 +311,6 @@ func LoadConfig(ctx context.Context, path string, out *Config) error {
310311
pending = append(pending, imports...)
311312
}
312313

313-
// Fix up the list of config files loaded
314-
out.Imports = []string{}
315-
for path := range loaded {
316-
out.Imports = append(out.Imports, path)
317-
}
318-
319314
err := out.ValidateVersion()
320315
if err != nil {
321316
return fmt.Errorf("failed to load TOML from %s: %w", path, err)
@@ -408,9 +403,11 @@ func resolveImports(parent string, imports []string) ([]string, error) {
408403
// 0 1 1
409404
// []{"1"} []{"2"} []{"1","2"}
410405
// []{"1"} []{} []{"1"}
406+
// []{"1", "2"} []{"1"} []{"1","2"}
407+
// []{} []{"2"} []{"2"}
411408
// Maps merged by keys, but values are replaced entirely.
412409
func mergeConfig(to, from *Config) error {
413-
err := mergo.Merge(to, from, mergo.WithOverride, mergo.WithAppendSlice)
410+
err := mergo.Merge(to, from, mergo.WithOverride, mergo.WithTransformers(sliceTransformer{}))
414411
if err != nil {
415412
return err
416413
}
@@ -435,6 +432,43 @@ func mergeConfig(to, from *Config) error {
435432
return nil
436433
}
437434

435+
type sliceTransformer struct{}
436+
437+
func (sliceTransformer) Transformer(t reflect.Type) func(dst, src reflect.Value) error {
438+
if t.Kind() != reflect.Slice {
439+
return nil
440+
}
441+
return func(dst, src reflect.Value) error {
442+
if !dst.CanSet() {
443+
return nil
444+
}
445+
if src.Type() != dst.Type() {
446+
return fmt.Errorf("cannot append two slice with different type (%s, %s)", src.Type(), dst.Type())
447+
}
448+
for i := 0; i < src.Len(); i++ {
449+
found := false
450+
for j := 0; j < dst.Len(); j++ {
451+
srcv := src.Index(i)
452+
dstv := dst.Index(j)
453+
if !srcv.CanInterface() || !dstv.CanInterface() {
454+
if srcv.Equal(dstv) {
455+
found = true
456+
break
457+
}
458+
} else if reflect.DeepEqual(srcv.Interface(), dstv.Interface()) {
459+
found = true
460+
break
461+
}
462+
}
463+
if !found {
464+
dst.Set(reflect.Append(dst, src.Index(i)))
465+
}
466+
}
467+
468+
return nil
469+
}
470+
}
471+
438472
// V2DisabledFilter matches based on URI
439473
func V2DisabledFilter(list []string) plugin.DisableFilter {
440474
set := make(map[string]struct{}, len(list))

cmd/containerd/server/config/config_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ func TestMergeConfigs(t *testing.T) {
5050
Version: 2,
5151
Root: "new_root",
5252
RequiredPlugins: []string{"io.containerd.new_plugin1.v1", "io.containerd.new_plugin2.v1"},
53+
DisabledPlugins: []string{"io.containerd.old_plugin.v1"},
5354
OOMScore: 2,
5455
Timeouts: map[string]string{"b": "2"},
5556
StreamProcessors: map[string]StreamProcessor{"1": {Path: "3"}},
@@ -191,8 +192,8 @@ imports = ["data1.toml", "data2.toml"]
191192

192193
sort.Strings(out.Imports)
193194
assert.Equal(t, []string{
194-
filepath.Join(tempDir, "data1.toml"),
195-
filepath.Join(tempDir, "data2.toml"),
195+
"data1.toml",
196+
"data2.toml",
196197
}, out.Imports)
197198
}
198199

0 commit comments

Comments
 (0)