Skip to content

Commit

Permalink
POC: appendable string arrays in containers.conf
Browse files Browse the repository at this point in the history
As discussed in containers/podman#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>
  • Loading branch information
vrothberg committed Oct 3, 2023
1 parent 745eaa4 commit 4f1f29f
Show file tree
Hide file tree
Showing 3 changed files with 201 additions and 0 deletions.
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ test: test-unit
test-unit: netavark-testplugin
go test --tags $(BUILDTAGS) -v ./libimage/...
go test --tags $(BUILDTAGS) -v ./libnetwork/...
go test --tags $(BUILDTAGS) -v ./internal/...
go test --tags $(BUILDTAGS) -v ./pkg/...
go test --tags remote,seccomp,$(BUILDTAGS) -v ./pkg/...

Expand Down
73 changes: 73 additions & 0 deletions internal/attributed-string-slice/attributed_string_slice.go
Original file line number Diff line number Diff line change
@@ -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
}
127 changes: 127 additions & 0 deletions internal/attributed-string-slice/attributed_string_slice_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
}

0 comments on commit 4f1f29f

Please sign in to comment.