Mark some of the option fields as ignored in pkg/bindings

I realized that `params.Del("SkipTLSVerify")` doesn't have any
effect because keys are always lowercased. So it should really
be `params.Del("skiptlsverify")`.

There's also a little bug introduced by 3bf52aa and b1d1248: if
one passes `ProgressWriter` object having `Stringer` interface
i.e. `bytes.Buffer` it ends up been serialized in query with
`util.ToParams()`.

To circumvent both problems I propose to mark non-serializable
parameters with `schema:"-"` so there's no need to delete them from
resulting `url.Values`.

Signed-off-by: Vladimir Kochnev <hashtable@yandex.ru>

10 files changed, 90 insertions, 22 deletions

diff --git a/pkg/bindings/images/images.go b/pkg/bindings/images/images.go
index bb7867c4e..ea7d445db 100644
--- a/pkg/bindings/images/images.go
+++ b/pkg/bindings/images/images.go
@@ -282,9 +282,9 @@ func Search(ctx context.Context, term string, options *SearchOptions) ([]entitie
 	}
 	params.Set("term", term)
 
-	// Note: we have to verify if skipped is false.
+	// SkipTLSVerify is special.  It's not being serialized by ToParams()
+	// because we need to flip the boolean.
 	if options.SkipTLSVerify != nil {
-		params.Del("SkipTLSVerify")
 		params.Set("tlsVerify", strconv.FormatBool(!options.GetSkipTLSVerify()))
 	}
 
diff --git a/pkg/bindings/images/pull.go b/pkg/bindings/images/pull.go
index 109981c63..8caf45c0e 100644
--- a/pkg/bindings/images/pull.go
+++ b/pkg/bindings/images/pull.go
@@ -35,9 +35,9 @@ func Pull(ctx context.Context, rawImage string, options *PullOptions) ([]string,
 	}
 	params.Set("reference", rawImage)
 
+	// SkipTLSVerify is special.  It's not being serialized by ToParams()
+	// because we need to flip the boolean.
 	if options.SkipTLSVerify != nil {
-		params.Del("SkipTLSVerify")
-		// Note: we have to verify if skipped is false.
 		params.Set("tlsVerify", strconv.FormatBool(!options.GetSkipTLSVerify()))
 	}
 
