Fix Oracle SSL tests (#15260)

Fix Oracle SSL tests Define new test for Oracle SSL connectivity, in oracle_test.clj, similar to how things work in mysql_test.clj Add new test util macro, with-env-keys-renamed-by, to support running tests with environ keys temporarily renamed Using new test macro from both MySQL and Oracle SSL connectivity tests Removing now unneeded be-tests-oracle-ssl-ee CircleCI job Removing now unneeded test-selector parameter for test-driver orb in CircleCI config.yml Updating JVM_OPTS to use a trust store that starts with cacerts and adds the RDS root CA, rather than one only containing the RDS root CA
metabase · Mar 31, 2021 · c938fb0 · c938fb0
1 parent c28d430
commit c938fb0
Show file tree

Hide file tree

Showing 9 changed files with 65 additions and 39 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -637,9 +637,6 @@ jobs:
       description:
         type: string
         default: ""
-      test-selector:
-        type: string
-        default: ":all"
       extra-env:
         type: string
         default: ""
@@ -656,7 +653,7 @@ jobs:
                 name: Test << parameters.driver >> driver << parameters.description >>
                 environment:
                   DRIVERS: << parameters.driver >>
-                command: << parameters.extra-env >> lein with-profile +ci,+junit,+ee test << parameters.test-selector >>
+                command: << parameters.extra-env >> lein with-profile +ci,+junit,+ee test
                 no_output_timeout: << parameters.timeout >>
             - store_test_results:
                 path: /home/circleci/metabase/metabase/target/junit
@@ -1086,20 +1083,12 @@ workflows:
                 source: ORACLE_JDBC_JAR
                 dest: ojdbc8.jar
           driver: oracle
-
-      - test-driver:
-          name: be-tests-oracle-ssl-ee
-          requires:
-            - be-tests-ee
-          before-steps:
-            - fetch-jdbc-driver:
-                source: ORACLE_JDBC_JAR
-                dest: ojdbc8.jar
-          driver: oracle
-          test-selector: :only metabase.driver.oracle-test
-          extra-env: >
-            MB_ORACLE_TEST_SSL=true MB_ORACLE_TEST_PORT=2484
-            JVM_OPTS="-Djavax.net.ssl.trustStore=/home/circleci/metabase/metabase/resources/certificates/rds_root_ca_truststore.jks -Djavax.net.ssl.trustStoreType=JKS -Djavax.net.ssl.trustStorePassword=metabase $JAVA_OPTS"
+          extra-env: >-
+            MB_ORACLE_SSL_TEST_SSL=true
+            MB_ORACLE_SSL_TEST_PORT=2484
+            JVM_OPTS="-Djavax.net.ssl.trustStore=/home/circleci/metabase/metabase/resources/certificates/cacerts_with_RDS_root_ca.jks
+                      -Djavax.net.ssl.trustStoreType=JKS
+                      -Djavax.net.ssl.trustStorePassword=metabase $JAVA_OPTS"
 
       - test-driver:
           name: be-tests-postgres-ee

diff --git a/enterprise/backend/test/metabase_enterprise/serialization/test_util.clj b/enterprise/backend/test/metabase_enterprise/serialization/test_util.clj
@@ -81,8 +81,8 @@
                                  :query
                                  {:source-query
                                   {:source-query
-                                   {:source-table ~'table-id
-                                    }}}}}]
+                                   {:source-table ~'table-id}}}}}]
+
                    Card       [{~'card-id-native-query :id}
                                {:query_type :native
                                 :name "My Native Nested Query Card"

diff --git a/modules/drivers/oracle/test/metabase/driver/oracle_test.clj b/modules/drivers/oracle/test/metabase/driver/oracle_test.clj
@@ -1,6 +1,7 @@
 (ns metabase.driver.oracle-test
   "Tests for specific behavior of the Oracle driver."
   (:require [clojure.java.jdbc :as jdbc]
+            [clojure.string :as str]
             [clojure.test :refer :all]
             [honeysql.core :as hsql]
             [metabase.driver :as driver]
@@ -12,6 +13,7 @@
             [metabase.models.table :refer [Table]]
             [metabase.query-processor :as qp]
             [metabase.query-processor-test :as qp.test]
+            [metabase.query-processor-test.order-by-test :as qp-test.order-by-test] ; used for one SSL connectivity test
             [metabase.test :as mt]
             [metabase.test.data.oracle :as oracle.tx]
             [metabase.test.data.sql :as sql.tx]
@@ -231,3 +233,14 @@
                                                   &test_data_categories__via__cat.categories.id]
                                    :fk-field-id  (mt/id :venues :category_id)
                                    :fields       :none}]}))))))))
