Make mutateDbConfig only return errors for actual errors (#6952)

* Make mutateDbConfig only return errors when there is an error * explicit 200/updated response
couchbase · Jul 9, 2024 · 5ccf1a6 · 5ccf1a6
1 parent 5e46564
commit 5ccf1a6
Showing 1 changed file with 43 additions and 44 deletions.
diff --git a/rest/handler_config_database.go b/rest/handler_config_database.go
@@ -35,63 +35,62 @@ func (h *handler) getDBConfig() (config *DbConfig, etagVersion string, err error
 	}
 }
 
-// Updates the database config via a callback function that can modify a `DbConfig`.
-// Note: This always returns a non-nil error; on success it's an HTTPError with status OK.
-// The calling handler method is expected to simply return the result.
+// Updates the database config via a callback function that can modify a `DbConfig`
 func (h *handler) mutateDbConfig(mutator func(*DbConfig) error) error {
 	h.assertAdminOnly()
+
+	if !h.server.persistentConfig {
+		return base.HTTPErrorf(http.StatusServiceUnavailable, "persistent config is disabled")
+	}
+
 	dbName := h.db.Name
 	validateOIDC := !h.getBoolQuery(paramDisableOIDCValidation)
 
-	if h.server.persistentConfig {
-		// Update persistently-stored config:
-		bucket := h.db.Bucket.GetName()
-		var updatedDbConfig *DatabaseConfig
-		cas, err := h.server.BootstrapContext.UpdateConfig(h.ctx(), bucket, h.server.Config.Bootstrap.ConfigGroupID, dbName, func(bucketDbConfig *DatabaseConfig) (updatedConfig *DatabaseConfig, err error) {
-
-			if h.headerDoesNotMatchEtag(bucketDbConfig.Version) {
-				return nil, base.HTTPErrorf(http.StatusPreconditionFailed, "Provided If-Match header does not match current config version")
-			}
+	// Update persistently-stored config:
+	bucket := h.db.Bucket.GetName()
+	var updatedDbConfig *DatabaseConfig
+	cas, err := h.server.BootstrapContext.UpdateConfig(h.ctx(), bucket, h.server.Config.Bootstrap.ConfigGroupID, dbName, func(bucketDbConfig *DatabaseConfig) (updatedConfig *DatabaseConfig, err error) {
 
-			// Now call the mutator function:
-			if err := mutator(&bucketDbConfig.DbConfig); err != nil {
-				return nil, err
-			}
+		if h.headerDoesNotMatchEtag(bucketDbConfig.Version) {
+			return nil, base.HTTPErrorf(http.StatusPreconditionFailed, "Provided If-Match header does not match current config version")
+		}
 
-			validateReplications := false
-			if err := bucketDbConfig.validate(h.ctx(), validateOIDC, validateReplications); err != nil {
-				return nil, base.HTTPErrorf(http.StatusBadRequest, err.Error())
-			}
+		// Now call the mutator function:
+		if err := mutator(&bucketDbConfig.DbConfig); err != nil {
+			return nil, err
+		}
 
-			bucketDbConfig.Version, err = GenerateDatabaseConfigVersionID(h.ctx(), bucketDbConfig.Version, &bucketDbConfig.DbConfig)
-			if err != nil {
-				return nil, err
-			}
-			updatedDbConfig = bucketDbConfig
-			return bucketDbConfig, nil
-		})
-		if err != nil {
-			return err
+		validateReplications := false
+		if err := bucketDbConfig.validate(h.ctx(), validateOIDC, validateReplications); err != nil {
+			return nil, base.HTTPErrorf(http.StatusBadRequest, err.Error())
 		}
-		updatedDbConfig.cfgCas = cas
 
-		dbCreds := h.server.Config.DatabaseCredentials[dbName]
-		bucketCreds := h.server.Config.BucketCredentials[bucket]
-		if err := updatedDbConfig.setup(h.ctx(), dbName, h.server.Config.Bootstrap, dbCreds, bucketCreds, h.server.Config.IsServerless()); err != nil {
-			return err
+		bucketDbConfig.Version, err = GenerateDatabaseConfigVersionID(h.ctx(), bucketDbConfig.Version, &bucketDbConfig.DbConfig)
+		if err != nil {
+			return nil, err
 		}
+		updatedDbConfig = bucketDbConfig
+		return bucketDbConfig, nil
+	})
+	if err != nil {
+		return err
+	}
+	updatedDbConfig.cfgCas = cas
 
-		h.server.lock.Lock()
-		defer h.server.lock.Unlock()
+	dbCreds := h.server.Config.DatabaseCredentials[dbName]
+	bucketCreds := h.server.Config.BucketCredentials[bucket]
+	if err := updatedDbConfig.setup(h.ctx(), dbName, h.server.Config.Bootstrap, dbCreds, bucketCreds, h.server.Config.IsServerless()); err != nil {
+		return err
+	}
 
-		// TODO: Dynamic update instead of reload
-		if err := h.server._reloadDatabaseWithConfig(h.ctx(), *updatedDbConfig, false); err != nil {
-			return err
-		}
-		h.setEtag(updatedDbConfig.Version)
-		return base.HTTPErrorf(http.StatusOK, "updated")
+	h.server.lock.Lock()
+	defer h.server.lock.Unlock()
 
-	} else {
-		return base.HTTPErrorf(http.StatusServiceUnavailable, "Unavailable")
+	// TODO: Dynamic update instead of reload
+	if err := h.server._reloadDatabaseWithConfig(h.ctx(), *updatedDbConfig, false); err != nil {
+		return err
 	}
+	h.setEtag(updatedDbConfig.Version)
+	h.setStatus(http.StatusOK, "updated")
+	return nil
 }