Skip to content

Commit 7e8e0bf

Browse files
authored
Merge pull request #101 from buildkite/feat_clean_before_restore
feat: adds clean before restore to avoid merging data
2 parents e0f433a + e09e959 commit 7e8e0bf

File tree

2 files changed

+189
-0
lines changed

2 files changed

+189
-0
lines changed

restore.go

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,13 @@ package zstash
22

33
import (
44
"context"
5+
"errors"
56
"fmt"
7+
"io/fs"
8+
"log/slog"
69
"os"
710
"path/filepath"
11+
"runtime"
812
"strings"
913
"time"
1014

@@ -151,6 +155,25 @@ func (c *Cache) Restore(ctx context.Context, cacheID string) (RestoreResult, err
151155
Concurrency: transferInfo.Concurrency,
152156
}
153157

158+
c.callProgress(cacheID, "cleaning", "Cleaning paths", 0, 0)
159+
160+
for _, path := range cacheConfig.Paths {
161+
extractedPath, err := archive.ResolveHomeDir(path)
162+
if err != nil {
163+
span.RecordError(err)
164+
span.SetStatus(codes.Error, "failed to resolve home dir")
165+
return result, fmt.Errorf("failed to resolve home dir for %q: %w", path, err)
166+
}
167+
168+
slog.Debug("cleaning path", "path", path, "extractedPath", extractedPath)
169+
170+
if err := cleanPath(ctx, extractedPath); err != nil {
171+
span.RecordError(err)
172+
span.SetStatus(codes.Error, "failed to clean path")
173+
return result, fmt.Errorf("failed to clean path %q: %w", extractedPath, err)
174+
}
175+
}
176+
154177
c.callProgress(cacheID, "extracting", "Extracting files from cache", 0, int(transferInfo.BytesTransferred))
155178

156179
// Extract files
@@ -279,3 +302,70 @@ func (c *Cache) extractCache(ctx context.Context, archiveFile string, archiveSiz
279302

280303
return archiveInfo, nil
281304
}
305+
306+
// cleanPath removes a directory tree for a configured cache path.
307+
// It handles Go module cache directories that have 0555 permissions by
308+
// making them writable before removal.
309+
func cleanPath(ctx context.Context, dir string) error {
310+
if dir == "" {
311+
return fmt.Errorf("cleanPath: empty directory path")
312+
}
313+
314+
clean := filepath.Clean(dir)
315+
316+
// Refuse to delete root or current directory
317+
if clean == "." || clean == string(os.PathSeparator) {
318+
return fmt.Errorf("cleanPath: refusing to remove %q", clean)
319+
}
320+
321+
// On Windows, also check for drive roots like "C:\"
322+
if runtime.GOOS == "windows" && len(clean) == 3 && clean[1] == ':' && clean[2] == '\\' {
323+
return fmt.Errorf("cleanPath: refusing to remove drive root %q", clean)
324+
}
325+
326+
// Refuse to delete home directory
327+
if home, err := os.UserHomeDir(); err == nil {
328+
if clean == filepath.Clean(home) {
329+
return fmt.Errorf("cleanPath: refusing to remove home directory %q", clean)
330+
}
331+
}
332+
333+
// Module cache has 0555 directories; make them writable in order to remove content.
334+
err := filepath.WalkDir(clean, func(path string, info fs.DirEntry, walkErr error) error {
335+
// Respect context cancellation for long directory trees
336+
select {
337+
case <-ctx.Done():
338+
return ctx.Err()
339+
default:
340+
}
341+
342+
if walkErr != nil {
343+
slog.Debug("cleanPath: error walking path", "path", path, "err", walkErr)
344+
return nil
345+
}
346+
347+
if info.IsDir() {
348+
if chmodErr := os.Chmod(path, 0o755); chmodErr != nil {
349+
return fmt.Errorf("chmod %q: %w", path, chmodErr)
350+
}
351+
}
352+
return nil
353+
})
354+
if err != nil {
355+
if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) {
356+
return err
357+
}
358+
return fmt.Errorf("cleanPath: error preparing %q for removal: %w", clean, err)
359+
}
360+
361+
// Check context again before potentially long RemoveAll
362+
if ctx.Err() != nil {
363+
return ctx.Err()
364+
}
365+
366+
if err := os.RemoveAll(clean); err != nil {
367+
return fmt.Errorf("cleanPath: failed to remove %q: %w", clean, err)
368+
}
369+
370+
return nil
371+
}

