Merge pull request containers#840 from vrothberg/backport-podman-fix-…

…11933

Backport podman fix 11933

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"
+}

pkg/flag/flag_test.go

@@ -0,0 +1,222 @@
+package flag
+
+import (
+	"testing"
+
+	"github.com/spf13/cobra"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestOptionalBoolSet(t *testing.T) {
+	for _, c := range []struct {
+		input    string
+		accepted bool
+		value    bool
+	}{
+		// Valid inputs documented for strconv.ParseBool == flag.BoolVar
+		{"1", true, true},
+		{"t", true, true},
+		{"T", true, true},
+		{"TRUE", true, true},
+		{"true", true, true},
+		{"True", true, true},
+		{"0", true, false},
+		{"f", true, false},
+		{"F", true, false},
+		{"FALSE", true, false},
+		{"false", true, false},
+		{"False", true, false},
+		// A few invalid inputs
+		{"", false, false},
+		{"yes", false, false},
+		{"no", false, false},
+		{"2", false, false},
+	} {
+		var ob OptionalBool
+		v := internalNewOptionalBoolValue(&ob)
+		require.False(t, ob.Present())
+		err := v.Set(c.input)
+		if c.accepted {
+			assert.NoError(t, err, c.input)
+			assert.Equal(t, c.value, ob.Value())
+		} else {
+			assert.Error(t, err, c.input)
+			assert.False(t, ob.Present()) // Just to be extra paranoid.
+		}
+	}
+
+	// Nothing actually explicitly says that .Set() is never called when the flag is not present on the command line;
+	// so, check that it is not being called, at least in the straightforward case (it's not possible to test that it
+	// is not called in any possible situation).
+	var globalOB, commandOB OptionalBool
+	actionRun := false
+	app := &cobra.Command{
+		Use: "app",
+	}
+	OptionalBoolFlag(app.PersistentFlags(), &globalOB, "global-OB", "")
+	cmd := &cobra.Command{
+		Use: "cmd",
+		RunE: func(cmd *cobra.Command, args []string) error {
+			assert.False(t, globalOB.Present())
+			assert.False(t, commandOB.Present())
+			actionRun = true
+			return nil
+		},
+	}
+	OptionalBoolFlag(cmd.Flags(), &commandOB, "command-OB", "")
+	app.AddCommand(cmd)
+	app.SetArgs([]string{"cmd"})
+	err := app.Execute()
+	require.NoError(t, err)
+	assert.True(t, actionRun)
+}
+
+func TestOptionalBoolString(t *testing.T) {
+	for _, c := range []struct {
+		input    OptionalBool
+		expected string
+	}{
+		{OptionalBool{present: true, value: true}, "true"},
+		{OptionalBool{present: true, value: false}, "false"},
+		{OptionalBool{present: false, value: true}, ""},
+		{OptionalBool{present: false, value: false}, ""},
+	} {
+		var ob OptionalBool
+		v := internalNewOptionalBoolValue(&ob)
+		ob = c.input
+		res := v.String()
+		assert.Equal(t, c.expected, res)
+	}
+}
+
+func TestOptionalBoolIsBoolFlag(t *testing.T) {
+	// IsBoolFlag means that the argument value must either be part of the same argument, with =;
+	// if there is no =, the value is set to true.
+	// This differs form other flags, where the argument is required and may be either separated with = or supplied in the next argument.
+	for _, c := range []struct {
+		input        []string
+		expectedOB   OptionalBool
+		expectedArgs []string
+	}{
+		{[]string{"1", "2"}, OptionalBool{present: false}, []string{"1", "2"}},                                       // Flag not present
+		{[]string{"--OB=true", "1", "2"}, OptionalBool{present: true, value: true}, []string{"1", "2"}},              // --OB=true
+		{[]string{"--OB=false", "1", "2"}, OptionalBool{present: true, value: false}, []string{"1", "2"}},            // --OB=false
+		{[]string{"--OB", "true", "1", "2"}, OptionalBool{present: true, value: true}, []string{"true", "1", "2"}},   // --OB true
+		{[]string{"--OB", "false", "1", "2"}, OptionalBool{present: true, value: true}, []string{"false", "1", "2"}}, // --OB false
+	} {
+		var ob OptionalBool
+		actionRun := false
+		app := &cobra.Command{Use: "app"}
+		cmd := &cobra.Command{
+			Use: "cmd",
+			RunE: func(cmd *cobra.Command, args []string) error {
+				assert.Equal(t, c.expectedOB, ob)     // nolint
+				assert.Equal(t, c.expectedArgs, args) //nolint
+				actionRun = true
+				return nil
+			},
+		}
+		OptionalBoolFlag(cmd.Flags(), &ob, "OB", "")
+		app.AddCommand(cmd)
+
+		app.SetArgs(append([]string{"cmd"}, c.input...))
+		err := app.Execute()
+		require.NoError(t, err)
+		assert.True(t, actionRun)
+	}
+}
+
+func TestOptionalStringSet(t *testing.T) {
+	// Really just a smoke test, but differentiating between not present and empty.
+	for _, c := range []string{"", "hello"} {
+		var os OptionalString
+		v := NewOptionalStringValue(&os)
+		require.False(t, os.Present())
+		err := v.Set(c)
+		assert.NoError(t, err, c)
+		assert.Equal(t, c, os.Value())
+	}
+
+	// Nothing actually explicitly says that .Set() is never called when the flag is not present on the command line;
+	// so, check that it is not being called, at least in the straightforward case (it's not possible to test that it
+	// is not called in any possible situation).
+	var globalOS, commandOS OptionalString
+	actionRun := false
+	app := &cobra.Command{
+		Use: "app",
+	}
+	app.PersistentFlags().Var(NewOptionalStringValue(&globalOS), "global-OS", "")
+	cmd := &cobra.Command{
+		Use: "cmd",
+		RunE: func(cmd *cobra.Command, args []string) error {
+			assert.False(t, globalOS.Present())
+			assert.False(t, commandOS.Present())
+			actionRun = true
+			return nil
+		},
+	}
+	cmd.Flags().Var(NewOptionalStringValue(&commandOS), "command-OS", "")
+	app.AddCommand(cmd)
+	app.SetArgs([]string{"cmd"})
+	err := app.Execute()
+	require.NoError(t, err)
+	assert.True(t, actionRun)
+}
+
+func TestOptionalStringString(t *testing.T) {
+	for _, c := range []struct {
+		input    OptionalString
+		expected string
+	}{
+		{OptionalString{present: true, value: "hello"}, "hello"},
+		{OptionalString{present: true, value: ""}, ""},
+		{OptionalString{present: false, value: "hello"}, ""},
+		{OptionalString{present: false, value: ""}, ""},
+	} {
+		var os OptionalString
+		v := NewOptionalStringValue(&os)
+		os = c.input
+		res := v.String()
+		assert.Equal(t, c.expected, res)
+	}
+}
+
+func TestOptionalStringIsBoolFlag(t *testing.T) {
+	// NOTE: OptionalStringValue does not implement IsBoolFlag!
+	// IsBoolFlag means that the argument value must either be part of the same argument, with =;
+	// if there is no =, the value is set to true.
+	// This differs form other flags, where the argument is required and may be either separated with = or supplied in the next argument.
+	for _, c := range []struct {
+		input        []string
+		expectedOS   OptionalString
+		expectedArgs []string
+	}{
+		{[]string{"1", "2"}, OptionalString{present: false}, []string{"1", "2"}},                                 // Flag not present
+		{[]string{"--OS=hello", "1", "2"}, OptionalString{present: true, value: "hello"}, []string{"1", "2"}},    // --OS=true
+		{[]string{"--OS=", "1", "2"}, OptionalString{present: true, value: ""}, []string{"1", "2"}},              // --OS=false
+		{[]string{"--OS", "hello", "1", "2"}, OptionalString{present: true, value: "hello"}, []string{"1", "2"}}, // --OS true
+		{[]string{"--OS", "", "1", "2"}, OptionalString{present: true, value: ""}, []string{"1", "2"}},           // --OS false
+	} {
+		var os OptionalString
+		actionRun := false
+		app := &cobra.Command{
+			Use: "app",
+		}
+		cmd := &cobra.Command{
+			Use: "cmd",
+			RunE: func(cmd *cobra.Command, args []string) error {
+				assert.Equal(t, c.expectedOS, os)     // nolint
+				assert.Equal(t, c.expectedArgs, args) // nolint
+				actionRun = true
+				return nil
+			},
+		}
+		cmd.Flags().Var(NewOptionalStringValue(&os), "OS", "")
+		app.AddCommand(cmd)
+		app.SetArgs(append([]string{"cmd"}, c.input...))
+		err := app.Execute()
+		require.NoError(t, err)
+		assert.True(t, actionRun)
+	}
+}