Connects to Mongo (#10130)

* [mongo][ci mongo] use connection string to connect to mongo

* [ci mongo][style]

* [style][ci mongo]

* [style][ci mongo]

* [test] app and test runners wip

* [test] test runners wip

* [mongo][ci mongo] pass in conn opts from ui

* [mongo][ci mongo] test

* [test]

* [style] rm test file

* [mongo][ci mongo] use authdb if provided

* [ci mongo] add domain to hostname for dns-srv to work with localhost testing

* [ci mongo]

* [ci mongo] line len

* [ci mongo] actually use fqdn

* [ci mongo] lint

* [ci mongo] swap protocol depending on if host is fqdn

* [ci mongo] lint

* [ci mongo] fallback to non-srv

* [ci mongo] util fns are private

* [ci mongo] lint

* [ci mongo] inline

* [ci mongo] docstrings

* [ci mongo][test] basic tests for mongo conn options, wip

* [ci mongo][test] tests for mongo conn options, wip

* [ci mongo][i18n] exception string

* [ci mongo][style] refactor

* [ci mongo][style] refactor

* [ci mongo][style]

* [ci mongo][docs]

* [ci mongo] fqdn? true if >= 2 '.'s in hostname

* [ci mongo][srv?] srv toggle in mongo connection UI

* [ci mongo] doc

* [ci mongo] doc

* [ci mongo][ui] srv toggle works

.gitignore

@@ -60,3 +60,5 @@ bin/node_modules/
 .vscode
 target/checksum.txt
 /resources/frontend_client/app/locales
+*.iml
+.idea/

dev/user.clj

+(ns user)

frontend/src/metabase/components/DatabaseDetailsForm.jsx

@@ -250,6 +250,31 @@ export default class DatabaseDetailsForm extends Component {
           </div>
         </FormField>
       );
+    } else if (field.name === "use-srv") {
+      const on =
+        this.state.details["use-srv"] == undefined
+          ? false
+          : this.state.details["use-srv"];
+      return (
+        <FormField key={field.name} fieldName={field.name}>
+          <div className="flex align-center Form-offset">
+            <div className="Grid-cell--top">
+              <Toggle
+                value={on}
+                onChange={val => this.onChange("use-srv", val)}
+              />
+            </div>
+            <div className="px2">
+              <h3>{t`Use DNS SRV when connecting`}</h3>
+              <div style={{ maxWidth: "40rem" }} className="pt1">
+                {t`Using this option requires that provided host is a FQDN.  If connecting to 
+                an Atlas cluster, you might need to enable this option.  If you don't know what this means,
+                leave this disabled.`}
+              </div>
+            </div>
+          </div>
+        </FormField>
+      );
     } else if (field.name === "let-user-control-scheduling") {
       const on =
         this.state.details["let-user-control-scheduling"] == undefined

modules/drivers/mongo/resources/metabase-plugin.yaml

@@ -24,7 +24,10 @@ driver:
     - merge:
         - additional-options
         - display-name: Additional Mongo connection string options
-          placeholder: 'readPreference=nearest&replicaSet=test'
+          placeholder: 'retryWrites=true&w=majority&authSource=admin&readPreference=nearest&replicaSet=test'
+    - name: use-srv
+      type: boolean
+      default: false
   connection-properties-include-tunnel-config: true
 init:
   - step: load-namespace

modules/drivers/mongo/src/metabase/driver/mongo/util.clj

@@ -5,12 +5,14 @@
              [config :as config]
              [util :as u]]
             [metabase.models.database :refer [Database]]
-            [metabase.util.ssh :as ssh]
+            [metabase.util
+             [i18n :refer [trs tru]]
+             [ssh :as ssh]]
             [monger
              [core :as mg]
              [credentials :as mcred]]
             [toucan.db :as db])
-  (:import [com.mongodb MongoClientOptions MongoClientOptions$Builder MongoClientURI]))
+  (:import [com.mongodb MongoClient MongoClientOptions MongoClientOptions$Builder MongoClientURI]))
 
 (def ^:const ^:private connection-timeout-ms
   "Number of milliseconds to wait when attempting to establish a Mongo connection. By default, Monger uses a 10-second
@@ -53,7 +55,7 @@
     (MongoClientOptions$Builder. client-options)
     (MongoClientOptions$Builder.)))
 
