From e32de618dba04101ca62d9e2832a6472a8ba05e4 Mon Sep 17 00:00:00 2001
From: Noah Moss <32746338+noahmoss@users.noreply.github.com>
Date: Tue, 11 Jan 2022 12:58:54 -0500
Subject: [PATCH] Move `is-hosted?` check out of memoized function for cloud
gateway IP setting (#19576)
---
.../test/metabase/driver/oracle_test.clj | 115 +++++++++---------
src/metabase/public_settings.clj | 19 +--
test/metabase/public_settings_test.clj | 11 +-
3 files changed, 74 insertions(+), 71 deletions(-)
diff --git a/modules/drivers/oracle/test/metabase/driver/oracle_test.clj b/modules/drivers/oracle/test/metabase/driver/oracle_test.clj
index ca0b68ed38d..5519c798802 100644
--- a/modules/drivers/oracle/test/metabase/driver/oracle_test.clj
+++ b/modules/drivers/oracle/test/metabase/driver/oracle_test.clj
@@ -14,6 +14,7 @@
[metabase.models.database :refer [Database]]
[metabase.models.field :refer [Field]]
[metabase.models.table :refer [Table]]
+ [metabase.public-settings.premium-features :as premium-features]
[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
@@ -88,63 +89,63 @@
(deftest connection-properties-test
(testing "Connection properties should be returned properly (including transformation of secret types)"
- (let [expected [{:name "host"}
- {:name "port"}
- {:name "sid"}
- {:name "service-name"}
- {:name "user"}
- {:name "password"}
- {:name "cloud-ip-address-info"}
- {:name "ssl"}
- {:name "ssl-use-keystore"}
- {:name "ssl-keystore-options"
- :type "select"
- :options [{:name "Local file path"
- :value "local"}
- {:name "Uploaded file path"
- :value "uploaded"}]
- :visible-if {:ssl-use-keystore true}}
- {:name "ssl-keystore-value"
- :type "textFile"
- :visible-if {:ssl-keystore-options "uploaded"}}
- {:name "ssl-keystore-path"
- :type "string"
- :visible-if {:ssl-keystore-options "local"}}
- {:name "ssl-keystore-password-value"
- :type "password"}
- {:name "ssl-use-truststore"}
- {:name "ssl-truststore-options"
- :type "select"
- :options [{:name "Local file path"
- :value "local"}
- {:name "Uploaded file path"
- :value "uploaded"}]
- :visible-if {:ssl-use-truststore true}}
- {:name "ssl-truststore-value"
- :type "textFile"
- :visible-if {:ssl-truststore-options "uploaded"}}
- {:name "ssl-truststore-path"
- :type "string"
- :visible-if {:ssl-truststore-options "local"}}
- {:name "ssl-truststore-password-value"
- :type "password"}
- {:name "tunnel-enabled"}
- {:name "tunnel-host"}
- {:name "tunnel-port"}
- {:name "tunnel-user"}
- {:name "tunnel-auth-option"}
- {:name "tunnel-pass"}
- {:name "tunnel-private-key"}
- {:name "tunnel-private-key-passphrase"}
- {:name "advanced-options"}
- {:name "auto_run_queries"}
- {:name "let-user-control-scheduling"}
- {:name "schedules.metadata_sync"}
- {:name "schedules.cache_field_values"}
- {:name "refingerprint"}]
- actual (->> (driver/connection-properties :oracle)
- (driver.u/connection-props-server->client :oracle))]
- (is (= expected (mt/select-keys-sequentially expected actual))))))
+ (with-redefs [premium-features/is-hosted? (constantly false)]
+ (let [expected [{:name "host"}
+ {:name "port"}
+ {:name "sid"}
+ {:name "service-name"}
+ {:name "user"}
+ {:name "password"}
+ {:name "ssl"}
+ {:name "ssl-use-keystore"}
+ {:name "ssl-keystore-options"
+ :type "select"
+ :options [{:name "Local file path"
+ :value "local"}
+ {:name "Uploaded file path"
+ :value "uploaded"}]
+ :visible-if {:ssl-use-keystore true}}
+ {:name "ssl-keystore-value"
+ :type "textFile"
+ :visible-if {:ssl-keystore-options "uploaded"}}
+ {:name "ssl-keystore-path"
+ :type "string"
+ :visible-if {:ssl-keystore-options "local"}}
+ {:name "ssl-keystore-password-value"
+ :type "password"}
+ {:name "ssl-use-truststore"}
+ {:name "ssl-truststore-options"
+ :type "select"
+ :options [{:name "Local file path"
+ :value "local"}
+ {:name "Uploaded file path"
+ :value "uploaded"}]
+ :visible-if {:ssl-use-truststore true}}
+ {:name "ssl-truststore-value"
+ :type "textFile"
+ :visible-if {:ssl-truststore-options "uploaded"}}
+ {:name "ssl-truststore-path"
+ :type "string"
+ :visible-if {:ssl-truststore-options "local"}}
+ {:name "ssl-truststore-password-value"
+ :type "password"}
+ {:name "tunnel-enabled"}
+ {:name "tunnel-host"}
+ {:name "tunnel-port"}
+ {:name "tunnel-user"}
+ {:name "tunnel-auth-option"}
+ {:name "tunnel-pass"}
+ {:name "tunnel-private-key"}
+ {:name "tunnel-private-key-passphrase"}
+ {:name "advanced-options"}
+ {:name "auto_run_queries"}
+ {:name "let-user-control-scheduling"}
+ {:name "schedules.metadata_sync"}
+ {:name "schedules.cache_field_values"}
+ {:name "refingerprint"}]
+ actual (->> (driver/connection-properties :oracle)
+ (driver.u/connection-props-server->client :oracle))]
+ (is (= expected (mt/select-keys-sequentially expected actual)))))))
(deftest test-ssh-connection
(testing "Gets an error when it can't connect to oracle via ssh tunnel"
diff --git a/src/metabase/public_settings.clj b/src/metabase/public_settings.clj
index cb2d1e2ac00..ee4fed4c8db 100644
--- a/src/metabase/public_settings.clj
+++ b/src/metabase/public_settings.clj
@@ -454,14 +454,13 @@
(def ^:private fetch-cloud-gateway-ips-fn
(memoize/ttl
(fn []
- (when (premium-features/is-hosted?)
- (try
- (-> (http/get (cloud-gateway-ips-url))
- :body
- (json/parse-string keyword)
- :ip_addresses)
- (catch Exception e
- (log/error e (trs "Error fetching Metabase Cloud gateway IP addresses:"))))))
+ (try
+ (-> (http/get (cloud-gateway-ips-url))
+ :body
+ (json/parse-string keyword)
+ :ip_addresses)
+ (catch Exception e
+ (log/error e (trs "Error fetching Metabase Cloud gateway IP addresses:")))))
:ttl/threshold (* 1000 60 60 24)))
(defsetting cloud-gateway-ips
@@ -469,7 +468,9 @@
:visibility :public
:type :json
:setter :none
- :getter fetch-cloud-gateway-ips-fn)
+ :getter (fn []
+ (when (premium-features/is-hosted?)
+ (fetch-cloud-gateway-ips-fn))))
(defsetting show-database-syncing-modal
(str (deferred-tru "Whether an introductory modal should be shown after the next database connection is added.")
diff --git a/test/metabase/public_settings_test.clj b/test/metabase/public_settings_test.clj
index 064c5d43492..50360b05f51 100644
--- a/test/metabase/public_settings_test.clj
+++ b/test/metabase/public_settings_test.clj
@@ -206,8 +206,9 @@
(is (= nil (public-settings/cloud-gateway-ips))))))
(testing "Setting returns nil in self-hosted environments"
- (memoize/memo-clear! @#'public-settings/fetch-cloud-gateway-ips-fn)
- (http-fake/with-fake-routes-in-isolation
- {{:address (public-settings/cloud-gateway-ips-url)}
- (constantly {:status 200 :body "{\"ip_addresses\": [\"127.0.0.1\"]}"})}
- (is (= nil (public-settings/cloud-gateway-ips))))))
+ (with-redefs [premium-features/is-hosted? (constantly false)]
+ (memoize/memo-clear! @#'public-settings/fetch-cloud-gateway-ips-fn)
+ (http-fake/with-fake-routes-in-isolation
+ {{:address (public-settings/cloud-gateway-ips-url)}
+ (constantly {:status 200 :body "{\"ip_addresses\": [\"127.0.0.1\"]}"})}
+ (is (= nil (public-settings/cloud-gateway-ips)))))))
--
GitLab