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>

9 files changed, 419 insertions, 148 deletions

diff --git a/pkg/bindings/connection.go b/pkg/bindings/connection.go
index b2e949f67..332aa97c8 100644
--- a/pkg/bindings/connection.go
+++ b/pkg/bindings/connection.go
@@ -15,7 +15,6 @@ import (
 	"github.com/blang/semver"
 	"github.com/containers/podman/v3/pkg/terminal"
 	"github.com/containers/podman/v3/version"
-	jsoniter "github.com/json-iterator/go"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	"golang.org/x/crypto/ssh"
@@ -35,16 +34,24 @@ type Connection struct {
 type valueKey string
 
 const (
-	clientKey = valueKey("Client")
+	clientKey  = valueKey("Client")
+	versionKey = valueKey("ServiceVersion")
 )
 
 // GetClient from context build by NewConnection()
 func GetClient(ctx context.Context) (*Connection, error) {
-	c, ok := ctx.Value(clientKey).(*Connection)
-	if !ok {
-		return nil, errors.Errorf("ClientKey not set in context")
+	if c, ok := ctx.Value(clientKey).(*Connection); ok {
+		return c, nil
 	}
-	return c, nil
+	return nil, errors.Errorf("%s not set in context", clientKey)
+}
+
+// ServiceVersion from context build by NewConnection()
+func ServiceVersion(ctx context.Context) *semver.Version {
+	if v, ok := ctx.Value(versionKey).(*semver.Version); ok {
+		return v
+	}
+	return new(semver.Version)
 }
 
 // JoinURL elements with '/'
@@ -52,6 +59,7 @@ func JoinURL(elements ...string) string {
 	return "/" + strings.Join(elements, "/")
 }
 
+// NewConnection creates a new service connection without an identity
 func NewConnection(ctx context.Context, uri string) (context.Context, error) {
 	return NewConnectionWithIdentity(ctx, uri, "")
 }
@@ -116,9 +124,11 @@ func NewConnectionWithIdentity(ctx context.Context, uri string, identity string)
 	}
 
 	ctx = context.WithValue(ctx, clientKey, &connection)
-	if err := pingNewConnection(ctx); err != nil {
+	serviceVersion, err := pingNewConnection(ctx)
+	if err != nil {
 		return nil, errors.Wrap(err, "unable to connect to Podman socket")
 	}
+	ctx = context.WithValue(ctx, versionKey, serviceVersion)
 	return ctx, nil
 }
 
@@ -139,15 +149,15 @@ func tcpClient(_url *url.URL) Connection {
 
 // pingNewConnection pings to make sure the RESTFUL service is up
 // and running. it should only be used when initializing a connection
-func pingNewConnection(ctx context.Context) error {
+func pingNewConnection(ctx context.Context) (*semver.Version, error) {
 	client, err := GetClient(ctx)
 	if err != nil {
-		return err
+		return nil, err
 	}
 	// the ping endpoint sits at / in this case
 	response, err := client.DoRequest(ctx, nil, http.MethodGet, "/_ping", nil, nil)
 	if err != nil {
-		return err
+		return nil, err
 	}
 	defer response.Body.Close()
 
@@ -155,23 +165,23 @@ func pingNewConnection(ctx context.Context) error {
 		versionHdr := response.Header.Get("Libpod-API-Version")
 		if versionHdr == "" {
 			logrus.Info("Service did not provide Libpod-API-Version Header")
-			return nil
+			return new(semver.Version), nil
 		}
 		versionSrv, err := semver.ParseTolerant(versionHdr)
 		if err != nil {
-			return err
+			return nil, err
 		}
 
 		switch version.APIVersion[version.Libpod][version.MinimalAPI].Compare(versionSrv) {
 		case -1, 0:
 			// Server's job when Client version is equal or older
-			return nil
+			return &versionSrv, nil
 		case 1:
-			return errors.Errorf("server API version is too old. Client %q server %q",
+			return nil, errors.Errorf("server API version is too old. Client %q server %q",
 				version.APIVersion[version.Libpod][version.MinimalAPI].String(), versionSrv.String())
 		}
 	}
-	return errors.Errorf("ping response was %d", response.StatusCode)
+	return nil, errors.Errorf("ping response was %d", response.StatusCode)
 }
 
 func sshClient(_url *url.URL, secure bool, passPhrase string, identity string) (Connection, error) {
@@ -306,26 +316,29 @@ func unixClient(_url *url.URL) Connection {
 }
 
 // DoRequest assembles the http request and returns the response
-func (c *Connection) DoRequest(ctx context.Context, httpBody io.Reader, httpMethod, endpoint string, queryParams url.Values, header map[string]string, pathValues ...string) (*APIResponse, error) {
+func (c *Connection) DoRequest(ctx context.Context, httpBody io.Reader, httpMethod, endpoint string, queryParams url.Values, headers http.Header, pathValues ...string) (*APIResponse, error) {
 	var (
 		err      error
 		response *http.Response
 	)
 
-	params := make([]interface{}, len(pathValues)+3)
+	params := make([]interface{}, len(pathValues)+1)
+
+	if v := headers.Values("API-Version"); len(v) > 0 {
+		params[0] = v[0]
+	} else {
+		// Including the semver suffices breaks older services... so do not include them
+		v := version.APIVersion[version.Libpod][version.CurrentAPI]
+		params[0] = fmt.Sprintf("%d.%d.%d", v.Major, v.Minor, v.Patch)
+	}
 
-	// Including the semver suffices breaks older services... so do not include them
-	v := version.APIVersion[version.Libpod][version.CurrentAPI]
-	params[0] = v.Major
-	params[1] = v.Minor
-	params[2] = v.Patch
 	for i, pv := range pathValues {
 		// url.URL lacks the semantics for escaping embedded path parameters... so we manually
 		//   escape each one and assume the caller included the correct formatting in "endpoint"
-		params[i+3] = url.PathEscape(pv)
+		params[i+1] = url.PathEscape(pv)
 	}
 
-	uri := fmt.Sprintf("http://d/v%d.%d.%d/libpod"+endpoint, params...)
+	uri := fmt.Sprintf("http://d/v%s/libpod"+endpoint, params...)
 	logrus.Debugf("DoRequest Method: %s URI: %v", httpMethod, uri)
 
 	req, err := http.NewRequestWithContext(ctx, httpMethod, uri, httpBody)
@@ -335,9 +348,17 @@ func (c *Connection) DoRequest(ctx context.Context, httpBody io.Reader, httpMeth
 	if len(queryParams) > 0 {
 		req.URL.RawQuery = queryParams.Encode()
 	}
-	for key, val := range header {
-		req.Header.Set(key, val)
+
+	for key, val := range headers {
+		if key == "API-Version" {
+			continue
+		}
+
+		for _, v := range val {
+			req.Header.Add(key, v)
+		}
 	}
+
 	// Give the Do three chances in the case of a comm/service hiccup
 	for i := 1; i <= 3; i++ {
 		response, err = c.Client.Do(req) // nolint
@@ -349,7 +370,7 @@ func (c *Connection) DoRequest(ctx context.Context, httpBody io.Reader, httpMeth
 	return &APIResponse{response, req}, err
 }
 
-// Get raw Transport.DialContext from client
+// GetDialer returns raw Transport.DialContext from client
 func (c *Connection) GetDialer(ctx context.Context) (net.Conn, error) {
 	client := c.Client
 	transport := client.Transport.(*http.Transport)
@@ -360,16 +381,6 @@ func (c *Connection) GetDialer(ctx context.Context) (net.Conn, error) {
 	return nil, errors.New("Unable to get dial context")
 }
 
-// FiltersToString converts our typical filter format of a
-// map[string][]string to a query/html safe string.
-func FiltersToString(filters map[string][]string) (string, error) {
-	lowerCaseKeys := make(map[string][]string)
-	for k, v := range filters {
-		lowerCaseKeys[strings.ToLower(k)] = v
-	}
-	return jsoniter.MarshalToString(lowerCaseKeys)
-}
-
 // IsInformational returns true if the response code is 1xx
 func (h *APIResponse) IsInformational() bool {
 	return h.Response.StatusCode/100 == 1
diff --git a/pkg/bindings/containers/attach.go b/pkg/bindings/containers/attach.go
index baa3f182e..c02265cd8 100644
--- a/pkg/bindings/containers/attach.go
+++ b/pkg/bindings/containers/attach.go
@@ -108,9 +108,9 @@ func Attach(ctx context.Context, nameOrID string, stdin io.Reader, stdout io.Wri
 		}()
 	}
 
-	headers := make(map[string]string)
-	headers["Connection"] = "Upgrade"
-	headers["Upgrade"] = "tcp"
+	headers := make(http.Header)
+	headers.Add("Connection", "Upgrade")
+	headers.Add("Upgrade", "tcp")
 
 	var socket net.Conn
 	socketSet := false
@@ -157,7 +157,7 @@ func Attach(ctx context.Context, nameOrID string, stdin io.Reader, stdout io.Wri
 	}
 
 	stdoutChan := make(chan error)
-	stdinChan := make(chan error, 1) //stdin channel should not block
+	stdinChan := make(chan error, 1) // stdin channel should not block
 
 	if isSet.stdin {
 		go func() {
diff --git a/pkg/bindings/errors.go b/pkg/bindings/errors.go
index ec837b39c..be184b916 100644
--- a/pkg/bindings/errors.go
+++ b/pkg/bindings/errors.go
@@ -25,7 +25,7 @@ func (h APIResponse) Process(unmarshalInto interface{}) error {
 	return h.ProcessWithError(unmarshalInto, &errorhandling.ErrorModel{})
 }
 
-// Process drains the response body, and processes the HTTP status code
+// ProcessWithError drains the response body, and processes the HTTP status code
 // Note: Closing the response.Body is left to the caller
 func (h APIResponse) ProcessWithError(unmarshalInto interface{}, unmarshalErrorInto interface{}) error {
 	data, err := ioutil.ReadAll(h.Response.Body)
diff --git a/pkg/bindings/images/build.go b/pkg/bindings/images/build.go
index 6b5159f52..9880c73e4 100644
--- a/pkg/bindings/images/build.go
+++ b/pkg/bindings/images/build.go
@@ -300,7 +300,7 @@ func Build(ctx context.Context, containerFiles []string, options entities.BuildO
 	}
 
 	var (
-		headers map[string]string
+		headers http.Header
 		err     error
 	)
 	if options.SystemContext != nil && options.SystemContext.DockerAuthConfig != nil {
@@ -421,7 +421,7 @@ func Build(ctx context.Context, containerFiles []string, options entities.BuildO
 								return nil, err
 							}
 
-							//add tmp file to context dir
+							// add tmp file to context dir
 							tarContent = append(tarContent, tmpSecretFile.Name())
 
 							modifiedSrc := fmt.Sprintf("src=%s", filepath.Base(tmpSecretFile.Name()))
@@ -634,7 +634,7 @@ func nTar(excludes []string, sources ...string) (io.ReadCloser, error) {
 					if lerr := tw.WriteHeader(hdr); lerr != nil {
 						return lerr
 					}
-				} //skip other than file,folder and symlinks
+				} // skip other than file,folder and symlinks
 				return nil
 			})
 			merr = multierror.Append(merr, err)
diff --git a/pkg/bindings/internal/util/util.go b/pkg/bindings/internal/util/util.go
index bcf6959f2..f8f99d6c1 100644
--- a/pkg/bindings/internal/util/util.go
+++ b/pkg/bindings/internal/util/util.go
@@ -104,3 +104,11 @@ func ToParams(o interface{}) (url.Values, error) {
 	}
 	return params, nil
 }
+
+func MapToArrayString(data map[string]string) []string {
+	l := make([]string, 0)
+	for k, v := range data {
+		l = append(l, k+"="+v)
+	}
+	return l
+}
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
+}
diff --git a/pkg/bindings/test/manifests_test.go b/pkg/bindings/test/manifests_test.go
index e65632057..280006d15 100644
--- a/pkg/bindings/test/manifests_test.go
+++ b/pkg/bindings/test/manifests_test.go
@@ -12,7 +12,7 @@ import (
 	"github.com/onsi/gomega/gexec"
 )
 
-var _ = Describe("Podman containers ", func() {
+var _ = Describe("podman manifest", func() {
 	var (
 		bt *bindingTest
 		s  *gexec.Session
@@ -24,7 +24,8 @@ var _ = Describe("Podman containers ", func() {
 		s = bt.startAPIService()
 		time.Sleep(1 * time.Second)
 		err := bt.NewConnection()
-		Expect(err).To(BeNil())
+		Expect(err).ToNot(HaveOccurred())
+
 	})
 
 	AfterEach(func() {
@@ -32,17 +33,19 @@ var _ = Describe("Podman containers ", func() {
 		bt.cleanup()
 	})
 
-	It("create manifest", func() {
+	It("create", func() {
 		// create manifest list without images
-		id, err := manifests.Create(bt.conn, []string{"quay.io/libpod/foobar:latest"}, []string{}, nil)
-		Expect(err).To(BeNil())
+		id, err := manifests.Create(bt.conn, "quay.io/libpod/foobar:latest", []string{}, nil)
+		Expect(err).ToNot(HaveOccurred(), err)
 		list, err := manifests.Inspect(bt.conn, id, nil)
-		Expect(err).To(BeNil())
+		Expect(err).ToNot(HaveOccurred())
+
 		Expect(len(list.Manifests)).To(BeZero())
 
 		// creating a duplicate should fail as a 500
-		_, err = manifests.Create(bt.conn, []string{"quay.io/libpod/foobar:latest"}, []string{}, nil)
-		Expect(err).ToNot(BeNil())
+		_, err = manifests.Create(bt.conn, "quay.io/libpod/foobar:latest", nil, nil)
+		Expect(err).To(HaveOccurred())
+
 		code, _ := bindings.CheckResponseCode(err)
 		Expect(code).To(BeNumerically("==", http.StatusInternalServerError))
 
@@ -50,96 +53,113 @@ var _ = Describe("Podman containers ", func() {
 		Expect(len(errs)).To(BeZero())
 
 		// create manifest list with images
-		id, err = manifests.Create(bt.conn, []string{"quay.io/libpod/foobar:latest"}, []string{alpine.name}, nil)
-		Expect(err).To(BeNil())
+		id, err = manifests.Create(bt.conn, "quay.io/libpod/foobar:latest", []string{alpine.name}, nil)
+		Expect(err).ToNot(HaveOccurred())
+
 		list, err = manifests.Inspect(bt.conn, id, nil)
-		Expect(err).To(BeNil())
+		Expect(err).ToNot(HaveOccurred())
+
 		Expect(len(list.Manifests)).To(BeNumerically("==", 1))
 	})
 
-	It("inspect bogus manifest", func() {
+	It("inspect", func() {
 		_, err := manifests.Inspect(bt.conn, "larry", nil)
-		Expect(err).ToNot(BeNil())
+		Expect(err).To(HaveOccurred())
+
 		code, _ := bindings.CheckResponseCode(err)
 		Expect(code).To(BeNumerically("==", http.StatusNotFound))
 	})
 
-	It("add manifest", func() {
+	It("add", func() {
 		// add to bogus should 404
 		_, err := manifests.Add(bt.conn, "foobar", nil)
-		Expect(err).ToNot(BeNil())
+		Expect(err).To(HaveOccurred())
+
 		code, _ := bindings.CheckResponseCode(err)
-		Expect(code).To(BeNumerically("==", http.StatusNotFound))
+		Expect(code).To(BeNumerically("==", http.StatusNotFound), err.Error())
+
+		id, err := manifests.Create(bt.conn, "quay.io/libpod/foobar:latest", []string{}, nil)
+		Expect(err).ToNot(HaveOccurred())
 
-		id, err := manifests.Create(bt.conn, []string{"quay.io/libpod/foobar:latest"}, []string{}, nil)
-		Expect(err).To(BeNil())
 		options := new(manifests.AddOptions).WithImages([]string{alpine.name})
 		_, err = manifests.Add(bt.conn, id, options)
-		Expect(err).To(BeNil())
+		Expect(err).ToNot(HaveOccurred())
+
 		list, err := manifests.Inspect(bt.conn, id, nil)
-		Expect(err).To(BeNil())
+		Expect(err).ToNot(HaveOccurred())
+
 		Expect(len(list.Manifests)).To(BeNumerically("==", 1))
 
 		// add bogus name to existing list should fail
 		options.WithImages([]string{"larry"})
 		_, err = manifests.Add(bt.conn, id, options)
-		Expect(err).ToNot(BeNil())
+		Expect(err).To(HaveOccurred())
+
 		code, _ = bindings.CheckResponseCode(err)
 		Expect(code).To(BeNumerically("==", http.StatusInternalServerError))
 	})
 
-	It("remove manifest", func() {
+	It("remove digest", func() {
 		// removal on bogus manifest list should be 404
 		_, err := manifests.Remove(bt.conn, "larry", "1234", nil)
-		Expect(err).ToNot(BeNil())
+		Expect(err).To(HaveOccurred())
+
 		code, _ := bindings.CheckResponseCode(err)
 		Expect(code).To(BeNumerically("==", http.StatusNotFound))
 
-		id, err := manifests.Create(bt.conn, []string{"quay.io/libpod/foobar:latest"}, []string{alpine.name}, nil)
-		Expect(err).To(BeNil())
+		id, err := manifests.Create(bt.conn, "quay.io/libpod/foobar:latest", []string{alpine.name}, nil)
+		Expect(err).ToNot(HaveOccurred())
+
 		data, err := manifests.Inspect(bt.conn, id, nil)
-		Expect(err).To(BeNil())
+		Expect(err).ToNot(HaveOccurred())
+
 		Expect(len(data.Manifests)).To(BeNumerically("==", 1))
 
 		// removal on a good manifest list with a bad digest should be 400
 		_, err = manifests.Remove(bt.conn, id, "!234", nil)
-		Expect(err).ToNot(BeNil())
+		Expect(err).To(HaveOccurred())
+
 		code, _ = bindings.CheckResponseCode(err)
 		Expect(code).To(BeNumerically("==", http.StatusBadRequest))
 
 		digest := data.Manifests[0].Digest.String()
 		_, err = manifests.Remove(bt.conn, id, digest, nil)
-		Expect(err).To(BeNil())
+		Expect(err).ToNot(HaveOccurred())
 
 		// removal on good manifest with good digest should work
 		data, err = manifests.Inspect(bt.conn, id, nil)
-		Expect(err).To(BeNil())
-		Expect(len(data.Manifests)).To(BeZero())
+		Expect(err).ToNot(HaveOccurred())
+
+		Expect(data.Manifests).Should(BeEmpty())
 	})
 
-	// There is NO annotate endpoint, this could never work.:w
-
-	//It("annotate manifest", func() {
-	//	id, err := manifests.Create(bt.conn, []string{"quay.io/libpod/foobar:latest"}, []string{}, nil)
-	//	Expect(err).To(BeNil())
-	//	opts := image.ManifestAddOpts{Images: []string{"docker.io/library/alpine:latest"}}
-	//
-	//	_, err = manifests.Add(bt.conn, id, opts)
-	//	Expect(err).To(BeNil())
-	//	data, err := manifests.Inspect(bt.conn, id)
-	//	Expect(err).To(BeNil())
-	//	Expect(len(data.Manifests)).To(BeNumerically("==", 1))
-	//	digest := data.Manifests[0].Digest.String()
-	//	annoOpts := image.ManifestAnnotateOpts{OS: "foo"}
-	//	_, err = manifests.Annotate(bt.conn, id, digest, annoOpts)
-	//	Expect(err).To(BeNil())
-	//	list, err := manifests.Inspect(bt.conn, id)
-	//	Expect(err).To(BeNil())
-	//	Expect(len(list.Manifests)).To(BeNumerically("==", 1))
-	//	Expect(list.Manifests[0].Platform.OS).To(Equal("foo"))
-	//})
+	It("annotate", func() {
+		id, err := manifests.Create(bt.conn, "quay.io/libpod/foobar:latest", []string{}, nil)
+		Expect(err).ToNot(HaveOccurred())
+
+		opts := manifests.AddOptions{Images: []string{"quay.io/libpod/alpine:latest"}}
+
+		_, err = manifests.Add(bt.conn, id, &opts)
+		Expect(err).ToNot(HaveOccurred())
+
+		data, err := manifests.Inspect(bt.conn, id, nil)
+		Expect(err).ToNot(HaveOccurred())
+
+		Expect(len(data.Manifests)).To(BeNumerically("==", 1))
+
+		digest := data.Manifests[0].Digest.String()
+		annoOpts := new(manifests.ModifyOptions).WithOS("foo")
+		_, err = manifests.Annotate(bt.conn, id, []string{digest}, annoOpts)
+		Expect(err).ToNot(HaveOccurred())
+
+		list, err := manifests.Inspect(bt.conn, id, nil)
+		Expect(err).ToNot(HaveOccurred())
+
+		Expect(len(list.Manifests)).To(BeNumerically("==", 1))
+		Expect(list.Manifests[0].Platform.OS).To(Equal("foo"))
+	})
 
 	It("push manifest", func() {
-		Skip("TODO")
+		Skip("TODO: implement test for manifest push to registry")
 	})
 })