From 56836364a42911a85fba0f78535e60f0a326fd10 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 8 Jun 2023 13:11:34 +0200 Subject: [PATCH 01/14] backend: pass context into every backend constructor --- cmd/restic/global.go | 6 +++--- internal/backend/gs/gs.go | 2 +- internal/backend/gs/gs_test.go | 4 ++-- internal/backend/rclone/backend.go | 12 ++++++------ internal/backend/rclone/backend_test.go | 2 +- internal/backend/rclone/internal_test.go | 4 ++-- internal/backend/rest/rest.go | 4 ++-- internal/backend/rest/rest_int_test.go | 2 +- internal/backend/rest/rest_test.go | 2 +- 9 files changed, 19 insertions(+), 19 deletions(-) diff --git a/cmd/restic/global.go b/cmd/restic/global.go index 1b9c5b33d..3136e8990 100644 --- a/cmd/restic/global.go +++ b/cmd/restic/global.go @@ -584,7 +584,7 @@ func open(ctx context.Context, s string, gopts GlobalOptions, opts options.Optio case "s3": be, err = s3.Open(ctx, *cfg.(*s3.Config), rt) case "gs": - be, err = gs.Open(*cfg.(*gs.Config), rt) + be, err = gs.Open(ctx, *cfg.(*gs.Config), rt) case "azure": be, err = azure.Open(ctx, *cfg.(*azure.Config), rt) case "swift": @@ -592,9 +592,9 @@ func open(ctx context.Context, s string, gopts GlobalOptions, opts options.Optio case "b2": be, err = b2.Open(ctx, *cfg.(*b2.Config), rt) case "rest": - be, err = rest.Open(*cfg.(*rest.Config), rt) + be, err = rest.Open(ctx, *cfg.(*rest.Config), rt) case "rclone": - be, err = rclone.Open(*cfg.(*rclone.Config), lim) + be, err = rclone.Open(ctx, *cfg.(*rclone.Config), lim) default: return nil, errors.Fatalf("invalid backend: %q", loc.Scheme) diff --git a/internal/backend/gs/gs.go b/internal/backend/gs/gs.go index 7b5489111..022f2534a 100644 --- a/internal/backend/gs/gs.go +++ b/internal/backend/gs/gs.go @@ -117,7 +117,7 @@ func open(cfg Config, rt http.RoundTripper) (*Backend, error) { } // Open opens the gs backend at the specified bucket. -func Open(cfg Config, rt http.RoundTripper) (restic.Backend, error) { +func Open(_ context.Context, cfg Config, rt http.RoundTripper) (restic.Backend, error) { return open(cfg, rt) } diff --git a/internal/backend/gs/gs_test.go b/internal/backend/gs/gs_test.go index f96b6c62b..a73fe77fb 100644 --- a/internal/backend/gs/gs_test.go +++ b/internal/backend/gs/gs_test.go @@ -58,12 +58,12 @@ func newGSTestSuite(t testing.TB) *test.Suite[gs.Config] { // OpenFn is a function that opens a previously created temporary repository. Open: func(cfg gs.Config) (restic.Backend, error) { - return gs.Open(cfg, tr) + return gs.Open(context.TODO(), cfg, tr) }, // CleanupFn removes data created during the tests. Cleanup: func(cfg gs.Config) error { - be, err := gs.Open(cfg, tr) + be, err := gs.Open(context.TODO(), cfg, tr) if err != nil { return err } diff --git a/internal/backend/rclone/backend.go b/internal/backend/rclone/backend.go index 085c89945..881990bc3 100644 --- a/internal/backend/rclone/backend.go +++ b/internal/backend/rclone/backend.go @@ -134,7 +134,7 @@ func wrapConn(c *StdioConn, lim limiter.Limiter) *wrappedConn { } // New initializes a Backend and starts the process. -func newBackend(cfg Config, lim limiter.Limiter) (*Backend, error) { +func newBackend(ctx context.Context, cfg Config, lim limiter.Limiter) (*Backend, error) { var ( args []string err error @@ -197,7 +197,7 @@ func newBackend(cfg Config, lim limiter.Limiter) (*Backend, error) { wg: wg, } - ctx, cancel := context.WithCancel(context.Background()) + ctx, cancel := context.WithCancel(ctx) defer cancel() wg.Add(1) @@ -256,8 +256,8 @@ func newBackend(cfg Config, lim limiter.Limiter) (*Backend, error) { } // Open starts an rclone process with the given config. -func Open(cfg Config, lim limiter.Limiter) (*Backend, error) { - be, err := newBackend(cfg, lim) +func Open(ctx context.Context, cfg Config, lim limiter.Limiter) (*Backend, error) { + be, err := newBackend(ctx, cfg, lim) if err != nil { return nil, err } @@ -272,7 +272,7 @@ func Open(cfg Config, lim limiter.Limiter) (*Backend, error) { URL: url, } - restBackend, err := rest.Open(restConfig, debug.RoundTripper(be.tr)) + restBackend, err := rest.Open(ctx, restConfig, debug.RoundTripper(be.tr)) if err != nil { _ = be.Close() return nil, err @@ -284,7 +284,7 @@ func Open(cfg Config, lim limiter.Limiter) (*Backend, error) { // Create initializes a new restic repo with rclone. func Create(ctx context.Context, cfg Config) (*Backend, error) { - be, err := newBackend(cfg, nil) + be, err := newBackend(ctx, cfg, nil) if err != nil { return nil, err } diff --git a/internal/backend/rclone/backend_test.go b/internal/backend/rclone/backend_test.go index c497271f6..a562a99e6 100644 --- a/internal/backend/rclone/backend_test.go +++ b/internal/backend/rclone/backend_test.go @@ -39,7 +39,7 @@ func newTestSuite(t testing.TB) *test.Suite[rclone.Config] { // OpenFn is a function that opens a previously created temporary repository. Open: func(cfg rclone.Config) (restic.Backend, error) { t.Logf("Open()") - return rclone.Open(cfg, nil) + return rclone.Open(context.TODO(), cfg, nil) }, } } diff --git a/internal/backend/rclone/internal_test.go b/internal/backend/rclone/internal_test.go index bfec2b98c..32fe850a0 100644 --- a/internal/backend/rclone/internal_test.go +++ b/internal/backend/rclone/internal_test.go @@ -15,7 +15,7 @@ func TestRcloneExit(t *testing.T) { dir := rtest.TempDir(t) cfg := NewConfig() cfg.Remote = dir - be, err := Open(cfg, nil) + be, err := Open(context.TODO(), cfg, nil) var e *exec.Error if errors.As(err, &e) && e.Err == exec.ErrNotFound { t.Skipf("program %q not found", e.Name) @@ -45,7 +45,7 @@ func TestRcloneFailedStart(t *testing.T) { cfg := NewConfig() // exits with exit code 1 cfg.Program = "false" - _, err := Open(cfg, nil) + _, err := Open(context.TODO(), cfg, nil) var e *exec.ExitError if !errors.As(err, &e) { // unexpected error diff --git a/internal/backend/rest/rest.go b/internal/backend/rest/rest.go index 68397cd1b..0906a7f10 100644 --- a/internal/backend/rest/rest.go +++ b/internal/backend/rest/rest.go @@ -36,7 +36,7 @@ const ( ) // Open opens the REST backend with the given config. -func Open(cfg Config, rt http.RoundTripper) (*Backend, error) { +func Open(_ context.Context, cfg Config, rt http.RoundTripper) (*Backend, error) { // use url without trailing slash for layout url := cfg.URL.String() if url[len(url)-1] == '/' { @@ -55,7 +55,7 @@ func Open(cfg Config, rt http.RoundTripper) (*Backend, error) { // Create creates a new REST on server configured in config. func Create(ctx context.Context, cfg Config, rt http.RoundTripper) (*Backend, error) { - be, err := Open(cfg, rt) + be, err := Open(ctx, cfg, rt) if err != nil { return nil, err } diff --git a/internal/backend/rest/rest_int_test.go b/internal/backend/rest/rest_int_test.go index 7184f5fbe..e7810c5e3 100644 --- a/internal/backend/rest/rest_int_test.go +++ b/internal/backend/rest/rest_int_test.go @@ -117,7 +117,7 @@ func TestListAPI(t *testing.T) { URL: srvURL, } - be, err := rest.Open(cfg, http.DefaultTransport) + be, err := rest.Open(context.TODO(), cfg, http.DefaultTransport) if err != nil { t.Fatal(err) } diff --git a/internal/backend/rest/rest_test.go b/internal/backend/rest/rest_test.go index 2ebd00f5e..4d069e63c 100644 --- a/internal/backend/rest/rest_test.go +++ b/internal/backend/rest/rest_test.go @@ -90,7 +90,7 @@ func newTestSuite(_ context.Context, t testing.TB, url *url.URL, minimalData boo // OpenFn is a function that opens a previously created temporary repository. Open: func(cfg rest.Config) (restic.Backend, error) { - return rest.Open(cfg, tr) + return rest.Open(context.TODO(), cfg, tr) }, // CleanupFn removes data created during the tests. From 7d12c292863efbd51c28781ce8be478bb404b994 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 8 Jun 2023 13:04:34 +0200 Subject: [PATCH 02/14] backend: Unify backend construction using factory and registry This unified construction removes most backend-specific code from global.go. The backend registry will also enable integration tests to use custom backends if necessary. --- cmd/restic/cmd_init.go | 10 +-- cmd/restic/global.go | 76 ++++++----------- cmd/restic/integration_helpers_test.go | 2 + internal/backend/azure/azure.go | 5 ++ internal/backend/b2/b2.go | 5 ++ internal/backend/gs/gs.go | 5 ++ internal/backend/local/local.go | 10 +++ internal/backend/location/location.go | 67 ++++----------- internal/backend/location/location_test.go | 67 +++++++-------- internal/backend/location/registry.go | 94 ++++++++++++++++++++++ internal/backend/rclone/backend.go | 9 ++- internal/backend/rclone/backend_test.go | 2 +- internal/backend/rest/rest.go | 5 ++ internal/backend/s3/s3.go | 5 ++ internal/backend/sftp/sftp.go | 10 +++ internal/backend/swift/swift.go | 5 ++ 16 files changed, 235 insertions(+), 142 deletions(-) create mode 100644 internal/backend/location/registry.go diff --git a/cmd/restic/cmd_init.go b/cmd/restic/cmd_init.go index 43de7ff89..b9dabdc2d 100644 --- a/cmd/restic/cmd_init.go +++ b/cmd/restic/cmd_init.go @@ -87,9 +87,9 @@ func runInit(ctx context.Context, opts InitOptions, gopts GlobalOptions, args [] return err } - be, err := create(ctx, repo, gopts.extended) + be, err := create(ctx, repo, gopts, gopts.extended) if err != nil { - return errors.Fatalf("create repository at %s failed: %v\n", location.StripPassword(gopts.Repo), err) + return errors.Fatalf("create repository at %s failed: %v\n", location.StripPassword(gopts.backends, gopts.Repo), err) } s, err := repository.New(be, repository.Options{ @@ -102,11 +102,11 @@ func runInit(ctx context.Context, opts InitOptions, gopts GlobalOptions, args [] err = s.Init(ctx, version, gopts.password, chunkerPolynomial) if err != nil { - return errors.Fatalf("create key in repository at %s failed: %v\n", location.StripPassword(gopts.Repo), err) + return errors.Fatalf("create key in repository at %s failed: %v\n", location.StripPassword(gopts.backends, gopts.Repo), err) } if !gopts.JSON { - Verbosef("created restic repository %v at %s", s.Config().ID[:10], location.StripPassword(gopts.Repo)) + Verbosef("created restic repository %v at %s", s.Config().ID[:10], location.StripPassword(gopts.backends, gopts.Repo)) if opts.CopyChunkerParameters && chunkerPolynomial != nil { Verbosef(" with chunker parameters copied from secondary repository\n") } else { @@ -121,7 +121,7 @@ func runInit(ctx context.Context, opts InitOptions, gopts GlobalOptions, args [] status := initSuccess{ MessageType: "initialized", ID: s.Config().ID, - Repository: location.StripPassword(gopts.Repo), + Repository: location.StripPassword(gopts.backends, gopts.Repo), } return json.NewEncoder(globalOptions.stdout).Encode(status) } diff --git a/cmd/restic/global.go b/cmd/restic/global.go index 3136e8990..f4886ec90 100644 --- a/cmd/restic/global.go +++ b/cmd/restic/global.go @@ -75,6 +75,7 @@ type GlobalOptions struct { stdout io.Writer stderr io.Writer + backends *location.Registry backendTestHook, backendInnerTestHook backendWrapper // verbosity is set as follows: @@ -98,6 +99,18 @@ var isReadingPassword bool var internalGlobalCtx context.Context func init() { + backends := location.NewRegistry() + backends.Register("b2", b2.NewFactory()) + backends.Register("local", local.NewFactory()) + backends.Register("sftp", sftp.NewFactory()) + backends.Register("s3", s3.NewFactory()) + backends.Register("gs", gs.NewFactory()) + backends.Register("azure", azure.NewFactory()) + backends.Register("swift", swift.NewFactory()) + backends.Register("rest", rest.NewFactory()) + backends.Register("rclone", rclone.NewFactory()) + globalOptions.backends = backends + var cancel context.CancelFunc internalGlobalCtx, cancel = context.WithCancel(context.Background()) AddCleanupHandler(func(code int) (int, error) { @@ -554,8 +567,8 @@ func parseConfig(loc location.Location, opts options.Options) (interface{}, erro // Open the backend specified by a location config. func open(ctx context.Context, s string, gopts GlobalOptions, opts options.Options) (restic.Backend, error) { - debug.Log("parsing location %v", location.StripPassword(s)) - loc, err := location.Parse(s) + debug.Log("parsing location %v", location.StripPassword(gopts.backends, s)) + loc, err := location.Parse(gopts.backends, s) if err != nil { return nil, errors.Fatalf("parsing repository location failed: %v", err) } @@ -576,32 +589,14 @@ func open(ctx context.Context, s string, gopts GlobalOptions, opts options.Optio lim := limiter.NewStaticLimiter(gopts.Limits) rt = lim.Transport(rt) - switch loc.Scheme { - case "local": - be, err = local.Open(ctx, *cfg.(*local.Config)) - case "sftp": - be, err = sftp.Open(ctx, *cfg.(*sftp.Config)) - case "s3": - be, err = s3.Open(ctx, *cfg.(*s3.Config), rt) - case "gs": - be, err = gs.Open(ctx, *cfg.(*gs.Config), rt) - case "azure": - be, err = azure.Open(ctx, *cfg.(*azure.Config), rt) - case "swift": - be, err = swift.Open(ctx, *cfg.(*swift.Config), rt) - case "b2": - be, err = b2.Open(ctx, *cfg.(*b2.Config), rt) - case "rest": - be, err = rest.Open(ctx, *cfg.(*rest.Config), rt) - case "rclone": - be, err = rclone.Open(ctx, *cfg.(*rclone.Config), lim) - - default: + factory := gopts.backends.Lookup(loc.Scheme) + if factory == nil { return nil, errors.Fatalf("invalid backend: %q", loc.Scheme) } + be, err = factory.Open(ctx, cfg, rt, lim) if err != nil { - return nil, errors.Fatalf("unable to open repository at %v: %v", location.StripPassword(s), err) + return nil, errors.Fatalf("unable to open repository at %v: %v", location.StripPassword(gopts.backends, s), err) } // wrap with debug logging and connection limiting @@ -623,7 +618,7 @@ func open(ctx context.Context, s string, gopts GlobalOptions, opts options.Optio // check if config is there fi, err := be.Stat(ctx, restic.Handle{Type: restic.ConfigFile}) if err != nil { - return nil, errors.Fatalf("unable to open config file: %v\nIs there a repository at the following location?\n%v", err, location.StripPassword(s)) + return nil, errors.Fatalf("unable to open config file: %v\nIs there a repository at the following location?\n%v", err, location.StripPassword(gopts.backends, s)) } if fi.Size == 0 { @@ -634,9 +629,9 @@ func open(ctx context.Context, s string, gopts GlobalOptions, opts options.Optio } // Create the backend specified by URI. -func create(ctx context.Context, s string, opts options.Options) (restic.Backend, error) { +func create(ctx context.Context, s string, gopts GlobalOptions, opts options.Options) (restic.Backend, error) { debug.Log("parsing location %v", s) - loc, err := location.Parse(s) + loc, err := location.Parse(gopts.backends, s) if err != nil { return nil, err } @@ -651,31 +646,12 @@ func create(ctx context.Context, s string, opts options.Options) (restic.Backend return nil, err } - var be restic.Backend - switch loc.Scheme { - case "local": - be, err = local.Create(ctx, *cfg.(*local.Config)) - case "sftp": - be, err = sftp.Create(ctx, *cfg.(*sftp.Config)) - case "s3": - be, err = s3.Create(ctx, *cfg.(*s3.Config), rt) - case "gs": - be, err = gs.Create(ctx, *cfg.(*gs.Config), rt) - case "azure": - be, err = azure.Create(ctx, *cfg.(*azure.Config), rt) - case "swift": - be, err = swift.Open(ctx, *cfg.(*swift.Config), rt) - case "b2": - be, err = b2.Create(ctx, *cfg.(*b2.Config), rt) - case "rest": - be, err = rest.Create(ctx, *cfg.(*rest.Config), rt) - case "rclone": - be, err = rclone.Create(ctx, *cfg.(*rclone.Config)) - default: - debug.Log("invalid repository scheme: %v", s) - return nil, errors.Fatalf("invalid scheme %q", loc.Scheme) + factory := gopts.backends.Lookup(loc.Scheme) + if factory == nil { + return nil, errors.Fatalf("invalid backend: %q", loc.Scheme) } + be, err := factory.Create(ctx, cfg, rt, nil) if err != nil { return nil, err } diff --git a/cmd/restic/integration_helpers_test.go b/cmd/restic/integration_helpers_test.go index a0e4d49d6..b7cb5b333 100644 --- a/cmd/restic/integration_helpers_test.go +++ b/cmd/restic/integration_helpers_test.go @@ -206,6 +206,8 @@ func withTestEnvironment(t testing.TB) (env *testEnvironment, cleanup func()) { // replace this hook with "nil" if listing a filetype more than once is necessary backendTestHook: func(r restic.Backend) (restic.Backend, error) { return newOrderedListOnceBackend(r), nil }, + // start with default set of backends + backends: globalOptions.backends, } // always overwrite global options diff --git a/internal/backend/azure/azure.go b/internal/backend/azure/azure.go index 4041d3adc..b33b8dca6 100644 --- a/internal/backend/azure/azure.go +++ b/internal/backend/azure/azure.go @@ -14,6 +14,7 @@ import ( "github.com/restic/restic/internal/backend" "github.com/restic/restic/internal/backend/layout" + "github.com/restic/restic/internal/backend/location" "github.com/restic/restic/internal/debug" "github.com/restic/restic/internal/errors" "github.com/restic/restic/internal/restic" @@ -43,6 +44,10 @@ const defaultListMaxItems = 5000 // make sure that *Backend implements backend.Backend var _ restic.Backend = &Backend{} +func NewFactory() location.Factory { + return location.NewHTTPBackendFactory(ParseConfig, location.NoPassword, Create, Open) +} + func open(cfg Config, rt http.RoundTripper) (*Backend, error) { debug.Log("open, config %#v", cfg) var client *azContainer.Client diff --git a/internal/backend/b2/b2.go b/internal/backend/b2/b2.go index 7f4dba831..eb2cfe3c2 100644 --- a/internal/backend/b2/b2.go +++ b/internal/backend/b2/b2.go @@ -11,6 +11,7 @@ import ( "github.com/restic/restic/internal/backend" "github.com/restic/restic/internal/backend/layout" + "github.com/restic/restic/internal/backend/location" "github.com/restic/restic/internal/debug" "github.com/restic/restic/internal/errors" "github.com/restic/restic/internal/restic" @@ -36,6 +37,10 @@ const defaultListMaxItems = 10 * 1000 // ensure statically that *b2Backend implements restic.Backend. var _ restic.Backend = &b2Backend{} +func NewFactory() location.Factory { + return location.NewHTTPBackendFactory(ParseConfig, location.NoPassword, Create, Open) +} + type sniffingRoundTripper struct { sync.Mutex lastErr error diff --git a/internal/backend/gs/gs.go b/internal/backend/gs/gs.go index 022f2534a..445ccc77d 100644 --- a/internal/backend/gs/gs.go +++ b/internal/backend/gs/gs.go @@ -15,6 +15,7 @@ import ( "github.com/pkg/errors" "github.com/restic/restic/internal/backend" "github.com/restic/restic/internal/backend/layout" + "github.com/restic/restic/internal/backend/location" "github.com/restic/restic/internal/debug" "github.com/restic/restic/internal/restic" @@ -47,6 +48,10 @@ type Backend struct { // Ensure that *Backend implements restic.Backend. var _ restic.Backend = &Backend{} +func NewFactory() location.Factory { + return location.NewHTTPBackendFactory(ParseConfig, location.NoPassword, Create, Open) +} + func getStorageClient(rt http.RoundTripper) (*storage.Client, error) { // create a new HTTP client httpClient := &http.Client{ diff --git a/internal/backend/local/local.go b/internal/backend/local/local.go index d6bdef1e4..02ac81b8d 100644 --- a/internal/backend/local/local.go +++ b/internal/backend/local/local.go @@ -10,6 +10,8 @@ import ( "github.com/restic/restic/internal/backend" "github.com/restic/restic/internal/backend/layout" + "github.com/restic/restic/internal/backend/limiter" + "github.com/restic/restic/internal/backend/location" "github.com/restic/restic/internal/debug" "github.com/restic/restic/internal/errors" "github.com/restic/restic/internal/fs" @@ -28,6 +30,14 @@ type Local struct { // ensure statically that *Local implements restic.Backend. var _ restic.Backend = &Local{} +func NewFactory() location.Factory { + return location.NewLimitedBackendFactory(ParseConfig, location.NoPassword, func(ctx context.Context, cfg Config, _ limiter.Limiter) (*Local, error) { + return Create(ctx, cfg) + }, func(ctx context.Context, cfg Config, _ limiter.Limiter) (*Local, error) { + return Open(ctx, cfg) + }) +} + const defaultLayout = "default" func open(ctx context.Context, cfg Config) (*Local, error) { diff --git a/internal/backend/location/location.go b/internal/backend/location/location.go index 612ae1b4c..947ca17c3 100644 --- a/internal/backend/location/location.go +++ b/internal/backend/location/location.go @@ -4,15 +4,6 @@ package location import ( "strings" - "github.com/restic/restic/internal/backend/azure" - "github.com/restic/restic/internal/backend/b2" - "github.com/restic/restic/internal/backend/gs" - "github.com/restic/restic/internal/backend/local" - "github.com/restic/restic/internal/backend/rclone" - "github.com/restic/restic/internal/backend/rest" - "github.com/restic/restic/internal/backend/s3" - "github.com/restic/restic/internal/backend/sftp" - "github.com/restic/restic/internal/backend/swift" "github.com/restic/restic/internal/errors" ) @@ -23,34 +14,8 @@ type Location struct { Config interface{} } -type parser struct { - scheme string - parse func(string) (interface{}, error) - stripPassword func(string) string -} - -func configToAny[C any](parser func(string) (*C, error)) func(string) (interface{}, error) { - return func(s string) (interface{}, error) { - return parser(s) - } -} - -// parsers is a list of valid config parsers for the backends. The first parser -// is the fallback and should always be set to the local backend. -var parsers = []parser{ - {"b2", configToAny(b2.ParseConfig), noPassword}, - {"local", configToAny(local.ParseConfig), noPassword}, - {"sftp", configToAny(sftp.ParseConfig), noPassword}, - {"s3", configToAny(s3.ParseConfig), noPassword}, - {"gs", configToAny(gs.ParseConfig), noPassword}, - {"azure", configToAny(azure.ParseConfig), noPassword}, - {"swift", configToAny(swift.ParseConfig), noPassword}, - {"rest", configToAny(rest.ParseConfig), rest.StripPassword}, - {"rclone", configToAny(rclone.ParseConfig), noPassword}, -} - -// noPassword returns the repository location unchanged (there's no sensitive information there) -func noPassword(s string) string { +// NoPassword returns the repository location unchanged (there's no sensitive information there) +func NoPassword(s string) string { return s } @@ -88,16 +53,13 @@ func isPath(s string) bool { // starts with a backend name followed by a colon, that backend's Parse() // function is called. Otherwise, the local backend is used which interprets s // as the name of a directory. -func Parse(s string) (u Location, err error) { +func Parse(registry *Registry, s string) (u Location, err error) { scheme := extractScheme(s) u.Scheme = scheme - for _, parser := range parsers { - if parser.scheme != scheme { - continue - } - - u.Config, err = parser.parse(s) + factory := registry.Lookup(scheme) + if factory != nil { + u.Config, err = factory.ParseConfig(s) if err != nil { return Location{}, err } @@ -111,7 +73,12 @@ func Parse(s string) (u Location, err error) { } u.Scheme = "local" - u.Config, err = local.ParseConfig("local:" + s) + factory = registry.Lookup(u.Scheme) + if factory == nil { + return Location{}, errors.New("local backend not available") + } + + u.Config, err = factory.ParseConfig("local:" + s) if err != nil { return Location{}, err } @@ -120,14 +87,12 @@ func Parse(s string) (u Location, err error) { } // StripPassword returns a displayable version of a repository location (with any sensitive information removed) -func StripPassword(s string) string { +func StripPassword(registry *Registry, s string) string { scheme := extractScheme(s) - for _, parser := range parsers { - if parser.scheme != scheme { - continue - } - return parser.stripPassword(s) + factory := registry.Lookup(scheme) + if factory != nil { + return factory.StripPassword(s) } return s } diff --git a/internal/backend/location/location_test.go b/internal/backend/location/location_test.go index 9f5db70c9..6e9042200 100644 --- a/internal/backend/location/location_test.go +++ b/internal/backend/location/location_test.go @@ -1,4 +1,4 @@ -package location +package location_test import ( "net/url" @@ -7,6 +7,7 @@ import ( "github.com/restic/restic/internal/backend/b2" "github.com/restic/restic/internal/backend/local" + "github.com/restic/restic/internal/backend/location" "github.com/restic/restic/internal/backend/rest" "github.com/restic/restic/internal/backend/s3" "github.com/restic/restic/internal/backend/sftp" @@ -24,11 +25,11 @@ func parseURL(s string) *url.URL { var parseTests = []struct { s string - u Location + u location.Location }{ { "local:/srv/repo", - Location{Scheme: "local", + location.Location{Scheme: "local", Config: &local.Config{ Path: "/srv/repo", Connections: 2, @@ -37,7 +38,7 @@ var parseTests = []struct { }, { "local:dir1/dir2", - Location{Scheme: "local", + location.Location{Scheme: "local", Config: &local.Config{ Path: "dir1/dir2", Connections: 2, @@ -46,7 +47,7 @@ var parseTests = []struct { }, { "local:dir1/dir2", - Location{Scheme: "local", + location.Location{Scheme: "local", Config: &local.Config{ Path: "dir1/dir2", Connections: 2, @@ -55,7 +56,7 @@ var parseTests = []struct { }, { "dir1/dir2", - Location{Scheme: "local", + location.Location{Scheme: "local", Config: &local.Config{ Path: "dir1/dir2", Connections: 2, @@ -64,7 +65,7 @@ var parseTests = []struct { }, { "/dir1/dir2", - Location{Scheme: "local", + location.Location{Scheme: "local", Config: &local.Config{ Path: "/dir1/dir2", Connections: 2, @@ -73,7 +74,7 @@ var parseTests = []struct { }, { "local:../dir1/dir2", - Location{Scheme: "local", + location.Location{Scheme: "local", Config: &local.Config{ Path: "../dir1/dir2", Connections: 2, @@ -82,7 +83,7 @@ var parseTests = []struct { }, { "/dir1/dir2", - Location{Scheme: "local", + location.Location{Scheme: "local", Config: &local.Config{ Path: "/dir1/dir2", Connections: 2, @@ -91,7 +92,7 @@ var parseTests = []struct { }, { "/dir1:foobar/dir2", - Location{Scheme: "local", + location.Location{Scheme: "local", Config: &local.Config{ Path: "/dir1:foobar/dir2", Connections: 2, @@ -100,7 +101,7 @@ var parseTests = []struct { }, { `\dir1\foobar\dir2`, - Location{Scheme: "local", + location.Location{Scheme: "local", Config: &local.Config{ Path: `\dir1\foobar\dir2`, Connections: 2, @@ -109,7 +110,7 @@ var parseTests = []struct { }, { `c:\dir1\foobar\dir2`, - Location{Scheme: "local", + location.Location{Scheme: "local", Config: &local.Config{ Path: `c:\dir1\foobar\dir2`, Connections: 2, @@ -118,7 +119,7 @@ var parseTests = []struct { }, { `C:\Users\appveyor\AppData\Local\Temp\1\restic-test-879453535\repo`, - Location{Scheme: "local", + location.Location{Scheme: "local", Config: &local.Config{ Path: `C:\Users\appveyor\AppData\Local\Temp\1\restic-test-879453535\repo`, Connections: 2, @@ -127,7 +128,7 @@ var parseTests = []struct { }, { `c:/dir1/foobar/dir2`, - Location{Scheme: "local", + location.Location{Scheme: "local", Config: &local.Config{ Path: `c:/dir1/foobar/dir2`, Connections: 2, @@ -136,7 +137,7 @@ var parseTests = []struct { }, { "sftp:user@host:/srv/repo", - Location{Scheme: "sftp", + location.Location{Scheme: "sftp", Config: &sftp.Config{ User: "user", Host: "host", @@ -147,7 +148,7 @@ var parseTests = []struct { }, { "sftp:host:/srv/repo", - Location{Scheme: "sftp", + location.Location{Scheme: "sftp", Config: &sftp.Config{ User: "", Host: "host", @@ -158,7 +159,7 @@ var parseTests = []struct { }, { "sftp://user@host/srv/repo", - Location{Scheme: "sftp", + location.Location{Scheme: "sftp", Config: &sftp.Config{ User: "user", Host: "host", @@ -169,7 +170,7 @@ var parseTests = []struct { }, { "sftp://user@host//srv/repo", - Location{Scheme: "sftp", + location.Location{Scheme: "sftp", Config: &sftp.Config{ User: "user", Host: "host", @@ -181,7 +182,7 @@ var parseTests = []struct { { "s3://eu-central-1/bucketname", - Location{Scheme: "s3", + location.Location{Scheme: "s3", Config: &s3.Config{ Endpoint: "eu-central-1", Bucket: "bucketname", @@ -192,7 +193,7 @@ var parseTests = []struct { }, { "s3://hostname.foo/bucketname", - Location{Scheme: "s3", + location.Location{Scheme: "s3", Config: &s3.Config{ Endpoint: "hostname.foo", Bucket: "bucketname", @@ -203,7 +204,7 @@ var parseTests = []struct { }, { "s3://hostname.foo/bucketname/prefix/directory", - Location{Scheme: "s3", + location.Location{Scheme: "s3", Config: &s3.Config{ Endpoint: "hostname.foo", Bucket: "bucketname", @@ -214,7 +215,7 @@ var parseTests = []struct { }, { "s3:eu-central-1/repo", - Location{Scheme: "s3", + location.Location{Scheme: "s3", Config: &s3.Config{ Endpoint: "eu-central-1", Bucket: "repo", @@ -225,7 +226,7 @@ var parseTests = []struct { }, { "s3:eu-central-1/repo/prefix/directory", - Location{Scheme: "s3", + location.Location{Scheme: "s3", Config: &s3.Config{ Endpoint: "eu-central-1", Bucket: "repo", @@ -236,7 +237,7 @@ var parseTests = []struct { }, { "s3:https://hostname.foo/repo", - Location{Scheme: "s3", + location.Location{Scheme: "s3", Config: &s3.Config{ Endpoint: "hostname.foo", Bucket: "repo", @@ -247,7 +248,7 @@ var parseTests = []struct { }, { "s3:https://hostname.foo/repo/prefix/directory", - Location{Scheme: "s3", + location.Location{Scheme: "s3", Config: &s3.Config{ Endpoint: "hostname.foo", Bucket: "repo", @@ -258,7 +259,7 @@ var parseTests = []struct { }, { "s3:http://hostname.foo/repo", - Location{Scheme: "s3", + location.Location{Scheme: "s3", Config: &s3.Config{ Endpoint: "hostname.foo", Bucket: "repo", @@ -270,7 +271,7 @@ var parseTests = []struct { }, { "swift:container17:/", - Location{Scheme: "swift", + location.Location{Scheme: "swift", Config: &swift.Config{ Container: "container17", Prefix: "", @@ -280,7 +281,7 @@ var parseTests = []struct { }, { "swift:container17:/prefix97", - Location{Scheme: "swift", + location.Location{Scheme: "swift", Config: &swift.Config{ Container: "container17", Prefix: "prefix97", @@ -290,7 +291,7 @@ var parseTests = []struct { }, { "rest:http://hostname.foo:1234/", - Location{Scheme: "rest", + location.Location{Scheme: "rest", Config: &rest.Config{ URL: parseURL("http://hostname.foo:1234/"), Connections: 5, @@ -298,7 +299,7 @@ var parseTests = []struct { }, }, { - "b2:bucketname:/prefix", Location{Scheme: "b2", + "b2:bucketname:/prefix", location.Location{Scheme: "b2", Config: &b2.Config{ Bucket: "bucketname", Prefix: "prefix", @@ -307,7 +308,7 @@ var parseTests = []struct { }, }, { - "b2:bucketname", Location{Scheme: "b2", + "b2:bucketname", location.Location{Scheme: "b2", Config: &b2.Config{ Bucket: "bucketname", Prefix: "", @@ -320,7 +321,7 @@ var parseTests = []struct { func TestParse(t *testing.T) { for i, test := range parseTests { t.Run(test.s, func(t *testing.T) { - u, err := Parse(test.s) + u, err := location.Parse(test.s) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -346,7 +347,7 @@ func TestInvalidScheme(t *testing.T) { for _, s := range invalidSchemes { t.Run(s, func(t *testing.T) { - _, err := Parse(s) + _, err := location.Parse(s) if err == nil { t.Fatalf("error for invalid location %q not found", s) } diff --git a/internal/backend/location/registry.go b/internal/backend/location/registry.go new file mode 100644 index 000000000..5d644dfb9 --- /dev/null +++ b/internal/backend/location/registry.go @@ -0,0 +1,94 @@ +package location + +import ( + "context" + "net/http" + + "github.com/restic/restic/internal/backend/limiter" + "github.com/restic/restic/internal/restic" +) + +type Registry struct { + factories map[string]Factory +} + +func NewRegistry() *Registry { + return &Registry{ + factories: make(map[string]Factory), + } +} + +func (r *Registry) Register(scheme string, factory Factory) { + if r.factories[scheme] != nil { + panic("duplicate backend") + } + r.factories[scheme] = factory +} + +func (r *Registry) Lookup(scheme string) Factory { + return r.factories[scheme] +} + +type Factory interface { + ParseConfig(s string) (interface{}, error) + StripPassword(s string) string + Create(ctx context.Context, cfg interface{}, rt http.RoundTripper, lim limiter.Limiter) (restic.Backend, error) + Open(ctx context.Context, cfg interface{}, rt http.RoundTripper, lim limiter.Limiter) (restic.Backend, error) +} + +type GenericBackendFactory[C any, T restic.Backend] struct { + parseConfigFn func(s string) (*C, error) + stripPasswordFn func(s string) string + createFn func(ctx context.Context, cfg C, rt http.RoundTripper, lim limiter.Limiter) (T, error) + openFn func(ctx context.Context, cfg C, rt http.RoundTripper, lim limiter.Limiter) (T, error) +} + +func (f *GenericBackendFactory[C, T]) ParseConfig(s string) (interface{}, error) { + return f.parseConfigFn(s) +} +func (f *GenericBackendFactory[C, T]) StripPassword(s string) string { + if f.stripPasswordFn != nil { + return f.stripPasswordFn(s) + } + return s +} +func (f *GenericBackendFactory[C, T]) Create(ctx context.Context, cfg interface{}, rt http.RoundTripper, lim limiter.Limiter) (restic.Backend, error) { + return f.createFn(ctx, *cfg.(*C), rt, lim) +} +func (f *GenericBackendFactory[C, T]) Open(ctx context.Context, cfg interface{}, rt http.RoundTripper, lim limiter.Limiter) (restic.Backend, error) { + return f.openFn(ctx, *cfg.(*C), rt, lim) +} + +func NewHTTPBackendFactory[C any, T restic.Backend](parseConfigFn func(s string) (*C, error), + stripPasswordFn func(s string) string, + createFn func(ctx context.Context, cfg C, rt http.RoundTripper) (T, error), + openFn func(ctx context.Context, cfg C, rt http.RoundTripper) (T, error)) *GenericBackendFactory[C, T] { + + return &GenericBackendFactory[C, T]{ + parseConfigFn: parseConfigFn, + stripPasswordFn: stripPasswordFn, + createFn: func(ctx context.Context, cfg C, rt http.RoundTripper, _ limiter.Limiter) (T, error) { + return createFn(ctx, cfg, rt) + }, + openFn: func(ctx context.Context, cfg C, rt http.RoundTripper, _ limiter.Limiter) (T, error) { + return openFn(ctx, cfg, rt) + }, + } +} + +func NewLimitedBackendFactory[C any, T restic.Backend](parseConfigFn func(s string) (*C, error), + stripPasswordFn func(s string) string, + createFn func(ctx context.Context, cfg C, lim limiter.Limiter) (T, error), + openFn func(ctx context.Context, cfg C, lim limiter.Limiter) (T, error)) *GenericBackendFactory[C, T] { + + return &GenericBackendFactory[C, T]{ + parseConfigFn: parseConfigFn, + stripPasswordFn: stripPasswordFn, + createFn: func(ctx context.Context, cfg C, _ http.RoundTripper, lim limiter.Limiter) (T, error) { + return createFn(ctx, cfg, lim) + }, + openFn: func(ctx context.Context, cfg C, _ http.RoundTripper, lim limiter.Limiter) (T, error) { + return openFn(ctx, cfg, lim) + }, + } +} diff --git a/internal/backend/rclone/backend.go b/internal/backend/rclone/backend.go index 881990bc3..f3a97ef75 100644 --- a/internal/backend/rclone/backend.go +++ b/internal/backend/rclone/backend.go @@ -19,6 +19,7 @@ import ( "github.com/cenkalti/backoff/v4" "github.com/restic/restic/internal/backend" "github.com/restic/restic/internal/backend/limiter" + "github.com/restic/restic/internal/backend/location" "github.com/restic/restic/internal/backend/rest" "github.com/restic/restic/internal/debug" "github.com/restic/restic/internal/errors" @@ -36,6 +37,10 @@ type Backend struct { conn *StdioConn } +func NewFactory() location.Factory { + return location.NewLimitedBackendFactory(ParseConfig, location.NoPassword, Create, Open) +} + // run starts command with args and initializes the StdioConn. func run(command string, args ...string) (*StdioConn, *sync.WaitGroup, chan struct{}, func() error, error) { cmd := exec.Command(command, args...) @@ -283,8 +288,8 @@ func Open(ctx context.Context, cfg Config, lim limiter.Limiter) (*Backend, error } // Create initializes a new restic repo with rclone. -func Create(ctx context.Context, cfg Config) (*Backend, error) { - be, err := newBackend(ctx, cfg, nil) +func Create(ctx context.Context, cfg Config, lim limiter.Limiter) (*Backend, error) { + be, err := newBackend(ctx, cfg, lim) if err != nil { return nil, err } diff --git a/internal/backend/rclone/backend_test.go b/internal/backend/rclone/backend_test.go index a562a99e6..738462577 100644 --- a/internal/backend/rclone/backend_test.go +++ b/internal/backend/rclone/backend_test.go @@ -27,7 +27,7 @@ func newTestSuite(t testing.TB) *test.Suite[rclone.Config] { // CreateFn is a function that creates a temporary repository for the tests. Create: func(cfg rclone.Config) (restic.Backend, error) { t.Logf("Create()") - be, err := rclone.Create(context.TODO(), cfg) + be, err := rclone.Create(context.TODO(), cfg, nil) var e *exec.Error if errors.As(err, &e) && e.Err == exec.ErrNotFound { t.Skipf("program %q not found", e.Name) diff --git a/internal/backend/rest/rest.go b/internal/backend/rest/rest.go index 0906a7f10..4fb2d54de 100644 --- a/internal/backend/rest/rest.go +++ b/internal/backend/rest/rest.go @@ -13,6 +13,7 @@ import ( "github.com/restic/restic/internal/backend" "github.com/restic/restic/internal/backend/layout" + "github.com/restic/restic/internal/backend/location" "github.com/restic/restic/internal/debug" "github.com/restic/restic/internal/errors" "github.com/restic/restic/internal/restic" @@ -29,6 +30,10 @@ type Backend struct { layout.Layout } +func NewFactory() location.Factory { + return location.NewHTTPBackendFactory(ParseConfig, StripPassword, Create, Open) +} + // the REST API protocol version is decided by HTTP request headers, these are the constants. const ( ContentTypeV1 = "application/vnd.x.restic.rest.v1" diff --git a/internal/backend/s3/s3.go b/internal/backend/s3/s3.go index 7b7a761ce..dd5cc36e6 100644 --- a/internal/backend/s3/s3.go +++ b/internal/backend/s3/s3.go @@ -13,6 +13,7 @@ import ( "github.com/restic/restic/internal/backend" "github.com/restic/restic/internal/backend/layout" + "github.com/restic/restic/internal/backend/location" "github.com/restic/restic/internal/debug" "github.com/restic/restic/internal/errors" "github.com/restic/restic/internal/restic" @@ -31,6 +32,10 @@ type Backend struct { // make sure that *Backend implements backend.Backend var _ restic.Backend = &Backend{} +func NewFactory() location.Factory { + return location.NewHTTPBackendFactory(ParseConfig, location.NoPassword, Create, Open) +} + const defaultLayout = "default" func open(ctx context.Context, cfg Config, rt http.RoundTripper) (*Backend, error) { diff --git a/internal/backend/sftp/sftp.go b/internal/backend/sftp/sftp.go index 12c355003..f0a7ef9bc 100644 --- a/internal/backend/sftp/sftp.go +++ b/internal/backend/sftp/sftp.go @@ -15,6 +15,8 @@ import ( "github.com/restic/restic/internal/backend" "github.com/restic/restic/internal/backend/layout" + "github.com/restic/restic/internal/backend/limiter" + "github.com/restic/restic/internal/backend/location" "github.com/restic/restic/internal/debug" "github.com/restic/restic/internal/errors" "github.com/restic/restic/internal/restic" @@ -41,6 +43,14 @@ type SFTP struct { var _ restic.Backend = &SFTP{} +func NewFactory() location.Factory { + return location.NewLimitedBackendFactory(ParseConfig, location.NoPassword, func(ctx context.Context, cfg Config, _ limiter.Limiter) (*SFTP, error) { + return Create(ctx, cfg) + }, func(ctx context.Context, cfg Config, _ limiter.Limiter) (*SFTP, error) { + return Open(ctx, cfg) + }) +} + const defaultLayout = "default" func startClient(cfg Config) (*SFTP, error) { diff --git a/internal/backend/swift/swift.go b/internal/backend/swift/swift.go index cfa9ed665..019456be7 100644 --- a/internal/backend/swift/swift.go +++ b/internal/backend/swift/swift.go @@ -15,6 +15,7 @@ import ( "github.com/restic/restic/internal/backend" "github.com/restic/restic/internal/backend/layout" + "github.com/restic/restic/internal/backend/location" "github.com/restic/restic/internal/debug" "github.com/restic/restic/internal/errors" "github.com/restic/restic/internal/restic" @@ -34,6 +35,10 @@ type beSwift struct { // ensure statically that *beSwift implements restic.Backend. var _ restic.Backend = &beSwift{} +func NewFactory() location.Factory { + return location.NewHTTPBackendFactory(ParseConfig, location.NoPassword, Open, Open) +} + // Open opens the swift backend at a container in region. The container is // created if it does not exist yet. func Open(ctx context.Context, cfg Config, rt http.RoundTripper) (restic.Backend, error) { From 9aa9e0d1ec6c8453406337756f211eaaeaffcf83 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 8 Jun 2023 13:06:25 +0200 Subject: [PATCH 03/14] local/sftp: move limiter setup into backend --- cmd/restic/global.go | 5 ----- internal/backend/limiter/limiter_backend.go | 15 +++++++++++++++ internal/backend/local/local.go | 6 +----- internal/backend/sftp/sftp.go | 6 +----- 4 files changed, 17 insertions(+), 15 deletions(-) diff --git a/cmd/restic/global.go b/cmd/restic/global.go index f4886ec90..f08a75b13 100644 --- a/cmd/restic/global.go +++ b/cmd/restic/global.go @@ -610,11 +610,6 @@ func open(ctx context.Context, s string, gopts GlobalOptions, opts options.Optio } } - if loc.Scheme == "local" || loc.Scheme == "sftp" { - // wrap the backend in a LimitBackend so that the throughput is limited - be = limiter.LimitBackend(be, lim) - } - // check if config is there fi, err := be.Stat(ctx, restic.Handle{Type: restic.ConfigFile}) if err != nil { diff --git a/internal/backend/limiter/limiter_backend.go b/internal/backend/limiter/limiter_backend.go index 7fcca59cc..a91794037 100644 --- a/internal/backend/limiter/limiter_backend.go +++ b/internal/backend/limiter/limiter_backend.go @@ -7,6 +7,21 @@ import ( "github.com/restic/restic/internal/restic" ) +func WrapBackendConstructor[B restic.Backend, C any](constructor func(ctx context.Context, cfg C) (B, error)) func(ctx context.Context, cfg C, lim Limiter) (restic.Backend, error) { + return func(ctx context.Context, cfg C, lim Limiter) (restic.Backend, error) { + var be restic.Backend + be, err := constructor(ctx, cfg) + if err != nil { + return nil, err + } + + if lim != nil { + be = LimitBackend(be, lim) + } + return be, nil + } +} + // LimitBackend wraps a Backend and applies rate limiting to Load() and Save() // calls on the backend. func LimitBackend(be restic.Backend, l Limiter) restic.Backend { diff --git a/internal/backend/local/local.go b/internal/backend/local/local.go index 02ac81b8d..e9d00abf7 100644 --- a/internal/backend/local/local.go +++ b/internal/backend/local/local.go @@ -31,11 +31,7 @@ type Local struct { var _ restic.Backend = &Local{} func NewFactory() location.Factory { - return location.NewLimitedBackendFactory(ParseConfig, location.NoPassword, func(ctx context.Context, cfg Config, _ limiter.Limiter) (*Local, error) { - return Create(ctx, cfg) - }, func(ctx context.Context, cfg Config, _ limiter.Limiter) (*Local, error) { - return Open(ctx, cfg) - }) + return location.NewLimitedBackendFactory(ParseConfig, location.NoPassword, limiter.WrapBackendConstructor(Create), limiter.WrapBackendConstructor(Open)) } const defaultLayout = "default" diff --git a/internal/backend/sftp/sftp.go b/internal/backend/sftp/sftp.go index f0a7ef9bc..1e12df808 100644 --- a/internal/backend/sftp/sftp.go +++ b/internal/backend/sftp/sftp.go @@ -44,11 +44,7 @@ type SFTP struct { var _ restic.Backend = &SFTP{} func NewFactory() location.Factory { - return location.NewLimitedBackendFactory(ParseConfig, location.NoPassword, func(ctx context.Context, cfg Config, _ limiter.Limiter) (*SFTP, error) { - return Create(ctx, cfg) - }, func(ctx context.Context, cfg Config, _ limiter.Limiter) (*SFTP, error) { - return Open(ctx, cfg) - }) + return location.NewLimitedBackendFactory(ParseConfig, location.NoPassword, limiter.WrapBackendConstructor(Create), limiter.WrapBackendConstructor(Open)) } const defaultLayout = "default" From 555be49a7928c568877a4a60e5d92c3b701cb10f Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 8 Jun 2023 15:05:07 +0200 Subject: [PATCH 04/14] location: Make ParseConfig-test backend agnostic The backend specific parts of the test are now directly handled by the respective backend. Duplicate tests were removed. --- internal/backend/local/config_test.go | 28 ++ internal/backend/location/location_test.go | 363 +++------------------ internal/backend/s3/config_test.go | 18 + 3 files changed, 87 insertions(+), 322 deletions(-) diff --git a/internal/backend/local/config_test.go b/internal/backend/local/config_test.go index c9b6be61c..4c2ebc7bc 100644 --- a/internal/backend/local/config_test.go +++ b/internal/backend/local/config_test.go @@ -11,6 +11,34 @@ var configTests = []test.ConfigTestData[Config]{ Path: "/some/path", Connections: 2, }}, + {S: "local:dir1/dir2", Cfg: Config{ + Path: "dir1/dir2", + Connections: 2, + }}, + {S: "local:../dir1/dir2", Cfg: Config{ + Path: "../dir1/dir2", + Connections: 2, + }}, + {S: "local:/dir1:foobar/dir2", Cfg: Config{ + Path: "/dir1:foobar/dir2", + Connections: 2, + }}, + {S: `local:\dir1\foobar\dir2`, Cfg: Config{ + Path: `\dir1\foobar\dir2`, + Connections: 2, + }}, + {S: `local:c:\dir1\foobar\dir2`, Cfg: Config{ + Path: `c:\dir1\foobar\dir2`, + Connections: 2, + }}, + {S: `local:C:\Users\appveyor\AppData\Local\Temp\1\restic-test-879453535\repo`, Cfg: Config{ + Path: `C:\Users\appveyor\AppData\Local\Temp\1\restic-test-879453535\repo`, + Connections: 2, + }}, + {S: `local:c:/dir1/foobar/dir2`, Cfg: Config{ + Path: `c:/dir1/foobar/dir2`, + Connections: 2, + }}, } func TestParseConfig(t *testing.T) { diff --git a/internal/backend/location/location_test.go b/internal/backend/location/location_test.go index 6e9042200..933f2fc08 100644 --- a/internal/backend/location/location_test.go +++ b/internal/backend/location/location_test.go @@ -1,345 +1,64 @@ package location_test import ( - "net/url" - "reflect" "testing" - "github.com/restic/restic/internal/backend/b2" - "github.com/restic/restic/internal/backend/local" "github.com/restic/restic/internal/backend/location" - "github.com/restic/restic/internal/backend/rest" - "github.com/restic/restic/internal/backend/s3" - "github.com/restic/restic/internal/backend/sftp" - "github.com/restic/restic/internal/backend/swift" + "github.com/restic/restic/internal/restic" + "github.com/restic/restic/internal/test" ) -func parseURL(s string) *url.URL { - u, err := url.Parse(s) - if err != nil { - panic(err) - } - - return u +type testConfig struct { + loc string } -var parseTests = []struct { - s string - u location.Location -}{ - { - "local:/srv/repo", - location.Location{Scheme: "local", - Config: &local.Config{ - Path: "/srv/repo", - Connections: 2, - }, - }, - }, - { - "local:dir1/dir2", - location.Location{Scheme: "local", - Config: &local.Config{ - Path: "dir1/dir2", - Connections: 2, - }, - }, - }, - { - "local:dir1/dir2", - location.Location{Scheme: "local", - Config: &local.Config{ - Path: "dir1/dir2", - Connections: 2, - }, - }, - }, - { - "dir1/dir2", - location.Location{Scheme: "local", - Config: &local.Config{ - Path: "dir1/dir2", - Connections: 2, - }, - }, - }, - { - "/dir1/dir2", - location.Location{Scheme: "local", - Config: &local.Config{ - Path: "/dir1/dir2", - Connections: 2, - }, - }, - }, - { - "local:../dir1/dir2", - location.Location{Scheme: "local", - Config: &local.Config{ - Path: "../dir1/dir2", - Connections: 2, - }, - }, - }, - { - "/dir1/dir2", - location.Location{Scheme: "local", - Config: &local.Config{ - Path: "/dir1/dir2", - Connections: 2, - }, - }, - }, - { - "/dir1:foobar/dir2", - location.Location{Scheme: "local", - Config: &local.Config{ - Path: "/dir1:foobar/dir2", - Connections: 2, - }, - }, - }, - { - `\dir1\foobar\dir2`, - location.Location{Scheme: "local", - Config: &local.Config{ - Path: `\dir1\foobar\dir2`, - Connections: 2, - }, - }, - }, - { - `c:\dir1\foobar\dir2`, - location.Location{Scheme: "local", - Config: &local.Config{ - Path: `c:\dir1\foobar\dir2`, - Connections: 2, - }, - }, - }, - { - `C:\Users\appveyor\AppData\Local\Temp\1\restic-test-879453535\repo`, - location.Location{Scheme: "local", - Config: &local.Config{ - Path: `C:\Users\appveyor\AppData\Local\Temp\1\restic-test-879453535\repo`, - Connections: 2, - }, - }, - }, - { - `c:/dir1/foobar/dir2`, - location.Location{Scheme: "local", - Config: &local.Config{ - Path: `c:/dir1/foobar/dir2`, - Connections: 2, - }, - }, - }, - { - "sftp:user@host:/srv/repo", - location.Location{Scheme: "sftp", - Config: &sftp.Config{ - User: "user", - Host: "host", - Path: "/srv/repo", - Connections: 5, - }, - }, - }, - { - "sftp:host:/srv/repo", - location.Location{Scheme: "sftp", - Config: &sftp.Config{ - User: "", - Host: "host", - Path: "/srv/repo", - Connections: 5, - }, - }, - }, - { - "sftp://user@host/srv/repo", - location.Location{Scheme: "sftp", - Config: &sftp.Config{ - User: "user", - Host: "host", - Path: "srv/repo", - Connections: 5, - }, - }, - }, - { - "sftp://user@host//srv/repo", - location.Location{Scheme: "sftp", - Config: &sftp.Config{ - User: "user", - Host: "host", - Path: "/srv/repo", - Connections: 5, - }, - }, - }, - - { - "s3://eu-central-1/bucketname", - location.Location{Scheme: "s3", - Config: &s3.Config{ - Endpoint: "eu-central-1", - Bucket: "bucketname", - Prefix: "", - Connections: 5, - }, - }, - }, - { - "s3://hostname.foo/bucketname", - location.Location{Scheme: "s3", - Config: &s3.Config{ - Endpoint: "hostname.foo", - Bucket: "bucketname", - Prefix: "", - Connections: 5, - }, - }, - }, - { - "s3://hostname.foo/bucketname/prefix/directory", - location.Location{Scheme: "s3", - Config: &s3.Config{ - Endpoint: "hostname.foo", - Bucket: "bucketname", - Prefix: "prefix/directory", - Connections: 5, - }, - }, - }, - { - "s3:eu-central-1/repo", - location.Location{Scheme: "s3", - Config: &s3.Config{ - Endpoint: "eu-central-1", - Bucket: "repo", - Prefix: "", - Connections: 5, - }, - }, - }, - { - "s3:eu-central-1/repo/prefix/directory", - location.Location{Scheme: "s3", - Config: &s3.Config{ - Endpoint: "eu-central-1", - Bucket: "repo", - Prefix: "prefix/directory", - Connections: 5, - }, - }, - }, - { - "s3:https://hostname.foo/repo", - location.Location{Scheme: "s3", - Config: &s3.Config{ - Endpoint: "hostname.foo", - Bucket: "repo", - Prefix: "", - Connections: 5, - }, - }, - }, - { - "s3:https://hostname.foo/repo/prefix/directory", - location.Location{Scheme: "s3", - Config: &s3.Config{ - Endpoint: "hostname.foo", - Bucket: "repo", - Prefix: "prefix/directory", - Connections: 5, - }, - }, - }, - { - "s3:http://hostname.foo/repo", - location.Location{Scheme: "s3", - Config: &s3.Config{ - Endpoint: "hostname.foo", - Bucket: "repo", - Prefix: "", - UseHTTP: true, - Connections: 5, - }, - }, - }, - { - "swift:container17:/", - location.Location{Scheme: "swift", - Config: &swift.Config{ - Container: "container17", - Prefix: "", - Connections: 5, - }, - }, - }, - { - "swift:container17:/prefix97", - location.Location{Scheme: "swift", - Config: &swift.Config{ - Container: "container17", - Prefix: "prefix97", - Connections: 5, - }, - }, - }, - { - "rest:http://hostname.foo:1234/", - location.Location{Scheme: "rest", - Config: &rest.Config{ - URL: parseURL("http://hostname.foo:1234/"), - Connections: 5, - }, - }, - }, - { - "b2:bucketname:/prefix", location.Location{Scheme: "b2", - Config: &b2.Config{ - Bucket: "bucketname", - Prefix: "prefix", - Connections: 5, - }, - }, - }, - { - "b2:bucketname", location.Location{Scheme: "b2", - Config: &b2.Config{ - Bucket: "bucketname", - Prefix: "", - Connections: 5, - }, - }, - }, +func testFactory() location.Factory { + return location.NewHTTPBackendFactory[testConfig, restic.Backend]( + func(s string) (*testConfig, error) { + return &testConfig{loc: s}, nil + }, nil, nil, nil, + ) } func TestParse(t *testing.T) { - for i, test := range parseTests { - t.Run(test.s, func(t *testing.T) { - u, err := location.Parse(test.s) + registry := location.NewRegistry() + registry.Register("test", testFactory()) + + path := "test:example" + u, err := location.Parse(registry, path) + test.OK(t, err) + test.Equals(t, "test", u.Scheme) + test.Equals(t, &testConfig{loc: path}, u.Config) +} + +func TestParseFallback(t *testing.T) { + fallbackTests := []string{ + "dir1/dir2", + "/dir1/dir2", + "/dir1:foobar/dir2", + `\dir1\foobar\dir2`, + `c:\dir1\foobar\dir2`, + `C:\Users\appveyor\AppData\Local\Temp\1\restic-test-879453535\repo`, + `c:/dir1/foobar/dir2`, + } + + registry := location.NewRegistry() + registry.Register("local", testFactory()) + + for _, path := range fallbackTests { + t.Run(path, func(t *testing.T) { + u, err := location.Parse(registry, path) if err != nil { t.Fatalf("unexpected error: %v", err) } - - if test.u.Scheme != u.Scheme { - t.Errorf("test %d: scheme does not match, want %q, got %q", - i, test.u.Scheme, u.Scheme) - } - - if !reflect.DeepEqual(test.u.Config, u.Config) { - t.Errorf("test %d: cfg map does not match, want:\n %#v\ngot: \n %#v", - i, test.u.Config, u.Config) - } + test.Equals(t, "local", u.Scheme) + test.Equals(t, "local:"+path, u.Config.(*testConfig).loc) }) } } func TestInvalidScheme(t *testing.T) { + registry := location.NewRegistry() var invalidSchemes = []string{ "foobar:xxx", "foobar:/dir/dir2", @@ -347,7 +66,7 @@ func TestInvalidScheme(t *testing.T) { for _, s := range invalidSchemes { t.Run(s, func(t *testing.T) { - _, err := location.Parse(s) + _, err := location.Parse(registry, s) if err == nil { t.Fatalf("error for invalid location %q not found", s) } diff --git a/internal/backend/s3/config_test.go b/internal/backend/s3/config_test.go index 21fbb27b9..085dbeedb 100644 --- a/internal/backend/s3/config_test.go +++ b/internal/backend/s3/config_test.go @@ -56,6 +56,24 @@ var configTests = []test.ConfigTestData[Config]{ Prefix: "prefix/directory", Connections: 5, }}, + {S: "s3:hostname.foo/foobar", Cfg: Config{ + Endpoint: "hostname.foo", + Bucket: "foobar", + Prefix: "", + Connections: 5, + }}, + {S: "s3:hostname.foo/foobar/prefix/directory", Cfg: Config{ + Endpoint: "hostname.foo", + Bucket: "foobar", + Prefix: "prefix/directory", + Connections: 5, + }}, + {S: "s3:https://hostname/foobar", Cfg: Config{ + Endpoint: "hostname", + Bucket: "foobar", + Prefix: "", + Connections: 5, + }}, {S: "s3:https://hostname:9999/foobar", Cfg: Config{ Endpoint: "hostname:9999", Bucket: "foobar", From 3325a7c8623ecf9fd39416baf61d9fbe2d00b051 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 8 Jun 2023 15:17:00 +0200 Subject: [PATCH 05/14] location: extract backend specific part of StripPassword The tests for the rest backend now reside there. --- .../backend/location/display_location_test.go | 115 ++++-------------- internal/backend/rest/config_test.go | 68 +++++++++++ 2 files changed, 92 insertions(+), 91 deletions(-) diff --git a/internal/backend/location/display_location_test.go b/internal/backend/location/display_location_test.go index 30d3cc286..4a4055a84 100644 --- a/internal/backend/location/display_location_test.go +++ b/internal/backend/location/display_location_test.go @@ -1,96 +1,29 @@ -package location +package location_test -import "testing" +import ( + "testing" -var passwordTests = []struct { - input string - expected string -}{ - { - "local:/srv/repo", - "local:/srv/repo", - }, - { - "/dir1/dir2", - "/dir1/dir2", - }, - { - `c:\dir1\foobar\dir2`, - `c:\dir1\foobar\dir2`, - }, - { - "sftp:user@host:/srv/repo", - "sftp:user@host:/srv/repo", - }, - { - "s3://eu-central-1/bucketname", - "s3://eu-central-1/bucketname", - }, - { - "swift:container17:/prefix97", - "swift:container17:/prefix97", - }, - { - "b2:bucketname:/prefix", - "b2:bucketname:/prefix", - }, - { - "rest:", - "rest:/", - }, - { - "rest:localhost/", - "rest:localhost/", - }, - { - "rest::123/", - "rest::123/", - }, - { - "rest:http://", - "rest:http://", - }, - { - "rest:http://hostname.foo:1234/", - "rest:http://hostname.foo:1234/", - }, - { - "rest:http://user@hostname.foo:1234/", - "rest:http://user@hostname.foo:1234/", - }, - { - "rest:http://user:@hostname.foo:1234/", - "rest:http://user:***@hostname.foo:1234/", - }, - { - "rest:http://user:p@hostname.foo:1234/", - "rest:http://user:***@hostname.foo:1234/", - }, - { - "rest:http://user:pppppaaafhhfuuwiiehhthhghhdkjaoowpprooghjjjdhhwuuhgjsjhhfdjhruuhsjsdhhfhshhsppwufhhsjjsjs@hostname.foo:1234/", - "rest:http://user:***@hostname.foo:1234/", - }, - { - "rest:http://user:password@hostname", - "rest:http://user:***@hostname/", - }, - { - "rest:http://user:password@:123", - "rest:http://user:***@:123/", - }, - { - "rest:http://user:password@", - "rest:http://user:***@/", - }, -} + "github.com/restic/restic/internal/backend/location" + "github.com/restic/restic/internal/restic" + "github.com/restic/restic/internal/test" +) func TestStripPassword(t *testing.T) { - for i, test := range passwordTests { - t.Run(test.input, func(t *testing.T) { - result := StripPassword(test.input) - if result != test.expected { - t.Errorf("test %d: expected '%s' but got '%s'", i, test.expected, result) - } - }) - } + registry := location.NewRegistry() + registry.Register("test", + location.NewHTTPBackendFactory[any, restic.Backend](nil, + func(s string) string { + return "cleaned" + }, nil, nil, + ), + ) + + t.Run("valid", func(t *testing.T) { + clean := location.StripPassword(registry, "test:secret") + test.Equals(t, "cleaned", clean) + }) + t.Run("unknown", func(t *testing.T) { + clean := location.StripPassword(registry, "invalid:secret") + test.Equals(t, "invalid:secret", clean) + }) } diff --git a/internal/backend/rest/config_test.go b/internal/backend/rest/config_test.go index 8cfc78407..23ea9095b 100644 --- a/internal/backend/rest/config_test.go +++ b/internal/backend/rest/config_test.go @@ -36,3 +36,71 @@ var configTests = []test.ConfigTestData[Config]{ func TestParseConfig(t *testing.T) { test.ParseConfigTester(t, ParseConfig, configTests) } + +var passwordTests = []struct { + input string + expected string +}{ + { + "rest:", + "rest:/", + }, + { + "rest:localhost/", + "rest:localhost/", + }, + { + "rest::123/", + "rest::123/", + }, + { + "rest:http://", + "rest:http://", + }, + { + "rest:http://hostname.foo:1234/", + "rest:http://hostname.foo:1234/", + }, + { + "rest:http://user@hostname.foo:1234/", + "rest:http://user@hostname.foo:1234/", + }, + { + "rest:http://user:@hostname.foo:1234/", + "rest:http://user:***@hostname.foo:1234/", + }, + { + "rest:http://user:p@hostname.foo:1234/", + "rest:http://user:***@hostname.foo:1234/", + }, + { + "rest:http://user:pppppaaafhhfuuwiiehhthhghhdkjaoowpprooghjjjdhhwuuhgjsjhhfdjhruuhsjsdhhfhshhsppwufhhsjjsjs@hostname.foo:1234/", + "rest:http://user:***@hostname.foo:1234/", + }, + { + "rest:http://user:password@hostname", + "rest:http://user:***@hostname/", + }, + { + "rest:http://user:password@:123", + "rest:http://user:***@:123/", + }, + { + "rest:http://user:password@", + "rest:http://user:***@/", + }, +} + +func TestStripPassword(t *testing.T) { + // Make sure that the factory uses the correct method + StripPassword := NewFactory().StripPassword + + for i, test := range passwordTests { + t.Run(test.input, func(t *testing.T) { + result := StripPassword(test.input) + if result != test.expected { + t.Errorf("test %d: expected '%s' but got '%s'", i, test.expected, result) + } + }) + } +} From 19ac12d95b5226d75a484378be87ace1a093c812 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 8 Jun 2023 15:18:43 +0200 Subject: [PATCH 06/14] location: make genericBackendFactory private --- internal/backend/location/registry.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/internal/backend/location/registry.go b/internal/backend/location/registry.go index 5d644dfb9..f15095590 100644 --- a/internal/backend/location/registry.go +++ b/internal/backend/location/registry.go @@ -36,35 +36,35 @@ type Factory interface { Open(ctx context.Context, cfg interface{}, rt http.RoundTripper, lim limiter.Limiter) (restic.Backend, error) } -type GenericBackendFactory[C any, T restic.Backend] struct { +type genericBackendFactory[C any, T restic.Backend] struct { parseConfigFn func(s string) (*C, error) stripPasswordFn func(s string) string createFn func(ctx context.Context, cfg C, rt http.RoundTripper, lim limiter.Limiter) (T, error) openFn func(ctx context.Context, cfg C, rt http.RoundTripper, lim limiter.Limiter) (T, error) } -func (f *GenericBackendFactory[C, T]) ParseConfig(s string) (interface{}, error) { +func (f *genericBackendFactory[C, T]) ParseConfig(s string) (interface{}, error) { return f.parseConfigFn(s) } -func (f *GenericBackendFactory[C, T]) StripPassword(s string) string { +func (f *genericBackendFactory[C, T]) StripPassword(s string) string { if f.stripPasswordFn != nil { return f.stripPasswordFn(s) } return s } -func (f *GenericBackendFactory[C, T]) Create(ctx context.Context, cfg interface{}, rt http.RoundTripper, lim limiter.Limiter) (restic.Backend, error) { +func (f *genericBackendFactory[C, T]) Create(ctx context.Context, cfg interface{}, rt http.RoundTripper, lim limiter.Limiter) (restic.Backend, error) { return f.createFn(ctx, *cfg.(*C), rt, lim) } -func (f *GenericBackendFactory[C, T]) Open(ctx context.Context, cfg interface{}, rt http.RoundTripper, lim limiter.Limiter) (restic.Backend, error) { +func (f *genericBackendFactory[C, T]) Open(ctx context.Context, cfg interface{}, rt http.RoundTripper, lim limiter.Limiter) (restic.Backend, error) { return f.openFn(ctx, *cfg.(*C), rt, lim) } func NewHTTPBackendFactory[C any, T restic.Backend](parseConfigFn func(s string) (*C, error), stripPasswordFn func(s string) string, createFn func(ctx context.Context, cfg C, rt http.RoundTripper) (T, error), - openFn func(ctx context.Context, cfg C, rt http.RoundTripper) (T, error)) *GenericBackendFactory[C, T] { + openFn func(ctx context.Context, cfg C, rt http.RoundTripper) (T, error)) Factory { - return &GenericBackendFactory[C, T]{ + return &genericBackendFactory[C, T]{ parseConfigFn: parseConfigFn, stripPasswordFn: stripPasswordFn, createFn: func(ctx context.Context, cfg C, rt http.RoundTripper, _ limiter.Limiter) (T, error) { @@ -79,9 +79,9 @@ func NewHTTPBackendFactory[C any, T restic.Backend](parseConfigFn func(s string) func NewLimitedBackendFactory[C any, T restic.Backend](parseConfigFn func(s string) (*C, error), stripPasswordFn func(s string) string, createFn func(ctx context.Context, cfg C, lim limiter.Limiter) (T, error), - openFn func(ctx context.Context, cfg C, lim limiter.Limiter) (T, error)) *GenericBackendFactory[C, T] { + openFn func(ctx context.Context, cfg C, lim limiter.Limiter) (T, error)) Factory { - return &GenericBackendFactory[C, T]{ + return &genericBackendFactory[C, T]{ parseConfigFn: parseConfigFn, stripPasswordFn: stripPasswordFn, createFn: func(ctx context.Context, cfg C, _ http.RoundTripper, lim limiter.Limiter) (T, error) { From 3a3cf608f5bbf59b0a72e9796141dba09032cdec Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 8 Jun 2023 15:28:07 +0200 Subject: [PATCH 07/14] b2/s3: Move config validation from ApplyEnvironment to Open/Create Conceptually the backend configuration should be validated when creating or opening the backend, but not when filling in information from environment variables into the configuration. --- cmd/restic/global.go | 4 +--- internal/backend/azure/azure_test.go | 6 +----- internal/backend/azure/config.go | 3 +-- internal/backend/b2/b2.go | 7 +++++++ internal/backend/b2/b2_test.go | 6 +----- internal/backend/b2/config.go | 12 +----------- internal/backend/gs/config.go | 3 +-- internal/backend/s3/config.go | 12 +----------- internal/backend/s3/s3.go | 6 ++++++ internal/backend/swift/config.go | 3 +-- internal/backend/swift/swift_test.go | 4 +--- internal/restic/backend.go | 2 +- 12 files changed, 23 insertions(+), 45 deletions(-) diff --git a/cmd/restic/global.go b/cmd/restic/global.go index f08a75b13..1c13fb887 100644 --- a/cmd/restic/global.go +++ b/cmd/restic/global.go @@ -550,9 +550,7 @@ func OpenRepository(ctx context.Context, opts GlobalOptions) (*repository.Reposi func parseConfig(loc location.Location, opts options.Options) (interface{}, error) { cfg := loc.Config if cfg, ok := cfg.(restic.ApplyEnvironmenter); ok { - if err := cfg.ApplyEnvironment(""); err != nil { - return nil, err - } + cfg.ApplyEnvironment("") } // only apply options for a particular backend here diff --git a/internal/backend/azure/azure_test.go b/internal/backend/azure/azure_test.go index 0fab5da26..5aee96fbd 100644 --- a/internal/backend/azure/azure_test.go +++ b/internal/backend/azure/azure_test.go @@ -35,11 +35,7 @@ func newAzureTestSuite(t testing.TB) *test.Suite[azure.Config] { return nil, err } - err = cfg.ApplyEnvironment("RESTIC_TEST_") - if err != nil { - return nil, err - } - + cfg.ApplyEnvironment("RESTIC_TEST_") cfg.Prefix = fmt.Sprintf("test-%d", time.Now().UnixNano()) return cfg, nil }, diff --git a/internal/backend/azure/config.go b/internal/backend/azure/config.go index 4d4e839ff..6786ec626 100644 --- a/internal/backend/azure/config.go +++ b/internal/backend/azure/config.go @@ -59,7 +59,7 @@ func ParseConfig(s string) (*Config, error) { var _ restic.ApplyEnvironmenter = &Config{} // ApplyEnvironment saves values from the environment to the config. -func (cfg *Config) ApplyEnvironment(prefix string) error { +func (cfg *Config) ApplyEnvironment(prefix string) { if cfg.AccountName == "" { cfg.AccountName = os.Getenv(prefix + "AZURE_ACCOUNT_NAME") } @@ -71,5 +71,4 @@ func (cfg *Config) ApplyEnvironment(prefix string) error { if cfg.AccountSAS.String() == "" { cfg.AccountSAS = options.NewSecretString(os.Getenv(prefix + "AZURE_ACCOUNT_SAS")) } - return nil } diff --git a/internal/backend/b2/b2.go b/internal/backend/b2/b2.go index eb2cfe3c2..3560cca49 100644 --- a/internal/backend/b2/b2.go +++ b/internal/backend/b2/b2.go @@ -58,6 +58,13 @@ func (s *sniffingRoundTripper) RoundTrip(req *http.Request) (*http.Response, err } func newClient(ctx context.Context, cfg Config, rt http.RoundTripper) (*b2.Client, error) { + if cfg.AccountID == "" { + return nil, errors.Fatalf("unable to open B2 backend: Account ID ($B2_ACCOUNT_ID) is empty") + } + if cfg.Key.String() == "" { + return nil, errors.Fatalf("unable to open B2 backend: Key ($B2_ACCOUNT_KEY) is empty") + } + sniffer := &sniffingRoundTripper{RoundTripper: rt} opts := []b2.ClientOption{b2.Transport(sniffer)} diff --git a/internal/backend/b2/b2_test.go b/internal/backend/b2/b2_test.go index 8e982adda..3a649db1e 100644 --- a/internal/backend/b2/b2_test.go +++ b/internal/backend/b2/b2_test.go @@ -35,11 +35,7 @@ func newB2TestSuite(t testing.TB) *test.Suite[b2.Config] { return nil, err } - err = cfg.ApplyEnvironment("RESTIC_TEST_") - if err != nil { - return nil, err - } - + cfg.ApplyEnvironment("RESTIC_TEST_") cfg.Prefix = fmt.Sprintf("test-%d", time.Now().UnixNano()) return cfg, nil }, diff --git a/internal/backend/b2/config.go b/internal/backend/b2/config.go index 548fbef99..94614e44f 100644 --- a/internal/backend/b2/config.go +++ b/internal/backend/b2/config.go @@ -85,21 +85,11 @@ func ParseConfig(s string) (*Config, error) { var _ restic.ApplyEnvironmenter = &Config{} // ApplyEnvironment saves values from the environment to the config. -func (cfg *Config) ApplyEnvironment(prefix string) error { +func (cfg *Config) ApplyEnvironment(prefix string) { if cfg.AccountID == "" { cfg.AccountID = os.Getenv(prefix + "B2_ACCOUNT_ID") } - - if cfg.AccountID == "" { - return errors.Fatalf("unable to open B2 backend: Account ID ($B2_ACCOUNT_ID) is empty") - } - if cfg.Key.String() == "" { cfg.Key = options.NewSecretString(os.Getenv(prefix + "B2_ACCOUNT_KEY")) } - - if cfg.Key.String() == "" { - return errors.Fatalf("unable to open B2 backend: Key ($B2_ACCOUNT_KEY) is empty") - } - return nil } diff --git a/internal/backend/gs/config.go b/internal/backend/gs/config.go index b2d52c5f8..61a31113f 100644 --- a/internal/backend/gs/config.go +++ b/internal/backend/gs/config.go @@ -62,9 +62,8 @@ func ParseConfig(s string) (*Config, error) { var _ restic.ApplyEnvironmenter = &Config{} // ApplyEnvironment saves values from the environment to the config. -func (cfg *Config) ApplyEnvironment(prefix string) error { +func (cfg *Config) ApplyEnvironment(prefix string) { if cfg.ProjectID == "" { cfg.ProjectID = os.Getenv(prefix + "GOOGLE_PROJECT_ID") } - return nil } diff --git a/internal/backend/s3/config.go b/internal/backend/s3/config.go index 525373d16..8dcad9eee 100644 --- a/internal/backend/s3/config.go +++ b/internal/backend/s3/config.go @@ -97,24 +97,14 @@ func createConfig(endpoint, bucket, prefix string, useHTTP bool) (*Config, error var _ restic.ApplyEnvironmenter = &Config{} // ApplyEnvironment saves values from the environment to the config. -func (cfg *Config) ApplyEnvironment(prefix string) error { +func (cfg *Config) ApplyEnvironment(prefix string) { if cfg.KeyID == "" { cfg.KeyID = os.Getenv(prefix + "AWS_ACCESS_KEY_ID") } - if cfg.Secret.String() == "" { cfg.Secret = options.NewSecretString(os.Getenv(prefix + "AWS_SECRET_ACCESS_KEY")) } - - if cfg.KeyID == "" && cfg.Secret.String() != "" { - return errors.Fatalf("unable to open S3 backend: Key ID ($AWS_ACCESS_KEY_ID) is empty") - } else if cfg.KeyID != "" && cfg.Secret.String() == "" { - return errors.Fatalf("unable to open S3 backend: Secret ($AWS_SECRET_ACCESS_KEY) is empty") - } - if cfg.Region == "" { cfg.Region = os.Getenv(prefix + "AWS_DEFAULT_REGION") } - - return nil } diff --git a/internal/backend/s3/s3.go b/internal/backend/s3/s3.go index dd5cc36e6..10512e809 100644 --- a/internal/backend/s3/s3.go +++ b/internal/backend/s3/s3.go @@ -41,6 +41,12 @@ const defaultLayout = "default" func open(ctx context.Context, cfg Config, rt http.RoundTripper) (*Backend, error) { debug.Log("open, config %#v", cfg) + if cfg.KeyID == "" && cfg.Secret.String() != "" { + return nil, errors.Fatalf("unable to open S3 backend: Key ID ($AWS_ACCESS_KEY_ID) is empty") + } else if cfg.KeyID != "" && cfg.Secret.String() == "" { + return nil, errors.Fatalf("unable to open S3 backend: Secret ($AWS_SECRET_ACCESS_KEY) is empty") + } + if cfg.MaxRetries > 0 { minio.MaxRetry = int(cfg.MaxRetries) } diff --git a/internal/backend/swift/config.go b/internal/backend/swift/config.go index b9f5d3995..5be2d9ce0 100644 --- a/internal/backend/swift/config.go +++ b/internal/backend/swift/config.go @@ -77,7 +77,7 @@ func ParseConfig(s string) (*Config, error) { var _ restic.ApplyEnvironmenter = &Config{} // ApplyEnvironment saves values from the environment to the config. -func (cfg *Config) ApplyEnvironment(prefix string) error { +func (cfg *Config) ApplyEnvironment(prefix string) { for _, val := range []struct { s *string env string @@ -130,5 +130,4 @@ func (cfg *Config) ApplyEnvironment(prefix string) error { *val.s = options.NewSecretString(os.Getenv(val.env)) } } - return nil } diff --git a/internal/backend/swift/swift_test.go b/internal/backend/swift/swift_test.go index cb0992010..52278943e 100644 --- a/internal/backend/swift/swift_test.go +++ b/internal/backend/swift/swift_test.go @@ -48,9 +48,7 @@ func newSwiftTestSuite(t testing.TB) *test.Suite[swift.Config] { return nil, err } - if err = cfg.ApplyEnvironment("RESTIC_TEST_"); err != nil { - return nil, err - } + cfg.ApplyEnvironment("RESTIC_TEST_") cfg.Prefix += fmt.Sprintf("/test-%d", time.Now().UnixNano()) t.Logf("using prefix %v", cfg.Prefix) return cfg, nil diff --git a/internal/restic/backend.go b/internal/restic/backend.go index b6653fcb4..555b9d96e 100644 --- a/internal/restic/backend.go +++ b/internal/restic/backend.go @@ -83,5 +83,5 @@ type FileInfo struct { // ApplyEnvironmenter fills in a backend configuration from the environment type ApplyEnvironmenter interface { - ApplyEnvironment(prefix string) error + ApplyEnvironment(prefix string) } From 3d3bb887453d84ef8561e5c8e97b17c9dc51a8e8 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 8 Jun 2023 16:21:59 +0200 Subject: [PATCH 08/14] b2: remove duplicate check for config file during repository creation No other backend implements that check. The check that a repository is not yet initialized is handled by the Repository later on. --- internal/backend/b2/b2.go | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/internal/backend/b2/b2.go b/internal/backend/b2/b2.go index 3560cca49..700fff099 100644 --- a/internal/backend/b2/b2.go +++ b/internal/backend/b2/b2.go @@ -147,16 +147,6 @@ func Create(ctx context.Context, cfg Config, rt http.RoundTripper) (restic.Backe }, listMaxItems: defaultListMaxItems, } - - _, err = be.Stat(ctx, restic.Handle{Type: restic.ConfigFile}) - if err != nil && !be.IsNotExist(err) { - return nil, err - } - - if err == nil { - return nil, errors.New("config already exists") - } - return be, nil } From 13a8b5822f1115ca11f6d15e47a3a6a61e839aa5 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 8 Jun 2023 16:53:55 +0200 Subject: [PATCH 09/14] backend: Adjust tests to use the Factory to instantiate the backend This drastically reduces the amount of duplicated test code. --- internal/backend/azure/azure_test.go | 43 +------ internal/backend/b2/b2_test.go | 28 +---- internal/backend/gs/gs_test.go | 43 +------ internal/backend/local/local_test.go | 27 +---- internal/backend/mem/mem_backend.go | 20 +++ internal/backend/mem/mem_backend_test.go | 48 +------- internal/backend/rclone/backend_test.go | 29 ++--- internal/backend/rest/rest_test.go | 22 +--- internal/backend/s3/s3_test.go | 147 +++++------------------ internal/backend/sftp/sftp_test.go | 28 +---- internal/backend/swift/swift_test.go | 42 +------ internal/backend/test/suite.go | 63 +++++++--- internal/backend/test/tests.go | 2 +- 13 files changed, 119 insertions(+), 423 deletions(-) diff --git a/internal/backend/azure/azure_test.go b/internal/backend/azure/azure_test.go index 5aee96fbd..8465cc3b0 100644 --- a/internal/backend/azure/azure_test.go +++ b/internal/backend/azure/azure_test.go @@ -12,18 +12,12 @@ import ( "github.com/restic/restic/internal/backend" "github.com/restic/restic/internal/backend/azure" "github.com/restic/restic/internal/backend/test" - "github.com/restic/restic/internal/errors" "github.com/restic/restic/internal/options" "github.com/restic/restic/internal/restic" rtest "github.com/restic/restic/internal/test" ) func newAzureTestSuite(t testing.TB) *test.Suite[azure.Config] { - tr, err := backend.Transport(backend.TransportOptions{}) - if err != nil { - t.Fatalf("cannot create transport for tests: %v", err) - } - return &test.Suite[azure.Config]{ // do not use excessive data MinimalData: true, @@ -40,42 +34,7 @@ func newAzureTestSuite(t testing.TB) *test.Suite[azure.Config] { return cfg, nil }, - // CreateFn is a function that creates a temporary repository for the tests. - Create: func(cfg azure.Config) (restic.Backend, error) { - ctx := context.TODO() - be, err := azure.Create(ctx, cfg, tr) - if err != nil { - return nil, err - } - - _, err = be.Stat(context.TODO(), restic.Handle{Type: restic.ConfigFile}) - if err != nil && !be.IsNotExist(err) { - return nil, err - } - - if err == nil { - return nil, errors.New("config already exists") - } - - return be, nil - }, - - // OpenFn is a function that opens a previously created temporary repository. - Open: func(cfg azure.Config) (restic.Backend, error) { - ctx := context.TODO() - return azure.Open(ctx, cfg, tr) - }, - - // CleanupFn removes data created during the tests. - Cleanup: func(cfg azure.Config) error { - ctx := context.TODO() - be, err := azure.Open(ctx, cfg, tr) - if err != nil { - return err - } - - return be.Delete(context.TODO()) - }, + Factory: azure.NewFactory(), } } diff --git a/internal/backend/b2/b2_test.go b/internal/backend/b2/b2_test.go index 3a649db1e..348af9095 100644 --- a/internal/backend/b2/b2_test.go +++ b/internal/backend/b2/b2_test.go @@ -1,26 +1,18 @@ package b2_test import ( - "context" "fmt" "os" "testing" "time" - "github.com/restic/restic/internal/backend" "github.com/restic/restic/internal/backend/b2" "github.com/restic/restic/internal/backend/test" - "github.com/restic/restic/internal/restic" rtest "github.com/restic/restic/internal/test" ) func newB2TestSuite(t testing.TB) *test.Suite[b2.Config] { - tr, err := backend.Transport(backend.TransportOptions{}) - if err != nil { - t.Fatalf("cannot create transport for tests: %v", err) - } - return &test.Suite[b2.Config]{ // do not use excessive data MinimalData: true, @@ -40,25 +32,7 @@ func newB2TestSuite(t testing.TB) *test.Suite[b2.Config] { return cfg, nil }, - // CreateFn is a function that creates a temporary repository for the tests. - Create: func(cfg b2.Config) (restic.Backend, error) { - return b2.Create(context.Background(), cfg, tr) - }, - - // OpenFn is a function that opens a previously created temporary repository. - Open: func(cfg b2.Config) (restic.Backend, error) { - return b2.Open(context.Background(), cfg, tr) - }, - - // CleanupFn removes data created during the tests. - Cleanup: func(cfg b2.Config) error { - be, err := b2.Open(context.Background(), cfg, tr) - if err != nil { - return err - } - - return be.Delete(context.TODO()) - }, + Factory: b2.NewFactory(), } } diff --git a/internal/backend/gs/gs_test.go b/internal/backend/gs/gs_test.go index a73fe77fb..47085dc4e 100644 --- a/internal/backend/gs/gs_test.go +++ b/internal/backend/gs/gs_test.go @@ -1,26 +1,17 @@ package gs_test import ( - "context" "fmt" "os" "testing" "time" - "github.com/restic/restic/internal/backend" "github.com/restic/restic/internal/backend/gs" "github.com/restic/restic/internal/backend/test" - "github.com/restic/restic/internal/errors" - "github.com/restic/restic/internal/restic" rtest "github.com/restic/restic/internal/test" ) func newGSTestSuite(t testing.TB) *test.Suite[gs.Config] { - tr, err := backend.Transport(backend.TransportOptions{}) - if err != nil { - t.Fatalf("cannot create transport for tests: %v", err) - } - return &test.Suite[gs.Config]{ // do not use excessive data MinimalData: true, @@ -37,39 +28,7 @@ func newGSTestSuite(t testing.TB) *test.Suite[gs.Config] { return cfg, nil }, - // CreateFn is a function that creates a temporary repository for the tests. - Create: func(cfg gs.Config) (restic.Backend, error) { - be, err := gs.Create(context.Background(), cfg, tr) - if err != nil { - return nil, err - } - - _, err = be.Stat(context.TODO(), restic.Handle{Type: restic.ConfigFile}) - if err != nil && !be.IsNotExist(err) { - return nil, err - } - - if err == nil { - return nil, errors.New("config already exists") - } - - return be, nil - }, - - // OpenFn is a function that opens a previously created temporary repository. - Open: func(cfg gs.Config) (restic.Backend, error) { - return gs.Open(context.TODO(), cfg, tr) - }, - - // CleanupFn removes data created during the tests. - Cleanup: func(cfg gs.Config) error { - be, err := gs.Open(context.TODO(), cfg, tr) - if err != nil { - return err - } - - return be.Delete(context.TODO()) - }, + Factory: gs.NewFactory(), } } diff --git a/internal/backend/local/local_test.go b/internal/backend/local/local_test.go index ca9e3b71b..2a8b626d4 100644 --- a/internal/backend/local/local_test.go +++ b/internal/backend/local/local_test.go @@ -8,7 +8,6 @@ import ( "github.com/restic/restic/internal/backend/local" "github.com/restic/restic/internal/backend/test" - "github.com/restic/restic/internal/restic" rtest "github.com/restic/restic/internal/test" ) @@ -16,11 +15,7 @@ func newTestSuite(t testing.TB) *test.Suite[local.Config] { return &test.Suite[local.Config]{ // NewConfig returns a config for a new temporary backend that will be used in tests. NewConfig: func() (*local.Config, error) { - dir, err := os.MkdirTemp(rtest.TestTempDir, "restic-test-local-") - if err != nil { - t.Fatal(err) - } - + dir := rtest.TempDir(t) t.Logf("create new backend at %v", dir) cfg := &local.Config{ @@ -30,25 +25,7 @@ func newTestSuite(t testing.TB) *test.Suite[local.Config] { return cfg, nil }, - // CreateFn is a function that creates a temporary repository for the tests. - Create: func(cfg local.Config) (restic.Backend, error) { - return local.Create(context.TODO(), cfg) - }, - - // OpenFn is a function that opens a previously created temporary repository. - Open: func(cfg local.Config) (restic.Backend, error) { - return local.Open(context.TODO(), cfg) - }, - - // CleanupFn removes data created during the tests. - Cleanup: func(cfg local.Config) error { - if !rtest.TestCleanupTempDirs { - t.Logf("leaving test backend dir at %v", cfg.Path) - } - - rtest.RemoveAll(t, cfg.Path) - return nil - }, + Factory: local.NewFactory(), } } diff --git a/internal/backend/mem/mem_backend.go b/internal/backend/mem/mem_backend.go index 618ef5752..a467d33f7 100644 --- a/internal/backend/mem/mem_backend.go +++ b/internal/backend/mem/mem_backend.go @@ -6,10 +6,12 @@ import ( "encoding/base64" "hash" "io" + "net/http" "sync" "github.com/cespare/xxhash/v2" "github.com/restic/restic/internal/backend" + "github.com/restic/restic/internal/backend/location" "github.com/restic/restic/internal/debug" "github.com/restic/restic/internal/errors" "github.com/restic/restic/internal/restic" @@ -20,6 +22,24 @@ type memMap map[restic.Handle][]byte // make sure that MemoryBackend implements backend.Backend var _ restic.Backend = &MemoryBackend{} +// NewFactory creates a persistent mem backend +func NewFactory() location.Factory { + be := New() + + return location.NewHTTPBackendFactory[struct{}, *MemoryBackend]( + func(s string) (*struct{}, error) { + return &struct{}{}, nil + }, + location.NoPassword, + func(_ context.Context, _ struct{}, _ http.RoundTripper) (*MemoryBackend, error) { + return be, nil + }, + func(_ context.Context, _ struct{}, _ http.RoundTripper) (*MemoryBackend, error) { + return be, nil + }, + ) +} + var errNotFound = errors.New("not found") const connectionCount = 2 diff --git a/internal/backend/mem/mem_backend_test.go b/internal/backend/mem/mem_backend_test.go index 3dea089bc..c4dad0fb2 100644 --- a/internal/backend/mem/mem_backend_test.go +++ b/internal/backend/mem/mem_backend_test.go @@ -1,58 +1,20 @@ package mem_test import ( - "context" "testing" - "github.com/restic/restic/internal/errors" - "github.com/restic/restic/internal/restic" - "github.com/restic/restic/internal/backend/mem" "github.com/restic/restic/internal/backend/test" ) -type memConfig struct { - be restic.Backend -} - -func newTestSuite() *test.Suite[*memConfig] { - return &test.Suite[*memConfig]{ +func newTestSuite() *test.Suite[struct{}] { + return &test.Suite[struct{}]{ // NewConfig returns a config for a new temporary backend that will be used in tests. - NewConfig: func() (**memConfig, error) { - cfg := &memConfig{} - return &cfg, nil + NewConfig: func() (*struct{}, error) { + return &struct{}{}, nil }, - // CreateFn is a function that creates a temporary repository for the tests. - Create: func(cfg *memConfig) (restic.Backend, error) { - if cfg.be != nil { - _, err := cfg.be.Stat(context.TODO(), restic.Handle{Type: restic.ConfigFile}) - if err != nil && !cfg.be.IsNotExist(err) { - return nil, err - } - - if err == nil { - return nil, errors.New("config already exists") - } - } - - cfg.be = mem.New() - return cfg.be, nil - }, - - // OpenFn is a function that opens a previously created temporary repository. - Open: func(cfg *memConfig) (restic.Backend, error) { - if cfg.be == nil { - cfg.be = mem.New() - } - return cfg.be, nil - }, - - // CleanupFn removes data created during the tests. - Cleanup: func(cfg *memConfig) error { - // no cleanup needed - return nil - }, + Factory: mem.NewFactory(), } } diff --git a/internal/backend/rclone/backend_test.go b/internal/backend/rclone/backend_test.go index 738462577..742031585 100644 --- a/internal/backend/rclone/backend_test.go +++ b/internal/backend/rclone/backend_test.go @@ -1,14 +1,11 @@ package rclone_test import ( - "context" "os/exec" "testing" "github.com/restic/restic/internal/backend/rclone" "github.com/restic/restic/internal/backend/test" - "github.com/restic/restic/internal/errors" - "github.com/restic/restic/internal/restic" rtest "github.com/restic/restic/internal/test" ) @@ -24,23 +21,15 @@ func newTestSuite(t testing.TB) *test.Suite[rclone.Config] { return &cfg, nil }, - // CreateFn is a function that creates a temporary repository for the tests. - Create: func(cfg rclone.Config) (restic.Backend, error) { - t.Logf("Create()") - be, err := rclone.Create(context.TODO(), cfg, nil) - var e *exec.Error - if errors.As(err, &e) && e.Err == exec.ErrNotFound { - t.Skipf("program %q not found", e.Name) - return nil, nil - } - return be, err - }, + Factory: rclone.NewFactory(), + } +} - // OpenFn is a function that opens a previously created temporary repository. - Open: func(cfg rclone.Config) (restic.Backend, error) { - t.Logf("Open()") - return rclone.Open(context.TODO(), cfg, nil) - }, +func findRclone(t testing.TB) { + // try to find a rclone binary + _, err := exec.LookPath("rclone") + if err != nil { + t.Skip(err) } } @@ -51,9 +40,11 @@ func TestBackendRclone(t *testing.T) { } }() + findRclone(t) newTestSuite(t).RunTests(t) } func BenchmarkBackendREST(t *testing.B) { + findRclone(t) newTestSuite(t).RunBenchmarks(t) } diff --git a/internal/backend/rest/rest_test.go b/internal/backend/rest/rest_test.go index 4d069e63c..60cc40afe 100644 --- a/internal/backend/rest/rest_test.go +++ b/internal/backend/rest/rest_test.go @@ -9,10 +9,8 @@ import ( "testing" "time" - "github.com/restic/restic/internal/backend" "github.com/restic/restic/internal/backend/rest" "github.com/restic/restic/internal/backend/test" - "github.com/restic/restic/internal/restic" rtest "github.com/restic/restic/internal/test" ) @@ -68,11 +66,6 @@ func runRESTServer(ctx context.Context, t testing.TB, dir string) (*url.URL, fun } func newTestSuite(_ context.Context, t testing.TB, url *url.URL, minimalData bool) *test.Suite[rest.Config] { - tr, err := backend.Transport(backend.TransportOptions{}) - if err != nil { - t.Fatalf("cannot create transport for tests: %v", err) - } - return &test.Suite[rest.Config]{ MinimalData: minimalData, @@ -83,20 +76,7 @@ func newTestSuite(_ context.Context, t testing.TB, url *url.URL, minimalData boo return &cfg, nil }, - // CreateFn is a function that creates a temporary repository for the tests. - Create: func(cfg rest.Config) (restic.Backend, error) { - return rest.Create(context.TODO(), cfg, tr) - }, - - // OpenFn is a function that opens a previously created temporary repository. - Open: func(cfg rest.Config) (restic.Backend, error) { - return rest.Open(context.TODO(), cfg, tr) - }, - - // CleanupFn removes data created during the tests. - Cleanup: func(cfg rest.Config) error { - return nil - }, + Factory: rest.NewFactory(), } } diff --git a/internal/backend/s3/s3_test.go b/internal/backend/s3/s3_test.go index 1cdc6d7e9..e645fe03e 100644 --- a/internal/backend/s3/s3_test.go +++ b/internal/backend/s3/s3_test.go @@ -4,22 +4,18 @@ import ( "context" "crypto/rand" "encoding/hex" - "errors" "fmt" "io" "net" - "net/http" "os" "os/exec" "path/filepath" "testing" "time" - "github.com/restic/restic/internal/backend" "github.com/restic/restic/internal/backend/s3" "github.com/restic/restic/internal/backend/test" "github.com/restic/restic/internal/options" - "github.com/restic/restic/internal/restic" rtest "github.com/restic/restic/internal/test" ) @@ -98,84 +94,34 @@ func newRandomCredentials(t testing.TB) (key, secret string) { return key, secret } -type MinioTestConfig struct { - s3.Config - - tempdir string - stopServer func() -} - -func createS3(t testing.TB, cfg MinioTestConfig, tr http.RoundTripper) (be restic.Backend, err error) { - for i := 0; i < 10; i++ { - be, err = s3.Create(context.TODO(), cfg.Config, tr) - if err != nil { - t.Logf("s3 open: try %d: error %v", i, err) - time.Sleep(500 * time.Millisecond) - continue - } - - break - } - - return be, err -} - -func newMinioTestSuite(ctx context.Context, t testing.TB) *test.Suite[MinioTestConfig] { - tr, err := backend.Transport(backend.TransportOptions{}) - if err != nil { - t.Fatalf("cannot create transport for tests: %v", err) - } - - return &test.Suite[MinioTestConfig]{ +func newMinioTestSuite(ctx context.Context, t testing.TB, key string, secret string) *test.Suite[s3.Config] { + return &test.Suite[s3.Config]{ // NewConfig returns a config for a new temporary backend that will be used in tests. - NewConfig: func() (*MinioTestConfig, error) { - cfg := MinioTestConfig{} - - cfg.tempdir = rtest.TempDir(t) - key, secret := newRandomCredentials(t) - cfg.stopServer = runMinio(ctx, t, cfg.tempdir, key, secret) - - cfg.Config = s3.NewConfig() - cfg.Config.Endpoint = "localhost:9000" - cfg.Config.Bucket = "restictestbucket" - cfg.Config.Prefix = fmt.Sprintf("test-%d", time.Now().UnixNano()) - cfg.Config.UseHTTP = true - cfg.Config.KeyID = key - cfg.Config.Secret = options.NewSecretString(secret) + NewConfig: func() (*s3.Config, error) { + cfg := s3.NewConfig() + cfg.Endpoint = "localhost:9000" + cfg.Bucket = "restictestbucket" + cfg.Prefix = fmt.Sprintf("test-%d", time.Now().UnixNano()) + cfg.UseHTTP = true + cfg.KeyID = key + cfg.Secret = options.NewSecretString(secret) return &cfg, nil }, - // CreateFn is a function that creates a temporary repository for the tests. - Create: func(cfg MinioTestConfig) (restic.Backend, error) { - be, err := createS3(t, cfg, tr) - if err != nil { - return nil, err - } + Factory: s3.NewFactory(), + } +} - _, err = be.Stat(context.TODO(), restic.Handle{Type: restic.ConfigFile}) - if err != nil && !be.IsNotExist(err) { - return nil, err - } +func createMinioTestSuite(t testing.TB) (*test.Suite[s3.Config], func()) { + ctx, cancel := context.WithCancel(context.Background()) - if err == nil { - return nil, errors.New("config already exists") - } + tempdir := rtest.TempDir(t) + key, secret := newRandomCredentials(t) + cleanup := runMinio(ctx, t, tempdir, key, secret) - return be, nil - }, - - // OpenFn is a function that opens a previously created temporary repository. - Open: func(cfg MinioTestConfig) (restic.Backend, error) { - return s3.Open(ctx, cfg.Config, tr) - }, - - // CleanupFn removes data created during the tests. - Cleanup: func(cfg MinioTestConfig) error { - if cfg.stopServer != nil { - cfg.stopServer() - } - return nil - }, + return newMinioTestSuite(ctx, t, key, secret), func() { + defer cancel() + defer cleanup() } } @@ -193,10 +139,10 @@ func TestBackendMinio(t *testing.T) { return } - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() + suite, cleanup := createMinioTestSuite(t) + defer cleanup() - newMinioTestSuite(ctx, t).RunTests(t) + suite.RunTests(t) } func BenchmarkBackendMinio(t *testing.B) { @@ -207,18 +153,13 @@ func BenchmarkBackendMinio(t *testing.B) { return } - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() + suite, cleanup := createMinioTestSuite(t) + defer cleanup() - newMinioTestSuite(ctx, t).RunBenchmarks(t) + suite.RunBenchmarks(t) } func newS3TestSuite(t testing.TB) *test.Suite[s3.Config] { - tr, err := backend.Transport(backend.TransportOptions{}) - if err != nil { - t.Fatalf("cannot create transport for tests: %v", err) - } - return &test.Suite[s3.Config]{ // do not use excessive data MinimalData: true, @@ -236,39 +177,7 @@ func newS3TestSuite(t testing.TB) *test.Suite[s3.Config] { return cfg, nil }, - // CreateFn is a function that creates a temporary repository for the tests. - Create: func(cfg s3.Config) (restic.Backend, error) { - be, err := s3.Create(context.TODO(), cfg, tr) - if err != nil { - return nil, err - } - - _, err = be.Stat(context.TODO(), restic.Handle{Type: restic.ConfigFile}) - if err != nil && !be.IsNotExist(err) { - return nil, err - } - - if err == nil { - return nil, errors.New("config already exists") - } - - return be, nil - }, - - // OpenFn is a function that opens a previously created temporary repository. - Open: func(cfg s3.Config) (restic.Backend, error) { - return s3.Open(context.TODO(), cfg, tr) - }, - - // CleanupFn removes data created during the tests. - Cleanup: func(cfg s3.Config) error { - be, err := s3.Open(context.TODO(), cfg, tr) - if err != nil { - return err - } - - return be.Delete(context.TODO()) - }, + Factory: s3.NewFactory(), } } diff --git a/internal/backend/sftp/sftp_test.go b/internal/backend/sftp/sftp_test.go index 98175ca26..75adc0c6b 100644 --- a/internal/backend/sftp/sftp_test.go +++ b/internal/backend/sftp/sftp_test.go @@ -1,7 +1,6 @@ package sftp_test import ( - "context" "fmt" "os" "path/filepath" @@ -11,7 +10,6 @@ import ( "github.com/restic/restic/internal/backend/sftp" "github.com/restic/restic/internal/backend/test" "github.com/restic/restic/internal/errors" - "github.com/restic/restic/internal/restic" rtest "github.com/restic/restic/internal/test" ) @@ -33,11 +31,7 @@ func newTestSuite(t testing.TB) *test.Suite[sftp.Config] { return &test.Suite[sftp.Config]{ // NewConfig returns a config for a new temporary backend that will be used in tests. NewConfig: func() (*sftp.Config, error) { - dir, err := os.MkdirTemp(rtest.TestTempDir, "restic-test-sftp-") - if err != nil { - t.Fatal(err) - } - + dir := rtest.TempDir(t) t.Logf("create new backend at %v", dir) cfg := &sftp.Config{ @@ -48,25 +42,7 @@ func newTestSuite(t testing.TB) *test.Suite[sftp.Config] { return cfg, nil }, - // CreateFn is a function that creates a temporary repository for the tests. - Create: func(cfg sftp.Config) (restic.Backend, error) { - return sftp.Create(context.TODO(), cfg) - }, - - // OpenFn is a function that opens a previously created temporary repository. - Open: func(cfg sftp.Config) (restic.Backend, error) { - return sftp.Open(context.TODO(), cfg) - }, - - // CleanupFn removes data created during the tests. - Cleanup: func(cfg sftp.Config) error { - if !rtest.TestCleanupTempDirs { - t.Logf("leaving test backend dir at %v", cfg.Path) - } - - rtest.RemoveAll(t, cfg.Path) - return nil - }, + Factory: sftp.NewFactory(), } } diff --git a/internal/backend/swift/swift_test.go b/internal/backend/swift/swift_test.go index 52278943e..98ee5b1c1 100644 --- a/internal/backend/swift/swift_test.go +++ b/internal/backend/swift/swift_test.go @@ -1,26 +1,18 @@ package swift_test import ( - "context" "fmt" "os" "testing" "time" - "github.com/restic/restic/internal/backend" "github.com/restic/restic/internal/backend/swift" "github.com/restic/restic/internal/backend/test" - "github.com/restic/restic/internal/errors" "github.com/restic/restic/internal/restic" rtest "github.com/restic/restic/internal/test" ) func newSwiftTestSuite(t testing.TB) *test.Suite[swift.Config] { - tr, err := backend.Transport(backend.TransportOptions{}) - if err != nil { - t.Fatalf("cannot create transport for tests: %v", err) - } - return &test.Suite[swift.Config]{ // do not use excessive data MinimalData: true, @@ -54,39 +46,7 @@ func newSwiftTestSuite(t testing.TB) *test.Suite[swift.Config] { return cfg, nil }, - // CreateFn is a function that creates a temporary repository for the tests. - Create: func(cfg swift.Config) (restic.Backend, error) { - be, err := swift.Open(context.TODO(), cfg, tr) - if err != nil { - return nil, err - } - - _, err = be.Stat(context.TODO(), restic.Handle{Type: restic.ConfigFile}) - if err != nil && !be.IsNotExist(err) { - return nil, err - } - - if err == nil { - return nil, errors.New("config already exists") - } - - return be, nil - }, - - // OpenFn is a function that opens a previously created temporary repository. - Open: func(cfg swift.Config) (restic.Backend, error) { - return swift.Open(context.TODO(), cfg, tr) - }, - - // CleanupFn removes data created during the tests. - Cleanup: func(cfg swift.Config) error { - be, err := swift.Open(context.TODO(), cfg, tr) - if err != nil { - return err - } - - return be.Delete(context.TODO()) - }, + Factory: swift.NewFactory(), } } diff --git a/internal/backend/test/suite.go b/internal/backend/test/suite.go index 75ae0630b..bb77124d7 100644 --- a/internal/backend/test/suite.go +++ b/internal/backend/test/suite.go @@ -1,11 +1,16 @@ package test import ( + "context" + "fmt" "reflect" "strings" "testing" "time" + "github.com/restic/restic/internal/backend" + "github.com/restic/restic/internal/backend/location" + "github.com/restic/restic/internal/errors" "github.com/restic/restic/internal/restic" "github.com/restic/restic/internal/test" ) @@ -18,14 +23,8 @@ type Suite[C any] struct { // NewConfig returns a config for a new temporary backend that will be used in tests. NewConfig func() (*C, error) - // CreateFn is a function that creates a temporary repository for the tests. - Create func(cfg C) (restic.Backend, error) - - // OpenFn is a function that opens a previously created temporary repository. - Open func(cfg C) (restic.Backend, error) - - // CleanupFn removes data created during the tests. - Cleanup func(cfg C) error + // Factory contains a factory that can be used to create or open a repository for the tests. + Factory location.Factory // MinimalData instructs the tests to not use excessive data. MinimalData bool @@ -60,11 +59,7 @@ func (s *Suite[C]) RunTests(t *testing.T) { return } - if s.Cleanup != nil { - if err = s.Cleanup(*s.Config); err != nil { - t.Fatal(err) - } - } + s.cleanup(t) } type testFunction struct { @@ -158,13 +153,34 @@ func (s *Suite[C]) RunBenchmarks(b *testing.B) { return } - if err = s.Cleanup(*s.Config); err != nil { - b.Fatal(err) + s.cleanup(b) +} + +func (s *Suite[C]) createOrError() (restic.Backend, error) { + tr, err := backend.Transport(backend.TransportOptions{}) + if err != nil { + return nil, fmt.Errorf("cannot create transport for tests: %v", err) } + + be, err := s.Factory.Create(context.TODO(), s.Config, tr, nil) + if err != nil { + return nil, err + } + + _, err = be.Stat(context.TODO(), restic.Handle{Type: restic.ConfigFile}) + if err != nil && !be.IsNotExist(err) { + return nil, err + } + + if err == nil { + return nil, errors.New("config already exists") + } + + return be, nil } func (s *Suite[C]) create(t testing.TB) restic.Backend { - be, err := s.Create(*s.Config) + be, err := s.createOrError() if err != nil { t.Fatal(err) } @@ -172,13 +188,26 @@ func (s *Suite[C]) create(t testing.TB) restic.Backend { } func (s *Suite[C]) open(t testing.TB) restic.Backend { - be, err := s.Open(*s.Config) + tr, err := backend.Transport(backend.TransportOptions{}) + if err != nil { + t.Fatalf("cannot create transport for tests: %v", err) + } + + be, err := s.Factory.Open(context.TODO(), s.Config, tr, nil) if err != nil { t.Fatal(err) } return be } +func (s *Suite[C]) cleanup(t testing.TB) { + be := s.open(t) + if err := be.Delete(context.TODO()); err != nil { + t.Fatal(err) + } + s.close(t, be) +} + func (s *Suite[C]) close(t testing.TB, be restic.Backend) { err := be.Close() if err != nil { diff --git a/internal/backend/test/tests.go b/internal/backend/test/tests.go index c4462495f..4a6a3f2a0 100644 --- a/internal/backend/test/tests.go +++ b/internal/backend/test/tests.go @@ -57,7 +57,7 @@ func (s *Suite[C]) TestCreateWithConfig(t *testing.T) { store(t, b, restic.ConfigFile, []byte("test config")) // now create the backend again, this must fail - _, err = s.Create(*s.Config) + _, err = s.createOrError() if err == nil { t.Fatalf("expected error not found for creating a backend with an existing config file") } From 705ad51bcc46cac89bcaf2b1b5f58d479a3e1e2e Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 8 Jun 2023 17:05:20 +0200 Subject: [PATCH 10/14] backend: check that StripPassword can be called --- internal/backend/test/tests.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/internal/backend/test/tests.go b/internal/backend/test/tests.go index 4a6a3f2a0..14faad0d5 100644 --- a/internal/backend/test/tests.go +++ b/internal/backend/test/tests.go @@ -36,6 +36,12 @@ func beTest(ctx context.Context, be restic.Backend, h restic.Handle) (bool, erro return err == nil, err } +// TestStripPasswordCall tests that the StripPassword method of a factory can be called without crashing. +// It does not verify whether passwords are removed correctly +func (s *Suite[C]) TestStripPasswordCall(t *testing.T) { + s.Factory.StripPassword("some random string") +} + // TestCreateWithConfig tests that creating a backend in a location which already // has a config file fails. func (s *Suite[C]) TestCreateWithConfig(t *testing.T) { From 50e0d5e6b5d0938e2926fcbed860c1af1e16fd0c Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 8 Jun 2023 17:32:43 +0200 Subject: [PATCH 11/14] backend: Hardcode backend scheme in Factory Our ParseConfig implementations always expect a specific scheme, thus no other scheme would work. --- cmd/restic/global.go | 18 +++++++-------- internal/backend/azure/azure.go | 2 +- internal/backend/b2/b2.go | 2 +- internal/backend/gs/gs.go | 2 +- internal/backend/local/local.go | 2 +- .../backend/location/display_location_test.go | 4 ++-- internal/backend/location/location_test.go | 9 ++++---- internal/backend/location/registry.go | 22 ++++++++++++++----- internal/backend/mem/mem_backend.go | 1 + internal/backend/rclone/backend.go | 2 +- internal/backend/rest/rest.go | 2 +- internal/backend/s3/s3.go | 2 +- internal/backend/sftp/sftp.go | 2 +- internal/backend/swift/swift.go | 2 +- 14 files changed, 43 insertions(+), 29 deletions(-) diff --git a/cmd/restic/global.go b/cmd/restic/global.go index 1c13fb887..000ffac0b 100644 --- a/cmd/restic/global.go +++ b/cmd/restic/global.go @@ -100,15 +100,15 @@ var internalGlobalCtx context.Context func init() { backends := location.NewRegistry() - backends.Register("b2", b2.NewFactory()) - backends.Register("local", local.NewFactory()) - backends.Register("sftp", sftp.NewFactory()) - backends.Register("s3", s3.NewFactory()) - backends.Register("gs", gs.NewFactory()) - backends.Register("azure", azure.NewFactory()) - backends.Register("swift", swift.NewFactory()) - backends.Register("rest", rest.NewFactory()) - backends.Register("rclone", rclone.NewFactory()) + backends.Register(azure.NewFactory()) + backends.Register(b2.NewFactory()) + backends.Register(gs.NewFactory()) + backends.Register(local.NewFactory()) + backends.Register(rclone.NewFactory()) + backends.Register(rest.NewFactory()) + backends.Register(s3.NewFactory()) + backends.Register(sftp.NewFactory()) + backends.Register(swift.NewFactory()) globalOptions.backends = backends var cancel context.CancelFunc diff --git a/internal/backend/azure/azure.go b/internal/backend/azure/azure.go index b33b8dca6..a9267a945 100644 --- a/internal/backend/azure/azure.go +++ b/internal/backend/azure/azure.go @@ -45,7 +45,7 @@ const defaultListMaxItems = 5000 var _ restic.Backend = &Backend{} func NewFactory() location.Factory { - return location.NewHTTPBackendFactory(ParseConfig, location.NoPassword, Create, Open) + return location.NewHTTPBackendFactory("azure", ParseConfig, location.NoPassword, Create, Open) } func open(cfg Config, rt http.RoundTripper) (*Backend, error) { diff --git a/internal/backend/b2/b2.go b/internal/backend/b2/b2.go index 700fff099..0bd3b994c 100644 --- a/internal/backend/b2/b2.go +++ b/internal/backend/b2/b2.go @@ -38,7 +38,7 @@ const defaultListMaxItems = 10 * 1000 var _ restic.Backend = &b2Backend{} func NewFactory() location.Factory { - return location.NewHTTPBackendFactory(ParseConfig, location.NoPassword, Create, Open) + return location.NewHTTPBackendFactory("b2", ParseConfig, location.NoPassword, Create, Open) } type sniffingRoundTripper struct { diff --git a/internal/backend/gs/gs.go b/internal/backend/gs/gs.go index 445ccc77d..5c12654d6 100644 --- a/internal/backend/gs/gs.go +++ b/internal/backend/gs/gs.go @@ -49,7 +49,7 @@ type Backend struct { var _ restic.Backend = &Backend{} func NewFactory() location.Factory { - return location.NewHTTPBackendFactory(ParseConfig, location.NoPassword, Create, Open) + return location.NewHTTPBackendFactory("gs", ParseConfig, location.NoPassword, Create, Open) } func getStorageClient(rt http.RoundTripper) (*storage.Client, error) { diff --git a/internal/backend/local/local.go b/internal/backend/local/local.go index e9d00abf7..4198102c2 100644 --- a/internal/backend/local/local.go +++ b/internal/backend/local/local.go @@ -31,7 +31,7 @@ type Local struct { var _ restic.Backend = &Local{} func NewFactory() location.Factory { - return location.NewLimitedBackendFactory(ParseConfig, location.NoPassword, limiter.WrapBackendConstructor(Create), limiter.WrapBackendConstructor(Open)) + return location.NewLimitedBackendFactory("local", ParseConfig, location.NoPassword, limiter.WrapBackendConstructor(Create), limiter.WrapBackendConstructor(Open)) } const defaultLayout = "default" diff --git a/internal/backend/location/display_location_test.go b/internal/backend/location/display_location_test.go index 4a4055a84..19502d85b 100644 --- a/internal/backend/location/display_location_test.go +++ b/internal/backend/location/display_location_test.go @@ -10,8 +10,8 @@ import ( func TestStripPassword(t *testing.T) { registry := location.NewRegistry() - registry.Register("test", - location.NewHTTPBackendFactory[any, restic.Backend](nil, + registry.Register( + location.NewHTTPBackendFactory[any, restic.Backend]("test", nil, func(s string) string { return "cleaned" }, nil, nil, diff --git a/internal/backend/location/location_test.go b/internal/backend/location/location_test.go index 933f2fc08..b2623032e 100644 --- a/internal/backend/location/location_test.go +++ b/internal/backend/location/location_test.go @@ -14,6 +14,7 @@ type testConfig struct { func testFactory() location.Factory { return location.NewHTTPBackendFactory[testConfig, restic.Backend]( + "local", func(s string) (*testConfig, error) { return &testConfig{loc: s}, nil }, nil, nil, nil, @@ -22,12 +23,12 @@ func testFactory() location.Factory { func TestParse(t *testing.T) { registry := location.NewRegistry() - registry.Register("test", testFactory()) + registry.Register(testFactory()) - path := "test:example" + path := "local:example" u, err := location.Parse(registry, path) test.OK(t, err) - test.Equals(t, "test", u.Scheme) + test.Equals(t, "local", u.Scheme) test.Equals(t, &testConfig{loc: path}, u.Config) } @@ -43,7 +44,7 @@ func TestParseFallback(t *testing.T) { } registry := location.NewRegistry() - registry.Register("local", testFactory()) + registry.Register(testFactory()) for _, path := range fallbackTests { t.Run(path, func(t *testing.T) { diff --git a/internal/backend/location/registry.go b/internal/backend/location/registry.go index f15095590..a8818bd73 100644 --- a/internal/backend/location/registry.go +++ b/internal/backend/location/registry.go @@ -18,11 +18,11 @@ func NewRegistry() *Registry { } } -func (r *Registry) Register(scheme string, factory Factory) { - if r.factories[scheme] != nil { +func (r *Registry) Register(factory Factory) { + if r.factories[factory.Scheme()] != nil { panic("duplicate backend") } - r.factories[scheme] = factory + r.factories[factory.Scheme()] = factory } func (r *Registry) Lookup(scheme string) Factory { @@ -30,6 +30,7 @@ func (r *Registry) Lookup(scheme string) Factory { } type Factory interface { + Scheme() string ParseConfig(s string) (interface{}, error) StripPassword(s string) string Create(ctx context.Context, cfg interface{}, rt http.RoundTripper, lim limiter.Limiter) (restic.Backend, error) @@ -37,12 +38,17 @@ type Factory interface { } type genericBackendFactory[C any, T restic.Backend] struct { + scheme string parseConfigFn func(s string) (*C, error) stripPasswordFn func(s string) string createFn func(ctx context.Context, cfg C, rt http.RoundTripper, lim limiter.Limiter) (T, error) openFn func(ctx context.Context, cfg C, rt http.RoundTripper, lim limiter.Limiter) (T, error) } +func (f *genericBackendFactory[C, T]) Scheme() string { + return f.scheme +} + func (f *genericBackendFactory[C, T]) ParseConfig(s string) (interface{}, error) { return f.parseConfigFn(s) } @@ -59,12 +65,15 @@ func (f *genericBackendFactory[C, T]) Open(ctx context.Context, cfg interface{}, return f.openFn(ctx, *cfg.(*C), rt, lim) } -func NewHTTPBackendFactory[C any, T restic.Backend](parseConfigFn func(s string) (*C, error), +func NewHTTPBackendFactory[C any, T restic.Backend]( + scheme string, + parseConfigFn func(s string) (*C, error), stripPasswordFn func(s string) string, createFn func(ctx context.Context, cfg C, rt http.RoundTripper) (T, error), openFn func(ctx context.Context, cfg C, rt http.RoundTripper) (T, error)) Factory { return &genericBackendFactory[C, T]{ + scheme: scheme, parseConfigFn: parseConfigFn, stripPasswordFn: stripPasswordFn, createFn: func(ctx context.Context, cfg C, rt http.RoundTripper, _ limiter.Limiter) (T, error) { @@ -76,12 +85,15 @@ func NewHTTPBackendFactory[C any, T restic.Backend](parseConfigFn func(s string) } } -func NewLimitedBackendFactory[C any, T restic.Backend](parseConfigFn func(s string) (*C, error), +func NewLimitedBackendFactory[C any, T restic.Backend]( + scheme string, + parseConfigFn func(s string) (*C, error), stripPasswordFn func(s string) string, createFn func(ctx context.Context, cfg C, lim limiter.Limiter) (T, error), openFn func(ctx context.Context, cfg C, lim limiter.Limiter) (T, error)) Factory { return &genericBackendFactory[C, T]{ + scheme: scheme, parseConfigFn: parseConfigFn, stripPasswordFn: stripPasswordFn, createFn: func(ctx context.Context, cfg C, _ http.RoundTripper, lim limiter.Limiter) (T, error) { diff --git a/internal/backend/mem/mem_backend.go b/internal/backend/mem/mem_backend.go index a467d33f7..86ec48756 100644 --- a/internal/backend/mem/mem_backend.go +++ b/internal/backend/mem/mem_backend.go @@ -27,6 +27,7 @@ func NewFactory() location.Factory { be := New() return location.NewHTTPBackendFactory[struct{}, *MemoryBackend]( + "mem", func(s string) (*struct{}, error) { return &struct{}{}, nil }, diff --git a/internal/backend/rclone/backend.go b/internal/backend/rclone/backend.go index f3a97ef75..fd6f5b262 100644 --- a/internal/backend/rclone/backend.go +++ b/internal/backend/rclone/backend.go @@ -38,7 +38,7 @@ type Backend struct { } func NewFactory() location.Factory { - return location.NewLimitedBackendFactory(ParseConfig, location.NoPassword, Create, Open) + return location.NewLimitedBackendFactory("rclone", ParseConfig, location.NoPassword, Create, Open) } // run starts command with args and initializes the StdioConn. diff --git a/internal/backend/rest/rest.go b/internal/backend/rest/rest.go index 4fb2d54de..8391df681 100644 --- a/internal/backend/rest/rest.go +++ b/internal/backend/rest/rest.go @@ -31,7 +31,7 @@ type Backend struct { } func NewFactory() location.Factory { - return location.NewHTTPBackendFactory(ParseConfig, StripPassword, Create, Open) + return location.NewHTTPBackendFactory("rest", ParseConfig, StripPassword, Create, Open) } // the REST API protocol version is decided by HTTP request headers, these are the constants. diff --git a/internal/backend/s3/s3.go b/internal/backend/s3/s3.go index 10512e809..3fe32d215 100644 --- a/internal/backend/s3/s3.go +++ b/internal/backend/s3/s3.go @@ -33,7 +33,7 @@ type Backend struct { var _ restic.Backend = &Backend{} func NewFactory() location.Factory { - return location.NewHTTPBackendFactory(ParseConfig, location.NoPassword, Create, Open) + return location.NewHTTPBackendFactory("s3", ParseConfig, location.NoPassword, Create, Open) } const defaultLayout = "default" diff --git a/internal/backend/sftp/sftp.go b/internal/backend/sftp/sftp.go index 1e12df808..3e127ef05 100644 --- a/internal/backend/sftp/sftp.go +++ b/internal/backend/sftp/sftp.go @@ -44,7 +44,7 @@ type SFTP struct { var _ restic.Backend = &SFTP{} func NewFactory() location.Factory { - return location.NewLimitedBackendFactory(ParseConfig, location.NoPassword, limiter.WrapBackendConstructor(Create), limiter.WrapBackendConstructor(Open)) + return location.NewLimitedBackendFactory("sftp", ParseConfig, location.NoPassword, limiter.WrapBackendConstructor(Create), limiter.WrapBackendConstructor(Open)) } const defaultLayout = "default" diff --git a/internal/backend/swift/swift.go b/internal/backend/swift/swift.go index 019456be7..1cfc0a65b 100644 --- a/internal/backend/swift/swift.go +++ b/internal/backend/swift/swift.go @@ -36,7 +36,7 @@ type beSwift struct { var _ restic.Backend = &beSwift{} func NewFactory() location.Factory { - return location.NewHTTPBackendFactory(ParseConfig, location.NoPassword, Open, Open) + return location.NewHTTPBackendFactory("swift", ParseConfig, location.NoPassword, Open, Open) } // Open opens the swift backend at a container in region. The container is From b5511e8e4c4231bb4b746ec727c963b527fe896f Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 8 Jun 2023 19:35:20 +0200 Subject: [PATCH 12/14] Fix linter warnings --- internal/backend/azure/azure_test.go | 6 ++-- internal/backend/b2/b2_test.go | 6 ++-- internal/backend/gs/gs_test.go | 6 ++-- internal/backend/rest/rest_test.go | 11 +++--- internal/backend/s3/s3_test.go | 52 +++++++++++++--------------- internal/backend/test/tests.go | 2 +- 6 files changed, 38 insertions(+), 45 deletions(-) diff --git a/internal/backend/azure/azure_test.go b/internal/backend/azure/azure_test.go index 8465cc3b0..33f65bd52 100644 --- a/internal/backend/azure/azure_test.go +++ b/internal/backend/azure/azure_test.go @@ -17,7 +17,7 @@ import ( rtest "github.com/restic/restic/internal/test" ) -func newAzureTestSuite(t testing.TB) *test.Suite[azure.Config] { +func newAzureTestSuite() *test.Suite[azure.Config] { return &test.Suite[azure.Config]{ // do not use excessive data MinimalData: true, @@ -59,7 +59,7 @@ func TestBackendAzure(t *testing.T) { } t.Logf("run tests") - newAzureTestSuite(t).RunTests(t) + newAzureTestSuite().RunTests(t) } func BenchmarkBackendAzure(t *testing.B) { @@ -77,7 +77,7 @@ func BenchmarkBackendAzure(t *testing.B) { } t.Logf("run tests") - newAzureTestSuite(t).RunBenchmarks(t) + newAzureTestSuite().RunBenchmarks(t) } func TestUploadLargeFile(t *testing.T) { diff --git a/internal/backend/b2/b2_test.go b/internal/backend/b2/b2_test.go index 348af9095..ab1dcd37b 100644 --- a/internal/backend/b2/b2_test.go +++ b/internal/backend/b2/b2_test.go @@ -12,7 +12,7 @@ import ( rtest "github.com/restic/restic/internal/test" ) -func newB2TestSuite(t testing.TB) *test.Suite[b2.Config] { +func newB2TestSuite() *test.Suite[b2.Config] { return &test.Suite[b2.Config]{ // do not use excessive data MinimalData: true, @@ -59,10 +59,10 @@ func TestBackendB2(t *testing.T) { }() testVars(t) - newB2TestSuite(t).RunTests(t) + newB2TestSuite().RunTests(t) } func BenchmarkBackendb2(t *testing.B) { testVars(t) - newB2TestSuite(t).RunBenchmarks(t) + newB2TestSuite().RunBenchmarks(t) } diff --git a/internal/backend/gs/gs_test.go b/internal/backend/gs/gs_test.go index 47085dc4e..22953cad3 100644 --- a/internal/backend/gs/gs_test.go +++ b/internal/backend/gs/gs_test.go @@ -11,7 +11,7 @@ import ( rtest "github.com/restic/restic/internal/test" ) -func newGSTestSuite(t testing.TB) *test.Suite[gs.Config] { +func newGSTestSuite() *test.Suite[gs.Config] { return &test.Suite[gs.Config]{ // do not use excessive data MinimalData: true, @@ -56,7 +56,7 @@ func TestBackendGS(t *testing.T) { } t.Logf("run tests") - newGSTestSuite(t).RunTests(t) + newGSTestSuite().RunTests(t) } func BenchmarkBackendGS(t *testing.B) { @@ -77,5 +77,5 @@ func BenchmarkBackendGS(t *testing.B) { } t.Logf("run tests") - newGSTestSuite(t).RunBenchmarks(t) + newGSTestSuite().RunBenchmarks(t) } diff --git a/internal/backend/rest/rest_test.go b/internal/backend/rest/rest_test.go index 60cc40afe..6a5b4f8a5 100644 --- a/internal/backend/rest/rest_test.go +++ b/internal/backend/rest/rest_test.go @@ -65,7 +65,7 @@ func runRESTServer(ctx context.Context, t testing.TB, dir string) (*url.URL, fun return url, cleanup } -func newTestSuite(_ context.Context, t testing.TB, url *url.URL, minimalData bool) *test.Suite[rest.Config] { +func newTestSuite(url *url.URL, minimalData bool) *test.Suite[rest.Config] { return &test.Suite[rest.Config]{ MinimalData: minimalData, @@ -94,7 +94,7 @@ func TestBackendREST(t *testing.T) { serverURL, cleanup := runRESTServer(ctx, t, dir) defer cleanup() - newTestSuite(ctx, t, serverURL, false).RunTests(t) + newTestSuite(serverURL, false).RunTests(t) } func TestBackendRESTExternalServer(t *testing.T) { @@ -108,10 +108,7 @@ func TestBackendRESTExternalServer(t *testing.T) { t.Fatal(err) } - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - - newTestSuite(ctx, t, cfg.URL, true).RunTests(t) + newTestSuite(cfg.URL, true).RunTests(t) } func BenchmarkBackendREST(t *testing.B) { @@ -122,5 +119,5 @@ func BenchmarkBackendREST(t *testing.B) { serverURL, cleanup := runRESTServer(ctx, t, dir) defer cleanup() - newTestSuite(ctx, t, serverURL, false).RunBenchmarks(t) + newTestSuite(serverURL, false).RunBenchmarks(t) } diff --git a/internal/backend/s3/s3_test.go b/internal/backend/s3/s3_test.go index e645fe03e..17f5f7016 100644 --- a/internal/backend/s3/s3_test.go +++ b/internal/backend/s3/s3_test.go @@ -94,35 +94,31 @@ func newRandomCredentials(t testing.TB) (key, secret string) { return key, secret } -func newMinioTestSuite(ctx context.Context, t testing.TB, key string, secret string) *test.Suite[s3.Config] { - return &test.Suite[s3.Config]{ - // NewConfig returns a config for a new temporary backend that will be used in tests. - NewConfig: func() (*s3.Config, error) { - cfg := s3.NewConfig() - cfg.Endpoint = "localhost:9000" - cfg.Bucket = "restictestbucket" - cfg.Prefix = fmt.Sprintf("test-%d", time.Now().UnixNano()) - cfg.UseHTTP = true - cfg.KeyID = key - cfg.Secret = options.NewSecretString(secret) - return &cfg, nil - }, - - Factory: s3.NewFactory(), - } -} - -func createMinioTestSuite(t testing.TB) (*test.Suite[s3.Config], func()) { +func newMinioTestSuite(t testing.TB) (*test.Suite[s3.Config], func()) { ctx, cancel := context.WithCancel(context.Background()) tempdir := rtest.TempDir(t) key, secret := newRandomCredentials(t) cleanup := runMinio(ctx, t, tempdir, key, secret) - return newMinioTestSuite(ctx, t, key, secret), func() { - defer cancel() - defer cleanup() - } + return &test.Suite[s3.Config]{ + // NewConfig returns a config for a new temporary backend that will be used in tests. + NewConfig: func() (*s3.Config, error) { + cfg := s3.NewConfig() + cfg.Endpoint = "localhost:9000" + cfg.Bucket = "restictestbucket" + cfg.Prefix = fmt.Sprintf("test-%d", time.Now().UnixNano()) + cfg.UseHTTP = true + cfg.KeyID = key + cfg.Secret = options.NewSecretString(secret) + return &cfg, nil + }, + + Factory: s3.NewFactory(), + }, func() { + defer cancel() + defer cleanup() + } } func TestBackendMinio(t *testing.T) { @@ -139,7 +135,7 @@ func TestBackendMinio(t *testing.T) { return } - suite, cleanup := createMinioTestSuite(t) + suite, cleanup := newMinioTestSuite(t) defer cleanup() suite.RunTests(t) @@ -153,13 +149,13 @@ func BenchmarkBackendMinio(t *testing.B) { return } - suite, cleanup := createMinioTestSuite(t) + suite, cleanup := newMinioTestSuite(t) defer cleanup() suite.RunBenchmarks(t) } -func newS3TestSuite(t testing.TB) *test.Suite[s3.Config] { +func newS3TestSuite() *test.Suite[s3.Config] { return &test.Suite[s3.Config]{ // do not use excessive data MinimalData: true, @@ -202,7 +198,7 @@ func TestBackendS3(t *testing.T) { } t.Logf("run tests") - newS3TestSuite(t).RunTests(t) + newS3TestSuite().RunTests(t) } func BenchmarkBackendS3(t *testing.B) { @@ -220,5 +216,5 @@ func BenchmarkBackendS3(t *testing.B) { } t.Logf("run tests") - newS3TestSuite(t).RunBenchmarks(t) + newS3TestSuite().RunBenchmarks(t) } diff --git a/internal/backend/test/tests.go b/internal/backend/test/tests.go index 14faad0d5..c2e5d0fc0 100644 --- a/internal/backend/test/tests.go +++ b/internal/backend/test/tests.go @@ -38,7 +38,7 @@ func beTest(ctx context.Context, be restic.Backend, h restic.Handle) (bool, erro // TestStripPasswordCall tests that the StripPassword method of a factory can be called without crashing. // It does not verify whether passwords are removed correctly -func (s *Suite[C]) TestStripPasswordCall(t *testing.T) { +func (s *Suite[C]) TestStripPasswordCall(_ *testing.T) { s.Factory.StripPassword("some random string") } From cbf87fbdb35442c6b67cb333072c1605896e8bf9 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sat, 17 Jun 2023 14:40:10 +0200 Subject: [PATCH 13/14] init: don't include password in debug log --- cmd/restic/global.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/restic/global.go b/cmd/restic/global.go index 000ffac0b..823a82e36 100644 --- a/cmd/restic/global.go +++ b/cmd/restic/global.go @@ -623,7 +623,7 @@ func open(ctx context.Context, s string, gopts GlobalOptions, opts options.Optio // Create the backend specified by URI. func create(ctx context.Context, s string, gopts GlobalOptions, opts options.Options) (restic.Backend, error) { - debug.Log("parsing location %v", s) + debug.Log("parsing location %v", location.StripPassword(gopts.backends, s)) loc, err := location.Parse(gopts.backends, s) if err != nil { return nil, err From 93038ed8f4c73cdebb54d8d3af7ea34b8bb01c2b Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sat, 17 Jun 2023 15:25:08 +0200 Subject: [PATCH 14/14] s3: restore retries for minio tests --- internal/backend/s3/s3_test.go | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/internal/backend/s3/s3_test.go b/internal/backend/s3/s3_test.go index 17f5f7016..3051d8ddb 100644 --- a/internal/backend/s3/s3_test.go +++ b/internal/backend/s3/s3_test.go @@ -7,15 +7,18 @@ import ( "fmt" "io" "net" + "net/http" "os" "os/exec" "path/filepath" "testing" "time" + "github.com/restic/restic/internal/backend/location" "github.com/restic/restic/internal/backend/s3" "github.com/restic/restic/internal/backend/test" "github.com/restic/restic/internal/options" + "github.com/restic/restic/internal/restic" rtest "github.com/restic/restic/internal/test" ) @@ -114,7 +117,18 @@ func newMinioTestSuite(t testing.TB) (*test.Suite[s3.Config], func()) { return &cfg, nil }, - Factory: s3.NewFactory(), + Factory: location.NewHTTPBackendFactory("s3", s3.ParseConfig, location.NoPassword, func(ctx context.Context, cfg s3.Config, rt http.RoundTripper) (be restic.Backend, err error) { + for i := 0; i < 10; i++ { + be, err = s3.Create(ctx, cfg, rt) + if err != nil { + t.Logf("s3 open: try %d: error %v", i, err) + time.Sleep(500 * time.Millisecond) + continue + } + break + } + return be, err + }, s3.Open), }, func() { defer cancel() defer cleanup()