Avoid ListBuckets to keep Stat() ops on bucket simpler

Due to multi-user feature on Minio and mc adoption in many different restrictive environments, most users disable access to ListBuckets() calls. We can avoid such network operations as they only add marginal value.
minio · Feb 28, 2019 · 561afdb · 561afdb
1 parent bd5372f
commit 561afdb
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 58 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -16,7 +16,7 @@ matrix:
       env:
         - ARCH=x86_64
         - GO111MODULE=on
-      go: 1.11.4
+      go: 1.12
       script:
         - diff -au <(gofmt -d *.go) <(printf "")
         - diff -au <(gofmt -d cmd) <(printf "")
@@ -28,7 +28,7 @@ matrix:
       env:
         - ARCH=x86_64
         - GO111MODULE=on
-      go: 1.11.4
+      go: 1.12
       script:
         - go build --ldflags="$(go run buildscripts/gen-ldflags.go)" -o %GOPATH%\bin\mc.exe
         - for d in $(go list ./...); do go test -v -mod=vendor -race "$d"; done

diff --git a/Makefile b/Makefile
@@ -14,7 +14,7 @@ checks:
 
 getdeps:
 	@GO111MODULE=on
-	@echo "Installing golint" && go get -u golang.org/x/lint/golint
+	@echo "Installing golint" && go get golang.org/x/lint/golint
 	@echo "Installing gocyclo" && go get -u github.com/fzipp/gocyclo
 	@echo "Installing misspell" && go get -u github.com/client9/misspell/cmd/misspell
 	@echo "Installing ineffassign" && go get -u github.com/gordonklaus/ineffassign