+
+(deftest oracle-connect-with-ssl-test
+  (mt/test-driver :oracle
+    (if (System/getenv "MB_ORACLE_SSL_TEST_SSL")
+      (testing "Oracle with SSL connectivity"
+        (mt/with-env-keys-renamed-by #(str/replace-first % "mb-oracle-ssl-test" "mb-oracle-test")
+          (qp-test.order-by-test/order-by-aggregate-fields-test)))
+      (println (u/format-color 'yellow
+                               "Skipping %s because %s env var is not set"
+                               "oracle-connect-with-ssl-test"
+                               "MB_ORACLE_SSL_TEST_SSL")))))
diff --git a/modules/drivers/oracle/test/metabase/test/data/oracle.clj b/modules/drivers/oracle/test/metabase/test/data/oracle.clj
@@ -36,7 +36,8 @@
     :port     (Integer/parseInt (tx/db-test-env-var-or-throw :oracle :port "1521"))
     :user     (tx/db-test-env-var-or-throw :oracle :user)
     :password (tx/db-test-env-var-or-throw :oracle :password)
-    :sid      (tx/db-test-env-var-or-throw :oracle :sid)}))
+    :sid      (tx/db-test-env-var-or-throw :oracle :sid)
+    :ssl      (tx/db-test-env-var :oracle :ssl false)}))
 
 (defmethod tx/dbdef->connection-details :oracle [& _] @connection-details)
 

diff --git a/resources/certificates/README.md b/resources/certificates/README.md
@@ -1,9 +1,11 @@
 # Directory Contents
 
-## `rds_root_ca_truststore.jks`
-This is a JKS trust store, which was created via
-[these instructions](https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/UsingWithRDS.SSL.html).  It contains the
-latest RDS root CA (`rds-ca-2019-root.pem`) as of March 2021. The keystore password is: metabase
+## `cacerts_with_RDS_root_ca.jks`
+This is a JKS trust store, which was created by adding the RDS root CA (latest as of March 2021) to the built-in
+`cacerts` file included with OpenJDK 11. It was added by following similar instructions as those outlined for
+creating a new trust store file documented
+[here](https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/UsingWithRDS.SSL.html). The keystore password is:
+`metabase`
 
 ## `rds-combined-ca-bundle.pem`
 This is simply a copy of the "combined" CA bundle from

diff --git a/resources/certificates/cacerts_with_RDS_root_ca.jks b/resources/certificates/cacerts_with_RDS_root_ca.jks
diff --git a/test/metabase/driver/mysql_test.clj b/test/metabase/driver/mysql_test.clj
@@ -2,10 +2,8 @@
   (:require [clojure.java.jdbc :as jdbc]
             [clojure.string :as str]
             [clojure.test :refer :all]
-            [environ.core :as env]
             [honeysql.core :as hsql]
             [java-time :as t]
-            [medley.core :as m]
             [metabase.db.metadata-queries :as metadata-queries]
             [metabase.driver :as driver]
             [metabase.driver.mysql :as mysql]
@@ -14,7 +12,8 @@
             [metabase.models.field :refer [Field]]
             [metabase.models.table :refer [Table]]
             [metabase.query-processor :as qp]
-            [metabase.query-processor-test.string-extracts-test] ; used for one SSL with PEM connectivity test
+            ;; used for one SSL with PEM connectivity test
+            [metabase.query-processor-test.string-extracts-test :as string-extracts-test]
             [metabase.sync :as sync]
             [metabase.sync.analyze.fingerprint :as fingerprint]
             [metabase.test :as mt]
@@ -26,7 +25,6 @@
             [toucan.hydrate :refer [hydrate]]
             [toucan.util.test :as tt]))
 