-(defn- build-connection-options
+(defn- connection-options-builder
   "Build connection options for Mongo.
   We have to use `MongoClientOptions.Builder` directly to configure our Mongo connection since Monger's wrapper method
   doesn't support `.serverSelectionTimeout` or `.sslEnabled`. `additional-options`, a String like
@@ -66,8 +68,7 @@
       (.description config/mb-app-id-string)
       (.connectTimeout connection-timeout-ms)
       (.serverSelectionTimeout connection-timeout-ms)
-      (.sslEnabled ssl?)
-      .build))
+      (.sslEnabled ssl?)))
 
 ;; The arglists metadata for mg/connect are actually *WRONG* -- the function additionally supports a 3-arg airity
 ;; where you can pass options and credentials, as we'd like to do. We need to go in and alter the metadata of this
@@ -89,35 +90,123 @@
     :else                         (throw (Exception. (str "with-mongo-connection failed: bad connection details:"
                                                           (:details database))))))
 
+(defn- srv-conn-str
+  "Creates Mongo client connection string to connect using
+   DNS + SRV discovery mechanism."
+  [user pass host authdb]
+  (format "mongodb+srv://%s:%s@%s/%s" user pass host authdb))
+
+(defn- normalize-details [details]
+  (let [{:keys [dbname host port user pass ssl authdb tunnel-host tunnel-user tunnel-pass additional-options use-srv]
+         :or   {port 27017, pass "", ssl false, use-srv false}} details
+        ;; ignore empty :user and :pass strings
+        user             (when (seq user)
+                           user)
+        pass             (when (seq pass)
+                           pass)
+        authdb           (if (seq authdb)
+                           authdb
+                           dbname)]
+    {:host               host
+     :port               port
+     :user               user
+     :authdb             authdb
+     :pass               pass
+     :dbname             dbname
+     :ssl                ssl
+     :additional-options additional-options
+     :srv?               use-srv}))
+
+(defn- fqdn?
+  "A very simple way to check if a hostname is fully-qualified:
+   Check if there are two or more periods in the name."
+  [host]
+  (<= 2 (-> host frequencies (get \. 0))))
+
+(defn- srv-connection-info
+  "Connection info for Mongo using DNS SRV.  Requires FQDN for `host` in the format
+   'subdomain. ... .domain.top-level-domain'.  Only a single host is supported, but a
+   replica list could easily provided instead of a single host.
+   Using SRV automatically enables SSL, though we explicitly set SSL to true anyway.
+   Docs to generate URI string: https://docs.mongodb.com/manual/reference/connection-string/#dns-seedlist-connection-format"
+  [{:keys [host port user authdb pass dbname ssl additional-options]}]
+  (if-not (fqdn? host)
+    (throw (ex-info (str (tru "Using DNS SRV requires a FQDN for host" ))
+                    {:host host}))
+    (let [conn-opts (connection-options-builder :ssl? ssl, :additional-options additional-options)
+          authdb (if (seq authdb)
+                   authdb
+                   dbname)
+          conn-str (srv-conn-str user pass host authdb)]
+      {:type :srv
+       :uri  (MongoClientURI. conn-str conn-opts)})))
+
+(defn- normal-connection-info
+  "Connection info for Mongo.  Returns options for the fallback method to connect
+   to hostnames that are not FQDNs.  This works with 'localhost', but has been problematic with FQDNs.
+   If you would like to provide a FQDN, use `srv-connection-info`"
+  [{:keys [host port user authdb pass dbname ssl additional-options]}]
+  (let [server-address                   (mg/server-address host port)
+        credentials                      (when user
+                                           (mcred/create user authdb pass))
+        ^MongoClientOptions$Builder opts (connection-options-builder :ssl? ssl, :additional-options additional-options)]
+    {:type           :normal
+     :server-address server-address
+     :credentials    credentials
+     :dbname         dbname
+     :options        (-> opts .build)}))
+
+(defn- details->mongo-connection-info [{:keys [srv?], :as details}]
+  ((if srv?
+     srv-connection-info
+     normal-connection-info) details))
+
+(defmulti ^:private connect
+  "Connect to MongoDB using Mongo `connection-info`, return a tuple of `[mongo-client db]`, instances of `MongoClient`
+   and `DB` respectively.
+
+   If `host` is a fully-qualified domain name, then we need to connect to Mongo
+   differently.  It has been problematic to connect to Mongo with an FQDN using
+   `mg/connect`.  The fix was to create a connection string and use DNS SRV for
+   FQDNS.  In this fn we provide the correct connection fn based on host."
+  {:arglists '([connection-info])}
+  :type)
+
+(defmethod connect :srv
+  [{:keys [^MongoClientURI uri ]}]
+  (let [mongo-client (MongoClient. uri)]
+    (if-let [db-name (.getDatabase uri)]
+      [mongo-client (.getDB mongo-client db-name)]
+      (throw (ex-info (str (tru "No database name specified in URI. Monger requires a database to be explicitly configured." ))
+                      {:hosts (-> uri .getHosts)
+                       :uri   (-> uri .getURI)
+                       :opts  (-> uri .getOptions)})))))
+
+(defmethod connect :normal
+  [{:keys [server-address options credentials dbname]}]
+  (let [do-connect (partial mg/connect server-address options)
+        mongo-client (if credentials
+                       (do-connect credentials)
+                       (do-connect))]
+    [mongo-client (mg/get-db mongo-client dbname)]))
+
+
 (defn -with-mongo-connection
-  "Run F with a new connection (bound to `*mongo-connection*`) to DATABASE.
-   Don't use this directly; use `with-mongo-connection`."
+  "Run `f` with a new connection (bound to `*mongo-connection*`) to `database`. Don't use this directly; use
+  `with-mongo-connection`."
   [f database]
   (let [details (database->details database)]
     (ssh/with-ssh-tunnel [details-with-tunnel details]
-      (let [{:keys [dbname host port user pass ssl authdb tunnel-host tunnel-user tunnel-pass additional-options]
-             :or   {port 27017, pass "", ssl false}} details-with-tunnel
-            user             (when (seq user) ; ignore empty :user and :pass strings
-                               user)
-            pass             (when (seq pass)
-                               pass)
-            authdb           (if (seq authdb)
-                               authdb
-                               dbname)
-            server-address   (mg/server-address host port)
-            credentials      (when user
-                               (mcred/create user authdb pass))
-            connect          (partial mg/connect server-address (build-connection-options :ssl? ssl, :additional-options additional-options))
-            conn             (if credentials
-                               (connect credentials)
-                               (connect))
-            mongo-connection (mg/get-db conn dbname)]
-        (log/debug (u/format-color 'cyan "<< OPENED NEW MONGODB CONNECTION >>"))
-        (try
-          (binding [*mongo-connection* mongo-connection]
-            (f *mongo-connection*))
-          (finally        (mg/disconnect conn)
-                          (log/debug (u/format-color 'cyan "<< CLOSED MONGODB CONNECTION >>"))))))))
+      (let [connection-info (details->mongo-connection-info (normalize-details details-with-tunnel))
+           [mongo-client db] (connect connection-info)]
+       (log/debug (u/format-color 'cyan (trs "Opened new MongoDB connection.")))
+       (try
+         (binding [*mongo-connection* db]
+           (f *mongo-connection*))
+         (finally
+           (mg/disconnect mongo-client)
+           (log/debug (u/format-color 'cyan (trs "Closed MongoDB connection.")))))))))
+
 
 (defmacro with-mongo-connection
   "Open a new MongoDB connection to ``database-or-connection-string`, bind connection to `binding`, execute `body`, and

modules/drivers/mongo/test/metabase/driver/mongo/util_test.clj

@@ -3,35 +3,222 @@
             [metabase.driver.mongo.util :as mongo-util]
             [metabase.driver.util :as driver.u]
             [metabase.test.util.log :as tu.log])
-  (:import com.mongodb.ReadPreference))
+  (:import com.mongodb.ReadPreference
+           (com.mongodb MongoClient DB ServerAddress MongoClientException)))
+
+
+(defn- connect-mongo [opts]
+  (let [connection-info (#'mongo-util/details->mongo-connection-info
+                          (#'mongo-util/normalize-details
+                            opts))]
+    (#'mongo-util/connect connection-info)))
+
+(def connect-passthrough
+  (fn [{map-type :type}]
+    map-type))
+
+(def srv-passthrough
+  (fn [_] {:type :srv}))
+
+
+;; test hostname is fqdn
+
+(expect
+  true
+  (#'mongo-util/fqdn? "db.mongo.com"))
+
+(expect
+  true
+  (#'mongo-util/fqdn? "replica-01.db.mongo.com"))
+
+(expect
+  false
+  (#'mongo-util/fqdn? "localhost"))
+
+(expect
+  false
+  (#'mongo-util/fqdn? "localhost.localdomain"))
+
+
+;; test srv connection string
+
+(expect
+  "mongodb+srv://test-user:test-pass@test-host.place.com/authdb"
+  (#'mongo-util/srv-conn-str "test-user" "test-pass" "test-host.place.com" "authdb"))
+
+
+;; test that srv toggle works
+
+(expect
+  :srv
+  (with-redefs [mongo-util/srv-connection-info srv-passthrough
+                mongo-util/connect connect-passthrough]
+    (let [host "my.fake.domain"
+          opts {:host               host
+                :port               1015
+                :user               "test-user"
+                :authdb             "test-authdb"
+                :pass               "test-passwd"
+                :dbname             "test-dbname"
+                :ssl                true
+                :additional-options ""
+                :use-srv            true}]
+      (connect-mongo opts))))
+
+(expect
+  :normal
+  (with-redefs [mongo-util/connect connect-passthrough]
+    (let [host "localhost"
+          opts {:host               host
+                :port               1010
+                :user               "test-user"
+                :authdb             "test-authdb"
+                :pass               "test-passwd"
+                :dbname             "test-dbname"
+                :ssl                true
+                :additional-options ""
+                :use-srv            false}]
+      (connect-mongo opts))))
+
+(expect
+  :normal
+  (with-redefs [mongo-util/connect connect-passthrough]
+    (let [host "localhost.domain"
+          opts {:host               host
+                :port               1010
+                :user               "test-user"
+                :authdb             "test-authdb"
+                :pass               "test-passwd"
+                :dbname             "test-dbname"
+                :ssl                true
+                :additional-options ""}]
+      (connect-mongo opts))))
+
+;; test that connection properties when using srv
+
+(expect
+  "No SRV record available for host fake.fqdn.com"
+  (try
+    (let [host "fake.fqdn.com"
+          opts {:host               host
+                :port               1015
+                :user               "test-user"
+                :authdb             "test-authdb"
+                :pass               "test-passwd"
+                :dbname             "test-dbname"
+                :ssl                true
+                :additional-options ""
+                :use-srv            true}
+          [^MongoClient mongo-client ^DB db] (connect-mongo opts)
+          ^ServerAddress mongo-addr (-> mongo-client
+                                        (.getAllAddress)
+                                        first)
+          mongo-host (-> mongo-addr .getHost)
+          mongo-port (-> mongo-addr .getPort)]
+      [mongo-host mongo-port])
+    (catch MongoClientException e
+      (.getMessage e))))
+
+(expect
+  "Using DNS SRV requires a FQDN for host"
+  (try
+    (let [host "host1"
+          opts {:host               host
+                :port               1015
+                :user               "test-user"
+                :authdb             "test-authdb"
+                :pass               "test-passwd"
+                :dbname             "test-dbname"
+                :ssl                true
+                :additional-options ""
+                :use-srv            true}
+          [^MongoClient mongo-client ^DB db] (connect-mongo opts)
+          ^ServerAddress mongo-addr (-> mongo-client
+                                        (.getAllAddress)
+                                        first)
+          mongo-host (-> mongo-addr .getHost)
+          mongo-port (-> mongo-addr .getPort)]
+      [mongo-host mongo-port])
+    (catch Exception e
+      (.getMessage e))))
+
+(expect
+  "Unable to look up SRV record for host fake.fqdn.org"
+  (try
+    (let [host "fake.fqdn.org"
+          opts {:host               host
+                :port               1015
+                :user               "test-user"
+                :authdb             "test-authdb"
+                :pass               "test-passwd"
+                :dbname             "test-dbname"
+                :ssl                true
+                :additional-options ""
+                :use-srv            true}
+          [^MongoClient mongo-client ^DB db] (connect-mongo opts)
+          ^ServerAddress mongo-addr (-> mongo-client
+                                        (.getAllAddress)
+                                        first)
+          mongo-host (-> mongo-addr .getHost)
+          mongo-port (-> mongo-addr .getPort)]
+      [mongo-host mongo-port])
+    (catch MongoClientException e
+      (.getMessage e))))
+
+;; test host and port are correct for both srv and normal
+
+(expect
+  ["localhost" 1010]
+  (let [host "localhost"
+        opts {:host               host
+              :port               1010
+              :user               "test-user"
+              :authdb             "test-authdb"
+              :pass               "test-passwd"
+              :dbname             "test-dbname"
+              :ssl                true
+              :additional-options ""}
+        [^MongoClient mongo-client ^DB db] (connect-mongo opts)
+        ^ServerAddress mongo-addr (-> mongo-client
+                                      (.getAllAddress)
+                                      first)
+        mongo-host (-> mongo-addr .getHost)
+        mongo-port (-> mongo-addr .getPort)]
+    [mongo-host mongo-port]))
+
 
 ;; test that people can specify additional connection options like `?readPreference=nearest`
 (expect
   (ReadPreference/nearest)
-  (.getReadPreference (#'mongo-util/build-connection-options :additional-options "readPreference=nearest")))
+  (.getReadPreference (-> (#'mongo-util/connection-options-builder :additional-options "readPreference=nearest")
+                          .build)))
 
 (expect
   (ReadPreference/secondaryPreferred)
-  (.getReadPreference (#'mongo-util/build-connection-options :additional-options "readPreference=secondaryPreferred")))
+  (.getReadPreference (-> (#'mongo-util/connection-options-builder :additional-options "readPreference=secondaryPreferred")
+                          .build)))
 
 ;; make sure we can specify multiple options
 (expect
   "test"
-  (.getRequiredReplicaSetName (#'mongo-util/build-connection-options :additional-options "readPreference=secondary&replicaSet=test")))
+  (.getRequiredReplicaSetName (-> (#'mongo-util/connection-options-builder :additional-options "readPreference=secondary&replicaSet=test")
+                                  .build)))
 
 (expect
   (ReadPreference/secondary)
-  (.getReadPreference (#'mongo-util/build-connection-options :additional-options "readPreference=secondary&replicaSet=test")))
+  (.getReadPreference (-> (#'mongo-util/connection-options-builder :additional-options "readPreference=secondary&replicaSet=test")
+                          .build)))
 
 ;; make sure that invalid additional options throw an Exception
 (expect
   IllegalArgumentException
-  (#'mongo-util/build-connection-options :additional-options "readPreference=ternary"))
+  (-> (#'mongo-util/connection-options-builder :additional-options "readPreference=ternary")
+      .build))
 
 (expect
   #"We couldn't connect to the ssh tunnel host"
   (try
-    (let [engine  :mongo
+    (let [engine :mongo
           details {:ssl            false
                    :password       "changeme"
                    :tunnel-host    "localhost"
@@ -44,5 +231,5 @@
                    :tunnel-user    "bogus"}]
       (tu.log/suppress-output
         (driver.u/can-connect-with-details? engine details :throw-exceptions)))
-       (catch Exception e
-         (.getMessage e))))
+    (catch Exception e
+      (.getMessage e))))