Refactor manifest list operations

* Update method/function signatures use the manifest list name and
  images associated with the operation explicitly, in general

  func f(ctx context.Context, manifestListName string,
         ImageNames []string, options *fOptions)

* Leverage gorilla/mux Subrouters to support API v3.x and v4.x for
  manifests
* Make manifest API endpoints more RESTful
* Add PUT /manifest/{id} to update existing manifests
* Add manifests.Annotate to go bindings, uncommented unit test
* Add DELETE /manifest/{Id} to remove existing manifest list, use
  PUT /manifest/{id} to remove images from a list
* Deprecated POST /manifest/{id}/add and /manifest/{id}/remove, use
  PUT /manifest/{id} instead
* Corrected swagger godoc and updated to cover API changes
* Update podman manifest commands to use registry.Context()
* Expose utils.GetVar() to obtain query parameters by name
* Unexpose server.registerSwaggerHandlers, not sure why this was ever
  exposed.
* Refactored code to use http.Header instead of map[string]string when
  operating on HTTP headers.
* Add API-Version header support in bindings to allow calling explicate
  versions of the API. Header is _NOT_ forwarded to the API service.

Signed-off-by: Jhon Honce <jhonce@redhat.com>

3 files changed, 283 insertions, 51 deletions

diff --git a/pkg/bindings/manifests/manifests.go b/pkg/bindings/manifests/manifests.go
index af74eb406..50e324efa 100644
--- a/pkg/bindings/manifests/manifests.go
+++ b/pkg/bindings/manifests/manifests.go
@@ -3,15 +3,18 @@ package manifests
 import (
 	"context"
 	"errors"
+	"fmt"
 	"net/http"
 	"net/url"
 	"strconv"
 	"strings"
 
+	"github.com/blang/semver"
 	"github.com/containers/image/v5/manifest"
 	"github.com/containers/podman/v3/pkg/api/handlers"
 	"github.com/containers/podman/v3/pkg/bindings"
 	"github.com/containers/podman/v3/pkg/bindings/images"
+	"github.com/containers/podman/v3/version"
 	jsoniter "github.com/json-iterator/go"
 )
 
