Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Config.Merge fails to merge correctly with arrays that contain a map #150

Closed
blakerouse opened this issue Feb 12, 2020 · 2 comments
Closed
Assignees

Comments

@blakerouse
Copy link
Contributor

Found this debugging elastic/beats#16190. Seems that when two configuration objects are merged that arrays that contain maps are merged incorrectly. Instead of appending the second config array item after the first item from the first config, it merges the maps. This creates an array length of 1 with the maps merged together.

Wrote a quick unit test to show the issue below (test fails):

func TestMergeArrayOfMaps(t *testing.T) {
	c := New()
	err := c.Merge(map[string]interface{}{
		"processors": []interface{}{
			map[string]interface{}{
				"add_locale": map[string]interface{}{},
			},
		},
	})
	assert.NoError(t, err)

	err = c.Merge(map[string]interface{}{
		"processors": []interface{}{
			map[string]interface{}{
				"add_fields": map[string]interface{}{
					"foo": "bar",
				},
			},
		},
	})
	assert.NoError(t, err)

	unpacked := make(map[string]interface{})
	assert.NoError(t, c.Unpack(unpacked))

	processors, _ := unpacked["processors"]
	assert.Len(t, processors, 2)  // array length is 1 currently
}
@urso
Copy link

urso commented Feb 12, 2020

By default go-ucfg tries to treat all settings like the setting names have been flattened. In the example given we have the settings in config 1:

processors.0.add_locale: {}

and in config 2:

processors.0.add_fields.foo: bar

When merging these two configurations we have:

processors.0.add_locale: {}
processors.0.add_fields.foo: bar

Now when we dedot we get:

processors:
- add_locale: {}
  add_fields.foo: bar

By default when merging arrays are not appended, but merged like any other objects. A Config object in go-ucfg acts like a 'table' and arrays are treated like normal objects.

One can modify the behavior "globally" by passing one of these options to Merge: "ReplaceValues", "AppendValues", or "PrependValues". Go structs can overwrite array merging for a field (and it's children) by setting the struct-tag options: "merge", "replace", "append", "prepend".

@blakerouse
Copy link
Contributor Author

blakerouse commented Feb 12, 2020

@urso Thanks for the explanation. Changing the c.Merge to:

err = c.Merge(map[string]interface{}{
	"processors": []interface{}{
		map[string]interface{}{
			"add_fields": map[string]interface{}{
				"foo": "bar",
			},
		},
	},
}, AppendValues)

Fixed it, so not a bug. Just unexpected default behavior on my part.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants