containers
/
storage
mirror of https://github.com/containers/storage.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

store: drop rootless from arguments 

drop the rootless argument from DefaultStoreOptions and
UpdateStoreOptions since this can be retrieved internally through the
unshare package.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
This commit is contained in:
Giuseppe Scrivano 2023-11-14 17:47:19 +01:00
parent c72a594c83
commit b0885dfba9
No known key found for this signature in database
GPG Key ID: 67E38F7A8BA21772
8 changed files with 69 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
cmd/containers-storage/main.go
View File

@ -73,7 +73,7 @@ func main() {
os.Exit(1)
}
if options.GraphRoot == "" && options.RunRoot == "" && options.GraphDriverName == "" && len(options.GraphDriverOptions) == 0 {
options, _ = types.DefaultStoreOptionsAutoDetectUID()
options, _ = types.DefaultStoreOptions()
}
args := flags.Args()
if len(args) < 1 {

2
pkg/chunked/storage_linux.go
View File

@ -254,7 +254,7 @@ func convertTarToZstdChunked(destDirectory string, blobSize int64, iss ImageSour
// GetDiffer returns a differ than can be used with ApplyDiffWithDiffer.
func GetDiffer(ctx context.Context, store storage.Store, blobSize int64, annotations map[string]string, iss ImageSourceSeekable) (graphdriver.Differ, error) {
storeOpts, err := types.DefaultStoreOptionsAutoDetectUID()
storeOpts, err := types.DefaultStoreOptions()
if err != nil {
return nil, err
}

4
store.go
View File

@ -3545,8 +3545,8 @@ func SetDefaultConfigFilePath(path string) {
}
// DefaultConfigFile returns the path to the storage config file used
func DefaultConfigFile(rootless bool) (string, error) {
return types.DefaultConfigFile(rootless)
func DefaultConfigFile() (string, error) {
return types.DefaultConfigFile()
}
// ReloadConfigurationFile parses the specified configuration file and overrides

70
types/options.go
View File

@ -89,7 +89,7 @@ func loadDefaultStoreOptions() {
_, err := os.Stat(defaultOverrideConfigFile)
if err == nil {
// The DefaultConfigFile(rootless) function returns the path
// The DefaultConfigFile() function returns the path
// of the used storage.conf file, by returning defaultConfigFile
// If override exists containers/storage uses it by default.
defaultConfigFile = defaultOverrideConfigFile
@ -111,21 +111,41 @@ func loadDefaultStoreOptions() {
setDefaults()
}
// defaultStoreOptionsIsolated is an internal implementation detail of DefaultStoreOptions to allow testing.
// Everyone but the tests this is intended for should only call DefaultStoreOptions, never this function.
func defaultStoreOptionsIsolated(rootless bool, rootlessUID int, storageConf string) (StoreOptions, error) {
// loadStoreOptions returns the default storage ops for containers
func loadStoreOptions() (StoreOptions, error) {
storageConf, err := DefaultConfigFile()
if err != nil {
return defaultStoreOptions, err
}
return loadStoreOptionsFromConfFile(storageConf)
}
// usePerUserStorage returns whether the user private storage must be used.
// We cannot simply use the unshare.IsRootless() condition, because
// that checks only if the current process needs a user namespace to
// work and it would break cases where the process is already created
// in a user namespace (e.g. nested Podman/Buildah) and the desired
// behavior is to use system paths instead of user private paths.
func usePerUserStorage() bool {
return unshare.IsRootless() && unshare.GetRootlessUID() != 0
}
// loadStoreOptionsFromConfFile is an internal implementation detail of DefaultStoreOptions to allow testing.
// Everyone but the tests this is intended for should only call loadStoreOptions, never this function.
func loadStoreOptionsFromConfFile(storageConf string) (StoreOptions, error) {
var (
defaultRootlessRunRoot string
defaultRootlessGraphRoot string
err error
)
defaultStoreOptionsOnce.Do(loadDefaultStoreOptions)
if loadDefaultStoreOptionsErr != nil {
return StoreOptions{}, loadDefaultStoreOptionsErr
}
storageOpts := defaultStoreOptions
if rootless && rootlessUID != 0 {
storageOpts, err = getRootlessStorageOpts(rootlessUID, storageOpts)
if usePerUserStorage() {
storageOpts, err = getRootlessStorageOpts(storageOpts)
if err != nil {
return storageOpts, err
}
@ -139,7 +159,7 @@ func defaultStoreOptionsIsolated(rootless bool, rootlessUID int, storageConf str
defaultRootlessGraphRoot = storageOpts.GraphRoot
storageOpts = StoreOptions{}
reloadConfigurationFileIfNeeded(storageConf, &storageOpts)
if rootless && rootlessUID != 0 {
if usePerUserStorage() {
// If the file did not specify a graphroot or runroot,
// set sane defaults so we don't try and use root-owned
// directories
@ -158,6 +178,7 @@ func defaultStoreOptionsIsolated(rootless bool, rootlessUID int, storageConf str
if storageOpts.RunRoot == "" {
return storageOpts, fmt.Errorf("runroot must be set")
}
rootlessUID := unshare.GetRootlessUID()
runRoot, err := expandEnvPath(storageOpts.RunRoot, rootlessUID)
if err != nil {
return storageOpts, err
@ -188,26 +209,17 @@ func defaultStoreOptionsIsolated(rootless bool, rootlessUID int, storageConf str
return storageOpts, nil
}
// loadStoreOptions returns the default storage ops for containers
func loadStoreOptions(rootless bool, rootlessUID int) (StoreOptions, error) {
storageConf, err := DefaultConfigFile(rootless && rootlessUID != 0)
if err != nil {
return defaultStoreOptions, err
}
return defaultStoreOptionsIsolated(rootless, rootlessUID, storageConf)
}
// UpdateOptions should be called iff container engine received a SIGHUP,
// otherwise use DefaultStoreOptions
func UpdateStoreOptions(rootless bool, rootlessUID int) (StoreOptions, error) {
storeOptions, storeError = loadStoreOptions(rootless, rootlessUID)
func UpdateStoreOptions() (StoreOptions, error) {
storeOptions, storeError = loadStoreOptions()
return storeOptions, storeError
}
// DefaultStoreOptions returns the default storage ops for containers
func DefaultStoreOptions(rootless bool, rootlessUID int) (StoreOptions, error) {
func DefaultStoreOptions() (StoreOptions, error) {
once.Do(func() {
storeOptions, storeError = loadStoreOptions(rootless, rootlessUID)
storeOptions, storeError = loadStoreOptions()
})
return storeOptions, storeError
}
@ -272,9 +284,11 @@ func isRootlessDriver(driver string) bool {
}
// getRootlessStorageOpts returns the storage opts for containers running as non root
func getRootlessStorageOpts(rootlessUID int, systemOpts StoreOptions) (StoreOptions, error) {
func getRootlessStorageOpts(systemOpts StoreOptions) (StoreOptions, error) {
var opts StoreOptions
rootlessUID := unshare.GetRootlessUID()
dataDir, err := homedir.GetDataHome()
if err != nil {
return opts, err
@ -355,12 +369,6 @@ func getRootlessStorageOpts(rootlessUID int, systemOpts StoreOptions) (StoreOpti
return opts, nil
}
// DefaultStoreOptionsAutoDetectUID returns the default storage ops for containers
func DefaultStoreOptionsAutoDetectUID() (StoreOptions, error) {
uid := unshare.GetRootlessUID()
return DefaultStoreOptions(uid != 0, uid)
}
var prevReloadConfig = struct {
storeOptions *StoreOptions
mod time.Time
@ -530,8 +538,8 @@ func Options() (StoreOptions, error) {
}
// Save overwrites the tomlConfig in storage.conf with the given conf
func Save(conf TomlConfig, rootless bool) error {
configFile, err := DefaultConfigFile(rootless)
func Save(conf TomlConfig) error {
configFile, err := DefaultConfigFile()
if err != nil {
return err
}
@ -549,10 +557,10 @@ func Save(conf TomlConfig, rootless bool) error {
}
// StorageConfig is used to retrieve the storage.conf toml in order to overwrite it
func StorageConfig(rootless bool) (*TomlConfig, error) {
func StorageConfig() (*TomlConfig, error) {
config := new(TomlConfig)
configFile, err := DefaultConfigFile(rootless)
configFile, err := DefaultConfigFile()
if err != nil {
return nil, err
}

25
types/options_test.go
View File

@ -10,6 +10,7 @@ import (
"testing"
"github.com/containers/storage/pkg/idtools"
"github.com/containers/storage/pkg/unshare"
"github.com/sirupsen/logrus"
"github.com/stretchr/testify/require"
"gotest.tools/assert"
@ -32,7 +33,7 @@ func TestGetRootlessStorageOpts(t *testing.T) {
systemOpts.GraphRoot = home
systemOpts.RunRoot = runhome
storageOpts, err := getRootlessStorageOpts(os.Geteuid(), systemOpts)
storageOpts, err := getRootlessStorageOpts(systemOpts)
assert.NilError(t, err)
expectedDriver := vfsDriver
@ -45,7 +46,7 @@ func TestGetRootlessStorageOpts(t *testing.T) {
t.Run("systemDriver=btrfs", func(t *testing.T) {
systemOpts := StoreOptions{}
systemOpts.GraphDriverName = "btrfs"
storageOpts, err := getRootlessStorageOpts(1000, systemOpts)
storageOpts, err := getRootlessStorageOpts(systemOpts)
assert.NilError(t, err)
assert.Equal(t, storageOpts.GraphDriverName, "btrfs")
})
@ -53,7 +54,7 @@ func TestGetRootlessStorageOpts(t *testing.T) {
t.Run("systemDriver=overlay", func(t *testing.T) {
systemOpts := StoreOptions{}
systemOpts.GraphDriverName = overlayDriver
storageOpts, err := getRootlessStorageOpts(1000, systemOpts)
storageOpts, err := getRootlessStorageOpts(systemOpts)
assert.NilError(t, err)
assert.Equal(t, storageOpts.GraphDriverName, overlayDriver)
})
@ -61,7 +62,7 @@ func TestGetRootlessStorageOpts(t *testing.T) {
t.Run("systemDriver=overlay2", func(t *testing.T) {
systemOpts := StoreOptions{}
systemOpts.GraphDriverName = "overlay2"
storageOpts, err := getRootlessStorageOpts(1000, systemOpts)
storageOpts, err := getRootlessStorageOpts(systemOpts)
assert.NilError(t, err)
assert.Equal(t, storageOpts.GraphDriverName, overlayDriver)
})
@ -69,7 +70,7 @@ func TestGetRootlessStorageOpts(t *testing.T) {
t.Run("systemDriver=vfs", func(t *testing.T) {
systemOpts := StoreOptions{}
systemOpts.GraphDriverName = vfsDriver
storageOpts, err := getRootlessStorageOpts(1000, systemOpts)
storageOpts, err := getRootlessStorageOpts(systemOpts)
assert.NilError(t, err)
assert.Equal(t, storageOpts.GraphDriverName, vfsDriver)
})
@ -77,7 +78,7 @@ func TestGetRootlessStorageOpts(t *testing.T) {
t.Run("systemDriver=aufs", func(t *testing.T) {
systemOpts := StoreOptions{}
systemOpts.GraphDriverName = "aufs"
storageOpts, err := getRootlessStorageOpts(1000, systemOpts)
storageOpts, err := getRootlessStorageOpts(systemOpts)
assert.NilError(t, err)
assert.Assert(t, storageOpts.GraphDriverName == overlayDriver || storageOpts.GraphDriverName == vfsDriver, fmt.Sprintf("The rootless driver should be set to 'overlay' or 'vfs' not '%v'", storageOpts.GraphDriverName))
})
@ -85,7 +86,7 @@ func TestGetRootlessStorageOpts(t *testing.T) {
t.Run("systemDriver=devmapper", func(t *testing.T) {
systemOpts := StoreOptions{}
systemOpts.GraphDriverName = "devmapper"
storageOpts, err := getRootlessStorageOpts(1000, systemOpts)
storageOpts, err := getRootlessStorageOpts(systemOpts)
assert.NilError(t, err)
assert.Assert(t, storageOpts.GraphDriverName == overlayDriver || storageOpts.GraphDriverName == vfsDriver, fmt.Sprintf("The rootless driver should be set to 'overlay' or 'vfs' not '%v'", storageOpts.GraphDriverName))
})
@ -93,7 +94,7 @@ func TestGetRootlessStorageOpts(t *testing.T) {
t.Run("systemDriver=zfs", func(t *testing.T) {
systemOpts := StoreOptions{}
systemOpts.GraphDriverName = "zfs"
storageOpts, err := getRootlessStorageOpts(1000, systemOpts)
storageOpts, err := getRootlessStorageOpts(systemOpts)
assert.NilError(t, err)
assert.Assert(t, storageOpts.GraphDriverName == overlayDriver || storageOpts.GraphDriverName == vfsDriver, fmt.Sprintf("The rootless driver should be set to 'overlay' or 'vfs' not '%v'", storageOpts.GraphDriverName))
})
@ -102,7 +103,7 @@ func TestGetRootlessStorageOpts(t *testing.T) {
t.Setenv("STORAGE_DRIVER", "btrfs")
systemOpts := StoreOptions{}
systemOpts.GraphDriverName = vfsDriver
storageOpts, err := getRootlessStorageOpts(1000, systemOpts)
storageOpts, err := getRootlessStorageOpts(systemOpts)
assert.NilError(t, err)
assert.Equal(t, storageOpts.GraphDriverName, "btrfs")
})
@ -111,7 +112,7 @@ func TestGetRootlessStorageOpts(t *testing.T) {
t.Setenv("STORAGE_DRIVER", "zfs")
systemOpts := StoreOptions{}
systemOpts.GraphDriverName = vfsDriver
storageOpts, err := getRootlessStorageOpts(1000, systemOpts)
storageOpts, err := getRootlessStorageOpts(systemOpts)
assert.NilError(t, err)
assert.Equal(t, storageOpts.GraphDriverName, "zfs")
})
@ -127,8 +128,8 @@ func TestGetRootlessStorageOpts2(t *testing.T) {
opts := StoreOptions{
RootlessStoragePath: "/$HOME/$UID/containers/storage",
}
expectedPath := filepath.Join(os.Getenv("HOME"), "2000", "containers/storage")
storageOpts, err := getRootlessStorageOpts(2000, opts)
expectedPath := filepath.Join(os.Getenv("HOME"), fmt.Sprintf("%d", unshare.GetRootlessUID()), "containers/storage")
storageOpts, err := getRootlessStorageOpts(opts)
assert.NilError(t, err)
assert.Equal(t, storageOpts.GraphRoot, expectedPath)
}

4
types/utils.go
View File

@ -22,7 +22,7 @@ func expandEnvPath(path string, rootlessUID int) (string, error) {
return newpath, nil
}
func DefaultConfigFile(rootless bool) (string, error) {
func DefaultConfigFile() (string, error) {
if defaultConfigFileSet {
return defaultConfigFile, nil
}
@ -30,7 +30,7 @@ func DefaultConfigFile(rootless bool) (string, error) {
if path, ok := os.LookupEnv(storageConfEnv); ok {
return path, nil
}
if !rootless {
if !usePerUserStorage() {
if _, err := os.Stat(defaultOverrideConfigFile); err == nil {
return defaultOverrideConfigFile, nil
}

14
types/utils_test.go
View File

@ -1,17 +1,21 @@
package types
import (
"fmt"
"os"
"path/filepath"
"testing"
"github.com/containers/storage/pkg/unshare"
"gotest.tools/assert"
)
func TestDefaultStoreOpts(t *testing.T) {
storageOpts, err := defaultStoreOptionsIsolated(true, 1000, "./storage_test.conf")
expectedPath := filepath.Join(os.Getenv("HOME"), "1000", "containers/storage")
if !usePerUserStorage() {
t.Skip()
}
storageOpts, err := loadStoreOptionsFromConfFile("./storage_test.conf")
expectedPath := filepath.Join(os.Getenv("HOME"), fmt.Sprintf("%d", unshare.GetRootlessUID()), "containers/storage")
assert.NilError(t, err)
assert.Equal(t, storageOpts.RunRoot, expectedPath)
@ -21,7 +25,7 @@ func TestDefaultStoreOpts(t *testing.T) {
func TestStorageConfOverrideEnvironmentDefaultConfigFileRootless(t *testing.T) {
t.Setenv("CONTAINERS_STORAGE_CONF", "default_override_test.conf")
defaultFile, err := DefaultConfigFile(true)
defaultFile, err := DefaultConfigFile()
expectedPath := "default_override_test.conf"
@ -31,7 +35,7 @@ func TestStorageConfOverrideEnvironmentDefaultConfigFileRootless(t *testing.T) {
func TestStorageConfOverrideEnvironmentDefaultConfigFileRoot(t *testing.T) {
t.Setenv("CONTAINERS_STORAGE_CONF", "default_override_test.conf")
defaultFile, err := DefaultConfigFile(false)
defaultFile, err := DefaultConfigFile()
expectedPath := "default_override_test.conf"

9
utils.go
View File

@ -11,14 +11,9 @@ func ParseIDMapping(UIDMapSlice, GIDMapSlice []string, subUIDMap, subGIDMap stri
return types.ParseIDMapping(UIDMapSlice, GIDMapSlice, subUIDMap, subGIDMap)
}
// DefaultStoreOptionsAutoDetectUID returns the default storage options for containers
func DefaultStoreOptionsAutoDetectUID() (types.StoreOptions, error) {
return types.DefaultStoreOptionsAutoDetectUID()
}
// DefaultStoreOptions returns the default storage options for containers
func DefaultStoreOptions(rootless bool, rootlessUID int) (types.StoreOptions, error) {
return types.DefaultStoreOptions(rootless, rootlessUID)
func DefaultStoreOptions() (types.StoreOptions, error) {
return types.DefaultStoreOptions()
}
func validateMountOptions(mountOptions []string) error {