From 1647552dfd49b5e4b7f3053b7003f82742dccea8 Mon Sep 17 00:00:00 2001
From: john-metabase <92878045+john-metabase@users.noreply.github.com>
Date: Thu, 15 Jun 2023 16:35:24 -0400
Subject: [PATCH] Fix Snowflake sync with uploaded private key (#31593)
---
.../snowflake/resources/metabase-plugin.yaml | 2 +-
.../src/metabase/driver/snowflake.clj | 49 ++++++++++---------
src/metabase/driver/util.clj | 2 +-
3 files changed, 27 insertions(+), 26 deletions(-)
diff --git a/modules/drivers/snowflake/resources/metabase-plugin.yaml b/modules/drivers/snowflake/resources/metabase-plugin.yaml
index 9a82cd386bf..2c0f6bd750a 100644
--- a/modules/drivers/snowflake/resources/metabase-plugin.yaml
+++ b/modules/drivers/snowflake/resources/metabase-plugin.yaml
@@ -10,7 +10,7 @@ driver:
connection-properties:
- name: account
display-name: Account name
- helper-text: Enter your Account ID with the region that your Snowflake cluster is running on e.g. "xxxxxxxx.us-east-2.aws". Some regions don't have this suffix.
+ helper-text: Enter your Account ID with the region that your Snowflake cluster is running on e.g. "xxxxxxxx.us-east-2.aws". Some regions don't have this suffix.
placeholder: xxxxxxxx.us-east-2.aws
required: true
- user
diff --git a/modules/drivers/snowflake/src/metabase/driver/snowflake.clj b/modules/drivers/snowflake/src/metabase/driver/snowflake.clj
index 2a347c6bcd5..a166d59405d 100644
--- a/modules/drivers/snowflake/src/metabase/driver/snowflake.clj
+++ b/modules/drivers/snowflake/src/metabase/driver/snowflake.clj
@@ -20,7 +20,6 @@
[metabase.driver.sql.util :as sql.u]
[metabase.driver.sql.util.unprepare :as unprepare]
[metabase.driver.sync :as driver.s]
- [metabase.driver.util :as driver.u]
[metabase.models.secret :as secret]
[metabase.query-processor.error-type :as qp.error-type]
[metabase.query-processor.store :as qp.store]
@@ -34,7 +33,6 @@
[ring.util.codec :as codec])
(:import
(java.io File)
- (java.nio.charset StandardCharsets)
(java.sql Connection DatabaseMetaData ResultSet Types)
(java.time OffsetDateTime ZonedDateTime)))
@@ -76,34 +74,37 @@
{:user (codec/url-encode user)
:private_key_file (codec/url-encode (.getCanonicalPath ^File private-key-file))})
new-conn-uri (sql-jdbc.common/conn-str-with-additional-opts existing-conn-uri :url opts-str)]
- (assoc details :connection-uri new-conn-uri)))
+ (-> details
+ (assoc :connection-uri new-conn-uri)
+ ;; The Snowflake driver uses the :account property, but we need to drop the region from it first
+ (assoc :account (first (str/split account #"\."))))))
(defn- resolve-private-key
"Convert the private-key secret properties into a private_key_file property in `details`.
Setting the Snowflake driver property privatekey would be easier, but that doesn't work
because clojure.java.jdbc (properly) converts the property values into strings while the
Snowflake driver expects a java.security.PrivateKey instance."
- [{:keys [user password account private-key-value private-key-path] :as details}]
- (cond
- password
- details
-
- private-key-path
- (let [secret-map (secret/db-details-prop->secret-map details "private-key")
- private-key-file (when (some? (:value secret-map))
- (secret/value->file! secret-map :snowflake))]
- (cond-> (apply dissoc details (vals (secret/get-sub-props "private-key")))
- private-key-file (handle-conn-uri user account private-key-file)))
-
- private-key-value
- (let [private-key-str (if (bytes? private-key-value)
- (String. ^bytes private-key-value StandardCharsets/UTF_8)
- private-key-value)
- private-key-file-val (cond-> private-key-str
- (re-find driver.u/data-url-pattern private-key-str) driver.u/decode-uploaded)
- private-key-file (secret/value->file! {:connection-property-name "private-key-file"
- :value private-key-file-val})]
- (handle-conn-uri details user account private-key-file))))
+ [{:keys [user password account private-key-path]
+ :as details}]
+ (let [base-details (apply dissoc details (vals (secret/get-sub-props "private-key")))]
+ (cond
+ password
+ details
+
+ private-key-path
+ (let [secret-map (secret/db-details-prop->secret-map details "private-key")
+ private-key-file (when (some? (:value secret-map))
+ (secret/value->file! secret-map :snowflake))]
+ (cond-> base-details
+ private-key-file (handle-conn-uri user account private-key-file)))
+
+ :else
+ ;; get-secret-string should get the right value (using private-key-value or private-key-id) and decode it automatically
+ (let [decoded (secret/get-secret-string details "private-key")
+ file (secret/value->file! {:connection-property-name "private-key-file"
+ :value decoded})]
+ (assoc (handle-conn-uri base-details user account file)
+ :private_key_file file)))))
(defmethod sql-jdbc.conn/connection-details->spec :snowflake
[_ {:keys [account additional-options], :as details}]
diff --git a/src/metabase/driver/util.clj b/src/metabase/driver/util.clj
index 6a5a3d75bc5..1b427f46a62 100644
--- a/src/metabase/driver/util.clj
+++ b/src/metabase/driver/util.clj
@@ -427,7 +427,7 @@
;; version of the -value property (the :type "textFile" one)
(let [textfile-prop (val-kw secrets-server->client)]
(:treat-before-posting textfile-prop)))))
- value (let [^String v (val-kw acc)]
+ value (when-let [^String v (val-kw acc)]
(case (get-treat)
"base64" (decode-uploaded v)
v))]
--
GitLab