Execute git clean partially when drift detection for every app is done (#5312)

* Execute git clean partially when drift detection for every app is done

Signed-off-by: Yoshiki Fujikane <[email protected]>

* Rename to CleanPath

Signed-off-by: Yoshiki Fujikane <[email protected]>

* Add test for the outside dir pattern

Signed-off-by: Yoshiki Fujikane <[email protected]>

---------

Signed-off-by: Yoshiki Fujikane <[email protected]>

Author: ffjlabo

pkg/app/piped/driftdetector/kubernetes/detector.go

@@ -169,6 +169,23 @@ func (d *detector) check(ctx context.Context) {
 			if err := d.checkApplication(ctx, app, gitRepo, headCommit); err != nil {
 				d.logger.Error(fmt.Sprintf("failed to check application: %s", app.Id), zap.Error(err))
 			}
+
+			// Reset the app dir to the head commit.
+			// Some tools may create temporary files locally to render manifests.
+			// The detector reuses the same located local repository and it causes unexpected behavior by reusing such temporary files.
+			// So regularly run git clean on the app dir after each detection.
+			d.logger.Info("cleaning partially cloned repository",
+				zap.String("repo-id", repoID),
+				zap.String("app-id", app.Id),
+				zap.String("app-path", app.GitPath.Path),
+			)
+			if err := gitRepo.CleanPath(ctx, app.GitPath.Path); err != nil {
+				d.logger.Error("failed to clean partially cloned repository",
+					zap.String("repo-id", repoID),
+					zap.String("app-id", app.Id),
+					zap.String("app-path", app.GitPath.Path),
+					zap.Error(err))
+			}
 		}
 	}
 }

pkg/git/gittest/git.mock.go

@@ -43,6 +43,7 @@ type Repo interface {
 	Checkout(ctx context.Context, commitish string) error
 	CheckoutPullRequest(ctx context.Context, number int, branch string) error
 	Clean() error
+	CleanPath(ctx context.Context, relativePath string) error
 
 	Pull(ctx context.Context, branch string) error
 	MergeRemoteBranch(ctx context.Context, branch, commit, mergeCommitMessage string) error
@@ -259,6 +260,15 @@ func (r repo) Clean() error {
 	return os.RemoveAll(r.dir)
 }
 
+// CleanPath deletes data in the given relative path in the repo with git clean.
+func (r repo) CleanPath(ctx context.Context, relativePath string) error {
+	out, err := r.runGitCommand(ctx, "clean", "-f", relativePath)
+	if err != nil {
+		return formatCommandError(err, out)
+	}
+	return nil
+}
+
 func (r *repo) checkoutNewBranch(ctx context.Context, branch string) error {
 	out, err := r.runGitCommand(ctx, "checkout", "-b", branch)
 	if err != nil {

pkg/git/repo.go

@@ -279,3 +279,68 @@ func TestGetCommitForRev(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, commits[0].Hash, commit.Hash)
 }
+
+func TestCleanPath(t *testing.T) {
+	faker, err := newFaker()
+	require.NoError(t, err)
+	defer faker.clean()
+
+	var (
+		org      = "test-repo-org"
+		repoName = "repo-clean-path"
+		ctx      = context.Background()
+	)
+
+	err = faker.makeRepo(org, repoName)
+	require.NoError(t, err)
+	r := &repo{
+		dir:     faker.repoDir(org, repoName),
+		gitPath: faker.gitPath,
+	}
+
+	// create two directories and a file in each
+	// repo-clean-path/part1/new-file.txt
+	// repo-clean-path/part2/new-file.txt
+	dirs := []string{"part1", "part2"}
+	for _, dir := range dirs {
+		partDir := filepath.Join(r.dir, dir)
+		err = os.MkdirAll(partDir, os.ModePerm)
+		require.NoError(t, err)
+
+		path := filepath.Join(partDir, "new-file.txt")
+		err = os.WriteFile(path, []byte("content"), os.ModePerm)
+		require.NoError(t, err)
+	}
+
+	// create other dir outside the repo
+	// repo-clean-path/outside-dir/new-file.txt
+	outsideDir := filepath.Join(r.dir, "..", "outside-dir")
+	require.NoError(t, err)
+
+	err = os.MkdirAll(outsideDir, os.ModePerm)
+	require.NoError(t, err)
+
+	path := filepath.Join(outsideDir, "new-file.txt")
+	err = os.WriteFile(path, []byte("content"), os.ModePerm)
+	require.NoError(t, err)
+
+	// clean the repo-dir/part1
+	err = r.CleanPath(ctx, "part1")
+	require.NoError(t, err)
+
+	// check the repo-dir/part1 is removed
+	_, err = os.Stat(filepath.Join(r.dir, "part1"))
+	assert.True(t, os.IsNotExist(err))
+
+	// check the repo-dir/part2 is still there
+	_, err = os.Stat(filepath.Join(r.dir, "part2"))
+	assert.NoError(t, err)
+
+	// check the outside dir can't be cleaned with relative path
+	err = r.CleanPath(ctx, "../outside-dir")
+	require.Error(t, err)
+
+	// check the outside dir can't be cleaned with relative path
+	err = r.CleanPath(ctx, outsideDir)
+	require.Error(t, err)
+}