POC: appendable string arrays in containers.conf
As discussed in https://github.com/containers/podman/issues/20000, we need an opt-in mechanism to _append_ string arrays during loading sequence of containers.conf files. At the moment, existing fields/data will be overriden with each loaded config that sets the specified field/option. The TOML (toml.io) config format does not allow for attributing fields and structs are implicitly represented as "tables". I wanted to extend a string array with a simple boolean field, for instance: ```TOML env=["FOO=bar"] env.append=true ``` TOML doesn't suppor tthe upper idea as it's not a properly formatted table. So I looked for alternatives and found that TOML supports so-called "mixed-type arrays". As the same suggests, such arrays allow for including more than one type and that seemed like a reasonable candidate as it allows for _extending_ the existing syntax without introducing new fields or even yet-another way of loading conf files. The new format can be seen in the tests. Please note that this is just a _tested_ POC. Integrating the POC in containers.conf may turn into a bigger journey as Podman is directly (ab)using many of the fields. Since they have to be changed to the new type (see POC), Podman will not compile without changes. Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
This commit is contained in:
		
							parent
							
								
									abe07fd625
								
							
						
					
					
						commit
						395ba05c44
					
				|  | @ -0,0 +1,73 @@ | |||
| package attributedstringslice | ||||
| 
 | ||||
| import ( | ||||
| 	"bytes" | ||||
| 	"fmt" | ||||
| 
 | ||||
| 	"github.com/BurntSushi/toml" | ||||
| ) | ||||
| 
 | ||||
| type attributedStringSlice struct { // A "mixed-type array" in TOML.
 | ||||
| 	slice      []string | ||||
| 	attributes struct { // Using a struct allows for adding more attributes in the feature.
 | ||||
| 		append *bool // Nil if not set by the user
 | ||||
| 	} | ||||
| } | ||||
| 
 | ||||
| func (ts *attributedStringSlice) UnmarshalTOML(data interface{}) error { | ||||
| 	iFaceSlice, ok := data.([]interface{}) | ||||
| 	if !ok { | ||||
| 		return fmt.Errorf("unable to cast to interface array: %v", data) | ||||
| 	} | ||||
| 
 | ||||
| 	var loadedStrings []string | ||||
| 	for _, x := range iFaceSlice { // Iterate over each item in the slice.
 | ||||
| 		switch val := x.(type) { | ||||
| 		case string: // Strings are directly appended to the slice.
 | ||||
| 			loadedStrings = append(loadedStrings, val) | ||||
| 		case map[string]interface{}: // The attribute struct is represented as a map.
 | ||||
| 			for k, v := range val { // Iterate over all _supported_ keys.
 | ||||
| 				switch k { | ||||
| 				case "append": | ||||
| 					boolVal, ok := v.(bool) | ||||
| 					if !ok { | ||||
| 						return fmt.Errorf("unable to cast append to bool: %v", k) | ||||
| 					} | ||||
| 					ts.attributes.append = &boolVal | ||||
| 				default: // Unsupported map key.
 | ||||
| 					return fmt.Errorf("unsupported key %q in map: %v", k, val) | ||||
| 				} | ||||
| 			} | ||||
| 		default: // Unsupported item.
 | ||||
| 			return fmt.Errorf("unsupported item in attributed string slice: %v", x) | ||||
| 		} | ||||
| 	} | ||||
| 
 | ||||
| 	if ts.attributes.append != nil && *ts.attributes.append { // If _explicitly_ configured, append the loaded slice.
 | ||||
| 		ts.slice = append(ts.slice, loadedStrings...) | ||||
| 	} else { // Default: override the existing slice.
 | ||||
| 		ts.slice = loadedStrings | ||||
| 	} | ||||
| 	return nil | ||||
| } | ||||
| 
 | ||||
