s3: fix no error being returned when creating a bucket we don't own

Before this change if you tried to create a bucket that already
existed, but someone else owned then rclone did not return an error.

This now will return an error on providers that return the
AlreadyOwnedByYou error code or no error on bucket creation of an
existing bucket owned by you.

This introduces a new provider quirk and this has been set or cleared
for as many providers as can be tested. This can be overridden by the
--s3-use-already-exists flag.

Fixes #7351
This commit is contained in:
Nick Craig-Wood 2023-10-06 11:45:03 +01:00
parent d6ba60c04d
commit f56ea2bee2
2 changed files with 111 additions and 11 deletions

View file

@ -2488,6 +2488,33 @@ In this case, you might want to try disabling this option.
Help: "Endpoint for STS.\n\nLeave blank if using AWS to use the default endpoint for the region.", Help: "Endpoint for STS.\n\nLeave blank if using AWS to use the default endpoint for the region.",
Provider: "AWS", Provider: "AWS",
Advanced: true, Advanced: true,
}, {
Name: "use_already_exists",
Help: strings.ReplaceAll(`Set if rclone should report BucketAlreadyExists errors on bucket creation.
At some point during the evolution of the s3 protocol, AWS started
returning an |AlreadyOwnedByYou| error when attempting to create a
bucket that the user already owned, rather than a
|BucketAlreadyExists| error.
Unfortunately exactly what has been implemented by s3 clones is a
little inconsistent, some return |AlreadyOwnedByYou|, some return
|BucketAlreadyExists| and some return no error at all.
This is important to rclone because it ensures the bucket exists by
creating it on quite a lot of operations (unless
|--s3-no-check-bucket| is used).
If rclone knows the provider can return |AlreadyOwnedByYou| or returns
no error then it can report |BucketAlreadyExists| errors when the user
attempts to create a bucket not owned by them. Otherwise rclone
ignores the |BucketAlreadyExists| error which can lead to confusion.
This should be automatically set correctly for all providers rclone
knows about - please make a bug report if not.
`, "|", "`"),
Default: fs.Tristate{},
Advanced: true,
}, },
}}) }})
} }
@ -2614,6 +2641,7 @@ type Options struct {
MightGzip fs.Tristate `config:"might_gzip"` MightGzip fs.Tristate `config:"might_gzip"`
UseAcceptEncodingGzip fs.Tristate `config:"use_accept_encoding_gzip"` UseAcceptEncodingGzip fs.Tristate `config:"use_accept_encoding_gzip"`
NoSystemMetadata bool `config:"no_system_metadata"` NoSystemMetadata bool `config:"no_system_metadata"`
UseAlreadyExists fs.Tristate `config:"use_already_exists"`
} }
// Fs represents a remote s3 server // Fs represents a remote s3 server
@ -3012,6 +3040,7 @@ func setQuirks(opt *Options) {
useMultipartEtag = true // Set if Etags for multpart uploads are compatible with AWS useMultipartEtag = true // Set if Etags for multpart uploads are compatible with AWS
useAcceptEncodingGzip = true // Set Accept-Encoding: gzip useAcceptEncodingGzip = true // Set Accept-Encoding: gzip
mightGzip = true // assume all providers might use content encoding gzip until proven otherwise mightGzip = true // assume all providers might use content encoding gzip until proven otherwise
useAlreadyExists = true // Set if provider returns AlreadyOwnedByYou or no error if you try to remake your own bucket
) )
switch opt.Provider { switch opt.Provider {
case "AWS": case "AWS":
@ -3019,18 +3048,22 @@ func setQuirks(opt *Options) {
mightGzip = false // Never auto gzips objects mightGzip = false // Never auto gzips objects
case "Alibaba": case "Alibaba":
useMultipartEtag = false // Alibaba seems to calculate multipart Etags differently from AWS useMultipartEtag = false // Alibaba seems to calculate multipart Etags differently from AWS
useAlreadyExists = true // returns 200 OK
case "HuaweiOBS": case "HuaweiOBS":
// Huawei OBS PFS is not support listObjectV2, and if turn on the urlEncodeListing, marker will not work and keep list same page forever. // Huawei OBS PFS is not support listObjectV2, and if turn on the urlEncodeListing, marker will not work and keep list same page forever.
urlEncodeListings = false urlEncodeListings = false
listObjectsV2 = false listObjectsV2 = false
useAlreadyExists = false // untested
case "Ceph": case "Ceph":
listObjectsV2 = false listObjectsV2 = false
virtualHostStyle = false virtualHostStyle = false
urlEncodeListings = false urlEncodeListings = false
useAlreadyExists = false // untested
case "ChinaMobile": case "ChinaMobile":
listObjectsV2 = false listObjectsV2 = false
virtualHostStyle = false virtualHostStyle = false
urlEncodeListings = false urlEncodeListings = false
useAlreadyExists = false // untested
case "Cloudflare": case "Cloudflare":
virtualHostStyle = false virtualHostStyle = false
useMultipartEtag = false // currently multipart Etags are random useMultipartEtag = false // currently multipart Etags are random
@ -3038,88 +3071,104 @@ func setQuirks(opt *Options) {
listObjectsV2 = false listObjectsV2 = false
virtualHostStyle = false virtualHostStyle = false
urlEncodeListings = false urlEncodeListings = false
useAlreadyExists = false // untested
case "DigitalOcean": case "DigitalOcean":
urlEncodeListings = false urlEncodeListings = false
useAlreadyExists = false // untested
case "Dreamhost": case "Dreamhost":
urlEncodeListings = false urlEncodeListings = false
useAlreadyExists = false // untested
case "IBMCOS": case "IBMCOS":
listObjectsV2 = false // untested listObjectsV2 = false // untested
virtualHostStyle = false virtualHostStyle = false
urlEncodeListings = false urlEncodeListings = false
useMultipartEtag = false // untested useMultipartEtag = false // untested
useAlreadyExists = false // returns BucketAlreadyExists
case "IDrive": case "IDrive":
virtualHostStyle = false virtualHostStyle = false
useAlreadyExists = false // untested
case "IONOS": case "IONOS":
// listObjectsV2 supported - https://api.ionos.com/docs/s3/#Basic-Operations-get-Bucket-list-type-2 // listObjectsV2 supported - https://api.ionos.com/docs/s3/#Basic-Operations-get-Bucket-list-type-2
virtualHostStyle = false virtualHostStyle = false
urlEncodeListings = false urlEncodeListings = false
useAlreadyExists = false // untested
case "Petabox": case "Petabox":
// No quirks useAlreadyExists = false // untested
case "Liara": case "Liara":
virtualHostStyle = false virtualHostStyle = false
urlEncodeListings = false urlEncodeListings = false
useMultipartEtag = false useMultipartEtag = false
useAlreadyExists = false // untested
case "Linode": case "Linode":
// No quirks useAlreadyExists = true // returns 200 OK
case "LyveCloud": case "LyveCloud":
useMultipartEtag = false // LyveCloud seems to calculate multipart Etags differently from AWS useMultipartEtag = false // LyveCloud seems to calculate multipart Etags differently from AWS
useAlreadyExists = false // untested
case "Minio": case "Minio":
virtualHostStyle = false virtualHostStyle = false
case "Netease": case "Netease":
listObjectsV2 = false // untested listObjectsV2 = false // untested
urlEncodeListings = false urlEncodeListings = false
useMultipartEtag = false // untested useMultipartEtag = false // untested
useAlreadyExists = false // untested
case "RackCorp": case "RackCorp":
// No quirks // No quirks
useMultipartEtag = false // untested useMultipartEtag = false // untested
useAlreadyExists = false // untested
case "Scaleway": case "Scaleway":
// Scaleway can only have 1000 parts in an upload // Scaleway can only have 1000 parts in an upload
if opt.MaxUploadParts > 1000 { if opt.MaxUploadParts > 1000 {
opt.MaxUploadParts = 1000 opt.MaxUploadParts = 1000
} }
urlEncodeListings = false urlEncodeListings = false
useAlreadyExists = false // untested
case "SeaweedFS": case "SeaweedFS":
listObjectsV2 = false // untested listObjectsV2 = false // untested
virtualHostStyle = false virtualHostStyle = false
urlEncodeListings = false urlEncodeListings = false
useMultipartEtag = false // untested useMultipartEtag = false // untested
useAlreadyExists = false // untested
case "StackPath": case "StackPath":
listObjectsV2 = false // untested listObjectsV2 = false // untested
virtualHostStyle = false virtualHostStyle = false
urlEncodeListings = false urlEncodeListings = false
useAlreadyExists = false // untested
case "Storj": case "Storj":
// Force chunk size to >= 64 MiB // Force chunk size to >= 64 MiB
if opt.ChunkSize < 64*fs.Mebi { if opt.ChunkSize < 64*fs.Mebi {
opt.ChunkSize = 64 * fs.Mebi opt.ChunkSize = 64 * fs.Mebi
} }
useAlreadyExists = false // returns BucketAlreadyExists
case "Synology": case "Synology":
useMultipartEtag = false useMultipartEtag = false
useAlreadyExists = false // untested
case "TencentCOS": case "TencentCOS":
listObjectsV2 = false // untested listObjectsV2 = false // untested
useMultipartEtag = false // untested useMultipartEtag = false // untested
useAlreadyExists = false // untested
case "Wasabi": case "Wasabi":
// No quirks useAlreadyExists = true // returns 200 OK
case "Leviia": case "Leviia":
// No quirks useAlreadyExists = false // untested
case "Qiniu": case "Qiniu":
useMultipartEtag = false useMultipartEtag = false
urlEncodeListings = false urlEncodeListings = false
virtualHostStyle = false virtualHostStyle = false
useAlreadyExists = false // untested
case "GCS": case "GCS":
// Google break request Signature by mutating accept-encoding HTTP header // Google break request Signature by mutating accept-encoding HTTP header
// https://github.com/rclone/rclone/issues/6670 // https://github.com/rclone/rclone/issues/6670
useAcceptEncodingGzip = false useAcceptEncodingGzip = false
useAlreadyExists = true // returns BucketNameUnavailable instead of BucketAlreadyExists but good enough!
default:
fs.Logf("s3", "s3 provider %q not known - please set correctly", opt.Provider)
fallthrough
case "Other": case "Other":
listObjectsV2 = false listObjectsV2 = false
virtualHostStyle = false virtualHostStyle = false
urlEncodeListings = false urlEncodeListings = false
useMultipartEtag = false useMultipartEtag = false
default: useAlreadyExists = false
fs.Logf("s3", "s3 provider %q not known - please set correctly", opt.Provider)
listObjectsV2 = false
virtualHostStyle = false
urlEncodeListings = false
useMultipartEtag = false
} }
// Path Style vs Virtual Host style // Path Style vs Virtual Host style
@ -3159,6 +3208,12 @@ func setQuirks(opt *Options) {
opt.UseAcceptEncodingGzip.Valid = true opt.UseAcceptEncodingGzip.Valid = true
opt.UseAcceptEncodingGzip.Value = useAcceptEncodingGzip opt.UseAcceptEncodingGzip.Value = useAcceptEncodingGzip
} }
// Has the provider got AlreadyOwnedByYou error?
if !opt.UseAlreadyExists.Valid {
opt.UseAlreadyExists.Valid = true
opt.UseAlreadyExists.Value = useAlreadyExists
}
} }
// setRoot changes the root of the Fs // setRoot changes the root of the Fs
@ -4187,8 +4242,17 @@ func (f *Fs) makeBucket(ctx context.Context, bucket string) error {
fs.Infof(f, "Bucket %q created with ACL %q", bucket, f.opt.BucketACL) fs.Infof(f, "Bucket %q created with ACL %q", bucket, f.opt.BucketACL)
} }
if awsErr, ok := err.(awserr.Error); ok { if awsErr, ok := err.(awserr.Error); ok {
if code := awsErr.Code(); code == "BucketAlreadyOwnedByYou" || code == "BucketAlreadyExists" { switch awsErr.Code() {
case "BucketAlreadyOwnedByYou":
err = nil err = nil
case "BucketAlreadyExists", "BucketNameUnavailable":
if f.opt.UseAlreadyExists.Value {
// We can trust BucketAlreadyExists to mean not owned by us, so make it non retriable
err = fserrors.NoRetryError(err)
} else {
// We can't trust BucketAlreadyExists to mean not owned by us, so ignore it
err = nil
}
} }
} }
return err return err

