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 <derek@mcg.dev>
This commit is contained in:
		| @@ -30,6 +30,7 @@ import ( | |||||||
| 	"io" | 	"io" | ||||||
| 	"os" | 	"os" | ||||||
| 	"path/filepath" | 	"path/filepath" | ||||||
|  | 	"reflect" | ||||||
| 	"strings" | 	"strings" | ||||||
|  |  | ||||||
| 	"dario.cat/mergo" | 	"dario.cat/mergo" | ||||||
| @@ -310,12 +311,6 @@ func LoadConfig(ctx context.Context, path string, out *Config) error { | |||||||
| 		pending = append(pending, imports...) | 		pending = append(pending, imports...) | ||||||
| 	} | 	} | ||||||
|  |  | ||||||
| 	// Fix up the list of config files loaded |  | ||||||
| 	out.Imports = []string{} |  | ||||||
| 	for path := range loaded { |  | ||||||
| 		out.Imports = append(out.Imports, path) |  | ||||||
| 	} |  | ||||||
|  |  | ||||||
| 	err := out.ValidateVersion() | 	err := out.ValidateVersion() | ||||||
| 	if err != nil { | 	if err != nil { | ||||||
| 		return fmt.Errorf("failed to load TOML from %s: %w", path, err) | 		return fmt.Errorf("failed to load TOML from %s: %w", path, err) | ||||||
| @@ -408,9 +403,11 @@ func resolveImports(parent string, imports []string) ([]string, error) { | |||||||
| // 0            1           1 | // 0            1           1 | ||||||
| // []{"1"}      []{"2"}     []{"1","2"} | // []{"1"}      []{"2"}     []{"1","2"} | ||||||
| // []{"1"}      []{}        []{"1"} | // []{"1"}      []{}        []{"1"} | ||||||
|  | // []{"1", "2"} []{"1"}     []{"1","2"} | ||||||
|  | // []{}         []{"2"}     []{"2"} | ||||||
| // Maps merged by keys, but values are replaced entirely. | // Maps merged by keys, but values are replaced entirely. | ||||||
| func mergeConfig(to, from *Config) error { | func mergeConfig(to, from *Config) error { | ||||||
| 	err := mergo.Merge(to, from, mergo.WithOverride, mergo.WithAppendSlice) | 	err := mergo.Merge(to, from, mergo.WithOverride, mergo.WithTransformers(sliceTransformer{})) | ||||||
| 	if err != nil { | 	if err != nil { | ||||||
| 		return err | 		return err | ||||||
| 	} | 	} | ||||||
| @@ -435,6 +432,43 @@ func mergeConfig(to, from *Config) error { | |||||||
| 	return nil | 	return nil | ||||||
| } | } | ||||||
|  |  | ||||||
|  | type sliceTransformer struct{} | ||||||
|  |  | ||||||
|  | func (sliceTransformer) Transformer(t reflect.Type) func(dst, src reflect.Value) error { | ||||||
|  | 	if t.Kind() != reflect.Slice { | ||||||
|  | 		return nil | ||||||
|  | 	} | ||||||
|  | 	return func(dst, src reflect.Value) error { | ||||||
|  | 		if !dst.CanSet() { | ||||||
|  | 			return nil | ||||||
|  | 		} | ||||||
|  | 		if src.Type() != dst.Type() { | ||||||
|  | 			return fmt.Errorf("cannot append two slice with different type (%s, %s)", src.Type(), dst.Type()) | ||||||
|  | 		} | ||||||
|  | 		for i := 0; i < src.Len(); i++ { | ||||||
|  | 			found := false | ||||||
|  | 			for j := 0; j < dst.Len(); j++ { | ||||||
|  | 				srcv := src.Index(i) | ||||||
|  | 				dstv := dst.Index(j) | ||||||
|  | 				if !srcv.CanInterface() || !dstv.CanInterface() { | ||||||
|  | 					if srcv.Equal(dstv) { | ||||||
|  | 						found = true | ||||||
|  | 						break | ||||||
|  | 					} | ||||||
|  | 				} else if reflect.DeepEqual(srcv.Interface(), dstv.Interface()) { | ||||||
|  | 					found = true | ||||||
|  | 					break | ||||||
|  | 				} | ||||||
|  | 			} | ||||||
|  | 			if !found { | ||||||
|  | 				dst.Set(reflect.Append(dst, src.Index(i))) | ||||||
|  | 			} | ||||||
|  | 		} | ||||||
|  |  | ||||||
|  | 		return nil | ||||||
|  | 	} | ||||||
|  | } | ||||||
|  |  | ||||||
| // V2DisabledFilter matches based on URI | // V2DisabledFilter matches based on URI | ||||||
| func V2DisabledFilter(list []string) plugin.DisableFilter { | func V2DisabledFilter(list []string) plugin.DisableFilter { | ||||||
| 	set := make(map[string]struct{}, len(list)) | 	set := make(map[string]struct{}, len(list)) | ||||||
|   | |||||||
| @@ -50,6 +50,7 @@ func TestMergeConfigs(t *testing.T) { | |||||||
| 		Version:          2, | 		Version:          2, | ||||||
| 		Root:             "new_root", | 		Root:             "new_root", | ||||||
| 		RequiredPlugins:  []string{"io.containerd.new_plugin1.v1", "io.containerd.new_plugin2.v1"}, | 		RequiredPlugins:  []string{"io.containerd.new_plugin1.v1", "io.containerd.new_plugin2.v1"}, | ||||||
|  | 		DisabledPlugins:  []string{"io.containerd.old_plugin.v1"}, | ||||||
| 		OOMScore:         2, | 		OOMScore:         2, | ||||||
| 		Timeouts:         map[string]string{"b": "2"}, | 		Timeouts:         map[string]string{"b": "2"}, | ||||||
| 		StreamProcessors: map[string]StreamProcessor{"1": {Path: "3"}}, | 		StreamProcessors: map[string]StreamProcessor{"1": {Path: "3"}}, | ||||||
| @@ -191,8 +192,8 @@ imports = ["data1.toml", "data2.toml"] | |||||||
|  |  | ||||||
| 	sort.Strings(out.Imports) | 	sort.Strings(out.Imports) | ||||||
| 	assert.Equal(t, []string{ | 	assert.Equal(t, []string{ | ||||||
| 		filepath.Join(tempDir, "data1.toml"), | 		"data1.toml", | ||||||
| 		filepath.Join(tempDir, "data2.toml"), | 		"data2.toml", | ||||||
| 	}, out.Imports) | 	}, out.Imports) | ||||||
| } | } | ||||||
|  |  | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user
	 Derek McGowan
					Derek McGowan