diff --git a/cmd/client-s3.go b/cmd/client-s3.go
@@ -1012,15 +1012,11 @@ func (c *s3Client) Stat(isIncomplete, isFetchMeta bool, sse encrypt.ServerSide)
 	}
 
 	if object == "" {
-		exists, e := c.api.BucketExists(bucket)
-		if e != nil {
-			return nil, probe.NewError(e)
-		}
-		if !exists {
-			return nil, probe.NewError(BucketDoesNotExist{Bucket: bucket})
+		content, err := c.bucketStat(bucket)
+		if err != nil {
+			return nil, err.Trace(bucket)
 		}
-		bucketMetadata := c.bucketStat()
-		return &bucketMetadata, nil
+		return content, nil
 	}
 
 	// The following code tries to calculate if a given prefix/object does really exist
@@ -1474,18 +1470,18 @@ func (c *s3Client) listIncompleteRecursiveInRoutineDirOpt(contentCh chan *client
 	var allBuckets bool
 	// List all buckets if bucket and object are empty.
 	if bucket == "" && object == "" {
-		var err error
+		var e error
 		allBuckets = true
-		buckets, err = c.api.ListBuckets()
-		if err != nil {
-			contentCh <- &clientContent{URL: *c.targetURL, Err: probe.NewError(err)}
+		buckets, e = c.api.ListBuckets()
+		if e != nil {
+			contentCh <- &clientContent{Err: probe.NewError(e)}
+			return
 		}
 	} else if object == "" {
 		// Get bucket stat if object is empty.
-		content := c.bucketStat()
-		cContent = &content
-		if content.Err != nil {
-			contentCh <- cContent
+		content, err := c.bucketStat(bucket)
+		if err != nil {
+			contentCh <- &clientContent{Err: err.Trace(bucket)}
 			return
 		}
 		buckets = append(buckets, minio.BucketInfo{Name: bucket, CreationDate: content.Time})
@@ -1495,7 +1491,7 @@ func (c *s3Client) listIncompleteRecursiveInRoutineDirOpt(contentCh chan *client
 		content, perr := c.Stat(isIncomplete, false, nil)
 		cContent = content
 		if perr != nil {
-			contentCh <- &clientContent{URL: *c.targetURL, Err: perr}
+			contentCh <- &clientContent{Err: perr.Trace(bucket)}
 			return
 		}
 		buckets = append(buckets, minio.BucketInfo{Name: bucket, CreationDate: content.Time})
@@ -1553,21 +1549,15 @@ func (c *s3Client) objectInfo2ClientContent(bucket string, entry minio.ObjectInf
 }
 
 // Returns bucket stat info of current bucket.
-func (c *s3Client) bucketStat() clientContent {
-	bucketName, _ := c.url2BucketAndObject()
-
-	buckets, err := c.api.ListBuckets()
-	if err != nil {
-		return clientContent{Err: probe.NewError(err)}
+func (c *s3Client) bucketStat(bucket string) (*clientContent, *probe.Error) {
+	exists, e := c.api.BucketExists(bucket)
+	if e != nil {
+		return nil, probe.NewError(e)
 	}
-
-	for _, bucket := range buckets {
-		if bucket.Name == bucketName {
-			return clientContent{URL: *c.targetURL, Time: bucket.CreationDate, Type: os.ModeDir}
-		}
+	if !exists {
+		return nil, probe.NewError(BucketDoesNotExist{Bucket: bucket})
 	}
-
-	return clientContent{Err: probe.NewError(BucketDoesNotExist{Bucket: bucketName})}
+	return &clientContent{URL: *c.targetURL, Time: time.Unix(0, 0), Type: os.ModeDir}, nil
 }
 
 // Recursively lists objects.
@@ -1618,18 +1608,18 @@ func (c *s3Client) listRecursiveInRoutineDirOpt(contentCh chan *clientContent, d
 	var allBuckets bool
 	// List all buckets if bucket and object are empty.
 	if bucket == "" && object == "" {
-		var err error
+		var e error
 		allBuckets = true
-		buckets, err = c.api.ListBuckets()
-		if err != nil {
-			contentCh <- &clientContent{URL: *c.targetURL, Err: probe.NewError(err)}
+		buckets, e = c.api.ListBuckets()
+		if e != nil {
+			contentCh <- &clientContent{Err: probe.NewError(e)}
+			return
 		}
 	} else if object == "" {
 		// Get bucket stat if object is empty.
-		content := c.bucketStat()
-		cContent = &content
-		if content.Err != nil {
-			contentCh <- cContent
+		content, err := c.bucketStat(bucket)
+		if err != nil {
+			contentCh <- &clientContent{Err: err.Trace(bucket)}
 			return
 		}
 		buckets = append(buckets, minio.BucketInfo{Name: bucket, CreationDate: content.Time})
@@ -1640,7 +1630,7 @@ func (c *s3Client) listRecursiveInRoutineDirOpt(contentCh chan *clientContent, d
 		content, perr := c.Stat(isIncomplete, isFetchMeta, nil)
 		cContent = content
 		if perr != nil {
-			contentCh <- &clientContent{URL: *c.targetURL, Err: perr}
+			contentCh <- &clientContent{Err: perr.Trace(bucket)}
 			return
 		}
 		buckets = append(buckets, minio.BucketInfo{Name: bucket, CreationDate: content.Time})
@@ -1692,23 +1682,12 @@ func (c *s3Client) listInRoutine(contentCh chan *clientContent) {
 			contentCh <- content
 		}
 	case b != "" && !strings.HasSuffix(c.targetURL.Path, string(c.targetURL.Separator)) && o == "":
-		buckets, e := c.api.ListBuckets()
-		if e != nil {
-			contentCh <- &clientContent{
-				Err: probe.NewError(e),
-			}
-		}
-		for _, bucket := range buckets {
-			if bucket.Name == b {
-				content := &clientContent{}
-				content.URL = *c.targetURL
-				content.Size = 0
-				content.Time = bucket.CreationDate
-				content.Type = os.ModeDir
-				contentCh <- content
-				break
-			}
+		content, err := c.bucketStat(b)
+		if err != nil {
+			contentCh <- &clientContent{Err: err.Trace(b)}
+			return
 		}
+		contentCh <- content
 	default:
 		isRecursive := false
 		for object := range c.listObjectWrapper(b, o, isRecursive, nil) {

diff --git a/go.sum b/go.sum
@@ -180,6 +180,7 @@ google.golang.org/genproto v0.0.0-20180817151627-c66870c02cf8 h1:Nw54tB0rB7hY/N0
 google.golang.org/genproto v0.0.0-20180817151627-c66870c02cf8/go.mod h1:JiN7NxoALGmiZfu7CAH4rXhgtRTLTxftemlI0sWmxmc=
 google.golang.org/grpc v1.18.0 h1:IZl7mfBGfbhYx2p2rKRtYgDFw6SBz+kclmxYrCksPPA=
 google.golang.org/grpc v1.18.0/go.mod h1:6QZJwpn2B+Zp71q/5VxRsJ6NXXVCE5NRUHRo+f3cWCs=
+google.golang.org/grpc v1.19.0 h1:cfg4PD8YEdSFnm7qLV4++93WcmhH2nIUhMjhdCvl3j8=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 h1:qIbj1fsPNlZgppZ+VLlY7N33q108Sa+fhmuc+sWQYwY=
 gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=