Added RemoveStaleItems in copy-merge (#306)

* added RemoveStaleItems in copy-merge

Signed-off-by: lapentafd <francesco.lapenta@est.tech>

* adding unit tests

Signed-off-by: lapentafd <francesco.lapenta@est.tech>

---------

Signed-off-by: lapentafd <francesco.lapenta@est.tech>
Co-authored-by: Kushal Harish Naidu <kushal.harish.naidu@ericsson.com>

Author: lapentad

internal/kpt/util/pkgutil/pkgutil.go

@@ -169,6 +169,62 @@ func CopyPackage(src, dst string, copyRootKptfile bool, matcher pkg.SubpackageMa
 	return nil
 }
 
+// RemoveStaleItems removes files and directories from the dst package that were present in the org package,
+// but are not present in the src package. It does not remove the root Kptfile of the dst package.
+func RemoveStaleItems(org, src, dst string, copyRootKptfile bool, matcher pkg.SubpackageMatcher) error {
+	var dirsToDelete []string
+	walkErr := filepath.Walk(dst, func(path string, info os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+		// The root directory should never be deleted.
+		if path == dst {
+			return nil
+		}
+
+		relPath, err := filepath.Rel(dst, path)
+		if err != nil {
+			return err
+		}
+
+		// Skip the root Kptfile
+		if relPath == kptfilev1.KptFileName {
+			return nil
+		}
+
+		srcPath := filepath.Join(src, relPath)
+		orgPath := filepath.Join(org, relPath)
+
+		_, srcErr := os.Stat(srcPath)
+		_, orgErr := os.Stat(orgPath)
+
+		// Only remove if:
+		// - not present in src (srcErr is os.IsNotExist)
+		// - present in org (orgErr is nil)
+		if os.IsNotExist(srcErr) && orgErr == nil {
+			if info.IsDir() {
+				dirsToDelete = append(dirsToDelete, path)
+			} else {
+				if err := os.Remove(path); err != nil {
+					return err
+				}
+			}
+		}
+		return nil
+	})
+	if walkErr != nil {
+		return walkErr
+	}
+	sort.Slice(dirsToDelete, SubPkgFirstSorter(dirsToDelete))
+	for _, dir := range dirsToDelete {
+		if err := os.Remove(dir); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 func RemovePackageContent(path string, removeRootKptfile bool) error {
 	// Walk the package (while ignoring subpackages) and delete all files.
 	// We capture the paths to any subdirectories in the package so we

internal/kpt/util/pkgutil/pkgutil_test.go

@@ -549,3 +549,43 @@ func TestFindLocalRecursiveSubpackagesForPaths(t *testing.T) {
 		})
 	}
 }
+
+
+func TestRemoveStaleItems_RemovesFile(t *testing.T) {
+    org := t.TempDir()
+    src := t.TempDir()
+    dst := t.TempDir()
+
+    // Create a file in org and dst, but not in src
+    fileName := "file.txt"
+    assert.NoError(t, os.WriteFile(filepath.Join(org, fileName), []byte("content"), 0644))
+    assert.NoError(t, os.WriteFile(filepath.Join(dst, fileName), []byte("content"), 0644))
+
+    // Should remove file.txt from dst
+    err := pkgutil.RemoveStaleItems(org, src, dst, true, pkg.All)
+    assert.NoError(t, err)
+    _, err = os.Stat(filepath.Join(dst, fileName))
+    assert.True(t, os.IsNotExist(err))
+}
+
+func TestRemoveStaleItems_ErrorOnRemove(t *testing.T) {
+    org := t.TempDir()
+    src := t.TempDir()
+    dst := t.TempDir()
+
+    fileName := "file.txt"
+    filePathDst := filepath.Join(dst, fileName)
+    filePathOrg := filepath.Join(org, fileName)
+
+    assert.NoError(t, os.WriteFile(filePathOrg, []byte("content"), 0644))
+    assert.NoError(t, os.WriteFile(filePathDst, []byte("content"), 0644))
+
+    // Replace file in dst with a non-empty directory to force os.Remove error
+    assert.NoError(t, os.Remove(filePathDst))
+    assert.NoError(t, os.Mkdir(filePathDst, 0755))
+    assert.NoError(t, os.WriteFile(filepath.Join(filePathDst, "dummy"), []byte("x"), 0644))
+
+    err := pkgutil.RemoveStaleItems(org, src, dst, true, pkg.All)
+    assert.Error(t, err)
+    assert.Contains(t, err.Error(), "directory not empty")
+}

internal/kpt/util/update/copy-merge.go

@@ -27,19 +27,25 @@ import (
 type CopyMergeUpdater struct{}
 
 // Update synchronizes the destination/local package with the source/update package by updating the Kptfile
-// and copying package contents. It takes an Options struct as input, which specifies the paths
+// and copying package contents. It deletes resources from the destination package if they were present
+// in the original package, but not present anymore in the source package.
+// It takes an Options struct as input, which specifies the paths
 // and other parameters for the update operation. Returns an error if the update fails.
 func (u CopyMergeUpdater) Update(options Options) error {
 	const op errors.Op = "update.Update"
 
 	dst := options.LocalPath
 	src := options.UpdatedPath
+	org := options.OriginPath
 
 	if err := kptfileutil.UpdateKptfile(dst, src, options.OriginPath, true); err != nil {
 		return errors.E(op, types.UniquePath(dst), err)
 	}
 	if err := pkgutil.CopyPackage(src, dst, options.IsRoot, pkg.All); err != nil {
 		return errors.E(op, types.UniquePath(dst), err)
 	}
+	if err := pkgutil.RemoveStaleItems(org, src, dst, options.IsRoot, pkg.All); err != nil {
+		return errors.E(op, types.UniquePath(dst), err)
+	}
 	return nil
 }

internal/kpt/util/update/copy-merge_test.go

@@ -181,7 +181,7 @@ func TestCopyMerge(t *testing.T) {
 						WithResource(pkgbuilder.DeploymentResource),
 				),
 		},
-		"update existing file in origin, local, and updated": {
+		"file removal if file exists in origin but not in update": {
 			origin: pkgbuilder.NewRootPkg().
 				WithKptfile(
 					pkgbuilder.NewKptfile().
@@ -207,8 +207,7 @@ func TestCopyMerge(t *testing.T) {
 					pkgbuilder.NewKptfile().
 						WithUpstream(kptRepo, "/origin", "master", copyMergeLiteral).
 						WithUpstreamLock(kptRepo, "/origin", "master", "abc123"),
-				).
-				WithResource(pkgbuilder.DeploymentResource),
+				),
 		},
 	}
 
@@ -415,3 +414,33 @@ func TestCopyMergeDifferentMetadata(t *testing.T) {
 		})
 	}
 }
+
+func TestCopyMergeErrorRemovingFile(t *testing.T) {
+	src := t.TempDir()
+	dst := t.TempDir()
+	org := t.TempDir()
+
+	// Create a file in org and dst, but not in src (so RemoveStaleItems will try to remove it)
+	fileName := "file.txt"
+	filePathDst := filepath.Join(dst, fileName)
+	filePathOrg := filepath.Join(org, fileName)
+
+	assert.NoError(t, os.WriteFile(filePathDst, []byte("content"), 0644))
+	assert.NoError(t, os.WriteFile(filePathOrg, []byte("content"), 0644))
+
+	assert.NoError(t, os.Remove(filePathDst))
+	assert.NoError(t, os.Mkdir(filePathDst, 0755))
+	assert.NoError(t, os.WriteFile(filepath.Join(filePathDst, "dummy"), []byte("x"), 0644))
+
+	updater := &CopyMergeUpdater{}
+	options := Options{
+		OriginPath:  org,
+		UpdatedPath: src,
+		LocalPath:   dst,
+		IsRoot:      true,
+	}
+
+	err := updater.Update(options)
+	assert.Error(t, err)
+	assert.Contains(t, err.Error(), "directory not empty")
+}