From 5181becfdec31bd362bce4b0d0e811b20df3aa6e Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Mon, 25 Nov 2024 15:13:52 -0500 Subject: [PATCH] Add podman manifest rm --ignore When removing manifests, users should be allowed to ignore ones that no longer exists. Signed-off-by: Daniel J Walsh --- cmd/podman/manifest/rm.go | 11 +++-- docs/source/markdown/podman-manifest-rm.1.md | 8 +++- pkg/api/handlers/compat/images_remove.go | 2 + pkg/api/handlers/libpod/images.go | 3 +- pkg/api/handlers/libpod/manifests.go | 43 +++++++++++++++----- pkg/api/server/register_manifest.go | 4 ++ pkg/domain/entities/engine_image.go | 2 +- pkg/domain/infra/abi/manifest.go | 4 +- pkg/domain/infra/tunnel/manifest.go | 4 +- test/apiv2/15-manifest.at | 3 ++ test/system/012-manifest.bats | 4 ++ 11 files changed, 68 insertions(+), 20 deletions(-) diff --git a/cmd/podman/manifest/rm.go b/cmd/podman/manifest/rm.go index f5f13a86e6..c182b46480 100644 --- a/cmd/podman/manifest/rm.go +++ b/cmd/podman/manifest/rm.go @@ -6,13 +6,15 @@ import ( "github.com/containers/podman/v5/cmd/podman/common" "github.com/containers/podman/v5/cmd/podman/registry" + "github.com/containers/podman/v5/pkg/domain/entities" "github.com/containers/podman/v5/pkg/errorhandling" "github.com/spf13/cobra" ) var ( - rmCmd = &cobra.Command{ - Use: "rm LIST [LIST...]", + rmOptions = entities.ImageRemoveOptions{} + rmCmd = &cobra.Command{ + Use: "rm [options] LIST [LIST...]", Short: "Remove manifest list or image index from local storage", Long: "Remove manifest list or image index from local storage.", RunE: rm, @@ -27,10 +29,13 @@ func init() { Command: rmCmd, Parent: manifestCmd, }) + + flags := rmCmd.Flags() + flags.BoolVarP(&rmOptions.Ignore, "ignore", "i", false, "Ignore errors when a specified manifest is missing") } func rm(cmd *cobra.Command, args []string) error { - report, rmErrors := registry.ImageEngine().ManifestRm(context.Background(), args) + report, rmErrors := registry.ImageEngine().ManifestRm(context.Background(), args, rmOptions) if report != nil { for _, u := range report.Untagged { fmt.Println("Untagged: " + u) diff --git a/docs/source/markdown/podman-manifest-rm.1.md b/docs/source/markdown/podman-manifest-rm.1.md index 2c5bbf0096..0613db19fa 100644 --- a/docs/source/markdown/podman-manifest-rm.1.md +++ b/docs/source/markdown/podman-manifest-rm.1.md @@ -4,11 +4,17 @@ podman\-manifest\-rm - Remove manifest list or image index from local storage ## SYNOPSIS -**podman manifest rm** *list-or-index* [...] +**podman manifest rm** [*options*] *list-or-index* [...] ## DESCRIPTION Removes one or more locally stored manifest lists. +## OPTIONS + +#### **--ignore**, **-i** + +If a specified manifest does not exist in the local storage, ignore it and do not throw an error. + ## EXAMPLE podman manifest rm `` diff --git a/pkg/api/handlers/compat/images_remove.go b/pkg/api/handlers/compat/images_remove.go index 5dd7a56958..8379da99b1 100644 --- a/pkg/api/handlers/compat/images_remove.go +++ b/pkg/api/handlers/compat/images_remove.go @@ -22,6 +22,7 @@ func RemoveImage(w http.ResponseWriter, r *http.Request) { query := struct { Force bool `schema:"force"` NoPrune bool `schema:"noprune"` + Ignore bool `schema:"ignore"` }{ // This is where you can override the golang default value for one of fields } @@ -42,6 +43,7 @@ func RemoveImage(w http.ResponseWriter, r *http.Request) { options := entities.ImageRemoveOptions{ Force: query.Force, NoPrune: query.NoPrune, + Ignore: query.Ignore, } report, rmerrors := imageEngine.Remove(r.Context(), []string{possiblyNormalizedName}, options) if len(rmerrors) > 0 && rmerrors[0] != nil { diff --git a/pkg/api/handlers/libpod/images.go b/pkg/api/handlers/libpod/images.go index 3a86121f7d..0356b6e145 100644 --- a/pkg/api/handlers/libpod/images.go +++ b/pkg/api/handlers/libpod/images.go @@ -668,6 +668,7 @@ func ImagesRemove(w http.ResponseWriter, r *http.Request) { query := struct { Force bool `schema:"force"` LookupManifest bool `schema:"lookupManifest"` + Ignore bool `schema:"ignore"` }{ Force: false, } @@ -677,7 +678,7 @@ func ImagesRemove(w http.ResponseWriter, r *http.Request) { return } - opts := entities.ImageRemoveOptions{Force: query.Force, LookupManifest: query.LookupManifest} + opts := entities.ImageRemoveOptions{Force: query.Force, LookupManifest: query.LookupManifest, Ignore: query.Ignore} imageEngine := abi.ImageEngine{Libpod: runtime} rmReport, rmErrors := imageEngine.Remove(r.Context(), []string{utils.GetName(r)}, opts) diff --git a/pkg/api/handlers/libpod/manifests.go b/pkg/api/handlers/libpod/manifests.go index 8f19554de9..ca1ae26673 100644 --- a/pkg/api/handlers/libpod/manifests.go +++ b/pkg/api/handlers/libpod/manifests.go @@ -745,20 +745,43 @@ func ManifestModify(w http.ResponseWriter, r *http.Request) { // ManifestDelete removes a manifest list from storage func ManifestDelete(w http.ResponseWriter, r *http.Request) { + decoder := r.Context().Value(api.DecoderKey).(*schema.Decoder) runtime := r.Context().Value(api.RuntimeKey).(*libpod.Runtime) imageEngine := abi.ImageEngine{Libpod: runtime} - name := utils.GetName(r) - if _, err := runtime.LibimageRuntime().LookupManifestList(name); err != nil { - utils.Error(w, http.StatusNotFound, err) - return + query := struct { + Ignore bool `schema:"ignore"` + }{ + // Add defaults here once needed. } - results, errs := imageEngine.ManifestRm(r.Context(), []string{name}) - errsString := errorhandling.ErrorsToStrings(errs) - report := handlers.LibpodImagesRemoveReport{ - ImageRemoveReport: *results, - Errors: errsString, + if err := decoder.Decode(&query, r.URL.Query()); err != nil { + utils.Error(w, http.StatusBadRequest, + fmt.Errorf("failed to parse parameters for %s: %w", r.URL.String(), err)) + return + } + opts := entities.ImageRemoveOptions{ + Ignore: query.Ignore, + } + + name := utils.GetName(r) + rmReport, rmErrors := imageEngine.ManifestRm(r.Context(), []string{name}, opts) + // In contrast to batch-removal, where we're only setting the exit + // code, we need to have another closer look at the errors here and set + // the appropriate http status code. + + switch rmReport.ExitCode { + case 0: + report := handlers.LibpodImagesRemoveReport{ImageRemoveReport: *rmReport, Errors: []string{}} + utils.WriteResponse(w, http.StatusOK, report) + case 1: + // 404 - no such image + utils.Error(w, http.StatusNotFound, errorhandling.JoinErrors(rmErrors)) + case 2: + // 409 - conflict error (in use by containers) + utils.Error(w, http.StatusConflict, errorhandling.JoinErrors(rmErrors)) + default: + // 500 - internal error + utils.Error(w, http.StatusInternalServerError, errorhandling.JoinErrors(rmErrors)) } - utils.WriteResponse(w, http.StatusOK, report) } diff --git a/pkg/api/server/register_manifest.go b/pkg/api/server/register_manifest.go index a6de54d0b6..ff629cf24b 100644 --- a/pkg/api/server/register_manifest.go +++ b/pkg/api/server/register_manifest.go @@ -322,6 +322,10 @@ func (s *APIServer) registerManifestHandlers(r *mux.Router) error { // type: string // required: true // description: The name or ID of the list to be deleted + // - in: query + // name: ignore + // description: Ignore if a specified manifest does not exist and do not throw an error. + // type: boolean // responses: // 200: // $ref: "#/responses/imagesRemoveResponseLibpod" diff --git a/pkg/domain/entities/engine_image.go b/pkg/domain/entities/engine_image.go index 65c8cd8150..04b215ac82 100644 --- a/pkg/domain/entities/engine_image.go +++ b/pkg/domain/entities/engine_image.go @@ -45,7 +45,7 @@ type ImageEngine interface { //nolint:interfacebloat ManifestAddArtifact(ctx context.Context, name string, files []string, opts ManifestAddArtifactOptions) (string, error) ManifestAnnotate(ctx context.Context, names, image string, opts ManifestAnnotateOptions) (string, error) ManifestRemoveDigest(ctx context.Context, names, image string) (string, error) - ManifestRm(ctx context.Context, names []string) (*ImageRemoveReport, []error) + ManifestRm(ctx context.Context, names []string, imageRmOpts ImageRemoveOptions) (*ImageRemoveReport, []error) ManifestPush(ctx context.Context, name, destination string, imagePushOpts ImagePushOptions) (string, error) ManifestListClear(ctx context.Context, name string) (string, error) Sign(ctx context.Context, names []string, options SignOptions) (*SignReport, error) diff --git a/pkg/domain/infra/abi/manifest.go b/pkg/domain/infra/abi/manifest.go index 47b15718aa..52b731588c 100644 --- a/pkg/domain/infra/abi/manifest.go +++ b/pkg/domain/infra/abi/manifest.go @@ -460,8 +460,8 @@ func (ir *ImageEngine) ManifestRemoveDigest(ctx context.Context, name, image str } // ManifestRm removes the specified manifest list from storage -func (ir *ImageEngine) ManifestRm(ctx context.Context, names []string) (report *entities.ImageRemoveReport, rmErrors []error) { - return ir.Remove(ctx, names, entities.ImageRemoveOptions{LookupManifest: true}) +func (ir *ImageEngine) ManifestRm(ctx context.Context, names []string, opts entities.ImageRemoveOptions) (report *entities.ImageRemoveReport, rmErrors []error) { + return ir.Remove(ctx, names, entities.ImageRemoveOptions{LookupManifest: true, Ignore: opts.Ignore}) } // ManifestPush pushes a manifest list or image index to the destination diff --git a/pkg/domain/infra/tunnel/manifest.go b/pkg/domain/infra/tunnel/manifest.go index 7b98d6bc88..d89ff80699 100644 --- a/pkg/domain/infra/tunnel/manifest.go +++ b/pkg/domain/infra/tunnel/manifest.go @@ -178,8 +178,8 @@ func (ir *ImageEngine) ManifestRemoveDigest(ctx context.Context, name string, im } // ManifestRm removes the specified manifest list from storage -func (ir *ImageEngine) ManifestRm(ctx context.Context, names []string) (*entities.ImageRemoveReport, []error) { - return ir.Remove(ctx, names, entities.ImageRemoveOptions{LookupManifest: true}) +func (ir *ImageEngine) ManifestRm(ctx context.Context, names []string, opts entities.ImageRemoveOptions) (report *entities.ImageRemoveReport, rmErrors []error) { + return ir.Remove(ctx, names, entities.ImageRemoveOptions{LookupManifest: true, Ignore: opts.Ignore}) } // ManifestPush pushes a manifest list or image index to the destination diff --git a/test/apiv2/15-manifest.at b/test/apiv2/15-manifest.at index 44f3c7c83e..630d267e79 100644 --- a/test/apiv2/15-manifest.at +++ b/test/apiv2/15-manifest.at @@ -66,6 +66,9 @@ t POST "/v4.0.0/libpod/manifests/xyz:latest/registry/localhost:$REGISTRY_PORT%2F # /v3.x cannot delete a manifest list t DELETE /v4.0.0/libpod/manifests/$id_abc 200 t DELETE /v4.0.0/libpod/manifests/$id_xyz 200 +t GET /v4.0.0/libpod/manifests/$id_xyz/exists 404 +t DELETE /v4.0.0/libpod/manifests/$id_xyz 404 +t DELETE /v4.0.0/libpod/manifests/$id_xyz?ignore=true 200 # manifest add --artifact tests truncate -s 20M $WORKDIR/zeroes diff --git a/test/system/012-manifest.bats b/test/system/012-manifest.bats index 920a09e3e0..4b70e82519 100644 --- a/test/system/012-manifest.bats +++ b/test/system/012-manifest.bats @@ -94,6 +94,10 @@ function validate_instance_compression { --tls-verify=false $mid \ $manifest1 run_podman manifest rm $manifest1 + run_podman 1 manifest rm $manifest1 + is "$output" "Error: $manifest1: image not known" "Missing manifest is reported" + run_podman manifest rm --ignore $manifest1 + is "$output" "" "Missing manifest is ignored" # Default is to require TLS; also test explicit opts for opt in '' '--insecure=false' '--tls-verify=true' "--authfile=$authfile"; do