gcs: compatible with old gcs bug (#677) (#689)

* cherry pick #677 to release-5.0-rc

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

* fix build

Co-authored-by: 3pointer <luancheng@pingcap.com>

Makefile

@@ -84,6 +84,7 @@ bins:
 	@which bin/tiflash
 	@which bin/libtiflash_proxy.so
 	@which bin/cdc
+	@which bin/fake-gcs-server
 	if [ ! -d bin/flash_cluster_manager ]; then echo "flash_cluster_manager not exist"; exit 1; fi
 
 tools:

go.mod1

@@ -36,3 +36,5 @@ require (
 	google.golang.org/api v0.22.0
 	google.golang.org/grpc v1.27.1
 )
+
+replace cloud.google.com/go/storage => github.com/3pointer/google-cloud-go/storage v1.6.1-0.20210108125931-b59bfa0720b2

go.sum1

@@ -7,7 +7,6 @@ cloud.google.com/go v0.44.2/go.mod h1:60680Gw3Yr4ikxnPRS/oxxkBccT6SA1yMk63TGekxK
 cloud.google.com/go v0.45.1/go.mod h1:RpBamKRgapWJb87xiFSdk4g1CME7QZg3uwTez+TSTjc=
 cloud.google.com/go v0.46.3/go.mod h1:a6bKKbmY7er1mI7TEI4lsAkts/mkhTSZK8w33B4RAg0=
 cloud.google.com/go v0.49.0/go.mod h1:hGvAdzcWNbyuxS3nWhD7H2cIJxjRRTRLQVB0bdputVY=
-cloud.google.com/go v0.50.0/go.mod h1:r9sluTvynVuxRIOHXQEHMFffphuXHOMZMycpNR5e6To=
 cloud.google.com/go v0.51.0/go.mod h1:hWtGJ6gnXH+KgDv+V0zFGDvpi07n3z8ZNj3T1RW0Gcw=
 cloud.google.com/go v0.52.0/go.mod h1:pXajvRH/6o3+F9jDHZWQ5PbGhn+o8w9qiu/CffaVdO4=
 cloud.google.com/go v0.53.0 h1:MZQCQQaRwOrAcuKjiHWHrgKykt4fZyuwF2dtiG3fGW8=
@@ -25,12 +24,10 @@ cloud.google.com/go/pubsub v1.1.0/go.mod h1:EwwdRX2sKPjnvnqCa270oGRyludottCI76h+
 cloud.google.com/go/pubsub v1.2.0 h1:Lpy6hKgdcl7a3WGSfJIFmxmcdjSpP6OmBEfcOv1Y680=
 cloud.google.com/go/pubsub v1.2.0/go.mod h1:jhfEVHT8odbXTkndysNHCcx0awwzvfOlguIAii9o8iA=
 cloud.google.com/go/spanner v1.1.0/go.mod h1:TzTaF9l2ZY2CIetNvVpUu6ZQy8YEOtzB6ICa5EwYjL0=
-cloud.google.com/go/storage v1.0.0/go.mod h1:IhtSnM/ZTZV8YYJWCY8RULGVqBDmpoyjwiyrjsg+URw=
-cloud.google.com/go/storage v1.5.0/go.mod h1:tpKbwo567HUNpVclU5sGELwQWBDZ8gh0ZeosJ0Rtdos=
-cloud.google.com/go/storage v1.6.0 h1:UDpwYIwla4jHGzZJaEJYx1tOejbgSoNqsAfHAUYe2r8=
-cloud.google.com/go/storage v1.6.0/go.mod h1:N7U0C8pVQ/+NIKOBQyamJIeKQKkZ+mxpohlUTyfDhBk=
 collectd.org v0.3.0/go.mod h1:A/8DzQBkF6abtvrT2j/AU/4tiBgJWYyh0y/oB/4MlWE=
 dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU=
+github.com/3pointer/google-cloud-go/storage v1.6.1-0.20210108125931-b59bfa0720b2 h1:Bnm+q0FAI1AXoPX3cPIi2/a4sg2J2f/D0yPr+7EMPg0=
+github.com/3pointer/google-cloud-go/storage v1.6.1-0.20210108125931-b59bfa0720b2/go.mod h1:N7U0C8pVQ/+NIKOBQyamJIeKQKkZ+mxpohlUTyfDhBk=
 github.com/AndreasBriese/bbloom v0.0.0-20180913140656-343706a395b7/go.mod h1:bOvUY6CB00SOBii9/FifXqc0awNKxLFCL/+pkDPuyl8=
 github.com/AstroProfundis/sysinfo v0.0.0-20200423033635-f6f7687215fd/go.mod h1:4m15VhW6ZffaqJNAK/HtN3Qvf97aCe1T39u3UXaA2lA=
 github.com/AstroProfundis/tabby v1.1.0-color/go.mod h1:Wcm+uinH1saEOFGLK2LdY37lAOts8HLevz64Y3y3M3Q=
@@ -784,7 +781,6 @@ github.com/segmentio/kafka-go v0.2.0/go.mod h1:X6itGqS9L4jDletMsxZ7Dz+JFWxM6JHfP
 github.com/sergi/go-diff v1.0.0/go.mod h1:0CfEIISq7TuYL3j771MWULgwwjU+GofnZX9QAmXWZgo=
 github.com/sergi/go-diff v1.0.1-0.20180205163309-da645544ed44/go.mod h1:0CfEIISq7TuYL3j771MWULgwwjU+GofnZX9QAmXWZgo=
 github.com/shirou/gopsutil v2.19.10+incompatible/go.mod h1:5b4v6he4MtMOwMlS0TUMTu2PcXUg8+E1lC7eC3UO/RA=
-github.com/shirou/gopsutil v2.20.3+incompatible h1:0JVooMPsT7A7HqEYdydp/OfjSOYSjhXV7w1hkKj/NPQ=
 github.com/shirou/gopsutil v2.20.3+incompatible/go.mod h1:5b4v6he4MtMOwMlS0TUMTu2PcXUg8+E1lC7eC3UO/RA=
 github.com/shirou/gopsutil v2.20.6+incompatible h1:P37G9YH8M4vqkKcwBosp+URN5O8Tay67D2MbR361ioY=
 github.com/shirou/gopsutil v2.20.6+incompatible/go.mod h1:5b4v6he4MtMOwMlS0TUMTu2PcXUg8+E1lC7eC3UO/RA=
@@ -1119,7 +1115,6 @@ golang.org/x/sys v0.0.0-20190610200419-93c9922d18ae/go.mod h1:h1NjWce9XRLGQEsW7w
 golang.org/x/sys v0.0.0-20190616124812-15dcb6c0061f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20190624142023-c5567b49c5d0/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20190626221950-04f50cda93cb/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
-golang.org/x/sys v0.0.0-20190726091711-fc99dfbffb4e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20190801041406-cbf593c0f2f3/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20190813064441-fde4db37ae7a/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20190826190057-c7b8b68b1456/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
@@ -1193,7 +1188,6 @@ golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtn
 golang.org/x/tools v0.0.0-20191125144606-a911d9008d1f/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
 golang.org/x/tools v0.0.0-20191130070609-6e064ea0cf2d/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
 golang.org/x/tools v0.0.0-20191210221141-98df12377212/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
-golang.org/x/tools v0.0.0-20191216173652-a0e659d51361/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28=
 golang.org/x/tools v0.0.0-20191227053925-7b8e75db28f4/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28=
 golang.org/x/tools v0.0.0-20200108203644-89082a384178/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28=
 golang.org/x/tools v0.0.0-20200117161641-43d50277825c/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28=
@@ -1255,7 +1249,6 @@ google.golang.org/genproto v0.0.0-20191108220845-16a3f7862a1a/go.mod h1:n3cpQtvx
 google.golang.org/genproto v0.0.0-20191114150713-6bbd007550de/go.mod h1:n3cpQtvxv34hfy77yVDNjmbRyujviMdxYliBSkLhpCc=
 google.golang.org/genproto v0.0.0-20191115194625-c23dd37a84c9/go.mod h1:n3cpQtvxv34hfy77yVDNjmbRyujviMdxYliBSkLhpCc=
 google.golang.org/genproto v0.0.0-20191206224255-0243a4be9c8f/go.mod h1:n3cpQtvxv34hfy77yVDNjmbRyujviMdxYliBSkLhpCc=
-google.golang.org/genproto v0.0.0-20191216164720-4f79533eabd1/go.mod h1:n3cpQtvxv34hfy77yVDNjmbRyujviMdxYliBSkLhpCc=
 google.golang.org/genproto v0.0.0-20191230161307-f3c370f40bfb/go.mod h1:n3cpQtvxv34hfy77yVDNjmbRyujviMdxYliBSkLhpCc=
 google.golang.org/genproto v0.0.0-20200108215221-bd8f9a0ef82f/go.mod h1:n3cpQtvxv34hfy77yVDNjmbRyujviMdxYliBSkLhpCc=
 google.golang.org/genproto v0.0.0-20200115191322-ca5a22157cba/go.mod h1:n3cpQtvxv34hfy77yVDNjmbRyujviMdxYliBSkLhpCc=

pkg/mock/mock_cluster.go

@@ -59,11 +59,10 @@ func NewCluster() (*Cluster, error) {
 
 	mvccStore := mocktikv.MustNewMVCCStore()
 	client, cluster, pdClient, err := mocktikv.NewTiKVAndPDClient("")
-	mocktikv.BootstrapWithSingleStore(cluster)
-
 	if err != nil {
 		return nil, errors.Trace(err)
 	}
+	mocktikv.BootstrapWithSingleStore(cluster)
 	storage, err := tikv.NewTestTiKVStore(client, pdClient, nil, nil, 0)
 	if err != nil {
 		return nil, errors.Trace(err)

pkg/storage/gcs.go

@@ -7,12 +7,16 @@ import (
 	"io"
 	"io/ioutil"
 	"path"
+	"strings"
 
 	"cloud.google.com/go/storage"
 	"github.com/pingcap/errors"
 	"github.com/pingcap/kvproto/pkg/backup"
+	"github.com/pingcap/log"
 	"github.com/spf13/pflag"
+	"go.uber.org/zap"
 	"golang.org/x/oauth2/google"
+	"google.golang.org/api/iterator"
 	"google.golang.org/api/option"
 
 	berrors "github.com/pingcap/br/pkg/errors"
@@ -111,8 +115,15 @@ func (s *gcsStorage) Read(ctx context.Context, name string) ([]byte, error) {
 	}
 	defer rc.Close()
 
-	b := make([]byte, rc.Attrs.Size)
-	_, err = io.ReadFull(rc, b)
+	size := rc.Attrs.Size
+	var b []byte
+	if size < 0 {
+		// happened when using fake-gcs-server in integration test
+		b, err = ioutil.ReadAll(rc)
+	} else {
+		b = make([]byte, size)
+		_, err = io.ReadFull(rc, b)
+	}
 	return b, errors.Trace(err)
 }
 
@@ -171,7 +182,9 @@ func newGCSStorage(ctx context.Context, gcs *backup.GCS, opts *ExternalStorageOp
 					"You should provide '--gcs.credentials_file' when '--send-credentials-to-tikv' is true")
 			}
 		}
-		clientOps = append(clientOps, option.WithCredentials(creds))
+		if creds != nil {
+			clientOps = append(clientOps, option.WithCredentials(creds))
+		}
 	} else {
 		clientOps = append(clientOps, option.WithCredentialsJSON([]byte(gcs.GetCredentialsBlob())))
 	}
