Add linter for metabase-plugin.yaml files (#19240)

Add new deps for finding close misspellings of map keys (spell-spec) and printing more readable spec errors (expound)

Adding spec definitions for driver YAML format

Add call to driver YAML validation from verify.clj

Fixing a couple issues in existing drivers found by the validation

bin/build-drivers/deps.edn

@@ -5,11 +5,13 @@
   com.github.seancorfield/depstar {:mvn/version "2.1.278"}
   cheshire/cheshire               {:mvn/version "5.8.1"}
   commons-codec/commons-codec     {:mvn/version "1.14"}
+  expound/expound                 {:mvn/version "0.7.0"} ; better output of spec validation errors
   hiccup/hiccup                   {:mvn/version "1.0.5"}
   io.forward/yaml                 {:mvn/version "1.0.9"} ; Don't upgrade yet, new version doesn't support Java 8 (see https://github.com/owainlewis/yaml/issues/37)
   io.github.clojure/tools.build   {:git/tag "v0.1.6", :git/sha "5636e61"}
   org.clojure/tools.deps.alpha    {:mvn/version "0.12.985"}
   org.flatland/ordered            {:mvn/version "1.5.9"} ; used by io.forward/yaml -- need the newer version
+  com.bhauman/spell-spec          {:mvn/version "0.1.1"} ; used to find misspellings in YAML files
   stencil/stencil                 {:mvn/version "0.5.0"}
   ;; local source
   metabase/metabase-core          {:local/root "../.."}

bin/build-drivers/src/build_drivers/lint_manifest_file.clj

+(ns build-drivers.lint-manifest-file
+  (:require [clojure.spec.alpha :as s]
+            [spell-spec.alpha :as spell]))
+
+(s/def ::init-step (spell/keys :req-un [::step]))
+
+(defmulti init-step-type :step)
+
+(s/def ::namespace string?)
+(defmethod init-step-type "load-namespace" [_]
+  (spell/keys :req-un [::namespace]))
+
+(s/def ::class string?)
+(defmethod init-step-type "register-jdbc-driver" [_]
+  (spell/keys :req-un [::class]))
+
+(s/def ::name string?)
+(s/def ::version string?)
+(s/def ::description string?)
+(s/def ::info (spell/keys :req-un [::name ::version ::description]))
+
+(def ^:private property-types #{"string" "textFile" "boolean" "secret" "info"})
+
+(s/def ::display-name string?)
+(s/def ::default any?)
+(s/def ::lazy-load boolean?)
+(s/def ::abstract boolean?)
+(s/def ::parent (s/or :single-parent string? :multiple-parent (s/coll-of string?)))
+
+(s/def ::required boolean?)
+(s/def ::placeholder string?)
+(s/def ::type #(contains? property-types %))
+(s/def ::visible-if (s/map-of keyword? any?))
+
+(s/def ::connection-property-map (spell/keys :opt-un [::display-name ::default ::required ::placeholder ::type
+                                                      ::visible-if]))
+
+(s/def ::raw-property-name-ref string?)
+(s/def ::merge (s/cat :property-name ::raw-property-name-ref
+                      :merge-map ::connection-property-map))
+(s/def ::merge-map (spell/keys :req-un [::merge]))
+
+(s/def ::connection-property (s/or :merge-with ::merge-map
+                                   :property-name ::raw-property-name-ref
+                                   :property-map (s/merge (spell/keys :req-un [::name]) ::connection-property-map)))
+
+(s/def ::connection-properties (s/coll-of ::connection-property))
+
+(s/def ::connection-properties-include-tunnel-config boolean?)
+
+(s/def ::single-driver (s/keys :req-un [::name ::lazy-load]
+                                   :opt-un [::parent ::display-name ::abstract ::connection-properties
+                                            ::connection-properties-include-tunnel-config]))
+
+(s/def ::driver (s/or :single-driver ::single-driver :multiple-drivers (s/coll-of ::single-driver)))
+
+(s/def ::init (s/coll-of (s/multi-spec init-step-type #(get % :step))))
+
+(s/def ::plugin-manifest
+  (spell/keys :req-un [::info ::driver] :opt-un [::init]))

bin/build-drivers/src/build_drivers/verify.clj

 (ns build-drivers.verify
   (:require [build-drivers.common :as c]
+            [build-drivers.lint-manifest-file :as lint-manifest-file]
+            [clojure.spec.alpha :as s]
             [colorize.core :as colorize]
-            [metabuild-common.core :as u])
+            [expound.alpha :as expound]
+            [clojure.java.io :as io]
+            [metabuild-common.core :as u]
+            [spell-spec.expound]
+            [yaml.core :as yaml])
   (:import [java.util.zip ZipEntry ZipFile]))
 
-(defn- jar-contains-file? [^String jar-path ^String filename]
+(defn- get-jar-entry [^String jar-path ^String filename]
   (with-open [zip-file (ZipFile. jar-path)]
-    (some
-     (fn [^ZipEntry zip-entry]
-       (= (str zip-entry) filename))
-     (enumeration-seq (.entries zip-file)))))
+    (first
+      (filter
+        (fn [^ZipEntry zip-entry]
+          (= (str zip-entry) filename))
+        (enumeration-seq (.entries zip-file))))))
+
+(defn- jar-contains-file? [^String jar-path ^String filename]
+  (some? (get-jar-entry jar-path filename)))
 
 (defn- verify-has-init-class [driver]
   (let [jar-filename               (c/driver-jar-destination-path driver)
@@ -32,8 +42,19 @@
 (defn- verify-has-plugin-manifest [driver]
   (let [jar-filename (c/driver-jar-destination-path driver)]
     (u/step (format "Check %s contains metabase-plugin.yaml" jar-filename)
-      (if (jar-contains-file? jar-filename "metabase-plugin.yaml")
-        (u/announce "Plugin manifest found.")
+      (if-let [manifest-entry (get-jar-entry jar-filename "metabase-plugin.yaml")]
+        (with-open [zip-file (ZipFile. jar-filename)]
+          (let [entry-is (.getInputStream zip-file manifest-entry)
+                yaml-str (slurp entry-is)
+                yml      (yaml/parse-string yaml-str)]
+            (u/announce "Plugin manifest found; validating it")
+            (if-not (s/valid? ::lint-manifest-file/plugin-manifest yml)
+              (do
+                ;; print a readable explanation of the spec error
+                (expound/expound ::lint-manifest-file/plugin-manifest yml)
+                (throw (ex-info "Driver verification failed: plugin manifest was invalid; see full explanation above"
+                                {:invalid-driver driver})))
+              (u/announce "Plugin manifest passed spec validation"))))
         (throw (ex-info "Driver verification failed: plugin manifest missing" {}))))))
 
 (defn verify-driver

modules/drivers/mongo/resources/metabase-plugin.yaml

@@ -35,8 +35,7 @@ driver:
     - name: use-srv
       type: boolean
       default: false
-    - merge:
-        - ssl
+    - ssl
     - name: ssl-cert
       type: string
       display-name: Server SSL certificate chain

modules/drivers/oracle/resources-ee/metabase-plugin.yaml

@@ -14,7 +14,7 @@ driver:
         - default: 1521
     - name: sid
       display-name: Oracle system ID (SID)
-      placehoder: Usually something like ORCL or XE. Optional if using service name
+      placeholder: Usually something like ORCL or XE. Optional if using service name
     - name: service-name
       display-name: Oracle service name
       placeholder: Optional TNS alias

modules/drivers/oracle/resources/metabase-plugin.yaml

@@ -20,7 +20,7 @@ driver:
         - default: 1521
     - name: sid
       display-name: Oracle system ID (SID)
-      placehoder: Usually something like ORCL or XE. Optional if using service name
+      placeholder: Usually something like ORCL or XE. Optional if using service name
     - name: service-name
       display-name: Oracle service name
       placeholder: Optional TNS alias