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 <nalin@redhat.com>

Author: nalind

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,
+	})
+}

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)

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
   <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)
 

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"))

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.

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"