fix: change SetRemoteTarget API to allow editing remote target granularly (#12175)

Currently, only credentials could be updated with
`mc admin bucket remote edit`. 

Allow updating synchronous replication flag, path, 
bandwidth and healthcheck duration on buckets, and
a flag to disable proxying in active-active replication.

cmd/admin-bucket-handlers.go

@@ -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:

cmd/bucket-replication.go

@@ -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,

cmd/bucket-targets.go

@@ -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{
@@ -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
@@ -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 {

pkg/madmin/remote-target-commands.go

@@ -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
@@ -110,7 +111,7 @@ 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,
@@ -118,6 +119,7 @@ func (t *BucketTarget) Clone() BucketTarget {
 		ReplicationSync:     t.ReplicationSync,
 		StorageClass:        t.StorageClass, // target storage class
 		HealthCheckDuration: t.HealthCheckDuration,
+		DisableProxy:        t.DisableProxy,
 	}
 }
 
@@ -235,8 +237,54 @@ 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 update creds
+	CredentialsUpdateType TargetUpdateType = 1 + iota
+	// SyncUpdateType update synchronous replication setting
+	SyncUpdateType
+	// ProxyUpdateType update proxy setting
+	ProxyUpdateType
+	// BandwidthLimitUpdateType update bandwidth limit
+	BandwidthLimitUpdateType
+	// HealthCheckDurationUpdateType update health check duration
+	HealthCheckDurationUpdateType
+	// PathUpdateType update Path
+	PathUpdateType
+)
+
+// GetTargetUpdateOps returns a slice of update operations being
+// performed with `mc admin bucket remote edit`
+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")
 	}
@@ -252,6 +300,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,

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)
+		}
+	}
+}