Clarify definition of --pull options

Use github.com/containers/common/pkg/config for handling pull policy.

No longer document --pull=true|false

Fixes: containers#5406

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>

buildah.go

@@ -14,6 +14,7 @@ import (
 	"github.com/containers/buildah/define"
 	"github.com/containers/buildah/docker"
 	nettypes "github.com/containers/common/libnetwork/types"
+	"github.com/containers/common/pkg/config"
 	"github.com/containers/image/v5/types"
 	encconfig "github.com/containers/ocicrypt/config"
 	"github.com/containers/storage"
@@ -40,29 +41,6 @@ const (
 	stateFile = Package + ".json"
 )
 
-// PullPolicy takes the value PullIfMissing, PullAlways, PullIfNewer, or PullNever.
-type PullPolicy = define.PullPolicy
-
-const (
-	// PullIfMissing is one of the values that BuilderOptions.PullPolicy
-	// can take, signalling that the source image should be pulled from a
-	// registry if a local copy of it is not already present.
-	PullIfMissing = define.PullIfMissing
-	// PullAlways is one of the values that BuilderOptions.PullPolicy can
-	// take, signalling that a fresh, possibly updated, copy of the image
-	// should be pulled from a registry before the build proceeds.
-	PullAlways = define.PullAlways
-	// PullIfNewer is one of the values that BuilderOptions.PullPolicy
-	// can take, signalling that the source image should only be pulled
-	// from a registry if a local copy is not already present or if a
-	// newer version the image is present on the repository.
-	PullIfNewer = define.PullIfNewer
-	// PullNever is one of the values that BuilderOptions.PullPolicy can
-	// take, signalling that the source image should not be pulled from a
-	// registry if a local copy of it is not already present.
-	PullNever = define.PullNever
-)
-
 // NetworkConfigurationPolicy takes the value NetworkDefault, NetworkDisabled,
 // or NetworkEnabled.
 type NetworkConfigurationPolicy = define.NetworkConfigurationPolicy
@@ -273,7 +251,7 @@ type BuilderOptions struct {
 	// PullPolicy decides whether or not we should pull the image that
 	// we're using as a base image.  It should be PullIfMissing,
 	// PullAlways, or PullNever.
-	PullPolicy define.PullPolicy
+	PullPolicy config.PullPolicy
 	// Registry is a value which is prepended to the image's name, if it
 	// needs to be pulled and the image name alone can not be resolved to a
 	// reference to a source image.  No separator is implicitly added.

cmd/buildah/from.go

@@ -70,7 +70,12 @@ func init() {
 	flags.StringVar(&opts.creds, "creds", "", "use `[username[:password]]` for accessing the registry")
 	flags.StringVarP(&opts.format, "format", "f", defaultFormat(), "`format` of the image manifest and metadata")
 	flags.StringVar(&opts.name, "name", "", "`name` for the working container")
-	flags.StringVar(&opts.pull, "pull", "true", "pull images from the registry if newer or not present in store, if false, only pull images if not present, if always, pull images even if the named images are present in store, if never, only use images present in store if available")
+	flags.StringVar(&opts.pull, "pull", "missing", `pull images from the registry values:
+always: pull images even if the named images are present in store,
+missing: pull images if the named images are not present in store,
+never: only use images present in store if available,
+newer: pull if the image on the registry is newer than the in store`)
+
 	flags.Lookup("pull").NoOptDefVal = "true" //allow `--pull ` to be set to `true` as expected.
 
 	flags.BoolVar(&opts.pullAlways, "pull-always", false, "pull the image even if the named image is present in store")

cmd/buildah/pull.go

@@ -8,10 +8,10 @@ import (
 	"time"
 
 	"github.com/containers/buildah"
-	"github.com/containers/buildah/define"
 	"github.com/containers/buildah/pkg/cli"
 	"github.com/containers/buildah/pkg/parse"
 	"github.com/containers/common/pkg/auth"
+	"github.com/containers/common/pkg/config"
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 )
@@ -119,9 +119,9 @@ func pullCmd(c *cobra.Command, args []string, iopts pullOptions) error {
 		return fmt.Errorf("unable to obtain decryption config: %w", err)
 	}
 
-	policy, ok := define.PolicyMap[iopts.pullPolicy]
-	if !ok {
-		return fmt.Errorf("unsupported pull policy %q", iopts.pullPolicy)
+	policy, err := config.ParsePullPolicy(iopts.pullPolicy)
+	if err != nil {
+		return err
 	}
 	options := buildah.PullOptions{
 		SignaturePolicyPath: iopts.signaturePolicy,

convertcw.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/containers/buildah/define"
 	"github.com/containers/buildah/internal/mkcw"
+	"github.com/containers/common/pkg/config"
 	"github.com/containers/image/v5/docker/reference"
 	"github.com/containers/image/v5/types"
 	encconfig "github.com/containers/ocicrypt/config"
@@ -50,7 +51,7 @@ type CWConvertImageOptions struct {
 	// Passed through to BuilderOptions. Most settings won't make
 	// sense to be made available here because we don't launch a process.
 	ContainerSuffix     string
-	PullPolicy          PullPolicy
+	PullPolicy          config.PullPolicy
 	BlobDirectory       string
 	SignaturePolicyPath string
 	ReportWriter        io.Writer

define/build.go

@@ -5,6 +5,7 @@ import (
 	"time"
 
 	nettypes "github.com/containers/common/libnetwork/types"
+	"github.com/containers/common/pkg/config"
 	"github.com/containers/image/v5/docker/reference"
 	"github.com/containers/image/v5/types"
 	encconfig "github.com/containers/ocicrypt/config"
@@ -120,7 +121,7 @@ type BuildOptions struct {
 	ContextDirectory string
 	// PullPolicy controls whether or not we pull images.  It should be one
 	// of PullIfMissing, PullAlways, PullIfNewer, or PullNever.
-	PullPolicy PullPolicy
+	PullPolicy config.PullPolicy
 	// Registry is a value which is prepended to the image's name, if it
 	// needs to be pulled and the image name alone can not be resolved to a
 	// reference to a source image.  No separator is implicitly added.

define/types.go

@@ -14,6 +14,7 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/containers/common/pkg/config"
 	"github.com/containers/image/v5/manifest"
 	"github.com/containers/storage/pkg/archive"
 	"github.com/containers/storage/pkg/chrootarchive"
@@ -159,7 +160,7 @@ const (
 type SBOMScanOptions struct {
 	Type            []string          // a shorthand name for a defined group of these options
 	Image           string            // the scanner image to use
-	PullPolicy      PullPolicy        // how to get the scanner image
+	PullPolicy      config.PullPolicy // how to get the scanner image
 	Commands        []string          // one or more commands to invoke for the image rootfs or ContextDir locations
 	ContextDir      []string          // one or more "source" directory locations
 	SBOMOutput      string            // where to save SBOM scanner output outside of the image (i.e., the local filesystem)

docs/buildah-build.1.md

@@ -742,31 +742,12 @@ The `buildah build` command allows building images for all Linux architectures,
 
 **--pull**
 
-When the *--pull* flag is enabled or set explicitly to `true` (with
-*--pull=true*), attempt to pull the latest versions of base and SBOM scanner
-images from the registries listed in registries.conf if a local base or SBOM
-scanner image does not exist or the image in the registry is newer than the one
-in local storage. Raise an error if the base or SBOM scanner image is not in
-any listed registry and is not present locally.
-
-If the flag is disabled (with *--pull=false*), do not pull base and SBOM
-scanner images from registries, use only local versions. Raise an error if a
-base or SBOM scanner image is not present locally.
-
-If the pull flag is set to `always` (with *--pull=always*), pull base and SBOM
-scanner images from the registries listed in registries.conf.  Raise an error
-if a base or SBOM scanner image is not found in the registries, even if an
-image with the same name is present locally.
-
-If the pull flag is set to `missing` (with *--pull=missing*), pull base and
-SBOM scanner images only if they could not be found in the local containers
-storage.  Raise an error if no image could be found and the pull fails.
-
-If the pull flag is set to `never` (with *--pull=never*), do not pull base and
-SBOM scanner images from registries, use only the local versions.  Raise an
-error if the image is not present locally.
-
-Defaults to *true*.
+Pull image policy. The default is **missing**.
+
+- **always**: Always pull the image and throw an error if the pull fails.
+- **missing**: Pull the image only when the image is not in the local containers storage.  Throw an error if no image is found and the pull fails.
+- **never**: Never pull the image but use the one from the local containers storage.  Throw an error if no image is found.
+- **newer**: Pull if the image on the registry is newer than the one in the local containers storage.  An image is considered to be newer when the digests are different.  Comparing the time stamps is prone to errors.  Pull errors are suppressed if a local image was found.
 
 **--quiet**, **-q**
 

docs/buildah-from.1.md

@@ -372,24 +372,11 @@ the help of emulation provided by packages like `qemu-user-static`.
 
 **--pull**
 
-When the flag is enabled or set explicitly to `true` (with *--pull=true*), attempt to pull the latest image from the registries
-listed in registries.conf if a local image does not exist or the image is newer
-than the one in storage. Raise an error if the image is not in any listed
-registry and is not present locally.
+Pull image policy. The default is **missing**.
 
-If the flag is disabled (with *--pull=false*), do not pull the image from the
-registry, use only the local version. Raise an error if the image is not
-present locally.
-
-If the pull flag is set to `always` (with *--pull=always*),
-pull the image from the first registry it is found in as listed in registries.conf.
-Raise an error if not found in the registries, even if the image is present locally.
-
-If the pull flag is set to `never` (with *--pull=never*),
-Do not pull the image from the registry, use only the local version. Raise an error
-if the image is not present locally.
-
-Defaults to *true*.
+- **always**, **true**: Always pull the image and throw an error if the pull fails.
+- **missing**: Only pull the image when it does not exist in the local containers storage.  Throw an error if no image is found and the pull fails.
+- **never**, **false**: Never pull the image but use the one from the local containers storage.  Throw an error when no image is found.
 
 **--quiet**, **-q**
 

imagebuildah/build.go

@@ -42,10 +42,10 @@ import (
 )
 
 const (
-	PullIfMissing = define.PullIfMissing
-	PullAlways    = define.PullAlways
-	PullIfNewer   = define.PullIfNewer
-	PullNever     = define.PullNever
+	PullAlways    = config.PullPolicyAlways
+	PullIfMissing = config.PullPolicyMissing
+	PullIfNewer   = config.PullPolicyNewer
+	PullNever     = config.PullPolicyNever
 
 	Gzip         = archive.Gzip
 	Bzip2        = archive.Bzip2

imagebuildah/executor.go

@@ -69,7 +69,7 @@ type Executor struct {
 	stages                         map[string]*StageExecutor
 	store                          storage.Store
 	contextDir                     string
-	pullPolicy                     define.PullPolicy
+	pullPolicy                     config.PullPolicy
 	registry                       string
 	ignoreUnrecognizedInstructions bool
 	quiet                          bool

imagebuildah/stage_executor.go

@@ -837,7 +837,7 @@ func (s *StageExecutor) UnrecognizedInstruction(step *imagebuilder.Step) error {
 // prepare creates a working container based on the specified image, or if one
 // isn't specified, the first argument passed to the first FROM instruction we
 // can find in the stage's parsed tree.
-func (s *StageExecutor) prepare(ctx context.Context, from string, initializeIBConfig, rebase, preserveBaseImageAnnotations bool, pullPolicy define.PullPolicy) (builder *buildah.Builder, err error) {
+func (s *StageExecutor) prepare(ctx context.Context, from string, initializeIBConfig, rebase, preserveBaseImageAnnotations bool, pullPolicy config.PullPolicy) (builder *buildah.Builder, err error) {
 	stage := s.stage
 	ib := stage.Builder
 	node := stage.Node
@@ -1076,7 +1076,7 @@ func (s *StageExecutor) Execute(ctx context.Context, base string) (imgID string,
 	var preserveBaseImageAnnotationsAtStageStart bool
 	if stageImage, isPreviousStage := s.executor.imageMap[base]; isPreviousStage {
 		base = stageImage
-		pullPolicy = define.PullNever
+		pullPolicy = config.PullPolicyNever
 		preserveBaseImageAnnotationsAtStageStart = true
 	}
 	s.executor.stagesLock.Unlock()
@@ -1655,7 +1655,7 @@ func (s *StageExecutor) Execute(ctx context.Context, base string) (imgID string,
 			// Enforce pull "never" since we already have an image
 			// ID that we really should not be pulling anymore (see
 			// containers/podman/issues/10307).
-			if _, err := s.prepare(ctx, imgID, false, true, true, define.PullNever); err != nil {
+			if _, err := s.prepare(ctx, imgID, false, true, true, config.PullPolicyNever); err != nil {
 				return "", nil, false, fmt.Errorf("preparing container for next step: %w", err)
 			}
 		}
@@ -2010,7 +2010,7 @@ func (s *StageExecutor) pullCache(ctx context.Context, cacheKey string) (referen
 			RetryDelay:          s.executor.retryPullPushDelay,
 			AllTags:             false,
 			ReportWriter:        nil,
-			PullPolicy:          define.PullIfNewer,
+			PullPolicy:          config.PullPolicyNewer,
 		}
 		id, err := buildah.Pull(ctx, src.DockerReference().String(), options)
 		if err != nil {

pkg/cli/common.go

@@ -262,7 +262,12 @@ func GetBudFlags(flags *BudResults) pflag.FlagSet {
 	fs.String("os", runtime.GOOS, "set the OS to the provided value instead of the current operating system of the host")
 	fs.StringArrayVar(&flags.OSFeatures, "os-feature", []string{}, "set required OS `feature` for the target image in addition to values from the base image")
 	fs.StringVar(&flags.OSVersion, "os-version", "", "set required OS `version` for the target image instead of the value from the base image")
-	fs.StringVar(&flags.Pull, "pull", "true", "pull base and SBOM scanner images from the registry if newer or not present in store, if false, only pull base and SBOM scanner images if not present, if always, pull base and SBOM scanner images even if the named images are present in store, if never, only use images present in store if available")
+	fs.StringVar(&flags.Pull, "pull", "missing", `pull base and SBOM scanner images from the registry. Values:
+true:    pull if newer or not present in store.
+false:   never pull base and SBOM scanner images.
+always:  pull base and SBOM scanner images even if the named images are present in store.
+missing: pull base and SBOM scanner images if the named images are not present in store.
+never:   only use images present in store if available`)
 	fs.Lookup("pull").NoOptDefVal = "true" //allow `--pull ` to be set to `true` as expected.
 	fs.BoolVar(&flags.PullAlways, "pull-always", false, "pull the image even if the named image is present in store")
 	if err := fs.MarkHidden("pull-always"); err != nil {

pkg/parse/parse.go

@@ -449,13 +449,13 @@ func SystemContextFromFlagSet(flags *pflag.FlagSet, findFlagFunc func(name strin
 
 // PullPolicyFromOptions returns a PullPolicy that reflects the combination of
 // the specified "pull" and undocumented "pull-always" and "pull-never" flags.
-func PullPolicyFromOptions(c *cobra.Command) (define.PullPolicy, error) {
+func PullPolicyFromOptions(c *cobra.Command) (config.PullPolicy, error) {
 	return PullPolicyFromFlagSet(c.Flags(), c.Flag)
 }
 
 // PullPolicyFromFlagSet returns a PullPolicy that reflects the combination of
 // the specified "pull" and undocumented "pull-always" and "pull-never" flags.
-func PullPolicyFromFlagSet(flags *pflag.FlagSet, findFlagFunc func(name string) *pflag.Flag) (define.PullPolicy, error) {
+func PullPolicyFromFlagSet(flags *pflag.FlagSet, findFlagFunc func(name string) *pflag.Flag) (config.PullPolicy, error) {
 	pullFlagsCount := 0
 
 	if findFlagFunc("pull").Changed {
@@ -475,30 +475,28 @@ func PullPolicyFromFlagSet(flags *pflag.FlagSet, findFlagFunc func(name string)
 	// Allow for --pull, --pull=true, --pull=false, --pull=never, --pull=always
 	// --pull-always and --pull-never.  The --pull-never and --pull-always options
 	// will not be documented.
-	pullPolicy := define.PullIfMissing
+
 	pullFlagValue := findFlagFunc("pull").Value.String()
-	if strings.EqualFold(pullFlagValue, "true") || strings.EqualFold(pullFlagValue, "ifnewer") {
-		pullPolicy = define.PullIfNewer
-	}
 	pullAlwaysFlagValue, err := flags.GetBool("pull-always")
 	if err != nil {
 		return 0, err
 	}
-	if pullAlwaysFlagValue || strings.EqualFold(pullFlagValue, "always") {
-		pullPolicy = define.PullAlways
+	if pullAlwaysFlagValue ||
+		strings.EqualFold(pullFlagValue, "true") {
+		return config.PullPolicyAlways, nil
 	}
+
 	pullNeverFlagValue, err := flags.GetBool("pull-never")
 	if err != nil {
 		return 0, err
 	}
+
 	if pullNeverFlagValue ||
-		strings.EqualFold(pullFlagValue, "never") ||
 		strings.EqualFold(pullFlagValue, "false") {
-		pullPolicy = define.PullNever
+		return config.PullPolicyNever, nil
 	}
-	logrus.Debugf("Pull Policy for pull [%v]", pullPolicy)
 
-	return pullPolicy, nil
+	return config.ParsePullPolicy(pullFlagValue)
 }
 
 func getAuthFile(authfile string) string {