@@ -193,6 +206,18 @@ func newGCSStorage(ctx context.Context, gcs *backup.GCS, opts *ExternalStorageOp
 	}
 
 	bucket := client.Bucket(gcs.Bucket)
+	// check whether it's a bug before #647, to solve case #2
+	// If the storage is set as gcs://bucket/prefix/,
+	// the backupmeta is written correctly to gcs://bucket/prefix/backupmeta,
+	// but the SSTs are written wrongly to gcs://bucket/prefix//*.sst (note the extra slash).
+	// see details about case 2 at https://github.com/pingcap/br/issues/675#issuecomment-753780742
+	sstInPrefix := hasSSTFiles(ctx, bucket, gcs.Prefix)
+	sstInPrefixSlash := hasSSTFiles(ctx, bucket, gcs.Prefix+"//")
+	if sstInPrefixSlash && !sstInPrefix {
+		// This is a old bug, but we must make it compatible.
+		// so we need find sst in slash directory
+		gcs.Prefix += "//"
+	}
 	if !opts.SkipCheckPath {
 		// check bucket exists
 		_, err = bucket.Attrs(ctx)
@@ -202,3 +227,24 @@ func newGCSStorage(ctx context.Context, gcs *backup.GCS, opts *ExternalStorageOp
 	}
 	return &gcsStorage{gcs: gcs, bucket: bucket}, nil
 }
