Improve sftp provider logic

Tests are failing because pkg/sftp won't overwrite an existing file
(Rename function) and the test is actually doing that with
blobtesting.VerifyStorage.

The solution is to use pkg/sftp's PosixRename function:
"PosixRename renames a file using the posix-rename@openssh.com
extension which will replace newname if it already exists."

Additionally, the provider now creates the path on the server, if it
doesn't exist.
This commit is contained in:
Juan B. Rodriguez
2019-07-16 15:17:59 -05:00
committed by Jarek Kowalski
parent 41530777a7
commit 4e2ceea146
2 changed files with 11 additions and 24 deletions

View File

@@ -86,12 +86,10 @@ func (s *sftpImpl) PutBlobInPath(ctx context.Context, dirPath, path string, data
tempFile := fmt.Sprintf("%s.tmp.%x", path, randSuffix)
f, err := s.createTempFileAndDir(tempFile)
if err != nil {
fmt.Printf("putblob-createtempfileanddir(%s)\n", err)
return errors.Wrap(err, "cannot create temporary file")
}
if _, err = f.Write(data); err != nil {
fmt.Printf("putblob-write(%s)\n", err)
return errors.Wrap(err, "can't write temporary file")
}
@@ -99,7 +97,7 @@ func (s *sftpImpl) PutBlobInPath(ctx context.Context, dirPath, path string, data
return errors.Wrap(err, "can't close temporary file")
}
err = s.cli.Rename(tempFile, path)
err = s.cli.PosixRename(tempFile, path)
if err != nil {
if removeErr := s.cli.Remove(tempFile); removeErr != nil {
fmt.Printf("warning: can't remove temp file: %v", removeErr)
@@ -259,7 +257,13 @@ func New(ctx context.Context, opts *Options) (blob.Storage, error) {
}
if _, err = c.Stat(opts.Path); err != nil {
return nil, errors.Wrapf(err, "path doesn't exist: %s", opts.Path)
if os.IsNotExist(err) {
if err = c.MkdirAll(opts.Path); err != nil {
return nil, errors.Wrap(err, "cannot create path")
}
} else {
return nil, errors.Wrapf(err, "path doesn't exist: %s", opts.Path)
}
}
r := &sftpStorage{

View File

@@ -20,8 +20,7 @@
func TestSFTPStorageValid(t *testing.T) {
ctx := context.Background()
additionalPath := ""
st, err := createSFTPStorage(ctx, additionalPath, t)
st, err := createSFTPStorage(ctx, t)
if err != nil {
t.Fatalf("unable to connect to SSH: %v", err)
@@ -46,22 +45,6 @@ func TestSFTPStorageValid(t *testing.T) {
}
}
func TestSFTPStorageInvalid(t *testing.T) {
ctx := context.Background()
additionalPath := "-no-such-path"
st, err := createSFTPStorage(ctx, additionalPath, t)
if err != nil {
t.Fatalf("unable to connect to SSH: %v", err)
}
defer st.Close(ctx)
if err := st.PutBlob(ctx, t1, []byte{1}); err == nil {
t.Errorf("unexpected success when adding to non-existent path")
}
}
func assertNoError(t *testing.T, err error) {
t.Helper()
if err != nil {
@@ -77,7 +60,7 @@ func deleteBlobs(ctx context.Context, t *testing.T, st blob.Storage) {
}
}
func createSFTPStorage(ctx context.Context, additionalPath string, t *testing.T) (blob.Storage, error) {
func createSFTPStorage(ctx context.Context, t *testing.T) (blob.Storage, error) {
host := os.Getenv("KOPIA_SFTP_TEST_HOST")
if host == "" {
t.Skip("KOPIA_SFTP_TEST_HOST not provided")
@@ -113,7 +96,7 @@ func createSFTPStorage(ctx context.Context, additionalPath string, t *testing.T)
}
return sftp.New(ctx, &sftp.Options{
Path: path + additionalPath,
Path: path,
Host: host,
Username: usr,
Port: int(port),