Merge be2d802d13 into e4d039a1aa

## Changes

The issue reported in #1828 illustrates how using a YAML timestamp-like
value (a date in this case) causes an issue during conversion to and
from the typed configuration tree.

We use the `AsAny()` function on the `dyn.Value` when normalizing for
the `any` type. We only use the `any` type for variable values, because
they can assume every type. The `AsAny()` function returns a `time.Time`
for the time value during conversion **to** the typed configuration
tree. Upon conversion **from** the typed configuration tree back into
the dynamic configuration tree, we cannot distinguish a `time.Time`
struct from any other struct.

To address this, we use the underlying string value of the time value
when we normalize for the `any` type.

Fixes #1828.

## Tests

Existing unit tests pass

bundle/config/mutator/apply_presets.go

@@ -11,6 +11,7 @@ import (
 	"github.com/databricks/cli/bundle/config"
 	"github.com/databricks/cli/libs/diag"
 	"github.com/databricks/cli/libs/dyn"
+	"github.com/databricks/cli/libs/log"
 	"github.com/databricks/cli/libs/textutil"
 	"github.com/databricks/databricks-sdk-go/service/catalog"
 	"github.com/databricks/databricks-sdk-go/service/jobs"
@@ -188,6 +189,14 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos
 			diags = diags.Extend(diag.Errorf("schema %s is not defined", key))
 			continue
 		}
+
+		// If the catalog is already namespaced to the current user, we don't need
+		// to prefix the schema name since it already falls under the user's namespace.
+		if containsUserIdentity(s.CatalogName, b.Config.Workspace.CurrentUser) {
+			log.Debugf(ctx, "Skipping schema %s since catalog %s already contains the user's identity", s.Name, s.CatalogName)
+			continue
+		}
+
 		s.Name = normalizePrefix(prefix) + s.Name
 		// HTTP API for schemas doesn't yet support tags. It's only supported in
 		// the Databricks UI and via the SQL API.

bundle/config/mutator/apply_presets_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/databricks/cli/bundle/config/mutator"
 	"github.com/databricks/cli/bundle/config/resources"
 	"github.com/databricks/databricks-sdk-go/service/catalog"
+	"github.com/databricks/databricks-sdk-go/service/iam"
 	"github.com/databricks/databricks-sdk-go/service/jobs"
 	"github.com/stretchr/testify/require"
 )
@@ -96,12 +97,53 @@ func TestApplyPresetsPrefixForUcSchema(t *testing.T) {
 			},
 			want: "schema1",
 		},