| func (ts *attributedStringSlice) MarshalTOML() ([]byte, error) { | ||||
| 	iFaceSlice := make([]interface{}, 0, len(ts.slice)) | ||||
| 
 | ||||
| 	for _, x := range ts.slice { | ||||
| 		iFaceSlice = append(iFaceSlice, x) | ||||
| 	} | ||||
| 
 | ||||
| 	if ts.attributes.append != nil { | ||||
| 		attributes := make(map[string]any) | ||||
| 		attributes["append"] = *ts.attributes.append | ||||
| 		iFaceSlice = append(iFaceSlice, attributes) | ||||
| 	} | ||||
| 
 | ||||
| 	buf := new(bytes.Buffer) | ||||
| 	enc := toml.NewEncoder(buf) | ||||
| 	if err := enc.Encode(iFaceSlice); err != nil { | ||||
| 		return nil, err | ||||
| 	} | ||||
| 	return buf.Bytes(), nil | ||||
| } | ||||
|  | @ -0,0 +1,127 @@ | |||
| package attributedstringslice | ||||
| 
 | ||||
| import ( | ||||
| 	"bytes" | ||||
| 	"testing" | ||||
| 
 | ||||
| 	"github.com/BurntSushi/toml" | ||||
| 	"github.com/stretchr/testify/require" | ||||
| ) | ||||
| 
 | ||||
| type testConfig struct { | ||||
| 	Array attributedStringSlice `toml:"array,omitempty"` | ||||
| } | ||||
| 
 | ||||
| const ( | ||||
| 	confingDefault    = `array=["1", "2", "3"]` | ||||
| 	configAppendFront = `array=[{append=true},"4", "5", "6"]` | ||||
| 	configAppendMid   = `array=["7", {append=true}, "8"]` | ||||
| 	configAppendBack  = `array=["9", {append=true}]` | ||||
| 	configAppendFalse = `array=["10", {append=false}]` | ||||
| ) | ||||
| 
 | ||||
| var ( | ||||
| 	bTrue  = true | ||||
| 	bFalse = false | ||||
| ) | ||||
| 
 | ||||
| func loadConfigs(configs []string) (*testConfig, error) { | ||||
| 	var config testConfig | ||||
| 	for _, c := range configs { | ||||
| 		if _, err := toml.Decode(c, &config); err != nil { | ||||
| 			return nil, err | ||||
| 		} | ||||
| 	} | ||||
| 	return &config, nil | ||||
| } | ||||
| 
 | ||||
| func TestAttributedStringSliceLoading(t *testing.T) { | ||||
| 	for _, test := range []struct { | ||||
| 		configs                []string | ||||
| 		expectedSlice          []string | ||||
| 		expectedAppend         *bool | ||||
| 		expectedErrorSubstring string | ||||
| 	}{ | ||||
| 		// Load single configs
 | ||||
| 		{[]string{confingDefault}, []string{"1", "2", "3"}, nil, ""}, | ||||
| 		{[]string{configAppendFront}, []string{"4", "5", "6"}, &bTrue, ""}, | ||||
| 		{[]string{configAppendMid}, []string{"7", "8"}, &bTrue, ""}, | ||||
| 		{[]string{configAppendBack}, []string{"9"}, &bTrue, ""}, | ||||
| 		{[]string{configAppendFalse}, []string{"10"}, &bFalse, ""}, | ||||
| 		// Append=true
 | ||||
| 		{[]string{confingDefault, configAppendFront}, []string{"1", "2", "3", "4", "5", "6"}, &bTrue, ""}, | ||||
| 		{[]string{configAppendFront, confingDefault}, []string{"4", "5", "6", "1", "2", "3"}, &bTrue, ""}, // The attribute is sticky unless explicitly being turned off in a later config
 | ||||
| 		{[]string{configAppendFront, confingDefault, configAppendBack}, []string{"4", "5", "6", "1", "2", "3", "9"}, &bTrue, ""}, | ||||
| 		// Append=false
 | ||||
| 		{[]string{confingDefault, configAppendFalse}, []string{"10"}, &bFalse, ""}, | ||||
| 		{[]string{confingDefault, configAppendMid, configAppendFalse}, []string{"10"}, &bFalse, ""}, | ||||
| 		{[]string{confingDefault, configAppendFalse, configAppendMid}, []string{"10", "7", "8"}, &bTrue, ""}, // Append can be re-enabled by a later config
 | ||||
| 
 | ||||
| 		// Error checks
 | ||||
| 		{[]string{`array=["1", false]`}, nil, nil, `unsupported item in attributed string slice: false`}, | ||||
| 		{[]string{`array=["1", 42]`}, nil, nil, `unsupported item in attributed string slice: 42`}, // Stop a `int` such that it passes on 32bit as well
 | ||||
| 		{[]string{`array=["1", {foo=true}]`}, nil, nil, `unsupported key "foo" in map: `}, | ||||
| 		{[]string{`array=["1", {append="false"}]`}, nil, nil, `unable to cast append to bool: `}, | ||||
| 	} { | ||||
| 		result, err := loadConfigs(test.configs) | ||||
| 		if test.expectedErrorSubstring != "" { | ||||
| 			require.Error(t, err, "test is expected to fail: %v", test) | ||||
| 			require.ErrorContains(t, err, test.expectedErrorSubstring, "error does not match: %v", test) | ||||
| 			continue | ||||
| 		} | ||||
| 		require.NoError(t, err, "test is expected to succeed: %v", test) | ||||
| 		require.NotNil(t, result, "loaded config must not be nil: %v", test) | ||||
| 		require.Equal(t, result.Array.slice, test.expectedSlice, "slices do not match: %v", test) | ||||
| 		require.Equal(t, result.Array.attributes.append, test.expectedAppend, "append field does not match: %v", test) | ||||
| 	} | ||||
| } | ||||
| 
 | ||||