View file

@ -12,6 +12,7 @@ import (
"time" "time"
"github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/awserr"
"github.com/aws/aws-sdk-go/service/s3" "github.com/aws/aws-sdk-go/service/s3"
"github.com/rclone/rclone/fs" "github.com/rclone/rclone/fs"
"github.com/rclone/rclone/fs/cache" "github.com/rclone/rclone/fs/cache"
@ -393,6 +394,41 @@ func (f *Fs) InternalTestVersions(t *testing.T) {
} }
}) })
t.Run("Mkdir", func(t *testing.T) {
// Test what happens when we create a bucket we already own and see whether the
// quirk is set correctly
req := s3.CreateBucketInput{
Bucket: &f.rootBucket,
ACL: stringPointerOrNil(f.opt.BucketACL),
}
if f.opt.LocationConstraint != "" {
req.CreateBucketConfiguration = &s3.CreateBucketConfiguration{
LocationConstraint: &f.opt.LocationConstraint,
}
}
err := f.pacer.Call(func() (bool, error) {
_, err := f.c.CreateBucketWithContext(ctx, &req)
return f.shouldRetry(ctx, err)
})
var errString string
if err == nil {
errString = "No Error"
} else if awsErr, ok := err.(awserr.Error); ok {
errString = awsErr.Code()
} else {
assert.Fail(t, "Unknown error %T %v", err, err)
}
t.Logf("Creating a bucket we already have created returned code: %s", errString)
switch errString {
case "BucketAlreadyExists":
assert.False(t, f.opt.UseAlreadyExists.Value, "Need to clear UseAlreadyExists quirk")
case "No Error", "BucketAlreadyOwnedByYou":
assert.True(t, f.opt.UseAlreadyExists.Value, "Need to set UseAlreadyExists quirk")
default:
assert.Fail(t, "Unknown error string %q", errString)
}
})
t.Run("Cleanup", func(t *testing.T) { t.Run("Cleanup", func(t *testing.T) {
require.NoError(t, f.CleanUpHidden(ctx)) require.NoError(t, f.CleanUpHidden(ctx))
items := append([]fstest.Item{newItem}, fstests.InternalTestFiles...) items := append([]fstest.Item{newItem}, fstests.InternalTestFiles...)