diff --git a/pkg/app/piped/driftdetector/kubernetes/detector.go b/pkg/app/piped/driftdetector/kubernetes/detector.go index e5bee48e66..5330876894 100644 --- a/pkg/app/piped/driftdetector/kubernetes/detector.go +++ b/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)) + } } } } diff --git a/pkg/git/gittest/git.mock.go b/pkg/git/gittest/git.mock.go index 3ebd0ef5a2..7f01dabde3 100644 --- a/pkg/git/gittest/git.mock.go +++ b/pkg/git/gittest/git.mock.go @@ -92,6 +92,20 @@ func (mr *MockRepoMockRecorder) Clean() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Clean", reflect.TypeOf((*MockRepo)(nil).Clean)) } +// CleanPath mocks base method. +func (m *MockRepo) CleanPath(arg0 context.Context, arg1 string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CleanPath", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// CleanPath indicates an expected call of CleanPath. +func (mr *MockRepoMockRecorder) CleanPath(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CleanPath", reflect.TypeOf((*MockRepo)(nil).CleanPath), arg0, arg1) +} + // CommitChanges mocks base method. func (m *MockRepo) CommitChanges(arg0 context.Context, arg1, arg2 string, arg3 bool, arg4 map[string][]byte, arg5 map[string]string) error { m.ctrl.T.Helper() diff --git a/pkg/git/repo.go b/pkg/git/repo.go index 3363f6eee1..421632ac37 100644 --- a/pkg/git/repo.go +++ b/pkg/git/repo.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 { diff --git a/pkg/git/repo_test.go b/pkg/git/repo_test.go index 7166b63a09..ffca37a366 100644 --- a/pkg/git/repo_test.go +++ b/pkg/git/repo_test.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) +}