From 6b329ce01fee1f2983ba1d65a60e7f31686051e2 Mon Sep 17 00:00:00 2001 From: Anis Elleuch Date: Wed, 6 Oct 2021 17:56:57 +0100 Subject: [PATCH] rb: Fix side wide removal of an alias (#3796) * rb: Fix side wide removal of an alias rb --force --dangerous should remove all buckets under an alias. This PR fixes a recent regression. --- .github/workflows/go.yml | 5 ++++ cmd/rb-main.go | 65 ++++++++++++++++++++++++++++++++-------- functional-tests.sh | 31 +++++++++++++++++++ testdata/localhost.crt | 22 ++++++++++++++ testdata/localhost.key | 27 +++++++++++++++++ 5 files changed, 137 insertions(+), 13 deletions(-) create mode 100644 testdata/localhost.crt create mode 100644 testdata/localhost.key diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index fc05d017..bf15b331 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -38,6 +38,11 @@ jobs: - name: Build on ${{ matrix.os }} if: matrix.os == 'ubuntu-latest' run: | + wget https://dl.min.io/server/minio/release/linux-amd64/minio && chmod +x minio + mkdir -p ~/.minio/certs/ && cp testdata/localhost.crt ~/.minio/certs/public.crt && cp testdata/localhost.key ~/.minio/certs/private.key + sudo cp testdata/localhost.crt /usr/local/share/ca-certificates/ && sudo update-ca-certificates + ./minio server /tmp/test-xl/{1...4}/ & sleep 10 + export SERVER_ENDPOINT=localhost:9000 ACCESS_KEY=minioadmin SECRET_KEY=minioadmin ENABLE_HTTPS=1 make make test-race make verify diff --git a/cmd/rb-main.go b/cmd/rb-main.go index 9b305310..926870ce 100644 --- a/cmd/rb-main.go +++ b/cmd/rb-main.go @@ -20,9 +20,9 @@ package cmd import ( "context" "fmt" + "path" "path/filepath" "strings" - "time" "github.com/fatih/color" "github.com/minio/cli" @@ -107,7 +107,7 @@ func checkRbSyntax(ctx context.Context, cliCtx *cli.Context) { isDangerous := cliCtx.Bool("dangerous") for _, url := range cliCtx.Args() { - if isNamespaceRemoval(ctx, url) { + if isS3NamespaceRemoval(ctx, url) { if isForce && isDangerous { continue } @@ -117,6 +117,40 @@ func checkRbSyntax(ctx context.Context, cliCtx *cli.Context) { } } +// Return a list of aliased urls of buckets under the passed url +func listBucketsURLs(ctx context.Context, url string) ([]string, *probe.Error) { + var buckets []string + + targetAlias, targetURL, _ := mustExpandAlias(url) + clnt, err := newClientFromAlias(targetAlias, targetURL) + if err != nil { + return nil, err + } + + opts := ListOptions{ + ShowDir: DirLast, + } + + for content := range clnt.List(ctx, opts) { + if content.Err != nil { + errorIf(content.Err, "") + continue + } + + select { + case <-ctx.Done(): + return nil, probe.NewError(ctx.Err()) + default: + } + + bucketName := strings.TrimPrefix(content.URL.Path, clnt.GetURL().Path) + bucketPath := path.Join(url, bucketName) + buckets = append(buckets, bucketPath) + } + + return buckets, nil +} + // Delete a bucket and all its objects and versions will be removed as well. func deleteBucket(ctx context.Context, url string, isForce bool) *probe.Error { targetAlias, targetURL, _ := mustExpandAlias(url) @@ -179,18 +213,15 @@ func deleteBucket(ctx context.Context, url string, isForce bool) *probe.Error { return err } -// isNamespaceRemoval returns true if alias +// isS3NamespaceRemoval returns true if alias // is not qualified by bucket -func isNamespaceRemoval(ctx context.Context, url string) bool { +func isS3NamespaceRemoval(ctx context.Context, url string) bool { // clean path for aliases like s3/. //Note: UNC path using / works properly in go 1.9.2 even though it breaks the UNC specification. url = filepath.ToSlash(filepath.Clean(url)) // namespace removal applies only for non FS. So filter out if passed url represents a directory - if !isAliasURLDir(ctx, url, nil, time.Time{}) { - _, path := url2Alias(url) - return (path == "") - } - return false + _, path := url2Alias(url) + return (path == "") } // mainRemoveBucket is entry point for rb command. @@ -250,12 +281,20 @@ func mainRemoveBucket(cliCtx *cli.Context) error { fatalIf(errDummy().Trace(), "`"+targetURL+"` is not empty. Retry this command with ‘--force’ flag if you want to remove `"+targetURL+"` and all its contents") } - e := deleteBucket(ctx, targetURL, isForce) - fatalIf(e.Trace(targetURL), "Failed to remove `"+targetURL+"`.") + var bucketsURL []string + if isS3NamespaceRemoval(ctx, targetURL) { + bucketsURL, err = listBucketsURLs(ctx, targetURL) + fatalIf(err.Trace(targetURL), "Failed to remove `"+targetURL+"`.") + } else { + bucketsURL = []string{targetURL} + } + + for _, bucketURL := range bucketsURL { + e := deleteBucket(ctx, bucketURL, isForce) + fatalIf(e.Trace(bucketURL), "Failed to remove `"+bucketURL+"`.") - if !isNamespaceRemoval(ctx, targetURL) { printMsg(removeBucketMessage{ - Bucket: targetURL, Status: "success", + Bucket: bucketURL, Status: "success", }) } } diff --git a/functional-tests.sh b/functional-tests.sh index 8631a5f6..340e7a5e 100755 --- a/functional-tests.sh +++ b/functional-tests.sh @@ -255,6 +255,36 @@ function test_make_bucket_error() { log_success "$start_time" "${FUNCNAME[0]}" } +function test_rb() +{ + show "${FUNCNAME[0]}" + + start_time=$(get_time) + bucket1="mc-test-bucket-$RANDOM-1" + bucket2="mc-test-bucket-$RANDOM-2" + object_name="mc-test-object-$RANDOM" + + # Tets rb when the bucket is empty + assert_success "$start_time" "${FUNCNAME[0]}" mc_cmd mb "${SERVER_ALIAS}/${bucket1}" + assert_success "$start_time" "${FUNCNAME[0]}" mc_cmd rb "${SERVER_ALIAS}/${bucket1}" + + # Test rb with --force flag when the bucket is not empty + assert_success "$start_time" "${FUNCNAME[0]}" mc_cmd mb "${SERVER_ALIAS}/${bucket1}" + assert_success "$start_time" "${FUNCNAME[0]}" mc_cmd cp "${FILE_1_MB}" "${SERVER_ALIAS}/${bucket1}/${object_name}" + assert_failure "$start_time" "${FUNCNAME[0]}" mc_cmd rb "${SERVER_ALIAS}/${bucket1}" + assert_success "$start_time" "${FUNCNAME[0]}" mc_cmd rb --force "${SERVER_ALIAS}/${bucket1}" + + # Test rb with --force and --dangerous to remove a site content + assert_success "$start_time" "${FUNCNAME[0]}" mc_cmd mb "${SERVER_ALIAS}/${bucket1}" + assert_success "$start_time" "${FUNCNAME[0]}" mc_cmd mb "${SERVER_ALIAS}/${bucket2}" + assert_success "$start_time" "${FUNCNAME[0]}" mc_cmd cp "${FILE_1_MB}" "${SERVER_ALIAS}/${bucket1}/${object_name}" + assert_success "$start_time" "${FUNCNAME[0]}" mc_cmd cp "${FILE_1_MB}" "${SERVER_ALIAS}/${bucket2}/${object_name}" + assert_failure "$start_time" "${FUNCNAME[0]}" mc_cmd rb --force "${SERVER_ALIAS}/" + assert_success "$start_time" "${FUNCNAME[0]}" mc_cmd rb --force --dangerous "${SERVER_ALIAS}" + + log_success "$start_time" "${FUNCNAME[0]}" +} + function setup() { start_time=$(get_time) @@ -889,6 +919,7 @@ function run_test() { test_make_bucket test_make_bucket_error + test_rb setup test_put_object diff --git a/testdata/localhost.crt b/testdata/localhost.crt new file mode 100644 index 00000000..eefae856 --- /dev/null +++ b/testdata/localhost.crt @@ -0,0 +1,22 @@ +-----BEGIN CERTIFICATE----- +MIIDojCCAoqgAwIBAgIUHE3HUt7gKVBp7wT9Hjj4wRT1xywwDQYJKoZIhvcNAQEL +BQAwejELMAkGA1UEBhMCVVMxCzAJBgNVBAgMAkNBMRgwFgYDVQQHDA9jdXN0b20t +bG9jYXRpb24xHDAaBgNVBAoME2N1c3RvbS1vcmdhbml6YXRpb24xEjAQBgNVBAsM +CWN1c3RvbS1vdTESMBAGA1UEAwwJbG9jYWxob3N0MCAXDTIxMDkyMDEyMTAyOFoY +DzIxMjEwODI3MTIxMDI4WjB6MQswCQYDVQQGEwJVUzELMAkGA1UECAwCQ0ExGDAW +BgNVBAcMD2N1c3RvbS1sb2NhdGlvbjEcMBoGA1UECgwTY3VzdG9tLW9yZ2FuaXph +dGlvbjESMBAGA1UECwwJY3VzdG9tLW91MRIwEAYDVQQDDAlsb2NhbGhvc3QwggEi +MA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQC6mSJV5oz171+P/G1X07Y4HpTS +ZR/TC5lHAYh2YxecOvuuQHBYCgjXbSBmRQXuPRZIy8mXAa6miwssGgSlMKaH6ZV/ +s82ViCSf4Tj1W9on1DvD8bNgCVs0aYA2DmbOyeSyQyyH35TLIaZ2C9fURArixDce +rsUjLMv/GbrL2o2Ia+x4vBSEqUEXlfs7nA1jbm0fYoKlcxjnvDmkIYBCdMqNveHD +DLXzCuXKq4aMLlM+1s8h5ShVAhxpCzY3xncfOvAGA1JWVjEsMUd8PQ4+K88JKVIG +H3HZmL0exjNF4Ks7ocINqQJk7RY0ZUJc10JnCgdkY5bnHuGKzj4Ddq3Ta5FrAgMB +AAGjHjAcMBoGA1UdEQQTMBGHBH8AAAGCCWxvY2FsaG9zdDANBgkqhkiG9w0BAQsF +AAOCAQEAREoEGX34C0+osOgCPwZ4mFXVvssJRJTc4JL/kqOGPyzS/085MzeIpOhu +gkOm3zj175SKiZMWWpYhB+Q/1T6tDiVLidEY7HK/dcbjYfoTAd42cQWY4qmG68vG +E6WnTQjal0uPuCwKqvqIRgkWLpaIV4TmHtpCFLLlVlnDWQBpAdiK/Vk0EeTDDaqd +cROr4tm/8EBxqwUnIQF8vgbXgVT5BNdcp44LWs7A558CCRrCGifch5+kxkSSdAFG +Y7BEXvnnsxU1n9AAVKJYnp1wUN2Hk4KWyPcQb9/Ee45DKDR5KNyMqClsWWXMJr40 +FrgI6euT50Wzo8Qqbls5Bt/0IzObnA== +-----END CERTIFICATE----- diff --git a/testdata/localhost.key b/testdata/localhost.key new file mode 100644 index 00000000..ffc07f0b --- /dev/null +++ b/testdata/localhost.key @@ -0,0 +1,27 @@ +-----BEGIN RSA PRIVATE KEY----- +MIIEpAIBAAKCAQEAupkiVeaM9e9fj/xtV9O2OB6U0mUf0wuZRwGIdmMXnDr7rkBw +WAoI120gZkUF7j0WSMvJlwGuposLLBoEpTCmh+mVf7PNlYgkn+E49VvaJ9Q7w/Gz +YAlbNGmANg5mzsnkskMsh9+UyyGmdgvX1EQK4sQ3Hq7FIyzL/xm6y9qNiGvseLwU +hKlBF5X7O5wNY25tH2KCpXMY57w5pCGAQnTKjb3hwwy18wrlyquGjC5TPtbPIeUo +VQIcaQs2N8Z3HzrwBgNSVlYxLDFHfD0OPivPCSlSBh9x2Zi9HsYzReCrO6HCDakC +ZO0WNGVCXNdCZwoHZGOW5x7his4+A3at02uRawIDAQABAoIBAElz2mZCGR7+mXmO +fmRiPIqezyp7ECn9mNqwqc0geLzRIx2W1CJz4MMce/KGHS2I8mq5faNp0BxTA5Ta +sRVtr0A1HNpmJvlD3FbrS4aaH6gqDVS2okudoz9ggE3HIYUpSFM7yh26T1Ie7u3s +/4rZNgfKAYCcf5G3Ip5KvJNedvRKC2w5UX4iYU5eMuXs0YPy2+DtTKxZKIEnI3x4 +VpMe652I7GhjkLSsyuW5NoDeSWmmqhitm1bibuZQBR0ogFyAU94rZk8+j4hiUl9k +7ZVi00tKNp7EcLIwbY02ZCpZ2xvat8W1SLeWRUpExPj/7nnA9Fafrl5M8aHgl8R1 +N0Fd+zECgYEA4GruRDPPNPEXPByes6jjcxfvdoYIDyma1V+FN5Nn+/QPL/vWp9t0 ++mH344iwWkEyZZHNUpNuy5dpOA9724iFFLp9Btq06OgzzDuJj1LDbDpB4zwltuQA +4s7ekTFi9rHDOeAyTREHgDu3uya/2qq53Ms6w8gXM4/kmp9/S4N5VhcCgYEA1Nuv +J1khBitNiNbzHYG9DRV4HLfIB8FFDKtgnW8HvSaWUtHKI4YitwJVFfa5Epz9MDFy +tkFAD9Bu0HqZQ6OQZxGo0rDUcFics9Ftw+w3p/PIocPQBK7PNQ3L9BQS9M3stL+k +fW5r+kUvfZaJVE8agCQQnzkJJh9oexJjMWyxB80CgYBa5BQSPWWLjKWba//+xcUx +BR2wREKZWYFjL+e1hZcU3VkVVwsuOtza17jdR6wdMdCmgHHHIv05qd4snWDNnjJA +HfOrRgMFXZ409lwVVzDc8Y9j6CViOF//fEd6SKVLQt3N3/afbek6z3TvcJc9ie3y +9cCcMLrs4Dd3RGf6/omzCwKBgQChwWxCf6Xr9T5PjeFke/I5niYP1M2KryGU9itO +mFCOOmOj/k8ZXdbFsl0Meti7v1dcp0cgH0fafK+peHE+CG81FCNyMPTPh1dWAwHi +EIFe/ZBq9c3/sQQ/sgNasWKSbGbEGJqcwywFHUxwqNQloJNn64BCL2q3cMjKNffx +WELTxQKBgQDe7adrUBtk8+YZ1/c2VsV1oBq6eI2U+1bWa/8fwXWd9ylETBMtcv20 +Fs8UAFWLz78aWaXWWSSEmFvUbjXPBDvzXCObb6HdhXOCF1n74hKU+z06MAUa1eFC +V0GvBx4v1rc1pZ7grBZwGteeVWVgCAZ487m5TATWqxuarH6yfU0Ptg== +-----END RSA PRIVATE KEY-----