set settings from env vars

project.clj

@@ -65,6 +65,7 @@
                              [jonase/eastwood "0.2.1"]                ; Linting
                              [lein-ancient "0.6.5"]                   ; Check project for outdated dependencies + plugins w/ 'lein ancient'
                              [lein-bikeshed "0.2.0"]                  ; Linting
+                             [lein-environ "1.0.0"]                   ; Specify env-vars in project.clj
                              [lein-expectations "0.0.8"]              ; run unit tests with 'lein expectations'
                              [lein-instant-cheatsheet "2.1.1"]        ; use awesome instant cheatsheet created by yours truly w/ 'lein instant-cheatsheet'
                              [lein-marginalia "0.8.0"]                ; generate documentation with 'lein marg'
@@ -76,6 +77,7 @@
                               "-XX:+UseConcMarkSweepGC"]}             ; Concurrent Mark Sweep GC needs to be used for Class Unloading (above)
              :expectations {:injections [(require 'metabase.test-setup)]
                             :resource-paths ["test_resources"]
+                            :env {:mb-test-setting-1 "ABCDEFG"}
                             :jvm-opts ["-Dmb.db.file=target/metabase-test"
                                        "-Dmb.jetty.join=false"
                                        "-Dmb.jetty.port=3001"

src/metabase/models/setting.clj

 (ns metabase.models.setting
   (:refer-clojure :exclude [get set])
   (:require [clojure.core.match :refer [match]]
+            [environ.core :as env]
             [korma.core :refer :all :exclude [delete]]
             [metabase.db :refer [sel del]]))
 
 ;; Settings are a fast + simple way to create a setting that can be set
-;; from the admin page. They are saved to the Database, but intelligently
+;; from the SuperAdmin page. They are saved to the Database, but intelligently
 ;; cached internally for super-fast lookups.
 ;;
-;; Define a new Setting with `defsetting`:
+;; Define a new Setting with `defsetting` (optionally supplying a default value):
 ;;
 ;;    (defsetting mandrill-api-key "API key for Mandrill")
 ;;
-;; The setting and docstr will then be auto-magically accessible from the admin page.
+;; The setting and docstr will then be auto-magically accessible from the SuperAdmin page.
+;;
+;; You can also set the value via the corresponding env var, which looks like
+;; `MB_MANDRILL_API_KEY`, where the name of the setting is converted to uppercase and dashes to underscores.
 ;;
 ;; The var created with `defsetting` can be used as a getter/setter, or you can
 ;; use `get`/`set`/`delete`:
 ;;
 ;;     (require '[metabase.models.setting :as setting])
 ;;
-;;     (setting/get :mandrill-api-key)
-;;     (mandrill-api-key)
+;;     (setting/get :mandrill-api-key)           ; only returns values set explicitly from SuperAdmin
+;;     (mandrill-api-key)                        ; returns value set in SuperAdmin, OR value of corresponding env var, OR the default value, if any (in that order)
 ;;
 ;;     (setting/set :mandrill-api-key "NEW_KEY")
 ;;     (mandrill-api-key "NEW_KEY")
@@ -33,7 +37,7 @@
 ;;    (setting/all)
 
 (declare Setting
-         cached-setting-values
+         cached-setting->value
          restore-cache-if-needed
          settings-list)
 
@@ -42,18 +46,20 @@
 ;; ## ACCESSORS
 
 (defn get
-  "Fetch value of `Setting`, or return default value if none is set.
-   Cached lookup time is ~60µs, compared to ~1800µs for DB lookup."
+  "Fetch value of `Setting`, first trying our cache, or fetching the value
+   from the DB if that fails. (Cached lookup time is ~60µs, compared to ~1800µs for DB lookup)
+
+   Unlike using the setting getter fn, this will *not* return default values or values specified by env vars."
   [k]
   {:pre [(keyword? k)]}
   (restore-cache-if-needed)
-  (or (@cached-setting-values k)
+  (or (@cached-setting->value k)
       (when-let [v (sel :one :field [Setting :value] :key (name k))]
-        (swap! cached-setting-values assoc k v)
+        (swap! cached-setting->value assoc k v)
         v)))
 
 (defn set
-  "Set the value of `Setting` for `Org`.
+  "Set the value of a `Setting`.
 
     (set :mandrill-api-key \"xyz123\")"
   [k v]
@@ -66,27 +72,36 @@
               (values {:key (name k)
                        :value v})))
   (restore-cache-if-needed)
-  (swap! cached-setting-values assoc k v)
+  (swap! cached-setting->value assoc k v)
   v)
 
 (defn delete
-  "Delete a `Setting` value for `Org`."
+  "Delete a `Setting`."
   [k]
   {:pre [(keyword? k)]}
   (restore-cache-if-needed)
-  (swap! cached-setting-values dissoc k)
+  (swap! cached-setting->value dissoc k)
   (del Setting :key (name k)))
 
 ;; ## DEFSETTING
 
+(defn get-from-env-var
+  "Given a `Setting` like `:mandrill-api-key`, return the value of the corresponding env var,
+   e.g. `MB_MANDRILL_API_KEY`."
+  [setting-key]
+  (env/env (keyword (str "mb-" (name setting-key)))))
+
 (defmacro defsetting
   "Defines a new `Setting` that will be added to the DB at some point in the future.
    Conveniently can be used as a getter/setter as well:
 
      (defsetting mandrill-api-key \"API key for Mandrill.\")
-     (mandrill-api-key )          ; get the value
+     (mandrill-api-key)           ; get the value
      (mandrill-api-key new-value) ; update the value
-     (mandrill-api-key nil)       ; delete the value"
+     (mandrill-api-key nil)       ; delete the value
+
+   A setting can be set from the SuperAdmin page or via the corresponding env var,
+   eg. `MB_MANDRILL_API_KEY` for the example above."
   {:arglists '([setting-name description]
                [setting-name description default-value])}
   [nm description & [default-value]]
@@ -98,6 +113,7 @@
         ::default-value ~default-value}
        ([]
         (or (get ~setting-key)
+            (get-from-env-var ~setting-key)
             ~default-value))
        ([value#]
         (if-not value#
@@ -113,7 +129,7 @@
     (all) -> {:mandrill-api-key ...}"
   []
   (restore-cache-if-needed)
-  @cached-setting-values)
+  @cached-setting->value)
 
 (defn all-with-descriptions
   "Return a sequence of Settings maps, including value and description."
@@ -129,13 +145,14 @@
 ;; # IMPLEMENTATION
 
 (defn- restore-cache-if-needed []
-  (when-not @cached-setting-values
-    (reset! cached-setting-values (->> (sel :many Setting)
+  (when-not @cached-setting->value
+    (reset! cached-setting->value (->> (sel :many Setting)
                                        (map (fn [{k :key v :value}]
                                               {(keyword k) v}))
                                        (into {})))))
 
-(def ^:private cached-setting-values
+(def ^:private cached-setting->value
+  "Map of setting name (keyword) -> string value, as they exist in the DB."
   (atom nil))
 
 (defentity Setting
@@ -152,4 +169,5 @@
        (map (fn [{k :name desc :doc default ::default-value}]
               {:key (keyword k)
                :description desc
-               :default default}))))
+               :default (or (get-from-env-var k)
+                            default)}))))

test/metabase/api/setting_test.clj

@@ -20,7 +20,7 @@
 ;; ## GET /api/setting
 ;; Check that we can fetch all Settings for Org
 (expect-eval-actual-first
-    [{:key "test-setting-1", :value nil,     :description "Test setting - this only shows up in dev (1)", :default nil}
+    [{:key "test-setting-1", :value nil,     :description "Test setting - this only shows up in dev (1)", :default "ABCDEFG"}
      {:key "test-setting-2", :value "FANCY", :description "Test setting - this only shows up in dev (2)", :default "[Default Value]"}]
     (do (set-settings nil "FANCY")
         (fetch-all-settings)))
@@ -52,7 +52,7 @@
 
 ;; ## DELETE /api/setting/:key
 (expect-eval-actual-first
-    [nil
+    ["ABCDEFG"
      nil
      false]
   (do ((user->client :crowberto) :delete 204 "setting/test-setting-1")

test/metabase/models/setting_test.clj

@@ -32,12 +32,12 @@
 
 
 ;; ## GETTERS
-;; Test defsetting getter fn
-(expect nil
+;; Test defsetting getter fn. Should return the value from env var MB_TEST_SETTING_1
+(expect "ABCDEFG"
   (do (set-settings nil nil)
-    (test-setting-1)))
+      (test-setting-1)))
 
-;; Test `get` function
+;; Test `get` function. Shouldn't return env var value
 (expect nil
   (do (set-settings nil nil)
       (setting/get :test-setting-1)))
@@ -47,6 +47,11 @@
   (do (set-settings nil nil)
       (test-setting-2)))
 
+;; `get` shouldn't return default value
+(expect nil
+  (do (set-settings nil nil)
+      (setting/get :test-setting-2)))
+
 
 ;; ## SETTERS
 ;; Test defsetting setter fn
@@ -71,6 +76,7 @@
 (expect-eval-actual-first
     ["COOL"
      true
+     "ABCDEFG"
      nil
      false]
   [(do (test-setting-1 "COOL")
@@ -78,6 +84,7 @@
    (setting-exists? :test-setting-1)
    (do (test-setting-1 nil)
        (test-setting-1))
+   (setting/get :test-setting-1)
    (setting-exists? :test-setting-1)])
 
 ;; Test defsetting delete w/ default value
@@ -97,6 +104,7 @@
 (expect-eval-actual-first
     ["VERY NICE!"
      true
+     "ABCDEFG"
      nil
      false]
   [(do (test-setting-1 "VERY NICE!")
@@ -104,6 +112,7 @@
    (setting-exists? :test-setting-1)
    (do (test-setting-1 nil)
        (test-setting-1))
+   (setting/get :test-setting-1)
    (setting-exists? :test-setting-1)])
 
 ;; ## ALL SETTINGS FUNCTIONS
@@ -117,7 +126,7 @@
 
 ;; all-with-descriptions
 (expect-eval-actual-first
-    [{:key :test-setting-1, :value nil,  :description "Test setting - this only shows up in dev (1)", :default nil}
+    [{:key :test-setting-1, :value nil,  :description "Test setting - this only shows up in dev (1)", :default "ABCDEFG"} ; should return env-var value as default
      {:key :test-setting-2, :value "S2", :description "Test setting - this only shows up in dev (2)", :default "[Default Value]"}]
   (do (set-settings nil "S2")
       (filter (fn [{k :key}]