@@ -19,7 +22,7 @@ import (
 // the new manifest can also be specified.  The all boolean specifies to add all entries
 // of a list if the name provided is a manifest list.  The ID of the new manifest list
 // is returned as a string.
-func Create(ctx context.Context, names, images []string, options *CreateOptions) (string, error) {
+func Create(ctx context.Context, name string, images []string, options *CreateOptions) (string, error) {
 	var idr handlers.IDResponse
 	if options == nil {
 		options = new(CreateOptions)
@@ -28,21 +31,19 @@ func Create(ctx context.Context, names, images []string, options *CreateOptions)
 	if err != nil {
 		return "", err
 	}
-	if len(names) < 1 {
+	if len(name) < 1 {
 		return "", errors.New("creating a manifest requires at least one name argument")
 	}
 	params, err := options.ToParams()
 	if err != nil {
 		return "", err
 	}
-	for _, name := range names {
-		params.Add("name", name)
-	}
+
 	for _, i := range images {
-		params.Add("image", i)
+		params.Add("images", i)
 	}
 
-	response, err := conn.DoRequest(ctx, nil, http.MethodPost, "/manifests/create", params, nil)
+	response, err := conn.DoRequest(ctx, nil, http.MethodPost, "/manifests/%s", params, nil, name)
 	if err != nil {
 		return "", err
 	}
@@ -67,70 +68,96 @@ func Exists(ctx context.Context, name string, options *ExistsOptions) (bool, err
 }
 
 // Inspect returns a manifest list for a given name.
-func Inspect(ctx context.Context, name string, options *InspectOptions) (*manifest.Schema2List, error) {
-	var list manifest.Schema2List
-	if options == nil {
-		options = new(InspectOptions)
-	}
-	_ = options
+func Inspect(ctx context.Context, name string, _ *InspectOptions) (*manifest.Schema2List, error) {
 	conn, err := bindings.GetClient(ctx)
 	if err != nil {
 		return nil, err
 	}
+
 	response, err := conn.DoRequest(ctx, nil, http.MethodGet, "/manifests/%s/json", nil, nil, name)
 	if err != nil {
 		return nil, err
 	}
 	defer response.Body.Close()
 
+	var list manifest.Schema2List
 	return &list, response.Process(&list)
 }
 
 // Add adds a manifest to a given manifest list.  Additional options for the manifest
 // can also be specified.  The ID of the new manifest list is returned as a string
 func Add(ctx context.Context, name string, options *AddOptions) (string, error) {
-	var idr handlers.IDResponse
 	if options == nil {
 		options = new(AddOptions)
 	}
+
+	if bindings.ServiceVersion(ctx).GTE(semver.MustParse("4.0.0")) {
+		optionsv4 := ModifyOptions{
+			All:         options.All,
+			Annotations: options.Annotation,
+			Arch:        options.Arch,
+			Features:    options.Features,
+			Images:      options.Images,
+			OS:          options.OS,
+			OSFeatures:  nil,
+			OSVersion:   options.OSVersion,
+			Variant:     options.Variant,
+		}
+		optionsv4.WithOperation("update")
+		return Modify(ctx, name, options.Images, &optionsv4)
+	}
+
+	// API Version < 4.0.0
 	conn, err := bindings.GetClient(ctx)
 	if err != nil {
 		return "", err
 	}
-	optionsString, err := jsoniter.MarshalToString(options)
+	opts, err := jsoniter.MarshalToString(options)
 	if err != nil {
 		return "", err
 	}
-	stringReader := strings.NewReader(optionsString)
-	response, err := conn.DoRequest(ctx, stringReader, http.MethodPost, "/manifests/%s/add", nil, nil, name)
+	reader := strings.NewReader(opts)
+
+	headers := make(http.Header)
+	v := version.APIVersion[version.Libpod][version.MinimalAPI]
+	headers.Add("API-Version",
+		fmt.Sprintf("%d.%d.%d", v.Major, v.Minor, v.Patch))
+	response, err := conn.DoRequest(ctx, reader, http.MethodPost, "/manifests/%s/add", nil, headers, name)
 	if err != nil {
 		return "", err
 	}
 	defer response.Body.Close()
 
+	var idr handlers.IDResponse
 	return idr.ID, response.Process(&idr)
 }
 
 // Remove deletes a manifest entry from a manifest list.  Both name and the digest to be
 // removed are mandatory inputs.  The ID of the new manifest list is returned as a string.
-func Remove(ctx context.Context, name, digest string, options *RemoveOptions) (string, error) {
-	var idr handlers.IDResponse
-	if options == nil {
-		options = new(RemoveOptions)
+func Remove(ctx context.Context, name, digest string, _ *RemoveOptions) (string, error) {
+	if bindings.ServiceVersion(ctx).GTE(semver.MustParse("4.0.0")) {
+		optionsv4 := new(ModifyOptions).WithOperation("remove")
+		return Modify(ctx, name, []string{digest}, optionsv4)
 	}
-	_ = options
+
+	// API Version < 4.0.0
 	conn, err := bindings.GetClient(ctx)
 	if err != nil {
 		return "", err
 	}
+
+	headers := http.Header{}
+	headers.Add("API-Version", "3.4.0")
+
 	params := url.Values{}
 	params.Set("digest", digest)
-	response, err := conn.DoRequest(ctx, nil, http.MethodDelete, "/manifests/%s", params, nil, name)
+	response, err := conn.DoRequest(ctx, nil, http.MethodDelete, "/manifests/%s", params, headers, name)
 	if err != nil {
 		return "", err
 	}
 	defer response.Body.Close()
 
+	var idr handlers.IDResponse
 	return idr.ID, response.Process(&idr)
 }
 
@@ -151,19 +178,26 @@ func Push(ctx context.Context, name, destination string, options *images.PushOpt
 	if err != nil {
 		return "", err
 	}
+
 	params, err := options.ToParams()
 	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
+	// ToParams() and change the key and flip the bool
 	if options.SkipTLSVerify != nil {
 		params.Del("SkipTLSVerify")
 		params.Set("tlsVerify", strconv.FormatBool(!options.GetSkipTLSVerify()))
 	}
-	params.Set("image", name)
-	params.Set("destination", destination)
-	response, err := conn.DoRequest(ctx, nil, http.MethodPost, "/manifests/%s/push", params, nil, name)
+
+	var response *bindings.APIResponse
+	if bindings.ServiceVersion(ctx).GTE(semver.MustParse("4.0.0")) {
+		response, err = conn.DoRequest(ctx, nil, http.MethodPost, "/manifests/%s/registry/%s", params, nil, name, destination)
+	} else {
+		params.Set("image", name)
+		params.Set("destination", destination)
+		response, err = conn.DoRequest(ctx, nil, http.MethodPost, "/manifests/%s/push", params, nil, name)
+	}
 	if err != nil {
 		return "", err
 	}
@@ -172,25 +206,37 @@ func Push(ctx context.Context, name, destination string, options *images.PushOpt
 	return idr.ID, err
 }
 
-// There is NO annotate endpoint.  this binding could never work
-// Annotate updates the image configuration of a given manifest list
-//func Annotate(ctx context.Context, name, digest string, options image.ManifestAnnotateOpts) (string, error) {
-//	var idr handlers.IDResponse
-//	conn, err := bindings.GetClient(ctx)
-//	if err != nil {
-//		return "", err
-//	}
-//	params := url.Values{}
-//	params.Set("digest", digest)
-//	optionsString, err := jsoniter.MarshalToString(options)
-//	if err != nil {
-//		return "", err
-//	}
-//	stringReader := strings.NewReader(optionsString)
-//	response, err := conn.DoRequest(ctx, stringReader, http.MethodPost, "/manifests/%s/annotate", params, name)
-//	if err != nil {
-//		return "", err
-//	}
-//  defer response.Body.Close()
-//	return idr.ID, response.Process(&idr)
-//}
+// Modify modifies the given manifest list using options and the optional list of images
+func Modify(ctx context.Context, name string, images []string, options *ModifyOptions) (string, error) {
+	if options == nil || *options.Operation == "" {
+		return "", errors.New(`the field ModifyOptions.Operation must be set to either "update" or "remove"`)
+	}
+	options.WithImages(images)
+
+	conn, err := bindings.GetClient(ctx)
+	if err != nil {
+		return "", err
+	}
+	opts, err := jsoniter.MarshalToString(options)
+	if err != nil {
+		return "", err
+	}
+	reader := strings.NewReader(opts)
+
+	response, err := conn.DoRequest(ctx, reader, http.MethodPut, "/manifests/%s", nil, nil, name)
+	if err != nil {
+		return "", err
+	}
+	defer response.Body.Close()
+
+	var idr handlers.IDResponse
+	return idr.ID, response.Process(&idr)
+}
+
+// Annotate modifies the given manifest list using options and the optional list of images
+//
+// As of 4.0.0
+func Annotate(ctx context.Context, name string, images []string, options *ModifyOptions) (string, error) {
+	options.WithOperation("annotate")
+	return Modify(ctx, name, images, options)
+}
diff --git a/pkg/bindings/manifests/types.go b/pkg/bindings/manifests/types.go
index fde90a865..5ff28ee30 100644
--- a/pkg/bindings/manifests/types.go
+++ b/pkg/bindings/manifests/types.go
@@ -18,7 +18,7 @@ type ExistsOptions struct {
 }
 
 //go:generate go run ../generator/generator.go AddOptions
-// AddOptions are optional options for adding manifests
+// AddOptions are optional options for adding manifest lists
 type AddOptions struct {
 	All        *bool
 	Annotation map[string]string
@@ -31,6 +31,24 @@ type AddOptions struct {
 }
 
 //go:generate go run ../generator/generator.go RemoveOptions
-// RemoveOptions are optional options for removing manifests
+// RemoveOptions are optional options for removing manifest lists
 type RemoveOptions struct {
 }
+
+//go:generate go run ../generator/generator.go ModifyOptions
+// ModifyOptions are optional options for modifying manifest lists
+type ModifyOptions struct {
+	// Operation values are "update", "remove" and "annotate". This allows the service to
+	//   efficiently perform each update on a manifest list.
+	Operation   *string
+	All         *bool             // All when true, operate on all images in a manifest list that may be included in Images
+	Annotations map[string]string // Annotations to add to manifest list
+	Arch        *string           // Arch overrides the architecture for the image
+	Features    []string          // Feature list for the image
+	Images      []string          // Images is an optional list of images to add/remove to/from manifest list depending on operation
+	OS          *string           // OS overrides the operating system for the image
+	OSFeatures  []string          // OS features for the image
+	OSVersion   *string           // OSVersion overrides the operating system for the image
+	Variant     *string           // Variant overrides the operating system variant for the image
+
+}
diff --git a/pkg/bindings/manifests/types_modify_options.go b/pkg/bindings/manifests/types_modify_options.go
new file mode 100644
index 000000000..ee5d94dbf
--- /dev/null
+++ b/pkg/bindings/manifests/types_modify_options.go
@@ -0,0 +1,168 @@
+// Code generated by go generate; DO NOT EDIT.
+package manifests
+
+import (
+	"net/url"
+
+	"github.com/containers/podman/v3/pkg/bindings/internal/util"
+)
+
+// Changed returns true if named field has been set
+func (o *ModifyOptions) Changed(fieldName string) bool {
+	return util.Changed(o, fieldName)
+}
+
+// ToParams formats struct fields to be passed to API service
+func (o *ModifyOptions) ToParams() (url.Values, error) {
+	return util.ToParams(o)
+}
+
+// WithOperation set field Operation to given value
+func (o *ModifyOptions) WithOperation(value string) *ModifyOptions {
+	o.Operation = &value
+	return o
+}
+
+// GetOperation returns value of field Operation
+func (o *ModifyOptions) GetOperation() string {
+	if o.Operation == nil {
+		var z string
+		return z
+	}
+	return *o.Operation
+}
+
+// WithAll set all when true, operate on all images in a manifest list that may be included in Images
+func (o *ModifyOptions) WithAll(value bool) *ModifyOptions {
+	o.All = &value
+	return o
+}
+
+// GetAll returns value of all when true, operate on all images in a manifest list that may be included in Images
+func (o *ModifyOptions) GetAll() bool {
+	if o.All == nil {
+		var z bool
+		return z
+	}
+	return *o.All
+}
+
+// WithAnnotations set annotations to add to manifest list
+func (o *ModifyOptions) WithAnnotations(value map[string]string) *ModifyOptions {
+	o.Annotations = value
+	return o
+}
+
+// GetAnnotations returns value of annotations to add to manifest list
+func (o *ModifyOptions) GetAnnotations() map[string]string {
+	if o.Annotations == nil {
+		var z map[string]string
+		return z
+	}
+	return o.Annotations
+}
+
+// WithArch set arch overrides the architecture for the image
+func (o *ModifyOptions) WithArch(value string) *ModifyOptions {
+	o.Arch = &value
+	return o
+}
+
+// GetArch returns value of arch overrides the architecture for the image
+func (o *ModifyOptions) GetArch() string {
+	if o.Arch == nil {
+		var z string
+		return z
+	}
+	return *o.Arch
+}
+
+// WithFeatures set feature list for the image
+func (o *ModifyOptions) WithFeatures(value []string) *ModifyOptions {
+	o.Features = value
+	return o
+}
+
+// GetFeatures returns value of feature list for the image
+func (o *ModifyOptions) GetFeatures() []string {
+	if o.Features == nil {
+		var z []string
+		return z
+	}
+	return o.Features
+}
+
+// WithImages set images is an optional list of images to add/remove to/from manifest list depending on operation
+func (o *ModifyOptions) WithImages(value []string) *ModifyOptions {
+	o.Images = value
+	return o
+}
+
+// GetImages returns value of images is an optional list of images to add/remove to/from manifest list depending on operation
+func (o *ModifyOptions) GetImages() []string {
+	if o.Images == nil {
+		var z []string
+		return z
+	}
+	return o.Images
+}
+
+// WithOS set oS overrides the operating system for the image
+func (o *ModifyOptions) WithOS(value string) *ModifyOptions {
+	o.OS = &value
+	return o
+}
+
+// GetOS returns value of oS overrides the operating system for the image
+func (o *ModifyOptions) GetOS() string {
+	if o.OS == nil {
+		var z string
+		return z
+	}
+	return *o.OS
+}
+
+// WithOSFeatures set oS features for the image
+func (o *ModifyOptions) WithOSFeatures(value []string) *ModifyOptions {
+	o.OSFeatures = value
+	return o
+}
+
+// GetOSFeatures returns value of oS features for the image
+func (o *ModifyOptions) GetOSFeatures() []string {
+	if o.OSFeatures == nil {
+		var z []string
+		return z
+	}
+	return o.OSFeatures
+}
+
+// WithOSVersion set oSVersion overrides the operating system for the image
+func (o *ModifyOptions) WithOSVersion(value string) *ModifyOptions {
+	o.OSVersion = &value
+	return o
+}
+
+// GetOSVersion returns value of oSVersion overrides the operating system for the image
+func (o *ModifyOptions) GetOSVersion() string {
+	if o.OSVersion == nil {
+		var z string
+		return z
+	}
+	return *o.OSVersion
+}
+
+// WithVariant set variant overrides the operating system variant for the image
+func (o *ModifyOptions) WithVariant(value string) *ModifyOptions {
+	o.Variant = &value
+	return o
+}
+
+// GetVariant returns value of variant overrides the operating system variant for the image
+func (o *ModifyOptions) GetVariant() string {
+	if o.Variant == nil {
+		var z string
+		return z
+	}
+	return *o.Variant
+}