fix(providers): fixed rclone connection in KopiaUI (#1712) (#1713)

v0.10.3 introduced a regression where RClone connections did not
work in KopiaUI but were ok in the CLI.

The root cause was #1691 which caused storage context to be closed
after opening the repository.

Added test that verifies that storage does not rely on the context
remaining open.

repo/blob/azure/azure_storage_test.go

@@ -110,12 +110,16 @@ func TestAzureStorage(t *testing.T) {
 
 	ctx := testlogging.Context(t)
 
-	st, err := azure.New(ctx, &azure.Options{
+	// use context that gets canceled after opening storage to ensure it's not used beyond New().
+	newctx, cancel := context.WithCancel(ctx)
+	st, err := azure.New(newctx, &azure.Options{
 		Container:      container,
 		StorageAccount: storageAccount,
 		StorageKey:     storageKey,
 		Prefix:         fmt.Sprintf("test-%v-%x-", clock.Now().Unix(), data),
 	})
+
+	cancel()
 	require.NoError(t, err)
 
 	defer st.Close(ctx)
@@ -138,13 +142,18 @@ func TestAzureStorageSASToken(t *testing.T) {
 
 	ctx := testlogging.Context(t)
 
-	st, err := azure.New(ctx, &azure.Options{
+	// use context that gets canceled after storage is initialize,
+	// to verify we do not depend on the original context past initialization.
+	newctx, cancel := context.WithCancel(ctx)
+	st, err := azure.New(newctx, &azure.Options{
 		Container:      container,
 		StorageAccount: storageAccount,
 		SASToken:       sasToken,
 		Prefix:         fmt.Sprintf("sastest-%v-%x-", clock.Now().Unix(), data),
 	})
+
 	require.NoError(t, err)
+	cancel()
 
 	defer st.Close(ctx)
 	defer blobtesting.CleanupOldData(ctx, t, st, 0)

repo/blob/b2/b2_storage_test.go

@@ -73,7 +73,12 @@ func TestB2Storage(t *testing.T) {
 	}
 
 	ctx := testlogging.Context(t)
-	st, err := b2.New(ctx, opt)
+
+	// use context that gets canceled after opening storage to ensure it's not used beyond New().
+	newctx, cancel := context.WithCancel(ctx)
+	st, err := b2.New(newctx, opt)
+
+	cancel()
 	require.NoError(t, err)
 
 	defer st.Close(ctx)

repo/blob/filesystem/filesystem_storage_test.go

@@ -1,6 +1,7 @@
 package filesystem
 
 import (
+	"context"
 	"path/filepath"
 	"reflect"
 	"sort"
@@ -35,13 +36,17 @@ func TestFileStorage(t *testing.T) {
 	} {
 		path := testutil.TempDirectory(t)
 
-		r, err := New(ctx, &Options{
+		newctx, cancel := context.WithCancel(ctx)
+
+		// use context that gets canceled after opening storage to ensure it's not used beyond New().
+		r, err := New(newctx, &Options{
 			Path: path,
 			Options: sharded.Options{
 				DirectoryShards: shardSpec,
 			},
 		}, true)
 
+		cancel()
 		require.NoError(t, err)
 		require.NotNil(t, r)
 

repo/blob/gcs/gcs_storage_test.go

@@ -3,6 +3,7 @@
 import (
 	"bytes"
 	"compress/gzip"
+	"context"
 	"encoding/base64"
 	"io"
 	"os"
@@ -38,7 +39,11 @@ func TestGCSStorage(t *testing.T) {
 
 	ctx := testlogging.Context(t)
 
-	st, err := gcs.New(ctx, mustGetOptionsOrSkip(t, uuid.NewString()))
+	// use context that gets canceled after opening storage to ensure it's not used beyond New().
+	newctx, cancel := context.WithCancel(ctx)
+	st, err := gcs.New(newctx, mustGetOptionsOrSkip(t, uuid.NewString()))
+
+	cancel()
 	require.NoError(t, err)
 
 	defer st.Close(ctx)

repo/blob/rclone/rclone_storage.go

@@ -269,7 +269,7 @@ func New(ctx context.Context, opt *Options, isCreate bool) (blob.Storage, error)
 		arguments = append(arguments, "--config", tmpConfigFile)
 	}
 
-	r.cmd = exec.CommandContext(ctx, rcloneExe, arguments...) //nolint:gosec
+	r.cmd = exec.Command(rcloneExe, arguments...) //nolint:gosec
 	r.cmd.Env = append(r.cmd.Env, opt.RCloneEnv...)
 
 	startupTimeout := rcloneStartupTimeout

repo/blob/rclone/rclone_storage_test.go

@@ -1,6 +1,7 @@
 package rclone_test
 
 import (
+	"context"
 	"encoding/base64"
 	"encoding/json"
 	"os"
@@ -64,11 +65,16 @@ func TestRCloneStorage(t *testing.T) {
 	rcloneExe := mustGetRcloneExeOrSkip(t)
 	dataDir := testutil.TempDirectory(t)
 
-	st, err := rclone.New(ctx, &rclone.Options{
+	// use context that gets canceled after opening storage to ensure it's not used beyond New().
+	newctx, cancel := context.WithCancel(ctx)
+	st, err := rclone.New(newctx, &rclone.Options{
 		// pass local file as remote path.
 		RemotePath: dataDir,
 		RCloneExe:  rcloneExe,
 	}, true)
+
+	cancel()
+
 	if err != nil {
 		t.Fatalf("unable to connect to rclone backend: %v", err)
 	}

repo/blob/s3/s3_storage_test.go

@@ -1,6 +1,7 @@
 package s3
 
 import (
+	"context"
 	"crypto/tls"
 	"encoding/json"
 	"errors"
@@ -407,6 +408,7 @@ func testStorage(t *testing.T, options *Options, runValidationTest bool, opts bl
 	require.Equal(t, "", options.Prefix)
 
 	st0, err := New(ctx, options)
+
 	require.NoError(t, err)
 
 	defer st0.Close(ctx)
@@ -415,7 +417,11 @@ func testStorage(t *testing.T, options *Options, runValidationTest bool, opts bl
 
 	options.Prefix = uuid.NewString()
 
-	st, err := New(ctx, options)
+	// use context that gets canceled after opening storage to ensure it's not used beyond New().
+	newctx, cancel := context.WithCancel(ctx)
+	st, err := New(newctx, options)
+
+	cancel()
 	require.NoError(t, err)
 
 	defer st.Close(ctx)

repo/blob/sftp/sftp_storage_test.go

@@ -182,7 +182,9 @@ func TestSFTPStorageValid(t *testing.T) {
 		t.Run(fmt.Sprintf("Embed=%v", embedCreds), func(t *testing.T) {
 			ctx := testlogging.Context(t)
 
-			st, err := createSFTPStorage(ctx, t, sftp.Options{
+			// use context that gets canceled after opening storage to ensure it's not used beyond New().
+			newctx, cancel := context.WithCancel(ctx)
+			st, err := createSFTPStorage(newctx, t, sftp.Options{
 				Path:           "/upload",
 				Host:           host,
 				Username:       sftpUsernameWithKeyAuth,
@@ -194,6 +196,8 @@ func TestSFTPStorageValid(t *testing.T) {
 				t.Fatalf("unable to connect to SSH: %v", err)
 			}
 
+			cancel()
+
 			deleteBlobs(ctx, t, st)
 
 			blobtesting.VerifyStorage(ctx, t, st, blob.PutOptions{})

repo/blob/webdav/webdav_storage_test.go

@@ -2,6 +2,7 @@
 
 import (
 	"bytes"
+	"context"
 	"fmt"
 	"io"
 	"net/http"
@@ -148,7 +149,9 @@ func transformMissingPUTs(next http.Handler) http.HandlerFunc {
 func verifyWebDAVStorage(t *testing.T, url, username, password string, shardSpec []int) {
 	ctx := testlogging.Context(t)
 
-	st, err := New(testlogging.Context(t), &Options{
+	// use context that gets canceled after opening storage to ensure it's not used beyond New().
+	newctx, cancel := context.WithCancel(ctx)
+	st, err := New(newctx, &Options{
 		URL: url,
 		Options: sharded.Options{
 			DirectoryShards: shardSpec,
@@ -157,6 +160,8 @@ func verifyWebDAVStorage(t *testing.T, url, username, password string, shardSpec
 		Password: password,
 	}, false)
 
+	cancel()
+
 	if st == nil || err != nil {
 		t.Errorf("unexpected result: %v %v", st, err)
 	}