Make `TableName` field part of quality monitor schema (#1903)

## Changes

This field was special-cased in #1307 because it's not part of the JSON
payload in the SDK struct.

This approach, while pragmatic, meant it didn't show up in the JSON
schema. While debugging an issue with quality monitors in #1900, I
couldn't figure out why I was getting schema errors on this field, or
how it was passed through to the TF representation. This commit removes
the special case and makes it behave like everything else.

## Tests

* Unit tests pass.
* Confirmed that the updated schema failed validation before this
change.

bundle/config/mutator/configure_wsfs.go

@@ -5,14 +5,12 @@ import (
 	"strings"
 
 	"github.com/databricks/cli/bundle"
+	"github.com/databricks/cli/libs/dbr"
 	"github.com/databricks/cli/libs/diag"
-	"github.com/databricks/cli/libs/env"
 	"github.com/databricks/cli/libs/filer"
 	"github.com/databricks/cli/libs/vfs"
 )
 
-const envDatabricksRuntimeVersion = "DATABRICKS_RUNTIME_VERSION"
-
 type configureWSFS struct{}
 
 func ConfigureWSFS() bundle.Mutator {
@@ -32,7 +30,7 @@ func (m *configureWSFS) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagno
 	}
 
 	// The executable must be running on DBR.
-	if _, ok := env.Lookup(ctx, envDatabricksRuntimeVersion); !ok {
+	if !dbr.RunsOnRuntime(ctx) {
 		return nil
 	}
 

bundle/config/mutator/configure_wsfs_test.go

@@ -0,0 +1,65 @@
+package mutator_test
+
+import (
+	"context"
+	"runtime"
+	"testing"
+
+	"github.com/databricks/cli/bundle"
+	"github.com/databricks/cli/bundle/config/mutator"
+	"github.com/databricks/cli/libs/dbr"
+	"github.com/databricks/cli/libs/vfs"
+	"github.com/databricks/databricks-sdk-go/config"
+	"github.com/databricks/databricks-sdk-go/experimental/mocks"
+	"github.com/stretchr/testify/assert"
+)
+
+func mockBundleForConfigureWSFS(t *testing.T, syncRootPath string) *bundle.Bundle {
+	// The native path of the sync root on Windows will never match the /Workspace prefix,
+	// so the test case for nominal behavior will always fail.
+	if runtime.GOOS == "windows" {
+		t.Skip("this test is not applicable on Windows")
+	}
+
+	b := &bundle.Bundle{
+		SyncRoot: vfs.MustNew(syncRootPath),
+	}
+
+	w := mocks.NewMockWorkspaceClient(t)
+	w.WorkspaceClient.Config = &config.Config{}
+	b.SetWorkpaceClient(w.WorkspaceClient)
+
+	return b
+}
+
+func TestConfigureWSFS_SkipsIfNotWorkspacePrefix(t *testing.T) {
+	b := mockBundleForConfigureWSFS(t, "/foo")
+	originalSyncRoot := b.SyncRoot
+
+	ctx := context.Background()
+	diags := bundle.Apply(ctx, b, mutator.ConfigureWSFS())
+	assert.Empty(t, diags)
+	assert.Equal(t, originalSyncRoot, b.SyncRoot)
+}
+
+func TestConfigureWSFS_SkipsIfNotRunningOnRuntime(t *testing.T) {
+	b := mockBundleForConfigureWSFS(t, "/Workspace/foo")
+	originalSyncRoot := b.SyncRoot
+
+	ctx := context.Background()
+	ctx = dbr.MockRuntime(ctx, false)
+	diags := bundle.Apply(ctx, b, mutator.ConfigureWSFS())
+	assert.Empty(t, diags)
+	assert.Equal(t, originalSyncRoot, b.SyncRoot)
+}
+
+func TestConfigureWSFS_SwapSyncRoot(t *testing.T) {
+	b := mockBundleForConfigureWSFS(t, "/Workspace/foo")
+	originalSyncRoot := b.SyncRoot
+
+	ctx := context.Background()
+	ctx = dbr.MockRuntime(ctx, true)
+	diags := bundle.Apply(ctx, b, mutator.ConfigureWSFS())
+	assert.Empty(t, diags)
+	assert.NotEqual(t, originalSyncRoot, b.SyncRoot)
+}

bundle/config/mutator/initialize_urls_test.go

@@ -65,9 +65,8 @@ func TestInitializeURLs(t *testing.T) {
 				},
 				QualityMonitors: map[string]*resources.QualityMonitor{
 					"qualityMonitor1": {
-						CreateMonitor: &catalog.CreateMonitor{
 						TableName:     "catalog.schema.qualityMonitor1",
-						},
+						CreateMonitor: &catalog.CreateMonitor{},
 					},
 				},
 				Schemas: map[string]*resources.Schema{

bundle/config/mutator/process_target_mode_test.go

@@ -102,16 +102,23 @@ func mockBundle(mode config.Mode) *bundle.Bundle {
 					"registeredmodel1": {CreateRegisteredModelRequest: &catalog.CreateRegisteredModelRequest{Name: "registeredmodel1"}},
 				},
 				QualityMonitors: map[string]*resources.QualityMonitor{
-					"qualityMonitor1": {CreateMonitor: &catalog.CreateMonitor{TableName: "qualityMonitor1"}},
-					"qualityMonitor2": {
+					"qualityMonitor1": {
+						TableName: "qualityMonitor1",
 						CreateMonitor: &catalog.CreateMonitor{
+							OutputSchemaName: "catalog.schema",
+						},
+					},
+					"qualityMonitor2": {
 						TableName: "qualityMonitor2",
+						CreateMonitor: &catalog.CreateMonitor{
+							OutputSchemaName: "catalog.schema",
 							Schedule:         &catalog.MonitorCronSchedule{},
 						},
 					},
 					"qualityMonitor3": {
-						CreateMonitor: &catalog.CreateMonitor{
 						TableName: "qualityMonitor3",
+						CreateMonitor: &catalog.CreateMonitor{
+							OutputSchemaName: "catalog.schema",
 							Schedule: &catalog.MonitorCronSchedule{
 								PauseStatus: catalog.MonitorCronSchedulePauseStatusUnpaused,
 							},

bundle/config/resources/quality_monitor.go

@@ -13,17 +13,15 @@ import (
 )
 
 type QualityMonitor struct {
-	// Represents the Input Arguments for Terraform and will get
-	// converted to a HCL representation for CRUD
-	*catalog.CreateMonitor
-
-	// This represents the id which is the full name of the monitor
-	// (catalog_name.schema_name.table_name) that can be used
-	// as a reference in other resources. This value is returned by terraform.
 	ID             string         `json:"id,omitempty" bundle:"readonly"`
-
 	ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"internal"`
 	URL            string         `json:"url,omitempty" bundle:"internal"`
+
+	// The table name is a required field but not included as a JSON field in [catalog.CreateMonitor].
+	TableName string `json:"table_name"`
+
+	// This struct defines the creation payload for a monitor.
+	*catalog.CreateMonitor
 }
 
 func (s *QualityMonitor) UnmarshalJSON(b []byte) error {

bundle/deploy/terraform/tfdyn/convert_quality_monitor_test.go

@@ -15,8 +15,8 @@ import (
 
 func TestConvertQualityMonitor(t *testing.T) {
 	var src = resources.QualityMonitor{
-		CreateMonitor: &catalog.CreateMonitor{
 		TableName: "test_table_name",
+		CreateMonitor: &catalog.CreateMonitor{
 			AssetsDir:        "assets_dir",
 			OutputSchemaName: "output_schema_name",
 			InferenceLog: &catalog.MonitorInferenceLog{

bundle/internal/schema/testdata/pass/quality_monitor.yml

@@ -4,6 +4,7 @@ bundle:
 resources:
   quality_monitors:
     myqualitymonitor:
+      table_name: catalog.schema.quality_monitor
       inference_log:
         granularities:
           - a

bundle/schema/jsonschema.json

@@ -684,6 +684,9 @@
                         "description": "Configuration for monitoring snapshot tables.",
                         "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/catalog.MonitorSnapshot"
                       },
+                      "table_name": {
+                        "$ref": "#/$defs/string"
+                      },
                       "time_series": {
                         "description": "Configuration for monitoring time series tables.",
                         "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/catalog.MonitorTimeSeries"
@@ -695,6 +698,7 @@
                     },
                     "additionalProperties": false,
                     "required": [
+                      "table_name",
                       "assets_dir",
                       "output_schema_name"
                     ]

cmd/root/root.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/databricks/cli/internal/build"
 	"github.com/databricks/cli/libs/cmdio"
+	"github.com/databricks/cli/libs/dbr"
 	"github.com/databricks/cli/libs/log"
 	"github.com/spf13/cobra"
 )
@@ -73,6 +74,9 @@ func New(ctx context.Context) *cobra.Command {
 		// get the context back
 		ctx = cmd.Context()
 
+		// Detect if the CLI is running on DBR and store this on the context.
+		ctx = dbr.DetectRuntime(ctx)
+
 		// Configure our user agent with the command that's about to be executed.
 		ctx = withCommandInUserAgent(ctx, cmd)
 		ctx = withCommandExecIdInUserAgent(ctx)

libs/dbr/context.go

@@ -0,0 +1,49 @@
+package dbr
+
+import "context"
+
+// key is a package-local type to use for context keys.
+//
+// Using an unexported type for context keys prevents key collisions across
+// packages since external packages cannot create values of this type.
+type key int
+
+const (
+	// dbrKey is the context key for the detection result.
+	// The value of 1 is arbitrary and can be any number.
+	// Other keys in the same package must have different values.
+	dbrKey = key(1)
+)
+
+// DetectRuntime detects whether or not the current
+// process is running inside a Databricks Runtime environment.
+// It return a new context with the detection result set.
+func DetectRuntime(ctx context.Context) context.Context {
+	if v := ctx.Value(dbrKey); v != nil {
+		panic("dbr.DetectRuntime called twice on the same context")
+	}
+	return context.WithValue(ctx, dbrKey, detect(ctx))
+}
+
+// MockRuntime is a helper function to mock the detection result.
+// It returns a new context with the detection result set.
+func MockRuntime(ctx context.Context, b bool) context.Context {
+	if v := ctx.Value(dbrKey); v != nil {
+		panic("dbr.MockRuntime called twice on the same context")
+	}
+	return context.WithValue(ctx, dbrKey, b)
+}
+
+// RunsOnRuntime returns the detection result from the context.
+// It expects a context returned by [DetectRuntime] or [MockRuntime].
+//
+// We store this value in a context to avoid having to use either
+// a global variable, passing a boolean around everywhere, or
+// performing the same detection multiple times.
+func RunsOnRuntime(ctx context.Context) bool {
+	v := ctx.Value(dbrKey)
+	if v == nil {
+		panic("dbr.RunsOnRuntime called without calling dbr.DetectRuntime first")
+	}
+	return v.(bool)
+}

libs/dbr/context_test.go

@@ -0,0 +1,59 @@
+package dbr
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestContext_DetectRuntimePanics(t *testing.T) {
+	ctx := context.Background()
+
+	// Run detection.
+	ctx = DetectRuntime(ctx)
+
+	// Expect a panic if the detection is run twice.
+	assert.Panics(t, func() {
+		ctx = DetectRuntime(ctx)
+	})
+}
+
+func TestContext_MockRuntimePanics(t *testing.T) {
+	ctx := context.Background()
+
+	// Run detection.
+	ctx = MockRuntime(ctx, true)
+
+	// Expect a panic if the mock function is run twice.
+	assert.Panics(t, func() {
+		MockRuntime(ctx, true)
+	})
+}
+
+func TestContext_RunsOnRuntimePanics(t *testing.T) {
+	ctx := context.Background()
+
+	// Expect a panic if the detection is not run.
+	assert.Panics(t, func() {
+		RunsOnRuntime(ctx)
+	})
+}
+
+func TestContext_RunsOnRuntime(t *testing.T) {
+	ctx := context.Background()
+
+	// Run detection.
+	ctx = DetectRuntime(ctx)
+
+	// Expect no panic because detection has run.
+	assert.NotPanics(t, func() {
+		RunsOnRuntime(ctx)
+	})
+}
+
+func TestContext_RunsOnRuntimeWithMock(t *testing.T) {
+	ctx := context.Background()
+	assert.True(t, RunsOnRuntime(MockRuntime(ctx, true)))
+	assert.False(t, RunsOnRuntime(MockRuntime(ctx, false)))
+}

libs/dbr/detect.go

@@ -0,0 +1,35 @@
+package dbr
+
+import (
+	"context"
+	"os"
+	"runtime"
+
+	"github.com/databricks/cli/libs/env"
+)
+
+// Dereference [os.Stat] to allow mocking in tests.
+var statFunc = os.Stat
+
+// detect returns true if the current process is running on a Databricks Runtime.
+// Its return value is meant to be cached in the context.
+func detect(ctx context.Context) bool {
+	// Databricks Runtime implies Linux.
+	// Return early on other operating systems.
+	if runtime.GOOS != "linux" {
+		return false
+	}
+
+	// Databricks Runtime always has the DATABRICKS_RUNTIME_VERSION environment variable set.
+	if value, ok := env.Lookup(ctx, "DATABRICKS_RUNTIME_VERSION"); !ok || value == "" {
+		return false
+	}
+
+	// Expect to see a "/databricks" directory.
+	if fi, err := statFunc("/databricks"); err != nil || !fi.IsDir() {
+		return false
+	}
+
+	// All checks passed.
+	return true
+}

libs/dbr/detect_test.go

@@ -0,0 +1,83 @@
+package dbr
+
+import (
+	"context"
+	"io/fs"
+	"runtime"
+	"testing"
+
+	"github.com/databricks/cli/libs/env"
+	"github.com/databricks/cli/libs/fakefs"
+	"github.com/stretchr/testify/assert"
+)
+
+func requireLinux(t *testing.T) {
+	if runtime.GOOS != "linux" {
+		t.Skipf("skipping test on %s", runtime.GOOS)
+	}
+}
+
+func configureStatFunc(t *testing.T, fi fs.FileInfo, err error) {
+	originalFunc := statFunc
+	statFunc = func(name string) (fs.FileInfo, error) {
+		assert.Equal(t, "/databricks", name)
+		return fi, err
+	}
+
+	t.Cleanup(func() {
+		statFunc = originalFunc
+	})
+}
+
+func TestDetect_NotLinux(t *testing.T) {
+	if runtime.GOOS == "linux" {
+		t.Skip("skipping test on Linux OS")
+	}
+
+	ctx := context.Background()
+	assert.False(t, detect(ctx))
+}
+
+func TestDetect_Env(t *testing.T) {
+	requireLinux(t)
+
+	// Configure other checks to pass.
+	configureStatFunc(t, fakefs.FileInfo{FakeDir: true}, nil)
+
+	t.Run("empty", func(t *testing.T) {
+		ctx := env.Set(context.Background(), "DATABRICKS_RUNTIME_VERSION", "")
+		assert.False(t, detect(ctx))
+	})
+
+	t.Run("non-empty cluster", func(t *testing.T) {
+		ctx := env.Set(context.Background(), "DATABRICKS_RUNTIME_VERSION", "15.4")
+		assert.True(t, detect(ctx))
+	})
+
+	t.Run("non-empty serverless", func(t *testing.T) {
+		ctx := env.Set(context.Background(), "DATABRICKS_RUNTIME_VERSION", "client.1.13")
+		assert.True(t, detect(ctx))
+	})
+}
+
+func TestDetect_Stat(t *testing.T) {
+	requireLinux(t)
+
+	// Configure other checks to pass.
+	ctx := env.Set(context.Background(), "DATABRICKS_RUNTIME_VERSION", "non-empty")
+
+	t.Run("error", func(t *testing.T) {
+		configureStatFunc(t, nil, fs.ErrNotExist)
+		assert.False(t, detect(ctx))
+	})
+
+	t.Run("not a directory", func(t *testing.T) {
+		configureStatFunc(t, fakefs.FileInfo{}, nil)
+		assert.False(t, detect(ctx))
+	})
+
+	t.Run("directory", func(t *testing.T) {
+		configureStatFunc(t, fakefs.FileInfo{FakeDir: true}, nil)
+		assert.True(t, detect(ctx))
+	})
+}

libs/dyn/convert/struct_info.go

@@ -6,7 +6,6 @@ import (
 	"sync"
 
 	"github.com/databricks/cli/libs/dyn"
-	"github.com/databricks/cli/libs/textutil"
 )
 
 // structInfo holds the type information we need to efficiently
@@ -85,14 +84,6 @@ func buildStructInfo(typ reflect.Type) structInfo {
 			}
 
 			name, _, _ := strings.Cut(sf.Tag.Get("json"), ",")
-			if typ.Name() == "QualityMonitor" && name == "-" {
-				urlName, _, _ := strings.Cut(sf.Tag.Get("url"), ",")
-				if urlName == "" || urlName == "-" {
-					name = textutil.CamelToSnakeCase(sf.Name)
-				} else {
-					name = urlName
-				}
-			}
 			if name == "" || name == "-" {
 				continue
 			}

libs/fakefs/fakefs.go

@@ -0,0 +1,55 @@
+package fakefs
+
+import (
+	"io/fs"
+	"time"
+)
+
+// DirEntry is a fake implementation of [fs.DirEntry].
+type DirEntry struct {
+	FileInfo
+}
+
+func (entry DirEntry) Type() fs.FileMode {
+	typ := fs.ModePerm
+	if entry.FakeDir {
+		typ |= fs.ModeDir
+	}
+	return typ
+}
+
+func (entry DirEntry) Info() (fs.FileInfo, error) {
+	return entry.FileInfo, nil
+}
+
+// FileInfo is a fake implementation of [fs.FileInfo].
+type FileInfo struct {
+	FakeName string
+	FakeSize int64
+	FakeDir  bool
+	FakeMode fs.FileMode
+}
+
+func (info FileInfo) Name() string {
+	return info.FakeName
+}
+
+func (info FileInfo) Size() int64 {
+	return info.FakeSize
+}
+
+func (info FileInfo) Mode() fs.FileMode {
+	return info.FakeMode
+}
+
+func (info FileInfo) ModTime() time.Time {
+	return time.Now()
+}
+
+func (info FileInfo) IsDir() bool {
+	return info.FakeDir
+}
+
+func (info FileInfo) Sys() any {
+	return nil
+}