Fix(quadlet): Deduplicate removal files for .app expansion (#27653)

Refactor removelist into sets

Add quadlet rm --all test with safename

add an integration test for quadlet rm --all

Signed-off-by: Kavish Gour <kavishgr@protonmail.com>

Author: kavishgr

pkg/domain/infra/abi/quadlet.go

@@ -760,12 +760,16 @@ func (ic *ContainerEngine) QuadletRemove(ctx context.Context, quadlets []string,
 		Errors:  make(map[string]error),
 		Removed: []string{},
 	}
-	removeList := []string{}
+
 	reverseMap, appMap, err := buildAppMap(systemdquadlet.GetInstallUnitDirPath(rootless.IsRootless()))
 	if err != nil {
 		return nil, fmt.Errorf("unable to build app map: %w", err)
 	}
-	expandQuadletList := []string{}
+
+	// Use sets for deduplication
+	removeSet := make(map[string]struct{})
+	expandQuadletSet := make(map[string]struct{})
+
 	// Process all `.app` files in arguments, if `.app` file
 	// is found then expand it to its respective quadlet files
 	// and remove it from the processing list.
@@ -777,32 +781,36 @@ func (ic *ContainerEngine) QuadletRemove(ctx context.Context, quadlets []string,
 			if ok {
 				for _, file := range files {
 					if !systemdquadlet.IsExtSupported(file) {
-						removeList = append(removeList, file)
+						removeSet[file] = struct{}{}
 					} else {
-						expandQuadletList = append(expandQuadletList, file)
+						expandQuadletSet[file] = struct{}{}
 					}
 				}
 			}
 			// also add .app file itself to the remove list so it can
 			// be cleaned after removal of all components in the list
-			if !slices.Contains(removeList, quadlet) {
-				removeList = append(removeList, quadlet)
-			}
+			removeSet[quadlet] = struct{}{}
 		} else {
-			expandQuadletList = append(expandQuadletList, quadlet)
+			expandQuadletSet[quadlet] = struct{}{}
 		}
 	}
-	quadlets = expandQuadletList
+
+	// Convert expandQuadletSet to slice
+	quadlets = make([]string, 0, len(expandQuadletSet))
+	for quadlet := range expandQuadletSet {
+		quadlets = append(quadlets, quadlet)
+	}
+
+	if len(quadlets) == 0 && !options.All {
+		return nil, errors.New("must provide at least 1 quadlet to remove")
+	}
+
 	allQuadletPaths := make([]string, 0, len(quadlets))
 	allServiceNames := make([]string, 0, len(quadlets))
 	runningQuadlets := make([]string, 0, len(quadlets))
 	serviceNameToQuadletName := make(map[string]string)
 	needReload := options.ReloadSystemd
 
-	if len(quadlets) == 0 && !options.All {
-		return nil, errors.New("must provide at least 1 quadlet to remove")
-	}
-
 	// Is systemd available to the current user?
 	// We cannot proceed if not.
 	conn, err := systemd.ConnectToDBUS()
@@ -813,7 +821,12 @@ func (ic *ContainerEngine) QuadletRemove(ctx context.Context, quadlets []string,
 
 	if options.All {
 		allQuadlets := getAllQuadletPaths()
-		quadlets = allQuadlets
+		for _, quadlet := range allQuadlets {
+			if _, exists := expandQuadletSet[quadlet]; !exists {
+				quadlets = append(quadlets, quadlet)
+				expandQuadletSet[quadlet] = struct{}{}
+			}
+		}
 	}
 
 	// We are using index wise iteration here instead of `range`
@@ -838,30 +851,37 @@ func (ic *ContainerEngine) QuadletRemove(ctx context.Context, quadlets []string,
 			}
 			continue
 		}
-		value, ok := reverseMap[quadlet]
+		// Use base filename for reverseMap lookup since map keys are filenames, not full paths
+		quadletBaseName := filepath.Base(quadlet)
+		value, ok := reverseMap[quadletBaseName]
 		if ok {
 			// If this is part of app and we are cleaning entire .app
 			// make sure to add .app file itself to the removal list
 			// if it does not already exists.
-			if !slices.Contains(removeList, value) {
-				removeList = append(removeList, value)
-			}
+			removeSet[value] = struct{}{}
 			appFilePath := filepath.Join(systemdquadlet.GetInstallUnitDirPath(rootless.IsRootless()), value)
 			filesToRemove, err := getAssetListFromFile(appFilePath)
 			if err != nil {
 				return nil, fmt.Errorf("unable to get list of files to remove: %w", err)
 			}
 			for _, entry := range filesToRemove {
 				if !systemdquadlet.IsExtSupported(entry) {
-					removeList = append(removeList, entry)
-					if !slices.Contains(removeList, value) {
-						// In the last also clean .<quadlet>.app file
-						removeList = append(removeList, value)
-					}
+					removeSet[entry] = struct{}{}
+					removeSet[value] = struct{}{}
 					continue
 				}
-				if !slices.Contains(quadlets, entry) {
-					quadlets = append(quadlets, entry)
+				// When options.All is true, use full paths; when false, use filenames.
+				// this ensures consistency since getAllQuadletPaths() returns full paths
+				// but getQuadletPathByName() expects filenames
+				var entryToAdd string
+				if options.All {
+					entryToAdd = filepath.Join(systemdquadlet.GetInstallUnitDirPath(rootless.IsRootless()), entry)
+				} else {
+					entryToAdd = entry
+				}
+				if _, exists := expandQuadletSet[entryToAdd]; !exists {
+					quadlets = append(quadlets, entryToAdd)
+					expandQuadletSet[entryToAdd] = struct{}{}
 				}
 			}
 		}
@@ -937,13 +957,24 @@ func (ic *ContainerEngine) QuadletRemove(ctx context.Context, quadlets []string,
 					continue
 				}
 			}
