feat(metabase): use instance label for urn component (#351)

* feat(metabase): use instance label for urn component * feat(metabase): make username optional if session_id is passed
raystack · May 31, 2022 · 76ee919 · 76ee919
1 parent 062956c
commit 76ee919
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 30 deletions.
diff --git a/plugins/extractors/metabase/README.md b/plugins/extractors/metabase/README.md
@@ -7,6 +7,7 @@ source:
   name: metabase
   config:
     host: http://localhost:3000
+    instance_label: my-metabase
     username: meteor_tester
     password: meteor_pass_1234
 ```
@@ -16,14 +17,16 @@ source:
 | Key | Value | Example | Description |    |
 | :-- | :---- | :------ | :---------- | :- |
 | `host` | `string` | `http://localhost:4002` | The host at which metabase is running | *required* |
-| `username` | `string` | `meteor_tester` | Username/email to access the metabase| *required* |
-| `password` | `string` | `meteor_pass_1234` | Password for the metabase | *required* |
+| `instance_label` | `string` | `my-metabase` | Instance alias, the value will be used as part of the urn component | *required* |
+| `username` | `string` | `meteor_tester` | Username/email to access the metabase| *optional* |
+| `password` | `string` | `meteor_pass_1234` | Password for the metabase | *optional* |
+| `session_id` | `string` | `meteor_pass_1234` | Use existing session ID from metabase to create requests. (this will ignore username and password) | *optional* |
 
 ## Outputs
 
 | Field | Sample Value |
 | :---- | :---- |
-| `resource.urn` | `metabase.dashboard_name` |
+| `resource.urn` | `metabase::my-metabase/dashboard/5123` |
 | `resource.name` | `dashboard_name` |
 | `resource.service` | `metabase` |
 | `description` | `table description` |
@@ -33,9 +36,9 @@ source:
 
 | Field | Sample Value |
 | :---- | :---- |
-| `urn` | `metabase.dashboard_name.card_name` |
+| `urn` | `metabase::my-metabase/card/9123` |
 | `source` | `metabase` |
-| `dashboard_urn` | `metabase.dashboard_name` |
+| `dashboard_urn` | `metabase::my-metabase/dashboard/5123` |
 | `dashboard_source` | `metabase` |
 
 ## Contributing

diff --git a/plugins/extractors/metabase/metabase.go b/plugins/extractors/metabase/metabase.go
@@ -25,15 +25,17 @@ var summary string
 
 var sampleConfig = `
 host: http://localhost:3000
+instance_label: my-metabase
 user_id: meteor_tester
 password: meteor_pass_1234`
 
 // Config holds the set of configuration for the metabase extractor
 type Config struct {
-	Host      string `mapstructure:"host" validate:"required"`
-	Username  string `mapstructure:"username" validate:"required"`
-	Password  string `mapstructure:"password" validate:"required"`
-	SessionID string `mapstructure:"session_id"`
+	Host          string `mapstructure:"host" validate:"required"`
+	InstanceLabel string `mapstructure:"instance_label" validate:"required"`
+	Username      string `mapstructure:"username" validate:"required_without=SessionID"`
+	Password      string `mapstructure:"password"`
+	SessionID     string `mapstructure:"session_id"`
 }
 
 // Extractor manages the extraction of data
@@ -108,7 +110,7 @@ func (e *Extractor) buildDashboard(d Dashboard) (data *assetsv1beta1.Dashboard,
 		return
 	}
 
-	dashboardUrn := models.DashboardURN("metabase", e.config.Host, fmt.Sprintf("dashboard/%d", dashboard.ID))
+	dashboardUrn := models.DashboardURN("metabase", e.config.InstanceLabel, fmt.Sprintf("dashboard/%d", dashboard.ID))
 	charts := e.buildCharts(dashboardUrn, dashboard)
 	dashboardUpstreams := e.buildDashboardUpstreams(charts)
 
@@ -160,7 +162,7 @@ func (e *Extractor) buildChart(card Card, dashboardUrn string) (chart *assetsv1b
 	}
 
 	return &assetsv1beta1.Chart{
-		Urn:          fmt.Sprintf("metabase::%s/card/%d", e.config.Host, card.ID),
+		Urn:          fmt.Sprintf("metabase::%s/card/%d", e.config.InstanceLabel, card.ID),
 		DashboardUrn: dashboardUrn,
 		Source:       "metabase",
 		Name:         card.Name,

diff --git a/plugins/extractors/metabase/metabase_test.go b/plugins/extractors/metabase/metabase_test.go
@@ -28,23 +28,36 @@ func TestInit(t *testing.T) {
 	t.Run("should return error for invalid config", func(t *testing.T) {
 		client := new(mockClient)
 		config := map[string]interface{}{
-			"username": "user",
-			"host":     "",
+			"host":           "sample-host",
+			"instance_label": "my-metabase",
 		}
 		err := metabase.New(client, testutils.Logger).Init(context.TODO(), config)
 
 		assert.Equal(t, plugins.InvalidConfigError{}, err)
 	})
 	t.Run("should authenticate with client if config is valid", func(t *testing.T) {
 		config := map[string]interface{}{
-			"username":   "user",
-			"host":       "sample-host",
-			"password":   "sample-password",
-			"session_id": "sample-session",
+			"host":           "sample-host",
+			"instance_label": "my-metabase",
+			"username":       "user",
+			"password":       "sample-password",
 		}
 
 		client := new(mockClient)
-		client.On("Authenticate", "sample-host", "user", "sample-password", "sample-session").Return(nil)
+		client.On("Authenticate", "sample-host", "user", "sample-password", "").Return(nil)
+
+		err := metabase.New(client, testutils.Logger).Init(context.TODO(), config)
+		assert.NoError(t, err)
+	})
+	t.Run("should allow session_id to replace username and password", func(t *testing.T) {
+		config := map[string]interface{}{
+			"host":           "sample-host",
+			"instance_label": "my-metabase",
+			"session_id":     "sample-session",
+		}
+
+		client := new(mockClient)
+		client.On("Authenticate", "sample-host", "", "", "sample-session").Return(nil)
 
 		err := metabase.New(client, testutils.Logger).Init(context.TODO(), config)
 		assert.NoError(t, err)
@@ -69,9 +82,10 @@ func TestExtract(t *testing.T) {
 		emitter := mocks.NewEmitter()
 		extr := metabase.New(client, plugins.GetLog())
 		err := extr.Init(context.TODO(), map[string]interface{}{
-			"host":     host,
-			"username": "test-user",
-			"password": "test-pass",
+			"host":           host,
+			"username":       "test-user",
+			"password":       "test-pass",
+			"instance_label": "my-metabase",
 		})
 		if err != nil {
 			t.Fatal(err)

diff --git a/plugins/extractors/metabase/testdata/expected.json b/plugins/extractors/metabase/testdata/expected.json
@@ -1,6 +1,6 @@
 [{
     "resource": {
-        "urn": "metabase::https://my-metabase.com/dashboard/1",
+        "urn": "metabase::my-metabase/dashboard/1",
         "name": "Main",
         "service": "metabase",
         "type": "dashboard",
@@ -39,8 +39,8 @@
     },
     "charts": [
         {
-            "urn": "metabase::https://my-metabase.com/card/1",
-            "dashboard_urn": "metabase::https://my-metabase.com/dashboard/1",
+            "urn": "metabase::my-metabase/card/1",
+            "dashboard_urn": "metabase::my-metabase/dashboard/1",
             "source": "metabase",
             "name": "Orders, Filtered by Quantity",
             "description": "HELPFUL CHART DESC",
@@ -67,8 +67,8 @@
             }
         },
         {
-            "urn": "metabase::https://my-metabase.com/card/2",
-            "dashboard_urn": "metabase::https://my-metabase.com/dashboard/1",
+            "urn": "metabase::my-metabase/card/2",
+            "dashboard_urn": "metabase::my-metabase/dashboard/1",
             "source": "metabase",
             "name": "Exceptional Users",
             "description": "This shows only exceptional users.",
@@ -95,8 +95,8 @@
             }
         },
         {
-            "urn": "metabase::https://my-metabase.com/card/3",
-            "dashboard_urn": "metabase::https://my-metabase.com/dashboard/1",
+            "urn": "metabase::my-metabase/card/3",
+            "dashboard_urn": "metabase::my-metabase/dashboard/1",
             "source": "metabase",
             "name": "Users, Average of Total Followers and Cumulative sum of Total Likes, Filtered by Total Followers",
             "description": "Users, Average of Total Followers",
@@ -123,7 +123,7 @@
             }
         },
         {
-            "dashboard_urn": "metabase::https://my-metabase.com/dashboard/1",
+            "dashboard_urn": "metabase::my-metabase/dashboard/1",
             "lineage": {
                 "upstreams": [
                     {
@@ -152,7 +152,7 @@
                 }
             },
             "source": "metabase",
-            "urn": "metabase::https://my-metabase.com/card/4"
+            "urn": "metabase::my-metabase/card/4"
         }
     ],
     "timestamps": {