volume rm require exact name

This commit reverts the behavior of allowing volume removal by partial name in favor of requiring the exact name. It does this by favoring usage of `GetVolume` over `LookupVolume`

In the past, issue containers#3891 was raised to allow partial name matching on `podman volume rm`, which was fixed by PR containers#3896. However, it has been determined in issue containers#23943 that the pros of allowing removal by partial name (less keys to type) are outweighed by the cons (possibility of unintentionally deleting a volume you didn't mean to). So the behavior will be reverted.

Since PR containers#3896, there has also been PR containers#4832 and PR containers#6736, which extended the partial name removal behavior to API endpoints. This commit also makes these endpoints require exact names.

Closes containers#23943

Signed-off-by: Zachary Hanham <[email protected]>

docs/source/locale/ja/LC_MESSAGES/markdown.po

@@ -30940,7 +30940,7 @@ msgid ""
 "removed. If a volume is being used by a container, an error is returned "
 "unless the **--force** flag is being used. To remove all volumes, use the"
 " **--all** flag. Volumes can be removed individually by providing their "
-"full name or a unique partial name."
+"full name."
 msgstr ""
 
 #: ../../source/markdown/podman-volume-rm.1.md:21

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

@@ -11,7 +11,7 @@ podman\-volume\-rm - Remove one or more volumes
 Removes one or more volumes. Only volumes that are not being used are removed.
 If a volume is being used by a container, an error is returned unless the **--force**
 flag is being used. To remove all volumes, use the **--all** flag.
-Volumes can be removed individually by providing their full name or a unique partial name.
+Volumes can be removed individually by providing their full name.
 
 ## OPTIONS
 

pkg/api/handlers/compat/volumes.go

@@ -240,7 +240,7 @@ func RemoveVolume(w http.ResponseWriter, r *http.Request) {
 	 * respectively.
 	 */
 	name := utils.GetName(r)
-	vol, err := runtime.LookupVolume(name)
+	vol, err := runtime.GetVolume(name)
 	if err == nil {
 		// As above, we do not pass `force` from the query parameters here
 		if err := runtime.RemoveVolume(r.Context(), vol, false, query.Timeout); err != nil {

pkg/api/handlers/libpod/volumes.go

@@ -202,7 +202,7 @@ func RemoveVolume(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	name := utils.GetName(r)
-	vol, err := runtime.LookupVolume(name)
+	vol, err := runtime.GetVolume(name)
 	if err != nil {
 		utils.VolumeNotFound(w, name, err)
 		return

pkg/domain/infra/abi/volumes.go

@@ -61,7 +61,7 @@ func (ic *ContainerEngine) VolumeRm(ctx context.Context, namesOrIds []string, op
 		}
 	} else {
 		for _, id := range namesOrIds {
-			vol, err := ic.Libpod.LookupVolume(id)
+			vol, err := ic.Libpod.GetVolume(id)
 			if err != nil {
 				if opts.Ignore && errors.Is(err, define.ErrNoSuchVolume) {
 					continue

test/apiv2/30-volumes.at

@@ -85,8 +85,11 @@ t GET libpod/volumes/nonexistent/json 404 \
     .response=404
 
 ## Remove volumes
+# Should NOT work on partial name
+t DELETE libpod/volumes/fo 404
+# Remove it for real
 t DELETE libpod/volumes/foo1 204
-#After remove foo1 volume, this volume should not exist
+# After remove foo1 volume, this volume should not exist
 t GET libpod/volumes/foo1/json 404
 # Negative test
 t DELETE libpod/volumes/foo1 404 \

test/e2e/volume_rm_test.go

@@ -76,42 +76,18 @@ var _ = Describe("Podman volume rm", func() {
 		Expect(session.OutputToStringArray()).To(BeEmpty())
 	})
 
-	It("podman volume rm by partial name", func() {
+	It("podman volume remove require exact name", func() {
 		session := podmanTest.Podman([]string{"volume", "create", "myvol"})
 		session.WaitWithDefaultTimeout()
 		Expect(session).Should(ExitCleanly())
 
 		session = podmanTest.Podman([]string{"volume", "rm", "myv"})
 		session.WaitWithDefaultTimeout()
-		Expect(session).Should(ExitCleanly())
-
-		session = podmanTest.Podman([]string{"volume", "ls"})
-		session.WaitWithDefaultTimeout()
-		Expect(session).Should(ExitCleanly())
-		Expect(session.OutputToStringArray()).To(BeEmpty())
-	})
-
-	It("podman volume rm by nonunique partial name", func() {
-		session := podmanTest.Podman([]string{"volume", "create", "myvol1"})
-		session.WaitWithDefaultTimeout()
-		Expect(session).Should(ExitCleanly())
-
-		session = podmanTest.Podman([]string{"volume", "create", "myvol2"})
-		session.WaitWithDefaultTimeout()
-		Expect(session).Should(ExitCleanly())
-
-		session = podmanTest.Podman([]string{"volume", "rm", "myv"})
-		session.WaitWithDefaultTimeout()
-		expect := "more than one result for volume name myv: volume already exists"
-		if podmanTest.DatabaseBackend == "boltdb" {
-			// boltdb issues volume name in quotes
-			expect = `more than one result for volume name "myv": volume already exists`
-		}
-		Expect(session).To(ExitWithError(125, expect))
+		Expect(session).Should(ExitWithError(1, `no volume with name "myv" found: no such volume`))
 
 		session = podmanTest.Podman([]string{"volume", "ls"})
 		session.WaitWithDefaultTimeout()
 		Expect(session).Should(ExitCleanly())
-		Expect(len(session.OutputToStringArray())).To(BeNumerically(">=", 2))
+		Expect(session.OutputToStringArray()).To(HaveLen(1))
 	})
 })