-
 (deftest all-zero-dates-test
   (mt/test-driver :mysql
     (testing (str "MySQL allows 0000-00-00 dates, but JDBC does not; make sure that MySQL is converting them to NULL "
@@ -339,13 +337,9 @@
   (mt/test-driver :mysql
     (if (System/getenv "MB_MYSQL_SSL_TEST_SSL_CERT")
       (testing "MySQL with SSL connectivity using PEM certificate"
-        (let [e      env/env
-              key-fn (fn [k]
-                       (keyword (str/replace-first (name k) "mb-mysql-ssl-test" "mb-mysql-test")))
-              new-e  (m/map-keys key-fn e)]
-          (with-redefs [env/env new-e]
-            ;; with the mysql-test vars having been set to the mysql-ssl variants, run some test to verify connectivity
-            (metabase.query-processor-test.string-extracts-test/test-breakout))))
-      (println (u/colorize 'yellow (format "Skipping %s because %s env var is not set\n"
-                                           "mysql-connect-with-ssl-and-pem-cert-test"
-                                           "MB_MYSQL_SSL_TEST_SSL_CERT"))))))
+        (mt/with-env-keys-renamed-by #(str/replace-first % "mb-mysql-ssl-test" "mb-mysql-test")
+          (string-extracts-test/test-breakout)))
+      (println (u/format-color 'yellow
+                               "Skipping %s because %s env var is not set"
+                               "mysql-connect-with-ssl-and-pem-cert-test"
+                               "MB_MYSQL_SSL_TEST_SSL_CERT")))))
diff --git a/test/metabase/test.clj b/test/metabase/test.clj
@@ -177,6 +177,7 @@
   throw-if-called
   with-column-remappings
   with-discarded-collections-perms-changes
+  with-env-keys-renamed-by
   with-locale
   with-log-level
   with-log-messages

diff --git a/test/metabase/test/util.clj b/test/metabase/test/util.clj
@@ -1,11 +1,13 @@
 (ns metabase.test.util
   "Helper functions and macros for writing unit tests."
   (:require [cheshire.core :as json]
+            [clojure.set :as set]
             [clojure.string :as str]
             [clojure.test :refer :all]
             [clojure.walk :as walk]
             [clojurewerkz.quartzite.scheduler :as qs]
             [colorize.core :as colorize]
+            [environ.core :as env]
             [java-time :as t]
             [metabase.driver :as driver]
             [metabase.models :refer [Card Collection Dashboard DashboardCardSeries Database Dimension Field FieldValues
@@ -794,3 +796,27 @@
   []
   (with-open [socket (ServerSocket. 0)]
     (.getLocalPort socket)))
+
+(defn do-with-env-keys-renamed-by
+  "Evaluates the thunk with the current core.environ/env being redefined, its keys having been renamed by the given
+  rename-fn. Prefer to use the with-env-keys-renamed-by macro version instead."
+  [rename-fn thunk]
+  (let [orig-e     env/env
+        renames-fn (fn [m k _]
+                     (let [k-str (name k)
+                           new-k (rename-fn k-str)]
+                       (if (not= k-str new-k)
+                         (assoc m k (keyword new-k))
+                         m)))
+        renames    (reduce-kv renames-fn {} orig-e)
+        new-e      (set/rename-keys orig-e renames)]
+    (testing (colorize/blue (format "\nRenaming env vars by map: %s\n" (u/pprint-to-str renames)))
+      (with-redefs [env/env new-e]
+        (thunk)))))
+
+(defmacro with-env-keys-renamed-by
+  "Evaluates body with the current core.environ/env being redefined, its keys having been renamed by the given
+  rename-fn."
+  {:arglists '([rename-fn & body])}
+  [rename-fn & body]
+  `(do-with-env-keys-renamed-by ~rename-fn (fn [] ~@body)))