Remove site replication config if it succeeded locally (#16279)

cmd/api-errors.go

@@ -1384,7 +1384,7 @@ var errorCodes = errorCodeMap{
 	ErrSiteReplicationPeerResp: {
 		Code:           "XMinioSiteReplicationPeerResp",
 		Description:    "Error received when contacting a peer site",
-		HTTPStatusCode: http.StatusServiceUnavailable,
+		HTTPStatusCode: http.StatusBadRequest,
 	},
 	ErrSiteReplicationBackendIssue: {
 		Code:           "XMinioSiteReplicationBackendIssue",

cmd/bucket-replication.go

@@ -131,8 +131,8 @@ func validateReplicationDestination(ctx context.Context, bucket string, rCfg *re
 			}
 		}
 		// validate replication ARN against target endpoint
-		c, ok := globalBucketTargetSys.arnRemotesMap[arnStr]
-		if ok {
+		c := globalBucketTargetSys.GetRemoteTargetClient(ctx, arnStr)
+		if c != nil {
 			if c.EndpointURL().String() == clnt.EndpointURL().String() {
 				selfTarget, _ := isLocalHost(clnt.EndpointURL().Hostname(), clnt.EndpointURL().Port(), globalMinioPort)
 				if !sameTarget {

cmd/site-replication.go

@@ -72,6 +72,10 @@ var (
 		Cause: errors.New("peer not found"),
 		Code:  ErrSiteReplicationInvalidRequest,
 	}
+	errSRRequestorNotFound = SRError{
+		Cause: errors.New("requesting site not found in site replication config"),
+		Code:  ErrSiteReplicationInvalidRequest,
+	}
 	errSRNotEnabled = SRError{
 		Cause: errors.New("site replication is not enabled"),
 		Code:  ErrSiteReplicationInvalidRequest,
@@ -2060,12 +2064,12 @@ func (c *SiteReplicationSys) RemovePeerCluster(ctx context.Context, objectAPI Ob
 		}
 	}
 	for _, s := range siteNames {
-		info, ok := peerMap[s]
+		pinfo, ok := peerMap[s]
 		if !ok {
 			return st, errSRConfigMissingError(errMissingSRConfig)
 		}
-		rmvEndpoints = append(rmvEndpoints, info.Endpoint)
-		delete(updatedPeers, info.DeploymentID)
+		rmvEndpoints = append(rmvEndpoints, pinfo.Endpoint)
+		delete(updatedPeers, pinfo.DeploymentID)
 	}
 	var wg sync.WaitGroup
 	errs := make(map[string]error, len(c.state.Peers))
@@ -2087,6 +2091,8 @@ func (c *SiteReplicationSys) RemovePeerCluster(ctx context.Context, objectAPI Ob
 				errs[pi.DeploymentID] = errSRPeerResp(fmt.Errorf("unable to create admin client for %s: %w", pi.Name, err))
 				return
 			}
+			// set the requesting site's deploymentID for verification of peer request
+			rreq.RequestingDepID = globalDeploymentID
 			if _, err = admClient.SRPeerRemove(ctx, rreq); err != nil {
 				if errors.Is(err, errMissingSRConfig) {
 					// ignore if peer is already removed.
@@ -2100,9 +2106,11 @@ func (c *SiteReplicationSys) RemovePeerCluster(ctx context.Context, objectAPI Ob
 	wg.Wait()
 
 	errdID := ""
+	selfTgtsDeleted := errs[globalDeploymentID] == nil // true if all remote targets and replication config cleared successfully on local cluster
+
 	for dID, err := range errs {
 		if err != nil {
-			if !rreq.RemoveAll {
+			if !rreq.RemoveAll && !selfTgtsDeleted {
 				return madmin.ReplicateRemoveStatus{
 					ErrDetail: err.Error(),
 					Status:    madmin.ReplicateRemoveStatusPartial,
@@ -2146,12 +2154,17 @@ func (c *SiteReplicationSys) RemovePeerCluster(ctx context.Context, objectAPI Ob
 		return madmin.ReplicateRemoveStatus{
 			Status:    madmin.ReplicateRemoveStatusPartial,
 			ErrDetail: fmt.Sprintf("unable to save cluster-replication state on local: %v", err),
-		}, nil
+		}, err
 	}
 
-	return madmin.ReplicateRemoveStatus{
+	st = madmin.ReplicateRemoveStatus{
 		Status: madmin.ReplicateRemoveStatusSuccess,
-	}, nil
+	}
+	if errs[errdID] != nil {
+		st.Status = madmin.ReplicateRemoveStatusPartial
+		st.ErrDetail = errs[errdID].Error()
+	}
+	return st, nil
 }
 
 // InternalRemoveReq - sends an unlink request to peer cluster to remove one or more sites
@@ -2160,6 +2173,19 @@ func (c *SiteReplicationSys) InternalRemoveReq(ctx context.Context, objectAPI Ob
 	if !c.isEnabled() {
 		return errSRNotEnabled
 	}
+	if rreq.RequestingDepID != "" {
+		// validate if requesting site is still part of site replication
+		var foundRequestor bool
+		for _, p := range c.state.Peers {
+			if p.DeploymentID == rreq.RequestingDepID {
+				foundRequestor = true
+				break
+			}
+		}
+		if !foundRequestor {
+			return errSRRequestorNotFound
+		}
+	}
 
 	ourName := ""
 	peerMap := make(map[string]madmin.PeerInfo)

go.mod

@@ -49,7 +49,7 @@ require (
 	github.com/minio/dperf v0.4.2
 	github.com/minio/highwayhash v1.0.2
 	github.com/minio/kes v0.22.2
-	github.com/minio/madmin-go/v2 v2.0.2
+	github.com/minio/madmin-go/v2 v2.0.3
 	github.com/minio/minio-go/v7 v7.0.45
 	github.com/minio/pkg v1.5.8
 	github.com/minio/selfupdate v0.5.0

go.sum

@@ -768,8 +768,8 @@ github.com/minio/highwayhash v1.0.2/go.mod h1:BQskDq+xkJ12lmlUUi7U0M5Swg3EWR+dLT
 github.com/minio/kes v0.22.2 h1:9NdgTx+TFJco0Pqdrq8WZbrTZVv0ichg+sbPRQiJ2HU=
 github.com/minio/kes v0.22.2/go.mod h1:J9sD6Pe8obPt7+JXFcznkWaYaj9pBWCfN9U9j//NsNw=
 github.com/minio/madmin-go v1.6.6/go.mod h1:ATvkBOLiP3av4D++2v1UEHC/QzsGtgXD5kYvvRYzdKs=
-github.com/minio/madmin-go/v2 v2.0.2 h1:VvCVnGMWiWIe/ds4frrZT4wQP/NpuAHC+pKU6LfjWDQ=
-github.com/minio/madmin-go/v2 v2.0.2/go.mod h1:5aFi/VLWBHC2DEFfGIlUmAeJhaF4ZAjuYpEWZFU14Zw=
+github.com/minio/madmin-go/v2 v2.0.3 h1:Q8qco+JrbRIim25tGrs0enVRJGoIMUHfULa5nJoSiqM=
+github.com/minio/madmin-go/v2 v2.0.3/go.mod h1:5aFi/VLWBHC2DEFfGIlUmAeJhaF4ZAjuYpEWZFU14Zw=
 github.com/minio/mc v0.0.0-20221209193822-daa06599e44d h1:6ixpv/LgK9KK/2oEdAGImS58oDi0jL/sSF4Iwy6MGEc=
 github.com/minio/mc v0.0.0-20221209193822-daa06599e44d/go.mod h1:zt7TJevWacrSFrVkQ6Ba2ZY9PCZpp3A8qNeFBq7b8rI=
 github.com/minio/md5-simd v1.1.2 h1:Gdi1DZK69+ZVMoNHRXJyNcxrMA4dSxoYHZSQbirFg34=