diff --git a/cmd/bucket-replication.go b/cmd/bucket-replication.go index a9c62ac72..69ec796ab 100644 --- a/cmd/bucket-replication.go +++ b/cmd/bucket-replication.go @@ -186,6 +186,7 @@ func checkReplicateDelete(ctx context.Context, bucket string, dobj ObjectToDelet UserTags: oi.UserTags, DeleteMarker: oi.DeleteMarker, VersionID: dobj.VersionID, + OpType: replication.DeleteReplicationType, } replicate = rcfg.Replicate(opts) // when incoming delete is removal of a delete marker( a.k.a versioned delete), diff --git a/pkg/bucket/replication/destination.go b/pkg/bucket/replication/destination.go index 3b63c1eb9..c115806f8 100644 --- a/pkg/bucket/replication/destination.go +++ b/pkg/bucket/replication/destination.go @@ -84,7 +84,7 @@ func (d *Destination) UnmarshalXML(dec *xml.Decoder, start xml.StartElement) (er switch dest.StorageClass { case "STANDARD", "REDUCED_REDUNDANCY": default: - return fmt.Errorf("unknown storage class %v", dest.StorageClass) + return fmt.Errorf("unknown storage class %s", dest.StorageClass) } } parsedDest.StorageClass = dest.StorageClass @@ -107,7 +107,7 @@ func (d Destination) Validate(bucketName string) error { // parseDestination - parses string to Destination. func parseDestination(s string) (Destination, error) { if !strings.HasPrefix(s, DestinationARNPrefix) { - return Destination{}, Errorf("invalid destination '%v'", s) + return Destination{}, Errorf("invalid destination '%s'", s) } bucketName := strings.TrimPrefix(s, DestinationARNPrefix) diff --git a/pkg/bucket/replication/replication.go b/pkg/bucket/replication/replication.go index 7336e230c..d6fd87ad2 100644 --- a/pkg/bucket/replication/replication.go +++ b/pkg/bucket/replication/replication.go @@ -114,6 +114,16 @@ func (c Config) Validate(bucket string, sameTarget bool) error { return nil } +// Type - replication type enum +type Type int + +// Types of replication +const ( + ObjectReplicationType Type = 1 + iota + DeleteReplicationType + MetadataReplicationType +) + // ObjectOpts provides information to deduce whether replication // can be triggered on the resultant object. type ObjectOpts struct { @@ -123,6 +133,7 @@ type ObjectOpts struct { IsLatest bool DeleteMarker bool SSEC bool + OpType Type } // FilterActionableRules returns the rules actions that need to be executed @@ -159,24 +170,24 @@ func (c Config) GetDestination() Destination { // Replicate returns true if the object should be replicated. func (c Config) Replicate(obj ObjectOpts) bool { - + if obj.SSEC { + return false + } for _, rule := range c.FilterActionableRules(obj) { - // check MinIO extension for versioned deletes - if !obj.DeleteMarker && obj.VersionID != "" && rule.DeleteReplication.Status == Disabled { - return false - } - if obj.DeleteMarker && rule.DeleteMarkerReplication.Status == Disabled { - // Indicates whether MinIO will remove a delete marker. By default, delete markers - // are not replicated. - return false - } - if obj.SSEC { - return false - } if rule.Status == Disabled { continue } - return true + if obj.OpType == DeleteReplicationType { + switch { + case obj.VersionID != "": + // // check MinIO extension for versioned deletes + return rule.DeleteReplication.Status == Enabled + default: + return rule.DeleteMarkerReplication.Status == Enabled + } + } else { // regular object/metadata replication + return true + } } return false } @@ -198,8 +209,9 @@ func (c Config) HasActiveRules(prefix string, recursive bool) bool { if !recursive && !strings.HasPrefix(prefix, rule.Filter.Prefix) { continue } - // If recursive, we can skip this rule if it doesn't match the tested prefix. - if recursive && !strings.HasPrefix(rule.Filter.Prefix, prefix) { + // If recursive, we can skip this rule if it doesn't match the tested prefix or level below prefix + // does not match + if recursive && !strings.HasPrefix(rule.Prefix(), prefix) && !strings.HasPrefix(prefix, rule.Prefix()) { continue } } diff --git a/pkg/bucket/replication/replication_test.go b/pkg/bucket/replication/replication_test.go new file mode 100644 index 000000000..144907d7b --- /dev/null +++ b/pkg/bucket/replication/replication_test.go @@ -0,0 +1,305 @@ +package replication + +import ( + "bytes" + "fmt" + "testing" +) + +func TestParseAndValidateReplicationConfig(t *testing.T) { + testCases := []struct { + inputConfig string + expectedParsingErr error + expectedValidationErr error + destBucket string + sameTarget bool + }{ + { //1 Invalid delete marker status in replication config + inputConfig: `arn:aws:iam::AcctID:role/role-nameEnabledstringkey-prefixarn:aws:s3:::destinationbucket`, + destBucket: "destinationbucket", + sameTarget: false, + expectedParsingErr: nil, + expectedValidationErr: errInvalidDeleteMarkerReplicationStatus, + }, + //2 Invalid delete replication status in replication config + {inputConfig: `arn:aws:iam::AcctID:role/role-nameEnabledDisabledkey-prefixarn:aws:s3:::destinationbucket`, + destBucket: "destinationbucket", + sameTarget: false, + expectedParsingErr: nil, + expectedValidationErr: errDeleteReplicationMissing, + }, + //3 valid replication config + {inputConfig: `arn:aws:iam::AcctID:role/role-nameEnabledDisabledDisabledkey-prefixarn:aws:s3:::destinationbucket`, + destBucket: "destinationbucket", + sameTarget: false, + expectedParsingErr: nil, + expectedValidationErr: nil, + }, + //4 missing role in config + {inputConfig: `EnabledDisabledDisabledkey-prefixarn:aws:s3:::destinationbucket`, + // destination bucket in config different from bucket specified + destBucket: "destinationbucket", + sameTarget: false, + expectedParsingErr: nil, + expectedValidationErr: errRoleArnMissing, + }, + //5 replication destination in different rules not identical + {inputConfig: `arn:aws:iam::AcctID:role/role-nameEnabledDisabledDisabledkey-prefixarn:aws:s3:::destinationbucketEnabled3DisabledDisabledkey-prefixarn:aws:s3:::destinationbucket2`, + destBucket: "destinationbucket", + sameTarget: false, + expectedParsingErr: nil, + expectedValidationErr: errReplicationDestinationMismatch, + }, + //6 missing rule status in replication config + {inputConfig: `arn:aws:iam::AcctID:role/role-nameDisabledDisabledkey-prefixarn:aws:s3:::destinationbucket`, + destBucket: "destinationbucket", + sameTarget: false, + expectedParsingErr: nil, + expectedValidationErr: errEmptyRuleStatus, + }, + //7 invalid rule status in replication config + {inputConfig: `arn:aws:iam::AcctID:role/role-nameEnssabledDisabledDisabledkey-prefixarn:aws:s3:::destinationbucketEnabledDisabledDisabledkey-prefixarn:aws:s3:::destinationbucket`, + destBucket: "destinationbucket", + sameTarget: false, + expectedParsingErr: nil, + expectedValidationErr: errInvalidRuleStatus, + }, + //8 invalid rule id exceeds length allowed in replication config + {inputConfig: `arn:aws:iam::AcctID:role/role-namevsUVERgOc8zZYagLSzSa5lE8qeI6nh1lyLNS4R9W052yfecrhhepGboswSWMMNO8CPcXM4GM3nKyQ72EadlMzzZBFoYWKn7ju5GoE5w9c57a0piHR1vexpdd9FrMquiruvAJ0MTGVupm0EegMVxoIOdjx7VgZhGrmi2XDvpVEFT7WmYMA9fSK297XkTHWyECaNHBySJ1Qp4vwX8tPNauKpfHx4kzUpnKe1PZbptGMWbY5qTcwlNuMhVSmgFffShqEnabledDisabledDisabledkey-prefixarn:aws:s3:::destinationbucket`, + destBucket: "destinationbucket", + sameTarget: false, + expectedParsingErr: nil, + expectedValidationErr: errInvalidRuleID, + }, + //9 invalid priority status in replication config + {inputConfig: `arn:aws:iam::AcctID:role/role-nameEnabledDisabledDisabledkey-prefixarn:aws:s3:::destinationbucketEnabledDisabledDisabledkey-prefixarn:aws:s3:::destinationbucket`, + destBucket: "destinationbucket", + sameTarget: false, + expectedParsingErr: nil, + expectedValidationErr: errReplicationUniquePriority, + }, + //10 no rule in replication config + {inputConfig: `arn:aws:iam::AcctID:role/role-name`, + destBucket: "destinationbucket", + sameTarget: false, + expectedParsingErr: nil, + expectedValidationErr: errReplicationNoRule, + }, + //11 no destination in replication config + {inputConfig: `arn:aws:iam::AcctID:role/role-nameEnabledDisabledDisabledkey-prefix`, + destBucket: "destinationbucket", + sameTarget: false, + expectedParsingErr: Errorf("invalid destination '%v'", ""), + expectedValidationErr: nil, + }, + //12 destination not matching ARN in replication config + {inputConfig: `arn:aws:iam::AcctID:role/role-nameEnabledDisabledDisabledkey-prefixdestinationbucket2`, + destBucket: "destinationbucket", + sameTarget: false, + expectedParsingErr: fmt.Errorf("invalid destination '%v'", "destinationbucket2"), + expectedValidationErr: nil, + }, + } + for i, tc := range testCases { + t.Run(fmt.Sprintf("Test %d", i+1), func(t *testing.T) { + cfg, err := ParseConfig(bytes.NewReader([]byte(tc.inputConfig))) + if err != nil && tc.expectedParsingErr != nil && err.Error() != tc.expectedParsingErr.Error() { + t.Fatalf("%d: Expected '%v' during parsing but got '%v'", i+1, tc.expectedParsingErr, err) + } + if err == nil && tc.expectedParsingErr != nil { + t.Fatalf("%d: Expected '%v' during parsing but got '%v'", i+1, tc.expectedParsingErr, err) + } + if tc.expectedParsingErr != nil { + // We already expect a parsing error, + // no need to continue this test. + return + } + err = cfg.Validate(tc.destBucket, tc.sameTarget) + if err != tc.expectedValidationErr { + t.Fatalf("%d: Expected %v during parsing but got %v", i+1, tc.expectedValidationErr, err) + } + }) + } +} +func TestReplicate(t *testing.T) { + cfgs := []Config{ + { //Config0 - Replication config has no filters, all replication enabled + Rules: []Rule{ + { + Status: Enabled, + Priority: 3, + DeleteMarkerReplication: DeleteMarkerReplication{Status: Enabled}, + DeleteReplication: DeleteReplication{Status: Enabled}, + Filter: Filter{}, + }, + }, + }, + { //Config1 - Replication config has no filters, delete,delete-marker replication disabled + Rules: []Rule{ + { + Status: Enabled, + Priority: 3, + DeleteMarkerReplication: DeleteMarkerReplication{Status: Disabled}, + DeleteReplication: DeleteReplication{Status: Disabled}, + Filter: Filter{}, + }, + }, + }, + { //Config2 - Replication config has filters and more than 1 matching rule, delete,delete-marker replication disabled + Rules: []Rule{ + { + Status: Enabled, + Priority: 2, + DeleteMarkerReplication: DeleteMarkerReplication{Status: Disabled}, + DeleteReplication: DeleteReplication{Status: Enabled}, + Filter: Filter{Prefix: "xy", And: And{}, Tag: Tag{Key: "k1", Value: "v1"}}, + }, + { + Status: Enabled, + Priority: 1, + DeleteMarkerReplication: DeleteMarkerReplication{Status: Enabled}, + DeleteReplication: DeleteReplication{Status: Disabled}, + Filter: Filter{Prefix: "xyz"}, + }, + }, + }, + { //Config3 - Replication config has filters and no overlapping rules + Rules: []Rule{ + { + Status: Enabled, + Priority: 2, + DeleteMarkerReplication: DeleteMarkerReplication{Status: Disabled}, + DeleteReplication: DeleteReplication{Status: Enabled}, + Filter: Filter{Prefix: "xy", And: And{}, Tag: Tag{Key: "k1", Value: "v1"}}, + }, + { + Status: Enabled, + Priority: 1, + DeleteMarkerReplication: DeleteMarkerReplication{Status: Enabled}, + DeleteReplication: DeleteReplication{Status: Disabled}, + Filter: Filter{Prefix: "abc"}, + }, + }, + }, + } + testCases := []struct { + opts ObjectOpts + c Config + expectedResult bool + }{ + // using config 1 - no filters, all replication enabled + {ObjectOpts{}, cfgs[0], false}, //1. invalid ObjectOpts missing object name + {ObjectOpts{Name: "c1test"}, cfgs[0], true}, //2. valid ObjectOpts passing empty Filter + {ObjectOpts{Name: "c1test", VersionID: "vid"}, cfgs[0], true}, //3. valid ObjectOpts passing empty Filter + + {ObjectOpts{Name: "c1test", DeleteMarker: true, OpType: DeleteReplicationType}, cfgs[0], true}, //4. DeleteMarker version replication valid case - matches DeleteMarkerReplication status + {ObjectOpts{Name: "c1test", VersionID: "vid", OpType: DeleteReplicationType}, cfgs[0], true}, //5. permanent delete of version, matches DeleteReplication status - valid case + {ObjectOpts{Name: "c1test", VersionID: "vid", DeleteMarker: true, OpType: DeleteReplicationType}, cfgs[0], true}, //6. permanent delete of version, matches DeleteReplication status + {ObjectOpts{Name: "c1test", VersionID: "vid", DeleteMarker: true, SSEC: true, OpType: DeleteReplicationType}, cfgs[0], false}, //7. permanent delete of version, disqualified by SSE-C + {ObjectOpts{Name: "c1test", DeleteMarker: true, SSEC: true, OpType: DeleteReplicationType}, cfgs[0], false}, //8. setting DeleteMarker on SSE-C encrypted object, disqualified by SSE-C + {ObjectOpts{Name: "c1test", SSEC: true}, cfgs[0], false}, //9. replication of SSE-C encrypted object, disqualified + + // using config 2 - no filters, only replication of object, metadata enabled + {ObjectOpts{Name: "c2test"}, cfgs[1], true}, //10. valid ObjectOpts passing empty Filter + {ObjectOpts{Name: "c2test", DeleteMarker: true, OpType: DeleteReplicationType}, cfgs[1], false}, //11. DeleteMarker version replication not allowed due to DeleteMarkerReplication status + {ObjectOpts{Name: "c2test", VersionID: "vid", OpType: DeleteReplicationType}, cfgs[1], false}, //12. permanent delete of version, disallowed by DeleteReplication status + {ObjectOpts{Name: "c2test", VersionID: "vid", DeleteMarker: true, OpType: DeleteReplicationType}, cfgs[1], false}, //13. permanent delete of DeleteMarker version, disallowed by DeleteReplication status + {ObjectOpts{Name: "c2test", VersionID: "vid", DeleteMarker: true, SSEC: true, OpType: DeleteReplicationType}, cfgs[1], false}, //14. permanent delete of version, disqualified by SSE-C & DeleteReplication status + {ObjectOpts{Name: "c2test", DeleteMarker: true, SSEC: true, OpType: DeleteReplicationType}, cfgs[1], false}, //15. setting DeleteMarker on SSE-C encrypted object, disqualified by SSE-C & DeleteMarkerReplication status + {ObjectOpts{Name: "c2test", SSEC: true}, cfgs[1], false}, //16. replication of SSE-C encrypted object, disqualified by default + // using config 2 - has more than one rule with overlapping prefixes + {ObjectOpts{Name: "xy/c3test", UserTags: "k1=v1"}, cfgs[2], true}, //17. matches rule 1 for replication of content/metadata + {ObjectOpts{Name: "xyz/c3test", UserTags: "k1=v1"}, cfgs[2], true}, //18. matches rule 1 for replication of content/metadata + {ObjectOpts{Name: "xyz/c3test", UserTags: "k1=v1", DeleteMarker: true, OpType: DeleteReplicationType}, cfgs[2], false}, //19. matches rule 1 - DeleteMarker replication disallowed by rule + {ObjectOpts{Name: "xyz/c3test", UserTags: "k1=v1", DeleteMarker: true, VersionID: "vid", OpType: DeleteReplicationType}, cfgs[2], true}, //20. matches rule 1 - DeleteReplication allowed by rule for permanent delete of DeleteMarker + {ObjectOpts{Name: "xyz/c3test", UserTags: "k1=v1", VersionID: "vid", OpType: DeleteReplicationType}, cfgs[2], true}, //21. matches rule 1 - DeleteReplication allowed by rule for permanent delete of version + {ObjectOpts{Name: "xyz/c3test"}, cfgs[2], true}, //22. matches rule 2 for replication of content/metadata + {ObjectOpts{Name: "xy/c3test", UserTags: "k1=v2"}, cfgs[2], false}, //23. does not match rule1 because tag value does not pass filter + {ObjectOpts{Name: "xyz/c3test", DeleteMarker: true, OpType: DeleteReplicationType}, cfgs[2], true}, //24. matches rule 2 - DeleteMarker replication allowed by rule + {ObjectOpts{Name: "xyz/c3test", DeleteMarker: true, VersionID: "vid", OpType: DeleteReplicationType}, cfgs[2], false}, //25. matches rule 2 - DeleteReplication disallowed by rule for permanent delete of DeleteMarker + {ObjectOpts{Name: "xyz/c3test", VersionID: "vid", OpType: DeleteReplicationType}, cfgs[2], false}, //26. matches rule 1 - DeleteReplication disallowed by rule for permanent delete of version + {ObjectOpts{Name: "abc/c3test"}, cfgs[2], false}, //27. matches no rule because object prefix does not match + + // using config 3 - has no overlapping rules + {ObjectOpts{Name: "xy/c4test", UserTags: "k1=v1"}, cfgs[3], true}, //28. matches rule 1 for replication of content/metadata + {ObjectOpts{Name: "xa/c4test", UserTags: "k1=v1"}, cfgs[3], false}, //29. no rule match object prefix not in rules + {ObjectOpts{Name: "xyz/c4test", DeleteMarker: true, OpType: DeleteReplicationType}, cfgs[3], false}, //30. rule 1 not matched because of tags filter + {ObjectOpts{Name: "xyz/c4test", UserTags: "k1=v1", DeleteMarker: true, OpType: DeleteReplicationType}, cfgs[3], false}, //31. matches rule 1 - DeleteMarker replication disallowed by rule + {ObjectOpts{Name: "xyz/c4test", UserTags: "k1=v1", DeleteMarker: true, VersionID: "vid", OpType: DeleteReplicationType}, cfgs[3], true}, //32. matches rule 1 - DeleteReplication allowed by rule for permanent delete of DeleteMarker + {ObjectOpts{Name: "xyz/c4test", UserTags: "k1=v1", VersionID: "vid", OpType: DeleteReplicationType}, cfgs[3], true}, //33. matches rule 1 - DeleteReplication allowed by rule for permanent delete of version + {ObjectOpts{Name: "abc/c4test"}, cfgs[3], true}, //34. matches rule 2 for replication of content/metadata + {ObjectOpts{Name: "abc/c4test", UserTags: "k1=v2"}, cfgs[3], true}, //35. matches rule 2 for replication of content/metadata + {ObjectOpts{Name: "abc/c4test", DeleteMarker: true, OpType: DeleteReplicationType}, cfgs[3], true}, //36. matches rule 2 - DeleteMarker replication allowed by rule + {ObjectOpts{Name: "abc/c4test", DeleteMarker: true, VersionID: "vid", OpType: DeleteReplicationType}, cfgs[3], false}, //37. matches rule 2 - DeleteReplication disallowed by rule for permanent delete of DeleteMarker + {ObjectOpts{Name: "abc/c4test", VersionID: "vid", OpType: DeleteReplicationType}, cfgs[3], false}, //38. matches rule 2 - DeleteReplication disallowed by rule for permanent delete of version + + } + + for i, testCase := range testCases { + result := testCase.c.Replicate(testCase.opts) + + if result != testCase.expectedResult { + t.Fatalf("case %v: expected: %v, got: %v", i+1, testCase.expectedResult, result) + } + } +} + +func TestHasActiveRules(t *testing.T) { + testCases := []struct { + inputConfig string + prefix string + expectedNonRec bool + expectedRec bool + }{ + + // case 1 - only one rule which is in Disabled status + {inputConfig: `arn:aws:iam::AcctID:role/role-nameDisabledDisabledDisabledkey-prefixarn:aws:s3:::destinationbucket`, + prefix: "miss/prefix", + expectedNonRec: false, + expectedRec: false, + }, + // case 2 - only one rule which matches prefix filter + {inputConfig: `arn:aws:iam::AcctID:role/role-nameEnabledDisabledDisabledkey/prefixarn:aws:s3:::destinationbucket`, + prefix: "key/prefix1", + expectedNonRec: true, + expectedRec: true, + }, + // case 3 - empty prefix + {inputConfig: `arn:aws:iam::AcctID:role/role-nameEnabledDisabledDisabledarn:aws:s3:::destinationbucket`, + prefix: "key-prefix", + expectedNonRec: true, + expectedRec: true, + }, + // case 4 - has Filter based on prefix + {inputConfig: `arn:aws:iam::AcctID:role/role-nameEnabledDisabledDisabledtestdir/dir1/arn:aws:s3:::destinationbucket`, + prefix: "testdir/", + expectedNonRec: false, + expectedRec: true, + }, + //case 5 - has filter with prefix and tags, here we are not matching on tags + {inputConfig: `arn:aws:iam::AcctID:role/role-nameEnabledDisabledDisabled + key-prefixkey1value1key2value2arn:aws:s3:::destinationbucket`, + prefix: "testdir/", + expectedNonRec: true, + expectedRec: true, + }, + } + + for i, tc := range testCases { + tc := tc + t.Run(fmt.Sprintf("Test_%d", i+1), func(t *testing.T) { + cfg, err := ParseConfig(bytes.NewReader([]byte(tc.inputConfig))) + if err != nil { + t.Fatalf("Got unexpected error: %v", err) + } + if got := cfg.HasActiveRules(tc.prefix, false); got != tc.expectedNonRec { + t.Fatalf("Expected result with recursive set to false: `%v`, got: `%v`", tc.expectedNonRec, got) + } + if got := cfg.HasActiveRules(tc.prefix, true); got != tc.expectedRec { + t.Fatalf("Expected result with recursive set to true: `%v`, got: `%v`", tc.expectedRec, got) + } + + }) + + } +} diff --git a/pkg/bucket/replication/rule.go b/pkg/bucket/replication/rule.go index a96f53746..7a4d9a068 100644 --- a/pkg/bucket/replication/rule.go +++ b/pkg/bucket/replication/rule.go @@ -108,7 +108,7 @@ var ( errInvalidRuleStatus = Errorf("Status must be set to either Enabled or Disabled") errDeleteMarkerReplicationMissing = Errorf("DeleteMarkerReplication must be specified") errPriorityMissing = Errorf("Priority must be specified") - errInvalidDeleteMarkerReplicationStatus = Errorf("Delete marker replication is currently not supported") + errInvalidDeleteMarkerReplicationStatus = Errorf("Delete marker replication status is invalid") errDestinationSourceIdentical = Errorf("Destination bucket cannot be the same as the source bucket.") errDeleteReplicationMissing = Errorf("Delete replication must be specified") errInvalidDeleteReplicationStatus = Errorf("Delete replication is either enable|disable")