types/options.go

@@ -135,15 +135,15 @@ func loadStoreOptions() (StoreOptions, error) {
 		return defaultStoreOptions, err
 	}
 
-	baseOptions, err = mergeConfigFromDirectory(baseOptions, defaultDropInConfigDir)
+	err = mergeConfigFromDirectory(&baseOptions, defaultDropInConfigDir)
 	if err != nil {
 		return defaultStoreOptions, err
 	}
 
 	return baseOptions, nil
 }
 
-func mergeConfigFromDirectory(baseOptions StoreOptions, configDir string) (StoreOptions, error) {
+func mergeConfigFromDirectory(baseOptions *StoreOptions, configDir string) error {
 	err := filepath.Walk(configDir, func(path string, info os.FileInfo, err error) error {
 		if err != nil {
 			return err
@@ -153,68 +153,17 @@ func mergeConfigFromDirectory(baseOptions StoreOptions, configDir string) (Store
 		}
 
 		// Load drop-in options from the current file
-		dropInOptions, err := loadStoreOptionsFromConfFile(path)
+		err = ReloadConfigurationFile(path, baseOptions)
 		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 err
 	}
-
-	return base
+	return nil
 }
 
 // usePerUserStorage returns whether the user private storage must be used.

types/options_test.go

@@ -215,33 +215,6 @@ 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 {
@@ -278,13 +251,13 @@ graphroot = 'temp/graph2'`,
 	}
 
 	// Run the merging function
-	mergedOptions, err := mergeConfigFromDirectory(baseOptions, tempDir)
+	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)
+	if baseOptions.RunRoot != expectedOptions.RunRoot || baseOptions.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, baseOptions.RunRoot, baseOptions.GraphRoot)
 	}
 }

types/utils.go

@@ -9,7 +9,6 @@ import (
 
 	"github.com/containers/storage/pkg/fileutils"
 	"github.com/containers/storage/pkg/homedir"
-	"github.com/containers/storage/pkg/idtools"
 	"github.com/sirupsen/logrus"
 )
 
@@ -73,31 +72,3 @@ 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
-}

types/utils_test.go

@@ -4,10 +4,8 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
-	"reflect"
 	"testing"
 
-	"github.com/containers/storage/pkg/idtools"
 	"github.com/containers/storage/pkg/unshare"
 	"gotest.tools/assert"
 )
@@ -44,25 +42,3 @@ 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)
-	}
-}