From 8a7e70919f4bab0757523ae97c170396cb13c83d Mon Sep 17 00:00:00 2001 From: Jhon Honce Date: Mon, 6 Dec 2021 16:45:58 -0700 Subject: 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 --- pkg/domain/infra/abi/manifest.go | 102 ++++++++++++++---------------------- pkg/domain/infra/runtime_abi.go | 2 +- pkg/domain/infra/tunnel/manifest.go | 30 +++++------ 3 files changed, 55 insertions(+), 79 deletions(-) (limited to 'pkg/domain/infra') diff --git a/pkg/domain/infra/abi/manifest.go b/pkg/domain/infra/abi/manifest.go index d1bd5e2e4..d8733130b 100644 --- a/pkg/domain/infra/abi/manifest.go +++ b/pkg/domain/infra/abi/manifest.go @@ -22,13 +22,10 @@ import ( ) // ManifestCreate implements logic for creating manifest lists via ImageEngine -func (ir *ImageEngine) ManifestCreate(ctx context.Context, names []string, images []string, opts entities.ManifestCreateOptions) (string, error) { - // FIXME: change the interface of manifest create `names []string` -> - // `name string`. - if len(names) == 0 { +func (ir *ImageEngine) ManifestCreate(ctx context.Context, name string, images []string, opts entities.ManifestCreateOptions) (string, error) { + if len(name) == 0 { return "", errors.New("no name specified for creating a manifest list") } - name := names[0] manifestList, err := ir.Libpod.LibimageRuntime().CreateManifestList(name) if err != nil { @@ -175,18 +172,12 @@ func (ir *ImageEngine) remoteManifestInspect(ctx context.Context, name string) ( } // ManifestAdd adds images to the manifest list -func (ir *ImageEngine) ManifestAdd(ctx context.Context, opts entities.ManifestAddOptions) (string, error) { - // FIXME: the name options below are *mandatory* arguments and should - // be reflected as such in the signature. - - if len(opts.Images) < 2 { - return "", errors.New("manifest add requires two images") +func (ir *ImageEngine) ManifestAdd(ctx context.Context, name string, images []string, opts entities.ManifestAddOptions) (string, error) { + if len(images) < 1 { + return "", errors.New("manifest add requires at least one image") } - imageName := opts.Images[0] - listName := opts.Images[1] - - manifestList, err := ir.Libpod.LibimageRuntime().LookupManifestList(listName) + manifestList, err := ir.Libpod.LibimageRuntime().LookupManifestList(name) if err != nil { return "", err } @@ -200,53 +191,46 @@ func (ir *ImageEngine) ManifestAdd(ctx context.Context, opts entities.ManifestAd Password: opts.Password, } - instanceDigest, err := manifestList.Add(ctx, imageName, addOptions) - if err != nil { - return "", err - } + for _, image := range images { + instanceDigest, err := manifestList.Add(ctx, image, addOptions) + if err != nil { + return "", err + } - annotateOptions := &libimage.ManifestListAnnotateOptions{ - Architecture: opts.Arch, - Features: opts.Features, - OS: opts.OS, - OSVersion: opts.OSVersion, - Variant: opts.Variant, - } - if len(opts.Annotation) != 0 { - annotations := make(map[string]string) - for _, annotationSpec := range opts.Annotation { - spec := strings.SplitN(annotationSpec, "=", 2) - if len(spec) != 2 { - return "", errors.Errorf("no value given for annotation %q", spec[0]) + annotateOptions := &libimage.ManifestListAnnotateOptions{ + Architecture: opts.Arch, + Features: opts.Features, + OS: opts.OS, + OSVersion: opts.OSVersion, + Variant: opts.Variant, + } + if len(opts.Annotation) != 0 { + annotations := make(map[string]string) + for _, annotationSpec := range opts.Annotation { + spec := strings.SplitN(annotationSpec, "=", 2) + if len(spec) != 2 { + return "", errors.Errorf("no value given for annotation %q", spec[0]) + } + annotations[spec[0]] = spec[1] } - annotations[spec[0]] = spec[1] + annotateOptions.Annotations = annotations } - annotateOptions.Annotations = annotations - } - if err := manifestList.AnnotateInstance(instanceDigest, annotateOptions); err != nil { - return "", err + if err := manifestList.AnnotateInstance(instanceDigest, annotateOptions); err != nil { + return "", err + } } - return manifestList.ID(), nil } // ManifestAnnotate updates an entry of the manifest list -func (ir *ImageEngine) ManifestAnnotate(ctx context.Context, names []string, opts entities.ManifestAnnotateOptions) (string, error) { - // FIXME: the `names` are *mandatory* arguments and should be - // reflected as such in the signature. - - if len(names) < 2 { - return "", errors.New("manifest annotate requires two names") - } - - listName := names[0] - instanceDigest, err := digest.Parse(names[1]) +func (ir *ImageEngine) ManifestAnnotate(ctx context.Context, name, image string, opts entities.ManifestAnnotateOptions) (string, error) { + instanceDigest, err := digest.Parse(image) if err != nil { - return "", errors.Errorf(`invalid image digest "%s": %v`, names[1], err) + return "", errors.Errorf(`invalid image digest "%s": %v`, image, err) } - manifestList, err := ir.Libpod.LibimageRuntime().LookupManifestList(listName) + manifestList, err := ir.Libpod.LibimageRuntime().LookupManifestList(name) if err != nil { return "", err } @@ -277,22 +261,14 @@ func (ir *ImageEngine) ManifestAnnotate(ctx context.Context, names []string, opt return manifestList.ID(), nil } -// ManifestRemove removes specified digest from the specified manifest list -func (ir *ImageEngine) ManifestRemove(ctx context.Context, names []string) (string, error) { - // FIXME: the `names` are *mandatory* arguments and should be - // reflected as such in the signature. - - if len(names) < 2 { - return "", errors.New("manifest remove requires two names") - } - - listName := names[0] - instanceDigest, err := digest.Parse(names[1]) +// ManifestRemoveDigest removes specified digest from the specified manifest list +func (ir *ImageEngine) ManifestRemoveDigest(ctx context.Context, name, image string) (string, error) { + instanceDigest, err := digest.Parse(image) if err != nil { - return "", errors.Errorf(`invalid image digest "%s": %v`, names[1], err) + return "", errors.Errorf(`invalid image digest "%s": %v`, image, err) } - manifestList, err := ir.Libpod.LibimageRuntime().LookupManifestList(listName) + manifestList, err := ir.Libpod.LibimageRuntime().LookupManifestList(name) if err != nil { return "", err } diff --git a/pkg/domain/infra/runtime_abi.go b/pkg/domain/infra/runtime_abi.go index 177e9cff4..ec7de2743 100644 --- a/pkg/domain/infra/runtime_abi.go +++ b/pkg/domain/infra/runtime_abi.go @@ -26,7 +26,7 @@ func NewContainerEngine(facts *entities.PodmanConfig) (entities.ContainerEngine, return nil, fmt.Errorf("runtime mode '%v' is not supported", facts.EngineMode) } -// NewContainerEngine factory provides a libpod runtime for image-related operations +// NewImageEngine factory provides a libpod runtime for image-related operations func NewImageEngine(facts *entities.PodmanConfig) (entities.ImageEngine, error) { switch facts.EngineMode { case entities.ABIMode: diff --git a/pkg/domain/infra/tunnel/manifest.go b/pkg/domain/infra/tunnel/manifest.go index 62634f561..f89c59bc7 100644 --- a/pkg/domain/infra/tunnel/manifest.go +++ b/pkg/domain/infra/tunnel/manifest.go @@ -7,16 +7,16 @@ import ( "strings" "github.com/containers/image/v5/types" - images "github.com/containers/podman/v3/pkg/bindings/images" + "github.com/containers/podman/v3/pkg/bindings/images" "github.com/containers/podman/v3/pkg/bindings/manifests" "github.com/containers/podman/v3/pkg/domain/entities" "github.com/pkg/errors" ) // ManifestCreate implements manifest create via ImageEngine -func (ir *ImageEngine) ManifestCreate(ctx context.Context, names, images []string, opts entities.ManifestCreateOptions) (string, error) { +func (ir *ImageEngine) ManifestCreate(ctx context.Context, name string, images []string, opts entities.ManifestCreateOptions) (string, error) { options := new(manifests.CreateOptions).WithAll(opts.All) - imageID, err := manifests.Create(ir.ClientCtx, names, images, options) + imageID, err := manifests.Create(ir.ClientCtx, name, images, options) if err != nil { return imageID, errors.Wrapf(err, "error creating manifest") } @@ -33,7 +33,7 @@ func (ir *ImageEngine) ManifestExists(ctx context.Context, name string) (*entiti } // ManifestInspect returns contents of manifest list with given name -func (ir *ImageEngine) ManifestInspect(ctx context.Context, name string) ([]byte, error) { +func (ir *ImageEngine) ManifestInspect(_ context.Context, name string) ([]byte, error) { list, err := manifests.Inspect(ir.ClientCtx, name, nil) if err != nil { return nil, errors.Wrapf(err, "error getting content of manifest list or image %s", name) @@ -47,9 +47,9 @@ func (ir *ImageEngine) ManifestInspect(ctx context.Context, name string) ([]byte } // ManifestAdd adds images to the manifest list -func (ir *ImageEngine) ManifestAdd(ctx context.Context, opts entities.ManifestAddOptions) (string, error) { +func (ir *ImageEngine) ManifestAdd(_ context.Context, name string, imageNames []string, opts entities.ManifestAddOptions) (string, error) { options := new(manifests.AddOptions).WithAll(opts.All).WithArch(opts.Arch).WithVariant(opts.Variant) - options.WithFeatures(opts.Features).WithImages(opts.Images).WithOS(opts.OS).WithOSVersion(opts.OSVersion) + options.WithFeatures(opts.Features).WithImages(imageNames).WithOS(opts.OS).WithOSVersion(opts.OSVersion) if len(opts.Annotation) != 0 { annotations := make(map[string]string) for _, annotationSpec := range opts.Annotation { @@ -62,25 +62,25 @@ func (ir *ImageEngine) ManifestAdd(ctx context.Context, opts entities.ManifestAd options.WithAnnotation(annotations) } - listID, err := manifests.Add(ir.ClientCtx, opts.Images[1], options) + id, err := manifests.Add(ir.ClientCtx, name, options) if err != nil { - return listID, errors.Wrapf(err, "error adding to manifest list %s", opts.Images[1]) + return id, errors.Wrapf(err, "error adding to manifest list %s", name) } - return listID, nil + return id, nil } // ManifestAnnotate updates an entry of the manifest list -func (ir *ImageEngine) ManifestAnnotate(ctx context.Context, names []string, opts entities.ManifestAnnotateOptions) (string, error) { +func (ir *ImageEngine) ManifestAnnotate(ctx context.Context, name, images string, opts entities.ManifestAnnotateOptions) (string, error) { return "", errors.New("not implemented") } -// ManifestRemove removes the digest from manifest list -func (ir *ImageEngine) ManifestRemove(ctx context.Context, names []string) (string, error) { - updatedListID, err := manifests.Remove(ir.ClientCtx, names[0], names[1], nil) +// ManifestRemoveDigest removes the digest from manifest list +func (ir *ImageEngine) ManifestRemoveDigest(ctx context.Context, name string, image string) (string, error) { + updatedListID, err := manifests.Remove(ir.ClientCtx, name, image, nil) if err != nil { - return updatedListID, errors.Wrapf(err, "error removing from manifest %s", names[0]) + return updatedListID, errors.Wrapf(err, "error removing from manifest %s", name) } - return fmt.Sprintf("%s :%s\n", updatedListID, names[1]), nil + return fmt.Sprintf("%s :%s\n", updatedListID, image), nil } // ManifestRm removes the specified manifest list from storage -- cgit v1.2.3-54-g00ecf