From 03e4c5c19ceb23b957e5e0d536ecd0e24941ea6e Mon Sep 17 00:00:00 2001 From: hagen1778 Date: Wed, 10 Jul 2024 13:06:27 +0200 Subject: [PATCH] lib/bakcup/azremote: follow-up after 5fd3aef54954c9744a370c5c11e1639cc52049e1 Simplify tests by converting them to f-tests. https://github.com/VictoriaMetrics/VictoriaMetrics/commit/5fd3aef54954c9744a370c5c11e1639cc52049e1 Signed-off-by: hagen1778 --- lib/backup/azremote/azblob.go | 38 +++---- lib/backup/azremote/azblob_test.go | 172 +++++++---------------------- 2 files changed, 54 insertions(+), 156 deletions(-) diff --git a/lib/backup/azremote/azblob.go b/lib/backup/azremote/azblob.go index ea4590048..0f0dd9d25 100644 --- a/lib/backup/azremote/azblob.go +++ b/lib/backup/azremote/azblob.go @@ -33,26 +33,6 @@ const ( storageErrorCodeBlobNotFound = "BlobNotFound" ) -var ( - errNoCredentials = fmt.Errorf( - `failed to detect credentials for AZBlob. -Ensure that one of the options is set: connection string at %q; shared key at %q and %q; account name at %q and set %q to "true"`, - envStorageAccCs, - envStorageAcctName, - envStorageAccKey, - envStorageAcctName, - envStorageDefault, - ) - - errInvalidCredentials = fmt.Errorf("failed to process credentials: only one of %s, %s and %s, or %s and %s can be specified", - envStorageAccCs, - envStorageAcctName, - envStorageAccKey, - envStorageAcctName, - envStorageDefault, - ) -) - // FS represents filesystem for backups in Azure Blob Storage. // // Init must be called before calling other FS methods. @@ -109,7 +89,13 @@ func (fs *FS) newClient() (*service.Client, error) { switch { // can't specify any combination of more than one credential case moreThanOne(hasConnString, (hasAccountName && hasAccountKey), (useDefault == "true" && hasAccountName)): - return nil, errInvalidCredentials + return nil, fmt.Errorf("failed to process credentials: only one of %s, %s and %s, or %s and %s can be specified", + envStorageAccCs, + envStorageAcctName, + envStorageAccKey, + envStorageAcctName, + envStorageDefault, + ) case hasConnString: logger.Infof("Creating AZBlob service client from connection string") return service.NewClientFromConnectionString(connString, nil) @@ -128,7 +114,15 @@ func (fs *FS) newClient() (*service.Client, error) { } return service.NewClient(serviceURL, creds, nil) default: - return nil, errNoCredentials + return nil, fmt.Errorf( + `failed to detect credentials for AZBlob. +Ensure that one of the options is set: connection string at %q; shared key at %q and %q; account name at %q and set %q to "true"`, + envStorageAccCs, + envStorageAcctName, + envStorageAccKey, + envStorageAcctName, + envStorageDefault, + ) } } diff --git a/lib/backup/azremote/azblob_test.go b/lib/backup/azremote/azblob_test.go index 821ef4417..13fda039e 100644 --- a/lib/backup/azremote/azblob_test.go +++ b/lib/backup/azremote/azblob_test.go @@ -1,155 +1,59 @@ package azremote import ( - "bytes" - "errors" "strings" "testing" - - "github.com/VictoriaMetrics/VictoriaMetrics/lib/logger" ) func Test_cleanDirectory(t *testing.T) { - cases := map[string]struct { - Dir string - ExpectedDir string - }{ - "dir / prefix is removed": { - Dir: "/foo/", - ExpectedDir: "foo/", - }, - "multiple dir prefix / is removed": { - Dir: "//foo/", - ExpectedDir: "foo/", - }, - "suffix is added": { - Dir: "foo", - ExpectedDir: "foo/", - }, - } - - for name, test := range cases { - t.Run(name, func(t *testing.T) { - dir := cleanDirectory(test.Dir) - - if dir != test.ExpectedDir { - t.Errorf("expected dir %q, got %q", test.ExpectedDir, dir) - } - }) + f := func(dir, exp string) { + t.Helper() + got := cleanDirectory(dir) + if got != exp { + t.Errorf("expected dir %q, got %q", exp, got) + } } + f("/foo/", "foo/") + f("//foo/", "foo/") + f("foo", "foo/") } func Test_FSInit(t *testing.T) { - cases := map[string]struct { - IgnoreFakeEnv bool - Env testEnv - ExpectedErr error - ExpectedLogs []string - }{ - "connection string env var is used": { - Env: map[string]string{ - envStorageAccCs: "BlobEndpoint=https://test.blob.core.windows.net/;SharedAccessSignature=", - }, - ExpectedLogs: []string{`Creating AZBlob service client from connection string`}, - }, - "base envtemplate package is used and connection string err bubbles": { - IgnoreFakeEnv: true, - Env: map[string]string{ - envStorageAccCs: "BlobEndpoint=https://test.blob.core.windows.net/;SharedAccessSignature=", - }, - ExpectedErr: errNoCredentials, - }, - "only storage account name is an err": { - Env: map[string]string{ - envStorageAcctName: "test", - }, - ExpectedErr: errNoCredentials, - }, - "uses shared key credential": { - Env: map[string]string{ - envStorageAcctName: "test", - envStorageAccKey: "dGVhcG90Cg==", - }, - ExpectedLogs: []string{`Creating AZBlob service client from account name and key`}, - }, - "allows overriding domain name with account name and key": { - Env: map[string]string{ - envStorageAcctName: "test", - envStorageAccKey: "dGVhcG90Cg==", - envStorageDomain: "foo.bar", - }, - ExpectedLogs: []string{ - `Creating AZBlob service client from account name and key`, - `Overriding default Azure blob domain with "foo.bar"`, - }, - }, - "can't specify both connection string and shared key": { - Env: map[string]string{ - envStorageAccCs: "teapot", - envStorageAcctName: "test", - envStorageAccKey: "dGVhcG90Cg==", - }, - ExpectedErr: errInvalidCredentials, - }, - "just use default is an err": { - Env: map[string]string{ - envStorageDefault: "true", - }, - ExpectedErr: errNoCredentials, - }, - "uses default credential": { - Env: map[string]string{ - envStorageDefault: "true", - envStorageAcctName: "test", - }, - ExpectedLogs: []string{`Creating AZBlob service client from default credential`}, - }, - } + f := func(expErr string, params ...string) { + t.Helper() - for name, test := range cases { - t.Run(name, func(t *testing.T) { - tlog := &testLogger{} + env := make(testEnv) + for i := 0; i < len(params); i += 2 { + env[params[i]] = params[i+1] + } - logger.SetOutputForTests(tlog) - t.Cleanup(logger.ResetOutputForTest) - - fs := &FS{Dir: "foo"} - if test.Env != nil && !test.IgnoreFakeEnv { - fs.env = test.Env.LookupEnv + fs := &FS{Dir: "foo"} + fs.env = env.LookupEnv + err := fs.Init() + if err != nil { + if expErr == "" { + t.Fatalf("unexpected error %v", err) } - - err := fs.Init() - if err != nil && !errors.Is(err, test.ExpectedErr) { - t.Errorf("expected error %q, got %q", test.ExpectedErr, err) + if !strings.Contains(err.Error(), expErr) { + t.Fatalf("expected error: \n%q, \ngot: \n%v", expErr, err) } - - tlog.MustContain(t, test.ExpectedLogs...) - }) - } -} - -type testLogger struct { - buf *bytes.Buffer -} - -func (l *testLogger) Write(p []byte) (n int, err error) { - if l.buf == nil { - l.buf = &bytes.Buffer{} - } - - return l.buf.Write(p) -} - -func (l *testLogger) MustContain(t *testing.T, vals ...string) { - t.Helper() - - contents := l.buf.String() - - for _, val := range vals { - if !strings.Contains(contents, val) { - t.Errorf("expected log to contain %q, got %q", val, l.buf.String()) + return + } + if expErr != "" { + t.Fatalf("expected to have an error %q, instead got nil", expErr) } } + + f("", envStorageAccCs, "BlobEndpoint=https://test.blob.core.windows.net/;SharedAccessSignature=") + f("", envStorageAcctName, "test", envStorageAccKey, "dGVhcG90Cg==") + f("", envStorageDefault, "true", envStorageAcctName, "test") + f("", envStorageAcctName, "test", envStorageAccKey, "dGVhcG90Cg==", envStorageDomain, "foo.bar") + + f("failed to detect credentials for AZBlob") + f("failed to detect credentials for AZBlob", envStorageAcctName, "test") + f("failed to create Shared Key", envStorageAcctName, "", envStorageAccKey, "!") + f("connection string is either blank or malformed", envStorageAccCs, "") + f("failed to process credentials: only one of", envStorageAccCs, "teapot", envStorageAcctName, "test", envStorageAccKey, "dGVhcG90Cg==") } type testEnv map[string]string