Skip to content

Commit

Permalink
Change SetRemoteTarget API to allow editing remote target
Browse files Browse the repository at this point in the history
more flexibly.

- Previously only credentials could be updated with
`mc admin bucket remote edit`. Change this to allow updating
synchronous replication flag, path, bandwidth and healthcheck

Add a flag to allow disabling proxying in active-active replication
  • Loading branch information
Poorna Krishnamoorthy committed Apr 28, 2021
1 parent 0c9d095 commit f172dc9
Show file tree
Hide file tree
Showing 5 changed files with 205 additions and 5 deletions.
34 changes: 32 additions & 2 deletions cmd/admin-bucket-handlers.go
Expand Up @@ -165,14 +165,44 @@ func (a adminAPIHandlers) SetRemoteTargetHandler(w http.ResponseWriter, r *http.
}

target.SourceBucket = bucket
if !update {
var ops []madmin.TargetUpdateType
if update {
ops = madmin.GetTargetUpdateOps(r.URL.Query())
} else {
target.Arn = globalBucketTargetSys.getRemoteARN(bucket, &target)
}
if target.Arn == "" {
writeErrorResponseJSON(ctx, w, errorCodes.ToAPIErrWithErr(ErrAdminConfigBadJSON, err), r.URL)
return
}

if update {
// overlay the updates on existing target
tgt := globalBucketTargetSys.GetRemoteBucketTargetByArn(ctx, bucket, target.Arn)
if tgt.Empty() {
writeErrorResponseJSON(ctx, w, errorCodes.ToAPIErrWithErr(ErrRemoteTargetNotFoundError, err), r.URL)
return
}
for _, op := range ops {
switch op {
case madmin.CredentialsUpdateType:
tgt.Credentials = target.Credentials
tgt.TargetBucket = target.TargetBucket
tgt.Secure = target.Secure
tgt.Endpoint = target.Endpoint
case madmin.SyncUpdateType:
tgt.ReplicationSync = target.ReplicationSync
case madmin.ProxyUpdateType:
tgt.DisableProxy = target.DisableProxy
case madmin.PathUpdateType:
tgt.Path = target.Path
case madmin.BandwidthLimitUpdateType:
tgt.BandwidthLimit = target.BandwidthLimit
case madmin.HealthCheckDurationUpdateType:
tgt.HealthCheckDuration = target.HealthCheckDuration
}
}
target = tgt
}
if err = globalBucketTargetSys.SetTarget(ctx, bucket, &target, update); err != nil {
switch err.(type) {
case BucketRemoteConnectionErr:
Expand Down
5 changes: 4 additions & 1 deletion cmd/bucket-replication.go
Expand Up @@ -1049,7 +1049,10 @@ func proxyHeadToRepTarget(ctx context.Context, bucket, object string, opts Objec
if tgt == nil || tgt.isOffline() {
return nil, oi, false, fmt.Errorf("target is offline or not configured")
}

// if proxying explicitly disabled on remote target
if tgt.disableProxy {
return nil, oi, false, nil
}
gopts := miniogo.GetObjectOptions{
VersionID: opts.VersionID,
ServerSideEncryption: opts.ServerSideEncryption,
Expand Down
17 changes: 17 additions & 0 deletions cmd/bucket-targets.go
Expand Up @@ -204,6 +204,21 @@ func (sys *BucketTargetSys) GetRemoteTargetClient(ctx context.Context, arn strin
return sys.arnRemotesMap[arn]
}

// GetRemoteBucketTargetByArn returns BucketTarget for a ARN
func (sys *BucketTargetSys) GetRemoteBucketTargetByArn(ctx context.Context, bucket, arn string) madmin.BucketTarget {
sys.RLock()
defer sys.RUnlock()
var tgt madmin.BucketTarget
for _, t := range sys.targetsMap[bucket] {
if t.Arn == arn {
tgt = t.Clone()
tgt.Credentials = t.Credentials
return tgt
}
}
return tgt
}

// NewBucketTargetSys - creates new replication system.
func NewBucketTargetSys() *BucketTargetSys {
return &BucketTargetSys{
Expand Down Expand Up @@ -315,6 +330,7 @@ func (sys *BucketTargetSys) getRemoteTargetClient(tcfg *madmin.BucketTarget) (*T
replicateSync: tcfg.ReplicationSync,
Bucket: tcfg.TargetBucket,
StorageClass: tcfg.StorageClass,
disableProxy: tcfg.DisableProxy,
}
go tc.healthCheck()
return tc, nil
Expand Down Expand Up @@ -393,6 +409,7 @@ type TargetClient struct {
Bucket string // remote bucket target
replicateSync bool
StorageClass string // storage class on remote
disableProxy bool
}

func (tc *TargetClient) isOffline() bool {
Expand Down
61 changes: 59 additions & 2 deletions pkg/madmin/remote-target-commands.go
Expand Up @@ -99,6 +99,7 @@ type BucketTarget struct {
ReplicationSync bool `json:"replicationSync"`
StorageClass string `json:"storageclass,omitempty"`
HealthCheckDuration time.Duration `json:"healthCheckDuration,omitempty"`
DisableProxy bool `json:"disableProxy"`
}

// Clone returns shallow clone of BucketTarget without secret key in credentials
Expand All @@ -110,14 +111,15 @@ func (t *BucketTarget) Clone() BucketTarget {
Credentials: &auth.Credentials{AccessKey: t.Credentials.AccessKey},
Secure: t.Secure,
Path: t.Path,
API: t.Path,
API: t.API,
Arn: t.Arn,
Type: t.Type,
Region: t.Region,
BandwidthLimit: t.BandwidthLimit,
ReplicationSync: t.ReplicationSync,
StorageClass: t.StorageClass, // target storage class
HealthCheckDuration: t.HealthCheckDuration,
DisableProxy: t.DisableProxy,
}
}

Expand Down Expand Up @@ -235,8 +237,46 @@ func (adm *AdminClient) SetRemoteTarget(ctx context.Context, bucket string, targ
return arn, nil
}

// TargetUpdateType - type of update on the remote target
type TargetUpdateType int

const (
CredentialsUpdateType TargetUpdateType = 1 + iota
SyncUpdateType
ProxyUpdateType
BandwidthLimitUpdateType
HealthCheckDurationUpdateType
PathUpdateType
)

func GetTargetUpdateOps(values url.Values) []TargetUpdateType {
var ops []TargetUpdateType
if values.Get("update") != "true" {
return ops
}
if values.Get("creds") == "true" {
ops = append(ops, CredentialsUpdateType)
}
if values.Get("sync") == "true" {
ops = append(ops, SyncUpdateType)
}
if values.Get("proxy") == "true" {
ops = append(ops, ProxyUpdateType)
}
if values.Get("healthcheck") == "true" {
ops = append(ops, HealthCheckDurationUpdateType)
}
if values.Get("bandwidth") == "true" {
ops = append(ops, BandwidthLimitUpdateType)
}
if values.Get("path") == "true" {
ops = append(ops, PathUpdateType)
}
return ops
}

// UpdateRemoteTarget updates credentials for a remote bucket target
func (adm *AdminClient) UpdateRemoteTarget(ctx context.Context, target *BucketTarget) (string, error) {
func (adm *AdminClient) UpdateRemoteTarget(ctx context.Context, target *BucketTarget, ops ...TargetUpdateType) (string, error) {
if target == nil {
return "", fmt.Errorf("target cannot be nil")
}
Expand All @@ -252,6 +292,23 @@ func (adm *AdminClient) UpdateRemoteTarget(ctx context.Context, target *BucketTa
queryValues.Set("bucket", target.SourceBucket)
queryValues.Set("update", "true")

for _, op := range ops {
switch op {
case CredentialsUpdateType:
queryValues.Set("creds", "true")
case SyncUpdateType:
queryValues.Set("sync", "true")
case ProxyUpdateType:
queryValues.Set("proxy", "true")
case BandwidthLimitUpdateType:
queryValues.Set("bandwidth", "true")
case HealthCheckDurationUpdateType:
queryValues.Set("healthcheck", "true")
case PathUpdateType:
queryValues.Set("path", "true")
}
}

reqData := requestData{
relPath: adminAPIPrefix + "/set-remote-target",
queryValues: queryValues,
Expand Down
93 changes: 93 additions & 0 deletions pkg/madmin/remote-target-commands_test.go
@@ -0,0 +1,93 @@
// Copyright (c) 2021 MinIO, Inc.
//
// This file is part of MinIO Object Storage stack
//
// This program is free software: you can redistribute it and/or modify
// it under the terms of the GNU Affero General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// This program is distributed in the hope that it will be useful
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU Affero General Public License for more details.
//
// You should have received a copy of the GNU Affero General Public License
// along with this program. If not, see <http://www.gnu.org/licenses/>.

package madmin

import (
"net/url"
"testing"
)

func isOpsEqual(op1 []TargetUpdateType, op2 []TargetUpdateType) bool {
if len(op1) != len(op2) {
return false
}
for _, o1 := range op1 {
found := false
for _, o2 := range op2 {
if o2 == o1 {
found = true
break
}
}
if !found {
return false
}
}
return true
}

// TestGetTargetUpdateOps tests GetTargetUpdateOps
func TestGetTargetUpdateOps(t *testing.T) {
testCases := []struct {
values url.Values
expectedOps []TargetUpdateType
}{
{values: url.Values{
"update": []string{"true"}},
expectedOps: []TargetUpdateType{},
},
{values: url.Values{
"update": []string{"false"},
"path": []string{"true"},
},
expectedOps: []TargetUpdateType{},
},
{values: url.Values{
"update": []string{"true"},
"path": []string{""},
},
expectedOps: []TargetUpdateType{},
},
{values: url.Values{
"update": []string{"true"},
"path": []string{"true"},
"bzzzz": []string{"true"},
},
expectedOps: []TargetUpdateType{PathUpdateType},
},

{values: url.Values{
"update": []string{"true"},
"path": []string{"true"},
"creds": []string{"true"},
"sync": []string{"true"},
"proxy": []string{"true"},
"bandwidth": []string{"true"},
"healthcheck": []string{"true"},
},
expectedOps: []TargetUpdateType{
PathUpdateType, CredentialsUpdateType, SyncUpdateType, ProxyUpdateType, BandwidthLimitUpdateType, HealthCheckDurationUpdateType},
},
}
for i, test := range testCases {
gotOps := GetTargetUpdateOps(test.values)
if !isOpsEqual(gotOps, test.expectedOps) {
t.Fatalf("test %d: expected %v got %v", i+1, test.expectedOps, gotOps)
}
}
}

0 comments on commit f172dc9

Please sign in to comment.