Skip to content

Commit

Permalink
containers.conf: appendable string arrays, Part 1
Browse files Browse the repository at this point in the history
Signed-off-by: Valentin Rothberg <[email protected]>
  • Loading branch information
vrothberg committed Oct 23, 2023
1 parent f6b9f46 commit 116f410
Show file tree
Hide file tree
Showing 8 changed files with 103 additions and 92 deletions.
73 changes: 0 additions & 73 deletions internal/attributed-string-slice/attributed_string_slice.go

This file was deleted.

84 changes: 84 additions & 0 deletions pkg/config/attributed_string_slice.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
package config

import (
"bytes"
"fmt"

"github.com/BurntSushi/toml"
)

// attributedStringSlice allows for extending a TOML string array with custom
// attributes that control how the array is marshaled into a Go string.
//
// Specifically, an attributedStringSlice can be configured to avoid it being
// overridden by a subsequent unmarshal sequence. When the `append` attribute
// is specified, the array will be appended instead (e.g., `array=["9",
// {append=true}]`).
type attributedStringSlice struct { // A "mixed-type array" in TOML.
// Note that the fields below _must_ be exported. Otherwise the TOML
// encoder would fail during type reflection.
Slice []string
Attributes struct { // Using a struct allows for adding more attributes in the feature.
Append *bool // Nil if not set by the user
}
}

// UnmarshalTOML is the custom unmarshal method for attributedStringSlice.
func (a *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)
}
a.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 a.Attributes.Append != nil && *a.Attributes.Append { // If _explicitly_ configured, append the loaded slice.
a.Slice = append(a.Slice, loadedStrings...)
} else { // Default: override the existing Slice.
a.Slice = loadedStrings
}
return nil
}

// MarshalTOML is the custom marshal method for attributedStringSlice.
func (a *attributedStringSlice) MarshalTOML() ([]byte, error) {
iFaceSlice := make([]interface{}, 0, len(a.Slice))

for _, x := range a.Slice {
iFaceSlice = append(iFaceSlice, x)
}

if a.Attributes.Append != nil {
Attributes := make(map[string]any)
Attributes["append"] = *a.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
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package attributedstringslice
package config

import (
"bytes"
Expand Down Expand Up @@ -71,8 +71,8 @@ func TestAttributedStringSliceLoading(t *testing.T) {
}
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)
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)
}
}

Expand Down Expand Up @@ -106,8 +106,8 @@ func TestAttributedStringSliceEncoding(t *testing.T) {
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)
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)
Expand All @@ -121,7 +121,7 @@ func TestAttributedStringSliceEncoding(t *testing.T) {
_, 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)
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)
}
}
4 changes: 2 additions & 2 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ type ContainersConfig struct {
EnableLabeledUsers bool `toml:"label_users,omitempty"`

// Env is the environment variable list for container process.
Env []string `toml:"env,omitempty"`
Env attributedStringSlice `toml:"env,omitempty"`

// EnvHost Pass all host environment variables into the container.
EnvHost bool `toml:"env_host,omitempty"`
Expand Down Expand Up @@ -907,7 +907,7 @@ func (c *Config) GetDefaultEnvEx(envHost, httpProxy bool) []string {
}
}
}
return append(env, c.Containers.Env...)
return append(env, c.Containers.Env.Slice...)
}

