Skip to content

Commit

Permalink
Fix Oracle SSL tests
Browse files Browse the repository at this point in the history
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 orbv 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
  • Loading branch information
jeff303 committed Mar 22, 2021
1 parent 7f83eb8 commit 43cd184
Show file tree
Hide file tree
Showing 8 changed files with 46 additions and 35 deletions.
25 changes: 7 additions & 18 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -637,9 +637,6 @@ jobs:
description:
type: string
default: ""
test-selector:
type: string
default: ":all"
extra-env:
type: string
default: ""
Expand All @@ -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
Expand Down Expand Up @@ -1078,20 +1075,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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
12 changes: 12 additions & 0 deletions modules/drivers/oracle/test/metabase/driver/oracle_test.clj
Original file line number Diff line number Diff line change
@@ -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]
Expand All @@ -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] ; used for one SSL connectivity test
[metabase.test :as mt]
[metabase.test.data.oracle :as oracle.tx]
[metabase.test.data.sql :as sql.tx]
Expand Down Expand Up @@ -231,3 +233,13 @@
&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"
(tu/with-env-keys-renamed-by #(str/replace-first % "mb-oracle-ssl-test" "mb-oracle-test")
(metabase.query-processor-test.order-by-test/order-by-aggregate-fields-test)))
(println (u/colorize 'yellow (format "Skipping %s because %s env var is not set\n"
"oracle-connect-with-ssl-test"
"MB_ORACLE_SSL_TEST_SSL"))))))
3 changes: 2 additions & 1 deletion modules/drivers/oracle/test/metabase/test/data/oracle.clj
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
10 changes: 6 additions & 4 deletions resources/certificates/README.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
Binary file not shown.
13 changes: 3 additions & 10 deletions test/metabase/driver/mysql_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -19,14 +17,14 @@
[metabase.sync.analyze.fingerprint :as fingerprint]
[metabase.test :as mt]
[metabase.test.data.interface :as tx]
[metabase.test.util :as tu]
[metabase.util :as u]
[metabase.util.date-2 :as u.date]
[metabase.util.honeysql-extensions :as hx]
[toucan.db :as db]
[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 "
Expand Down Expand Up @@ -339,13 +337,8 @@
(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))))
(tu/with-env-keys-renamed-by #(str/replace-first % "mb-mysql-ssl-test" "mb-mysql-test")
(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"))))))
14 changes: 14 additions & 0 deletions test/metabase/test/util.clj
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
[clojure.walk :as walk]
[clojurewerkz.quartzite.scheduler :as qs]
[colorize.core :as colorize]
[environ.core :as env]
[java-time :as t]
[medley.core :as m]
[metabase.driver :as driver]
[metabase.models :refer [Card Collection Dashboard DashboardCardSeries Database Dimension Field FieldValues
Metric NativeQuerySnippet Permissions PermissionsGroup Pulse PulseCard PulseChannel
Expand Down Expand Up @@ -794,3 +796,15 @@
[]
(with-open [socket (ServerSocket. 0)]
(.getLocalPort socket)))

(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]
`(let [e# env/env
key-fn# (fn [k#]
(keyword (~rename-fn (name k#))))
new-e# (m/map-keys key-fn# e#)]
(with-redefs [env/env new-e#]
~@body)))

0 comments on commit 43cd184

Please sign in to comment.