diff --git a/pkg/bindings/images/push.go b/pkg/bindings/images/push.go
index f1e059f8c..0e1309e91 100644
--- a/pkg/bindings/images/push.go
+++ b/pkg/bindings/images/push.go
@@ -38,10 +38,9 @@ func Push(ctx context.Context, source string, destination string, options *PushO
 	if err != nil {
 		return err
 	}
-	// SkipTLSVerify is special.  We need to delete the param added by
-	// toparams and change the key and flip the bool
+	// SkipTLSVerify is special.  It's not being serialized by ToParams()
+	// because we need to flip the boolean.
 	if options.SkipTLSVerify != nil {
-		params.Del("SkipTLSVerify")
 		params.Set("tlsVerify", strconv.FormatBool(!options.GetSkipTLSVerify()))
 	}
 	params.Set("destination", destination)
diff --git a/pkg/bindings/images/types.go b/pkg/bindings/images/types.go
index 3ecfb9e09..f8630926e 100644
--- a/pkg/bindings/images/types.go
+++ b/pkg/bindings/images/types.go
@@ -136,9 +136,9 @@ type PushOptions struct {
 	// ProgressWriter is a writer where push progress are sent.
 	// Since API handler for image push is quiet by default, WithQuiet(false) is necessary for
 	// the writer to receive progress messages.
-	ProgressWriter *io.Writer
+	ProgressWriter *io.Writer `schema:"-"`
 	// SkipTLSVerify to skip HTTPS and certificate verification.
-	SkipTLSVerify *bool
+	SkipTLSVerify *bool `schema:"-"`
 	// RemoveSignatures Discard any pre-existing signatures in the image.
 	RemoveSignatures *bool
 	// Username for authenticating against the registry.
@@ -158,7 +158,7 @@ type SearchOptions struct {
 	// Limit the number of results.
 	Limit *int
 	// SkipTLSVerify to skip  HTTPS and certificate verification.
-	SkipTLSVerify *bool
+	SkipTLSVerify *bool `schema:"-"`
 	// ListTags search the available tags of the repository
 	ListTags *bool
 }
@@ -183,12 +183,12 @@ type PullOptions struct {
 	// Password for authenticating against the registry.
 	Password *string
 	// ProgressWriter is a writer where pull progress are sent.
-	ProgressWriter *io.Writer
+	ProgressWriter *io.Writer `schema:"-"`
 	// Quiet can be specified to suppress pull progress when pulling.  Ignored
 	// for remote calls.
 	Quiet *bool
 	// SkipTLSVerify to skip HTTPS and certificate verification.
-	SkipTLSVerify *bool
+	SkipTLSVerify *bool `schema:"-"`
 	// Username for authenticating against the registry.
 	Username *string
 	// Variant will overwrite the local variant for image pulls.
diff --git a/pkg/bindings/internal/util/util.go b/pkg/bindings/internal/util/util.go
index f8f99d6c1..52ce14738 100644
--- a/pkg/bindings/internal/util/util.go
+++ b/pkg/bindings/internal/util/util.go
@@ -74,6 +74,9 @@ func ToParams(o interface{}) (url.Values, error) {
 		}
 		paramName := fieldName
 		if pn, ok := sType.Field(i).Tag.Lookup("schema"); ok {
+			if pn == "-" {
+				continue
+			}
 			paramName = pn
 		}
 		switch {
diff --git a/pkg/bindings/kube/kube.go b/pkg/bindings/kube/kube.go
index e727439cf..1b9f888ef 100644
--- a/pkg/bindings/kube/kube.go
+++ b/pkg/bindings/kube/kube.go
@@ -40,8 +40,10 @@ func PlayWithBody(ctx context.Context, body io.Reader, options *PlayOptions) (*e
 	if err != nil {
 		return nil, err
 	}
+	// SkipTLSVerify is special.  It's not being serialized by ToParams()
+	// because we need to flip the boolean.
 	if options.SkipTLSVerify != nil {
-		params.Set("tlsVerify", strconv.FormatBool(options.GetSkipTLSVerify()))
+		params.Set("tlsVerify", strconv.FormatBool(!options.GetSkipTLSVerify()))
 	}
 	if options.Start != nil {
 		params.Set("start", strconv.FormatBool(options.GetStart()))
diff --git a/pkg/bindings/kube/types.go b/pkg/bindings/kube/types.go
index 783d1912a..279a9f8f3 100644
--- a/pkg/bindings/kube/types.go
+++ b/pkg/bindings/kube/types.go
@@ -27,7 +27,7 @@ type PlayOptions struct {
 	SignaturePolicy *string
 	// SkipTLSVerify - skip https and certificate validation when
 	// contacting container registries.
-	SkipTLSVerify *bool
+	SkipTLSVerify *bool `schema:"-"`
 	// SeccompProfileRoot - path to a directory containing seccomp
 	// profiles.
 	SeccompProfileRoot *string
diff --git a/pkg/bindings/manifests/manifests.go b/pkg/bindings/manifests/manifests.go
index 0163d21a0..752366937 100644
--- a/pkg/bindings/manifests/manifests.go
+++ b/pkg/bindings/manifests/manifests.go
@@ -165,10 +165,9 @@ func Push(ctx context.Context, name, destination string, options *images.PushOpt
 	if err != nil {
 		return "", err
 	}
-	// SkipTLSVerify is special.  We need to delete the param added by
-	// ToParams() and change the key and flip the bool
+	// SkipTLSVerify is special.  It's not being serialized by ToParams()
+	// because we need to flip the boolean.
 	if options.SkipTLSVerify != nil {
-		params.Del("SkipTLSVerify")
 		params.Set("tlsVerify", strconv.FormatBool(!options.GetSkipTLSVerify()))
 	}
 
@@ -246,10 +245,9 @@ func Modify(ctx context.Context, name string, images []string, options *ModifyOp
 	if err != nil {
 		return "", err
 	}
-	// SkipTLSVerify is special.  We need to delete the param added by
-	// ToParams() and change the key and flip the bool
+	// SkipTLSVerify is special.  It's not being serialized by ToParams()
+	// because we need to flip the boolean.
 	if options.SkipTLSVerify != nil {
-		params.Del("SkipTLSVerify")
 		params.Set("tlsVerify", strconv.FormatBool(!options.GetSkipTLSVerify()))
 	}
 
diff --git a/pkg/bindings/manifests/types.go b/pkg/bindings/manifests/types.go
index 5f2557fe1..fec3f9d13 100644
--- a/pkg/bindings/manifests/types.go
+++ b/pkg/bindings/manifests/types.go
@@ -32,7 +32,7 @@ type AddOptions struct {
 	Authfile      *string
 	Password      *string
 	Username      *string
-	SkipTLSVerify *bool
+	SkipTLSVerify *bool `schema:"-"`
 }
 
 //go:generate go run ../generator/generator.go RemoveOptions
@@ -60,5 +60,5 @@ type ModifyOptions struct {
 	Authfile      *string
 	Password      *string
 	Username      *string
-	SkipTLSVerify *bool
+	SkipTLSVerify *bool `schema:"-"`
 }
diff --git a/pkg/bindings/test/types_test.go b/pkg/bindings/test/types_test.go
new file mode 100644
index 000000000..bc98c8b7d
--- /dev/null
+++ b/pkg/bindings/test/types_test.go
@@ -0,0 +1,66 @@
+package bindings_test
+
+import (
+	"bytes"
+
+	"github.com/containers/podman/v4/pkg/bindings/images"
+	"github.com/containers/podman/v4/pkg/bindings/kube"
+	"github.com/containers/podman/v4/pkg/bindings/manifests"
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("Binding types", func() {
+	It("serialize image pull options", func() {
+		var writer bytes.Buffer
+		opts := new(images.PullOptions).WithOS("foo").WithProgressWriter(&writer).WithSkipTLSVerify(true)
+		params, err := opts.ToParams()
+		Expect(err).ToNot(HaveOccurred())
+		Expect(params.Get("os")).To(Equal("foo"))
+		Expect(params.Has("progresswriter")).To(BeFalse())
+		Expect(params.Has("skiptlsverify")).To(BeFalse())
+	})
+
+	It("serialize image push options", func() {
+		var writer bytes.Buffer
+		opts := new(images.PushOptions).WithAll(true).WithProgressWriter(&writer).WithSkipTLSVerify(true)
+		params, err := opts.ToParams()
+		Expect(err).ToNot(HaveOccurred())
+		Expect(params.Get("all")).To(Equal("true"))
+		Expect(params.Has("progresswriter")).To(BeFalse())
+		Expect(params.Has("skiptlsverify")).To(BeFalse())
+	})
+
+	It("serialize image search options", func() {
+		opts := new(images.SearchOptions).WithLimit(123).WithSkipTLSVerify(true)
+		params, err := opts.ToParams()
+		Expect(err).ToNot(HaveOccurred())
+		Expect(params.Get("limit")).To(Equal("123"))
+		Expect(params.Has("skiptlsverify")).To(BeFalse())
+	})
+
+	It("serialize manifest modify options", func() {
+		opts := new(manifests.ModifyOptions).WithOS("foo").WithSkipTLSVerify(true)
+		params, err := opts.ToParams()
+		Expect(err).ToNot(HaveOccurred())
+		Expect(params.Get("os")).To(Equal("foo"))
+		Expect(params.Has("skiptlsverify")).To(BeFalse())
+	})
+
+	It("serialize manifest add options", func() {
+		opts := new(manifests.AddOptions).WithAll(true).WithOS("foo").WithSkipTLSVerify(true)
+		params, err := opts.ToParams()
+		Expect(err).ToNot(HaveOccurred())
+		Expect(params.Get("all")).To(Equal("true"))
+		Expect(params.Get("os")).To(Equal("foo"))
+		Expect(params.Has("skiptlsverify")).To(BeFalse())
+	})
+
+	It("serialize kube play options", func() {
+		opts := new(kube.PlayOptions).WithQuiet(true).WithSkipTLSVerify(true)
+		params, err := opts.ToParams()
+		Expect(err).ToNot(HaveOccurred())
+		Expect(params.Get("quiet")).To(Equal("true"))
+		Expect(params.Has("skiptlsverify")).To(BeFalse())
+	})
+})