xl/bootup: Server bootup shouldn't return for missing buckets. (#3255)

Ref #3196

cmd/bucket-notification-handlers.go

@@ -54,6 +54,14 @@ func (api objectAPIHandlers) GetBucketNotificationHandler(w http.ResponseWriter,
 	}
 	vars := mux.Vars(r)
 	bucket := vars["bucket"]
+
+	_, err := objAPI.GetBucketInfo(bucket)
+	if err != nil {
+		errorIf(err, "Unable to find bucket info.")
+		writeErrorResponse(w, r, toAPIErrorCode(err), r.URL.Path)
+		return
+	}
+
 	// Attempt to successfully load notification config.
 	nConfig, err := loadNotificationConfig(bucket, objAPI)
 	if err != nil && err != errNoSuchNotifications {
@@ -177,8 +185,7 @@ func PutBucketNotificationConfig(bucket string, ncfg *notificationConfig, objAPI
 		return fmt.Errorf("Unable to persist Bucket notification config to object layer - config=%v errMsg=%v", *ncfg, err)
 	}
 
-	// All servers (including local) are told to update in-memory
-	// config
+	// All servers (including local) are told to update in-memory config
 	S3PeersUpdateBucketNotification(bucket, ncfg)
 
 	return nil

cmd/bucket-policy-handlers.go

@@ -142,6 +142,14 @@ func (api objectAPIHandlers) PutBucketPolicyHandler(w http.ResponseWriter, r *ht
 	vars := mux.Vars(r)
 	bucket := vars["bucket"]
 
+	// Before proceeding validate if bucket exists.
+	_, err := objAPI.GetBucketInfo(bucket)
+	if err != nil {
+		errorIf(err, "Unable to find bucket info.")
+		writeErrorResponse(w, r, toAPIErrorCode(err), r.URL.Path)
+		return
+	}
+
 	// If Content-Length is unknown or zero, deny the
 	// request. PutBucketPolicy always needs a Content-Length if
 	// incoming request is not chunked.
@@ -200,13 +208,6 @@ func (api objectAPIHandlers) PutBucketPolicyHandler(w http.ResponseWriter, r *ht
 // persists it to storage, and notify nodes in the cluster about the
 // change. In-memory state is updated in response to the notification.
 func persistAndNotifyBucketPolicyChange(bucket string, pCh policyChange, objAPI ObjectLayer) error {
-	// Verify if bucket actually exists. FIXME: Ideally this check
-	// should not be used but is kept here to error out for
-	// invalid and non-existent buckets.
-	if err := isBucketExist(bucket, objAPI); err != nil {
-		return err
-	}
-
 	// Acquire a write lock on bucket before modifying its
 	// configuration.
 	bucketLock := nsMutex.NewNSLock(bucket, "")
@@ -253,12 +254,18 @@ func (api objectAPIHandlers) DeleteBucketPolicyHandler(w http.ResponseWriter, r
 	vars := mux.Vars(r)
 	bucket := vars["bucket"]
 
+	// Before proceeding validate if bucket exists.
+	_, err := objAPI.GetBucketInfo(bucket)
+	if err != nil {
+		errorIf(err, "Unable to find bucket info.")
+		writeErrorResponse(w, r, toAPIErrorCode(err), r.URL.Path)
+		return
+	}
+
 	// Delete bucket access policy, by passing an empty policy
 	// struct.
 	if err := persistAndNotifyBucketPolicyChange(bucket, policyChange{true, nil}, objAPI); err != nil {
 		switch err.(type) {
-		case BucketNameInvalid:
-			writeErrorResponse(w, r, ErrInvalidBucketName, r.URL.Path)
 		case BucketPolicyNotFound:
 			writeErrorResponse(w, r, ErrNoSuchBucketPolicy, r.URL.Path)
 		default:
@@ -292,13 +299,19 @@ func (api objectAPIHandlers) GetBucketPolicyHandler(w http.ResponseWriter, r *ht
 	vars := mux.Vars(r)
 	bucket := vars["bucket"]
 
+	// Before proceeding validate if bucket exists.
+	_, err := objAPI.GetBucketInfo(bucket)
+	if err != nil {
+		errorIf(err, "Unable to find bucket info.")
+		writeErrorResponse(w, r, toAPIErrorCode(err), r.URL.Path)
+		return
+	}
+
 	// Read bucket access policy.
 	policy, err := readBucketPolicy(bucket, objAPI)
 	if err != nil {
 		errorIf(err, "Unable to read bucket policy.")
 		switch err.(type) {
-		case BucketNameInvalid:
-			writeErrorResponse(w, r, ErrInvalidBucketName, r.URL.Path)
 		case BucketPolicyNotFound:
 			writeErrorResponse(w, r, ErrNoSuchBucketPolicy, r.URL.Path)
 		default:

cmd/bucket-policy-handlers_test.go

@@ -250,6 +250,11 @@ func testPutBucketPolicyHandler(obj ObjectLayer, instanceType, bucketName string
 	credentials credential, t *testing.T) {
 	initBucketPolicies(obj)
 
+	bucketName1 := fmt.Sprintf("%s-1", bucketName)
+	if err := obj.MakeBucket(bucketName1); err != nil {
+		t.Fatal(err)
+	}
+
 	// template for constructing HTTP request body for PUT bucket policy.
 	bucketPolicyTemplate := `{"Version":"2012-10-17","Statement":[{"Sid":"","Effect":"Allow","Principal":{"AWS":["*"]},"Action":["s3:GetBucketLocation","s3:ListBucket"],"Resource":["arn:aws:s3:::%s"]},{"Sid":"","Effect":"Allow","Principal":{"AWS":["*"]},"Action":["s3:GetObject"],"Resource":["arn:aws:s3:::%s/this*"]}]}`
 
@@ -327,7 +332,7 @@ func testPutBucketPolicyHandler(obj ObjectLayer, instanceType, bucketName string
 		// setting an invalid bucket policy.
 		// the bucket policy parser will fail.
 		{
-			bucketName:         "non-existent-bucket",
+			bucketName:         bucketName,
 			bucketPolicyReader: bytes.NewReader([]byte("dummy-policy")),
 
 			policyLen:          len([]byte("dummy-policy")),
@@ -339,7 +344,7 @@ func testPutBucketPolicyHandler(obj ObjectLayer, instanceType, bucketName string
 		// Different bucket name used in the HTTP request and the policy string.
 		// checkBucketPolicyResources should fail.
 		{
-			bucketName:         "different-bucket",
+			bucketName:         bucketName1,
 			bucketPolicyReader: bytes.NewReader([]byte(fmt.Sprintf(bucketPolicyTemplate, bucketName, bucketName))),
 
 			policyLen:          len(fmt.Sprintf(bucketPolicyTemplate, bucketName, bucketName)),
@@ -358,7 +363,7 @@ func testPutBucketPolicyHandler(obj ObjectLayer, instanceType, bucketName string
 			policyLen:          len(fmt.Sprintf(bucketPolicyTemplate, bucketName, bucketName)),
 			accessKey:          credentials.AccessKeyID,
 			secretKey:          credentials.SecretAccessKey,
-			expectedRespStatus: http.StatusInternalServerError,
+			expectedRespStatus: http.StatusNotFound,
 		},
 		// Test case - 9.
 		// invalid bucket name is used.
@@ -527,7 +532,7 @@ func testGetBucketPolicyHandler(obj ObjectLayer, instanceType, bucketName string
 			accessKey:            credentials.AccessKeyID,
 			secretKey:            credentials.SecretAccessKey,
 			expectedBucketPolicy: bucketPolicyTemplate,
-			expectedRespStatus:   http.StatusInternalServerError,
+			expectedRespStatus:   http.StatusNotFound,
 		},
 		// Test case - 3.
 		// Case with invalid bucket name.
@@ -736,7 +741,7 @@ func testDeleteBucketPolicyHandler(obj ObjectLayer, instanceType, bucketName str
 			bucketName:         "non-existent-bucket",
 			accessKey:          credentials.AccessKeyID,
 			secretKey:          credentials.SecretAccessKey,
-			expectedRespStatus: http.StatusInternalServerError,
+			expectedRespStatus: http.StatusNotFound,
 		},
 		// Test case - 3.
 		// Case with invalid bucket name.

cmd/bucket-policy.go

@@ -142,11 +142,6 @@ func getOldBucketsConfigPath() (string, error) {
 // readBucketPolicyJSON - reads bucket policy for an input bucket, returns BucketPolicyNotFound
 // if bucket policy is not found.
 func readBucketPolicyJSON(bucket string, objAPI ObjectLayer) (bucketPolicyReader io.Reader, err error) {
-	// Verify if bucket actually exists
-	if err = isBucketExist(bucket, objAPI); err != nil {
-		return nil, err
-	}
-
 	policyPath := pathJoin(bucketConfigPrefix, bucket, policyJSON)
 	objInfo, err := objAPI.GetObjectInfo(minioMetaBucket, policyPath)
 	err = errorCause(err)

cmd/event-notifier.go

@@ -466,7 +466,6 @@ func loadAllBucketNotifications(objAPI ObjectLayer) (map[string]*notificationCon
 		} else {
 			nConfigs[bucket.Name] = nCfg
 		}
-
 		lCfg, lErr := loadListenerConfig(bucket.Name, objAPI)
 		if lErr != nil {
 			if lErr != errNoSuchNotifications {

cmd/web-handlers_test.go

@@ -1118,8 +1118,6 @@ func testWebSetBucketPolicyHandler(obj ObjectLayer, instanceType string, t TestE
 		policy     string
 		pass       bool
 	}{
-		// Inexistent bucket
-		{"fooo", "", "readonly", false},
 		// Invalid bucket name
 		{"", "", "readonly", false},
 		// Invalid policy