| func TestAttributedStringSliceEncoding(t *testing.T) { | ||||
| 	for _, test := range []struct { | ||||
| 		configs        []string | ||||
| 		marshalledData string | ||||
| 		expectedSlice  []string | ||||
| 		expectedAppend *bool | ||||
| 	}{ | ||||
| 		{ | ||||
| 			[]string{confingDefault}, | ||||
| 			"array = [\"1\", \"2\", \"3\"]\n", | ||||
| 			[]string{"1", "2", "3"}, | ||||
| 			nil, | ||||
| 		}, | ||||
| 		{ | ||||
| 			[]string{configAppendFront}, | ||||
| 			"array = [\"4\", \"5\", \"6\", {append = true}]\n", | ||||
| 			[]string{"4", "5", "6"}, | ||||
| 			&bTrue, | ||||
| 		}, | ||||
| 		{ | ||||
| 			[]string{configAppendFront, configAppendFalse}, | ||||
| 			"array = [\"10\", {append = false}]\n", | ||||
| 			[]string{"10"}, | ||||
| 			&bFalse, | ||||
| 		}, | ||||
| 	} { | ||||
| 		// 1) Load the configs
 | ||||
| 		result, err := loadConfigs(test.configs) | ||||
| 		require.NoError(t, err, "loading config must succeed") | ||||
| 		require.NotNil(t, result, "loaded config must not be nil") | ||||
| 		require.Equal(t, result.Array.slice, test.expectedSlice, "slices do not match: %v", test) | ||||
| 		require.Equal(t, result.Array.attributes.append, test.expectedAppend, "append field does not match: %v", test) | ||||
| 
 | ||||
| 		// 2) Marshal the config to emulate writing it to disk
 | ||||
| 		buf := new(bytes.Buffer) | ||||
| 		enc := toml.NewEncoder(buf) | ||||
| 		encErr := enc.Encode(result) | ||||
| 		require.NoError(t, encErr, "encoding config must work") | ||||
| 		require.Equal(t, buf.String(), test.marshalledData) | ||||
| 
 | ||||
| 		// 3) Reload the marshaled config to make sure that data is preserved
 | ||||
| 		var reloadedConfig testConfig | ||||
| 		_, decErr := toml.Decode(buf.String(), &reloadedConfig) | ||||
| 		require.NoError(t, decErr, "loading config must succeed") | ||||
| 		require.NotNil(t, reloadedConfig, "re-loaded config must not be nil") | ||||
| 		require.Equal(t, reloadedConfig.Array.slice, test.expectedSlice, "slices do not match: %v", test) | ||||
| 		require.Equal(t, reloadedConfig.Array.attributes.append, test.expectedAppend, "append field does not match: %v", test) | ||||
| 	} | ||||
| } | ||||
		Loading…
	
		Reference in New Issue