From 435aa93e14d28ae9657367b319be97baf9acd09c Mon Sep 17 00:00:00 2001 From: Nalin Dahyabhai Date: Wed, 30 Oct 2024 16:20:29 -0400 Subject: [PATCH] Disable partial pulls (zstd:chunked) by default Disable the storage.options.pull_options.enable_partial_images option by default, so that it will have to be explicitly enabled in order to be used. Update the apply-diff-from-staging-directory integration test to call the test helper binary directly, so that the configuration file the test writes won't have its settings overridden by command line options that the storage() test helper function adds. Signed-off-by: Nalin Dahyabhai --- cmd/containers-storage/config.go | 33 +++++++++++++++++++++++++++++++ docs/containers-storage-config.md | 18 +++++++++++++++++ docs/containers-storage.conf.5.md | 4 ++-- pkg/chunked/storage_linux.go | 2 +- storage.conf | 18 ++++++++--------- tests/apply-diff.bats | 14 ++++++------- 6 files changed, 70 insertions(+), 19 deletions(-) create mode 100644 cmd/containers-storage/config.go create mode 100644 docs/containers-storage-config.md diff --git a/cmd/containers-storage/config.go b/cmd/containers-storage/config.go new file mode 100644 index 0000000000..cacf83c4d8 --- /dev/null +++ b/cmd/containers-storage/config.go @@ -0,0 +1,33 @@ +package main + +import ( + "fmt" + + "github.com/containers/storage" + "github.com/containers/storage/pkg/mflag" + "github.com/containers/storage/types" +) + +func config(flags *mflag.FlagSet, action string, m storage.Store, args []string) (int, error) { + options, err := types.DefaultStoreOptions() + if err != nil { + return 1, fmt.Errorf("default: %+v", err) + } + if len(args) > 0 { + if err = types.ReloadConfigurationFileIfNeeded(args[0], &options); err != nil { + return 1, fmt.Errorf("reload: %+v", err) + } + } + return outputJSON(options) +} + +func init() { + commands = append(commands, command{ + names: []string{"config"}, + usage: "Print storage library configuration as JSON", + minArgs: 0, + maxArgs: 1, + optionsHelp: "[configurationFile]", + action: config, + }) +} diff --git a/docs/containers-storage-config.md b/docs/containers-storage-config.md new file mode 100644 index 0000000000..5b452f3f65 --- /dev/null +++ b/docs/containers-storage-config.md @@ -0,0 +1,18 @@ +## containers-storage-config 1 "November 2024" + +## NAME +containers-storage config - Output the configuration for the storage library + +## SYNOPSIS +**containers-storage** **config** [configurationFile] + +## DESCRIPTION +Reads and outputs the current configuration for the storage library, or the +current configuration with the contents of a specified configuration file +loaded in, in a JSON format. + +## EXAMPLE +**containers-storage config** + +## SEE ALSO +containers-storage-version(1) diff --git a/docs/containers-storage.conf.5.md b/docs/containers-storage.conf.5.md index dc3974cdbb..2f8e29b01f 100644 --- a/docs/containers-storage.conf.5.md +++ b/docs/containers-storage.conf.5.md @@ -102,8 +102,8 @@ The `storage.options.pull_options` table supports the following keys: **enable_partial_images="true"|"false"** Enable the "zstd:chunked" feature, which allows partial pulls, reusing - content that already exists on the system. This is enabled by default, - but can be explicitly disabled. For more on zstd:chunked, see + content that already exists on the system. This is disabled by default, + and must be explicitly enabled to be used. For more on zstd:chunked, see . This is a "string bool": "false"|"true" (cannot be native TOML boolean) diff --git a/pkg/chunked/storage_linux.go b/pkg/chunked/storage_linux.go index 80c9a6a22a..8ecbfb9826 100644 --- a/pkg/chunked/storage_linux.go +++ b/pkg/chunked/storage_linux.go @@ -149,7 +149,7 @@ func (c *chunkedDiffer) convertTarToZstdChunked(destDirectory string, payload *o func GetDiffer(ctx context.Context, store storage.Store, blobDigest digest.Digest, blobSize int64, annotations map[string]string, iss ImageSourceSeekable) (graphdriver.Differ, error) { pullOptions := store.PullOptions() - if !parseBooleanPullOption(pullOptions, "enable_partial_images", true) { + if !parseBooleanPullOption(pullOptions, "enable_partial_images", false) { // If convertImages is set, the two options disagree whether fallback is permissible. // Right now, we enable it, but that’s not a promise; rather, such a configuration should ideally be rejected. return nil, newErrFallbackToOrdinaryLayerDownload(errors.New("partial images are disabled")) diff --git a/storage.conf b/storage.conf index 7ac8fdf8f0..962325233e 100644 --- a/storage.conf +++ b/storage.conf @@ -8,12 +8,12 @@ # /usr/containers/storage.conf # /etc/containers/storage.conf # $HOME/.config/containers/storage.conf -# $XDG_CONFIG_HOME/containers/storage.conf (If XDG_CONFIG_HOME is set) +# $XDG_CONFIG_HOME/containers/storage.conf (if XDG_CONFIG_HOME is set) # See man 5 containers-storage.conf for more information -# The "container storage" table contains all of the server options. +# The "storage" table contains all of the server options. [storage] -# Default Storage Driver, Must be set for proper operation. +# Default storage driver, must be set for proper operation. driver = "overlay" # Temporary storage location @@ -24,8 +24,8 @@ runroot = "/run/containers/storage" # driver_priority = ["overlay", "btrfs"] # Primary Read/Write location of container storage -# When changing the graphroot location on an SELINUX system, you must -# ensure the labeling matches the default locations labels with the +# When changing the graphroot location on an SELinux system, you must +# ensure the labeling matches the default location's labels with the # following commands: # semanage fcontext -a -e /var/lib/containers/storage /NEWSTORAGEPATH # restorecon -R -v /NEWSTORAGEPATH @@ -54,14 +54,14 @@ graphroot = "/var/lib/containers/storage" additionalimagestores = [ ] -# Options controlling how storage is populated when pulling images. +# Options controlling how storage is populated when pulling images. [storage.options.pull_options] # Enable the "zstd:chunked" feature, which allows partial pulls, reusing -# content that already exists on the system. This is enabled by default, -# but can be explicitly disabled. For more on zstd:chunked, see +# content that already exists on the system. This is disabled by default, +# and must be explicitly enabled to be used. For more on zstd:chunked, see # https://github.com/containers/storage/blob/main/docs/containers-storage-zstd-chunked.md # This is a "string bool": "false" | "true" (cannot be native TOML boolean) -# enable_partial_images = "true" +# enable_partial_images = "false" # Tells containers/storage to use hard links rather then create new files in # the image, if an identical file already existed in storage. diff --git a/tests/apply-diff.bats b/tests/apply-diff.bats index 83807c845e..afa49c0d28 100644 --- a/tests/apply-diff.bats +++ b/tests/apply-diff.bats @@ -71,28 +71,28 @@ driver="overlay" graphroot="$root" runroot="$runroot" -[storage.options] -pull_options = {enable_partial_images = "true" } +[storage.options.pull_options] +enable_partial_images = "true" EOF # Create a layer. - CONTAINERS_STORAGE_CONF=$sconf run storage --debug=false create-layer + CONTAINERS_STORAGE_CONF=$sconf run ${STORAGE_BINARY} create-layer [ "$status" -eq 0 ] [ "$output" != "" ] layer="$output" - CONTAINERS_STORAGE_CONF=$sconf run storage --debug=false applydiff-using-staging-dir $layer $SRC + CONTAINERS_STORAGE_CONF=$sconf run ${STORAGE_BINARY} applydiff-using-staging-dir $layer $SRC [ "$status" -eq 0 ] name=safe-image - CONTAINERS_STORAGE_CONF=$sconf run storage --debug=false create-image --name $name $layer + CONTAINERS_STORAGE_CONF=$sconf run ${STORAGE_BINARY} create-image --name $name $layer [ "$status" -eq 0 ] ctrname=foo - CONTAINERS_STORAGE_CONF=$sconf run storage --debug=false create-container --name $ctrname $name + CONTAINERS_STORAGE_CONF=$sconf run ${STORAGE_BINARY} create-container --name $ctrname $name [ "$status" -eq 0 ] - CONTAINERS_STORAGE_CONF=$sconf run storage --debug=false mount $ctrname + CONTAINERS_STORAGE_CONF=$sconf run ${STORAGE_BINARY} mount $ctrname [ "$status" -eq 0 ] mount="$output"