Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Do some best-effort cleanup in file backend #4684

Merged
merged 2 commits into from
Jun 4, 2018
Merged

Do some best-effort cleanup in file backend #4684

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
75fb75f
Do some best-effort cleanup in file backend
jefferai Jun 2, 2018
74ae12e
Also run a check on Get
jefferai Jun 2, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 44 additions & 5 deletions physical/file/file.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package file
import (
"context"
"encoding/json"
"errors"
"fmt"
"io"
"os"
Expand Down Expand Up @@ -162,6 +163,18 @@ func (b *FileBackend) GetInternal(ctx context.Context, k string) (*physical.Entr
path, key := b.expandPath(k)
path = filepath.Join(path, key)

// If we stat it and it exists but is size zero, it may be left from some
// previous FS error like out-of-space. No Vault entry will ever be zero
// length, so simply remove it and return nil.
fi, err := os.Stat(path)
if err == nil {
if fi.Size() == 0 {
// Best effort, ignore errors
os.Remove(path)
return nil, nil
}
}

f, err := os.Open(path)
if f != nil {
defer f.Close()
Expand Down Expand Up @@ -208,20 +221,46 @@ func (b *FileBackend) PutInternal(ctx context.Context, entry *physical.Entry) er
}

// JSON encode the entry and write it
fullPath := filepath.Join(path, key)
f, err := os.OpenFile(
filepath.Join(path, key),
fullPath,
os.O_CREATE|os.O_TRUNC|os.O_WRONLY,
0600)
if f != nil {
defer f.Close()
}
if err != nil {
if f != nil {
f.Close()
}
return err
}
if f == nil {
return errors.New("could not successfully get a file handle")
}

enc := json.NewEncoder(f)
return enc.Encode(&fileEntry{
encErr := enc.Encode(&fileEntry{
Value: entry.Value,
})
f.Close()
if encErr == nil {
return nil
}

// Everything below is best-effort and will result in encErr being returned

// See if we ended up with a zero-byte file and if so delete it, might be a
// case of disk being full but the file info is in metadata that is
// reserved.
fi, err := os.Stat(fullPath)
if err != nil {
return encErr
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since by this point we're in an questionable FS state, it may be useful to wrap/return both encErr and err.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason I didn't do that is I wasn't sure it was really meaningful to the user and might just confuse them if they don't realize that we've tried to do some things not specifically related to the original request. E.g. they do a write and get an error back saying that deletion failed.

}
if fi == nil {
return encErr
}
if fi.Size() == 0 {
os.Remove(fullPath)
}
return encErr
}

func (b *FileBackend) List(ctx context.Context, prefix string) ([]string, error) {
Expand Down