// Capabilities returns the capabilities parses the Add and Drop capability
Expand Down
8 changes: 4 additions & 4 deletions pkg/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ image_copy_tmp_dir="storage"`
// Then
gomega.Expect(err).To(gomega.BeNil())
gomega.Expect(defaultConfig.Engine.CgroupManager).To(gomega.Equal("systemd"))
gomega.Expect(defaultConfig.Containers.Env).To(gomega.BeEquivalentTo(envs))
gomega.Expect(defaultConfig.Containers.Env.Slice).To(gomega.BeEquivalentTo(envs))
gomega.Expect(defaultConfig.Containers.Mounts).To(gomega.BeEquivalentTo(mounts))
gomega.Expect(defaultConfig.Containers.PidsLimit).To(gomega.BeEquivalentTo(2048))
gomega.Expect(defaultConfig.Network.CNIPluginDirs).To(gomega.Equal(pluginDirs))
Expand Down Expand Up @@ -427,7 +427,7 @@ image_copy_tmp_dir="storage"`
gomega.Expect(err).To(gomega.BeNil())
gomega.Expect(config.Containers.ApparmorProfile).To(gomega.Equal(apparmor.Profile))
gomega.Expect(config.Containers.PidsLimit).To(gomega.BeEquivalentTo(2048))
gomega.Expect(config.Containers.Env).To(gomega.BeEquivalentTo(envs))
gomega.Expect(config.Containers.Env.Slice).To(gomega.BeEquivalentTo(envs))
gomega.Expect(config.Containers.UserNS).To(gomega.BeEquivalentTo(""))
gomega.Expect(config.Network.CNIPluginDirs).To(gomega.Equal(DefaultCNIPluginDirs))
gomega.Expect(config.Network.NetavarkPluginDirs).To(gomega.Equal(DefaultNetavarkPluginDirs))
Expand Down Expand Up @@ -866,8 +866,8 @@ env=["foo=bar"]`

expectOldEnv := []string{"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"}
expectNewEnv := []string{"foo=bar"}
gomega.Expect(cfg.Containers.Env).To(gomega.Equal(expectOldEnv))
gomega.Expect(newCfg.Containers.Env).To(gomega.Equal(expectNewEnv))
gomega.Expect(cfg.Containers.Env.Slice).To(gomega.Equal(expectOldEnv))
gomega.Expect(newCfg.Containers.Env.Slice).To(gomega.Equal(expectNewEnv))
// Reload change back to default global configuration
_, err = Reload()
gomega.Expect(err).To(gomega.BeNil())
Expand Down
6 changes: 3 additions & 3 deletions pkg/config/default.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,8 @@ func defaultConfig() (*Config, error) {
Devices: []string{},
EnableKeyring: true,
EnableLabeling: selinuxEnabled(),
Env: []string{
"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
Env: attributedStringSlice{
Slice: []string{"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"},
},
EnvHost: false,
HTTPProxy: true,
Expand Down Expand Up @@ -539,7 +539,7 @@ func (c *Config) DNSOptions() []string {

// Env returns the default additional environment variables to add to containers.
func (c *Config) Env() []string {
return c.Containers.Env
return c.Containers.Env.Slice
}

// IPCNS returns the default IPC Namespace configuration to run containers with.
Expand Down
4 changes: 2 additions & 2 deletions pkg/config/modules_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ var _ = Describe("Config Modules", func() {
gomega.Expect(err).To(gomega.BeNil())
gomega.Expect(options.additionalConfigs).To(gomega.HaveLen(3)) // 3 modules are getting loaded!
gomega.Expect(c.Containers.InitPath).To(gomega.Equal("etc four"))
gomega.Expect(c.Containers.Env).To(gomega.Equal([]string{"usr share only"}))
gomega.Expect(c.Containers.Env.Slice).To(gomega.Equal([]string{"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", "usr share only"}))
gomega.Expect(c.Network.DefaultNetwork).To(gomega.Equal("etc only conf"))
gomega.Expect(c.LoadedModules()).To(gomega.HaveLen(3))

Expand Down Expand Up @@ -184,7 +184,7 @@ var _ = Describe("Config Modules", func() {
gomega.Expect(err).To(gomega.BeNil())
gomega.Expect(options.additionalConfigs).To(gomega.HaveLen(4)) // 2 modules + abs path + override conf are getting loaded!
gomega.Expect(c.Containers.InitPath).To(gomega.Equal("etc four"))
gomega.Expect(c.Containers.Env).To(gomega.Equal([]string{"override conf always wins"}))
gomega.Expect(c.Containers.Env.Slice).To(gomega.Equal([]string{"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", "usr share only", "override conf always wins"}))
gomega.Expect(c.Containers.Volumes).To(gomega.Equal([]string{"home second"}))
})
})
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
[containers]
env=["usr share only"]
env=["usr share only", {append=true}]

0 comments on commit 116f410

Please sign in to comment.