Merge pull request #2156 from nalind/config-disable-partial-pull

Disable partial pulls (zstd:chunked) by default

Author: openshift-merge-bot[bot]

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"