From 7326e8fad585d85f53eec89be1f2dbb5c0eae46b Mon Sep 17 00:00:00 2001 From: Ali Dowair Date: Wed, 22 Dec 2021 16:46:29 -0500 Subject: [PATCH 01/15] Add RecreateIfExists field in PutOptions struct --- repo/blob/storage.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/repo/blob/storage.go b/repo/blob/storage.go index 5675348a8f..3004ec8bcc 100644 --- a/repo/blob/storage.go +++ b/repo/blob/storage.go @@ -59,8 +59,9 @@ type Reader interface { // PutOptions represents put-options for a single BLOB in a storage. type PutOptions struct { - RetentionMode string - RetentionPeriod time.Duration + RetentionMode string + RetentionPeriod time.Duration + RecreateIfExists bool } // HasRetentionOptions returns true when blob-retention settings have been From fe5bb7667d571441cc8babcf67ef7f422f3397df Mon Sep 17 00:00:00 2001 From: Ali Dowair Date: Wed, 22 Dec 2021 17:49:37 -0500 Subject: [PATCH 02/15] Add new error type and make it non-retriable The new error type `ErrBlobAlreadyExists` should be returned when we try to put a blob using a blob.ID that already exists, and that is somehow problematic for this operation. This error type may be useful for other operations in the future. --- repo/blob/retrying/retrying_storage.go | 3 +++ repo/blob/storage.go | 3 +++ 2 files changed, 6 insertions(+) diff --git a/repo/blob/retrying/retrying_storage.go b/repo/blob/retrying/retrying_storage.go index c10479ee41..d8f035ae5c 100644 --- a/repo/blob/retrying/retrying_storage.go +++ b/repo/blob/retrying/retrying_storage.go @@ -81,6 +81,9 @@ func isRetriable(err error) bool { case errors.Is(err, blob.ErrSetTimeUnsupported): return false + case errors.Is(err, blob.ErrBlobAlreadyExists): + return false + default: return true } diff --git a/repo/blob/storage.go b/repo/blob/storage.go index 3004ec8bcc..9e2230afb1 100644 --- a/repo/blob/storage.go +++ b/repo/blob/storage.go @@ -17,6 +17,9 @@ var ErrSetTimeUnsupported = errors.Errorf("SetTime is not supported") // ErrInvalidRange is returned when the requested blob offset or length is invalid. var ErrInvalidRange = errors.Errorf("invalid blob offset or length") +// ErrBlobAlreadyExists is returned when attempting to put a blob that already exists. +var ErrBlobAlreadyExists = errors.New("Blob already exists") + // Bytes encapsulates a sequence of bytes, possibly stored in a non-contiguous buffers, // which can be written sequentially or treated as a io.Reader. type Bytes interface { From b0add00817c39e55806435aa62bbc143f5ce62e4 Mon Sep 17 00:00:00 2001 From: Ali Dowair Date: Mon, 27 Dec 2021 16:18:33 -0500 Subject: [PATCH 03/15] Translate Kopia "recreate-if-exists" into GCS condition --- repo/blob/gcs/gcs_storage.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/repo/blob/gcs/gcs_storage.go b/repo/blob/gcs/gcs_storage.go index 1c6bda700d..639e5ecb53 100644 --- a/repo/blob/gcs/gcs_storage.go +++ b/repo/blob/gcs/gcs_storage.go @@ -98,7 +98,8 @@ func (gcs *gcsStorage) PutBlob(ctx context.Context, b blob.ID, data blob.Bytes, ctx, cancel := context.WithCancel(ctx) - obj := gcs.bucket.Object(gcs.getObjectNameString(b)) + conds := gcsclient.Conditions{DoesNotExist: !opts.RecreateIfExists} + obj := gcs.bucket.Object(gcs.getObjectNameString(b)).If(conds) writer := obj.NewWriter(ctx) writer.ChunkSize = writerChunkSize writer.ContentType = "application/x-kopia" From db5c659c7decb87fd689ec028d21fb694cb1a794 Mon Sep 17 00:00:00 2001 From: Ali Dowair Date: Mon, 27 Dec 2021 18:25:22 -0500 Subject: [PATCH 04/15] Handle GCS PreconditionFailed error --- repo/blob/gcs/gcs_storage.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/repo/blob/gcs/gcs_storage.go b/repo/blob/gcs/gcs_storage.go index 639e5ecb53..193ec64cb1 100644 --- a/repo/blob/gcs/gcs_storage.go +++ b/repo/blob/gcs/gcs_storage.go @@ -76,8 +76,11 @@ func translateError(err error) error { var ae *googleapi.Error if errors.As(err, &ae) { - if ae.Code == http.StatusRequestedRangeNotSatisfiable { + switch ae.Code { + case http.StatusRequestedRangeNotSatisfiable: return blob.ErrInvalidRange + case http.StatusPreconditionFailed: + return blob.ErrBlobAlreadyExists } } From eb0017558c70eb0dbfab24627a82f9626165d38f Mon Sep 17 00:00:00 2001 From: Ali Dowair Date: Wed, 22 Dec 2021 16:46:29 -0500 Subject: [PATCH 05/15] Add DoNotRecreate field in PutOptions struct This will be used to indicate that a PutBlob operation should fail if it is attempting to modify a blob that already exists. We can think of it also as "do not overwrite" or "create only if exists". --- repo/blob/storage.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/repo/blob/storage.go b/repo/blob/storage.go index 9e2230afb1..8098973900 100644 --- a/repo/blob/storage.go +++ b/repo/blob/storage.go @@ -62,9 +62,15 @@ type Reader interface { // PutOptions represents put-options for a single BLOB in a storage. type PutOptions struct { +<<<<<<< HEAD RetentionMode string RetentionPeriod time.Duration RecreateIfExists bool +======= + RetentionMode string + RetentionPeriod time.Duration + DoNotRecreate bool +>>>>>>> f4778d9a (Add DoNotRecreate field in PutOptions struct) } // HasRetentionOptions returns true when blob-retention settings have been From f96ba9a806dc8d5b351a42ad92fabd60daee81e0 Mon Sep 17 00:00:00 2001 From: Ali Dowair Date: Mon, 27 Dec 2021 16:18:33 -0500 Subject: [PATCH 06/15] Translate Kopia "DoNotRecreate" into GCS condition --- repo/blob/gcs/gcs_storage.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/repo/blob/gcs/gcs_storage.go b/repo/blob/gcs/gcs_storage.go index 193ec64cb1..1640ea1914 100644 --- a/repo/blob/gcs/gcs_storage.go +++ b/repo/blob/gcs/gcs_storage.go @@ -101,7 +101,11 @@ func (gcs *gcsStorage) PutBlob(ctx context.Context, b blob.ID, data blob.Bytes, ctx, cancel := context.WithCancel(ctx) +<<<<<<< HEAD conds := gcsclient.Conditions{DoesNotExist: !opts.RecreateIfExists} +======= + conds := gcsclient.Conditions{DoesNotExist: opts.DoNotRecreate} +>>>>>>> 18c5d175 (Translate Kopia "DoNotRecreate" into GCS condition) obj := gcs.bucket.Object(gcs.getObjectNameString(b)).If(conds) writer := obj.NewWriter(ctx) writer.ChunkSize = writerChunkSize From 4a09068b3e18d4308782ec2e31fe36d08bf64188 Mon Sep 17 00:00:00 2001 From: Ali Dowair Date: Tue, 28 Dec 2021 16:12:02 -0500 Subject: [PATCH 07/15] Catch ErrBlobAlreadyExists in VerifyStorage() Expanded the function `VerifyStorage` to test both write paths where DoNotRecreate is true and false. --- internal/blobtesting/verify.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/internal/blobtesting/verify.go b/internal/blobtesting/verify.go index 6a1d04d581..b66caebf6e 100644 --- a/internal/blobtesting/verify.go +++ b/internal/blobtesting/verify.go @@ -108,14 +108,21 @@ func VerifyStorage(ctx context.Context, t *testing.T, r blob.Storage, opts blob. }) t.Run("OverwriteBlobs", func(t *testing.T) { + newContents := []byte{99} + for _, b := range blocks { b := b t.Run(string(b.blk), func(t *testing.T) { t.Parallel() - - require.NoErrorf(t, r.PutBlob(ctx, b.blk, gather.FromSlice(b.contents), blob.PutOptions{}), "can't put blob: %v", b) - AssertGetBlob(ctx, t, r, b.blk, b.contents) + err := r.PutBlob(ctx, b.blk, gather.FromSlice(newContents), opts) + if opts.DoNotRecreate { + require.ErrorIsf(t, err, blob.ErrBlobAlreadyExists, "overwrote blob: %v", b) + AssertGetBlob(ctx, t, r, b.blk, b.contents) + } else { + require.NoErrorf(t, err, "can't put blob: %v", b) + AssertGetBlob(ctx, t, r, b.blk, newContents) + } }) } }) From 0c81ff3dd9bb5b233c542a46577caae5ea8b706c Mon Sep 17 00:00:00 2001 From: Ali Dowair Date: Tue, 28 Dec 2021 16:15:20 -0500 Subject: [PATCH 08/15] Implement positive unit test for GCSStorage --- repo/blob/gcs/gcs_storage_test.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/repo/blob/gcs/gcs_storage_test.go b/repo/blob/gcs/gcs_storage_test.go index 8dee07b2c2..681f0ddefb 100644 --- a/repo/blob/gcs/gcs_storage_test.go +++ b/repo/blob/gcs/gcs_storage_test.go @@ -49,6 +49,23 @@ func TestGCSStorage(t *testing.T) { require.NoError(t, providervalidation.ValidateProvider(ctx, st, blobtesting.TestValidationOptions)) } +func TestGCSStorageBlobRecreateForbidden(t *testing.T) { + t.Parallel() + testutil.ProviderTest(t) + + ctx := testlogging.Context(t) + + st, err := gcs.New(ctx, mustGetOptionsOrSkip(t, uuid.NewString())) + require.NoError(t, err) + + defer st.Close(ctx) + defer blobtesting.CleanupOldData(ctx, t, st, 0) + + blobtesting.VerifyStorage(ctx, t, st, blob.PutOptions{DoNotRecreate: true}) + blobtesting.AssertConnectionInfoRoundTrips(ctx, t, st) + require.NoError(t, providervalidation.ValidateProvider(ctx, st, blobtesting.TestValidationOptions)) +} + func TestGCSStorageInvalid(t *testing.T) { t.Parallel() testutil.ProviderTest(t) From eb6ec3be08363c304c042938a7baeb5cc07775c0 Mon Sep 17 00:00:00 2001 From: Ali Dowair Date: Tue, 28 Dec 2021 16:16:55 -0500 Subject: [PATCH 09/15] Throw error when DoNotRecreate is not supported For all storage types that do not (or do not yet) support idempotent blob creates, throw an explicit error indicating so. --- internal/blobtesting/map.go | 5 ++++- repo/blob/azure/azure_storage.go | 5 ++++- repo/blob/s3/s3_storage.go | 4 ++++ repo/blob/sharded/sharded.go | 5 ++++- 4 files changed, 16 insertions(+), 3 deletions(-) diff --git a/internal/blobtesting/map.go b/internal/blobtesting/map.go index f1ac69b30c..0b0ea6c57d 100644 --- a/internal/blobtesting/map.go +++ b/internal/blobtesting/map.go @@ -76,8 +76,11 @@ func (s *mapStorage) GetMetadata(ctx context.Context, id blob.ID) (blob.Metadata } func (s *mapStorage) PutBlob(ctx context.Context, id blob.ID, data blob.Bytes, opts blob.PutOptions) error { - if opts.HasRetentionOptions() { + switch { + case opts.HasRetentionOptions(): return errors.New("setting blob-retention is not supported") + case opts.DoNotRecreate: + return errors.New("setting blob do-not-recreate is not supported") } s.mutex.Lock() diff --git a/repo/blob/azure/azure_storage.go b/repo/blob/azure/azure_storage.go index e7d1865319..e1726d8ca5 100644 --- a/repo/blob/azure/azure_storage.go +++ b/repo/blob/azure/azure_storage.go @@ -103,8 +103,11 @@ func translateError(err error) error { } func (az *azStorage) PutBlob(ctx context.Context, b blob.ID, data blob.Bytes, opts blob.PutOptions) error { - if opts.HasRetentionOptions() { + switch { + case opts.HasRetentionOptions(): return errors.New("setting blob-retention is not supported") + case opts.DoNotRecreate: + return errors.New("setting blob do-not-recreate is not supported") } ctx, cancel := context.WithCancel(ctx) diff --git a/repo/blob/s3/s3_storage.go b/repo/blob/s3/s3_storage.go index b02fa11388..8014a39463 100644 --- a/repo/blob/s3/s3_storage.go +++ b/repo/blob/s3/s3_storage.go @@ -122,6 +122,10 @@ func (s *s3Storage) getVersionMetadata(ctx context.Context, b blob.ID, version s } func (s *s3Storage) PutBlob(ctx context.Context, b blob.ID, data blob.Bytes, opts blob.PutOptions) error { + if opts.DoNotRecreate { + return errors.New("setting blob do-not-recreate is not supported") + } + _, err := s.putBlob(ctx, b, data, opts) return err diff --git a/repo/blob/sharded/sharded.go b/repo/blob/sharded/sharded.go index 31f10b6736..0c35848c48 100644 --- a/repo/blob/sharded/sharded.go +++ b/repo/blob/sharded/sharded.go @@ -180,8 +180,11 @@ func (s *Storage) GetMetadata(ctx context.Context, blobID blob.ID) (blob.Metadat // PutBlob implements blob.Storage. func (s *Storage) PutBlob(ctx context.Context, blobID blob.ID, data blob.Bytes, opts blob.PutOptions) error { - if opts.HasRetentionOptions() { + switch { + case opts.HasRetentionOptions(): return errors.New("setting blob-retention is not supported") + case opts.DoNotRecreate: + return errors.New("setting blob do-not-recreate is not supported") } dirPath, filePath, err := s.GetShardedPathAndFilePath(ctx, blobID) From bf7ac3add091f25a0c9b0d0d71da063a9516a611 Mon Sep 17 00:00:00 2001 From: Ali Dowair Date: Wed, 29 Dec 2021 16:19:43 -0500 Subject: [PATCH 10/15] Fix building GCS Put condition Was using the old name `RecreateIfExists`, changed to use `DoNotRecreate`. --- repo/blob/gcs/gcs_storage.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/repo/blob/gcs/gcs_storage.go b/repo/blob/gcs/gcs_storage.go index b6e07c6b02..a98998e600 100644 --- a/repo/blob/gcs/gcs_storage.go +++ b/repo/blob/gcs/gcs_storage.go @@ -109,11 +109,7 @@ func (gcs *gcsStorage) PutBlob(ctx context.Context, b blob.ID, data blob.Bytes, ctx, cancel := context.WithCancel(ctx) -<<<<<<< HEAD - conds := gcsclient.Conditions{DoesNotExist: !opts.RecreateIfExists} -======= conds := gcsclient.Conditions{DoesNotExist: opts.DoNotRecreate} ->>>>>>> 18c5d175 (Translate Kopia "DoNotRecreate" into GCS condition) obj := gcs.bucket.Object(gcs.getObjectNameString(b)).If(conds) writer := obj.NewWriter(ctx) writer.ChunkSize = writerChunkSize From 53da15df73e6efe2c2d469c7b9a881a8230aab0b Mon Sep 17 00:00:00 2001 From: Ali Dowair Date: Wed, 29 Dec 2021 16:23:59 -0500 Subject: [PATCH 11/15] Fix whitespace --- repo/blob/s3/s3_storage.go | 12 ++++++------ repo/blob/storage.go | 9 +++++---- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/repo/blob/s3/s3_storage.go b/repo/blob/s3/s3_storage.go index fe06f46c96..bf3ddd32e5 100644 --- a/repo/blob/s3/s3_storage.go +++ b/repo/blob/s3/s3_storage.go @@ -122,12 +122,12 @@ func (s *s3Storage) getVersionMetadata(ctx context.Context, b blob.ID, version s } func (s *s3Storage) PutBlob(ctx context.Context, b blob.ID, data blob.Bytes, opts blob.PutOptions) error { - switch { - case opts.DoNotRecreate: - return errors.New("setting blob do-not-recreate is not supported") - case !opts.SetModTime.IsZero(): - return blob.ErrSetTimeUnsupported - } + switch { + case opts.DoNotRecreate: + return errors.New("setting blob do-not-recreate is not supported") + case !opts.SetModTime.IsZero(): + return blob.ErrSetTimeUnsupported + } _, err := s.putBlob(ctx, b, data, opts) diff --git a/repo/blob/storage.go b/repo/blob/storage.go index fd234e1e84..f23247faac 100644 --- a/repo/blob/storage.go +++ b/repo/blob/storage.go @@ -62,10 +62,11 @@ type Reader interface { // PutOptions represents put-options for a single BLOB in a storage. type PutOptions struct { - RetentionMode string - RetentionPeriod time.Duration - // if true, PutBlob will fail with ErrBlobAlready exists if a blob with the same ID exists. - DoNotRecreate bool + RetentionMode string + RetentionPeriod time.Duration + + // if true, PutBlob will fail with ErrBlobAlready exists if a blob with the same ID exists. + DoNotRecreate bool // if not empty, set the provided timestamp on the blob instead of server-assigned, // if unsupported by the server return ErrSetTimeUnsupported From 7bd0b049a255ce5c548645bd4dad9acc82e28269 Mon Sep 17 00:00:00 2001 From: Ali Dowair Date: Wed, 29 Dec 2021 16:28:07 -0500 Subject: [PATCH 12/15] Put GCS GCS tests w/ different options in one loop --- repo/blob/gcs/gcs_storage_test.go | 23 +++++++---------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/repo/blob/gcs/gcs_storage_test.go b/repo/blob/gcs/gcs_storage_test.go index 681f0ddefb..7a032d3d65 100644 --- a/repo/blob/gcs/gcs_storage_test.go +++ b/repo/blob/gcs/gcs_storage_test.go @@ -44,24 +44,15 @@ func TestGCSStorage(t *testing.T) { defer st.Close(ctx) defer blobtesting.CleanupOldData(ctx, t, st, 0) - blobtesting.VerifyStorage(ctx, t, st, blob.PutOptions{}) - blobtesting.AssertConnectionInfoRoundTrips(ctx, t, st) - require.NoError(t, providervalidation.ValidateProvider(ctx, st, blobtesting.TestValidationOptions)) -} - -func TestGCSStorageBlobRecreateForbidden(t *testing.T) { - t.Parallel() - testutil.ProviderTest(t) - - ctx := testlogging.Context(t) - - st, err := gcs.New(ctx, mustGetOptionsOrSkip(t, uuid.NewString())) - require.NoError(t, err) + options := []blob.PutOptions{ + {}, + {DoNotRecreate: true}, + } - defer st.Close(ctx) - defer blobtesting.CleanupOldData(ctx, t, st, 0) + for _, opt := range options { + blobtesting.VerifyStorage(ctx, t, st, opt) + } - blobtesting.VerifyStorage(ctx, t, st, blob.PutOptions{DoNotRecreate: true}) blobtesting.AssertConnectionInfoRoundTrips(ctx, t, st) require.NoError(t, providervalidation.ValidateProvider(ctx, st, blobtesting.TestValidationOptions)) } From 2737a125b4c1a68ebada33f8cf21996dfec96004 Mon Sep 17 00:00:00 2001 From: Ali Dowair Date: Wed, 29 Dec 2021 16:29:18 -0500 Subject: [PATCH 13/15] Implement negative test in provider validation --- cli/command_repository_validate_provider.go | 1 + internal/blobtesting/verify.go | 15 +++++----- .../providervalidation/providervalidation.go | 28 +++++++++++++------ repo/blob/gcs/gcs_storage_test.go | 4 ++- 4 files changed, 32 insertions(+), 16 deletions(-) diff --git a/cli/command_repository_validate_provider.go b/cli/command_repository_validate_provider.go index dd0f8b4fcf..caf48dbdac 100644 --- a/cli/command_repository_validate_provider.go +++ b/cli/command_repository_validate_provider.go @@ -23,6 +23,7 @@ func (c *commandRepositoryValidateProvider) setup(svc advancedAppServices, paren cmd.Flag("put-blob-workers", "Number of PutBlob workers").IntVar(&c.opt.NumPutBlobWorkers) cmd.Flag("get-blob-workers", "Number of GetBlob workers").IntVar(&c.opt.NumGetBlobWorkers) cmd.Flag("get-metadata-workers", "Number of GetMetadata workers").IntVar(&c.opt.NumGetMetadataWorkers) + cmd.Flag("support-idempotent-creates", "Whether store supports idempotent blob creates").BoolVar(&c.opt.SupportIdempotentCreates) c.out.setup(svc) cmd.Action(c.out.svc.directRepositoryWriteAction(c.run)) diff --git a/internal/blobtesting/verify.go b/internal/blobtesting/verify.go index 6affd88c30..9d8c865100 100644 --- a/internal/blobtesting/verify.go +++ b/internal/blobtesting/verify.go @@ -216,11 +216,12 @@ func AssertConnectionInfoRoundTrips(ctx context.Context, t *testing.T, s blob.St // TestValidationOptions is the set of options used when running providing validation from tests. // nolint:gomnd var TestValidationOptions = providervalidation.Options{ - MaxClockDrift: 3 * time.Minute, - ConcurrencyTestDuration: 15 * time.Second, - NumPutBlobWorkers: 3, - NumGetBlobWorkers: 3, - NumGetMetadataWorkers: 3, - NumListBlobsWorkers: 3, - MaxBlobLength: 10e6, + MaxClockDrift: 3 * time.Minute, + ConcurrencyTestDuration: 15 * time.Second, + NumPutBlobWorkers: 3, + NumGetBlobWorkers: 3, + NumGetMetadataWorkers: 3, + NumListBlobsWorkers: 3, + MaxBlobLength: 10e6, + SupportIdempotentCreates: false, } diff --git a/internal/providervalidation/providervalidation.go b/internal/providervalidation/providervalidation.go index 31c0d41708..4047d226ad 100644 --- a/internal/providervalidation/providervalidation.go +++ b/internal/providervalidation/providervalidation.go @@ -31,18 +31,21 @@ type Options struct { NumGetMetadataWorkers int NumListBlobsWorkers int MaxBlobLength int + + SupportIdempotentCreates bool } // DefaultOptions is the default set of options. // nolint:gomnd var DefaultOptions = Options{ - MaxClockDrift: 3 * time.Minute, - ConcurrencyTestDuration: 30 * time.Second, - NumPutBlobWorkers: 3, - NumGetBlobWorkers: 3, - NumGetMetadataWorkers: 3, - NumListBlobsWorkers: 3, - MaxBlobLength: 10e6, + MaxClockDrift: 3 * time.Minute, + ConcurrencyTestDuration: 30 * time.Second, + NumPutBlobWorkers: 3, + NumGetBlobWorkers: 3, + NumGetMetadataWorkers: 3, + NumListBlobsWorkers: 3, + MaxBlobLength: 10e6, + SupportIdempotentCreates: false, } const blobIDLength = 16 @@ -51,7 +54,7 @@ var log = logging.Module("providervalidation") // ValidateProvider runs a series of tests against provided storage to validate that // it can be used with Kopia. -// nolint:gomnd,funlen,gocyclo +// nolint:gomnd,funlen,gocyclo,cyclop func ValidateProvider(ctx context.Context, st blob.Storage, opt Options) error { if os.Getenv("KOPIA_SKIP_PROVIDER_VALIDATION") != "" { return nil @@ -183,6 +186,15 @@ func ValidateProvider(ctx context.Context, st blob.Storage, opt Options) error { return errors.Wrap(err, "error validating concurrency") } + log(ctx).Infof("Validating blob recreation...") + + if opt.SupportIdempotentCreates { + err := st.PutBlob(ctx, "dummy_id", gather.FromSlice([]byte{99}), blob.PutOptions{DoNotRecreate: true}) + if err == nil { + return errors.New("store should not support put-blob-no-overwrite, expected error") + } + } + log(ctx).Infof("All good.") return nil diff --git a/repo/blob/gcs/gcs_storage_test.go b/repo/blob/gcs/gcs_storage_test.go index 7a032d3d65..10b062797a 100644 --- a/repo/blob/gcs/gcs_storage_test.go +++ b/repo/blob/gcs/gcs_storage_test.go @@ -54,7 +54,9 @@ func TestGCSStorage(t *testing.T) { } blobtesting.AssertConnectionInfoRoundTrips(ctx, t, st) - require.NoError(t, providervalidation.ValidateProvider(ctx, st, blobtesting.TestValidationOptions)) + validateOpts := blobtesting.TestValidationOptions + validateOpts.SupportIdempotentCreates = true + require.NoError(t, providervalidation.ValidateProvider(ctx, st, validateOpts)) } func TestGCSStorageInvalid(t *testing.T) { From a8fdccf3b34adb59b9b3a04a354aa6d69c36235e Mon Sep 17 00:00:00 2001 From: Ali Dowair Date: Wed, 29 Dec 2021 17:00:13 -0500 Subject: [PATCH 14/15] Fix typo in provider validation test --- internal/providervalidation/providervalidation.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/providervalidation/providervalidation.go b/internal/providervalidation/providervalidation.go index 4047d226ad..39d762c38e 100644 --- a/internal/providervalidation/providervalidation.go +++ b/internal/providervalidation/providervalidation.go @@ -188,7 +188,7 @@ func ValidateProvider(ctx context.Context, st blob.Storage, opt Options) error { log(ctx).Infof("Validating blob recreation...") - if opt.SupportIdempotentCreates { + if !opt.SupportIdempotentCreates { err := st.PutBlob(ctx, "dummy_id", gather.FromSlice([]byte{99}), blob.PutOptions{DoNotRecreate: true}) if err == nil { return errors.New("store should not support put-blob-no-overwrite, expected error") From 9ab41389bedf2c9f6038f4bd5dcde2c03afbf1c8 Mon Sep 17 00:00:00 2001 From: Ali Dowair Date: Tue, 11 Jan 2022 08:19:26 -0800 Subject: [PATCH 15/15] Fix comment typo `ErrBlobAlready exists` -> `ErrBlobAlreadyExists` --- repo/blob/storage.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/repo/blob/storage.go b/repo/blob/storage.go index f23247faac..654f807e5f 100644 --- a/repo/blob/storage.go +++ b/repo/blob/storage.go @@ -65,7 +65,7 @@ type PutOptions struct { RetentionMode string RetentionPeriod time.Duration - // if true, PutBlob will fail with ErrBlobAlready exists if a blob with the same ID exists. + // if true, PutBlob will fail with ErrBlobAlreadyExists if a blob with the same ID exists. DoNotRecreate bool // if not empty, set the provided timestamp on the blob instead of server-assigned,