Skip to content

Commit

Permalink
Merge branch 'master' into imdsv2-support
Browse files Browse the repository at this point in the history
  • Loading branch information
michaelfoley1 authored May 17, 2021
2 parents 850208c + f0518ca commit 5b1664c
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 33 deletions.
2 changes: 1 addition & 1 deletion api-bucket-policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func (c Client) putBucketPolicy(ctx context.Context, bucketName, policy string)
return err
}
if resp != nil {
if resp.StatusCode != http.StatusNoContent {
if resp.StatusCode != http.StatusNoContent && resp.StatusCode != http.StatusOK {
return httpRespToErrorResponse(resp, bucketName, "")
}
}
Expand Down
2 changes: 1 addition & 1 deletion docs/API.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ import (
"fmt"

"github.com/minio/minio-go/v7"
"github.com/minio/minio-go/v7/credentials"
"github.com/minio/minio-go/v7/pkg/credentials"
)

func main() {
Expand Down
6 changes: 3 additions & 3 deletions pkg/lifecycle/lifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,9 @@ func (t Transition) MarshalXML(en *xml.Encoder, startElement xml.StartElement) e

// And And Rule for LifecycleTag, to be used in LifecycleRuleFilter
type And struct {
XMLName xml.Name `xml:"And,omitempty" json:"-"`
Prefix string `xml:"Prefix,omitempty" json:"Prefix,omitempty"`
Tags []Tag `xml:"Tag,omitempty" json:"Tags,omitempty"`
XMLName xml.Name `xml:"And" json:"-"`
Prefix string `xml:"Prefix" json:"Prefix,omitempty"`
Tags []Tag `xml:"Tag" json:"Tags,omitempty"`
}

// IsEmpty returns true if Tags field is null
Expand Down
60 changes: 41 additions & 19 deletions pkg/replication/replication.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (
"github.com/rs/xid"
)

var errInvalidFilter = fmt.Errorf("Invalid filter")
var errInvalidFilter = fmt.Errorf("invalid filter")

// OptionType specifies operation to be performed on config
type OptionType string
Expand Down Expand Up @@ -59,6 +59,7 @@ type Options struct {
IsSCSet bool
ReplicateDeletes string // replicate versioned deletes
ReplicateDeleteMarkers string // replicate soft deletes
ReplicaSync string // replicate replica metadata modifications
}

// Tags returns a slice of tags for a rule
Expand All @@ -71,7 +72,7 @@ func (opts Options) Tags() ([]Tag, error) {
}
kv := strings.SplitN(tok, "=", 2)
if len(kv) != 2 {
return []Tag{}, fmt.Errorf("Tags should be entered as comma separated k=v pairs")
return []Tag{}, fmt.Errorf("tags should be entered as comma separated k=v pairs")
}
tagList = append(tagList, Tag{
Key: kv[0],
Expand Down Expand Up @@ -102,7 +103,7 @@ func (c *Config) AddRule(opts Options) error {
return err
}
if opts.RoleArn != c.Role && c.Role != "" {
return fmt.Errorf("Role ARN does not match existing configuration")
return fmt.Errorf("role ARN does not match existing configuration")
}
var status Status
// toggle rule status for edit option
Expand All @@ -112,7 +113,7 @@ func (c *Config) AddRule(opts Options) error {
case "disable":
status = Disabled
default:
return fmt.Errorf("Rule state should be either [enable|disable]")
return fmt.Errorf("rule state should be either [enable|disable]")
}

tags, err := opts.Tags()
Expand Down Expand Up @@ -142,7 +143,7 @@ func (c *Config) AddRule(opts Options) error {
arnStr = c.Role
}
if arnStr == "" {
return fmt.Errorf("Role ARN required")
return fmt.Errorf("role ARN required")
}
tokens := strings.Split(arnStr, ":")
if len(tokens) != 6 {
Expand Down Expand Up @@ -183,6 +184,16 @@ func (c *Config) AddRule(opts Options) error {
return fmt.Errorf("ReplicateDeletes should be either enable|disable")
}
}
var replicaSync Status
// replica sync is by default Enabled, unless specified.
switch opts.ReplicaSync {
case "enable", "":
replicaSync = Enabled
case "disable":
replicaSync = Disabled
default:
return fmt.Errorf("replica metadata sync should be either [enable|disable]")
}

newRule := Rule{
ID: opts.ID,
Expand All @@ -200,7 +211,7 @@ func (c *Config) AddRule(opts Options) error {
// However AWS leaves this configurable https://docs.aws.amazon.com/AmazonS3/latest/dev/replication-for-metadata-changes.html
SourceSelectionCriteria: SourceSelectionCriteria{
ReplicaModifications: ReplicaModifications{
Status: Enabled,
Status: replicaSync,
},
},
}
Expand All @@ -211,13 +222,13 @@ func (c *Config) AddRule(opts Options) error {
}
for _, rule := range c.Rules {
if rule.Priority == newRule.Priority {
return fmt.Errorf("Priority must be unique. Replication configuration already has a rule with this priority")
return fmt.Errorf("priority must be unique. Replication configuration already has a rule with this priority")
}
if rule.Destination.Bucket != newRule.Destination.Bucket {
return fmt.Errorf("The destination bucket must be same for all rules")
return fmt.Errorf("the destination bucket must be same for all rules")
}
if rule.ID == newRule.ID {
return fmt.Errorf("A rule exists with this ID")
return fmt.Errorf("a rule exists with this ID")
}
}

Expand All @@ -228,7 +239,7 @@ func (c *Config) AddRule(opts Options) error {
// EditRule modifies an existing rule in replication config
func (c *Config) EditRule(opts Options) error {
if opts.ID == "" {
return fmt.Errorf("Rule ID missing")
return fmt.Errorf("rule ID missing")
}
rIdx := -1
var newRule Rule
Expand All @@ -240,7 +251,7 @@ func (c *Config) EditRule(opts Options) error {
}
}
if rIdx < 0 {
return fmt.Errorf("Rule with ID %s not found in replication configuration", opts.ID)
return fmt.Errorf("rule with ID %s not found in replication configuration", opts.ID)
}
prefixChg := opts.Prefix != newRule.Prefix()
if opts.IsTagSet || prefixChg {
Expand Down Expand Up @@ -286,7 +297,7 @@ func (c *Config) EditRule(opts Options) error {
case "disable":
newRule.Status = Disabled
default:
return fmt.Errorf("Rule state should be either [enable|disable]")
return fmt.Errorf("rule state should be either [enable|disable]")
}
}
// set DeleteMarkerReplication rule status for edit option
Expand Down Expand Up @@ -314,6 +325,17 @@ func (c *Config) EditRule(opts Options) error {
}
}

if opts.ReplicaSync != "" {
switch opts.ReplicaSync {
case "enable", "":
newRule.SourceSelectionCriteria.ReplicaModifications.Status = Enabled
case "disable":
newRule.SourceSelectionCriteria.ReplicaModifications.Status = Disabled
default:
return fmt.Errorf("replica metadata sync should be either [enable|disable]")
}
}

if opts.IsSCSet {
newRule.Destination.StorageClass = opts.StorageClass
}
Expand Down Expand Up @@ -343,10 +365,10 @@ func (c *Config) EditRule(opts Options) error {
// ensure priority and destination bucket restrictions are not violated
for idx, rule := range c.Rules {
if rule.Priority == newRule.Priority && rIdx != idx {
return fmt.Errorf("Priority must be unique. Replication configuration already has a rule with this priority")
return fmt.Errorf("priority must be unique. Replication configuration already has a rule with this priority")
}
if rule.Destination.Bucket != newRule.Destination.Bucket {
return fmt.Errorf("The destination bucket must be same for all rules")
return fmt.Errorf("the destination bucket must be same for all rules")
}
}

Expand All @@ -369,7 +391,7 @@ func (c *Config) RemoveRule(opts Options) error {
return fmt.Errorf("Rule with ID %s not found", opts.ID)
}
if len(newRules) == 0 {
return fmt.Errorf("Replication configuration should have at least one rule")
return fmt.Errorf("replication configuration should have at least one rule")
}
c.Rules = newRules
return nil
Expand Down Expand Up @@ -402,7 +424,7 @@ func (r Rule) Validate() error {
}

if r.Priority < 0 && r.Status == Enabled {
return fmt.Errorf("Priority must be set for the rule")
return fmt.Errorf("priority must be set for the rule")
}

if err := r.validateStatus(); err != nil {
Expand Down Expand Up @@ -525,11 +547,11 @@ func (tag Tag) IsEmpty() bool {
// Validate checks this tag.
func (tag Tag) Validate() error {
if len(tag.Key) == 0 || utf8.RuneCountInString(tag.Key) > 128 {
return fmt.Errorf("Invalid Tag Key")
return fmt.Errorf("invalid Tag Key")
}

if utf8.RuneCountInString(tag.Value) > 256 {
return fmt.Errorf("Invalid Tag Value")
return fmt.Errorf("invalid Tag Value")
}
return nil
}
Expand Down Expand Up @@ -585,7 +607,7 @@ func (d DeleteReplication) IsEmpty() bool {

// ReplicaModifications specifies if replica modification sync is enabled
type ReplicaModifications struct {
Status Status `xml:"Status" json:"Status"`
Status Status `xml:"Status" json:"Status"` // should be set to "Enabled" by default
}

// SourceSelectionCriteria - specifies additional source selection criteria in ReplicationConfiguration.
Expand Down
18 changes: 9 additions & 9 deletions pkg/replication/replication_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func TestAddReplicationRule(t *testing.T) {
RoleArn: "arn:minio:replication:eu-west-1:c5acb6ac-9918-4dc6-8534-6244ed1a611a:destbucket",
DestBucket: "arn:aws:s3:::destbucket",
},
expectedErr: "Rule state should be either [enable|disable]",
expectedErr: "rule state should be either [enable|disable]",
},
{ //test case :3
cfg: Config{Rules: []Rule{{Priority: 1}}},
Expand All @@ -68,7 +68,7 @@ func TestAddReplicationRule(t *testing.T) {
RoleArn: "arn:minio:replication:eu-west-1:c5acb6ac-9918-4dc6-8534-6244ed1a611a:destbucket",
DestBucket: "arn:aws:s3:::destbucket",
},
expectedErr: "Priority must be unique. Replication configuration already has a rule with this priority",
expectedErr: "priority must be unique. Replication configuration already has a rule with this priority",
},
{ //test case :4
cfg: Config{},
Expand Down Expand Up @@ -111,7 +111,7 @@ func TestAddReplicationRule(t *testing.T) {
RoleArn: "arn:minio:replication:eu-west-1:c5acb6ac-9918-4dc6-8534-6244ed1a611a:destbucket",
DestBucket: "arn:aws:s3:::destbucket",
},
expectedErr: "Role ARN does not match existing configuration",
expectedErr: "role ARN does not match existing configuration",
},
{ //test case :7
cfg: Config{},
Expand Down Expand Up @@ -139,7 +139,7 @@ func TestAddReplicationRule(t *testing.T) {
RoleArn: "arn:minio:replication:eu-west-1:c5acb6ac-9918-4dc6-8534-6244ed1a611a:destbucket",
DestBucket: "arn:aws:s3:::destbucket",
},
expectedErr: "A rule exists with this ID",
expectedErr: "a rule exists with this ID",
},
}
for i, testCase := range testCases {
Expand Down Expand Up @@ -201,7 +201,7 @@ func TestEditReplicationRule(t *testing.T) {
RoleArn: "arn:minio:replication:eu-west-1:c5acb6ac-9918-4dc6-8534-6244ed1a611a:destbucket",
DestBucket: "arn:aws:s3:::destbucket",
},
expectedErr: "Rule with ID xyz.id not found in replication configuration",
expectedErr: "rule with ID xyz.id not found in replication configuration",
},
{ //test case :3 missing rule id
cfg: Config{
Expand All @@ -221,7 +221,7 @@ func TestEditReplicationRule(t *testing.T) {
RoleArn: "arn:minio:replication:eu-west-1:c5acb6ac-9918-4dc6-8534-6244ed1a611a:destbucket",
DestBucket: "arn:aws:s3:::destbucket",
},
expectedErr: "Rule ID missing",
expectedErr: "rule ID missing",
},
{ //test case :4 different destination bucket
cfg: Config{
Expand All @@ -242,7 +242,7 @@ func TestEditReplicationRule(t *testing.T) {
RoleArn: "arn:minio:replication:eu-west-1:c5acb6ac-9918-4dc6-8534-6244ed1a611a:destbucket",
DestBucket: "arn:aws:s3:::differentbucket",
},
expectedErr: "The destination bucket must be same for all rules",
expectedErr: "the destination bucket must be same for all rules",
},
{ //test case :5 invalid destination bucket arn format
cfg: Config{
Expand Down Expand Up @@ -285,7 +285,7 @@ func TestEditReplicationRule(t *testing.T) {
RoleArn: "arn:minio:replication:eu-west-1:c5acb6ac-9918-4dc6-8534-6244ed1a611a:destbucket",
DestBucket: "arn:aws:s3:::destbucket",
},
expectedErr: "Rule state should be either [enable|disable]",
expectedErr: "rule state should be either [enable|disable]",
},
{ //test case :7 another rule has same priority
cfg: Config{
Expand All @@ -312,7 +312,7 @@ func TestEditReplicationRule(t *testing.T) {
RoleArn: "arn:minio:replication:eu-west-1:c5acb6ac-9918-4dc6-8534-6244ed1a611a:destbucket",
DestBucket: "arn:aws:s3:::destbucket",
},
expectedErr: "Priority must be unique. Replication configuration already has a rule with this priority",
expectedErr: "priority must be unique. Replication configuration already has a rule with this priority",
},
}

Expand Down

0 comments on commit 5b1664c

Please sign in to comment.