restore_test.go

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
package zstash
2+
3+
import (
4+
"context"
5+
"os"
6+
"path/filepath"
7+
"runtime"
8+
"testing"
9+
10+
"github.com/stretchr/testify/assert"
11+
"github.com/stretchr/testify/require"
12+
)
13+
14+
func TestCleanPath(t *testing.T) {
15+
t.Run("removes directory and contents", func(t *testing.T) {
16+
dir := t.TempDir()
17+
testDir := filepath.Join(dir, "cache")
18+
require.NoError(t, os.MkdirAll(filepath.Join(testDir, "subdir"), 0o755))
19+
require.NoError(t, os.WriteFile(filepath.Join(testDir, "file.txt"), []byte("test"), 0o600))
20+
require.NoError(t, os.WriteFile(filepath.Join(testDir, "subdir", "nested.txt"), []byte("nested"), 0o600))
21+
22+
err := cleanPath(context.Background(), testDir)
23+
require.NoError(t, err)
24+
25+
_, err = os.Stat(testDir)
26+
assert.True(t, os.IsNotExist(err), "directory should be removed")
27+
})
28+
29+
t.Run("handles read-only directories (like go module cache)", func(t *testing.T) {
30+
dir := t.TempDir()
31+
testDir := filepath.Join(dir, "modcache")
32+
subdir := filepath.Join(testDir, "pkg")
33+
require.NoError(t, os.MkdirAll(subdir, 0o755))
34+
require.NoError(t, os.WriteFile(filepath.Join(subdir, "mod.go"), []byte("package mod"), 0o400))
35+
require.NoError(t, os.Chmod(subdir, 0o555))
36+
require.NoError(t, os.Chmod(testDir, 0o555))
37+
38+
err := cleanPath(context.Background(), testDir)
39+
require.NoError(t, err)
40+
41+
_, err = os.Stat(testDir)
42+
assert.True(t, os.IsNotExist(err), "directory should be removed")
43+
})
44+
45+
t.Run("succeeds on non-existent path", func(t *testing.T) {
46+
err := cleanPath(context.Background(), "/nonexistent/path/that/does/not/exist")
47+
require.NoError(t, err)
48+
})
49+
50+
t.Run("rejects empty path", func(t *testing.T) {
51+
err := cleanPath(context.Background(), "")
52+
require.Error(t, err)
53+
assert.Contains(t, err.Error(), "empty directory path")
54+
})
55+
56+
t.Run("rejects root path", func(t *testing.T) {
57+
err := cleanPath(context.Background(), "/")
58+
require.Error(t, err)
59+
assert.Contains(t, err.Error(), "refusing to remove")
60+
})
61+
62+
t.Run("rejects current directory", func(t *testing.T) {
63+
err := cleanPath(context.Background(), ".")
64+
require.Error(t, err)
65+
assert.Contains(t, err.Error(), "refusing to remove")
66+
})
67+
68+
t.Run("rejects home directory", func(t *testing.T) {
69+
home, err := os.UserHomeDir()
70+
require.NoError(t, err)
71+
72+
err = cleanPath(context.Background(), home)
73+
require.Error(t, err)
74+
assert.Contains(t, err.Error(), "refusing to remove home directory")
75+
})
76+
77+
t.Run("respects context cancellation", func(t *testing.T) {
78+
dir := t.TempDir()
79+
testDir := filepath.Join(dir, "cache")
80+
require.NoError(t, os.MkdirAll(testDir, 0o755))
81+
82+
ctx, cancel := context.WithCancel(context.Background())
83+
cancel()
84+
85+
err := cleanPath(ctx, testDir)
86+
require.Error(t, err)
87+
assert.ErrorIs(t, err, context.Canceled)
88+
})
89+
}
90+
91+
func TestCleanPathWindowsDriveRoot(t *testing.T) {
92+
if runtime.GOOS != "windows" {
93+
t.Skip("Windows-specific test")
94+
}
95+
96+
err := cleanPath(context.Background(), "C:\\")
97+
require.Error(t, err)
98+
assert.Contains(t, err.Error(), "refusing to remove drive root")
99+
}

0 commit comments

Comments
 (0)