Delete database using config file (#46437)

== Goal ==

Currently the config file (`MB_CONFIG_FILE_PATH`) only allows adding or
changing databases, but not removing them.  Since connected databases
can be managed as a service by us, it becomes necessary to delete any
information Metabase has about them, including any stored credentials.

Since this is a destructive operation with no way to revert (without
restoring from a backup), we raise the bar for accidental deletion and
require not just a (boolean) flag, but a magic string to be set.

== How to test ==

1. Configure a database as described in https://www.metabase.com/docs/latest/configuring-metabase/config-file#databases.
2. Start your Metabase instance.
3. Verify the database shows up in the "admin" section and it works.
   Create a dashboard and questions using the database.
4. Add a `delete: "DELETE_WITH_DEPENDENTS:${name}"` line to your
   database configuration section.
5. Restart your Metabase instance.
6. Verify the database no longer shows up in the "admin" section.
   Notice that questions using it are gone, too.

References: https://github.com/metabase/harbormaster/issues/5173

enterprise/backend/src/metabase_enterprise/advanced_config/file/databases.clj

@@ -37,18 +37,31 @@
 
 (defn- init-from-config-file!
   [database]
-  ;; assert that we are able to connect to this Database. Otherwise, throw an Exception.
-  (driver.u/can-connect-with-details? (keyword (:engine database)) (:details database) :throw-exceptions)
-  (if-let [existing-database-id (t2/select-one-pk Database :engine (:engine database), :name (:name database))]
+  (if (contains? database :delete)
+    ;; Databases can be managed as a service by us.  When the service is canceled, we need to delete any information
+    ;; Metabase has about them, including any stored credentials.  This is a config file flag instead of a CLI command,
+    ;; so we can ensure the database stays deleted even after restoring backups.
+    (let [magic-request (format "DELETE_WITH_DEPENDENTS:%s" (:name database))]
+      (log/info (u/format-color :blue "Deleting databases via the config file is an internal feature subject to breaking changes."))
+      (when (not= magic-request (:delete database))
+        (throw (ex-info (format "To delete database %s set `delete` to %s" (pr-str (:name database)) (pr-str magic-request))
+                        {:database-name (:name database)})))
+      (when-let [existing-database-id (t2/select-one-pk Database :engine (:engine database), :name (:name database))]
+        (log/info (u/format-color :blue "Deleting Database %s %s" (:engine database) (pr-str (:name database))))
+        (t2/delete! Database existing-database-id)))
     (do
-      (log/info (u/format-color :blue "Updating Database %s %s" (:engine database) (pr-str (:name database))))
-      (t2/update! Database existing-database-id database))
-    (do
-      (log/info (u/format-color :green "Creating new %s Database %s" (:engine database) (pr-str (:name database))))
-      (let [db (first (t2/insert-returning-instances! Database database))]
-        (if (config-from-file-sync-databases)
-          ((requiring-resolve 'metabase.sync/sync-database!) db)
-          (log/info "Sync on database creation when initializing from file is disabled. Skipping sync."))))))
+      ;; assert that we are able to connect to this Database. Otherwise, throw an Exception.
+      (driver.u/can-connect-with-details? (keyword (:engine database)) (:details database) :throw-exceptions)
+      (if-let [existing-database-id (t2/select-one-pk Database :engine (:engine database), :name (:name database))]
+        (do
+          (log/info (u/format-color :blue "Updating Database %s %s" (:engine database) (pr-str (:name database))))
+          (t2/update! Database existing-database-id database))
+        (do
+          (log/info (u/format-color :green "Creating new %s Database %s" (:engine database) (pr-str (:name database))))
+          (let [db (first (t2/insert-returning-instances! Database database))]
+            (if (config-from-file-sync-databases)
+              ((requiring-resolve 'metabase.sync/sync-database!) db)
+              (log/info "Sync on database creation when initializing from file is disabled. Skipping sync."))))))))
 
 (defmethod advanced-config.file.i/initialize-section! :databases
   [_section-name databases]

enterprise/backend/test/metabase_enterprise/advanced_config/file/databases_test.clj

@@ -7,7 +7,9 @@
    [metabase.models :refer [Database Table]]
    [metabase.test :as mt]
    [metabase.util :as u]
-   [toucan2.core :as t2]))
+   [toucan2.core :as t2])
+  (:import
+    (clojure.lang ExceptionInfo)))
 
 (use-fixtures :each (fn [thunk]
                       (binding [advanced-config.file/*supported-versions* {:min 1, :max 1}
@@ -80,3 +82,37 @@
                 (is (zero? (t2/count Table :db_id (u/the-id db))))))))
         (finally
           (t2/delete! Database :name test-db-name))))))
+
+(deftest delete-test
+  (testing "We should be able to delete Databases from the config file if we pass the confirmation string"
+    (mt/with-temp [Database _ {:name   test-db-name
+                               :engine "h2"}]
+      (try
+        (binding [advanced-config.file/*config* {:version 1
+                                                 :config  {:settings  {:config-from-file-sync-databases false}
+                                                           :databases [{:name    test-db-name
+                                                                        :engine  "h2"
+                                                                        :details {}
+                                                                        :delete  (format "DELETE_WITH_DEPENDENTS:%s" test-db-name)}]}}]
+          (is (= :ok
+                 (advanced-config.file/initialize!)))
+          (is (not (t2/exists? Database :name test-db-name))))
+        (finally
+          (t2/delete! Database :name test-db-name)))))
+  (testing "We should not delete Databases from the config file if the confirmation string mismatches"
+    (mt/with-temp [Database _ {:name   test-db-name
+                               :engine "h2"}]
+      (try
+        (binding [advanced-config.file/*config* {:version 1
+                                                 :config  {:settings  {:config-from-file-sync-databases false}
+                                                           :databases [{:name    test-db-name
+                                                                        :engine  "h2"
+                                                                        :details {}
+                                                                        :delete  "DELETE_WITH_DEPENDENTS:copy-paste-mistake"}]}}]
+          (is (thrown-with-msg?
+                ExceptionInfo
+                (re-pattern (format "To delete database \"%s\" set `delete` to \"DELETE_WITH_DEPENDENTS:%s\"" test-db-name test-db-name))
+                (advanced-config.file/initialize!)))
+          (is (t2/exists? Database :name test-db-name)))
+        (finally
+          (t2/delete! Database :name test-db-name))))))