feat: Add artifact remove --all option

Prior to this commit `artifact remove --all` was not supported on remote
clients.

This patch adds a new artifact API endpoint `artifact/remove` which can
either take a list of artifacts to remove or remove all artifacts by
setting all=true.

This patch removes the temporary warning message in the tunnel interface
implementation of ArtifactRm if `--all` was passed on the command line
and uses the new `artifact/remove` endpoint.

This patch also updates the `artifact remove` command both remote and
local to accept a list of artifacts to remove rather than limiting to
just one.

Signed-off-by: Lewis Roy <lewis@redhat.com>

cmd/podman/artifact/rm.go

@@ -12,17 +12,18 @@ import (
 
 var (
 	rmCmd = &cobra.Command{
-		Use:     "rm [options] ARTIFACT",
-		Short:   "Remove an OCI artifact",
-		Long:    "Remove an OCI artifact from local storage",
-		RunE:    rm,
-		Aliases: []string{"remove"},
-		Args: func(cmd *cobra.Command, args []string) error { //nolint: gocritic
-			return checkAllAndArgs(cmd, args)
-		},
+		Use:               "rm [options] ARTIFACT [ARTIFACT...]",
+		Short:             "Remove one or more OCI artifacts",
+		Long:              "Remove one or more OCI artifacts from local storage",
+		RunE:              rm,
+		Aliases:           []string{"remove"},
+		Args:              checkAllAndArgs,
 		ValidArgsFunction: common.AutocompleteArtifacts,
-		Example: `podman artifact rm quay.io/myimage/myartifact:latest
-podman artifact rm -a`,
+		Example: `
+  podman artifact rm quay.io/myimage/myartifact:latest
+  podman artifact rm -a
+  podman artifact rm c4dfb1609ee2 93fd78260bd1 c0ed59d05ff7
+		`,
 	}
 
 	rmOptions = entities.ArtifactRemoveOptions{}
@@ -41,11 +42,9 @@ func init() {
 }
 
 func rm(cmd *cobra.Command, args []string) error {
-	var nameOrID string
-	if len(args) > 0 {
-		nameOrID = args[0]
-	}
-	artifactRemoveReport, err := registry.ImageEngine().ArtifactRm(registry.Context(), nameOrID, rmOptions)
+	rmOptions.Artifacts = args
+
+	artifactRemoveReport, err := registry.ImageEngine().ArtifactRm(registry.Context(), rmOptions)
 	if err != nil {
 		return err
 	}
@@ -67,10 +66,7 @@ func checkAllAndArgs(c *cobra.Command, args []string) error {
 	}
 	if !all {
 		if len(args) < 1 {
-			return errors.New("a single artifact name or digest must be specified")
-		}
-		if len(args) > 1 {
-			return errors.New("too many arguments: only accepts one artifact name or digest ")
+			return errors.New("at least one artifact name or digest must be specified")
 		}
 	}
 	return nil

docs/source/markdown/podman-artifact-rm.1.md

@@ -1,14 +1,14 @@
 % podman-artifact-rm 1
 
 ## NAME
-podman\-artifact\-rm - Remove an OCI from local storage
+podman\-artifact\-rm - Remove one or more OCI artifacts from local storage
 
 ## SYNOPSIS
 **podman artifact rm** [*options*] *name*
 
 ## DESCRIPTION
 
-Remove an artifact from the local artifact store.  The input may be the fully
+Remove one or more artifacts from the local artifact store.  The input may be the fully
 qualified artifact name or a full or partial artifact digest.
 
 ## OPTIONS
@@ -25,21 +25,24 @@ Print usage statement.
 
 ## EXAMPLES
 
-Remove an artifact by name
-
+Remove an artifact by name.
 ```
 $ podman artifact rm quay.io/artifact/foobar2:test
 Deleted: e7b417f49fc24fc7ead6485da0ebd5bc4419d8a3f394c169fee5a6f38faa4056
 ```
 
-Remove an artifact by partial digest
+Remove multiple artifacts by their shortened IDs.
+```
+$ podman artifact rm c4dfb1609ee2 93fd78260bd1 c0ed59d05ff7
+```
 
+Remove an artifact by partial digest.
 ```
 $ podman artifact rm e7b417f49fc
 Deleted: e7b417f49fc24fc7ead6485da0ebd5bc4419d8a3f394c169fee5a6f38faa4056
 ```
 
-Remove all artifacts in local storage
+Remove all artifacts in local storage.
 ```
 $ podman artifact rm -a
 Deleted: cee15f7c5ce3e86ae6ce60d84bebdc37ad34acfa9a2611cf47501469ac83a1ab

docs/source/markdown/podman-artifact.1.md

@@ -23,7 +23,7 @@ from its local "artifact store".
 | ls      | [podman-artifact-ls(1)](podman-artifact-ls.1.md)           | List OCI artifacts in local store                            |
 | pull    | [podman-artifact-pull(1)](podman-artifact-pull.1.md)       | Pulls an artifact from a registry and stores it locally      |
 | push    | [podman-artifact-push(1)](podman-artifact-push.1.md)       | Push an OCI artifact from local storage to an image registry |
-| rm      | [podman-artifact-rm(1)](podman-artifact-rm.1.md)           | Remove an OCI from local storage                             |
+| rm      | [podman-artifact-rm(1)](podman-artifact-rm.1.md)           | Remove one or more OCI artifacts from local storage          |
 
 
 ## SEE ALSO

pkg/api/handlers/libpod/artifacts.go

@@ -149,7 +149,7 @@ func RemoveArtifact(w http.ResponseWriter, r *http.Request) {
 
 	name := utils.GetName(r)
 
-	artifacts, err := imageEngine.ArtifactRm(r.Context(), name, entities.ArtifactRemoveOptions{})
+	artifacts, err := imageEngine.ArtifactRm(r.Context(), entities.ArtifactRemoveOptions{Artifacts: []string{name}})
 	if err != nil {
 		if errors.Is(err, libartifact_types.ErrArtifactNotExist) {
 			utils.ArtifactNotFound(w, name, err)
@@ -162,6 +162,50 @@ func RemoveArtifact(w http.ResponseWriter, r *http.Request) {
 	utils.WriteResponse(w, http.StatusOK, artifacts)
 }
 
+func BatchRemoveArtifact(w http.ResponseWriter, r *http.Request) {
+	runtime := r.Context().Value(api.RuntimeKey).(*libpod.Runtime)
+	decoder := r.Context().Value(api.DecoderKey).(*schema.Decoder)
+
+	query := struct {
+		All       bool     `schema:"all"`
+		Artifacts []string `schema:"artifacts"`
+	}{}
+
+	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
+	}
+
+	if query.All && len(query.Artifacts) > 0 {
+		utils.Error(w, http.StatusBadRequest, errors.New("when setting all to true, you may not pass any artifact names or digests"))
+		return
+	}
+
+	if !query.All && len(query.Artifacts) < 1 {
+		utils.Error(w, http.StatusBadRequest, errors.New("an artifact or all option must be specified"))
+		return
+	}
+
+	imageEngine := abi.ImageEngine{Libpod: runtime}
+
+	removeOptions := entities.ArtifactRemoveOptions{
+		Artifacts: query.Artifacts,
+		All:       query.All,
+	}
+
+	artifacts, err := imageEngine.ArtifactRm(r.Context(), removeOptions)
+	if err != nil {
+		if errors.Is(err, libartifact_types.ErrArtifactNotExist) {
+			utils.ArtifactNotFound(w, "", err)
+			return
+		}
+		utils.InternalServerError(w, err)
+		return
+	}
+
+	utils.WriteResponse(w, http.StatusOK, artifacts)
+}
+
 func AddArtifact(w http.ResponseWriter, r *http.Request) {
 	runtime := r.Context().Value(api.RuntimeKey).(*libpod.Runtime)
 	decoder := r.Context().Value(api.DecoderKey).(*schema.Decoder)

pkg/api/server/register_artifacts.go

@@ -94,6 +94,33 @@ func (s *APIServer) registerArtifactHandlers(r *mux.Router) error {
 	//   500:
 	//     $ref: "#/responses/internalError"
 	r.Handle(VersionedPath("/libpod/artifacts/pull"), s.APIHandler(libpod.PullArtifact)).Methods(http.MethodPost)
+	// swagger:operation DELETE /libpod/artifacts/remove libpod ArtifactDeleteAllLibpod
+	// ---
+	// tags:
+	//  - artifacts
+	// summary: Remove one or more Artifacts from local storage.
+	// description: Remove one or more Artifacts from local storage.
+	// produces:
+	//  - application/json
+	// parameters:
+	//  - name: artifacts
+	//    in: query
+	//    description: Artifact IDs or names to remove
+	//    type: array
+	//    items:
+	//        type: string
+	//  - name: all
+	//    in: query
+	//    description: Remove all Artifacts
+	//    type: boolean
+	// responses:
+	//   200:
+	//     $ref: "#/responses/artifactRemoveResponse"
+	//   404:
+	//     $ref: "#/responses/artifactNotFound"
+	//   500:
+	//     $ref: "#/responses/internalError"
+	r.Handle(VersionedPath("/libpod/artifacts/remove"), s.APIHandler(libpod.BatchRemoveArtifact)).Methods(http.MethodDelete)
 	// swagger:operation DELETE /libpod/artifacts/{name} libpod ArtifactDeleteLibpod
 	// ---
 	// tags:

pkg/bindings/artifacts/remove.go

@@ -9,13 +9,23 @@ import (
 )
 
 // Remove removes an artifact from local storage.
+// TODO (6.0): nameOrID parameter should be removed
 func Remove(ctx context.Context, nameOrID string, options *RemoveOptions) (*entities.ArtifactRemoveReport, error) {
 	conn, err := bindings.GetClient(ctx)
 	if err != nil {
 		return nil, err
 	}
 
-	response, err := conn.DoRequest(ctx, nil, http.MethodDelete, "/artifacts/%s", nil, nil, nameOrID)
+	if nameOrID != "" {
+		options.Artifacts = append(options.Artifacts, nameOrID)
+	}
+
+	params, err := options.ToParams()
+	if err != nil {
+		return nil, err
+	}
+
+	response, err := conn.DoRequest(ctx, nil, http.MethodDelete, "/artifacts/remove", params, nil)
 	if err != nil {
 		return nil, err
 	}

pkg/bindings/artifacts/types.go

@@ -50,6 +50,8 @@ type PushOptions struct {
 type RemoveOptions struct {
 	// Remove all artifacts
 	All *bool
+	// Artifacts is a list of Artifact IDs or names to remove
+	Artifacts []string
 }
 
 // AddOptions are optional options for removing images

pkg/bindings/artifacts/types_remove_options.go

@@ -31,3 +31,18 @@ func (o *RemoveOptions) GetAll() bool {
 	}
 	return *o.All
 }
+
+// WithArtifacts set field Artifacts to given value
+func (o *RemoveOptions) WithArtifacts(value []string) *RemoveOptions {
+	o.Artifacts = value
+	return o
+}
+
+// GetArtifacts returns value of field Artifacts
+func (o *RemoveOptions) GetArtifacts() []string {
+	if o.Artifacts == nil {
+		var z []string
+		return z
+	}
+	return o.Artifacts
+}

pkg/domain/entities/artifact.go

@@ -94,6 +94,8 @@ type ArtifactPushReport = entitiesTypes.ArtifactPushReport
 type ArtifactRemoveOptions struct {
 	// Remove all artifacts
 	All bool
+	// Artifacts is a list of Artifact IDs or names to remove
+	Artifacts []string
 }
 
 type ArtifactRemoveReport = entitiesTypes.ArtifactRemoveReport

pkg/domain/entities/engine_image.go

@@ -17,7 +17,7 @@ type ImageEngine interface { //nolint:interfacebloat
 	ArtifactList(ctx context.Context, opts ArtifactListOptions) ([]*ArtifactListReport, error)
 	ArtifactPull(ctx context.Context, name string, opts ArtifactPullOptions) (*ArtifactPullReport, error)
 	ArtifactPush(ctx context.Context, name string, opts ArtifactPushOptions) (*ArtifactPushReport, error)
-	ArtifactRm(ctx context.Context, name string, opts ArtifactRemoveOptions) (*ArtifactRemoveReport, error)
+	ArtifactRm(ctx context.Context, opts ArtifactRemoveOptions) (*ArtifactRemoveReport, error)
 	Build(ctx context.Context, containerFiles []string, opts BuildOptions) (*BuildReport, error)
 	Config(ctx context.Context) (*config.Config, error)
 	Exists(ctx context.Context, nameOrID string) (*BoolReport, error)

pkg/domain/infra/abi/artifact.go

@@ -90,7 +90,7 @@ func (ir *ImageEngine) ArtifactPull(ctx context.Context, name string, opts entit
 	}, nil
 }
 
-func (ir *ImageEngine) ArtifactRm(ctx context.Context, name string, opts entities.ArtifactRemoveOptions) (*entities.ArtifactRemoveReport, error) {
+func (ir *ImageEngine) ArtifactRm(ctx context.Context, opts entities.ArtifactRemoveOptions) (*entities.ArtifactRemoveReport, error) {
 	var (
 		namesOrDigests []string
 	)
@@ -115,8 +115,9 @@ func (ir *ImageEngine) ArtifactRm(ctx context.Context, name string, opts entitie
 		}
 	}
 
-	if name != "" {
-		namesOrDigests = append(namesOrDigests, name)
+	// NOTE: If opts.All is true, len(opts.Artifacts) will == 0
+	if len(opts.Artifacts) != 0 {
+		namesOrDigests = append(namesOrDigests, opts.Artifacts...)
 	}
 
 	artifactDigests := make([]*digest.Digest, 0, len(namesOrDigests))

pkg/domain/infra/tunnel/artifact.go

@@ -53,12 +53,13 @@ func (ir *ImageEngine) ArtifactPull(_ context.Context, name string, opts entitie
 	return artifacts.Pull(ir.ClientCtx, name, &options)
 }
 
-func (ir *ImageEngine) ArtifactRm(_ context.Context, name string, opts entities.ArtifactRemoveOptions) (*entities.ArtifactRemoveReport, error) {
-	if opts.All {
-		// Note: This will be added when artifacts remove all endpoint is implemented
-		return nil, fmt.Errorf("not implemented")
+func (ir *ImageEngine) ArtifactRm(_ context.Context, opts entities.ArtifactRemoveOptions) (*entities.ArtifactRemoveReport, error) {
+	removeOptions := artifacts.RemoveOptions{
+		All:       &opts.All,
+		Artifacts: opts.Artifacts,
 	}
-	return artifacts.Remove(ir.ClientCtx, name, &artifacts.RemoveOptions{})
+
+	return artifacts.Remove(ir.ClientCtx, "", &removeOptions)
 }
 
 func (ir *ImageEngine) ArtifactPush(_ context.Context, name string, opts entities.ArtifactPushOptions) (*entities.ArtifactPushReport, error) {
@@ -106,7 +107,10 @@ func (ir *ImageEngine) ArtifactAdd(_ context.Context, name string, artifactBlob
 
 		artifactAddReport, err = artifacts.Add(ir.ClientCtx, name, blob.FileName, f, &options)
 		if err != nil && i > 0 {
-			_, recoverErr := artifacts.Remove(ir.ClientCtx, name, &artifacts.RemoveOptions{})
+			removeOptions := artifacts.RemoveOptions{
+				Artifacts: []string{name},
+			}
+			_, recoverErr := artifacts.Remove(ir.ClientCtx, "", &removeOptions)
 			if recoverErr != nil {
 				return nil, fmt.Errorf("failed to cleanup unfinished artifact add: %w", errors.Join(err, recoverErr))
 			}

test/apiv2/python/rest_api/test_v2_0_0_artifact.py

@@ -430,6 +430,103 @@ class ArtifactTestCase(APITestCase):
         # Assert return response is json and contains digest
         self.assertIn("sha256:", rjson["ArtifactDigests"][0])
 
+    def test_remove_multiple(self):
+        # Create some artifacts to remove
+        artifact_names = [
+            "quay.io/myimage/myartifact1:latest",
+            "quay.io/myimage/myartifact2:latest",
+            "quay.io/myimage/myartifact3:latest"
+        ]
+
+        for name in artifact_names:
+            file = ArtifactFile()
+            parameters: dict[str, str | list[str]] = {
+                "name": name,
+                "fileName": file.name,
+            }
+
+            artifact = Artifact(self.uri(""), name, parameters, file)
+
+            add_response = artifact.add()
+
+            # Assert correct response code
+            self.assertEqual(add_response.status_code, 201, add_response.text)
+
+        # Test remove multiple artifacts
+        removeparameters: dict[str, str | list[str]] = {
+            "Artifacts": artifact_names,
+        }
+
+        url = self.uri("/artifacts/remove")
+        r = requests.delete(url, params=removeparameters)
+        rjson = r.json()
+
+        # Assert correct response code
+        self.assertEqual(r.status_code, 200, r.text)
+
+        # Assert return response is valid json and contains multiple digests
+        self.assertEqual(len(rjson["ArtifactDigests"]), len(artifact_names))
+
+        # Test removing an artifact that doesn't exist
+        removeparameters: dict[str, str | list[str]] = {
+            "Artifacts": "fake_artifact",
+        }
+
+        url = self.uri("/artifacts/remove")
+        r = requests.delete(url, params=removeparameters)
+        rjson = r.json()
+        print(r)
+
+        # Assert correct response code
+        self.assertEqual(r.status_code, 404, r.text)
+
+        # Assert return error response is json and contains correct message
+        self.assertEqual(
+            rjson["cause"],
+            "artifact does not exist",
+        )
+
+
+    def test_remove_all(self):
+        # Create some artifacts to remove
+        artifact_names = [
+            "quay.io/myimage/myartifact1:latest",
+            "quay.io/myimage/myartifact2:latest",
+            "quay.io/myimage/myartifact3:latest"
+        ]
+
+        for name in artifact_names:
+            file = ArtifactFile()
+            parameters: dict[str, str | list[str]] = {
+                "name": name,
+                "fileName": file.name,
+            }
+
+            artifact = Artifact(self.uri(""), name, parameters, file)
+
+            add_response = artifact.add()
+
+            # Assert correct response code
+            self.assertEqual(add_response.status_code, 201, add_response.text)
+
+        # Test remove all artifacts
+        removeparameters: dict[str, str | list[str]] = {
+            "all": "true",
+        }
+
+        url = self.uri("/artifacts/remove")
+        r = requests.delete(url, params=removeparameters)
+        rjson = r.json()
+
+        # Assert correct response code
+        self.assertEqual(r.status_code, 200, r.text)
+
+        # Assert no artifacts remain
+        url = self.uri("/artifacts/json")
+        r = requests.get(url)
+        rjson = r.json()
+        self.assertEqual(len(rjson), 0)
+
     def test_remove_absent_artifact_fails(self):
         ARTIFACT_NAME = "localhost/fake/artifact:latest"
         url = self.uri("/artifacts/" + ARTIFACT_NAME)

test/e2e/artifact_test.go

@@ -237,22 +237,14 @@ var _ = Describe("Podman artifact", func() {
 		// No args is an error
 		failNoArgs := podmanTest.Podman([]string{"artifact", "rm"})
 		failNoArgs.WaitWithDefaultTimeout()
-		Expect(failNoArgs).Should(ExitWithError(125, "Error: a single artifact name or digest must be specified"))
+		Expect(failNoArgs).Should(ExitWithError(125, "Error: at least one artifact name or digest must be specified"))
 
-		// Multiple args is an error
-		multipleArgs := podmanTest.Podman([]string{"artifact", "rm", artifact1Name, artifact2File})
-		multipleArgs.WaitWithDefaultTimeout()
-		Expect(multipleArgs).Should(ExitWithError(125, "Error: too many arguments: only accepts one artifact name or digest"))
+		// Remove all
+		podmanTest.PodmanExitCleanly("artifact", "rm", "-a")
 
-		// TODO: This should be removed once Artifact API remove endpoint supports the "all" flag
-		if !IsRemote() {
-			// Remove all
-			podmanTest.PodmanExitCleanly("artifact", "rm", "-a")
-
-			// There should be no artifacts in the store
-			rmAll := podmanTest.PodmanExitCleanly("artifact", "ls", "--noheading")
-			Expect(rmAll.OutputToString()).To(BeEmpty())
-		}
+		// There should be no artifacts in the store
+		rmAll := podmanTest.PodmanExitCleanly("artifact", "ls", "--noheading")
+		Expect(rmAll.OutputToString()).To(BeEmpty())
 	})
 
 	It("podman artifact inspect with full or partial digest", func() {