From cce7ffdc4f4b7178146afef7801ed9135292fa02 Mon Sep 17 00:00:00 2001 From: Harshal Patil Date: Thu, 11 Apr 2024 15:13:10 -0400 Subject: [PATCH] Add support for drop-in config Signed-off-by: Harshal Patil --- types/options.go | 100 ++++++++++++++++++++++++++++++++++++++- types/options_darwin.go | 2 - types/options_freebsd.go | 5 -- types/options_linux.go | 5 -- types/options_test.go | 74 +++++++++++++++++++++++++++++ types/options_windows.go | 5 -- types/utils.go | 29 ++++++++++++ types/utils_test.go | 24 ++++++++++ 8 files changed, 225 insertions(+), 19 deletions(-) diff --git a/types/options.go b/types/options.go index 03e5f7ab64..4f7e0e9a02 100644 --- a/types/options.go +++ b/types/options.go @@ -49,6 +49,12 @@ var ( defaultConfigFile = SystemConfigFile // DefaultStoreOptions is a reasonable default set of options. defaultStoreOptions StoreOptions + + // defaultOverrideConfigFile path to override the default system wide storage.conf file + defaultOverrideConfigFile = "/etc/containers/storage.conf" + + // defaultDropInConfigDir path to the folder containing drop in config files + defaultDropInConfigDir = "/etc/containers/storage.conf.d" ) func loadDefaultStoreOptions() { @@ -114,11 +120,101 @@ func loadDefaultStoreOptions() { // loadStoreOptions returns the default storage ops for containers func loadStoreOptions() (StoreOptions, error) { - storageConf, err := DefaultConfigFile() + baseConf, err := DefaultConfigFile() + if err != nil { + return defaultStoreOptions, err + } + + // Load the base config file + baseOptions, err := loadStoreOptionsFromConfFile(baseConf) + if err != nil { + return defaultStoreOptions, err + } + + if _, err := os.Stat(defaultDropInConfigDir); err != nil && os.IsNotExist(err) { + return defaultStoreOptions, err + } + + baseOptions, err = mergeConfigFromDirectory(baseOptions, defaultDropInConfigDir) if err != nil { return defaultStoreOptions, err } - return loadStoreOptionsFromConfFile(storageConf) + + return baseOptions, nil +} + +func mergeConfigFromDirectory(baseOptions StoreOptions, configDir string) (StoreOptions, error) { + err := filepath.Walk(configDir, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + if info.IsDir() { + return nil + } + + // Load drop-in options from the current file + dropInOptions, err := loadStoreOptionsFromConfFile(path) + if err != nil { + return err + } + + // Merge the drop-in options into the base options + baseOptions = mergeStoreOptions(baseOptions, dropInOptions) + return nil + }) + if err != nil { + return baseOptions, err + } + return baseOptions, nil +} + +func mergeStoreOptions(base, dropIn StoreOptions) StoreOptions { + if dropIn.RunRoot != "" { + base.RunRoot = dropIn.RunRoot + } + if dropIn.GraphRoot != "" { + base.GraphRoot = dropIn.GraphRoot + } + if dropIn.ImageStore != "" { + base.ImageStore = dropIn.ImageStore + } + if dropIn.RootlessStoragePath != "" { + base.RootlessStoragePath = dropIn.RootlessStoragePath + } + if dropIn.GraphDriverName != "" { + base.GraphDriverName = dropIn.GraphDriverName + } + if dropIn.RootAutoNsUser != "" { + base.RootAutoNsUser = dropIn.RootAutoNsUser + } + + base.GraphDriverPriority = appendUniqueStrings(base.GraphDriverPriority, dropIn.GraphDriverPriority) + base.GraphDriverOptions = appendUniqueStrings(base.GraphDriverOptions, dropIn.GraphDriverOptions) + base.UIDMap = appendUniqueIDMaps(base.UIDMap, dropIn.UIDMap) + base.GIDMap = appendUniqueIDMaps(base.GIDMap, dropIn.GIDMap) + + // For map fields, simply merge the key-value pairs. + for key, value := range dropIn.PullOptions { + base.PullOptions[key] = value + } + + // For boolean fields, use the drop-in value if it changes the default (assumed false). + if dropIn.DisableVolatile { + base.DisableVolatile = dropIn.DisableVolatile + } + if dropIn.TransientStore { + base.TransientStore = dropIn.TransientStore + } + + // For numeric fields, use non-zero values from drop-in. + if dropIn.AutoNsMinSize != 0 { + base.AutoNsMinSize = dropIn.AutoNsMinSize + } + if dropIn.AutoNsMaxSize != 0 { + base.AutoNsMaxSize = dropIn.AutoNsMaxSize + } + + return base } // usePerUserStorage returns whether the user private storage must be used. diff --git a/types/options_darwin.go b/types/options_darwin.go index 3eecc2b827..6084cbd7d5 100644 --- a/types/options_darwin.go +++ b/types/options_darwin.go @@ -8,8 +8,6 @@ const ( SystemConfigFile = "/usr/share/containers/storage.conf" ) -var defaultOverrideConfigFile = "/etc/containers/storage.conf" - // canUseRootlessOverlay returns true if the overlay driver can be used for rootless containers func canUseRootlessOverlay(home, runhome string) bool { return false diff --git a/types/options_freebsd.go b/types/options_freebsd.go index be2bc2f27d..86595afbe6 100644 --- a/types/options_freebsd.go +++ b/types/options_freebsd.go @@ -8,11 +8,6 @@ const ( SystemConfigFile = "/usr/local/share/containers/storage.conf" ) -// defaultConfigFile path to the system wide storage.conf file -var ( - defaultOverrideConfigFile = "/usr/local/etc/containers/storage.conf" -) - // canUseRootlessOverlay returns true if the overlay driver can be used for rootless containers func canUseRootlessOverlay(home, runhome string) bool { return false diff --git a/types/options_linux.go b/types/options_linux.go index a28e82883c..e2d60964c8 100644 --- a/types/options_linux.go +++ b/types/options_linux.go @@ -16,11 +16,6 @@ const ( SystemConfigFile = "/usr/share/containers/storage.conf" ) -// defaultConfigFile path to the system wide storage.conf file -var ( - defaultOverrideConfigFile = "/etc/containers/storage.conf" -) - // canUseRootlessOverlay returns true if the overlay driver can be used for rootless containers func canUseRootlessOverlay(home, runhome string) bool { // we check first for fuse-overlayfs since it is cheaper. diff --git a/types/options_test.go b/types/options_test.go index bd2d26564a..40debe5ae7 100644 --- a/types/options_test.go +++ b/types/options_test.go @@ -214,3 +214,77 @@ func TestReloadConfigurationFile(t *testing.T) { assert.Equal(t, strings.Contains(content.String(), "Failed to decode the keys [\\\"foo\\\" \\\"storage.options.graphroot\\\"] from \\\"./storage_broken.conf\\\"\""), true) } + +func TestMergeStoreOptions(t *testing.T) { + base := StoreOptions{ + RunRoot: "base/run", + GraphDriverPriority: []string{"overlay", "aufs"}, + PullOptions: map[string]string{"rate": "low"}, + DisableVolatile: false, + } + dropIn := StoreOptions{ + RunRoot: "dropin/run", + GraphDriverPriority: []string{"btrfs"}, + PullOptions: map[string]string{"rate": "high", "secure": "yes"}, + DisableVolatile: true, + } + + expected := StoreOptions{ + RunRoot: "dropin/run", + GraphDriverPriority: []string{"overlay", "aufs", "btrfs"}, + PullOptions: map[string]string{"rate": "high", "secure": "yes"}, + DisableVolatile: true, + } + + result := mergeStoreOptions(base, dropIn) + if !reflect.DeepEqual(result, expected) { + t.Errorf("Expected %+v, got %+v", expected, result) + } +} + +func TestMergeConfigFromDirectory(t *testing.T) { + tempDir, err := os.MkdirTemp("", "testConfigDir") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tempDir) + + fileNames := []string{"config1.toml", "config2.toml"} + contents := []string{ + `[storage] +runroot = 'temp/run1' +graphroot = 'temp/graph1'`, + `[storage] +runroot = 'temp/run2' +graphroot = 'temp/graph2'`, + } + for i, fileName := range fileNames { + filePath := filepath.Join(tempDir, fileName) + if err := os.WriteFile(filePath, []byte(contents[i]), 0o666); err != nil { + t.Fatalf("Failed to write to temp file: %v", err) + } + } + + // Set base options + baseOptions := StoreOptions{ + RunRoot: "initial/run", + GraphRoot: "initial/graph", + } + + // Expected results after merging configurations + expectedOptions := StoreOptions{ + RunRoot: "temp/run2", // Assuming the last file read overrides earlier values + GraphRoot: "temp/graph2", + } + + // Run the merging function + mergedOptions, err := mergeConfigFromDirectory(baseOptions, tempDir) + if err != nil { + t.Fatalf("Error merging config from directory: %v", err) + } + + // Assert the expected result + if mergedOptions.RunRoot != expectedOptions.RunRoot || mergedOptions.GraphRoot != expectedOptions.GraphRoot { + t.Errorf("Expected RunRoot to be %q and GraphRoot to be %q, got RunRoot %q and GraphRoot %q", expectedOptions.RunRoot, expectedOptions.GraphRoot, mergedOptions.RunRoot, mergedOptions.GraphRoot) + } +} diff --git a/types/options_windows.go b/types/options_windows.go index c1bea9fac0..6084cbd7d5 100644 --- a/types/options_windows.go +++ b/types/options_windows.go @@ -8,11 +8,6 @@ const ( SystemConfigFile = "/usr/share/containers/storage.conf" ) -// defaultConfigFile path to the system wide storage.conf file -var ( - defaultOverrideConfigFile = "/etc/containers/storage.conf" -) - // canUseRootlessOverlay returns true if the overlay driver can be used for rootless containers func canUseRootlessOverlay(home, runhome string) bool { return false diff --git a/types/utils.go b/types/utils.go index b313a47288..203f98ffa8 100644 --- a/types/utils.go +++ b/types/utils.go @@ -9,6 +9,7 @@ import ( "github.com/containers/storage/pkg/fileutils" "github.com/containers/storage/pkg/homedir" + "github.com/containers/storage/pkg/idtools" "github.com/sirupsen/logrus" ) @@ -72,3 +73,31 @@ func reloadConfigurationFileIfNeeded(configFile string, storeOptions *StoreOptio prevReloadConfig.mod = mtime prevReloadConfig.configFile = configFile } + +// Helper function to append unique strings to a slice. +func appendUniqueStrings(slice []string, elements []string) []string { + existing := make(map[string]bool) + for _, item := range slice { + existing[item] = true + } + for _, elem := range elements { + if !existing[elem] { + slice = append(slice, elem) + } + } + return slice +} + +// Helper function to append unique IDMaps to a slice of IDMaps. +func appendUniqueIDMaps(slice []idtools.IDMap, elements []idtools.IDMap) []idtools.IDMap { + seen := make(map[idtools.IDMap]bool) + for _, item := range slice { + seen[item] = true + } + for _, elem := range elements { + if !seen[elem] { + slice = append(slice, elem) + } + } + return slice +} diff --git a/types/utils_test.go b/types/utils_test.go index 124ad877b7..49d16b2794 100644 --- a/types/utils_test.go +++ b/types/utils_test.go @@ -4,8 +4,10 @@ import ( "fmt" "os" "path/filepath" + "reflect" "testing" + "github.com/containers/storage/pkg/idtools" "github.com/containers/storage/pkg/unshare" "gotest.tools/assert" ) @@ -42,3 +44,25 @@ func TestStorageConfOverrideEnvironmentDefaultConfigFileRoot(t *testing.T) { assert.NilError(t, err) assert.Equal(t, defaultFile, expectedPath) } + +func TestAppendUniqueStrings(t *testing.T) { + slice := []string{"one", "two"} + elements := []string{"two", "three"} + expected := []string{"one", "two", "three"} + + result := appendUniqueStrings(slice, elements) + if !reflect.DeepEqual(result, expected) { + t.Errorf("Expected %v, got %v", expected, result) + } +} + +func TestAppendUniqueIDMaps(t *testing.T) { + slice := []idtools.IDMap{{ContainerID: 100, HostID: 1000, Size: 1}} + elements := []idtools.IDMap{{ContainerID: 100, HostID: 1000, Size: 1}, {ContainerID: 101, HostID: 1001, Size: 1}} + expected := []idtools.IDMap{{ContainerID: 100, HostID: 1000, Size: 1}, {ContainerID: 101, HostID: 1001, Size: 1}} + + result := appendUniqueIDMaps(slice, elements) + if !reflect.DeepEqual(result, expected) { + t.Errorf("Expected %+v, got %+v", expected, result) + } +}