+
+func hasSSTFiles(ctx context.Context, bucket *storage.BucketHandle, prefix string) bool {
+	query := storage.Query{Prefix: prefix}
+	_ = query.SetAttrSelection([]string{"Name"})
+	it := bucket.Objects(ctx, &query)
+	for {
+		attrs, err := it.Next()
+		if err == iterator.Done { // nolint:errorlint
+			break
+		}
+		if err != nil {
+			log.Warn("failed to list objects on gcs, will use default value for `prefix`", zap.Error(err))
+			break
+		}
+		if strings.HasSuffix(attrs.Name, ".sst") {
+			log.Info("sst file found in prefix slash", zap.String("file", attrs.Name))
+			return true
+		}
+	}
+	return false
+}

pkg/storage/parse.go

@@ -22,18 +22,26 @@ type BackendOptions struct {
 	GCS GCSBackendOptions `json:"gcs" toml:"gcs"`
 }
 
+// ParseRawURL parse raw url to url object.
+func ParseRawURL(rawURL string) (*url.URL, error) {
+	// https://github.com/pingcap/br/issues/603
+	// In aws the secret key may contain '/+=' and '+' has a special meaning in URL.
+	// Replace "+" by "%2B" here to avoid this problem.
+	rawURL = strings.ReplaceAll(rawURL, "+", "%2B")
+	u, err := url.Parse(rawURL)
+	if err != nil {
+		return nil, errors.Trace(err)
+	}
+	return u, nil
+}
+
 // ParseBackend constructs a structured backend description from the
 // storage URL.
 func ParseBackend(rawURL string, options *BackendOptions) (*backup.StorageBackend, error) {
 	if len(rawURL) == 0 {
 		return nil, errors.Annotate(berrors.ErrStorageInvalidConfig, "empty store is not allowed")
 	}
-
-	// https://github.com/pingcap/br/issues/603
-	// In aws the secret key may contain '/+=' and '+' has a special meaning in URL.
-	// Replace "+" by "%2B" here to avoid this problem.
-	rawURL = strings.ReplaceAll(rawURL, "+", "%2B")
-	u, err := url.Parse(rawURL)
+	u, err := ParseRawURL(rawURL)
 	if err != nil {
 		return nil, errors.Trace(err)
 	}

pkg/task/common.go

@@ -7,9 +7,11 @@ import (
 	"crypto/tls"
 	"fmt"
 	"net/url"
+	"path"
 	"strings"
 	"time"
 
+	gcs "cloud.google.com/go/storage"
 	"github.com/gogo/protobuf/proto"
 	"github.com/pingcap/errors"
 	"github.com/pingcap/kvproto/pkg/backup"
@@ -394,7 +396,26 @@ func ReadBackupMeta(
 	}
 	metaData, err := s.Read(ctx, fileName)
 	if err != nil {
-		return nil, nil, nil, errors.Annotate(err, "load backupmeta failed")
+		if gcsObjectNotFound(err) {
+			// change gcs://bucket/abc/def to gcs://bucket/abc and read defbackupmeta
+			oldPrefix := u.GetGcs().GetPrefix()
+			newPrefix, file := path.Split(oldPrefix)
+			newFileName := file + fileName
+			u.GetGcs().Prefix = newPrefix
+			s, err = storage.Create(ctx, u, cfg.SendCreds)
+			if err != nil {
+				return nil, nil, nil, errors.Trace(err)
+			}
+			log.Info("retry load metadata in gcs", zap.String("newPrefix", newPrefix), zap.String("newFileName", newFileName))
+			metaData, err = s.Read(ctx, newFileName)
+			if err != nil {
+				return nil, nil, nil, errors.Trace(err)
+			}
+			// reset prefix for tikv download sst file correctly.
+			u.GetGcs().Prefix = oldPrefix
+		} else {
+			return nil, nil, nil, errors.Annotate(err, "load backupmeta failed")
+		}
 	}
 	backupMeta := &backup.BackupMeta{}
 	if err = proto.Unmarshal(metaData, backupMeta); err != nil {
@@ -466,3 +487,12 @@ func normalizePDURL(pd string, useTLS bool) (string, error) {
 	}
 	return pd, nil
 }
+
+// check whether it's a bug before #647, to solve case #1
+// If the storage is set as gcs://bucket/prefix,
+// the SSTs are written correctly to gcs://bucket/prefix/*.sst
+// but the backupmeta is written wrongly to gcs://bucket/prefixbackupmeta.
+// see details https://github.com/pingcap/br/issues/675#issuecomment-753780742
+func gcsObjectNotFound(err error) bool {
+	return errors.Cause(err) == gcs.ErrObjectNotExist // nolint:errorlint
+}

tests/br_gcs/oauth.py

@@ -0,0 +1,9 @@
+#!/usr/bin/env python3
+
+from flask import Flask
+
+app = Flask(__name__)
+
+@app.route('/oauth/token', methods=['GET', 'POST'])
+def oauth():
+    return '{"access_token": "ok", "token_type":"service_account", "expires_in":3600}'