Skip to content

Commit

Permalink
blob/fileblob: Add an option to avoid using os.TempDir for temp files (
Browse files Browse the repository at this point in the history
  • Loading branch information
vangent authored and ybourgery committed Jan 25, 2024
1 parent f286071 commit 861ac49
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 8 deletions.
20 changes: 17 additions & 3 deletions blob/fileblob/fileblob.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,14 @@ type Options struct {
// (using os.MkdirAll).
CreateDir bool

// If true, don't use os.TempDir for temporary files, but instead place them
// next to the actual files. This may result in "stranded" temporary files
// (e.g., if the application is killed before the file cleanup runs).
//
// If your bucket directory is on a different mount than os.TempDir, you will
// need to set this to true, as os.Rename will fail across mount points.
NoTempDir bool

// Refers to the strategy for how to deal with metadata (such as blob.Attributes).
// For supported values please see the Metadata* constants.
// If left unchanged, 'MetadataInSidecar' will be used.
Expand Down Expand Up @@ -642,7 +650,7 @@ func (r *reader) As(i interface{}) bool {
return true
}

func createTemp(path string) (*os.File, error) {
func createTemp(path string, noTempDir bool) (*os.File, error) {
// Use a custom createTemp function rather than os.CreateTemp() as
// os.CreateTemp() sets the permissions of the tempfile to 0600, rather than
// 0666, making it inconsistent with the directories and attribute files.
Expand All @@ -653,7 +661,13 @@ func createTemp(path string) (*os.File, error) {
// between each iteration to make a conflict unlikely. Using the full
// time lowers the chance of a collision with a file using a similar
// pattern, but has undefined behavior after the year 2262.
name := filepath.Join(os.TempDir(), filepath.Base(path)) + "." + strconv.FormatInt(time.Now().UnixNano(), 16) + ".tmp"
var name string
if noTempDir {
name = path
} else {
name = filepath.Join(os.TempDir(), filepath.Base(path))
}
name += "." + strconv.FormatInt(time.Now().UnixNano(), 16) + ".tmp"
f, err := os.OpenFile(name, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0666)
if os.IsExist(err) {
if try++; try < 10000 {
Expand All @@ -674,7 +688,7 @@ func (b *bucket) NewTypedWriter(ctx context.Context, key string, contentType str
if err := os.MkdirAll(filepath.Dir(path), os.FileMode(0777)); err != nil {
return nil, err
}
f, err := createTemp(path)
f, err := createTemp(path, b.opts.NoTempDir)
if err != nil {
return nil, err
}
Expand Down
19 changes: 14 additions & 5 deletions blob/fileblob/fileblob_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,13 @@ type harness struct {
dir string
prefix string
metadataHow metadataOption
noTempDir bool
server *httptest.Server
urlSigner URLSigner
closer func()
}

func newHarness(ctx context.Context, t *testing.T, prefix string, metadataHow metadataOption) (drivertest.Harness, error) {
func newHarness(ctx context.Context, t *testing.T, prefix string, metadataHow metadataOption, noTempDir bool) (drivertest.Harness, error) {
if metadataHow == MetadataDontWrite {
// Skip tests for if no metadata gets written.
// For these it is currently undefined whether any gets read (back).
Expand All @@ -64,7 +65,7 @@ func newHarness(ctx context.Context, t *testing.T, prefix string, metadataHow me
return nil, err
}
}
h := &harness{dir: dir, prefix: prefix, metadataHow: metadataHow}
h := &harness{dir: dir, prefix: prefix, metadataHow: metadataHow, noTempDir: noTempDir}

localServer := httptest.NewServer(http.HandlerFunc(h.serveSignedURL))
h.server = localServer
Expand Down Expand Up @@ -148,6 +149,7 @@ func (h *harness) MakeDriver(ctx context.Context) (driver.Bucket, error) {
opts := &Options{
URLSigner: h.urlSigner,
Metadata: h.metadataHow,
NoTempDir: h.noTempDir,
}
drv, err := openBucket(h.dir, opts)
if err != nil {
Expand All @@ -171,22 +173,29 @@ func (h *harness) Close() {

func TestConformance(t *testing.T) {
newHarnessNoPrefix := func(ctx context.Context, t *testing.T) (drivertest.Harness, error) {
return newHarness(ctx, t, "", MetadataInSidecar)
return newHarness(ctx, t, "", MetadataInSidecar, false)
}
drivertest.RunConformanceTests(t, newHarnessNoPrefix, []drivertest.AsTest{verifyAs{}})
}

func TestConformanceNoTempDir(t *testing.T) {
newHarnessNoTmpDir := func(ctx context.Context, t *testing.T) (drivertest.Harness, error) {
return newHarness(ctx, t, "", MetadataInSidecar, true)
}
drivertest.RunConformanceTests(t, newHarnessNoTmpDir, []drivertest.AsTest{verifyAs{}})
}

func TestConformanceWithPrefix(t *testing.T) {
const prefix = "some/prefix/dir/"
newHarnessWithPrefix := func(ctx context.Context, t *testing.T) (drivertest.Harness, error) {
return newHarness(ctx, t, prefix, MetadataInSidecar)
return newHarness(ctx, t, prefix, MetadataInSidecar, false)
}
drivertest.RunConformanceTests(t, newHarnessWithPrefix, []drivertest.AsTest{verifyAs{prefix: prefix}})
}

func TestConformanceSkipMetadata(t *testing.T) {
newHarnessSkipMetadata := func(ctx context.Context, t *testing.T) (drivertest.Harness, error) {
return newHarness(ctx, t, "", MetadataDontWrite)
return newHarness(ctx, t, "", MetadataDontWrite, false)
}
drivertest.RunConformanceTests(t, newHarnessSkipMetadata, []drivertest.AsTest{verifyAs{}})
}
Expand Down

0 comments on commit 861ac49

Please sign in to comment.