From 6e5a94698e829057b79b598657269240438ae1fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Niewrza=C5=82?= Date: Thu, 14 Apr 2022 13:20:18 +0200 Subject: [PATCH] satellite/metabase: add option to override metadata with CommitObject We were already able to override (or not) metadata with this method but to be explicit we are introducting new option to control storing metadata with object. Separate option should be less error prone. https://github.com/storj/team-metainfo/issues/105 Change-Id: I4c5bce953a633a0009b05c5ca84266ca6ceefc26 --- satellite/accounting/tally/tally_test.go | 6 +-- satellite/metabase/commit.go | 22 +++++--- satellite/metabase/commit_test.go | 50 ++++++++++++++++++- satellite/metabase/copy_object_test.go | 2 + satellite/metabase/iterator_test.go | 4 ++ satellite/metabase/loop_test.go | 1 + satellite/metabase/move_object_test.go | 1 + satellite/metainfo/endpoint_object.go | 1 + .../satellitedb/projectaccounting_test.go | 13 ++--- 9 files changed, 81 insertions(+), 19 deletions(-) diff --git a/satellite/accounting/tally/tally_test.go b/satellite/accounting/tally/tally_test.go index 8d18cb3d4..5da9700b1 100644 --- a/satellite/accounting/tally/tally_test.go +++ b/satellite/accounting/tally/tally_test.go @@ -65,7 +65,7 @@ func TestOnlyInline(t *testing.T) { SatelliteCount: 1, StorageNodeCount: 4, UplinkCount: 1, }, func(t *testing.T, ctx *testcontext.Context, planet *testplanet.Planet) { planet.Satellites[0].Accounting.Tally.Loop.Pause() - uplink := planet.Uplinks[0] + up := planet.Uplinks[0] // Setup: create data for the uplink to upload expectedData := testrand.Bytes(1 * memory.KiB) @@ -81,7 +81,7 @@ func TestOnlyInline(t *testing.T) { expectedBucketName := "testbucket" expectedTally := &accounting.BucketTally{ BucketLocation: metabase.BucketLocation{ - ProjectID: uplink.Projects[0].ID, + ProjectID: up.Projects[0].ID, BucketName: expectedBucketName, }, ObjectCount: 1, @@ -91,7 +91,7 @@ func TestOnlyInline(t *testing.T) { } // Execute test: upload a file, then calculate at rest data - err := uplink.Upload(ctx, planet.Satellites[0], expectedBucketName, "test/path", expectedData) + err := up.Upload(ctx, planet.Satellites[0], expectedBucketName, "test/path", expectedData) assert.NoError(t, err) // run multiple times to ensure we add tallies diff --git a/satellite/metabase/commit.go b/satellite/metabase/commit.go index cbe0d8597..b8f5b3731 100644 --- a/satellite/metabase/commit.go +++ b/satellite/metabase/commit.go @@ -466,6 +466,11 @@ type CommitObject struct { Encryption storj.EncryptionParameters + // this flag controls if we want to set metadata fields with CommitObject + // it's possible to set metadata with BeginObject request so we need to + // be explicit if we would like to set it with CommitObject which will + // override any existing metadata. + OverrideEncryptedMetadata bool EncryptedMetadata []byte // optional EncryptedMetadataNonce []byte // optional EncryptedMetadataEncryptedKey []byte // optional @@ -481,10 +486,12 @@ func (c *CommitObject) Verify() error { return ErrInvalidRequest.New("Encryption.BlockSize is negative or zero") } - if c.EncryptedMetadata == nil && (c.EncryptedMetadataNonce != nil || c.EncryptedMetadataEncryptedKey != nil) { - return ErrInvalidRequest.New("EncryptedMetadataNonce and EncryptedMetadataEncryptedKey must be not set if EncryptedMetadata is not set") - } else if c.EncryptedMetadata != nil && (c.EncryptedMetadataNonce == nil || c.EncryptedMetadataEncryptedKey == nil) { - return ErrInvalidRequest.New("EncryptedMetadataNonce and EncryptedMetadataEncryptedKey must be set if EncryptedMetadata is set") + if c.OverrideEncryptedMetadata { + if c.EncryptedMetadata == nil && (c.EncryptedMetadataNonce != nil || c.EncryptedMetadataEncryptedKey != nil) { + return ErrInvalidRequest.New("EncryptedMetadataNonce and EncryptedMetadataEncryptedKey must be not set if EncryptedMetadata is not set") + } else if c.EncryptedMetadata != nil && (c.EncryptedMetadataNonce == nil || c.EncryptedMetadataEncryptedKey == nil) { + return ErrInvalidRequest.New("EncryptedMetadataNonce and EncryptedMetadataEncryptedKey must be set if EncryptedMetadata is set") + } } return nil } @@ -543,7 +550,7 @@ func (db *DB) CommitObject(ctx context.Context, opts CommitObject) (object Objec encryptionParameters{&opts.Encryption}} metadataColumns := "" - if len(opts.EncryptedMetadata) != 0 { + if opts.OverrideEncryptedMetadata { args = append(args, opts.EncryptedMetadataNonce, opts.EncryptedMetadata, @@ -582,9 +589,11 @@ func (db *DB) CommitObject(ctx context.Context, opts CommitObject) (object Objec status = `+pendingStatus+` RETURNING created_at, expires_at, + encrypted_metadata, encrypted_metadata_encrypted_key, encrypted_metadata_nonce, encryption; `, args...).Scan( &object.CreatedAt, &object.ExpiresAt, + &object.EncryptedMetadata, &object.EncryptedMetadataEncryptedKey, &object.EncryptedMetadataNonce, encryptionParameters{&object.Encryption}, ) if err != nil { @@ -604,9 +613,6 @@ func (db *DB) CommitObject(ctx context.Context, opts CommitObject) (object Objec object.Version = opts.Version object.Status = Committed object.SegmentCount = int32(len(segments)) - object.EncryptedMetadataNonce = opts.EncryptedMetadataNonce - object.EncryptedMetadata = opts.EncryptedMetadata - object.EncryptedMetadataEncryptedKey = opts.EncryptedMetadataEncryptedKey object.TotalPlainSize = totalPlainSize object.TotalEncryptedSize = totalEncryptedSize object.FixedSegmentSize = fixedSegmentSize diff --git a/satellite/metabase/commit_test.go b/satellite/metabase/commit_test.go index 66df75346..a53c7432a 100644 --- a/satellite/metabase/commit_test.go +++ b/satellite/metabase/commit_test.go @@ -2193,7 +2193,8 @@ func TestCommitObject(t *testing.T) { Version: metabase.DefaultVersion, StreamID: obj.StreamID, }, - EncryptedMetadata: testrand.BytesInt(32), + OverrideEncryptedMetadata: true, + EncryptedMetadata: testrand.BytesInt(32), }, ErrClass: &metabase.ErrInvalidRequest, ErrText: "EncryptedMetadataNonce and EncryptedMetadataEncryptedKey must be set if EncryptedMetadata is set", @@ -2208,6 +2209,7 @@ func TestCommitObject(t *testing.T) { Version: metabase.DefaultVersion, StreamID: obj.StreamID, }, + OverrideEncryptedMetadata: true, EncryptedMetadataEncryptedKey: testrand.BytesInt(32), }, ErrClass: &metabase.ErrInvalidRequest, @@ -2267,6 +2269,7 @@ func TestCommitObject(t *testing.T) { Version: 5, StreamID: obj.StreamID, }, + OverrideEncryptedMetadata: true, EncryptedMetadataNonce: encryptedMetadataNonce[:], EncryptedMetadata: encryptedMetadata, EncryptedMetadataEncryptedKey: encryptedMetadataKey, @@ -2660,7 +2663,7 @@ func TestCommitObject(t *testing.T) { EncryptedMetadataEncryptedKey: expectedMetadataKey, EncryptedMetadataNonce: expectedMetadataNonce, }, - Version: 1, + Version: metabase.DefaultVersion, }.Check(ctx, t, db) metabasetest.CommitObject{ @@ -2713,6 +2716,7 @@ func TestCommitObject(t *testing.T) { ObjectStream: obj, Encryption: metabasetest.DefaultEncryption, + OverrideEncryptedMetadata: true, EncryptedMetadata: expectedMetadata, EncryptedMetadataEncryptedKey: expecedMetadataKey, EncryptedMetadataNonce: expecedMetadataNonce, @@ -2735,6 +2739,48 @@ func TestCommitObject(t *testing.T) { }, }.Check(ctx, t, db) }) + + t.Run("commit with empty metadata (overwrite)", func(t *testing.T) { + defer metabasetest.DeleteAll{}.Check(ctx, t, db) + + now := time.Now() + + metabasetest.BeginObjectExactVersion{ + Opts: metabase.BeginObjectExactVersion{ + ObjectStream: obj, + Encryption: metabasetest.DefaultEncryption, + + EncryptedMetadata: testrand.Bytes(memory.KiB), + EncryptedMetadataEncryptedKey: testrand.Bytes(32), + EncryptedMetadataNonce: testrand.Nonce().Bytes(), + }, + Version: 1, + }.Check(ctx, t, db) + + metabasetest.CommitObject{ + Opts: metabase.CommitObject{ + ObjectStream: obj, + Encryption: metabasetest.DefaultEncryption, + + OverrideEncryptedMetadata: true, + EncryptedMetadata: nil, + EncryptedMetadataEncryptedKey: nil, + EncryptedMetadataNonce: nil, + }, + }.Check(ctx, t, db) + + metabasetest.Verify{ + Objects: []metabase.RawObject{ + { + ObjectStream: obj, + CreatedAt: now, + Status: metabase.Committed, + + Encryption: metabasetest.DefaultEncryption, + }, + }, + }.Check(ctx, t, db) + }) }) } diff --git a/satellite/metabase/copy_object_test.go b/satellite/metabase/copy_object_test.go index c6d123315..345fb67e6 100644 --- a/satellite/metabase/copy_object_test.go +++ b/satellite/metabase/copy_object_test.go @@ -58,6 +58,7 @@ func TestBeginCopyObject(t *testing.T) { expectedObject, _ := metabasetest.CreateTestObject{ CommitObject: &metabase.CommitObject{ ObjectStream: obj, + OverrideEncryptedMetadata: true, EncryptedMetadata: testrand.Bytes(64), EncryptedMetadataNonce: expectedMetadataNonce[:], EncryptedMetadataEncryptedKey: expectedMetadataKey, @@ -534,6 +535,7 @@ func TestFinishCopyObject(t *testing.T) { originalObj, _ := metabasetest.CreateTestObject{ CommitObject: &metabase.CommitObject{ ObjectStream: obj, + OverrideEncryptedMetadata: true, EncryptedMetadata: originalMetadata, EncryptedMetadataNonce: originalMetadataNonce, EncryptedMetadataEncryptedKey: originalMetadataEncryptedKey, diff --git a/satellite/metabase/iterator_test.go b/satellite/metabase/iterator_test.go index 3b44c8078..2d9a97bfb 100644 --- a/satellite/metabase/iterator_test.go +++ b/satellite/metabase/iterator_test.go @@ -107,6 +107,7 @@ func TestIterateObjects(t *testing.T) { metabasetest.CommitObject{ Opts: metabase.CommitObject{ ObjectStream: committed, + OverrideEncryptedMetadata: true, EncryptedMetadataNonce: encryptedMetadataNonce[:], EncryptedMetadata: encryptedMetadata, EncryptedMetadataEncryptedKey: encryptedMetadataKey, @@ -568,6 +569,7 @@ func TestIterateObjectsWithStatus(t *testing.T) { metabasetest.CommitObject{ Opts: metabase.CommitObject{ ObjectStream: committed, + OverrideEncryptedMetadata: true, EncryptedMetadataNonce: encryptedMetadataNonce[:], EncryptedMetadata: encryptedMetadata, EncryptedMetadataEncryptedKey: encryptedMetadataKey, @@ -1153,6 +1155,7 @@ func TestIterateObjectsWithStatus(t *testing.T) { CommitObject: &metabase.CommitObject{ ObjectStream: obj1, Encryption: metabasetest.DefaultEncryption, + OverrideEncryptedMetadata: true, EncryptedMetadata: []byte{3}, EncryptedMetadataEncryptedKey: []byte{4}, EncryptedMetadataNonce: []byte{5}, @@ -1219,6 +1222,7 @@ func TestIterateObjectsWithStatus(t *testing.T) { CommitObject: &metabase.CommitObject{ ObjectStream: obj1, Encryption: metabasetest.DefaultEncryption, + OverrideEncryptedMetadata: true, EncryptedMetadata: []byte{3}, EncryptedMetadataEncryptedKey: []byte{4}, EncryptedMetadataNonce: []byte{5}, diff --git a/satellite/metabase/loop_test.go b/satellite/metabase/loop_test.go index 809ede6b3..e72e030ca 100644 --- a/satellite/metabase/loop_test.go +++ b/satellite/metabase/loop_test.go @@ -89,6 +89,7 @@ func TestIterateLoopObjects(t *testing.T) { metabasetest.CommitObject{ Opts: metabase.CommitObject{ ObjectStream: committed, + OverrideEncryptedMetadata: true, EncryptedMetadataNonce: encryptedMetadataNonce[:], EncryptedMetadata: encryptedMetadata, EncryptedMetadataEncryptedKey: encryptedMetadataKey, diff --git a/satellite/metabase/move_object_test.go b/satellite/metabase/move_object_test.go index 9f8e31f5c..6cdd49bee 100644 --- a/satellite/metabase/move_object_test.go +++ b/satellite/metabase/move_object_test.go @@ -56,6 +56,7 @@ func TestBeginMoveObject(t *testing.T) { expectedObject, _ := metabasetest.CreateTestObject{ CommitObject: &metabase.CommitObject{ ObjectStream: obj, + OverrideEncryptedMetadata: true, EncryptedMetadata: testrand.Bytes(64), EncryptedMetadataNonce: expectedMetadataNonce[:], EncryptedMetadataEncryptedKey: expectedMetadataKey, diff --git a/satellite/metainfo/endpoint_object.go b/satellite/metainfo/endpoint_object.go index b51020c96..018e4112c 100644 --- a/satellite/metainfo/endpoint_object.go +++ b/satellite/metainfo/endpoint_object.go @@ -240,6 +240,7 @@ func (endpoint *Endpoint) CommitObject(ctx context.Context, req *pb.ObjectCommit // we need to fix it on uplink side but that part will be // needed for backward compatibility if len(req.EncryptedMetadata) != 0 { + request.OverrideEncryptedMetadata = true request.EncryptedMetadata = req.EncryptedMetadata request.EncryptedMetadataNonce = req.EncryptedMetadataNonce[:] request.EncryptedMetadataEncryptedKey = req.EncryptedMetadataEncryptedKey diff --git a/satellite/satellitedb/projectaccounting_test.go b/satellite/satellitedb/projectaccounting_test.go index aaa0c2c2c..19435a6a4 100644 --- a/satellite/satellitedb/projectaccounting_test.go +++ b/satellite/satellitedb/projectaccounting_test.go @@ -96,8 +96,8 @@ func Test_GetSingleBucketRollup(t *testing.T) { var ( satelliteSys = planet.Satellites[0] - uplink = planet.Uplinks[0] - projectID = uplink.Projects[0].ID + upl = planet.Uplinks[0] + projectID = upl.Projects[0].ID ) newUser := console.CreateUser{ @@ -136,12 +136,13 @@ func Test_GetSingleBucketRollup(t *testing.T) { firstSegment := testrand.Bytes(100 * memory.KiB) secondSegment := testrand.Bytes(200 * memory.KiB) - err = uplink.Upload(ctx, satelliteSys, bucketName, firstPath, firstSegment) - require.NoError(t, err) - err = uplink.Upload(ctx, satelliteSys, bucketName, secondPath, secondSegment) + err = upl.Upload(ctx, satelliteSys, bucketName, firstPath, firstSegment) require.NoError(t, err) - _, err = uplink.Download(ctx, satelliteSys, bucketName, firstPath) + err = upl.Upload(ctx, satelliteSys, bucketName, secondPath, secondSegment) + require.NoError(t, err) + + _, err = upl.Download(ctx, satelliteSys, bucketName, firstPath) require.NoError(t, err) require.NoError(t, planet.WaitForStorageNodeEndpoints(ctx))