container create: fix --tls-verify parsing

Make sure that the value is only set if specified on the CLI.  c/image
already defaults to true but if set in the system context, we'd skip
settings in the registries.conf.

Backport of commit ff31f2264da.

Fixes: #11933
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>

11 files changed, 205 insertions, 14 deletions

diff --git a/cmd/podman/common/create.go b/cmd/podman/common/create.go
index 643f6728a..910aae6f0 100644
--- a/cmd/podman/common/create.go
+++ b/cmd/podman/common/create.go
@@ -5,6 +5,7 @@ import (
 
 	"github.com/containers/common/pkg/auth"
 	"github.com/containers/common/pkg/completion"
+	commonFlag "github.com/containers/common/pkg/flag"
 	"github.com/containers/podman/v3/cmd/podman/registry"
 	"github.com/containers/podman/v3/libpod/define"
 	"github.com/containers/podman/v3/pkg/domain/entities"
@@ -606,12 +607,9 @@ func DefineCreateFlags(cmd *cobra.Command, cf *entities.ContainerCreateOptions,
 		)
 		_ = cmd.RegisterFlagCompletionFunc(timeoutFlagName, completion.AutocompleteNone)
 
-		// Flag for TLS verification, so that `run` and `create` commands can make use of it.
-		// Make sure to use `=` while using this flag i.e `--tls-verify=false/true`
-		tlsVerifyFlagName := "tls-verify"
-		createFlags.BoolVar(
+		commonFlag.OptionalBoolFlag(createFlags,
 			&cf.TLSVerify,
-			tlsVerifyFlagName, true,
+			"tls-verify",
 			"Require HTTPS and verify certificates when contacting registries for pulling images",
 		)
 
diff --git a/cmd/podman/containers/create.go b/cmd/podman/containers/create.go
index aa34f9ba5..8bbdfa871 100644
--- a/cmd/podman/containers/create.go
+++ b/cmd/podman/containers/create.go
@@ -292,6 +292,11 @@ func PullImage(imageName string, cliVals entities.ContainerCreateOptions) (strin
 		}
 	}
 
+	skipTLSVerify := types.OptionalBoolUndefined
+	if cliVals.TLSVerify.Present() {
+		skipTLSVerify = types.NewOptionalBool(!cliVals.TLSVerify.Value())
+	}
+
 	pullReport, pullErr := registry.ImageEngine().Pull(registry.GetContext(), imageName, entities.ImagePullOptions{
 		Authfile:        cliVals.Authfile,
 		Quiet:           cliVals.Quiet,
@@ -300,7 +305,7 @@ func PullImage(imageName string, cliVals entities.ContainerCreateOptions) (strin
 		Variant:         cliVals.Variant,
 		SignaturePolicy: cliVals.SignaturePolicy,
 		PullPolicy:      pullPolicy,
-		SkipTLSVerify:   types.NewOptionalBool(!cliVals.TLSVerify), // If Flag changed for TLS Verification
+		SkipTLSVerify:   skipTLSVerify,
 	})
 	if pullErr != nil {
 		return "", pullErr
diff --git a/go.mod b/go.mod
index 5808d70d7..8521f89f7 100644
--- a/go.mod
+++ b/go.mod
@@ -12,7 +12,7 @@ require (
 	github.com/containernetworking/cni v0.8.1
 	github.com/containernetworking/plugins v0.9.1
 	github.com/containers/buildah v1.23.1
-	github.com/containers/common v0.44.3
+	github.com/containers/common v0.44.4
 	github.com/containers/conmon v2.0.20+incompatible
 	github.com/containers/image/v5 v5.16.0
 	github.com/containers/ocicrypt v1.1.2
diff --git a/go.sum b/go.sum
index 952774d35..c74f49736 100644
--- a/go.sum
+++ b/go.sum
@@ -247,8 +247,8 @@ github.com/containernetworking/plugins v0.9.1/go.mod h1:xP/idU2ldlzN6m4p5LmGiwRD
 github.com/containers/buildah v1.23.1 h1:Tpc9DsRuU+0Oofewpxb6OJVNQjCu7yloN/obUqzfDTY=
 github.com/containers/buildah v1.23.1/go.mod h1:4WnrN0yrA7ab0ppgunixu2WM1rlD2rG8QLJAKbEkZlQ=
 github.com/containers/common v0.44.2/go.mod h1:7sdP4vmI5Bm6FPFxb3lvAh1Iktb6tiO1MzjUzhxdoGo=
-github.com/containers/common v0.44.3 h1:Wx+mJT+gH/ie86JdZUmVnZwTieXw86UE6JOYuCNTV1g=
-github.com/containers/common v0.44.3/go.mod h1:7sdP4vmI5Bm6FPFxb3lvAh1Iktb6tiO1MzjUzhxdoGo=
+github.com/containers/common v0.44.4 h1:R9ggz2RmbNzu7gdxBHMr4p57fywTwuoZ67jgjt8/RFg=
+github.com/containers/common v0.44.4/go.mod h1:7sdP4vmI5Bm6FPFxb3lvAh1Iktb6tiO1MzjUzhxdoGo=
 github.com/containers/conmon v2.0.20+incompatible h1:YbCVSFSCqFjjVwHTPINGdMX1F6JXHGTUje2ZYobNrkg=
 github.com/containers/conmon v2.0.20+incompatible/go.mod h1:hgwZ2mtuDrppv78a/cOBNiCm6O0UMWGx1mu7P00nu5I=
 github.com/containers/image/v5 v5.16.0 h1:WQcNSzb7+ngS2cfynx0vUwhk+scpgiKlldVcsF8GPbI=
diff --git a/pkg/domain/entities/pods.go b/pkg/domain/entities/pods.go
index d9dd0c532..8214af19c 100644
--- a/pkg/domain/entities/pods.go
+++ b/pkg/domain/entities/pods.go
@@ -5,6 +5,7 @@ import (
 	"strings"
 	"time"
 
+	commonFlag "github.com/containers/common/pkg/flag"
 	"github.com/containers/podman/v3/libpod/define"
 	"github.com/containers/podman/v3/pkg/specgen"
 	"github.com/containers/podman/v3/pkg/util"
@@ -235,7 +236,7 @@ type ContainerCreateOptions struct {
 	Sysctl            []string
 	Systemd           string
 	Timeout           uint
-	TLSVerify         bool
+	TLSVerify         commonFlag.OptionalBool
 	TmpFS             []string
 	TTY               bool
 	Timezone          string
diff --git a/test/e2e/push_test.go b/test/e2e/push_test.go
index b7e8309fb..7b35acd35 100644
--- a/test/e2e/push_test.go
+++ b/test/e2e/push_test.go
@@ -146,7 +146,7 @@ var _ = Describe("Podman push", func() {
 		session = podmanTest.Podman([]string{"logs", "registry"})
 		session.WaitWithDefaultTimeout()
 
-		push := podmanTest.Podman([]string{"push", "--format=v2s2", "--creds=podmantest:test", ALPINE, "localhost:5000/tlstest"})
+		push := podmanTest.Podman([]string{"push", "--tls-verify=true", "--format=v2s2", "--creds=podmantest:test", ALPINE, "localhost:5000/tlstest"})
 		push.WaitWithDefaultTimeout()
 		Expect(push).To(ExitWithError())
 
@@ -163,7 +163,7 @@ var _ = Describe("Podman push", func() {
 
 		if !IsRemote() {
 			// remote does not support --cert-dir
-			push = podmanTest.Podman([]string{"push", "--creds=podmantest:test", "--cert-dir=fakedir", ALPINE, "localhost:5000/certdirtest"})
+			push = podmanTest.Podman([]string{"push", "--tls-verify=true", "--creds=podmantest:test", "--cert-dir=fakedir", ALPINE, "localhost:5000/certdirtest"})
 			push.WaitWithDefaultTimeout()
 			Expect(push).To(ExitWithError())
 		}
diff --git a/test/e2e/run_test.go b/test/e2e/run_test.go
index 8f640eacf..a5a0aad8b 100644
--- a/test/e2e/run_test.go
+++ b/test/e2e/run_test.go
@@ -186,6 +186,12 @@ var _ = Describe("Podman run", func() {
 		run.WaitWithDefaultTimeout()
 		Expect(run).Should(Exit(0))
 		Expect(podmanTest.NumberOfContainers()).To(Equal(3))
+
+		// Now registries.conf will be consulted where localhost:5000
+		// is set to be insecure.
+		run = podmanTest.Podman([]string{"run", ALPINE})
+		run.WaitWithDefaultTimeout()
+		Expect(run).Should(Exit(0))
 	})
 
 	It("podman run a container with a --rootfs", func() {
diff --git a/test/registries.conf b/test/registries.conf
index 0559c9e52..8e4671760 100644
--- a/test/registries.conf
+++ b/test/registries.conf
@@ -15,3 +15,9 @@ location="mirror.gcr.io"
 [[registry]]
 prefix="docker.io/library"
 location="quay.io/libpod"
+
+# For testing #11933 to make sure that registries.conf is consulted unless
+# --tls-verify is used during container creation.
+[[registry]]
+location="localhost:5000"
+insecure=true
diff --git a/vendor/github.com/containers/common/pkg/flag/flag.go b/vendor/github.com/containers/common/pkg/flag/flag.go
new file mode 100644
index 000000000..52eb50da0
--- /dev/null
+++ b/vendor/github.com/containers/common/pkg/flag/flag.go
@@ -0,0 +1,174 @@
+package flag
+
+import (
+	"strconv"
+
+	"github.com/spf13/pflag"
+)
+
+// OptionalBool is a boolean with a separate presence flag and value.
+type OptionalBool struct {
+	present bool
+	value   bool
+}
+
+// Present returns the bool's presence flag.
+func (ob *OptionalBool) Present() bool {
+	return ob.present
+}
+
+// Present returns the bool's value. Should only be used if Present() is true.
+func (ob *OptionalBool) Value() bool {
+	return ob.value
+}
+
+// optionalBool is a cli.Generic == flag.Value implementation equivalent to
+// the one underlying flag.Bool, except that it records whether the flag has been set.
+// This is distinct from optionalBool to (pretend to) force callers to use
+// optionalBoolFlag
+type optionalBoolValue OptionalBool
+
+// OptionalBoolFlag creates new flag for an optional in the specified flag with
+// the specified name and usage.
+func OptionalBoolFlag(fs *pflag.FlagSet, p *OptionalBool, name, usage string) *pflag.Flag {
+	flag := fs.VarPF(internalNewOptionalBoolValue(p), name, "", usage)
+	flag.NoOptDefVal = "true"
+	flag.DefValue = "false"
+	return flag
+}
+
+// WARNING: Do not directly use this method to define optionalBool flag.
+// Caller should use optionalBoolFlag
+func internalNewOptionalBoolValue(p *OptionalBool) pflag.Value {
+	p.present = false
+	return (*optionalBoolValue)(p)
+}
+
+// Set parses the string to a bool and sets it.
+func (ob *optionalBoolValue) Set(s string) error {
+	v, err := strconv.ParseBool(s)
+	if err != nil {
+		return err
+	}
+	ob.value = v
+	ob.present = true
+	return nil
+}
+
+// String returns the string representation of the string.
+func (ob *optionalBoolValue) String() string {
+	if !ob.present {
+		return "" // This is, sadly, not round-trip safe: --flag is interpreted as --flag=true
+	}
+	return strconv.FormatBool(ob.value)
+}
+
+// Type returns the type.
+func (ob *optionalBoolValue) Type() string {
+	return "bool"
+}
+
+// IsBoolFlag indicates that it's a bool flag.
+func (ob *optionalBoolValue) IsBoolFlag() bool {
+	return true
+}
+
+// OptionalString is a string with a separate presence flag.
+type OptionalString struct {
+	present bool
+	value   string
+}
+
+// Present returns the strings's presence flag.
+func (os *OptionalString) Present() bool {
+	return os.present
+}
+
+// Present returns the string's value. Should only be used if Present() is true.
+func (os *OptionalString) Value() string {
+	return os.value
+}
+
+// optionalString is a cli.Generic == flag.Value implementation equivalent to
+// the one underlying flag.String, except that it records whether the flag has been set.
+// This is distinct from optionalString to (pretend to) force callers to use
+// newoptionalString
+type optionalStringValue OptionalString
+
+// NewOptionalStringValue returns a pflag.Value fo the string.
+func NewOptionalStringValue(p *OptionalString) pflag.Value {
+	p.present = false
+	return (*optionalStringValue)(p)
+}
+
+// Set sets the string.
+func (ob *optionalStringValue) Set(s string) error {
+	ob.value = s
+	ob.present = true
+	return nil
+}
+
+// String returns the string if present.
+func (ob *optionalStringValue) String() string {
+	if !ob.present {
+		return "" // This is, sadly, not round-trip safe: --flag= is interpreted as {present:true, value:""}
+	}
+	return ob.value
+}
+
+// Type returns the string type.
+func (ob *optionalStringValue) Type() string {
+	return "string"
+}
+
+// OptionalInt is a int with a separate presence flag.
+type OptionalInt struct {
+	present bool
+	value   int
+}
+
+// Present returns the int's presence flag.
+func (oi *OptionalInt) Present() bool {
+	return oi.present
+}
+
+// Present returns the int's value. Should only be used if Present() is true.
+func (oi *OptionalInt) Value() int {
+	return oi.value
+}
+
+// optionalInt is a cli.Generic == flag.Value implementation equivalent to
+// the one underlying flag.Int, except that it records whether the flag has been set.
+// This is distinct from optionalInt to (pretend to) force callers to use
+// newoptionalIntValue
+type optionalIntValue OptionalInt
+
+// NewOptionalIntValue returns the pflag.Value of the int.
+func NewOptionalIntValue(p *OptionalInt) pflag.Value {
+	p.present = false
+	return (*optionalIntValue)(p)
+}
+
+// Set parses the string to an int and sets it.
+func (ob *optionalIntValue) Set(s string) error {
+	v, err := strconv.ParseInt(s, 0, strconv.IntSize)
+	if err != nil {
+		return err
+	}
+	ob.value = int(v)
+	ob.present = true
+	return nil
+}
+
+// String returns the string representation of the int.
+func (ob *optionalIntValue) String() string {
+	if !ob.present {
+		return "" // If the value is not present, just return an empty string, any other value wouldn't make sense.
+	}
+	return strconv.Itoa(int(ob.value))
+}
+
+// Type returns the int's type.
+func (ob *optionalIntValue) Type() string {
+	return "int"
+}
diff --git a/vendor/github.com/containers/common/version/version.go b/vendor/github.com/containers/common/version/version.go
index 4d122058d..f9c779e87 100644
--- a/vendor/github.com/containers/common/version/version.go
+++ b/vendor/github.com/containers/common/version/version.go
@@ -1,4 +1,4 @@
 package version
 
 // Version is the version of the build.
-const Version = "0.44.3"
+const Version = "0.44.4"
diff --git a/vendor/modules.txt b/vendor/modules.txt
index fad1d5093..8d26938b8 100644
--- a/vendor/modules.txt
+++ b/vendor/modules.txt
@@ -99,7 +99,7 @@ github.com/containers/buildah/pkg/rusage
 github.com/containers/buildah/pkg/sshagent
 github.com/containers/buildah/pkg/util
 github.com/containers/buildah/util
-# github.com/containers/common v0.44.3
+# github.com/containers/common v0.44.4
 github.com/containers/common/libimage
 github.com/containers/common/libimage/manifests
 github.com/containers/common/pkg/apparmor
@@ -112,6 +112,7 @@ github.com/containers/common/pkg/completion
 github.com/containers/common/pkg/config
 github.com/containers/common/pkg/defaultnet
 github.com/containers/common/pkg/filters
+github.com/containers/common/pkg/flag
 github.com/containers/common/pkg/manifests
 github.com/containers/common/pkg/parse
 github.com/containers/common/pkg/report