diff --git a/enterprise/backend/src/metabase_enterprise/advanced_config/file/databases.clj b/enterprise/backend/src/metabase_enterprise/advanced_config/file/databases.clj index 3bd01e3cc5acce78c8dcdb9c63406e5915889f2e..7d82de30786fb2e6ad5db20d55bf5e7fb230a42d 100644 --- a/enterprise/backend/src/metabase_enterprise/advanced_config/file/databases.clj +++ b/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] diff --git a/enterprise/backend/test/metabase_enterprise/advanced_config/file/databases_test.clj b/enterprise/backend/test/metabase_enterprise/advanced_config/file/databases_test.clj index aa79920920146b9aef4a4344bd60648d8ff291fa..cf1bc718a83116e12e3ca13ef5fcf5b459080f8e 100644 --- a/enterprise/backend/test/metabase_enterprise/advanced_config/file/databases_test.clj +++ b/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))))))