From 806b10b934684d5e8abeab5c8785b3cfff41921e Mon Sep 17 00:00:00 2001 From: Poorna Krishnamoorthy Date: Tue, 21 Sep 2021 16:03:20 -0400 Subject: [PATCH] fix: improve error messages returned during replication setup (#13261) --- cmd/api-errors.go | 2 -- cmd/bucket-handlers.go | 6 +++--- cmd/bucket-replication.go | 18 +++++++++--------- cmd/object-api-errors.go | 7 ------- go.mod | 2 +- go.sum | 4 ++-- 6 files changed, 15 insertions(+), 24 deletions(-) diff --git a/cmd/api-errors.go b/cmd/api-errors.go index f1d18c833..46c57c327 100644 --- a/cmd/api-errors.go +++ b/cmd/api-errors.go @@ -1967,8 +1967,6 @@ func toAPIErrorCode(ctx context.Context, err error) (apiErr APIErrorCode) { apiErr = ErrReplicationConfigurationNotFoundError case BucketRemoteDestinationNotFound: apiErr = ErrRemoteDestinationNotFoundError - case BucketReplicationDestinationMissingLock: - apiErr = ErrReplicationDestinationMissingLock case BucketRemoteTargetNotFound: apiErr = ErrRemoteTargetNotFoundError case BucketRemoteConnectionErr: diff --git a/cmd/bucket-handlers.go b/cmd/bucket-handlers.go index 9ad854624..7f1a3e1e5 100644 --- a/cmd/bucket-handlers.go +++ b/cmd/bucket-handlers.go @@ -1525,9 +1525,9 @@ func (api objectAPIHandlers) PutBucketReplicationConfigHandler(w http.ResponseWr writeErrorResponse(ctx, w, apiErr, r.URL) return } - sameTarget, err := validateReplicationDestination(ctx, bucket, replicationConfig) - if err != nil { - writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL) + sameTarget, apiErr := validateReplicationDestination(ctx, bucket, replicationConfig) + if apiErr != noError { + writeErrorResponse(ctx, w, apiErr, r.URL) return } // Validate the received bucket replication config diff --git a/cmd/bucket-replication.go b/cmd/bucket-replication.go index 7d50b41f0..237a89e9c 100644 --- a/cmd/bucket-replication.go +++ b/cmd/bucket-replication.go @@ -79,7 +79,7 @@ func getReplicationConfig(ctx context.Context, bucketName string) (rc *replicati // validateReplicationDestination returns error if replication destination bucket missing or not configured // It also returns true if replication destination is same as this server. -func validateReplicationDestination(ctx context.Context, bucket string, rCfg *replication.Config) (bool, error) { +func validateReplicationDestination(ctx context.Context, bucket string, rCfg *replication.Config) (bool, APIError) { var arns []string if rCfg.RoleArn != "" { arns = append(arns, rCfg.RoleArn) @@ -91,23 +91,23 @@ func validateReplicationDestination(ctx context.Context, bucket string, rCfg *re for _, arnStr := range arns { arn, err := madmin.ParseARN(arnStr) if err != nil { - return false, BucketRemoteArnInvalid{} + return false, errorCodes.ToAPIErrWithErr(ErrBucketRemoteArnInvalid, err) } if arn.Type != madmin.ReplicationService { - return false, BucketRemoteArnTypeInvalid{} + return false, toAPIError(ctx, BucketRemoteArnTypeInvalid{Bucket: bucket}) } clnt := globalBucketTargetSys.GetRemoteTargetClient(ctx, arnStr) if clnt == nil { - return false, BucketRemoteTargetNotFound{Bucket: bucket} + return false, toAPIError(ctx, BucketRemoteTargetNotFound{Bucket: bucket}) } - if found, _ := clnt.BucketExists(ctx, arn.Bucket); !found { - return false, BucketRemoteDestinationNotFound{Bucket: arn.Bucket} + if found, err := clnt.BucketExists(ctx, arn.Bucket); !found { + return false, errorCodes.ToAPIErrWithErr(ErrRemoteDestinationNotFoundError, err) } if ret, err := globalBucketObjectLockSys.Get(bucket); err == nil { if ret.LockEnabled { lock, _, _, _, err := clnt.GetObjectLockConfig(ctx, arn.Bucket) if err != nil || lock != "Enabled" { - return false, BucketReplicationDestinationMissingLock{Bucket: arn.Bucket} + return false, errorCodes.ToAPIErrWithErr(ErrReplicationDestinationMissingLock, err) } } } @@ -116,11 +116,11 @@ func validateReplicationDestination(ctx context.Context, bucket string, rCfg *re if ok { if c.EndpointURL().String() == clnt.EndpointURL().String() { sameTarget, _ := isLocalHost(clnt.EndpointURL().Hostname(), clnt.EndpointURL().Port(), globalMinioPort) - return sameTarget, nil + return sameTarget, toAPIError(ctx, nil) } } } - return false, BucketRemoteTargetNotFound{Bucket: bucket} + return false, toAPIError(ctx, BucketRemoteTargetNotFound{Bucket: bucket}) } type mustReplicateOptions struct { diff --git a/cmd/object-api-errors.go b/cmd/object-api-errors.go index 4a2f7a539..772594ad1 100644 --- a/cmd/object-api-errors.go +++ b/cmd/object-api-errors.go @@ -400,13 +400,6 @@ func (e BucketRemoteDestinationNotFound) Error() string { return "Destination bucket does not exist: " + e.Bucket } -// BucketReplicationDestinationMissingLock bucket does not have object lock enabled. -type BucketReplicationDestinationMissingLock GenericError - -func (e BucketReplicationDestinationMissingLock) Error() string { - return "Destination bucket does not have object lock enabled: " + e.Bucket -} - // BucketRemoteTargetNotFound remote target does not exist. type BucketRemoteTargetNotFound GenericError diff --git a/go.mod b/go.mod index 70ffc311c..19b1cd583 100644 --- a/go.mod +++ b/go.mod @@ -46,7 +46,7 @@ require ( github.com/minio/highwayhash v1.0.2 github.com/minio/kes v0.14.0 github.com/minio/madmin-go v1.1.6-0.20210917204419-f12dc0d0a8bd - github.com/minio/minio-go/v7 v7.0.15-0.20210917235750-a16dfb14e6f6 + github.com/minio/minio-go/v7 v7.0.15-0.20210921183434-174b4c070788 github.com/minio/parquet-go v1.0.0 github.com/minio/pkg v1.1.3 github.com/minio/selfupdate v0.3.1 diff --git a/go.sum b/go.sum index cbcbb4e00..07f9e3ded 100644 --- a/go.sum +++ b/go.sum @@ -1033,8 +1033,8 @@ github.com/minio/minio-go/v7 v7.0.10/go.mod h1:td4gW1ldOsj1PbSNS+WYK43j+P1XVhX/8 github.com/minio/minio-go/v7 v7.0.11-0.20210302210017-6ae69c73ce78/go.mod h1:mTh2uJuAbEqdhMVl6CMIIZLUeiMiWtJR4JB8/5g2skw= github.com/minio/minio-go/v7 v7.0.11-0.20210607181445-e162fdb8e584/go.mod h1:WoyW+ySKAKjY98B9+7ZbI8z8S3jaxaisdcvj9TGlazA= github.com/minio/minio-go/v7 v7.0.14/go.mod h1:S23iSP5/gbMwtxeY5FM71R+TkAYyzEdoNEDDwpt8yWs= -github.com/minio/minio-go/v7 v7.0.15-0.20210917235750-a16dfb14e6f6 h1:vkarc8wO5ozZavEJilw4O3gzhPRJFEkHxCzZU8f+PiE= -github.com/minio/minio-go/v7 v7.0.15-0.20210917235750-a16dfb14e6f6/go.mod h1:X6NObIqnx2xvri7kLQSDB3GD1Nt0vtF9WG5oMVEKhLc= +github.com/minio/minio-go/v7 v7.0.15-0.20210921183434-174b4c070788 h1:O+/N9vxhoObjuCuQczycuzdG240SoLrrdnyipJ5JJc0= +github.com/minio/minio-go/v7 v7.0.15-0.20210921183434-174b4c070788/go.mod h1:pUV0Pc+hPd1nccgmzQF/EXh48l/Z/yps6QPF1aaie4g= github.com/minio/operator v0.0.0-20210812082324-26350f153661 h1:dGAJHpfmhNukFg0M0wDqH+G1OB2YPgZCcT6uv4n9YQk= github.com/minio/operator v0.0.0-20210812082324-26350f153661/go.mod h1:zQqn6VGT46xlSpVXh1I/VZRv+eSgHtVu6URdg71YKX8= github.com/minio/operator/logsearchapi v0.0.0-20210812082324-26350f153661 h1:tJw15hS3b1dVTf5PwA4roXZ/oRNnHyZ/8Y+yNTmQ5rA=