Clarify definition of --pull options

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

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", "true", `pull images from the registry values:
+true: pull if newer or not present in store,
+false: never pull images even if not present,
+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`)
 	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

@@ -751,7 +751,7 @@ 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.
+base or SBOM scanner image is not present locally. Equivalent to *--pull=never*.
 
 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
@@ -764,7 +764,7 @@ 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.
+error if the image is not present locally. Equivalent to *--pull=false*.
 
 Defaults to *true*.
 

docs/buildah-from.1.md

@@ -379,15 +379,15 @@ registry and is not present locally.
 
 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.
+present locally. Equivalent to *--pull=never*.
 
 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.
+if the image is not present locally. Equivalent to *--pull=false*.
 
 Defaults to *true*.
 

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", "true", `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,33 @@ 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
+		return config.PullPolicyNewer, nil
 	}
 	pullAlwaysFlagValue, err := flags.GetBool("pull-always")
 	if err != nil {
 		return 0, err
 	}
 	if pullAlwaysFlagValue || strings.EqualFold(pullFlagValue, "always") {
-		pullPolicy = define.PullAlways
+		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
+	}
+
+	if pullFlagValue == "" {
+		return config.PullPolicyMissing, nil
 	}
-	logrus.Debugf("Pull Policy for pull [%v]", pullPolicy)
 
-	return pullPolicy, nil
+	return config.ParsePullPolicy(pullFlagValue)
 }
 
 func getAuthFile(authfile string) string {

pull.go

@@ -6,7 +6,6 @@ import (
 	"io"
 	"time"
 
-	"github.com/containers/buildah/define"
 	"github.com/containers/common/libimage"
 	"github.com/containers/common/pkg/config"
 	"github.com/containers/image/v5/types"
@@ -49,7 +48,7 @@ type PullOptions struct {
 	// encrypted if non-nil. If nil, it does not attempt to decrypt an image.
 	OciDecryptConfig *encconfig.DecryptConfig
 	// PullPolicy takes the value PullIfMissing, PullAlways, PullIfNewer, or PullNever.
-	PullPolicy define.PullPolicy
+	PullPolicy config.PullPolicy
 }
 
 // Pull copies the contents of the image from somewhere else to local storage.  Returns the

tests/bud.bats

@@ -4191,6 +4191,9 @@ _EOF
   run_buildah 125 build $WITH_POLICY_JSON -t ${target} --pull=false $BUDFILES/pull
   expect_output --substring "busybox: image not known"
 
+  run_buildah build $WITH_POLICY_JSON -t ${target} --pull=missing $BUDFILES/pull
+  expect_output --substring "COMMIT pull"
+
   run_buildah build $WITH_POLICY_JSON -t ${target} --pull $BUDFILES/pull
   expect_output --substring "COMMIT pull"