+		{
+			name:   "skip prefix because catalog contains short name",
+			prefix: "[prefix]",
+			schema: &resources.Schema{
+				CreateSchema: &catalog.CreateSchema{
+					Name:        "schema1",
+					CatalogName: "dev_john_smith_test_catalog",
+				},
+			},
+			want: "schema1",
+		},
+		{
+			name:   "skip prefix because catalog contains email",
+			prefix: "[prefix]",
+			schema: &resources.Schema{
+				CreateSchema: &catalog.CreateSchema{
+					Name:        "schema1",
+					CatalogName: "dev_john.smith@databricks.com_test_catalog",
+				},
+			},
+			want: "schema1",
+		},
+		{
+			name:   "add prefix because catalog is not namespaced to user",
+			prefix: "[prefix]",
+			schema: &resources.Schema{
+				CreateSchema: &catalog.CreateSchema{
+					Name:        "schema1",
+					CatalogName: "test_catalog",
+				},
+			},
+			want: "prefix_schema1",
+		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			b := &bundle.Bundle{
 				Config: config.Root{
+					Workspace: config.Workspace{
+						CurrentUser: &config.User{
+							ShortName: "john_smith",
+							User: &iam.User{
+								UserName: "john.smith@databricks.com",
+							},
+						},
+					},
 					Resources: config.Resources{
 						Schemas: map[string]*resources.Schema{
 							"schema1": tt.schema,

bundle/config/mutator/process_target_mode.go

@@ -63,6 +63,10 @@ func transformDevelopmentMode(ctx context.Context, b *bundle.Bundle) {
 	}
 }
 
+func containsUserIdentity(s string, u *config.User) bool {
+	return strings.Contains(s, u.ShortName) || strings.Contains(s, u.UserName)
+}
+
 func validateDevelopmentMode(b *bundle.Bundle) diag.Diagnostics {
 	var diags diag.Diagnostics
 	p := b.Config.Presets
@@ -92,7 +96,7 @@ func validateDevelopmentMode(b *bundle.Bundle) diag.Diagnostics {
 			diags = diags.Extend(diag.Errorf("%s must start with '~/' or contain the current username to ensure uniqueness when using 'mode: development'", path))
 		}
 	}
-	if p.NamePrefix != "" && !strings.Contains(p.NamePrefix, u.ShortName) && !strings.Contains(p.NamePrefix, u.UserName) {
+	if p.NamePrefix != "" && !containsUserIdentity(p.NamePrefix, u) {
 		// Resources such as pipelines require a unique name, e.g. '[dev steve] my_pipeline'.
 		// For this reason we require the name prefix to contain the current username;
 		// it's a pitfall for users if they don't include it and later find out that

bundle/tests/issue_1828/databricks.yml

@@ -0,0 +1,33 @@
+bundle:
+  name: issue_1828
+
+variables:
+  # One entry for each of the underlying YAML (or [dyn.Kind]) types.
+  # The test confirms we can convert to and from the typed configuration without losing information.
+
+  map:
+    default:
+      foo: bar
+
+  sequence:
+    default:
+      - foo
+      - bar
+
+  string:
+    default: foo
+
+  bool:
+    default: true
+
+  int:
+    default: 42
+
+  float:
+    default: 3.14
+
+  time:
+    default: 2021-01-01
+
+  nil:
+    default:

bundle/tests/issue_1828_test.go

@@ -0,0 +1,48 @@
+package config_tests
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestIssue1828(t *testing.T) {
+	b := load(t, "./issue_1828")
+
+	if assert.Contains(t, b.Config.Variables, "map") {
+		assert.Equal(t, map[string]any{
+			"foo": "bar",
+		}, b.Config.Variables["map"].Default)
+	}
+
+	if assert.Contains(t, b.Config.Variables, "sequence") {
+		assert.Equal(t, []any{
+			"foo",
+			"bar",
+		}, b.Config.Variables["sequence"].Default)
+	}
+
+	if assert.Contains(t, b.Config.Variables, "string") {
+		assert.Equal(t, "foo", b.Config.Variables["string"].Default)
+	}
+
+	if assert.Contains(t, b.Config.Variables, "bool") {
+		assert.Equal(t, true, b.Config.Variables["bool"].Default)
+	}
+
+	if assert.Contains(t, b.Config.Variables, "int") {
+		assert.Equal(t, 42, b.Config.Variables["int"].Default)
+	}
+
+	if assert.Contains(t, b.Config.Variables, "float") {
+		assert.Equal(t, 3.14, b.Config.Variables["float"].Default)
+	}
+
+	if assert.Contains(t, b.Config.Variables, "time") {
+		assert.Equal(t, "2021-01-01", b.Config.Variables["time"].Default)
+	}
+
+	if assert.Contains(t, b.Config.Variables, "nil") {
+		assert.Equal(t, nil, b.Config.Variables["nil"].Default)
+	}
+}

libs/dyn/convert/normalize.go

@@ -398,6 +398,34 @@ func (n normalizeOptions) normalizeFloat(typ reflect.Type, src dyn.Value, path d
 	return dyn.NewValue(out, src.Locations()), diags
 }
 
-func (n normalizeOptions) normalizeInterface(typ reflect.Type, src dyn.Value, path dyn.Path) (dyn.Value, diag.Diagnostics) {
+func (n normalizeOptions) normalizeInterface(_ reflect.Type, src dyn.Value, path dyn.Path) (dyn.Value, diag.Diagnostics) {
+	// Deal with every [dyn.Kind] here to ensure completeness.
+	switch src.Kind() {
+	case dyn.KindMap:
+		// Fall through
+	case dyn.KindSequence:
+		// Fall through
+	case dyn.KindString:
+		// Fall through
+	case dyn.KindBool:
+		// Fall through
+	case dyn.KindInt:
+		// Fall through
+	case dyn.KindFloat:
+		// Fall through
+	case dyn.KindTime:
+		// Conversion of a time value to an interface{}.
+		// The [dyn.Value.AsAny] equivalent for this kind is the [time.Time] struct.
+		// If we convert to a typed representation and back again, we cannot distinguish
+		// a [time.Time] struct from any other struct.
+		//
+		// Therefore, we normalize the time value to a string.
+		return dyn.NewValue(src.MustTime().String(), src.Locations()), nil
+	case dyn.KindNil:
+		// Fall through
+	default:
+		return dyn.InvalidValue, diag.Errorf("unsupported kind: %s", src.Kind())
+	}
+
 	return src, nil
 }

libs/dyn/convert/normalize_test.go

@@ -858,23 +858,7 @@ func TestNormalizeAnchors(t *testing.T) {
 	}, vout.AsAny())
 }
 
-func TestNormalizeBoolToAny(t *testing.T) {
-	var typ any
-	vin := dyn.NewValue(false, []dyn.Location{{File: "file", Line: 1, Column: 1}})
-	vout, err := Normalize(&typ, vin)
-	assert.Len(t, err, 0)
-	assert.Equal(t, dyn.NewValue(false, []dyn.Location{{File: "file", Line: 1, Column: 1}}), vout)
-}
-
-func TestNormalizeIntToAny(t *testing.T) {
-	var typ any
-	vin := dyn.NewValue(10, []dyn.Location{{File: "file", Line: 1, Column: 1}})
-	vout, err := Normalize(&typ, vin)
-	assert.Len(t, err, 0)
-	assert.Equal(t, dyn.NewValue(10, []dyn.Location{{File: "file", Line: 1, Column: 1}}), vout)
-}
-
-func TestNormalizeSliceToAny(t *testing.T) {
+func TestNormalizeAnyFromSlice(t *testing.T) {
 	var typ any
 	v1 := dyn.NewValue(1, []dyn.Location{{File: "file", Line: 1, Column: 1}})
 	v2 := dyn.NewValue(2, []dyn.Location{{File: "file", Line: 1, Column: 1}})
@@ -883,3 +867,35 @@ func TestNormalizeSliceToAny(t *testing.T) {
 	assert.Len(t, err, 0)
 	assert.Equal(t, dyn.NewValue([]dyn.Value{v1, v2}, []dyn.Location{{File: "file", Line: 1, Column: 1}}), vout)
 }
+
+func TestNormalizeAnyFromString(t *testing.T) {
+	var typ any
+	vin := dyn.NewValue("string", []dyn.Location{{File: "file", Line: 1, Column: 1}})
+	vout, err := Normalize(&typ, vin)
+	assert.Len(t, err, 0)
+	assert.Equal(t, dyn.NewValue("string", []dyn.Location{{File: "file", Line: 1, Column: 1}}), vout)
+}
+
+func TestNormalizeAnyFromBool(t *testing.T) {
+	var typ any
+	vin := dyn.NewValue(false, []dyn.Location{{File: "file", Line: 1, Column: 1}})
+	vout, err := Normalize(&typ, vin)
+	assert.Len(t, err, 0)
+	assert.Equal(t, dyn.NewValue(false, []dyn.Location{{File: "file", Line: 1, Column: 1}}), vout)
+}
+
+func TestNormalizeAnyFromInt(t *testing.T) {
+	var typ any
+	vin := dyn.NewValue(10, []dyn.Location{{File: "file", Line: 1, Column: 1}})
+	vout, err := Normalize(&typ, vin)
+	assert.Len(t, err, 0)
+	assert.Equal(t, dyn.NewValue(10, []dyn.Location{{File: "file", Line: 1, Column: 1}}), vout)
+}
+
+func TestNormalizeAnyFromTime(t *testing.T) {
+	var typ any
+	vin := dyn.NewValue(dyn.MustTime("2024-08-29"), []dyn.Location{{File: "file", Line: 1, Column: 1}})
+	vout, err := Normalize(&typ, vin)
+	assert.Empty(t, err)
+	assert.Equal(t, dyn.NewValue("2024-08-29", vin.Locations()), vout)
+}