-			for _, entry := range removeList {
-				os.Remove(filepath.Join(systemdquadlet.GetInstallUnitDirPath(rootless.IsRootless()), entry))
-			}
 			report.Removed = append(report.Removed, quadletName)
 		}
 	}
 
+	// Remove .app and .asset files after the main quadlet removal loop
+	// This ensures they are cleaned up properly since they are not included in allQuadletPaths
+	installDir := systemdquadlet.GetInstallUnitDirPath(rootless.IsRootless())
+	for entry := range removeSet {
+		entryPath := filepath.Join(installDir, entry)
+		if err := os.Remove(entryPath); err != nil {
+			if !errors.Is(err, fs.ErrNotExist) {
+				logrus.Warnf("Failed to remove metadata file %s: %v", entry, err)
+			}
+		} else {
+			logrus.Debugf("Removed metadata file %s", entry)
+		}
+	}
+
 	// Reload systemd, if necessary/requested.
 	if needReload {
 		if err := conn.ReloadContext(ctx); err != nil {

test/apiv2/36-quadlets.at

@@ -20,35 +20,37 @@ quadlet_build_name="$quadlet_name.build"
 
 TMPDIR=$(mktemp -d podman-apiv2-test.quadlet.XXXXXXXX)
 
-quadlet_container_file_content=$(cat << EOF
+quadlet_container_file_content=$(
+	cat <<EOF
 [Container]
 Image=$IMAGE
 EOF
 )
 
-quadlet_build_file_content=$(cat << EOF
+quadlet_build_file_content=$(
+	cat <<EOF
 [Build]
 ImageTag=localhost/$quadlet_name
 EOF
 )
 
-echo "$quadlet_container_file_content" > $TMPDIR/$quadlet_container_name
-echo "$quadlet_build_file_content" > $TMPDIR/$quadlet_build_name
+echo "$quadlet_container_file_content" >$TMPDIR/$quadlet_container_name
+echo "$quadlet_build_file_content" >$TMPDIR/$quadlet_build_name
 
 # this should ensure the .config/containers/systemd directory is created
 podman quadlet install $TMPDIR/$quadlet_container_name
 podman quadlet install $TMPDIR/$quadlet_build_name
 
 filter_param=$(printf '{"name":["%s"]}' "$quadlet_name")
 t GET "libpod/quadlets/json?filters=$filter_param" 200 \
-    length=2 \
-   .[0].Name="$quadlet_build_name" \
-   .[1].Name="$quadlet_container_name"
+	length=2 \
+	.[0].Name="$quadlet_build_name" \
+	.[1].Name="$quadlet_container_name"
 
 filter_param=$(printf '{"name":["%s"]}' "$quadlet_container_name")
 t GET "libpod/quadlets/json?filters=$filter_param" 200 \
-    length=1 \
-   .[0].Name="$quadlet_container_name"
+	length=1 \
+	.[0].Name="$quadlet_container_name"
 
 t GET "libpod/quadlets/$quadlet_name/file" 404
 
@@ -62,4 +64,105 @@ podman quadlet rm $quadlet_container_name
 podman quadlet rm $quadlet_build_name
 rm -rf $TMPDIR
 
+# podman quadlet rm -all --force : Testing removal of all quadlets at once
+#
+# Install quadlets(.container, .network, .volume)
+
+quadlet_name=quadlet-remove-all-test-$(cat /proc/sys/kernel/random/uuid)
+
+quadlet_container="$quadlet_name.container"
+
+quadlet_network="$quadlet_name.network"
+
+quadlet_volume="$quadlet_name.volume"
+
+TMPDIR=$(mktemp -d podman-apiv2-test.quadlet.remove.all.XXXXXXXX)
+
+quadlet_container_file_content=$(
+	cat <<EOF
+[Container]
+Image=$IMAGE
+EOF
+)
+
+quadlet_network_file_content=$(
+	cat <<EOF
+[Network]
+Label=app=nginx-network
+EOF
+)
+
+quadlet_volume_file_content=$(
+	cat <<EOF
+[Volume]
+VolumeName=does_not_exist
+EOF
+)
+
+echo "$quadlet_container_file_content" >$TMPDIR/$quadlet_container
+echo "$quadlet_network_file_content" >$TMPDIR/$quadlet_network
+echo "$quadlet_volume_file_content" >$TMPDIR/$quadlet_volume
+
+# Install all quadlets
+podman quadlet install $TMPDIR
+
+# Verify all quadlets exist via API (file endpoint)
+
+t GET libpod/quadlets/$quadlet_container/file 200
+is "$output" "$quadlet_container_file_content"
+
+t GET libpod/quadlets/$quadlet_network/file 200
+is "$output" "$quadlet_network_file_content"
+
+t GET libpod/quadlets/$quadlet_volume/file 200
+is "$output" "$quadlet_volume_file_content"
+
+# Verify all quadlets appear in the list using filters
+
+filter_param=$(printf '{"name":["%s"]}' "$quadlet_container")
+t GET "libpod/quadlets/json?filters=$filter_param" 200 \
+	length=1 \
+	.[0].Name="$quadlet_container"
+
+filter_param=$(printf '{"name":["%s"]}' "$quadlet_network")
+t GET "libpod/quadlets/json?filters=$filter_param" 200 \
+	length=1 \
+	.[0].Name="$quadlet_network"
+
+filter_param=$(printf '{"name":["%s"]}' "$quadlet_volume")
+t GET "libpod/quadlets/json?filters=$filter_param" 200 \
+	length=1 \
+	.[0].Name="$quadlet_volume"
+
+# Remove ALL quadlets using CLI
+podman quadlet rm -af
+
+# Verify all quadlets are gone via API (should return 404)
+
+t GET libpod/quadlets/$quadlet_container/file 404
+t GET libpod/quadlets/$quadlet_network/file 404
+t GET libpod/quadlets/$quadlet_volume/file 404
+
+# Verify all quadlets no longer appear in filtered lists
+
+filter_param=$(printf '{"name":["%s"]}' "$quadlet_container")
+t GET "libpod/quadlets/json?filters=$filter_param" 200 \
+	length=0
+
+filter_param=$(printf '{"name":["%s"]}' "$quadlet_network")
+t GET "libpod/quadlets/json?filters=$filter_param" 200 \
+	length=0
+
+filter_param=$(printf '{"name":["%s"]}' "$quadlet_volume")
+t GET "libpod/quadlets/json?filters=$filter_param" 200 \
+	length=0
+
+# Verify the complete list is now empty
+
+t GET libpod/quadlets/json 200 \
+	length=0
+
+# Cleanup
+rm -rf $TMPDIR
+
 # vim: filetype=sh

test/e2e/quadlet_test.go

@@ -1334,4 +1334,50 @@ BOGUS=foo
 			},
 		),
 	)
+
+	Describe("Running quadlet force remove all", func() {
+		It("Should remove all quadlets at once", func() {
+			quadletName := fmt.Sprintf("quadlet-remove-all-test-%d", GinkgoRandomSeed())
+			containerFile := quadletName + ".container"
+			networkFile := quadletName + ".network"
+			volumeFile := quadletName + ".volume"
+
+			tmpDir := filepath.Join(podmanTest.TempDir, quadletName)
+			err := os.Mkdir(tmpDir, 0o755)
+			Expect(err).ToNot(HaveOccurred())
+
+			containerContent := []byte(fmt.Sprintf("[Container]\nImage=%s\n", ALPINE))
+			networkContent := []byte("[Network]\nLabel=app=nginx-network\n")
+			volumeContent := []byte("[Volume]\nVolumeName=does_not_exist\n")
+
+			Expect(os.WriteFile(filepath.Join(tmpDir, containerFile), containerContent, 0o644)).To(Succeed())
+			Expect(os.WriteFile(filepath.Join(tmpDir, networkFile), networkContent, 0o644)).To(Succeed())
+			Expect(os.WriteFile(filepath.Join(tmpDir, volumeFile), volumeContent, 0o644)).To(Succeed())
+
+			installSession := podmanTest.Podman([]string{"quadlet", "install", tmpDir})
+			installSession.WaitWithDefaultTimeout()
+			Expect(installSession).Should(Exit(0))
+
+			// Verify quadlets appear in list
+			listSession := podmanTest.Podman([]string{"quadlet", "list", "--format", "json"})
+			listSession.WaitWithDefaultTimeout()
+			Expect(listSession).Should(Exit(0))
+			output := listSession.OutputToString()
+			Expect(output).To(ContainSubstring(containerFile))
+			Expect(output).To(ContainSubstring(networkFile))
+			Expect(output).To(ContainSubstring(volumeFile))
+
+			// Remove all quadlets
+			rmSession := podmanTest.Podman([]string{"quadlet", "rm", "-af"})
+			rmSession.WaitWithDefaultTimeout()
+			Expect(rmSession).Should(Exit(0))
+
+			// Verify list is empty
+			listSession = podmanTest.Podman([]string{"quadlet", "list", "--format", "json"})
+			listSession.WaitWithDefaultTimeout()
+			Expect(listSession).Should(Exit(0))
+			output = listSession.OutputToString()
+			Expect(output).To(Equal("[]"))
+		})
+	})
 })