From f772ce425960896e2c5b3226117e2c0de52cd714 Mon Sep 17 00:00:00 2001
From: Shreyas Goenka <shreyas.goenka@databricks.com>
Date: Mon, 9 Sep 2024 13:10:36 +0200
Subject: [PATCH 01/39] first comment

---
 bundle/config/resources.go                    |  1 +
 bundle/config/resources/volume.go             | 27 +++++++++
 .../deploy/terraform/tfdyn/convert_volume.go  | 56 +++++++++++++++++++
 3 files changed, 84 insertions(+)
 create mode 100644 bundle/config/resources/volume.go
 create mode 100644 bundle/deploy/terraform/tfdyn/convert_volume.go

diff --git a/bundle/config/resources.go b/bundle/config/resources.go
index 22d69ffb..3aecff86 100644
--- a/bundle/config/resources.go
+++ b/bundle/config/resources.go
@@ -19,6 +19,7 @@ type Resources struct {
 	RegisteredModels      map[string]*resources.RegisteredModel      `json:"registered_models,omitempty"`
 	QualityMonitors       map[string]*resources.QualityMonitor       `json:"quality_monitors,omitempty"`
 	Schemas               map[string]*resources.Schema               `json:"schemas,omitempty"`
+	Volumes               map[string]*resources.Volume               `json:"volumes,omitempty"`
 }
 
 type ConfigResource interface {
diff --git a/bundle/config/resources/volume.go b/bundle/config/resources/volume.go
new file mode 100644
index 00000000..386d2ee3
--- /dev/null
+++ b/bundle/config/resources/volume.go
@@ -0,0 +1,27 @@
+package resources
+
+import (
+	"github.com/databricks/databricks-sdk-go/marshal"
+	"github.com/databricks/databricks-sdk-go/service/catalog"
+)
+
+type Volume struct {
+	// List of grants to apply on this schema.
+	Grants []Grant `json:"grants,omitempty"`
+
+	// Full name of the schema (catalog_name.schema_name.volume_name). This value is read from
+	// the terraform state after deployment succeeds.
+	ID string `json:"id,omitempty" bundle:"readonly"`
+
+	*catalog.CreateVolumeRequestContent
+
+	ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"internal"`
+}
+
+func (v *Volume) UnmarshalJSON(b []byte) error {
+	return marshal.Unmarshal(b, v)
+}
+
+func (v Volume) MarshalJSON() ([]byte, error) {
+	return marshal.Marshal(v)
+}
diff --git a/bundle/deploy/terraform/tfdyn/convert_volume.go b/bundle/deploy/terraform/tfdyn/convert_volume.go
new file mode 100644
index 00000000..d841c7c6
--- /dev/null
+++ b/bundle/deploy/terraform/tfdyn/convert_volume.go
@@ -0,0 +1,56 @@
+package tfdyn
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/databricks/cli/bundle/internal/tf/schema"
+	"github.com/databricks/cli/libs/dyn"
+	"github.com/databricks/cli/libs/dyn/convert"
+	"github.com/databricks/cli/libs/log"
+)
+
+func convertVolumeResource(ctx context.Context, vin dyn.Value) (dyn.Value, error) {
+	// Normalize the output value to the target schema.
+	v, diags := convert.Normalize(schema.ResourceVolume{}, vin)
+	for _, diag := range diags {
+		log.Debugf(ctx, "volume normalization diagnostic: %s", diag.Summary)
+	}
+
+	// TODO: What happens if I try to delete a UC volume that has data in it?
+	// Do I need force destroy functionality here.
+
+	// We always set force destroy as it allows DABs to manage the lifecycle
+	// of the schema. It's the responsibility of the CLI to ensure the user
+	// is adequately warned when they try to delete a UC Volume.
+	vout, err := dyn.SetByPath(v, dyn.MustPathFromString("force_destroy"), dyn.V(true))
+	if err != nil {
+		return dyn.InvalidValue, err
+	}
+
+	return vout, nil
+}
+
+type volumeConverter struct{}
+
+func (volumeConverter) Convert(ctx context.Context, key string, vin dyn.Value, out *schema.Resources) error {
+	vout, err := convertVolumeResource(ctx, vin)
+	if err != nil {
+		return err
+	}
+
+	// Add the converted resource to the output.
+	out.Schema[key] = vout.AsAny()
+
+	// Configure grants for this resource.
+	if grants := convertGrantsResource(ctx, vin); grants != nil {
+		grants.Schema = fmt.Sprintf("${databricks_schema.%s.id}", key)
+		out.Grants["schema_"+key] = grants
+	}
+
+	return nil
+}
+
+func init() {
+	registerConverter("volumes", volumeConverter{})
+}

From ce5792c256801fdff1bee8c3fb7cd782af6a0756 Mon Sep 17 00:00:00 2001
From: Shreyas Goenka <shreyas.goenka@databricks.com>
Date: Mon, 9 Sep 2024 14:40:10 +0200
Subject: [PATCH 02/39] fix convertor and add unit test

---
 .../deploy/terraform/tfdyn/convert_volume.go  | 19 ++---
 .../terraform/tfdyn/convert_volume_test.go    | 70 +++++++++++++++++++
 2 files changed, 75 insertions(+), 14 deletions(-)
 create mode 100644 bundle/deploy/terraform/tfdyn/convert_volume_test.go

diff --git a/bundle/deploy/terraform/tfdyn/convert_volume.go b/bundle/deploy/terraform/tfdyn/convert_volume.go
index d841c7c6..f63e5069 100644
--- a/bundle/deploy/terraform/tfdyn/convert_volume.go
+++ b/bundle/deploy/terraform/tfdyn/convert_volume.go
@@ -10,24 +10,15 @@ import (
 	"github.com/databricks/cli/libs/log"
 )
 
+// TODO: Articulate the consequences of deleting a UC volume in the prompt message that
+// is displayed.
 func convertVolumeResource(ctx context.Context, vin dyn.Value) (dyn.Value, error) {
 	// Normalize the output value to the target schema.
-	v, diags := convert.Normalize(schema.ResourceVolume{}, vin)
+	vout, diags := convert.Normalize(schema.ResourceVolume{}, vin)
 	for _, diag := range diags {
 		log.Debugf(ctx, "volume normalization diagnostic: %s", diag.Summary)
 	}
 
-	// TODO: What happens if I try to delete a UC volume that has data in it?
-	// Do I need force destroy functionality here.
-
-	// We always set force destroy as it allows DABs to manage the lifecycle
-	// of the schema. It's the responsibility of the CLI to ensure the user
-	// is adequately warned when they try to delete a UC Volume.
-	vout, err := dyn.SetByPath(v, dyn.MustPathFromString("force_destroy"), dyn.V(true))
-	if err != nil {
-		return dyn.InvalidValue, err
-	}
-
 	return vout, nil
 }
 
@@ -44,8 +35,8 @@ func (volumeConverter) Convert(ctx context.Context, key string, vin dyn.Value, o
 
 	// Configure grants for this resource.
 	if grants := convertGrantsResource(ctx, vin); grants != nil {
-		grants.Schema = fmt.Sprintf("${databricks_schema.%s.id}", key)
-		out.Grants["schema_"+key] = grants
+		grants.Volume = fmt.Sprintf("${databricks_volume.%s.id}", key)
+		out.Grants["volume_"+key] = grants
 	}
 
 	return nil
diff --git a/bundle/deploy/terraform/tfdyn/convert_volume_test.go b/bundle/deploy/terraform/tfdyn/convert_volume_test.go
new file mode 100644
index 00000000..ded32fcb
--- /dev/null
+++ b/bundle/deploy/terraform/tfdyn/convert_volume_test.go
@@ -0,0 +1,70 @@
+package tfdyn
+
+import (
+	"context"
+	"testing"
+
+	"github.com/databricks/cli/bundle/config/resources"
+	"github.com/databricks/cli/bundle/internal/tf/schema"
+	"github.com/databricks/cli/libs/dyn"
+	"github.com/databricks/cli/libs/dyn/convert"
+	"github.com/databricks/databricks-sdk-go/service/catalog"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestConvertVolume(t *testing.T) {
+	var src = resources.Volume{
+		CreateVolumeRequestContent: &catalog.CreateVolumeRequestContent{
+			CatalogName:     "catalog",
+			Comment:         "comment",
+			Name:            "name",
+			SchemaName:      "schema",
+			StorageLocation: "s3://bucket/path",
+			VolumeType:      "EXTERNAL",
+		},
+		Grants: []resources.Grant{
+			{
+				Privileges: []string{"READ_VOLUME"},
+				Principal:  "jack@gmail.com",
+			},
+			{
+				Privileges: []string{"WRITE_VOLUME"},
+				Principal:  "jane@gmail.com",
+			},
+		},
+	}
+
+	vin, err := convert.FromTyped(src, dyn.NilValue)
+	require.NoError(t, err)
+
+	ctx := context.Background()
+	out := schema.NewResources()
+	err = volumeConverter{}.Convert(ctx, "my_volume", vin, out)
+	require.NoError(t, err)
+
+	// Assert equality on the volume
+	require.Equal(t, map[string]any{
+		"catalog_name":     "catalog",
+		"comment":          "comment",
+		"name":             "name",
+		"schema_name":      "schema",
+		"storage_location": "s3://bucket/path",
+		"volume_type":      "EXTERNAL",
+	}, out.Schema["my_volume"])
+
+	// Assert equality on the grants
+	assert.Equal(t, &schema.ResourceGrants{
+		Volume: "${databricks_volume.my_volume.id}",
+		Grant: []schema.ResourceGrantsGrant{
+			{
+				Privileges: []string{"READ_VOLUME"},
+				Principal:  "jack@gmail.com",
+			},
+			{
+				Privileges: []string{"WRITE_VOLUME"},
+				Principal:  "jane@gmail.com",
+			},
+		},
+	}, out.Grants["volume_my_volume"])
+}

From 7c7abeff815c7a46f39c5ac68e88723008512a63 Mon Sep 17 00:00:00 2001
From: Shreyas Goenka <shreyas.goenka@databricks.com>
Date: Mon, 9 Sep 2024 14:41:42 +0200
Subject: [PATCH 03/39] run as support

---
 bundle/config/mutator/run_as_test.go | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/bundle/config/mutator/run_as_test.go b/bundle/config/mutator/run_as_test.go
index e6cef9ba..fbfdba7b 100644
--- a/bundle/config/mutator/run_as_test.go
+++ b/bundle/config/mutator/run_as_test.go
@@ -40,6 +40,7 @@ func allResourceTypes(t *testing.T) []string {
 		"quality_monitors",
 		"registered_models",
 		"schemas",
+		"volumes",
 	},
 		resourceTypes,
 	)
@@ -138,6 +139,7 @@ func TestRunAsErrorForUnsupportedResources(t *testing.T) {
 		"registered_models",
 		"experiments",
 		"schemas",
+		"volumes",
 	}
 
 	base := config.Root{

From d04b6b08ea247daaf1fa8b3094ac265a307285aa Mon Sep 17 00:00:00 2001
From: Shreyas Goenka <shreyas.goenka@databricks.com>
Date: Mon, 9 Sep 2024 14:52:25 +0200
Subject: [PATCH 04/39] -

---
 bundle/deploy/terraform/tfdyn/convert_volume.go | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/bundle/deploy/terraform/tfdyn/convert_volume.go b/bundle/deploy/terraform/tfdyn/convert_volume.go
index f63e5069..dd06cf18 100644
--- a/bundle/deploy/terraform/tfdyn/convert_volume.go
+++ b/bundle/deploy/terraform/tfdyn/convert_volume.go
@@ -12,6 +12,8 @@ import (
 
 // TODO: Articulate the consequences of deleting a UC volume in the prompt message that
 // is displayed.
+// TODO: What sort of interpolation should be allowed at `artifact_path`? Should it be
+// ${volumes.foo.id} or ${volumes.foo.name} or something else?
 func convertVolumeResource(ctx context.Context, vin dyn.Value) (dyn.Value, error) {
 	// Normalize the output value to the target schema.
 	vout, diags := convert.Normalize(schema.ResourceVolume{}, vin)

From 9b66cd523bf4cad4e4aca6abce202a08eff65530 Mon Sep 17 00:00:00 2001
From: Shreyas Goenka <shreyas.goenka@databricks.com>
Date: Mon, 9 Sep 2024 15:11:57 +0200
Subject: [PATCH 05/39] add apply target mode prefix functionality

---
 bundle/config/mutator/apply_presets.go      |  7 +++
 bundle/config/mutator/apply_presets_test.go | 56 +++++++++++++++++++++
 2 files changed, 63 insertions(+)

diff --git a/bundle/config/mutator/apply_presets.go b/bundle/config/mutator/apply_presets.go
index 28d015c1..c255f357 100644
--- a/bundle/config/mutator/apply_presets.go
+++ b/bundle/config/mutator/apply_presets.go
@@ -160,6 +160,13 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos
 		// the Databricks UI and via the SQL API.
 	}
 
+	// Apply the prefix to volumes
+	for i := range r.Volumes {
+		r.Volumes[i].Name = normalizePrefix(prefix) + r.Volumes[i].Name
+		// HTTP API for volumes doesn't yet support tags. It's only supported in
+		// the Databricks UI and via the SQL API.
+	}
+
 	return nil
 }
 
diff --git a/bundle/config/mutator/apply_presets_test.go b/bundle/config/mutator/apply_presets_test.go
index ab2478ae..b3f4e2d0 100644
--- a/bundle/config/mutator/apply_presets_test.go
+++ b/bundle/config/mutator/apply_presets_test.go
@@ -125,6 +125,62 @@ func TestApplyPresetsPrefixForUcSchema(t *testing.T) {
 	}
 }
 
+func TestApplyPresetsPrefixForUcVolumes(t *testing.T) {
+	tests := []struct {
+		name   string
+		prefix string
+		volume *resources.Volume
+		want   string
+	}{
+		{
+			name:   "add prefix to volume",
+			prefix: "[prefix]",
+			volume: &resources.Volume{
+				CreateVolumeRequestContent: &catalog.CreateVolumeRequestContent{
+					Name: "volume1",
+				},
+			},
+			want: "prefix_volume1",
+		},
+		{
+			name:   "add empty prefix to volume",
+			prefix: "",
+			volume: &resources.Volume{
+				CreateVolumeRequestContent: &catalog.CreateVolumeRequestContent{
+					Name: "volume1",
+				},
+			},
+			want: "volume1",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			b := &bundle.Bundle{
+				Config: config.Root{
+					Resources: config.Resources{
+						Volumes: map[string]*resources.Volume{
+							"volume1": tt.volume,
+						},
+					},
+					Presets: config.Presets{
+						NamePrefix: tt.prefix,
+					},
+				},
+			}
+
+			ctx := context.Background()
+			diag := bundle.Apply(ctx, b, mutator.ApplyPresets())
+
+			if diag.HasError() {
+				t.Fatalf("unexpected error: %v", diag)
+			}
+
+			require.Equal(t, tt.want, b.Config.Resources.Volumes["volume1"].Name)
+		})
+	}
+}
+
 func TestApplyPresetsTags(t *testing.T) {
 	tests := []struct {
 		name string

From 4b22e2d6581d13bec46b62c71d7b2f586ac9cb4e Mon Sep 17 00:00:00 2001
From: Shreyas Goenka <shreyas.goenka@databricks.com>
Date: Mon, 9 Sep 2024 15:36:29 +0200
Subject: [PATCH 06/39] add conversion tests

---
 bundle/config/resources/volume.go       |  3 ++
 bundle/deploy/terraform/convert.go      | 15 +++++++
 bundle/deploy/terraform/convert_test.go | 56 +++++++++++++++++++++++++
 3 files changed, 74 insertions(+)

diff --git a/bundle/config/resources/volume.go b/bundle/config/resources/volume.go
index 386d2ee3..0ee4dce8 100644
--- a/bundle/config/resources/volume.go
+++ b/bundle/config/resources/volume.go
@@ -9,10 +9,13 @@ type Volume struct {
 	// List of grants to apply on this schema.
 	Grants []Grant `json:"grants,omitempty"`
 
+	// TODO: Confirm the accuracy of this comment.
 	// Full name of the schema (catalog_name.schema_name.volume_name). This value is read from
 	// the terraform state after deployment succeeds.
 	ID string `json:"id,omitempty" bundle:"readonly"`
 
+	// TODO: Are there fields in the edit API or terraform that are not in this struct?
+	// If so call it out in the PR.
 	*catalog.CreateVolumeRequestContent
 
 	ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"internal"`
diff --git a/bundle/deploy/terraform/convert.go b/bundle/deploy/terraform/convert.go
index f13c241c..37ea4164 100644
--- a/bundle/deploy/terraform/convert.go
+++ b/bundle/deploy/terraform/convert.go
@@ -394,6 +394,16 @@ func TerraformToBundle(state *resourcesState, config *config.Root) error {
 				}
 				cur.ID = instance.Attributes.ID
 				config.Resources.Schemas[resource.Name] = cur
+			case "databricks_volume":
+				if config.Resources.Volumes == nil {
+					config.Resources.Volumes = make(map[string]*resources.Volume)
+				}
+				cur := config.Resources.Volumes[resource.Name]
+				if cur == nil {
+					cur = &resources.Volume{ModifiedStatus: resources.ModifiedStatusDeleted}
+				}
+				cur.ID = instance.Attributes.ID
+				config.Resources.Volumes[resource.Name] = cur
 			case "databricks_permissions":
 			case "databricks_grants":
 				// Ignore; no need to pull these back into the configuration.
@@ -443,6 +453,11 @@ func TerraformToBundle(state *resourcesState, config *config.Root) error {
 			src.ModifiedStatus = resources.ModifiedStatusCreated
 		}
 	}
+	for _, src := range config.Resources.Volumes {
+		if src.ModifiedStatus == "" && src.ID == "" {
+			src.ModifiedStatus = resources.ModifiedStatusCreated
+		}
+	}
 
 	return nil
 }
diff --git a/bundle/deploy/terraform/convert_test.go b/bundle/deploy/terraform/convert_test.go
index e4ef6114..52c19a30 100644
--- a/bundle/deploy/terraform/convert_test.go
+++ b/bundle/deploy/terraform/convert_test.go
@@ -663,6 +663,14 @@ func TestTerraformToBundleEmptyLocalResources(t *testing.T) {
 					{Attributes: stateInstanceAttributes{ID: "1"}},
 				},
 			},
+			{
+				Type: "databricks_volume",
+				Mode: "managed",
+				Name: "test_volume",
+				Instances: []stateResourceInstance{
+					{Attributes: stateInstanceAttributes{ID: "1"}},
+				},
+			},
 		},
 	}
 	err := TerraformToBundle(&tfState, &config)
@@ -692,6 +700,9 @@ func TestTerraformToBundleEmptyLocalResources(t *testing.T) {
 	assert.Equal(t, "1", config.Resources.Schemas["test_schema"].ID)
 	assert.Equal(t, resources.ModifiedStatusDeleted, config.Resources.Schemas["test_schema"].ModifiedStatus)
 
+	assert.Equal(t, "1", config.Resources.Volumes["test_volume"].ID)
+	assert.Equal(t, resources.ModifiedStatusDeleted, config.Resources.Volumes["test_volume"].ModifiedStatus)
+
 	AssertFullResourceCoverage(t, &config)
 }
 
@@ -754,6 +765,13 @@ func TestTerraformToBundleEmptyRemoteResources(t *testing.T) {
 					},
 				},
 			},
+			Volumes: map[string]*resources.Volume{
+				"test_volume": {
+					CreateVolumeRequestContent: &catalog.CreateVolumeRequestContent{
+						Name: "test_volume",
+					},
+				},
+			},
 		},
 	}
 	var tfState = resourcesState{
@@ -786,6 +804,9 @@ func TestTerraformToBundleEmptyRemoteResources(t *testing.T) {
 	assert.Equal(t, "", config.Resources.Schemas["test_schema"].ID)
 	assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.Schemas["test_schema"].ModifiedStatus)
 
+	assert.Equal(t, "", config.Resources.Volumes["test_volume"].ID)
+	assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.Volumes["test_volume"].ModifiedStatus)
+
 	AssertFullResourceCoverage(t, &config)
 }
 
@@ -888,6 +909,18 @@ func TestTerraformToBundleModifiedResources(t *testing.T) {
 					},
 				},
 			},
+			Volumes: map[string]*resources.Volume{
+				"test_volume": {
+					CreateVolumeRequestContent: &catalog.CreateVolumeRequestContent{
+						Name: "test_volume",
+					},
+				},
+				"test_volume_new": {
+					CreateVolumeRequestContent: &catalog.CreateVolumeRequestContent{
+						Name: "test_volume_new",
+					},
+				},
+			},
 		},
 	}
 	var tfState = resourcesState{
@@ -1020,6 +1053,22 @@ func TestTerraformToBundleModifiedResources(t *testing.T) {
 					{Attributes: stateInstanceAttributes{ID: "2"}},
 				},
 			},
+			{
+				Type: "databricks_volume",
+				Mode: "managed",
+				Name: "test_volume",
+				Instances: []stateResourceInstance{
+					{Attributes: stateInstanceAttributes{ID: "1"}},
+				},
+			},
+			{
+				Type: "databricks_volume",
+				Mode: "managed",
+				Name: "test_volume_old",
+				Instances: []stateResourceInstance{
+					{Attributes: stateInstanceAttributes{ID: "2"}},
+				},
+			},
 		},
 	}
 	err := TerraformToBundle(&tfState, &config)
@@ -1081,6 +1130,13 @@ func TestTerraformToBundleModifiedResources(t *testing.T) {
 	assert.Equal(t, "", config.Resources.Schemas["test_schema_new"].ID)
 	assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.Schemas["test_schema_new"].ModifiedStatus)
 
+	assert.Equal(t, "1", config.Resources.Volumes["test_volume"].ID)
+	assert.Equal(t, "", config.Resources.Volumes["test_volume"].ModifiedStatus)
+	assert.Equal(t, "2", config.Resources.Volumes["test_volume_old"].ID)
+	assert.Equal(t, resources.ModifiedStatusDeleted, config.Resources.Volumes["test_volume_old"].ModifiedStatus)
+	assert.Equal(t, "", config.Resources.Volumes["test_volume_new"].ID)
+	assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.Volumes["test_volume_new"].ModifiedStatus)
+
 	AssertFullResourceCoverage(t, &config)
 }
 

From 88d0402f441561f31a1066af6c8c49aed086f769 Mon Sep 17 00:00:00 2001
From: Shreyas Goenka <shreyas.goenka@databricks.com>
Date: Mon, 9 Sep 2024 15:41:03 +0200
Subject: [PATCH 07/39] add inteprolation for volumes fields

---
 bundle/deploy/terraform/interpolate.go      | 2 ++
 bundle/deploy/terraform/interpolate_test.go | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/bundle/deploy/terraform/interpolate.go b/bundle/deploy/terraform/interpolate.go
index faa098e1..96a71c16 100644
--- a/bundle/deploy/terraform/interpolate.go
+++ b/bundle/deploy/terraform/interpolate.go
@@ -58,6 +58,8 @@ func (m *interpolateMutator) Apply(ctx context.Context, b *bundle.Bundle) diag.D
 				path = dyn.NewPath(dyn.Key("databricks_quality_monitor")).Append(path[2:]...)
 			case dyn.Key("schemas"):
 				path = dyn.NewPath(dyn.Key("databricks_schema")).Append(path[2:]...)
+			case dyn.Key("volumes"):
+				path = dyn.NewPath(dyn.Key("databricks_volume")).Append(path[2:]...)
 			default:
 				// Trigger "key not found" for unknown resource types.
 				return dyn.GetByPath(root, path)
diff --git a/bundle/deploy/terraform/interpolate_test.go b/bundle/deploy/terraform/interpolate_test.go
index 5ceb243b..48900999 100644
--- a/bundle/deploy/terraform/interpolate_test.go
+++ b/bundle/deploy/terraform/interpolate_test.go
@@ -31,6 +31,7 @@ func TestInterpolate(t *testing.T) {
 								"other_model_serving":    "${resources.model_serving_endpoints.other_model_serving.id}",
 								"other_registered_model": "${resources.registered_models.other_registered_model.id}",
 								"other_schema":           "${resources.schemas.other_schema.id}",
+								"other_volume":           "${resources.volumes.other_volume.id}",
 							},
 							Tasks: []jobs.Task{
 								{
@@ -67,6 +68,7 @@ func TestInterpolate(t *testing.T) {
 	assert.Equal(t, "${databricks_model_serving.other_model_serving.id}", j.Tags["other_model_serving"])
 	assert.Equal(t, "${databricks_registered_model.other_registered_model.id}", j.Tags["other_registered_model"])
 	assert.Equal(t, "${databricks_schema.other_schema.id}", j.Tags["other_schema"])
+	assert.Equal(t, "${databricks_volume.other_volume.id}", j.Tags["other_volume"])
 
 	m := b.Config.Resources.Models["my_model"]
 	assert.Equal(t, "my_model", m.Model.Name)

From 6f9817e194092e47c7e5b41da4df62f1ea7caeeb Mon Sep 17 00:00:00 2001
From: Shreyas Goenka <shreyas.goenka@databricks.com>
Date: Tue, 10 Sep 2024 12:52:31 +0200
Subject: [PATCH 08/39] add prompt and crud test for volumes

---
 .../deploy/terraform/tfdyn/convert_volume.go  |  2 +-
 bundle/phases/deploy.go                       | 46 ++++++++--------
 bundle/phases/deploy_test.go                  |  2 +-
 .../uc_volume/databricks_template_schema.json |  8 +++
 .../uc_volume/template/databricks.yml.tmpl    | 26 +++++++++
 .../bundle/bundles/uc_volume/template/nb.sql  |  2 +
 internal/bundle/deploy_test.go                | 53 +++++++++++++++++++
 7 files changed, 112 insertions(+), 27 deletions(-)
 create mode 100644 internal/bundle/bundles/uc_volume/databricks_template_schema.json
 create mode 100644 internal/bundle/bundles/uc_volume/template/databricks.yml.tmpl
 create mode 100644 internal/bundle/bundles/uc_volume/template/nb.sql

diff --git a/bundle/deploy/terraform/tfdyn/convert_volume.go b/bundle/deploy/terraform/tfdyn/convert_volume.go
index dd06cf18..281cd432 100644
--- a/bundle/deploy/terraform/tfdyn/convert_volume.go
+++ b/bundle/deploy/terraform/tfdyn/convert_volume.go
@@ -33,7 +33,7 @@ func (volumeConverter) Convert(ctx context.Context, key string, vin dyn.Value, o
 	}
 
 	// Add the converted resource to the output.
-	out.Schema[key] = vout.AsAny()
+	out.Volume[key] = vout.AsAny()
 
 	// Configure grants for this resource.
 	if grants := convertGrantsResource(ctx, vin); grants != nil {
diff --git a/bundle/phases/deploy.go b/bundle/phases/deploy.go
index 49544227..d71c6191 100644
--- a/bundle/phases/deploy.go
+++ b/bundle/phases/deploy.go
@@ -22,10 +22,12 @@ import (
 	tfjson "github.com/hashicorp/terraform-json"
 )
 
-func parseTerraformActions(changes []*tfjson.ResourceChange, toInclude func(typ string, actions tfjson.Actions) bool) []terraformlib.Action {
+func filterDeleteOrRecreateActions(changes []*tfjson.ResourceChange, resourceType string) []terraformlib.Action {
 	res := make([]terraformlib.Action, 0)
 	for _, rc := range changes {
-		if !toInclude(rc.Type, rc.Change.Actions) {
+		// TODO: Add end to end integration tests for the interactive prompt UXs.
+		// Good PR to introduce the first one, and make changes more confidently.
+		if rc.Type != resourceType {
 			continue
 		}
 
@@ -36,7 +38,7 @@ func parseTerraformActions(changes []*tfjson.ResourceChange, toInclude func(typ
 		case rc.Change.Actions.Replace():
 			actionType = terraformlib.ActionTypeRecreate
 		default:
-			// No use case for other action types yet.
+			// Filter other action types..
 			continue
 		}
 
@@ -62,30 +64,12 @@ func approvalForDeploy(ctx context.Context, b *bundle.Bundle) (bool, error) {
 		return false, err
 	}
 
-	schemaActions := parseTerraformActions(plan.ResourceChanges, func(typ string, actions tfjson.Actions) bool {
-		// Filter in only UC schema resources.
-		if typ != "databricks_schema" {
-			return false
-		}
-
-		// We only display prompts for destructive actions like deleting or
-		// recreating a schema.
-		return actions.Delete() || actions.Replace()
-	})
-
-	dltActions := parseTerraformActions(plan.ResourceChanges, func(typ string, actions tfjson.Actions) bool {
-		// Filter in only DLT pipeline resources.
-		if typ != "databricks_pipeline" {
-			return false
-		}
-
-		// Recreating DLT pipeline leads to metadata loss and for a transient period
-		// the underling tables will be unavailable.
-		return actions.Replace() || actions.Delete()
-	})
+	schemaActions := filterDeleteOrRecreateActions(plan.ResourceChanges, "databricks_schema")
+	dltActions := filterDeleteOrRecreateActions(plan.ResourceChanges, "databricks_pipeline")
+	volumeActions := filterDeleteOrRecreateActions(plan.ResourceChanges, "databricks_volume")
 
 	// We don't need to display any prompts in this case.
-	if len(dltActions) == 0 && len(schemaActions) == 0 {
+	if len(schemaActions) == 0 && len(dltActions) == 0 && len(volumeActions) == 0 {
 		return true, nil
 	}
 
@@ -110,6 +94,18 @@ properties such as the 'catalog' or 'storage' are changed:`
 		}
 	}
 
+	// One or more volumes is being recreated.
+	if len(volumeActions) != 0 {
+		msg := `
+This action will result in the deletion or recreation of the following Volumes. For managed volumes,
+this typically results in a deletion of the upstream data in the cloud tenant in ~30 days. For external
+volumes the upstream data in the cloud tenant is not affected:`
+		cmdio.LogString(ctx, msg)
+		for _, action := range volumeActions {
+			cmdio.Log(ctx, action)
+		}
+	}
+
 	if b.AutoApprove {
 		return true, nil
 	}
diff --git a/bundle/phases/deploy_test.go b/bundle/phases/deploy_test.go
index e00370b3..f239302d 100644
--- a/bundle/phases/deploy_test.go
+++ b/bundle/phases/deploy_test.go
@@ -40,7 +40,7 @@ func TestParseTerraformActions(t *testing.T) {
 		},
 	}
 
-	res := parseTerraformActions(changes, func(typ string, actions tfjson.Actions) bool {
+	res := filterDeleteOrRecreateActions(changes, func(typ string, actions tfjson.Actions) bool {
 		if typ != "databricks_pipeline" {
 			return false
 		}
diff --git a/internal/bundle/bundles/uc_volume/databricks_template_schema.json b/internal/bundle/bundles/uc_volume/databricks_template_schema.json
new file mode 100644
index 00000000..762f4470
--- /dev/null
+++ b/internal/bundle/bundles/uc_volume/databricks_template_schema.json
@@ -0,0 +1,8 @@
+{
+    "properties": {
+        "unique_id": {
+            "type": "string",
+            "description": "Unique ID for the schema and pipeline names"
+        }
+    }
+}
diff --git a/internal/bundle/bundles/uc_volume/template/databricks.yml.tmpl b/internal/bundle/bundles/uc_volume/template/databricks.yml.tmpl
new file mode 100644
index 00000000..9d377f8e
--- /dev/null
+++ b/internal/bundle/bundles/uc_volume/template/databricks.yml.tmpl
@@ -0,0 +1,26 @@
+bundle:
+  name: test-uc-volumes-{{.unique_id}}
+
+variables:
+  schema_name:
+    default: ${resources.schemas.schema1.name}
+
+resources:
+  schemas:
+    schema1:
+      name: schema1-{{.unique_id}}
+      catalog_name: main
+      comment: This schema was created from DABs
+
+    schema2:
+      name: schema2-{{.unique_id}}
+      catalog_name: main
+      comment: This schema was created from DABs
+
+  volumes:
+    foo:
+      catalog_name: main
+      name: my_volume
+      schema_name: ${var.schema_name}
+      volume_type: MANAGED
+      comment: This volume was created from DABs.
diff --git a/internal/bundle/bundles/uc_volume/template/nb.sql b/internal/bundle/bundles/uc_volume/template/nb.sql
new file mode 100644
index 00000000..199ff507
--- /dev/null
+++ b/internal/bundle/bundles/uc_volume/template/nb.sql
@@ -0,0 +1,2 @@
+-- Databricks notebook source
+select 1
diff --git a/internal/bundle/deploy_test.go b/internal/bundle/deploy_test.go
index 736c880d..225458e1 100644
--- a/internal/bundle/deploy_test.go
+++ b/internal/bundle/deploy_test.go
@@ -243,3 +243,56 @@ func TestAccDeployBasicBundleLogs(t *testing.T) {
 	}, "\n"), stderr)
 	assert.Equal(t, "", stdout)
 }
+
+func TestAccDeployUcVolume(t *testing.T) {
+	ctx, wt := acc.UcWorkspaceTest(t)
+	w := wt.W
+
+	uniqueId := uuid.New().String()
+	bundleRoot, err := initTestTemplate(t, ctx, "uc_volume", map[string]any{
+		"unique_id": uniqueId,
+	})
+	require.NoError(t, err)
+
+	err = deployBundle(t, ctx, bundleRoot)
+	require.NoError(t, err)
+
+	t.Cleanup(func() {
+		destroyBundle(t, ctx, bundleRoot)
+	})
+
+	// Assert the volume is created successfully
+	catalogName := "main"
+	schemaName := "schema1-" + uniqueId
+	volumeName := "my_volume"
+	volume, err := w.Volumes.ReadByName(ctx, fmt.Sprintf("%s.%s.%s", catalogName, schemaName, volumeName))
+	require.NoError(t, err)
+	require.Equal(t, volume.Name, volumeName)
+	require.Equal(t, catalogName, volume.CatalogName)
+	require.Equal(t, schemaName, volume.SchemaName)
+
+	// Recreation of the volume without --auto-approve should fail since prompting is not possible
+	t.Setenv("TERM", "dumb")
+	t.Setenv("BUNDLE_ROOT", bundleRoot)
+	stdout, stderr, err := internal.NewCobraTestRunnerWithContext(t, ctx, "bundle", "deploy", "--var=schema_name=${resources.schemas.schema2.name}").Run()
+	assert.Error(t, err)
+	assert.Contains(t, stderr.String(), `This action will result in the deletion or recreation of the following Volumes. For managed volumes,
+this typically results in a deletion of the upstream data in the cloud tenant in ~30 days. For external
+volumes the upstream data in the cloud tenant is not affected:
+  recreate volume foo`)
+	assert.Contains(t, stdout.String(), "the deployment requires destructive actions, but current console does not support prompting. Please specify --auto-approve if you would like to skip prompts and proceed")
+
+	// Recreation of the volume without --auto-approve should fail since prompting is not possible
+	t.Setenv("TERM", "dumb")
+	t.Setenv("BUNDLE_ROOT", bundleRoot)
+	_, _, err = internal.NewCobraTestRunnerWithContext(t, ctx, "bundle", "deploy", "--var=schema_name=${resources.schemas.schema2.name}", "--auto-approve").Run()
+	assert.NoError(t, err)
+
+	// Assert the volume is updated successfully
+	schemaName = "schema2-" + uniqueId
+	volume, err = w.Volumes.ReadByName(ctx, fmt.Sprintf("%s.%s.%s", catalogName, schemaName, volumeName))
+	require.NoError(t, err)
+	require.Equal(t, volume.Name, volumeName)
+	require.Equal(t, catalogName, volume.CatalogName)
+	require.Equal(t, schemaName, volume.SchemaName)
+}

From d180bab15d41c83b6556a1bd9466d7f37c9f1c71 Mon Sep 17 00:00:00 2001
From: Shreyas Goenka <shreyas.goenka@databricks.com>
Date: Sun, 15 Sep 2024 23:19:40 +0200
Subject: [PATCH 09/39] add filer

---
 bundle/artifacts/upload.go |  6 +--
 bundle/libraries/upload.go | 86 +++++++++++++++++++++++++++++++++-----
 libs/dyn/dynvar/ref.go     |  4 ++
 3 files changed, 82 insertions(+), 14 deletions(-)

diff --git a/bundle/artifacts/upload.go b/bundle/artifacts/upload.go
index 58c006dc..d7b3e1ba 100644
--- a/bundle/artifacts/upload.go
+++ b/bundle/artifacts/upload.go
@@ -26,9 +26,9 @@ func (m *cleanUp) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics
 		return diag.FromErr(err)
 	}
 
-	client, err := libraries.GetFilerForLibraries(b.WorkspaceClient(), uploadPath)
-	if err != nil {
-		return diag.FromErr(err)
+	client, diags := libraries.GetFilerForLibraries(ctx, b, uploadPath)
+	if diags.HasError() {
+		return diags
 	}
 
 	// We intentionally ignore the error because it is not critical to the deployment
diff --git a/bundle/libraries/upload.go b/bundle/libraries/upload.go
index 90a1a21f..a45db5dc 100644
--- a/bundle/libraries/upload.go
+++ b/bundle/libraries/upload.go
@@ -13,11 +13,10 @@ import (
 	"github.com/databricks/cli/libs/cmdio"
 	"github.com/databricks/cli/libs/diag"
 	"github.com/databricks/cli/libs/dyn"
+	"github.com/databricks/cli/libs/dyn/dynvar"
 	"github.com/databricks/cli/libs/filer"
 	"github.com/databricks/cli/libs/log"
 
-	"github.com/databricks/databricks-sdk-go"
-
 	"golang.org/x/sync/errgroup"
 )
 
@@ -138,9 +137,9 @@ func (u *upload) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
 	// If the client is not initialized, initialize it
 	// We use client field in mutator to allow for mocking client in testing
 	if u.client == nil {
-		filer, err := GetFilerForLibraries(b.WorkspaceClient(), uploadPath)
-		if err != nil {
-			return diag.FromErr(err)
+		filer, diags := GetFilerForLibraries(ctx, b, uploadPath)
+		if diags.HasError() {
+			return diags
 		}
 
 		u.client = filer
@@ -197,15 +196,80 @@ func (u *upload) Name() string {
 	return "libraries.Upload"
 }
 
-func GetFilerForLibraries(w *databricks.WorkspaceClient, uploadPath string) (filer.Filer, error) {
-	if isVolumesPath(uploadPath) {
-		return filer.NewFilesClient(w, uploadPath)
+// TODO: TODO: Nicer comments here.
+// Case 1: UC volume path is valid. Return the client.
+// Case 2: invalid path.
+// (a) Not enough elements.
+// (b) catalog and schema correspond to a volume define in the DAB.
+//
+//	-> exception for when the schema value is fully or partially interpolated.
+//	   In that case only check the catalog name.
+func GetFilerForLibraries(ctx context.Context, b *bundle.Bundle, uploadPath string) (filer.Filer, diag.Diagnostics) {
+	w := b.WorkspaceClient()
+	isVolumesPath := strings.HasPrefix(uploadPath, "/Volumes/")
+
+	// If the path is not a volume path, use the workspace file system.
+	if !isVolumesPath {
+		f, err := filer.NewWorkspaceFilesClient(w, uploadPath)
+		return f, diag.FromErr(err)
 	}
-	return filer.NewWorkspaceFilesClient(w, uploadPath)
+
+	parts := strings.Split(uploadPath, "/")
+	volumeFormatErr := fmt.Errorf("expected UC volume path to be in the format /Volumes/<catalog>/<schema>/<path>, got %s", uploadPath)
+	if len(strings.Split(uploadPath, "/")) < 5 {
+		return nil, diag.FromErr(volumeFormatErr)
+	}
+
+	catalogName := parts[2]
+	schemaName := parts[3]
+	volumeName := parts[4]
+
+	// Incorrect format.
+	if catalogName == "" || schemaName == "" || volumeName == "" {
+		return nil, diag.FromErr(volumeFormatErr)
+	}
+
+	// If the volume exists already, directly return the filer for the upload path.
+	volumePath := fmt.Sprintf("/Volumes/%s/%s/%s", catalogName, schemaName, volumeName)
+	vf, err := filer.NewFilesClient(w, volumePath)
+	if err != nil {
+		return nil, diag.FromErr(err)
+	}
+	if _, err := vf.Stat(ctx, "."); err == nil {
+		f, err := filer.NewFilesClient(w, uploadPath)
+		return f, diag.FromErr(err)
+	}
+
+	// The volume does not exist. Check if the volume is defined in the bundle.
+	// TODO: Does this break? Did it work before if the volume was not defined, but
+	// the schema was?
+	l, ok := locationForVolume(b, catalogName, schemaName, volumeName)
+	if !ok {
+		return nil, diag.Errorf("UC volume %s does not exist", volumePath)
+	}
+
+	return nil, diag.Errorf(`UC volume %s does not exist. Note: We detected that
+you have a UC volume defined that matched the path above at %s.
+Please deploy the UC volume separately before using it in as a
+destination to upload artifacts.`, l, volumePath)
 }
 
-func isVolumesPath(path string) bool {
-	return strings.HasPrefix(path, "/Volumes/")
+func locationForVolume(b *bundle.Bundle, catalogName, schemaName, volumeName string) (dyn.Location, bool) {
+	volumes := b.Config.Resources.Volumes
+	for k, v := range volumes {
+		if v.CatalogName != catalogName || v.Name != volumeName {
+			continue
+		}
+		// UC schemas can be defined in the bundle itself, and thus might be interpolated
+		// at runtime via the ${resources.schemas.<name>} syntax. Thus we match the volume
+		// definition if the schema name is the same as the one in the bundle, or if the
+		// schema name is interpolated.
+		if v.SchemaName != schemaName && !dynvar.ContainsVariableReference(v.SchemaName) {
+			continue
+		}
+		return b.Config.GetLocation(fmt.Sprintf("resources.volumes.%s", k)), true
+	}
+	return dyn.Location{}, false
 }
 
 // Function to upload file (a library, artifact and etc) to Workspace or UC volume
diff --git a/libs/dyn/dynvar/ref.go b/libs/dyn/dynvar/ref.go
index 338ac8ce..088b68d2 100644
--- a/libs/dyn/dynvar/ref.go
+++ b/libs/dyn/dynvar/ref.go
@@ -71,3 +71,7 @@ func (v ref) references() []string {
 func IsPureVariableReference(s string) bool {
 	return len(s) > 0 && re.FindString(s) == s
 }
+
+func ContainsVariableReference(s string) bool {
+	return len(s) > 0 && re.FindString(s) != ""
+}

From 73826acb2f7f5c9af6e84ea57488c44153e4f9c1 Mon Sep 17 00:00:00 2001
From: Shreyas Goenka <shreyas.goenka@databricks.com>
Date: Sun, 15 Sep 2024 23:31:04 +0200
Subject: [PATCH 10/39] fix test and improve error

---
 bundle/libraries/upload.go   | 11 ++++++-----
 bundle/phases/deploy_test.go | 12 +-----------
 2 files changed, 7 insertions(+), 16 deletions(-)

diff --git a/bundle/libraries/upload.go b/bundle/libraries/upload.go
index a45db5dc..c4d6273b 100644
--- a/bundle/libraries/upload.go
+++ b/bundle/libraries/upload.go
@@ -245,13 +245,14 @@ func GetFilerForLibraries(ctx context.Context, b *bundle.Bundle, uploadPath stri
 	// the schema was?
 	l, ok := locationForVolume(b, catalogName, schemaName, volumeName)
 	if !ok {
-		return nil, diag.Errorf("UC volume %s does not exist", volumePath)
+		return nil, diag.Errorf("the bundle is configured to upload artifacts to %s but a UC volume at %s does not exist", uploadPath, volumePath)
 	}
 
-	return nil, diag.Errorf(`UC volume %s does not exist. Note: We detected that
-you have a UC volume defined that matched the path above at %s.
-Please deploy the UC volume separately before using it in as a
-destination to upload artifacts.`, l, volumePath)
+	return nil, diag.Errorf(`the bundle is configured to upload artifacts to %s but a
+UC volume at %s does not exist. Note: We detected that you have a UC volume
+defined that matched the path above at %s. Please deploy the UC volume
+in a separate deployment before using it in as a destination to upload
+artifacts.`, uploadPath, volumePath, l)
 }
 
 func locationForVolume(b *bundle.Bundle, catalogName, schemaName, volumeName string) (dyn.Location, bool) {
diff --git a/bundle/phases/deploy_test.go b/bundle/phases/deploy_test.go
index f239302d..2cef9540 100644
--- a/bundle/phases/deploy_test.go
+++ b/bundle/phases/deploy_test.go
@@ -40,17 +40,7 @@ func TestParseTerraformActions(t *testing.T) {
 		},
 	}
 
-	res := filterDeleteOrRecreateActions(changes, func(typ string, actions tfjson.Actions) bool {
-		if typ != "databricks_pipeline" {
-			return false
-		}
-
-		if actions.Delete() || actions.Replace() {
-			return true
-		}
-
-		return false
-	})
+	res := filterDeleteOrRecreateActions(changes, "databricks_pipeline")
 
 	assert.Equal(t, []terraformlib.Action{
 		{

From de7eb94e457fadcdc4cba65855ec43659bffaaf8 Mon Sep 17 00:00:00 2001
From: Shreyas Goenka <shreyas.goenka@databricks.com>
Date: Sun, 15 Sep 2024 23:37:28 +0200
Subject: [PATCH 11/39] -

---
 bundle/libraries/upload.go | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/bundle/libraries/upload.go b/bundle/libraries/upload.go
index c4d6273b..06046804 100644
--- a/bundle/libraries/upload.go
+++ b/bundle/libraries/upload.go
@@ -204,6 +204,8 @@ func (u *upload) Name() string {
 //
 //	-> exception for when the schema value is fully or partially interpolated.
 //	   In that case only check the catalog name.
+//
+// TODO: Convert to warning? We don't error today if you specify an invalid volume path.
 func GetFilerForLibraries(ctx context.Context, b *bundle.Bundle, uploadPath string) (filer.Filer, diag.Diagnostics) {
 	w := b.WorkspaceClient()
 	isVolumesPath := strings.HasPrefix(uploadPath, "/Volumes/")
@@ -241,8 +243,7 @@ func GetFilerForLibraries(ctx context.Context, b *bundle.Bundle, uploadPath stri
 	}
 
 	// The volume does not exist. Check if the volume is defined in the bundle.
-	// TODO: Does this break? Did it work before if the volume was not defined, but
-	// the schema was?
+	// TODO: Note that this is not a breaking change.
 	l, ok := locationForVolume(b, catalogName, schemaName, volumeName)
 	if !ok {
 		return nil, diag.Errorf("the bundle is configured to upload artifacts to %s but a UC volume at %s does not exist", uploadPath, volumePath)

From f10038a20e0e0cd52288ccf722429b4eb3fc1fed Mon Sep 17 00:00:00 2001
From: Shreyas Goenka <shreyas.goenka@databricks.com>
Date: Sun, 15 Sep 2024 23:45:01 +0200
Subject: [PATCH 12/39] -

---
 bundle/config/mutator/process_target_mode_test.go    | 7 +++++++
 bundle/deploy/terraform/tfdyn/convert_volume_test.go | 2 +-
 bundle/libraries/upload.go                           | 1 +
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/bundle/config/mutator/process_target_mode_test.go b/bundle/config/mutator/process_target_mode_test.go
index 42f1929c..75888a12 100644
--- a/bundle/config/mutator/process_target_mode_test.go
+++ b/bundle/config/mutator/process_target_mode_test.go
@@ -119,6 +119,9 @@ func mockBundle(mode config.Mode) *bundle.Bundle {
 				Schemas: map[string]*resources.Schema{
 					"schema1": {CreateSchema: &catalog.CreateSchema{Name: "schema1"}},
 				},
+				Volumes: map[string]*resources.Volume{
+					"volume1": {CreateVolumeRequestContent: &catalog.CreateVolumeRequestContent{Name: "volume1"}},
+				},
 			},
 		},
 		// Use AWS implementation for testing.
@@ -281,6 +284,8 @@ func TestProcessTargetModeDefault(t *testing.T) {
 	assert.Equal(t, "servingendpoint1", b.Config.Resources.ModelServingEndpoints["servingendpoint1"].Name)
 	assert.Equal(t, "registeredmodel1", b.Config.Resources.RegisteredModels["registeredmodel1"].Name)
 	assert.Equal(t, "qualityMonitor1", b.Config.Resources.QualityMonitors["qualityMonitor1"].TableName)
+	assert.Equal(t, "schema1", b.Config.Resources.Schemas["schema1"].Name)
+	assert.Equal(t, "volume1", b.Config.Resources.Volumes["volume1"].Name)
 }
 
 func TestProcessTargetModeProduction(t *testing.T) {
@@ -322,6 +327,8 @@ func TestProcessTargetModeProduction(t *testing.T) {
 	assert.Equal(t, "servingendpoint1", b.Config.Resources.ModelServingEndpoints["servingendpoint1"].Name)
 	assert.Equal(t, "registeredmodel1", b.Config.Resources.RegisteredModels["registeredmodel1"].Name)
 	assert.Equal(t, "qualityMonitor1", b.Config.Resources.QualityMonitors["qualityMonitor1"].TableName)
+	assert.Equal(t, "schema1", b.Config.Resources.Schemas["schema1"].Name)
+	assert.Equal(t, "volume1", b.Config.Resources.Volumes["volume1"].Name)
 }
 
 func TestProcessTargetModeProductionOkForPrincipal(t *testing.T) {
diff --git a/bundle/deploy/terraform/tfdyn/convert_volume_test.go b/bundle/deploy/terraform/tfdyn/convert_volume_test.go
index ded32fcb..c897ae69 100644
--- a/bundle/deploy/terraform/tfdyn/convert_volume_test.go
+++ b/bundle/deploy/terraform/tfdyn/convert_volume_test.go
@@ -51,7 +51,7 @@ func TestConvertVolume(t *testing.T) {
 		"schema_name":      "schema",
 		"storage_location": "s3://bucket/path",
 		"volume_type":      "EXTERNAL",
-	}, out.Schema["my_volume"])
+	}, out.Volume["my_volume"])
 
 	// Assert equality on the grants
 	assert.Equal(t, &schema.ResourceGrants{
diff --git a/bundle/libraries/upload.go b/bundle/libraries/upload.go
index 06046804..0e651189 100644
--- a/bundle/libraries/upload.go
+++ b/bundle/libraries/upload.go
@@ -244,6 +244,7 @@ func GetFilerForLibraries(ctx context.Context, b *bundle.Bundle, uploadPath stri
 
 	// The volume does not exist. Check if the volume is defined in the bundle.
 	// TODO: Note that this is not a breaking change.
+	// TODO: Include error as well for why the stat call failed. It's more context.
 	l, ok := locationForVolume(b, catalogName, schemaName, volumeName)
 	if !ok {
 		return nil, diag.Errorf("the bundle is configured to upload artifacts to %s but a UC volume at %s does not exist", uploadPath, volumePath)

From df3bbad70b47a62cf1c2455e0723f386dd2a3026 Mon Sep 17 00:00:00 2001
From: Shreyas Goenka <shreyas.goenka@databricks.com>
Date: Mon, 16 Sep 2024 00:50:12 +0200
Subject: [PATCH 13/39] add integration tests for artifacts portion:

---
 internal/bundle/artifacts_test.go | 107 ++++++++++++++++++++++++++++++
 1 file changed, 107 insertions(+)

diff --git a/internal/bundle/artifacts_test.go b/internal/bundle/artifacts_test.go
index fa052e22..213aed20 100644
--- a/internal/bundle/artifacts_test.go
+++ b/internal/bundle/artifacts_test.go
@@ -1,6 +1,7 @@
 package bundle
 
 import (
+	"fmt"
 	"os"
 	"path"
 	"path/filepath"
@@ -13,6 +14,8 @@ import (
 	"github.com/databricks/cli/bundle/libraries"
 	"github.com/databricks/cli/internal"
 	"github.com/databricks/cli/internal/acc"
+	"github.com/databricks/cli/libs/dyn"
+	"github.com/databricks/databricks-sdk-go/service/catalog"
 	"github.com/databricks/databricks-sdk-go/service/compute"
 	"github.com/databricks/databricks-sdk-go/service/jobs"
 	"github.com/stretchr/testify/require"
@@ -225,3 +228,107 @@ func TestAccUploadArtifactFileToCorrectRemotePathForVolumes(t *testing.T) {
 		b.Config.Resources.Jobs["test"].JobSettings.Tasks[0].Libraries[0].Whl,
 	)
 }
+
+func TestAccUploadArtifactFileToInvalidVolume(t *testing.T) {
+	ctx, wt := acc.UcWorkspaceTest(t)
+	w := wt.W
+
+	schemaName := internal.RandomName("schema-")
+
+	_, err := w.Schemas.Create(ctx, catalog.CreateSchema{
+		CatalogName: "main",
+		Comment:     "test schema",
+		Name:        schemaName,
+	})
+	require.NoError(t, err)
+
+	t.Run("volume not in DAB", func(t *testing.T) {
+		volumePath := fmt.Sprintf("/Volumes/main/%s/doesnotexist", schemaName)
+		dir := t.TempDir()
+
+		b := &bundle.Bundle{
+			RootPath:     dir,
+			SyncRootPath: dir,
+			Config: config.Root{
+				Bundle: config.Bundle{
+					Target: "whatever",
+				},
+				Workspace: config.Workspace{
+					ArtifactPath: volumePath,
+				},
+				Resources: config.Resources{
+					Volumes: map[string]*resources.Volume{
+						"foo": {
+							CreateVolumeRequestContent: &catalog.CreateVolumeRequestContent{
+								CatalogName: "main",
+								Name:        "my_volume",
+								VolumeType:  "MANAGED",
+								SchemaName:  schemaName,
+							},
+						},
+					},
+				},
+			},
+		}
+
+		diags := bundle.Apply(ctx, b, bundle.Seq(libraries.ExpandGlobReferences(), libraries.Upload()))
+		require.EqualError(t, diags.Error(), fmt.Sprintf("the bundle is configured to upload artifacts to %s but a UC volume at %s does not exist", path.Join(volumePath, ".internal"), volumePath))
+	})
+
+	t.Run("volume in DAB config", func(t *testing.T) {
+		volumePath := fmt.Sprintf("/Volumes/main/%s/my_volume", schemaName)
+		dir := t.TempDir()
+
+		b := &bundle.Bundle{
+			RootPath:     dir,
+			SyncRootPath: dir,
+			Config: config.Root{
+				Bundle: config.Bundle{
+					Target: "whatever",
+				},
+				Workspace: config.Workspace{
+					ArtifactPath: volumePath,
+				},
+				Resources: config.Resources{
+					Volumes: map[string]*resources.Volume{
+						"foo": {
+							CreateVolumeRequestContent: &catalog.CreateVolumeRequestContent{
+								CatalogName: "main",
+								Name:        "my_volume",
+								VolumeType:  "MANAGED",
+								SchemaName:  schemaName,
+							},
+						},
+					},
+				},
+			},
+		}
+
+		// set location of volume definition in config.
+		b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) {
+			return dyn.Map(v, "resources.volumes.foo", func(p dyn.Path, volume dyn.Value) (dyn.Value, error) {
+				return volume.WithLocations([]dyn.Location{
+					{
+						File:   filepath.Join(dir, "databricks.yml"),
+						Line:   1,
+						Column: 2,
+					},
+				}), nil
+			})
+		})
+
+		diags := bundle.Apply(ctx, b, bundle.Seq(libraries.ExpandGlobReferences(), libraries.Upload()))
+		require.EqualError(
+			t,
+			diags.Error(),
+			fmt.Sprintf(`the bundle is configured to upload artifacts to %s but a
+UC volume at %s does not exist. Note: We detected that you have a UC volume
+defined that matched the path above at %s. Please deploy the UC volume
+in a separate deployment before using it in as a destination to upload
+artifacts.`, path.Join(volumePath, ".internal"), volumePath, dyn.Location{
+				File:   filepath.Join(dir, "databricks.yml"),
+				Line:   1,
+				Column: 2,
+			}))
+	})
+}

From aeab4efda1d7dbadbb83243238cc7ef62167bb67 Mon Sep 17 00:00:00 2001
From: Shreyas Goenka <shreyas.goenka@databricks.com>
Date: Mon, 16 Sep 2024 00:57:38 +0200
Subject: [PATCH 14/39] unit test for comparision of locatoin

---
 bundle/libraries/upload.go      |  4 +--
 bundle/libraries/upload_test.go | 47 +++++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/bundle/libraries/upload.go b/bundle/libraries/upload.go
index 0e651189..cac9fa18 100644
--- a/bundle/libraries/upload.go
+++ b/bundle/libraries/upload.go
@@ -245,7 +245,7 @@ func GetFilerForLibraries(ctx context.Context, b *bundle.Bundle, uploadPath stri
 	// The volume does not exist. Check if the volume is defined in the bundle.
 	// TODO: Note that this is not a breaking change.
 	// TODO: Include error as well for why the stat call failed. It's more context.
-	l, ok := locationForVolume(b, catalogName, schemaName, volumeName)
+	l, ok := locationOfVolumeInBundle(b, catalogName, schemaName, volumeName)
 	if !ok {
 		return nil, diag.Errorf("the bundle is configured to upload artifacts to %s but a UC volume at %s does not exist", uploadPath, volumePath)
 	}
@@ -257,7 +257,7 @@ in a separate deployment before using it in as a destination to upload
 artifacts.`, uploadPath, volumePath, l)
 }
 
-func locationForVolume(b *bundle.Bundle, catalogName, schemaName, volumeName string) (dyn.Location, bool) {
+func locationOfVolumeInBundle(b *bundle.Bundle, catalogName, schemaName, volumeName string) (dyn.Location, bool) {
 	volumes := b.Config.Resources.Volumes
 	for k, v := range volumes {
 		if v.CatalogName != catalogName || v.Name != volumeName {
diff --git a/bundle/libraries/upload_test.go b/bundle/libraries/upload_test.go
index 44b194c5..3eec4692 100644
--- a/bundle/libraries/upload_test.go
+++ b/bundle/libraries/upload_test.go
@@ -8,11 +8,15 @@ import (
 	"github.com/databricks/cli/bundle"
 	"github.com/databricks/cli/bundle/config"
 	"github.com/databricks/cli/bundle/config/resources"
+	"github.com/databricks/cli/bundle/internal/bundletest"
 	mockfiler "github.com/databricks/cli/internal/mocks/libs/filer"
 	"github.com/databricks/cli/internal/testutil"
+	"github.com/databricks/cli/libs/dyn"
 	"github.com/databricks/cli/libs/filer"
+	"github.com/databricks/databricks-sdk-go/service/catalog"
 	"github.com/databricks/databricks-sdk-go/service/compute"
 	"github.com/databricks/databricks-sdk-go/service/jobs"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 )
@@ -329,3 +333,46 @@ func TestUploadMultipleLibraries(t *testing.T) {
 	require.Contains(t, b.Config.Resources.Jobs["job"].JobSettings.Environments[0].Spec.Dependencies, "/Workspace/foo/bar/artifacts/.internal/source4.whl")
 	require.Contains(t, b.Config.Resources.Jobs["job"].JobSettings.Environments[0].Spec.Dependencies, "/Workspace/Users/foo@bar.com/mywheel.whl")
 }
+
+func TestLocationOfVolumeInBundle(t *testing.T) {
+	b := &bundle.Bundle{
+		Config: config.Root{
+			Resources: config.Resources{
+				Volumes: map[string]*resources.Volume{
+					"foo": {
+						CreateVolumeRequestContent: &catalog.CreateVolumeRequestContent{
+							CatalogName: "main",
+							Name:        "my_volume",
+							SchemaName:  "my_schema",
+						},
+					},
+				},
+			},
+		},
+	}
+
+	bundletest.SetLocation(b, "resources.volumes.foo", "volume.yml")
+
+	// volume is in DAB directly.
+	l, ok := locationOfVolumeInBundle(b, "main", "my_schema", "my_volume")
+	assert.True(t, ok)
+	assert.Equal(t, dyn.Location{
+		File: "volume.yml",
+	}, l)
+
+	// wrong volume name
+	_, ok = locationOfVolumeInBundle(b, "main", "my_schema", "doesnotexist")
+	assert.False(t, ok)
+
+	// wrong schema name
+	_, ok = locationOfVolumeInBundle(b, "main", "doesnotexist", "my_volume")
+	assert.False(t, ok)
+
+	// schema name is interpolated.
+	b.Config.Resources.Volumes["foo"].SchemaName = "${resources.schemas.my_schema}"
+	l, ok = locationOfVolumeInBundle(b, "main", "valuedoesnotmatter", "my_volume")
+	assert.True(t, ok)
+	assert.Equal(t, dyn.Location{
+		File: "volume.yml",
+	}, l)
+}

From aa2e16d757c4aa24a5892b587b4249b7d93fd094 Mon Sep 17 00:00:00 2001
From: Shreyas Goenka <shreyas.goenka@databricks.com>
Date: Mon, 16 Sep 2024 02:28:58 +0200
Subject: [PATCH 15/39] cleanup and add test

---
 bundle/artifacts/upload.go                    |   9 +-
 bundle/libraries/upload.go                    | 110 ++++++------
 bundle/libraries/upload_test.go               | 157 ++++++++++++++++--
 libs/filer/workspace_files_client.go          |  16 +-
 .../workspace_files_extensions_client_test.go |   2 +-
 5 files changed, 213 insertions(+), 81 deletions(-)

diff --git a/bundle/artifacts/upload.go b/bundle/artifacts/upload.go
index d7b3e1ba..c69939e8 100644
--- a/bundle/artifacts/upload.go
+++ b/bundle/artifacts/upload.go
@@ -21,18 +21,13 @@ func (m *cleanUp) Name() string {
 }
 
 func (m *cleanUp) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
-	uploadPath, err := libraries.GetUploadBasePath(b)
-	if err != nil {
-		return diag.FromErr(err)
-	}
-
-	client, diags := libraries.GetFilerForLibraries(ctx, b, uploadPath)
+	client, uploadPath, diags := libraries.GetFilerForLibraries(ctx, b)
 	if diags.HasError() {
 		return diags
 	}
 
 	// We intentionally ignore the error because it is not critical to the deployment
-	err = client.Delete(ctx, ".", filer.DeleteRecursively)
+	err := client.Delete(ctx, ".", filer.DeleteRecursively)
 	if err != nil {
 		log.Errorf(ctx, "failed to delete %s: %v", uploadPath, err)
 	}
diff --git a/bundle/libraries/upload.go b/bundle/libraries/upload.go
index cac9fa18..163b5613 100644
--- a/bundle/libraries/upload.go
+++ b/bundle/libraries/upload.go
@@ -129,24 +129,17 @@ func collectLocalLibraries(b *bundle.Bundle) (map[string][]configLocation, error
 }
 
 func (u *upload) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
-	uploadPath, err := GetUploadBasePath(b)
-	if err != nil {
-		return diag.FromErr(err)
-	}
-
 	// If the client is not initialized, initialize it
 	// We use client field in mutator to allow for mocking client in testing
+	var uploadPath string
+	var diags diag.Diagnostics
 	if u.client == nil {
-		filer, diags := GetFilerForLibraries(ctx, b, uploadPath)
+		u.client, uploadPath, diags = GetFilerForLibraries(ctx, b)
 		if diags.HasError() {
 			return diags
 		}
-
-		u.client = filer
 	}
 
-	var diags diag.Diagnostics
-
 	libs, err := collectLocalLibraries(b)
 	if err != nil {
 		return diag.FromErr(err)
@@ -196,30 +189,45 @@ func (u *upload) Name() string {
 	return "libraries.Upload"
 }
 
-// TODO: TODO: Nicer comments here.
-// Case 1: UC volume path is valid. Return the client.
-// Case 2: invalid path.
-// (a) Not enough elements.
-// (b) catalog and schema correspond to a volume define in the DAB.
+// This function returns the right filer to use, to upload artifacts to the configured locations.
+// Supported locations:
+// 1. WSFS
+// 2. UC volumes
 //
-//	-> exception for when the schema value is fully or partially interpolated.
-//	   In that case only check the catalog name.
-//
-// TODO: Convert to warning? We don't error today if you specify an invalid volume path.
-func GetFilerForLibraries(ctx context.Context, b *bundle.Bundle, uploadPath string) (filer.Filer, diag.Diagnostics) {
+// If a UC Volume is configured, this function checks if the UC volume exists in the workspace.
+// Then:
+//  1. If the UC volume existing in the workspace:
+//     Returns a filer for the UC volume.
+//  2. If the UC volume does not exist in the workspace but is very likely to be defined in
+//     the bundle configuration:
+//     Returns a warning along with the error that instructs the user to deploy the
+//     UC volume before using it in the artifact path.
+//  3. If the UC volume does not exist in the workspace and is not defined in the bundle configuration:
+//     Returns an error.
+func GetFilerForLibraries(ctx context.Context, b *bundle.Bundle) (filer.Filer, string, diag.Diagnostics) {
+	artifactPath := b.Config.Workspace.ArtifactPath
+	if artifactPath == "" {
+		return nil, "", diag.Errorf("remote artifact path not configured")
+	}
+
+	// path to upload artifact files to.
+	uploadPath := path.Join(artifactPath, ".internal")
+
 	w := b.WorkspaceClient()
 	isVolumesPath := strings.HasPrefix(uploadPath, "/Volumes/")
 
-	// If the path is not a volume path, use the workspace file system.
+	// Return early with a WSFS filer if the artifact path is not a UC volume path.
 	if !isVolumesPath {
 		f, err := filer.NewWorkspaceFilesClient(w, uploadPath)
-		return f, diag.FromErr(err)
+		return f, uploadPath, diag.FromErr(err)
 	}
 
-	parts := strings.Split(uploadPath, "/")
+	parts := strings.Split(artifactPath, "/")
 	volumeFormatErr := fmt.Errorf("expected UC volume path to be in the format /Volumes/<catalog>/<schema>/<path>, got %s", uploadPath)
-	if len(strings.Split(uploadPath, "/")) < 5 {
-		return nil, diag.FromErr(volumeFormatErr)
+
+	// Incorrect format.
+	if len(parts) < 5 {
+		return nil, "", diag.FromErr(volumeFormatErr)
 	}
 
 	catalogName := parts[2]
@@ -228,36 +236,36 @@ func GetFilerForLibraries(ctx context.Context, b *bundle.Bundle, uploadPath stri
 
 	// Incorrect format.
 	if catalogName == "" || schemaName == "" || volumeName == "" {
-		return nil, diag.FromErr(volumeFormatErr)
+		return nil, "", diag.FromErr(volumeFormatErr)
 	}
 
+	// Check if the UC volume exists in the workspace.
+	volumePath := fmt.Sprintf("/Volumes/%s/%s/%s", catalogName, schemaName, volumeName)
+	err := w.Files.GetDirectoryMetadataByDirectoryPath(ctx, volumePath)
+
 	// If the volume exists already, directly return the filer for the upload path.
-	volumePath := fmt.Sprintf("/Volumes/%s/%s/%s", catalogName, schemaName, volumeName)
-	vf, err := filer.NewFilesClient(w, volumePath)
-	if err != nil {
-		return nil, diag.FromErr(err)
-	}
-	if _, err := vf.Stat(ctx, "."); err == nil {
+	if err == nil {
 		f, err := filer.NewFilesClient(w, uploadPath)
-		return f, diag.FromErr(err)
+		return f, uploadPath, diag.FromErr(err)
 	}
 
-	// The volume does not exist. Check if the volume is defined in the bundle.
-	// TODO: Note that this is not a breaking change.
-	// TODO: Include error as well for why the stat call failed. It's more context.
-	l, ok := locationOfVolumeInBundle(b, catalogName, schemaName, volumeName)
+	diags := diag.Errorf("failed to fetch metadata for the UC volume %s that is configured in the artifact_path: %s", volumePath, err)
+
+	path, locations, ok := matchVolumeInBundle(b, catalogName, schemaName, volumeName)
 	if !ok {
-		return nil, diag.Errorf("the bundle is configured to upload artifacts to %s but a UC volume at %s does not exist", uploadPath, volumePath)
+		return nil, "", diags
 	}
 
-	return nil, diag.Errorf(`the bundle is configured to upload artifacts to %s but a
-UC volume at %s does not exist. Note: We detected that you have a UC volume
-defined that matched the path above at %s. Please deploy the UC volume
-in a separate deployment before using it in as a destination to upload
-artifacts.`, uploadPath, volumePath, l)
+	warning := diag.Diagnostic{
+		Severity:  diag.Warning,
+		Summary:   `the UC volume that is likely being used in the artifact_path has not been deployed yet. Please deploy the UC volume in a separate bundle deploy before using it in the artifact_path.`,
+		Locations: locations,
+		Paths:     []dyn.Path{path},
+	}
+	return nil, "", diags.Append(warning)
 }
 
-func locationOfVolumeInBundle(b *bundle.Bundle, catalogName, schemaName, volumeName string) (dyn.Location, bool) {
+func matchVolumeInBundle(b *bundle.Bundle, catalogName, schemaName, volumeName string) (dyn.Path, []dyn.Location, bool) {
 	volumes := b.Config.Resources.Volumes
 	for k, v := range volumes {
 		if v.CatalogName != catalogName || v.Name != volumeName {
@@ -270,9 +278,10 @@ func locationOfVolumeInBundle(b *bundle.Bundle, catalogName, schemaName, volumeN
 		if v.SchemaName != schemaName && !dynvar.ContainsVariableReference(v.SchemaName) {
 			continue
 		}
-		return b.Config.GetLocation(fmt.Sprintf("resources.volumes.%s", k)), true
+		pathString := fmt.Sprintf("resources.volumes.%s", k)
+		return dyn.MustPathFromString(pathString), b.Config.GetLocations(pathString), true
 	}
-	return dyn.Location{}, false
+	return nil, nil, false
 }
 
 // Function to upload file (a library, artifact and etc) to Workspace or UC volume
@@ -294,12 +303,3 @@ func UploadFile(ctx context.Context, file string, client filer.Filer) error {
 	log.Infof(ctx, "Upload succeeded")
 	return nil
 }
-
-func GetUploadBasePath(b *bundle.Bundle) (string, error) {
-	artifactPath := b.Config.Workspace.ArtifactPath
-	if artifactPath == "" {
-		return "", fmt.Errorf("remote artifact path not configured")
-	}
-
-	return path.Join(artifactPath, ".internal"), nil
-}
diff --git a/bundle/libraries/upload_test.go b/bundle/libraries/upload_test.go
index 3eec4692..f15e2b93 100644
--- a/bundle/libraries/upload_test.go
+++ b/bundle/libraries/upload_test.go
@@ -2,6 +2,7 @@ package libraries
 
 import (
 	"context"
+	"fmt"
 	"path/filepath"
 	"testing"
 
@@ -11,8 +12,11 @@ import (
 	"github.com/databricks/cli/bundle/internal/bundletest"
 	mockfiler "github.com/databricks/cli/internal/mocks/libs/filer"
 	"github.com/databricks/cli/internal/testutil"
+	"github.com/databricks/cli/libs/diag"
 	"github.com/databricks/cli/libs/dyn"
 	"github.com/databricks/cli/libs/filer"
+	sdkconfig "github.com/databricks/databricks-sdk-go/config"
+	"github.com/databricks/databricks-sdk-go/experimental/mocks"
 	"github.com/databricks/databricks-sdk-go/service/catalog"
 	"github.com/databricks/databricks-sdk-go/service/compute"
 	"github.com/databricks/databricks-sdk-go/service/jobs"
@@ -334,7 +338,7 @@ func TestUploadMultipleLibraries(t *testing.T) {
 	require.Contains(t, b.Config.Resources.Jobs["job"].JobSettings.Environments[0].Spec.Dependencies, "/Workspace/Users/foo@bar.com/mywheel.whl")
 }
 
-func TestLocationOfVolumeInBundle(t *testing.T) {
+func TestMatchVolumeInBundle(t *testing.T) {
 	b := &bundle.Bundle{
 		Config: config.Root{
 			Resources: config.Resources{
@@ -353,26 +357,159 @@ func TestLocationOfVolumeInBundle(t *testing.T) {
 
 	bundletest.SetLocation(b, "resources.volumes.foo", "volume.yml")
 
-	// volume is in DAB directly.
-	l, ok := locationOfVolumeInBundle(b, "main", "my_schema", "my_volume")
+	// volume is in DAB.
+	path, locations, ok := matchVolumeInBundle(b, "main", "my_schema", "my_volume")
 	assert.True(t, ok)
-	assert.Equal(t, dyn.Location{
+	assert.Equal(t, []dyn.Location{{
 		File: "volume.yml",
-	}, l)
+	}}, locations)
+	assert.Equal(t, dyn.MustPathFromString("resources.volumes.foo"), path)
 
 	// wrong volume name
-	_, ok = locationOfVolumeInBundle(b, "main", "my_schema", "doesnotexist")
+	_, _, ok = matchVolumeInBundle(b, "main", "my_schema", "doesnotexist")
 	assert.False(t, ok)
 
 	// wrong schema name
-	_, ok = locationOfVolumeInBundle(b, "main", "doesnotexist", "my_volume")
+	_, _, ok = matchVolumeInBundle(b, "main", "doesnotexist", "my_volume")
 	assert.False(t, ok)
 
 	// schema name is interpolated.
 	b.Config.Resources.Volumes["foo"].SchemaName = "${resources.schemas.my_schema}"
-	l, ok = locationOfVolumeInBundle(b, "main", "valuedoesnotmatter", "my_volume")
+	path, locations, ok = matchVolumeInBundle(b, "main", "valuedoesnotmatter", "my_volume")
 	assert.True(t, ok)
-	assert.Equal(t, dyn.Location{
+	assert.Equal(t, []dyn.Location{{
 		File: "volume.yml",
-	}, l)
+	}}, locations)
+	assert.Equal(t, dyn.MustPathFromString("resources.volumes.foo"), path)
+}
+
+func TestGetFilerForLibraries(t *testing.T) {
+	t.Run("valid wsfs", func(t *testing.T) {
+		b := &bundle.Bundle{
+			Config: config.Root{
+				Workspace: config.Workspace{
+					ArtifactPath: "/foo/bar/artifacts",
+				},
+			},
+		}
+
+		client, uploadPath, diags := GetFilerForLibraries(context.Background(), b)
+		require.NoError(t, diags.Error())
+		assert.Equal(t, "/foo/bar/artifacts/.internal", uploadPath)
+
+		assert.IsType(t, &filer.WorkspaceFilesClient{}, client)
+	})
+
+	t.Run("valid uc volume", func(t *testing.T) {
+		b := &bundle.Bundle{
+			Config: config.Root{
+				Workspace: config.Workspace{
+					ArtifactPath: "/Volumes/main/my_schema/my_volume",
+				},
+			},
+		}
+
+		m := mocks.NewMockWorkspaceClient(t)
+		m.WorkspaceClient.Config = &sdkconfig.Config{}
+		m.GetMockFilesAPI().EXPECT().GetDirectoryMetadataByDirectoryPath(mock.Anything, "/Volumes/main/my_schema/my_volume").Return(nil)
+		b.SetWorkpaceClient(m.WorkspaceClient)
+
+		client, uploadPath, diags := GetFilerForLibraries(context.Background(), b)
+		require.NoError(t, diags.Error())
+		assert.Equal(t, "/Volumes/main/my_schema/my_volume/.internal", uploadPath)
+
+		assert.IsType(t, &filer.FilesClient{}, client)
+	})
+
+	t.Run("volume not in DAB", func(t *testing.T) {
+		b := &bundle.Bundle{
+			Config: config.Root{
+				Workspace: config.Workspace{
+					ArtifactPath: "/Volumes/main/my_schema/doesnotexist",
+				},
+			},
+		}
+
+		m := mocks.NewMockWorkspaceClient(t)
+		m.WorkspaceClient.Config = &sdkconfig.Config{}
+		m.GetMockFilesAPI().EXPECT().GetDirectoryMetadataByDirectoryPath(mock.Anything, "/Volumes/main/my_schema/doesnotexist").Return(fmt.Errorf("error from API"))
+		b.SetWorkpaceClient(m.WorkspaceClient)
+
+		_, _, diags := GetFilerForLibraries(context.Background(), b)
+		require.EqualError(t, diags.Error(), "failed to fetch metadata for the UC volume /Volumes/main/my_schema/doesnotexist that is configured in the artifact_path: error from API")
+	})
+
+	t.Run("volume in DAB config", func(t *testing.T) {
+		b := &bundle.Bundle{
+			Config: config.Root{
+				Workspace: config.Workspace{
+					ArtifactPath: "/Volumes/main/my_schema/my_volume",
+				},
+				Resources: config.Resources{
+					Volumes: map[string]*resources.Volume{
+						"foo": {
+							CreateVolumeRequestContent: &catalog.CreateVolumeRequestContent{
+								CatalogName: "main",
+								Name:        "my_volume",
+								VolumeType:  "MANAGED",
+								SchemaName:  "my_schema",
+							},
+						},
+					},
+				},
+			},
+		}
+
+		bundletest.SetLocation(b, "resources.volumes.foo", "volume.yml")
+
+		m := mocks.NewMockWorkspaceClient(t)
+		m.WorkspaceClient.Config = &sdkconfig.Config{}
+		m.GetMockFilesAPI().EXPECT().GetDirectoryMetadataByDirectoryPath(mock.Anything, "/Volumes/main/my_schema/my_volume").Return(fmt.Errorf("error from API"))
+		b.SetWorkpaceClient(m.WorkspaceClient)
+
+		_, _, diags := GetFilerForLibraries(context.Background(), b)
+		assert.EqualError(t, diags.Error(), "failed to fetch metadata for the UC volume /Volumes/main/my_schema/my_volume that is configured in the artifact_path: error from API")
+		assert.Contains(t, diags, diag.Diagnostic{
+			Severity: diag.Warning,
+			Summary:  "the UC volume that is likely being used in the artifact_path has not been deployed yet. Please deploy the UC volume in a separate bundle deploy before using it in the artifact_path.",
+			Locations: []dyn.Location{{
+				File: "volume.yml",
+			}},
+			Paths: []dyn.Path{dyn.MustPathFromString("resources.volumes.foo")},
+		})
+	})
+
+	t.Run("remote path is not set", func(t *testing.T) {
+		b := &bundle.Bundle{}
+
+		_, _, diags := GetFilerForLibraries(context.Background(), b)
+		require.EqualError(t, diags.Error(), "remote artifact path not configured")
+	})
+
+	t.Run("invalid volume paths", func(t *testing.T) {
+		invalidPaths := []string{
+			"/Volumes",
+			"/Volumes/main",
+			"/Volumes/main/",
+			"/Volumes/main//",
+			"/Volumes/main//my_schema",
+			"/Volumes/main/my_schema",
+			"/Volumes/main/my_schema/",
+			"/Volumes/main/my_schema//",
+			"/Volumes//my_schema/my_volume",
+		}
+
+		for _, path := range invalidPaths {
+			b := &bundle.Bundle{
+				Config: config.Root{
+					Workspace: config.Workspace{
+						ArtifactPath: path,
+					},
+				},
+			}
+
+			_, _, diags := GetFilerForLibraries(context.Background(), b)
+			require.EqualError(t, diags.Error(), fmt.Sprintf("expected UC volume path to be in the format /Volumes/<catalog>/<schema>/<path>, got %s", filepath.Join(path, ".internal")))
+		}
+	})
 }
diff --git a/libs/filer/workspace_files_client.go b/libs/filer/workspace_files_client.go
index d8ab5a6b..5e2a9813 100644
--- a/libs/filer/workspace_files_client.go
+++ b/libs/filer/workspace_files_client.go
@@ -114,7 +114,7 @@ type apiClient interface {
 
 // NOTE: This API is available for files under /Repos if a workspace has files-in-repos enabled.
 // It can access any workspace path if files-in-workspace is enabled.
-type workspaceFilesClient struct {
+type WorkspaceFilesClient struct {
 	workspaceClient *databricks.WorkspaceClient
 	apiClient       apiClient
 
@@ -128,7 +128,7 @@ func NewWorkspaceFilesClient(w *databricks.WorkspaceClient, root string) (Filer,
 		return nil, err
 	}
 
-	return &workspaceFilesClient{
+	return &WorkspaceFilesClient{
 		workspaceClient: w,
 		apiClient:       apiClient,
 
@@ -136,7 +136,7 @@ func NewWorkspaceFilesClient(w *databricks.WorkspaceClient, root string) (Filer,
 	}, nil
 }
 
-func (w *workspaceFilesClient) Write(ctx context.Context, name string, reader io.Reader, mode ...WriteMode) error {
+func (w *WorkspaceFilesClient) Write(ctx context.Context, name string, reader io.Reader, mode ...WriteMode) error {
 	absPath, err := w.root.Join(name)
 	if err != nil {
 		return err
@@ -206,7 +206,7 @@ func (w *workspaceFilesClient) Write(ctx context.Context, name string, reader io
 	return err
 }
 
-func (w *workspaceFilesClient) Read(ctx context.Context, name string) (io.ReadCloser, error) {
+func (w *WorkspaceFilesClient) Read(ctx context.Context, name string) (io.ReadCloser, error) {
 	absPath, err := w.root.Join(name)
 	if err != nil {
 		return nil, err
@@ -230,7 +230,7 @@ func (w *workspaceFilesClient) Read(ctx context.Context, name string) (io.ReadCl
 	return w.workspaceClient.Workspace.Download(ctx, absPath)
 }
 
-func (w *workspaceFilesClient) Delete(ctx context.Context, name string, mode ...DeleteMode) error {
+func (w *WorkspaceFilesClient) Delete(ctx context.Context, name string, mode ...DeleteMode) error {
 	absPath, err := w.root.Join(name)
 	if err != nil {
 		return err
@@ -274,7 +274,7 @@ func (w *workspaceFilesClient) Delete(ctx context.Context, name string, mode ...
 	return err
 }
 
-func (w *workspaceFilesClient) ReadDir(ctx context.Context, name string) ([]fs.DirEntry, error) {
+func (w *WorkspaceFilesClient) ReadDir(ctx context.Context, name string) ([]fs.DirEntry, error) {
 	absPath, err := w.root.Join(name)
 	if err != nil {
 		return nil, err
@@ -307,7 +307,7 @@ func (w *workspaceFilesClient) ReadDir(ctx context.Context, name string) ([]fs.D
 	return wsfsDirEntriesFromObjectInfos(objects), nil
 }
 
-func (w *workspaceFilesClient) Mkdir(ctx context.Context, name string) error {
+func (w *WorkspaceFilesClient) Mkdir(ctx context.Context, name string) error {
 	dirPath, err := w.root.Join(name)
 	if err != nil {
 		return err
@@ -317,7 +317,7 @@ func (w *workspaceFilesClient) Mkdir(ctx context.Context, name string) error {
 	})
 }
 
-func (w *workspaceFilesClient) Stat(ctx context.Context, name string) (fs.FileInfo, error) {
+func (w *WorkspaceFilesClient) Stat(ctx context.Context, name string) (fs.FileInfo, error) {
 	absPath, err := w.root.Join(name)
 	if err != nil {
 		return nil, err
diff --git a/libs/filer/workspace_files_extensions_client_test.go b/libs/filer/workspace_files_extensions_client_test.go
index 321c4371..33c55888 100644
--- a/libs/filer/workspace_files_extensions_client_test.go
+++ b/libs/filer/workspace_files_extensions_client_test.go
@@ -123,7 +123,7 @@ func TestFilerWorkspaceFilesExtensionsErrorsOnDupName(t *testing.T) {
 				"return_export_info": "true",
 			}, mock.AnythingOfType("*filer.wsfsFileInfo"), []func(*http.Request) error(nil)).Return(nil, statNotebook)
 
-			workspaceFilesClient := workspaceFilesClient{
+			workspaceFilesClient := WorkspaceFilesClient{
 				workspaceClient: mockedWorkspaceClient.WorkspaceClient,
 				apiClient:       &mockedApiClient,
 				root:            NewWorkspaceRootPath("/dir"),

From a90eb57a5beff3d165cd088a24605f55a13c168c Mon Sep 17 00:00:00 2001
From: Shreyas Goenka <shreyas.goenka@databricks.com>
Date: Mon, 16 Sep 2024 03:00:22 +0200
Subject: [PATCH 16/39] fix unit tests

---
 bundle/libraries/upload.go      | 15 +++++++++------
 bundle/libraries/upload_test.go |  5 +++++
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/bundle/libraries/upload.go b/bundle/libraries/upload.go
index 163b5613..f8d331eb 100644
--- a/bundle/libraries/upload.go
+++ b/bundle/libraries/upload.go
@@ -131,13 +131,15 @@ func collectLocalLibraries(b *bundle.Bundle) (map[string][]configLocation, error
 func (u *upload) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
 	// If the client is not initialized, initialize it
 	// We use client field in mutator to allow for mocking client in testing
-	var uploadPath string
-	var diags diag.Diagnostics
+	client, uploadPath, diags := GetFilerForLibraries(ctx, b)
+	if diags.HasError() {
+		return diags
+	}
+
+	// Only set the filer client if it's not already set. This allows for using
+	// a mock client in tests.
 	if u.client == nil {
-		u.client, uploadPath, diags = GetFilerForLibraries(ctx, b)
-		if diags.HasError() {
-			return diags
-		}
+		u.client = client
 	}
 
 	libs, err := collectLocalLibraries(b)
@@ -189,6 +191,7 @@ func (u *upload) Name() string {
 	return "libraries.Upload"
 }
 
+// TODO: As a followup add testing for interactive flows.
 // This function returns the right filer to use, to upload artifacts to the configured locations.
 // Supported locations:
 // 1. WSFS
diff --git a/bundle/libraries/upload_test.go b/bundle/libraries/upload_test.go
index f15e2b93..f7b8a2ff 100644
--- a/bundle/libraries/upload_test.go
+++ b/bundle/libraries/upload_test.go
@@ -189,6 +189,11 @@ func TestArtifactUploadForVolumes(t *testing.T) {
 		filer.CreateParentDirectories,
 	).Return(nil)
 
+	m := mocks.NewMockWorkspaceClient(t)
+	m.WorkspaceClient.Config = &sdkconfig.Config{}
+	m.GetMockFilesAPI().EXPECT().GetDirectoryMetadataByDirectoryPath(mock.Anything, "/Volumes/foo/bar/artifacts").Return(nil)
+	b.SetWorkpaceClient(m.WorkspaceClient)
+
 	diags := bundle.Apply(context.Background(), b, bundle.Seq(ExpandGlobReferences(), UploadWithClient(mockFiler)))
 	require.NoError(t, diags.Error())
 

From 39cb5e84719004baa1f760c745824a8941cdfaa2 Mon Sep 17 00:00:00 2001
From: Shreyas Goenka <shreyas.goenka@databricks.com>
Date: Mon, 16 Sep 2024 03:08:42 +0200
Subject: [PATCH 17/39] fix test on windows

---
 bundle/libraries/upload_test.go | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/bundle/libraries/upload_test.go b/bundle/libraries/upload_test.go
index f7b8a2ff..1951105c 100644
--- a/bundle/libraries/upload_test.go
+++ b/bundle/libraries/upload_test.go
@@ -3,6 +3,7 @@ package libraries
 import (
 	"context"
 	"fmt"
+	"path"
 	"path/filepath"
 	"testing"
 
@@ -504,17 +505,17 @@ func TestGetFilerForLibraries(t *testing.T) {
 			"/Volumes//my_schema/my_volume",
 		}
 
-		for _, path := range invalidPaths {
+		for _, p := range invalidPaths {
 			b := &bundle.Bundle{
 				Config: config.Root{
 					Workspace: config.Workspace{
-						ArtifactPath: path,
+						ArtifactPath: p,
 					},
 				},
 			}
 
 			_, _, diags := GetFilerForLibraries(context.Background(), b)
-			require.EqualError(t, diags.Error(), fmt.Sprintf("expected UC volume path to be in the format /Volumes/<catalog>/<schema>/<path>, got %s", filepath.Join(path, ".internal")))
+			require.EqualError(t, diags.Error(), fmt.Sprintf("expected UC volume path to be in the format /Volumes/<catalog>/<schema>/<path>, got %s", path.Join(p, ".internal")))
 		}
 	})
 }

From 13748f177d605a9698c93228549d6394cbd3cc09 Mon Sep 17 00:00:00 2001
From: Shreyas Goenka <shreyas.goenka@databricks.com>
Date: Mon, 16 Sep 2024 03:14:30 +0200
Subject: [PATCH 18/39] cleanup todos

---
 bundle/config/resources/volume.go               | 3 ---
 bundle/deploy/terraform/tfdyn/convert_volume.go | 4 ----
 2 files changed, 7 deletions(-)

diff --git a/bundle/config/resources/volume.go b/bundle/config/resources/volume.go
index 0ee4dce8..386d2ee3 100644
--- a/bundle/config/resources/volume.go
+++ b/bundle/config/resources/volume.go
@@ -9,13 +9,10 @@ type Volume struct {
 	// List of grants to apply on this schema.
 	Grants []Grant `json:"grants,omitempty"`
 
-	// TODO: Confirm the accuracy of this comment.
 	// Full name of the schema (catalog_name.schema_name.volume_name). This value is read from
 	// the terraform state after deployment succeeds.
 	ID string `json:"id,omitempty" bundle:"readonly"`
 
-	// TODO: Are there fields in the edit API or terraform that are not in this struct?
-	// If so call it out in the PR.
 	*catalog.CreateVolumeRequestContent
 
 	ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"internal"`
diff --git a/bundle/deploy/terraform/tfdyn/convert_volume.go b/bundle/deploy/terraform/tfdyn/convert_volume.go
index 281cd432..4211e1f9 100644
--- a/bundle/deploy/terraform/tfdyn/convert_volume.go
+++ b/bundle/deploy/terraform/tfdyn/convert_volume.go
@@ -10,10 +10,6 @@ import (
 	"github.com/databricks/cli/libs/log"
 )
 
-// TODO: Articulate the consequences of deleting a UC volume in the prompt message that
-// is displayed.
-// TODO: What sort of interpolation should be allowed at `artifact_path`? Should it be
-// ${volumes.foo.id} or ${volumes.foo.name} or something else?
 func convertVolumeResource(ctx context.Context, vin dyn.Value) (dyn.Value, error) {
 	// Normalize the output value to the target schema.
 	vout, diags := convert.Normalize(schema.ResourceVolume{}, vin)

From bdecd08206f817b73193316b8efbc5b4fc96ab40 Mon Sep 17 00:00:00 2001
From: Shreyas Goenka <shreyas.goenka@databricks.com>
Date: Mon, 16 Sep 2024 03:17:05 +0200
Subject: [PATCH 19/39] -

---
 bundle/libraries/upload.go | 1 -
 bundle/phases/deploy.go    | 2 --
 2 files changed, 3 deletions(-)

diff --git a/bundle/libraries/upload.go b/bundle/libraries/upload.go
index f8d331eb..7053a44f 100644
--- a/bundle/libraries/upload.go
+++ b/bundle/libraries/upload.go
@@ -191,7 +191,6 @@ func (u *upload) Name() string {
 	return "libraries.Upload"
 }
 
-// TODO: As a followup add testing for interactive flows.
 // This function returns the right filer to use, to upload artifacts to the configured locations.
 // Supported locations:
 // 1. WSFS
diff --git a/bundle/phases/deploy.go b/bundle/phases/deploy.go
index d71c6191..c32dda15 100644
--- a/bundle/phases/deploy.go
+++ b/bundle/phases/deploy.go
@@ -25,8 +25,6 @@ import (
 func filterDeleteOrRecreateActions(changes []*tfjson.ResourceChange, resourceType string) []terraformlib.Action {
 	res := make([]terraformlib.Action, 0)
 	for _, rc := range changes {
-		// TODO: Add end to end integration tests for the interactive prompt UXs.
-		// Good PR to introduce the first one, and make changes more confidently.
 		if rc.Type != resourceType {
 			continue
 		}

From 274fd636e0dc4528c6eb8c83cdad23732cf9c1c8 Mon Sep 17 00:00:00 2001
From: Shreyas Goenka <shreyas.goenka@databricks.com>
Date: Mon, 16 Sep 2024 03:50:40 +0200
Subject: [PATCH 20/39] iter

---
 bundle/libraries/upload.go        | 28 ++++++++++++-------------
 bundle/libraries/upload_test.go   | 21 +++++++++++--------
 internal/bundle/artifacts_test.go | 34 +++++++++++++++++++------------
 libs/dyn/dynvar/ref.go            |  4 ----
 4 files changed, 47 insertions(+), 40 deletions(-)

diff --git a/bundle/libraries/upload.go b/bundle/libraries/upload.go
index 7053a44f..07fbd56b 100644
--- a/bundle/libraries/upload.go
+++ b/bundle/libraries/upload.go
@@ -129,15 +129,13 @@ func collectLocalLibraries(b *bundle.Bundle) (map[string][]configLocation, error
 }
 
 func (u *upload) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
-	// If the client is not initialized, initialize it
-	// We use client field in mutator to allow for mocking client in testing
 	client, uploadPath, diags := GetFilerForLibraries(ctx, b)
 	if diags.HasError() {
 		return diags
 	}
 
-	// Only set the filer client if it's not already set. This allows for using
-	// a mock client in tests.
+	// Only set the filer client if it's not already set. We use client field
+	// in mutator to mock the filer client in testing
 	if u.client == nil {
 		u.client = client
 	}
@@ -191,7 +189,7 @@ func (u *upload) Name() string {
 	return "libraries.Upload"
 }
 
-// This function returns the right filer to use, to upload artifacts to the configured locations.
+// This function returns the right filer to use, to upload artifacts to the configured location.
 // Supported locations:
 // 1. WSFS
 // 2. UC volumes
@@ -200,7 +198,7 @@ func (u *upload) Name() string {
 // Then:
 //  1. If the UC volume existing in the workspace:
 //     Returns a filer for the UC volume.
-//  2. If the UC volume does not exist in the workspace but is very likely to be defined in
+//  2. If the UC volume does not exist in the workspace but is (with high confidence) defined in
 //     the bundle configuration:
 //     Returns a warning along with the error that instructs the user to deploy the
 //     UC volume before using it in the artifact path.
@@ -212,11 +210,11 @@ func GetFilerForLibraries(ctx context.Context, b *bundle.Bundle) (filer.Filer, s
 		return nil, "", diag.Errorf("remote artifact path not configured")
 	}
 
-	// path to upload artifact files to.
-	uploadPath := path.Join(artifactPath, ".internal")
-
 	w := b.WorkspaceClient()
-	isVolumesPath := strings.HasPrefix(uploadPath, "/Volumes/")
+	isVolumesPath := strings.HasPrefix(artifactPath, "/Volumes/")
+
+	// Path to upload artifact files to.
+	uploadPath := path.Join(artifactPath, ".internal")
 
 	// Return early with a WSFS filer if the artifact path is not a UC volume path.
 	if !isVolumesPath {
@@ -225,7 +223,7 @@ func GetFilerForLibraries(ctx context.Context, b *bundle.Bundle) (filer.Filer, s
 	}
 
 	parts := strings.Split(artifactPath, "/")
-	volumeFormatErr := fmt.Errorf("expected UC volume path to be in the format /Volumes/<catalog>/<schema>/<path>, got %s", uploadPath)
+	volumeFormatErr := fmt.Errorf("expected UC volume path to be in the format /Volumes/<catalog>/<schema>/<volume>/..., got %s", uploadPath)
 
 	// Incorrect format.
 	if len(parts) < 5 {
@@ -253,21 +251,21 @@ func GetFilerForLibraries(ctx context.Context, b *bundle.Bundle) (filer.Filer, s
 
 	diags := diag.Errorf("failed to fetch metadata for the UC volume %s that is configured in the artifact_path: %s", volumePath, err)
 
-	path, locations, ok := matchVolumeInBundle(b, catalogName, schemaName, volumeName)
+	path, locations, ok := findVolumeInBundle(b, catalogName, schemaName, volumeName)
 	if !ok {
 		return nil, "", diags
 	}
 
 	warning := diag.Diagnostic{
 		Severity:  diag.Warning,
-		Summary:   `the UC volume that is likely being used in the artifact_path has not been deployed yet. Please deploy the UC volume in a separate bundle deploy before using it in the artifact_path.`,
+		Summary:   `You might be using a UC volume in your artifact_path that is managed by this bundle but which has not been deployed yet. Please deploy the UC volume in a separate bundle deploy before using it in the artifact_path.`,
 		Locations: locations,
 		Paths:     []dyn.Path{path},
 	}
 	return nil, "", diags.Append(warning)
 }
 
-func matchVolumeInBundle(b *bundle.Bundle, catalogName, schemaName, volumeName string) (dyn.Path, []dyn.Location, bool) {
+func findVolumeInBundle(b *bundle.Bundle, catalogName, schemaName, volumeName string) (dyn.Path, []dyn.Location, bool) {
 	volumes := b.Config.Resources.Volumes
 	for k, v := range volumes {
 		if v.CatalogName != catalogName || v.Name != volumeName {
@@ -277,7 +275,7 @@ func matchVolumeInBundle(b *bundle.Bundle, catalogName, schemaName, volumeName s
 		// at runtime via the ${resources.schemas.<name>} syntax. Thus we match the volume
 		// definition if the schema name is the same as the one in the bundle, or if the
 		// schema name is interpolated.
-		if v.SchemaName != schemaName && !dynvar.ContainsVariableReference(v.SchemaName) {
+		if v.SchemaName != schemaName && !dynvar.IsPureVariableReference(v.SchemaName) {
 			continue
 		}
 		pathString := fmt.Sprintf("resources.volumes.%s", k)
diff --git a/bundle/libraries/upload_test.go b/bundle/libraries/upload_test.go
index 1951105c..6ee72d4e 100644
--- a/bundle/libraries/upload_test.go
+++ b/bundle/libraries/upload_test.go
@@ -344,7 +344,7 @@ func TestUploadMultipleLibraries(t *testing.T) {
 	require.Contains(t, b.Config.Resources.Jobs["job"].JobSettings.Environments[0].Spec.Dependencies, "/Workspace/Users/foo@bar.com/mywheel.whl")
 }
 
-func TestMatchVolumeInBundle(t *testing.T) {
+func TestFindVolumeInBundle(t *testing.T) {
 	b := &bundle.Bundle{
 		Config: config.Root{
 			Resources: config.Resources{
@@ -364,7 +364,7 @@ func TestMatchVolumeInBundle(t *testing.T) {
 	bundletest.SetLocation(b, "resources.volumes.foo", "volume.yml")
 
 	// volume is in DAB.
-	path, locations, ok := matchVolumeInBundle(b, "main", "my_schema", "my_volume")
+	path, locations, ok := findVolumeInBundle(b, "main", "my_schema", "my_volume")
 	assert.True(t, ok)
 	assert.Equal(t, []dyn.Location{{
 		File: "volume.yml",
@@ -372,16 +372,20 @@ func TestMatchVolumeInBundle(t *testing.T) {
 	assert.Equal(t, dyn.MustPathFromString("resources.volumes.foo"), path)
 
 	// wrong volume name
-	_, _, ok = matchVolumeInBundle(b, "main", "my_schema", "doesnotexist")
+	_, _, ok = findVolumeInBundle(b, "main", "my_schema", "doesnotexist")
 	assert.False(t, ok)
 
 	// wrong schema name
-	_, _, ok = matchVolumeInBundle(b, "main", "doesnotexist", "my_volume")
+	_, _, ok = findVolumeInBundle(b, "main", "doesnotexist", "my_volume")
+	assert.False(t, ok)
+
+	// wrong catalog name
+	_, _, ok = findVolumeInBundle(b, "doesnotexist", "my_schema", "my_volume")
 	assert.False(t, ok)
 
 	// schema name is interpolated.
 	b.Config.Resources.Volumes["foo"].SchemaName = "${resources.schemas.my_schema}"
-	path, locations, ok = matchVolumeInBundle(b, "main", "valuedoesnotmatter", "my_volume")
+	path, locations, ok = findVolumeInBundle(b, "main", "valuedoesnotmatter", "my_volume")
 	assert.True(t, ok)
 	assert.Equal(t, []dyn.Location{{
 		File: "volume.yml",
@@ -442,7 +446,8 @@ func TestGetFilerForLibraries(t *testing.T) {
 		b.SetWorkpaceClient(m.WorkspaceClient)
 
 		_, _, diags := GetFilerForLibraries(context.Background(), b)
-		require.EqualError(t, diags.Error(), "failed to fetch metadata for the UC volume /Volumes/main/my_schema/doesnotexist that is configured in the artifact_path: error from API")
+		assert.EqualError(t, diags.Error(), "failed to fetch metadata for the UC volume /Volumes/main/my_schema/doesnotexist that is configured in the artifact_path: error from API")
+		assert.Len(t, diags, 1)
 	})
 
 	t.Run("volume in DAB config", func(t *testing.T) {
@@ -477,7 +482,7 @@ func TestGetFilerForLibraries(t *testing.T) {
 		assert.EqualError(t, diags.Error(), "failed to fetch metadata for the UC volume /Volumes/main/my_schema/my_volume that is configured in the artifact_path: error from API")
 		assert.Contains(t, diags, diag.Diagnostic{
 			Severity: diag.Warning,
-			Summary:  "the UC volume that is likely being used in the artifact_path has not been deployed yet. Please deploy the UC volume in a separate bundle deploy before using it in the artifact_path.",
+			Summary:  "You might be using a UC volume in your artifact_path that is managed by this bundle but which has not been deployed yet. Please deploy the UC volume in a separate bundle deploy before using it in the artifact_path.",
 			Locations: []dyn.Location{{
 				File: "volume.yml",
 			}},
@@ -515,7 +520,7 @@ func TestGetFilerForLibraries(t *testing.T) {
 			}
 
 			_, _, diags := GetFilerForLibraries(context.Background(), b)
-			require.EqualError(t, diags.Error(), fmt.Sprintf("expected UC volume path to be in the format /Volumes/<catalog>/<schema>/<path>, got %s", path.Join(p, ".internal")))
+			require.EqualError(t, diags.Error(), fmt.Sprintf("expected UC volume path to be in the format /Volumes/<catalog>/<schema>/<volume>/..., got %s", path.Join(p, ".internal")))
 		}
 	})
 }
diff --git a/internal/bundle/artifacts_test.go b/internal/bundle/artifacts_test.go
index 213aed20..5c7d0940 100644
--- a/internal/bundle/artifacts_test.go
+++ b/internal/bundle/artifacts_test.go
@@ -14,10 +14,12 @@ import (
 	"github.com/databricks/cli/bundle/libraries"
 	"github.com/databricks/cli/internal"
 	"github.com/databricks/cli/internal/acc"
+	"github.com/databricks/cli/libs/diag"
 	"github.com/databricks/cli/libs/dyn"
 	"github.com/databricks/databricks-sdk-go/service/catalog"
 	"github.com/databricks/databricks-sdk-go/service/compute"
 	"github.com/databricks/databricks-sdk-go/service/jobs"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
@@ -272,7 +274,7 @@ func TestAccUploadArtifactFileToInvalidVolume(t *testing.T) {
 		}
 
 		diags := bundle.Apply(ctx, b, bundle.Seq(libraries.ExpandGlobReferences(), libraries.Upload()))
-		require.EqualError(t, diags.Error(), fmt.Sprintf("the bundle is configured to upload artifacts to %s but a UC volume at %s does not exist", path.Join(volumePath, ".internal"), volumePath))
+		assert.ErrorContains(t, diags.Error(), fmt.Sprintf("failed to fetch metadata for the UC volume %s that is configured in the artifact_path:", volumePath))
 	})
 
 	t.Run("volume in DAB config", func(t *testing.T) {
@@ -318,17 +320,23 @@ func TestAccUploadArtifactFileToInvalidVolume(t *testing.T) {
 		})
 
 		diags := bundle.Apply(ctx, b, bundle.Seq(libraries.ExpandGlobReferences(), libraries.Upload()))
-		require.EqualError(
-			t,
-			diags.Error(),
-			fmt.Sprintf(`the bundle is configured to upload artifacts to %s but a
-UC volume at %s does not exist. Note: We detected that you have a UC volume
-defined that matched the path above at %s. Please deploy the UC volume
-in a separate deployment before using it in as a destination to upload
-artifacts.`, path.Join(volumePath, ".internal"), volumePath, dyn.Location{
-				File:   filepath.Join(dir, "databricks.yml"),
-				Line:   1,
-				Column: 2,
-			}))
+		assert.Contains(t, diags, diag.Diagnostic{
+			Severity: diag.Error,
+			Summary:  fmt.Sprintf("failed to fetch metadata for the UC volume %s that is configured in the artifact_path: Not Found", volumePath),
+		})
+		assert.Contains(t, diags, diag.Diagnostic{
+			Severity: diag.Warning,
+			Summary:  "You might be using a UC volume in your artifact_path that is managed by this bundle but which has not been deployed yet. Please deploy the UC volume in a separate bundle deploy before using it in the artifact_path.",
+			Locations: []dyn.Location{
+				{
+					File:   filepath.Join(dir, "databricks.yml"),
+					Line:   1,
+					Column: 2,
+				},
+			},
+			Paths: []dyn.Path{
+				dyn.MustPathFromString("resources.volumes.foo"),
+			},
+		})
 	})
 }
diff --git a/libs/dyn/dynvar/ref.go b/libs/dyn/dynvar/ref.go
index 088b68d2..338ac8ce 100644
--- a/libs/dyn/dynvar/ref.go
+++ b/libs/dyn/dynvar/ref.go
@@ -71,7 +71,3 @@ func (v ref) references() []string {
 func IsPureVariableReference(s string) bool {
 	return len(s) > 0 && re.FindString(s) == s
 }
-
-func ContainsVariableReference(s string) bool {
-	return len(s) > 0 && re.FindString(s) != ""
-}

From d3d5d4c0d6ffaf17102321d7da199d0db96226fb Mon Sep 17 00:00:00 2001
From: Shreyas Goenka <shreyas.goenka@databricks.com>
Date: Mon, 16 Sep 2024 03:54:15 +0200
Subject: [PATCH 21/39] -

---
 bundle/libraries/upload.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/bundle/libraries/upload.go b/bundle/libraries/upload.go
index 07fbd56b..36d64e43 100644
--- a/bundle/libraries/upload.go
+++ b/bundle/libraries/upload.go
@@ -196,11 +196,11 @@ func (u *upload) Name() string {
 //
 // If a UC Volume is configured, this function checks if the UC volume exists in the workspace.
 // Then:
-//  1. If the UC volume existing in the workspace:
+//  1. If the UC volume exists in the workspace:
 //     Returns a filer for the UC volume.
 //  2. If the UC volume does not exist in the workspace but is (with high confidence) defined in
 //     the bundle configuration:
-//     Returns a warning along with the error that instructs the user to deploy the
+//     Returns an error and a warning that instructs the user to deploy the
 //     UC volume before using it in the artifact path.
 //  3. If the UC volume does not exist in the workspace and is not defined in the bundle configuration:
 //     Returns an error.

From 227dfe95cab083c9af5b259b6269ad030061a287 Mon Sep 17 00:00:00 2001
From: Shreyas Goenka <shreyas.goenka@databricks.com>
Date: Mon, 16 Sep 2024 04:07:37 +0200
Subject: [PATCH 22/39] fixes

---
 bundle/libraries/upload_test.go | 2 +-
 internal/bundle/deploy_test.go  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/bundle/libraries/upload_test.go b/bundle/libraries/upload_test.go
index 6ee72d4e..a1a601e8 100644
--- a/bundle/libraries/upload_test.go
+++ b/bundle/libraries/upload_test.go
@@ -499,7 +499,7 @@ func TestGetFilerForLibraries(t *testing.T) {
 
 	t.Run("invalid volume paths", func(t *testing.T) {
 		invalidPaths := []string{
-			"/Volumes",
+			"/Volumes/",
 			"/Volumes/main",
 			"/Volumes/main/",
 			"/Volumes/main//",
diff --git a/internal/bundle/deploy_test.go b/internal/bundle/deploy_test.go
index 225458e1..849fd98b 100644
--- a/internal/bundle/deploy_test.go
+++ b/internal/bundle/deploy_test.go
@@ -282,7 +282,7 @@ volumes the upstream data in the cloud tenant is not affected:
   recreate volume foo`)
 	assert.Contains(t, stdout.String(), "the deployment requires destructive actions, but current console does not support prompting. Please specify --auto-approve if you would like to skip prompts and proceed")
 
-	// Recreation of the volume without --auto-approve should fail since prompting is not possible
+	// Successfully recreate the volume with --auto-approve
 	t.Setenv("TERM", "dumb")
 	t.Setenv("BUNDLE_ROOT", bundleRoot)
 	_, _, err = internal.NewCobraTestRunnerWithContext(t, ctx, "bundle", "deploy", "--var=schema_name=${resources.schemas.schema2.name}", "--auto-approve").Run()

From f919e94bce5d23a39c36fe79b9fc7d2e36a47bee Mon Sep 17 00:00:00 2001
From: Shreyas Goenka <shreyas.goenka@databricks.com>
Date: Mon, 14 Oct 2024 15:05:33 +0200
Subject: [PATCH 23/39] typo fix

---
 bundle/config/resources/volume.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/bundle/config/resources/volume.go b/bundle/config/resources/volume.go
index 386d2ee3..f7af05bd 100644
--- a/bundle/config/resources/volume.go
+++ b/bundle/config/resources/volume.go
@@ -6,10 +6,10 @@ import (
 )
 
 type Volume struct {
-	// List of grants to apply on this schema.
+	// List of grants to apply on this volume.
 	Grants []Grant `json:"grants,omitempty"`
 
-	// Full name of the schema (catalog_name.schema_name.volume_name). This value is read from
+	// Full name of the volume (catalog_name.schema_name.volume_name). This value is read from
 	// the terraform state after deployment succeeds.
 	ID string `json:"id,omitempty" bundle:"readonly"`
 

From 992126392888fe87ba3227a2e99a4868dc81eaa8 Mon Sep 17 00:00:00 2001
From: Shreyas Goenka <shreyas.goenka@databricks.com>
Date: Mon, 14 Oct 2024 15:18:58 +0200
Subject: [PATCH 24/39] separate GetFilerForLibraries tests

---
 bundle/libraries/upload_test.go | 238 +++++++++++++++++---------------
 1 file changed, 127 insertions(+), 111 deletions(-)

diff --git a/bundle/libraries/upload_test.go b/bundle/libraries/upload_test.go
index a1a601e8..955169fa 100644
--- a/bundle/libraries/upload_test.go
+++ b/bundle/libraries/upload_test.go
@@ -361,13 +361,21 @@ func TestFindVolumeInBundle(t *testing.T) {
 		},
 	}
 
-	bundletest.SetLocation(b, "resources.volumes.foo", "volume.yml")
+	bundletest.SetLocation(b, "resources.volumes.foo", []dyn.Location{
+		{
+			File:   "volume.yml",
+			Line:   1,
+			Column: 2,
+		},
+	})
 
 	// volume is in DAB.
 	path, locations, ok := findVolumeInBundle(b, "main", "my_schema", "my_volume")
 	assert.True(t, ok)
 	assert.Equal(t, []dyn.Location{{
-		File: "volume.yml",
+		File:   "volume.yml",
+		Line:   1,
+		Column: 2,
 	}}, locations)
 	assert.Equal(t, dyn.MustPathFromString("resources.volumes.foo"), path)
 
@@ -388,139 +396,147 @@ func TestFindVolumeInBundle(t *testing.T) {
 	path, locations, ok = findVolumeInBundle(b, "main", "valuedoesnotmatter", "my_volume")
 	assert.True(t, ok)
 	assert.Equal(t, []dyn.Location{{
-		File: "volume.yml",
+		File:   "volume.yml",
+		Line:   1,
+		Column: 2,
 	}}, locations)
 	assert.Equal(t, dyn.MustPathFromString("resources.volumes.foo"), path)
 }
 
-func TestGetFilerForLibraries(t *testing.T) {
-	t.Run("valid wsfs", func(t *testing.T) {
-		b := &bundle.Bundle{
-			Config: config.Root{
-				Workspace: config.Workspace{
-					ArtifactPath: "/foo/bar/artifacts",
-				},
+func TestGetFilerForLibrariesValidWsfs(t *testing.T) {
+	b := &bundle.Bundle{
+		Config: config.Root{
+			Workspace: config.Workspace{
+				ArtifactPath: "/foo/bar/artifacts",
 			},
-		}
+		},
+	}
 
-		client, uploadPath, diags := GetFilerForLibraries(context.Background(), b)
-		require.NoError(t, diags.Error())
-		assert.Equal(t, "/foo/bar/artifacts/.internal", uploadPath)
+	client, uploadPath, diags := GetFilerForLibraries(context.Background(), b)
+	require.NoError(t, diags.Error())
+	assert.Equal(t, "/foo/bar/artifacts/.internal", uploadPath)
 
-		assert.IsType(t, &filer.WorkspaceFilesClient{}, client)
-	})
+	assert.IsType(t, &filer.WorkspaceFilesClient{}, client)
+}
 
-	t.Run("valid uc volume", func(t *testing.T) {
-		b := &bundle.Bundle{
-			Config: config.Root{
-				Workspace: config.Workspace{
-					ArtifactPath: "/Volumes/main/my_schema/my_volume",
-				},
+func TestGetFilerForLibrariesValidUcVolume(t *testing.T) {
+	b := &bundle.Bundle{
+		Config: config.Root{
+			Workspace: config.Workspace{
+				ArtifactPath: "/Volumes/main/my_schema/my_volume",
 			},
-		}
+		},
+	}
 
-		m := mocks.NewMockWorkspaceClient(t)
-		m.WorkspaceClient.Config = &sdkconfig.Config{}
-		m.GetMockFilesAPI().EXPECT().GetDirectoryMetadataByDirectoryPath(mock.Anything, "/Volumes/main/my_schema/my_volume").Return(nil)
-		b.SetWorkpaceClient(m.WorkspaceClient)
+	m := mocks.NewMockWorkspaceClient(t)
+	m.WorkspaceClient.Config = &sdkconfig.Config{}
+	m.GetMockFilesAPI().EXPECT().GetDirectoryMetadataByDirectoryPath(mock.Anything, "/Volumes/main/my_schema/my_volume").Return(nil)
+	b.SetWorkpaceClient(m.WorkspaceClient)
 
-		client, uploadPath, diags := GetFilerForLibraries(context.Background(), b)
-		require.NoError(t, diags.Error())
-		assert.Equal(t, "/Volumes/main/my_schema/my_volume/.internal", uploadPath)
+	client, uploadPath, diags := GetFilerForLibraries(context.Background(), b)
+	require.NoError(t, diags.Error())
+	assert.Equal(t, "/Volumes/main/my_schema/my_volume/.internal", uploadPath)
 
-		assert.IsType(t, &filer.FilesClient{}, client)
-	})
+	assert.IsType(t, &filer.FilesClient{}, client)
+}
 
-	t.Run("volume not in DAB", func(t *testing.T) {
-		b := &bundle.Bundle{
-			Config: config.Root{
-				Workspace: config.Workspace{
-					ArtifactPath: "/Volumes/main/my_schema/doesnotexist",
-				},
+func TestGetFilerForLibrariesVolumeNotInBundle(t *testing.T) {
+	b := &bundle.Bundle{
+		Config: config.Root{
+			Workspace: config.Workspace{
+				ArtifactPath: "/Volumes/main/my_schema/doesnotexist",
 			},
-		}
+		},
+	}
 
-		m := mocks.NewMockWorkspaceClient(t)
-		m.WorkspaceClient.Config = &sdkconfig.Config{}
-		m.GetMockFilesAPI().EXPECT().GetDirectoryMetadataByDirectoryPath(mock.Anything, "/Volumes/main/my_schema/doesnotexist").Return(fmt.Errorf("error from API"))
-		b.SetWorkpaceClient(m.WorkspaceClient)
+	m := mocks.NewMockWorkspaceClient(t)
+	m.WorkspaceClient.Config = &sdkconfig.Config{}
+	m.GetMockFilesAPI().EXPECT().GetDirectoryMetadataByDirectoryPath(mock.Anything, "/Volumes/main/my_schema/doesnotexist").Return(fmt.Errorf("error from API"))
+	b.SetWorkpaceClient(m.WorkspaceClient)
 
-		_, _, diags := GetFilerForLibraries(context.Background(), b)
-		assert.EqualError(t, diags.Error(), "failed to fetch metadata for the UC volume /Volumes/main/my_schema/doesnotexist that is configured in the artifact_path: error from API")
-		assert.Len(t, diags, 1)
-	})
+	_, _, diags := GetFilerForLibraries(context.Background(), b)
+	assert.EqualError(t, diags.Error(), "failed to fetch metadata for the UC volume /Volumes/main/my_schema/doesnotexist that is configured in the artifact_path: error from API")
+	assert.Len(t, diags, 1)
+}
 
-	t.Run("volume in DAB config", func(t *testing.T) {
-		b := &bundle.Bundle{
-			Config: config.Root{
-				Workspace: config.Workspace{
-					ArtifactPath: "/Volumes/main/my_schema/my_volume",
-				},
-				Resources: config.Resources{
-					Volumes: map[string]*resources.Volume{
-						"foo": {
-							CreateVolumeRequestContent: &catalog.CreateVolumeRequestContent{
-								CatalogName: "main",
-								Name:        "my_volume",
-								VolumeType:  "MANAGED",
-								SchemaName:  "my_schema",
-							},
+func TestGetFilerForLibrariesVolumeInBundle(t *testing.T) {
+	b := &bundle.Bundle{
+		Config: config.Root{
+			Workspace: config.Workspace{
+				ArtifactPath: "/Volumes/main/my_schema/my_volume",
+			},
+			Resources: config.Resources{
+				Volumes: map[string]*resources.Volume{
+					"foo": {
+						CreateVolumeRequestContent: &catalog.CreateVolumeRequestContent{
+							CatalogName: "main",
+							Name:        "my_volume",
+							VolumeType:  "MANAGED",
+							SchemaName:  "my_schema",
 						},
 					},
 				},
 			},
-		}
+		},
+	}
 
-		bundletest.SetLocation(b, "resources.volumes.foo", "volume.yml")
-
-		m := mocks.NewMockWorkspaceClient(t)
-		m.WorkspaceClient.Config = &sdkconfig.Config{}
-		m.GetMockFilesAPI().EXPECT().GetDirectoryMetadataByDirectoryPath(mock.Anything, "/Volumes/main/my_schema/my_volume").Return(fmt.Errorf("error from API"))
-		b.SetWorkpaceClient(m.WorkspaceClient)
-
-		_, _, diags := GetFilerForLibraries(context.Background(), b)
-		assert.EqualError(t, diags.Error(), "failed to fetch metadata for the UC volume /Volumes/main/my_schema/my_volume that is configured in the artifact_path: error from API")
-		assert.Contains(t, diags, diag.Diagnostic{
-			Severity: diag.Warning,
-			Summary:  "You might be using a UC volume in your artifact_path that is managed by this bundle but which has not been deployed yet. Please deploy the UC volume in a separate bundle deploy before using it in the artifact_path.",
-			Locations: []dyn.Location{{
-				File: "volume.yml",
-			}},
-			Paths: []dyn.Path{dyn.MustPathFromString("resources.volumes.foo")},
-		})
+	bundletest.SetLocation(b, "resources.volumes.foo", []dyn.Location{
+		{
+			File:   "volume.yml",
+			Line:   1,
+			Column: 2,
+		},
 	})
 
-	t.Run("remote path is not set", func(t *testing.T) {
-		b := &bundle.Bundle{}
+	m := mocks.NewMockWorkspaceClient(t)
+	m.WorkspaceClient.Config = &sdkconfig.Config{}
+	m.GetMockFilesAPI().EXPECT().GetDirectoryMetadataByDirectoryPath(mock.Anything, "/Volumes/main/my_schema/my_volume").Return(fmt.Errorf("error from API"))
+	b.SetWorkpaceClient(m.WorkspaceClient)
 
-		_, _, diags := GetFilerForLibraries(context.Background(), b)
-		require.EqualError(t, diags.Error(), "remote artifact path not configured")
-	})
-
-	t.Run("invalid volume paths", func(t *testing.T) {
-		invalidPaths := []string{
-			"/Volumes/",
-			"/Volumes/main",
-			"/Volumes/main/",
-			"/Volumes/main//",
-			"/Volumes/main//my_schema",
-			"/Volumes/main/my_schema",
-			"/Volumes/main/my_schema/",
-			"/Volumes/main/my_schema//",
-			"/Volumes//my_schema/my_volume",
-		}
-
-		for _, p := range invalidPaths {
-			b := &bundle.Bundle{
-				Config: config.Root{
-					Workspace: config.Workspace{
-						ArtifactPath: p,
-					},
-				},
-			}
-
-			_, _, diags := GetFilerForLibraries(context.Background(), b)
-			require.EqualError(t, diags.Error(), fmt.Sprintf("expected UC volume path to be in the format /Volumes/<catalog>/<schema>/<volume>/..., got %s", path.Join(p, ".internal")))
-		}
+	_, _, diags := GetFilerForLibraries(context.Background(), b)
+	assert.EqualError(t, diags.Error(), "failed to fetch metadata for the UC volume /Volumes/main/my_schema/my_volume that is configured in the artifact_path: error from API")
+	assert.Contains(t, diags, diag.Diagnostic{
+		Severity: diag.Warning,
+		Summary:  "You might be using a UC volume in your artifact_path that is managed by this bundle but which has not been deployed yet. Please deploy the UC volume in a separate bundle deploy before using it in the artifact_path.",
+		Locations: []dyn.Location{{
+			File:   "volume.yml",
+			Line:   1,
+			Column: 2,
+		}},
+		Paths: []dyn.Path{dyn.MustPathFromString("resources.volumes.foo")},
 	})
 }
+
+func TestGetFilerForLibrariesVolumeInBundleWithArtifactPath(t *testing.T) {
+	b := &bundle.Bundle{}
+
+	_, _, diags := GetFilerForLibraries(context.Background(), b)
+	require.EqualError(t, diags.Error(), "remote artifact path not configured")
+}
+
+func TestGetFilerForLibrariesWithInvalidVolumePaths(t *testing.T) {
+	invalidPaths := []string{
+		"/Volumes/",
+		"/Volumes/main",
+		"/Volumes/main/",
+		"/Volumes/main//",
+		"/Volumes/main//my_schema",
+		"/Volumes/main/my_schema",
+		"/Volumes/main/my_schema/",
+		"/Volumes/main/my_schema//",
+		"/Volumes//my_schema/my_volume",
+	}
+
+	for _, p := range invalidPaths {
+		b := &bundle.Bundle{
+			Config: config.Root{
+				Workspace: config.Workspace{
+					ArtifactPath: p,
+				},
+			},
+		}
+
+		_, _, diags := GetFilerForLibraries(context.Background(), b)
+		require.EqualError(t, diags.Error(), fmt.Sprintf("expected UC volume path to be in the format /Volumes/<catalog>/<schema>/<volume>/..., got %s", path.Join(p, ".internal")))
+	}
+}

From 266c26ce09adbb13b72f82851cf1fce759e46515 Mon Sep 17 00:00:00 2001
From: Shreyas Goenka <shreyas.goenka@databricks.com>
Date: Mon, 14 Oct 2024 15:41:47 +0200
Subject: [PATCH 25/39] fix tesT

---
 internal/bundle/artifacts_test.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/internal/bundle/artifacts_test.go b/internal/bundle/artifacts_test.go
index cf12282f..4030db08 100644
--- a/internal/bundle/artifacts_test.go
+++ b/internal/bundle/artifacts_test.go
@@ -249,7 +249,7 @@ func TestAccUploadArtifactFileToInvalidVolume(t *testing.T) {
 		dir := t.TempDir()
 
 		b := &bundle.Bundle{
-			RootPath:     dir,
+			BundleRootPath:     dir,
 			SyncRootPath: dir,
 			Config: config.Root{
 				Bundle: config.Bundle{
@@ -282,7 +282,7 @@ func TestAccUploadArtifactFileToInvalidVolume(t *testing.T) {
 		dir := t.TempDir()
 
 		b := &bundle.Bundle{
-			RootPath:     dir,
+			BundleRootPath:     dir,
 			SyncRootPath: dir,
 			Config: config.Root{
 				Bundle: config.Bundle{

From a9b8575bc3667e7537d3232cba082354e2f18e54 Mon Sep 17 00:00:00 2001
From: Shreyas Goenka <shreyas.goenka@databricks.com>
Date: Mon, 14 Oct 2024 15:49:19 +0200
Subject: [PATCH 26/39] fmt and fix test

---
 bundle/config/resources.go        | 1 +
 internal/bundle/artifacts_test.go | 8 ++++----
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/bundle/config/resources.go b/bundle/config/resources.go
index 33040e7c..d3ddde89 100644
--- a/bundle/config/resources.go
+++ b/bundle/config/resources.go
@@ -76,6 +76,7 @@ func SupportedResources() map[string]ResourceDescription {
 		"registered_models":       {SingularName: "registered_model"},
 		"quality_monitors":        {SingularName: "quality_monitor"},
 		"schemas":                 {SingularName: "schema"},
+		"volumes":                 {SingularName: "volume"},
 		"clusters":                {SingularName: "cluster"},
 	}
 }
diff --git a/internal/bundle/artifacts_test.go b/internal/bundle/artifacts_test.go
index 4030db08..d2938fa7 100644
--- a/internal/bundle/artifacts_test.go
+++ b/internal/bundle/artifacts_test.go
@@ -249,8 +249,8 @@ func TestAccUploadArtifactFileToInvalidVolume(t *testing.T) {
 		dir := t.TempDir()
 
 		b := &bundle.Bundle{
-			BundleRootPath:     dir,
-			SyncRootPath: dir,
+			BundleRootPath: dir,
+			SyncRootPath:   dir,
 			Config: config.Root{
 				Bundle: config.Bundle{
 					Target: "whatever",
@@ -282,8 +282,8 @@ func TestAccUploadArtifactFileToInvalidVolume(t *testing.T) {
 		dir := t.TempDir()
 
 		b := &bundle.Bundle{
-			BundleRootPath:     dir,
-			SyncRootPath: dir,
+			BundleRootPath: dir,
+			SyncRootPath:   dir,
 			Config: config.Root{
 				Bundle: config.Bundle{
 					Target: "whatever",

From c5a02ef8fba4ef165b3d9a0fd00065966a4782d3 Mon Sep 17 00:00:00 2001
From: Shreyas Goenka <shreyas.goenka@databricks.com>
Date: Tue, 15 Oct 2024 15:22:41 +0200
Subject: [PATCH 27/39] split into filer files

---
 bundle/libraries/filer.go             |  29 ++++
 bundle/libraries/filer_test.go        |  63 ++++++++
 bundle/libraries/filer_volume.go      |  94 ++++++++++++
 bundle/libraries/filer_volume_test.go | 204 ++++++++++++++++++++++++++
 bundle/libraries/filer_workspace.go   |  15 ++
 bundle/libraries/upload.go            |  96 ------------
 bundle/libraries/upload_test.go       | 204 --------------------------
 7 files changed, 405 insertions(+), 300 deletions(-)
 create mode 100644 bundle/libraries/filer.go
 create mode 100644 bundle/libraries/filer_test.go
 create mode 100644 bundle/libraries/filer_volume.go
 create mode 100644 bundle/libraries/filer_volume_test.go
 create mode 100644 bundle/libraries/filer_workspace.go

diff --git a/bundle/libraries/filer.go b/bundle/libraries/filer.go
new file mode 100644
index 00000000..74f8175d
--- /dev/null
+++ b/bundle/libraries/filer.go
@@ -0,0 +1,29 @@
+package libraries
+
+import (
+	"context"
+	"strings"
+
+	"github.com/databricks/cli/bundle"
+	"github.com/databricks/cli/libs/diag"
+	"github.com/databricks/cli/libs/filer"
+)
+
+// This function returns the right filer to use, to upload artifacts to the configured location.
+// Supported locations:
+// 1. WSFS
+// 2. UC volumes
+func GetFilerForLibraries(ctx context.Context, b *bundle.Bundle) (filer.Filer, string, diag.Diagnostics) {
+	artifactPath := b.Config.Workspace.ArtifactPath
+	if artifactPath == "" {
+		return nil, "", diag.Errorf("remote artifact path not configured")
+	}
+
+	switch {
+	case strings.HasPrefix(artifactPath, "/Volumes/"):
+		return filerForVolume(ctx, b)
+
+	default:
+		return filerForWorkspace(b)
+	}
+}
diff --git a/bundle/libraries/filer_test.go b/bundle/libraries/filer_test.go
new file mode 100644
index 00000000..88ba152f
--- /dev/null
+++ b/bundle/libraries/filer_test.go
@@ -0,0 +1,63 @@
+package libraries
+
+import (
+	"context"
+	"testing"
+
+	"github.com/databricks/cli/bundle"
+	"github.com/databricks/cli/bundle/config"
+	"github.com/databricks/cli/libs/filer"
+	sdkconfig "github.com/databricks/databricks-sdk-go/config"
+	"github.com/databricks/databricks-sdk-go/experimental/mocks"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
+)
+
+func TestGetFilerForLibrariesValidWsfs(t *testing.T) {
+	b := &bundle.Bundle{
+		Config: config.Root{
+			Workspace: config.Workspace{
+				ArtifactPath: "/foo/bar/artifacts",
+			},
+		},
+	}
+
+	client, uploadPath, diags := GetFilerForLibraries(context.Background(), b)
+	require.NoError(t, diags.Error())
+	assert.Equal(t, "/foo/bar/artifacts/.internal", uploadPath)
+
+	assert.IsType(t, &filer.WorkspaceFilesClient{}, client)
+}
+
+func TestGetFilerForLibrariesValidUcVolume(t *testing.T) {
+	b := &bundle.Bundle{
+		Config: config.Root{
+			Workspace: config.Workspace{
+				ArtifactPath: "/Volumes/main/my_schema/my_volume",
+			},
+		},
+	}
+
+	m := mocks.NewMockWorkspaceClient(t)
+	m.WorkspaceClient.Config = &sdkconfig.Config{}
+	m.GetMockFilesAPI().EXPECT().GetDirectoryMetadataByDirectoryPath(mock.Anything, "/Volumes/main/my_schema/my_volume").Return(nil)
+	b.SetWorkpaceClient(m.WorkspaceClient)
+
+	client, uploadPath, diags := GetFilerForLibraries(context.Background(), b)
+	require.NoError(t, diags.Error())
+	assert.Equal(t, "/Volumes/main/my_schema/my_volume/.internal", uploadPath)
+
+	assert.IsType(t, &filer.FilesClient{}, client)
+}
+
+func TestGetFilerForLibrariesRemotePathNotSet(t *testing.T) {
+	b := &bundle.Bundle{
+		Config: config.Root{
+			Workspace: config.Workspace{},
+		},
+	}
+
+	_, _, diags := GetFilerForLibraries(context.Background(), b)
+	require.EqualError(t, diags.Error(), "remote artifact path not configured")
+}
diff --git a/bundle/libraries/filer_volume.go b/bundle/libraries/filer_volume.go
new file mode 100644
index 00000000..7308b65e
--- /dev/null
+++ b/bundle/libraries/filer_volume.go
@@ -0,0 +1,94 @@
+package libraries
+
+import (
+	"context"
+	"fmt"
+	"path"
+	"strings"
+
+	"github.com/databricks/cli/bundle"
+	"github.com/databricks/cli/libs/diag"
+	"github.com/databricks/cli/libs/dyn"
+	"github.com/databricks/cli/libs/dyn/dynvar"
+	"github.com/databricks/cli/libs/filer"
+)
+
+// This function returns a filer for ".internal" folder inside the directory configured
+// at `workspace.artifact_path`.
+// This function also checks if the UC volume exists in the workspace and then:
+//  1. If the UC volume exists in the workspace:
+//     Returns a filer for the UC volume.
+//  2. If the UC volume does not exist in the workspace but is (with high confidence) defined in
+//     the bundle configuration:
+//     Returns an error and a warning that instructs the user to deploy the
+//     UC volume before using it in the artifact path.
+//  3. If the UC volume does not exist in the workspace and is not defined in the bundle configuration:
+//     Returns an error.
+func filerForVolume(ctx context.Context, b *bundle.Bundle) (filer.Filer, string, diag.Diagnostics) {
+	artifactPath := b.Config.Workspace.ArtifactPath
+
+	w := b.WorkspaceClient()
+
+	parts := strings.Split(artifactPath, "/")
+	volumeFormatErr := fmt.Errorf("expected UC volume path to be in the format /Volumes/<catalog>/<schema>/<volume>/..., got %s", artifactPath)
+
+	// Incorrect format.
+	if len(parts) < 5 {
+		return nil, "", diag.FromErr(volumeFormatErr)
+	}
+
+	catalogName := parts[2]
+	schemaName := parts[3]
+	volumeName := parts[4]
+
+	// Incorrect format.
+	if catalogName == "" || schemaName == "" || volumeName == "" {
+		return nil, "", diag.FromErr(volumeFormatErr)
+	}
+
+	// Check if the UC volume exists in the workspace.
+	volumePath := fmt.Sprintf("/Volumes/%s/%s/%s", catalogName, schemaName, volumeName)
+	err := w.Files.GetDirectoryMetadataByDirectoryPath(ctx, volumePath)
+
+	// If the volume exists already, directly return the filer for the path to
+	// upload the artifacts to.
+	if err == nil {
+		uploadPath := path.Join(artifactPath, ".internal")
+		f, err := filer.NewFilesClient(w, uploadPath)
+		return f, uploadPath, diag.FromErr(err)
+	}
+
+	diags := diag.Errorf("failed to fetch metadata for the UC volume %s that is configured in the artifact_path: %s", volumePath, err)
+
+	path, locations, ok := findVolumeInBundle(b, catalogName, schemaName, volumeName)
+	if !ok {
+		return nil, "", diags
+	}
+
+	warning := diag.Diagnostic{
+		Severity:  diag.Warning,
+		Summary:   `You might be using a UC volume in your artifact_path that is managed by this bundle but which has not been deployed yet. Please deploy the UC volume in a separate bundle deploy before using it in the artifact_path.`,
+		Locations: locations,
+		Paths:     []dyn.Path{path},
+	}
+	return nil, "", diags.Append(warning)
+}
+
+func findVolumeInBundle(b *bundle.Bundle, catalogName, schemaName, volumeName string) (dyn.Path, []dyn.Location, bool) {
+	volumes := b.Config.Resources.Volumes
+	for k, v := range volumes {
+		if v.CatalogName != catalogName || v.Name != volumeName {
+			continue
+		}
+		// UC schemas can be defined in the bundle itself, and thus might be interpolated
+		// at runtime via the ${resources.schemas.<name>} syntax. Thus we match the volume
+		// definition if the schema name is the same as the one in the bundle, or if the
+		// schema name is interpolated.
+		if v.SchemaName != schemaName && !dynvar.IsPureVariableReference(v.SchemaName) {
+			continue
+		}
+		pathString := fmt.Sprintf("resources.volumes.%s", k)
+		return dyn.MustPathFromString(pathString), b.Config.GetLocations(pathString), true
+	}
+	return nil, nil, false
+}
diff --git a/bundle/libraries/filer_volume_test.go b/bundle/libraries/filer_volume_test.go
new file mode 100644
index 00000000..82b8ab55
--- /dev/null
+++ b/bundle/libraries/filer_volume_test.go
@@ -0,0 +1,204 @@
+package libraries
+
+import (
+	"context"
+	"fmt"
+	"path"
+	"testing"
+
+	"github.com/databricks/cli/bundle"
+	"github.com/databricks/cli/bundle/config"
+	"github.com/databricks/cli/bundle/config/resources"
+	"github.com/databricks/cli/bundle/internal/bundletest"
+	"github.com/databricks/cli/libs/diag"
+	"github.com/databricks/cli/libs/dyn"
+	"github.com/databricks/cli/libs/filer"
+	sdkconfig "github.com/databricks/databricks-sdk-go/config"
+	"github.com/databricks/databricks-sdk-go/experimental/mocks"
+	"github.com/databricks/databricks-sdk-go/service/catalog"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
+)
+
+func TestFindVolumeInBundle(t *testing.T) {
+	b := &bundle.Bundle{
+		Config: config.Root{
+			Resources: config.Resources{
+				Volumes: map[string]*resources.Volume{
+					"foo": {
+						CreateVolumeRequestContent: &catalog.CreateVolumeRequestContent{
+							CatalogName: "main",
+							Name:        "my_volume",
+							SchemaName:  "my_schema",
+						},
+					},
+				},
+			},
+		},
+	}
+
+	bundletest.SetLocation(b, "resources.volumes.foo", []dyn.Location{
+		{
+			File:   "volume.yml",
+			Line:   1,
+			Column: 2,
+		},
+	})
+
+	// volume is in DAB.
+	path, locations, ok := findVolumeInBundle(b, "main", "my_schema", "my_volume")
+	assert.True(t, ok)
+	assert.Equal(t, []dyn.Location{{
+		File:   "volume.yml",
+		Line:   1,
+		Column: 2,
+	}}, locations)
+	assert.Equal(t, dyn.MustPathFromString("resources.volumes.foo"), path)
+
+	// wrong volume name
+	_, _, ok = findVolumeInBundle(b, "main", "my_schema", "doesnotexist")
+	assert.False(t, ok)
+
+	// wrong schema name
+	_, _, ok = findVolumeInBundle(b, "main", "doesnotexist", "my_volume")
+	assert.False(t, ok)
+
+	// wrong catalog name
+	_, _, ok = findVolumeInBundle(b, "doesnotexist", "my_schema", "my_volume")
+	assert.False(t, ok)
+
+	// schema name is interpolated.
+	b.Config.Resources.Volumes["foo"].SchemaName = "${resources.schemas.my_schema}"
+	path, locations, ok = findVolumeInBundle(b, "main", "valuedoesnotmatter", "my_volume")
+	assert.True(t, ok)
+	assert.Equal(t, []dyn.Location{{
+		File:   "volume.yml",
+		Line:   1,
+		Column: 2,
+	}}, locations)
+	assert.Equal(t, dyn.MustPathFromString("resources.volumes.foo"), path)
+}
+
+func TestFilerForVolumeNotInBundle(t *testing.T) {
+	b := &bundle.Bundle{
+		Config: config.Root{
+			Workspace: config.Workspace{
+				ArtifactPath: "/Volumes/main/my_schema/doesnotexist",
+			},
+		},
+	}
+
+	m := mocks.NewMockWorkspaceClient(t)
+	m.WorkspaceClient.Config = &sdkconfig.Config{}
+	m.GetMockFilesAPI().EXPECT().GetDirectoryMetadataByDirectoryPath(mock.Anything, "/Volumes/main/my_schema/doesnotexist").Return(fmt.Errorf("error from API"))
+	b.SetWorkpaceClient(m.WorkspaceClient)
+
+	_, _, diags := filerForVolume(context.Background(), b)
+	assert.EqualError(t, diags.Error(), "failed to fetch metadata for the UC volume /Volumes/main/my_schema/doesnotexist that is configured in the artifact_path: error from API")
+	assert.Len(t, diags, 1)
+}
+
+func TestFilerForVolumeInBundle(t *testing.T) {
+	b := &bundle.Bundle{
+		Config: config.Root{
+			Workspace: config.Workspace{
+				ArtifactPath: "/Volumes/main/my_schema/my_volume",
+			},
+			Resources: config.Resources{
+				Volumes: map[string]*resources.Volume{
+					"foo": {
+						CreateVolumeRequestContent: &catalog.CreateVolumeRequestContent{
+							CatalogName: "main",
+							Name:        "my_volume",
+							VolumeType:  "MANAGED",
+							SchemaName:  "my_schema",
+						},
+					},
+				},
+			},
+		},
+	}
+
+	bundletest.SetLocation(b, "resources.volumes.foo", []dyn.Location{
+		{
+			File:   "volume.yml",
+			Line:   1,
+			Column: 2,
+		},
+	})
+
+	m := mocks.NewMockWorkspaceClient(t)
+	m.WorkspaceClient.Config = &sdkconfig.Config{}
+	m.GetMockFilesAPI().EXPECT().GetDirectoryMetadataByDirectoryPath(mock.Anything, "/Volumes/main/my_schema/my_volume").Return(fmt.Errorf("error from API"))
+	b.SetWorkpaceClient(m.WorkspaceClient)
+
+	_, _, diags := GetFilerForLibraries(context.Background(), b)
+	assert.EqualError(t, diags.Error(), "failed to fetch metadata for the UC volume /Volumes/main/my_schema/my_volume that is configured in the artifact_path: error from API")
+	assert.Contains(t, diags, diag.Diagnostic{
+		Severity: diag.Warning,
+		Summary:  "You might be using a UC volume in your artifact_path that is managed by this bundle but which has not been deployed yet. Please deploy the UC volume in a separate bundle deploy before using it in the artifact_path.",
+		Locations: []dyn.Location{{
+			File:   "volume.yml",
+			Line:   1,
+			Column: 2,
+		}},
+		Paths: []dyn.Path{dyn.MustPathFromString("resources.volumes.foo")},
+	})
+}
+
+func TestFilerForVolumeWithInvalidVolumePaths(t *testing.T) {
+	invalidPaths := []string{
+		"/Volumes/",
+		"/Volumes/main",
+		"/Volumes/main/",
+		"/Volumes/main//",
+		"/Volumes/main//my_schema",
+		"/Volumes/main/my_schema",
+		"/Volumes/main/my_schema/",
+		"/Volumes/main/my_schema//",
+		"/Volumes//my_schema/my_volume",
+	}
+
+	for _, p := range invalidPaths {
+		b := &bundle.Bundle{
+			Config: config.Root{
+				Workspace: config.Workspace{
+					ArtifactPath: p,
+				},
+			},
+		}
+
+		_, _, diags := GetFilerForLibraries(context.Background(), b)
+		require.EqualError(t, diags.Error(), fmt.Sprintf("expected UC volume path to be in the format /Volumes/<catalog>/<schema>/<volume>/..., got %s", path.Join(p, ".internal")))
+	}
+}
+
+func TestFilerForVolumeWithValidlVolumePaths(t *testing.T) {
+	validPaths := []string{
+		"/Volumes/main/my_schema/my_volume",
+		"/Volumes/main/my_schema/my_volume/",
+		"/Volumes/main/my_schema/my_volume/a/b/c",
+		"/Volumes/main/my_schema/my_volume/a/a/a",
+	}
+
+	for _, p := range validPaths {
+		b := &bundle.Bundle{
+			Config: config.Root{
+				Workspace: config.Workspace{
+					ArtifactPath: p,
+				},
+			},
+		}
+
+		m := mocks.NewMockWorkspaceClient(t)
+		m.WorkspaceClient.Config = &sdkconfig.Config{}
+		m.GetMockFilesAPI().EXPECT().GetDirectoryMetadataByDirectoryPath(mock.Anything, "/Volumes/main/my_schema/my_volume").Return(nil)
+		b.SetWorkpaceClient(m.WorkspaceClient)
+
+		client, uploadPath, diags := filerForVolume(context.Background(), b)
+		require.NoError(t, diags.Error())
+		assert.Equal(t, path.Join(p, ".internal"), uploadPath)
+		assert.IsType(t, &filer.FilesClient{}, client)
+	}
+}
diff --git a/bundle/libraries/filer_workspace.go b/bundle/libraries/filer_workspace.go
new file mode 100644
index 00000000..9172fef9
--- /dev/null
+++ b/bundle/libraries/filer_workspace.go
@@ -0,0 +1,15 @@
+package libraries
+
+import (
+	"path"
+
+	"github.com/databricks/cli/bundle"
+	"github.com/databricks/cli/libs/diag"
+	"github.com/databricks/cli/libs/filer"
+)
+
+func filerForWorkspace(b *bundle.Bundle) (filer.Filer, string, diag.Diagnostics) {
+	uploadPath := path.Join(b.Config.Workspace.ArtifactPath, ".internal")
+	f, err := filer.NewWorkspaceFilesClient(b.WorkspaceClient(), uploadPath)
+	return f, uploadPath, diag.FromErr(err)
+}
diff --git a/bundle/libraries/upload.go b/bundle/libraries/upload.go
index 36d64e43..8f1b425f 100644
--- a/bundle/libraries/upload.go
+++ b/bundle/libraries/upload.go
@@ -13,7 +13,6 @@ import (
 	"github.com/databricks/cli/libs/cmdio"
 	"github.com/databricks/cli/libs/diag"
 	"github.com/databricks/cli/libs/dyn"
-	"github.com/databricks/cli/libs/dyn/dynvar"
 	"github.com/databricks/cli/libs/filer"
 	"github.com/databricks/cli/libs/log"
 
@@ -189,101 +188,6 @@ func (u *upload) Name() string {
 	return "libraries.Upload"
 }
 
-// This function returns the right filer to use, to upload artifacts to the configured location.
-// Supported locations:
-// 1. WSFS
-// 2. UC volumes
-//
-// If a UC Volume is configured, this function checks if the UC volume exists in the workspace.
-// Then:
-//  1. If the UC volume exists in the workspace:
-//     Returns a filer for the UC volume.
-//  2. If the UC volume does not exist in the workspace but is (with high confidence) defined in
-//     the bundle configuration:
-//     Returns an error and a warning that instructs the user to deploy the
-//     UC volume before using it in the artifact path.
-//  3. If the UC volume does not exist in the workspace and is not defined in the bundle configuration:
-//     Returns an error.
-func GetFilerForLibraries(ctx context.Context, b *bundle.Bundle) (filer.Filer, string, diag.Diagnostics) {
-	artifactPath := b.Config.Workspace.ArtifactPath
-	if artifactPath == "" {
-		return nil, "", diag.Errorf("remote artifact path not configured")
-	}
-
-	w := b.WorkspaceClient()
-	isVolumesPath := strings.HasPrefix(artifactPath, "/Volumes/")
-
-	// Path to upload artifact files to.
-	uploadPath := path.Join(artifactPath, ".internal")
-
-	// Return early with a WSFS filer if the artifact path is not a UC volume path.
-	if !isVolumesPath {
-		f, err := filer.NewWorkspaceFilesClient(w, uploadPath)
-		return f, uploadPath, diag.FromErr(err)
-	}
-
-	parts := strings.Split(artifactPath, "/")
-	volumeFormatErr := fmt.Errorf("expected UC volume path to be in the format /Volumes/<catalog>/<schema>/<volume>/..., got %s", uploadPath)
-
-	// Incorrect format.
-	if len(parts) < 5 {
-		return nil, "", diag.FromErr(volumeFormatErr)
-	}
-
-	catalogName := parts[2]
-	schemaName := parts[3]
-	volumeName := parts[4]
-
-	// Incorrect format.
-	if catalogName == "" || schemaName == "" || volumeName == "" {
-		return nil, "", diag.FromErr(volumeFormatErr)
-	}
-
-	// Check if the UC volume exists in the workspace.
-	volumePath := fmt.Sprintf("/Volumes/%s/%s/%s", catalogName, schemaName, volumeName)
-	err := w.Files.GetDirectoryMetadataByDirectoryPath(ctx, volumePath)
-
-	// If the volume exists already, directly return the filer for the upload path.
-	if err == nil {
-		f, err := filer.NewFilesClient(w, uploadPath)
-		return f, uploadPath, diag.FromErr(err)
-	}
-
-	diags := diag.Errorf("failed to fetch metadata for the UC volume %s that is configured in the artifact_path: %s", volumePath, err)
-
-	path, locations, ok := findVolumeInBundle(b, catalogName, schemaName, volumeName)
-	if !ok {
-		return nil, "", diags
-	}
-
-	warning := diag.Diagnostic{
-		Severity:  diag.Warning,
-		Summary:   `You might be using a UC volume in your artifact_path that is managed by this bundle but which has not been deployed yet. Please deploy the UC volume in a separate bundle deploy before using it in the artifact_path.`,
-		Locations: locations,
-		Paths:     []dyn.Path{path},
-	}
-	return nil, "", diags.Append(warning)
-}
-
-func findVolumeInBundle(b *bundle.Bundle, catalogName, schemaName, volumeName string) (dyn.Path, []dyn.Location, bool) {
-	volumes := b.Config.Resources.Volumes
-	for k, v := range volumes {
-		if v.CatalogName != catalogName || v.Name != volumeName {
-			continue
-		}
-		// UC schemas can be defined in the bundle itself, and thus might be interpolated
-		// at runtime via the ${resources.schemas.<name>} syntax. Thus we match the volume
-		// definition if the schema name is the same as the one in the bundle, or if the
-		// schema name is interpolated.
-		if v.SchemaName != schemaName && !dynvar.IsPureVariableReference(v.SchemaName) {
-			continue
-		}
-		pathString := fmt.Sprintf("resources.volumes.%s", k)
-		return dyn.MustPathFromString(pathString), b.Config.GetLocations(pathString), true
-	}
-	return nil, nil, false
-}
-
 // Function to upload file (a library, artifact and etc) to Workspace or UC volume
 func UploadFile(ctx context.Context, file string, client filer.Filer) error {
 	filename := filepath.Base(file)
diff --git a/bundle/libraries/upload_test.go b/bundle/libraries/upload_test.go
index 955169fa..493785bf 100644
--- a/bundle/libraries/upload_test.go
+++ b/bundle/libraries/upload_test.go
@@ -2,26 +2,19 @@ package libraries
 
 import (
 	"context"
-	"fmt"
-	"path"
 	"path/filepath"
 	"testing"
 
 	"github.com/databricks/cli/bundle"
 	"github.com/databricks/cli/bundle/config"
 	"github.com/databricks/cli/bundle/config/resources"
-	"github.com/databricks/cli/bundle/internal/bundletest"
 	mockfiler "github.com/databricks/cli/internal/mocks/libs/filer"
 	"github.com/databricks/cli/internal/testutil"
-	"github.com/databricks/cli/libs/diag"
-	"github.com/databricks/cli/libs/dyn"
 	"github.com/databricks/cli/libs/filer"
 	sdkconfig "github.com/databricks/databricks-sdk-go/config"
 	"github.com/databricks/databricks-sdk-go/experimental/mocks"
-	"github.com/databricks/databricks-sdk-go/service/catalog"
 	"github.com/databricks/databricks-sdk-go/service/compute"
 	"github.com/databricks/databricks-sdk-go/service/jobs"
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 )
@@ -343,200 +336,3 @@ func TestUploadMultipleLibraries(t *testing.T) {
 	require.Contains(t, b.Config.Resources.Jobs["job"].JobSettings.Environments[0].Spec.Dependencies, "/Workspace/foo/bar/artifacts/.internal/source4.whl")
 	require.Contains(t, b.Config.Resources.Jobs["job"].JobSettings.Environments[0].Spec.Dependencies, "/Workspace/Users/foo@bar.com/mywheel.whl")
 }
-
-func TestFindVolumeInBundle(t *testing.T) {
-	b := &bundle.Bundle{
-		Config: config.Root{
-			Resources: config.Resources{
-				Volumes: map[string]*resources.Volume{
-					"foo": {
-						CreateVolumeRequestContent: &catalog.CreateVolumeRequestContent{
-							CatalogName: "main",
-							Name:        "my_volume",
-							SchemaName:  "my_schema",
-						},
-					},
-				},
-			},
-		},
-	}
-
-	bundletest.SetLocation(b, "resources.volumes.foo", []dyn.Location{
-		{
-			File:   "volume.yml",
-			Line:   1,
-			Column: 2,
-		},
-	})
-
-	// volume is in DAB.
-	path, locations, ok := findVolumeInBundle(b, "main", "my_schema", "my_volume")
-	assert.True(t, ok)
-	assert.Equal(t, []dyn.Location{{
-		File:   "volume.yml",
-		Line:   1,
-		Column: 2,
-	}}, locations)
-	assert.Equal(t, dyn.MustPathFromString("resources.volumes.foo"), path)
-
-	// wrong volume name
-	_, _, ok = findVolumeInBundle(b, "main", "my_schema", "doesnotexist")
-	assert.False(t, ok)
-
-	// wrong schema name
-	_, _, ok = findVolumeInBundle(b, "main", "doesnotexist", "my_volume")
-	assert.False(t, ok)
-
-	// wrong catalog name
-	_, _, ok = findVolumeInBundle(b, "doesnotexist", "my_schema", "my_volume")
-	assert.False(t, ok)
-
-	// schema name is interpolated.
-	b.Config.Resources.Volumes["foo"].SchemaName = "${resources.schemas.my_schema}"
-	path, locations, ok = findVolumeInBundle(b, "main", "valuedoesnotmatter", "my_volume")
-	assert.True(t, ok)
-	assert.Equal(t, []dyn.Location{{
-		File:   "volume.yml",
-		Line:   1,
-		Column: 2,
-	}}, locations)
-	assert.Equal(t, dyn.MustPathFromString("resources.volumes.foo"), path)
-}
-
-func TestGetFilerForLibrariesValidWsfs(t *testing.T) {
-	b := &bundle.Bundle{
-		Config: config.Root{
-			Workspace: config.Workspace{
-				ArtifactPath: "/foo/bar/artifacts",
-			},
-		},
-	}
-
-	client, uploadPath, diags := GetFilerForLibraries(context.Background(), b)
-	require.NoError(t, diags.Error())
-	assert.Equal(t, "/foo/bar/artifacts/.internal", uploadPath)
-
-	assert.IsType(t, &filer.WorkspaceFilesClient{}, client)
-}
-
-func TestGetFilerForLibrariesValidUcVolume(t *testing.T) {
-	b := &bundle.Bundle{
-		Config: config.Root{
-			Workspace: config.Workspace{
-				ArtifactPath: "/Volumes/main/my_schema/my_volume",
-			},
-		},
-	}
-
-	m := mocks.NewMockWorkspaceClient(t)
-	m.WorkspaceClient.Config = &sdkconfig.Config{}
-	m.GetMockFilesAPI().EXPECT().GetDirectoryMetadataByDirectoryPath(mock.Anything, "/Volumes/main/my_schema/my_volume").Return(nil)
-	b.SetWorkpaceClient(m.WorkspaceClient)
-
-	client, uploadPath, diags := GetFilerForLibraries(context.Background(), b)
-	require.NoError(t, diags.Error())
-	assert.Equal(t, "/Volumes/main/my_schema/my_volume/.internal", uploadPath)
-
-	assert.IsType(t, &filer.FilesClient{}, client)
-}
-
-func TestGetFilerForLibrariesVolumeNotInBundle(t *testing.T) {
-	b := &bundle.Bundle{
-		Config: config.Root{
-			Workspace: config.Workspace{
-				ArtifactPath: "/Volumes/main/my_schema/doesnotexist",
-			},
-		},
-	}
-
-	m := mocks.NewMockWorkspaceClient(t)
-	m.WorkspaceClient.Config = &sdkconfig.Config{}
-	m.GetMockFilesAPI().EXPECT().GetDirectoryMetadataByDirectoryPath(mock.Anything, "/Volumes/main/my_schema/doesnotexist").Return(fmt.Errorf("error from API"))
-	b.SetWorkpaceClient(m.WorkspaceClient)
-
-	_, _, diags := GetFilerForLibraries(context.Background(), b)
-	assert.EqualError(t, diags.Error(), "failed to fetch metadata for the UC volume /Volumes/main/my_schema/doesnotexist that is configured in the artifact_path: error from API")
-	assert.Len(t, diags, 1)
-}
-
-func TestGetFilerForLibrariesVolumeInBundle(t *testing.T) {
-	b := &bundle.Bundle{
-		Config: config.Root{
-			Workspace: config.Workspace{
-				ArtifactPath: "/Volumes/main/my_schema/my_volume",
-			},
-			Resources: config.Resources{
-				Volumes: map[string]*resources.Volume{
-					"foo": {
-						CreateVolumeRequestContent: &catalog.CreateVolumeRequestContent{
-							CatalogName: "main",
-							Name:        "my_volume",
-							VolumeType:  "MANAGED",
-							SchemaName:  "my_schema",
-						},
-					},
-				},
-			},
-		},
-	}
-
-	bundletest.SetLocation(b, "resources.volumes.foo", []dyn.Location{
-		{
-			File:   "volume.yml",
-			Line:   1,
-			Column: 2,
-		},
-	})
-
-	m := mocks.NewMockWorkspaceClient(t)
-	m.WorkspaceClient.Config = &sdkconfig.Config{}
-	m.GetMockFilesAPI().EXPECT().GetDirectoryMetadataByDirectoryPath(mock.Anything, "/Volumes/main/my_schema/my_volume").Return(fmt.Errorf("error from API"))
-	b.SetWorkpaceClient(m.WorkspaceClient)
-
-	_, _, diags := GetFilerForLibraries(context.Background(), b)
-	assert.EqualError(t, diags.Error(), "failed to fetch metadata for the UC volume /Volumes/main/my_schema/my_volume that is configured in the artifact_path: error from API")
-	assert.Contains(t, diags, diag.Diagnostic{
-		Severity: diag.Warning,
-		Summary:  "You might be using a UC volume in your artifact_path that is managed by this bundle but which has not been deployed yet. Please deploy the UC volume in a separate bundle deploy before using it in the artifact_path.",
-		Locations: []dyn.Location{{
-			File:   "volume.yml",
-			Line:   1,
-			Column: 2,
-		}},
-		Paths: []dyn.Path{dyn.MustPathFromString("resources.volumes.foo")},
-	})
-}
-
-func TestGetFilerForLibrariesVolumeInBundleWithArtifactPath(t *testing.T) {
-	b := &bundle.Bundle{}
-
-	_, _, diags := GetFilerForLibraries(context.Background(), b)
-	require.EqualError(t, diags.Error(), "remote artifact path not configured")
-}
-
-func TestGetFilerForLibrariesWithInvalidVolumePaths(t *testing.T) {
-	invalidPaths := []string{
-		"/Volumes/",
-		"/Volumes/main",
-		"/Volumes/main/",
-		"/Volumes/main//",
-		"/Volumes/main//my_schema",
-		"/Volumes/main/my_schema",
-		"/Volumes/main/my_schema/",
-		"/Volumes/main/my_schema//",
-		"/Volumes//my_schema/my_volume",
-	}
-
-	for _, p := range invalidPaths {
-		b := &bundle.Bundle{
-			Config: config.Root{
-				Workspace: config.Workspace{
-					ArtifactPath: p,
-				},
-			},
-		}
-
-		_, _, diags := GetFilerForLibraries(context.Background(), b)
-		require.EqualError(t, diags.Error(), fmt.Sprintf("expected UC volume path to be in the format /Volumes/<catalog>/<schema>/<volume>/..., got %s", path.Join(p, ".internal")))
-	}
-}

From eb94cd671723bb3cae41176694200f2ab54582be Mon Sep 17 00:00:00 2001
From: Shreyas Goenka <shreyas.goenka@databricks.com>
Date: Tue, 15 Oct 2024 15:27:58 +0200
Subject: [PATCH 28/39] -

---
 bundle/libraries/filer_volume.go      |  5 ++++-
 bundle/libraries/filer_volume_test.go | 13 +++++++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/bundle/libraries/filer_volume.go b/bundle/libraries/filer_volume.go
index 7308b65e..0348ef6f 100644
--- a/bundle/libraries/filer_volume.go
+++ b/bundle/libraries/filer_volume.go
@@ -26,9 +26,12 @@ import (
 //     Returns an error.
 func filerForVolume(ctx context.Context, b *bundle.Bundle) (filer.Filer, string, diag.Diagnostics) {
 	artifactPath := b.Config.Workspace.ArtifactPath
-
 	w := b.WorkspaceClient()
 
+	if !strings.HasPrefix(artifactPath, "/Volumes/") {
+		return nil, "", diag.Errorf("expected artifact_path to start with /Volumes/, got %s", artifactPath)
+	}
+
 	parts := strings.Split(artifactPath, "/")
 	volumeFormatErr := fmt.Errorf("expected UC volume path to be in the format /Volumes/<catalog>/<schema>/<volume>/..., got %s", artifactPath)
 
diff --git a/bundle/libraries/filer_volume_test.go b/bundle/libraries/filer_volume_test.go
index 82b8ab55..e45a7cb0 100644
--- a/bundle/libraries/filer_volume_test.go
+++ b/bundle/libraries/filer_volume_test.go
@@ -174,6 +174,19 @@ func TestFilerForVolumeWithInvalidVolumePaths(t *testing.T) {
 	}
 }
 
+func TestFilerForVolumeWithInvalidPrefix(t *testing.T) {
+	b := &bundle.Bundle{
+		Config: config.Root{
+			Workspace: config.Workspace{
+				ArtifactPath: "/Volume/main/my_schema/my_volume",
+			},
+		},
+	}
+
+	_, _, diags := filerForVolume(context.Background(), b)
+	require.EqualError(t, diags.Error(), "expected artifact_path to start with /Volumes/, got /Volume/main/my_schema/my_volume")
+}
+
 func TestFilerForVolumeWithValidlVolumePaths(t *testing.T) {
 	validPaths := []string{
 		"/Volumes/main/my_schema/my_volume",

From 3e3ddfd0cbd6b842cbdc7c0dba587586fd63ecbb Mon Sep 17 00:00:00 2001
From: Shreyas Goenka <shreyas.goenka@databricks.com>
Date: Tue, 15 Oct 2024 15:29:24 +0200
Subject: [PATCH 29/39] fix test

---
 bundle/libraries/filer_volume_test.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bundle/libraries/filer_volume_test.go b/bundle/libraries/filer_volume_test.go
index e45a7cb0..e4fe6630 100644
--- a/bundle/libraries/filer_volume_test.go
+++ b/bundle/libraries/filer_volume_test.go
@@ -170,7 +170,7 @@ func TestFilerForVolumeWithInvalidVolumePaths(t *testing.T) {
 		}
 
 		_, _, diags := GetFilerForLibraries(context.Background(), b)
-		require.EqualError(t, diags.Error(), fmt.Sprintf("expected UC volume path to be in the format /Volumes/<catalog>/<schema>/<volume>/..., got %s", path.Join(p, ".internal")))
+		require.EqualError(t, diags.Error(), fmt.Sprintf("expected UC volume path to be in the format /Volumes/<catalog>/<schema>/<volume>/..., got %s", p))
 	}
 }
 

From d241c2b39c915351a289a6b1a6950ad52e939c13 Mon Sep 17 00:00:00 2001
From: Shreyas Goenka <shreyas.goenka@databricks.com>
Date: Tue, 15 Oct 2024 16:05:23 +0200
Subject: [PATCH 30/39] add integration test for grant on volume

---
 .../uc_volume/template/databricks.yml.tmpl    |  5 +++++
 internal/bundle/deploy_test.go                | 20 +++++++++++++++++--
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/internal/bundle/bundles/uc_volume/template/databricks.yml.tmpl b/internal/bundle/bundles/uc_volume/template/databricks.yml.tmpl
index 9d377f8e..d7f31439 100644
--- a/internal/bundle/bundles/uc_volume/template/databricks.yml.tmpl
+++ b/internal/bundle/bundles/uc_volume/template/databricks.yml.tmpl
@@ -24,3 +24,8 @@ resources:
       schema_name: ${var.schema_name}
       volume_type: MANAGED
       comment: This volume was created from DABs.
+
+      grants:
+        - principal: account users
+          privileges:
+          - WRITE_VOLUME
diff --git a/internal/bundle/deploy_test.go b/internal/bundle/deploy_test.go
index 72ec7aaf..9399c648 100644
--- a/internal/bundle/deploy_test.go
+++ b/internal/bundle/deploy_test.go
@@ -265,12 +265,20 @@ func TestAccDeployUcVolume(t *testing.T) {
 	catalogName := "main"
 	schemaName := "schema1-" + uniqueId
 	volumeName := "my_volume"
-	volume, err := w.Volumes.ReadByName(ctx, fmt.Sprintf("%s.%s.%s", catalogName, schemaName, volumeName))
+	fullName := fmt.Sprintf("%s.%s.%s", catalogName, schemaName, volumeName)
+	volume, err := w.Volumes.ReadByName(ctx, fullName)
 	require.NoError(t, err)
 	require.Equal(t, volume.Name, volumeName)
 	require.Equal(t, catalogName, volume.CatalogName)
 	require.Equal(t, schemaName, volume.SchemaName)
 
+	// Assert that the grants were successfully applied.
+	grants, err := w.Grants.GetBySecurableTypeAndFullName(ctx, catalog.SecurableTypeVolume, fullName)
+	require.NoError(t, err)
+	assert.Len(t, grants.PrivilegeAssignments, 1)
+	assert.Equal(t, "account users", grants.PrivilegeAssignments[0].Principal)
+	assert.Equal(t, []catalog.Privilege{catalog.PrivilegeWriteVolume}, grants.PrivilegeAssignments[0].Privileges)
+
 	// Recreation of the volume without --auto-approve should fail since prompting is not possible
 	t.Setenv("TERM", "dumb")
 	t.Setenv("BUNDLE_ROOT", bundleRoot)
@@ -290,9 +298,17 @@ volumes the upstream data in the cloud tenant is not affected:
 
 	// Assert the volume is updated successfully
 	schemaName = "schema2-" + uniqueId
-	volume, err = w.Volumes.ReadByName(ctx, fmt.Sprintf("%s.%s.%s", catalogName, schemaName, volumeName))
+	fullName = fmt.Sprintf("%s.%s.%s", catalogName, schemaName, volumeName)
+	volume, err = w.Volumes.ReadByName(ctx, fullName)
 	require.NoError(t, err)
 	require.Equal(t, volume.Name, volumeName)
 	require.Equal(t, catalogName, volume.CatalogName)
 	require.Equal(t, schemaName, volume.SchemaName)
+
+	// assert that the grants were applied / retained on recreate.
+	grants, err = w.Grants.GetBySecurableTypeAndFullName(ctx, catalog.SecurableTypeVolume, fullName)
+	require.NoError(t, err)
+	assert.Len(t, grants.PrivilegeAssignments, 1)
+	assert.Equal(t, "account users", grants.PrivilegeAssignments[0].Principal)
+	assert.Equal(t, []catalog.Privilege{catalog.PrivilegeWriteVolume}, grants.PrivilegeAssignments[0].Privileges)
 }

From 6192835d6324e046d6f348c5954199b01d9728c3 Mon Sep 17 00:00:00 2001
From: Shreyas Goenka <shreyas.goenka@databricks.com>
Date: Tue, 15 Oct 2024 18:04:20 +0200
Subject: [PATCH 31/39] add custom prefixing behaviour for volumes

---
 bundle/config/mutator/apply_presets.go       | 23 +++++-
 bundle/config/mutator/apply_presets_test.go  | 87 ++++++++++++++++++++
 bundle/config/mutator/process_target_mode.go |  6 +-
 bundle/libraries/filer_volume.go             |  6 +-
 bundle/libraries/filer_volume_test.go        |  8 +-
 libs/dyn/dynvar/ref.go                       | 20 +++++
 libs/dyn/dynvar/ref_test.go                  | 31 +++++++
 7 files changed, 176 insertions(+), 5 deletions(-)

diff --git a/bundle/config/mutator/apply_presets.go b/bundle/config/mutator/apply_presets.go
index 23bf1ba4..4d5a4f51 100644
--- a/bundle/config/mutator/apply_presets.go
+++ b/bundle/config/mutator/apply_presets.go
@@ -11,6 +11,8 @@ import (
 	"github.com/databricks/cli/bundle/config"
 	"github.com/databricks/cli/libs/diag"
 	"github.com/databricks/cli/libs/dyn"
+	"github.com/databricks/cli/libs/dyn/dynvar"
+	"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"
@@ -194,8 +196,25 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos
 	}
 
 	// Apply the prefix to volumes
-	for i := range r.Volumes {
-		r.Volumes[i].Name = normalizePrefix(prefix) + r.Volumes[i].Name
+	for _, v := range r.Volumes {
+		if containsUserIdentity(v.CatalogName, b.Config.Workspace.CurrentUser) {
+			log.Debugf(ctx, "Skipping prefix for volume %s because catalog %s contains the current user's identity", v.Name, v.CatalogName)
+			continue
+		}
+		if containsUserIdentity(v.SchemaName, b.Config.Workspace.CurrentUser) {
+			log.Debugf(ctx, "Skipping prefix for volume %s because schema %s contains the current user's identity", v.Name, v.SchemaName)
+			continue
+		}
+		// We only have to check for ${resources.schemas...} references because any
+		// other valid reference (like ${var.foo}) would have been interpolated by this point.
+		if p, ok := dynvar.PureReferenceToPath(v.SchemaName); ok && p.HasPrefix(dyn.Path{dyn.Key("resources"), dyn.Key("schemas")}) {
+			log.Debugf(ctx, "Skipping prefix for volume %s because schema name %s is defined in the bundle and will be interpolated at runtime", v.Name, v.SchemaName)
+			continue
+
+		}
+
+		v.Name = normalizePrefix(prefix) + v.Name
+
 		// HTTP API for volumes doesn't yet support tags. It's only supported in
 		// the Databricks UI and via the SQL API.
 	}
diff --git a/bundle/config/mutator/apply_presets_test.go b/bundle/config/mutator/apply_presets_test.go
index d3165672..b6b71d46 100644
--- a/bundle/config/mutator/apply_presets_test.go
+++ b/bundle/config/mutator/apply_presets_test.go
@@ -9,7 +9,9 @@ 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/assert"
 	"github.com/stretchr/testify/require"
 )
 
@@ -125,6 +127,15 @@ func TestApplyPresetsPrefixForUcSchema(t *testing.T) {
 	}
 }
 
+func TestApplyPresentsReminderToAddSupportForSkippingPrefixes(t *testing.T) {
+	_, ok := config.SupportedResources()["catalogs"]
+	assert.False(t, ok,
+		`Since you are adding support for UC catalogs to DABs please ensure that
+you add logic to skip applying presets.name_prefix for UC schemas, UC volumes and
+any other resources that fall under a catalog in the UC hierarchy (like registered models).
+Once you do so feel free to remove this test.`)
+}
+
 func TestApplyPresetsPrefixForUcVolumes(t *testing.T) {
 	tests := []struct {
 		name   string
@@ -138,6 +149,8 @@ func TestApplyPresetsPrefixForUcVolumes(t *testing.T) {
 			volume: &resources.Volume{
 				CreateVolumeRequestContent: &catalog.CreateVolumeRequestContent{
 					Name: "volume1",
+					CatalogName: "catalog1",
+					SchemaName:  "schema1",
 				},
 			},
 			want: "prefix_volume1",
@@ -152,6 +165,72 @@ func TestApplyPresetsPrefixForUcVolumes(t *testing.T) {
 			},
 			want: "volume1",
 		},
+		{
+			name:   "skip prefix when catalog name contains user short name",
+			prefix: "[prefix]",
+			volume: &resources.Volume{
+				CreateVolumeRequestContent: &catalog.CreateVolumeRequestContent{
+					Name:        "volume1",
+					CatalogName: "dev_john_wick_targets",
+				},
+			},
+			want: "volume1",
+		},
+		{
+			name:   "skip prefix when catalog name contains user email",
+			prefix: "[prefix]",
+			volume: &resources.Volume{
+				CreateVolumeRequestContent: &catalog.CreateVolumeRequestContent{
+					Name:        "volume1",
+					CatalogName: "dev_john.wick@continental.com_targets",
+				},
+			},
+			want: "volume1",
+		},
+		{
+			name:   "skip prefix when schema name contains user short name",
+			prefix: "[prefix]",
+			volume: &resources.Volume{
+				CreateVolumeRequestContent: &catalog.CreateVolumeRequestContent{
+					Name:       "volume1",
+					SchemaName: "dev_john_wick_weapons",
+				},
+			},
+			want: "volume1",
+		},
+		{
+			name:   "skip prefix when schema name contains user email",
+			prefix: "[prefix]",
+			volume: &resources.Volume{
+				CreateVolumeRequestContent: &catalog.CreateVolumeRequestContent{
+					Name:       "volume1",
+					SchemaName: "dev_john.wick@continental.com_targets",
+				},
+			},
+			want: "volume1",
+		},
+		{
+			name:   "skip prefix when schema is defined in the bundle and will be interpolated at runtime",
+			prefix: "[prefix]",
+			volume: &resources.Volume{
+				CreateVolumeRequestContent: &catalog.CreateVolumeRequestContent{
+					Name:       "volume1",
+					SchemaName: "${resources.schemas.schema1.name}",
+				},
+			},
+			want: "volume1",
+		},
+		{
+			name:   "add prefix when schema is a reference without the resources.schemas prefix",
+			prefix: "[prefix]",
+			volume: &resources.Volume{
+				CreateVolumeRequestContent: &catalog.CreateVolumeRequestContent{
+					Name:       "volume1",
+					SchemaName: "${foo.bar.baz}",
+				},
+			},
+			want: "prefix_volume1",
+		},
 	}
 
 	for _, tt := range tests {
@@ -166,6 +245,14 @@ func TestApplyPresetsPrefixForUcVolumes(t *testing.T) {
 					Presets: config.Presets{
 						NamePrefix: tt.prefix,
 					},
+					Workspace: config.Workspace{
+						CurrentUser: &config.User{
+							ShortName: "john_wick",
+							User: &iam.User{
+								UserName: "john.wick@continental.com",
+							},
+						},
+					},
 				},
 			}
 
diff --git a/bundle/config/mutator/process_target_mode.go b/bundle/config/mutator/process_target_mode.go
index 44b53681..870afe9d 100644
--- a/bundle/config/mutator/process_target_mode.go
+++ b/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
diff --git a/bundle/libraries/filer_volume.go b/bundle/libraries/filer_volume.go
index 0348ef6f..ec88bb94 100644
--- a/bundle/libraries/filer_volume.go
+++ b/bundle/libraries/filer_volume.go
@@ -87,7 +87,11 @@ func findVolumeInBundle(b *bundle.Bundle, catalogName, schemaName, volumeName st
 		// at runtime via the ${resources.schemas.<name>} syntax. Thus we match the volume
 		// definition if the schema name is the same as the one in the bundle, or if the
 		// schema name is interpolated.
-		if v.SchemaName != schemaName && !dynvar.IsPureVariableReference(v.SchemaName) {
+		// We only have to check for ${resources.schemas...} references because any
+		// other valid reference (like ${var.foo}) would have been interpolated by this point.
+		p, ok := dynvar.PureReferenceToPath(v.SchemaName)
+		isSchemaDefinedInBundle := ok && p.HasPrefix(dyn.Path{dyn.Key("resources"), dyn.Key("schemas")})
+		if v.SchemaName != schemaName && !isSchemaDefinedInBundle {
 			continue
 		}
 		pathString := fmt.Sprintf("resources.volumes.%s", k)
diff --git a/bundle/libraries/filer_volume_test.go b/bundle/libraries/filer_volume_test.go
index e4fe6630..047a009a 100644
--- a/bundle/libraries/filer_volume_test.go
+++ b/bundle/libraries/filer_volume_test.go
@@ -68,8 +68,14 @@ func TestFindVolumeInBundle(t *testing.T) {
 	_, _, ok = findVolumeInBundle(b, "doesnotexist", "my_schema", "my_volume")
 	assert.False(t, ok)
 
+	// schema name is interpolated but does not have the right prefix. In this case
+	// we should not match the volume.
+	b.Config.Resources.Volumes["foo"].SchemaName = "${foo.bar.baz}"
+	_, _, ok = findVolumeInBundle(b, "main", "my_schema", "my_volume")
+	assert.False(t, ok)
+
 	// schema name is interpolated.
-	b.Config.Resources.Volumes["foo"].SchemaName = "${resources.schemas.my_schema}"
+	b.Config.Resources.Volumes["foo"].SchemaName = "${resources.schemas.my_schema.name}"
 	path, locations, ok = findVolumeInBundle(b, "main", "valuedoesnotmatter", "my_volume")
 	assert.True(t, ok)
 	assert.Equal(t, []dyn.Location{{
diff --git a/libs/dyn/dynvar/ref.go b/libs/dyn/dynvar/ref.go
index 338ac8ce..a2893882 100644
--- a/libs/dyn/dynvar/ref.go
+++ b/libs/dyn/dynvar/ref.go
@@ -71,3 +71,23 @@ func (v ref) references() []string {
 func IsPureVariableReference(s string) bool {
 	return len(s) > 0 && re.FindString(s) == s
 }
+
+// If s is a pure variable reference, this function returns the corresponding
+// dyn.Path. Otherwise, it returns false.
+func PureReferenceToPath(s string) (dyn.Path, bool) {
+	ref, ok := newRef(dyn.V(s))
+	if !ok {
+		return nil, false
+	}
+
+	if !ref.isPure() {
+		return nil, false
+	}
+
+	p, err := dyn.NewPathFromString(ref.references()[0])
+	if err != nil {
+		return nil, false
+	}
+
+	return p, true
+}
diff --git a/libs/dyn/dynvar/ref_test.go b/libs/dyn/dynvar/ref_test.go
index aff3643e..4110732f 100644
--- a/libs/dyn/dynvar/ref_test.go
+++ b/libs/dyn/dynvar/ref_test.go
@@ -51,3 +51,34 @@ func TestIsPureVariableReference(t *testing.T) {
 	assert.False(t, IsPureVariableReference("prefix ${foo.bar}"))
 	assert.True(t, IsPureVariableReference("${foo.bar}"))
 }
+
+func TestPureReferenceToPath(t *testing.T) {
+	for _, tc := range []struct {
+		in  string
+		out string
+		ok  bool
+	}{
+		{"${foo.bar}", "foo.bar", true},
+		{"${foo.bar.baz}", "foo.bar.baz", true},
+		{"${foo.bar.baz[0]}", "foo.bar.baz[0]", true},
+		{"${foo.bar.baz[0][1]}", "foo.bar.baz[0][1]", true},
+		{"${foo.bar.baz[0][1].qux}", "foo.bar.baz[0][1].qux", true},
+
+		{"${foo.one}${foo.two}", "", false},
+		{"prefix ${foo.bar}", "", false},
+		{"${foo.bar} suffix", "", false},
+		{"${foo.bar", "", false},
+		{"foo.bar}", "", false},
+		{"foo.bar", "", false},
+		{"{foo.bar}", "", false},
+		{"", "", false},
+	} {
+		path, ok := PureReferenceToPath(tc.in)
+		if tc.ok {
+			assert.True(t, ok)
+			assert.Equal(t, dyn.MustPathFromString(tc.out), path)
+		} else {
+			assert.False(t, ok)
+		}
+	}
+}

From 701b1786a85d21903e3d1718229035bbc57dc906 Mon Sep 17 00:00:00 2001
From: Shreyas Goenka <shreyas.goenka@databricks.com>
Date: Tue, 15 Oct 2024 22:03:29 +0200
Subject: [PATCH 32/39] fmt

---
 bundle/config/mutator/apply_presets_test.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bundle/config/mutator/apply_presets_test.go b/bundle/config/mutator/apply_presets_test.go
index b6b71d46..50c5618b 100644
--- a/bundle/config/mutator/apply_presets_test.go
+++ b/bundle/config/mutator/apply_presets_test.go
@@ -148,7 +148,7 @@ func TestApplyPresetsPrefixForUcVolumes(t *testing.T) {
 			prefix: "[prefix]",
 			volume: &resources.Volume{
 				CreateVolumeRequestContent: &catalog.CreateVolumeRequestContent{
-					Name: "volume1",
+					Name:        "volume1",
 					CatalogName: "catalog1",
 					SchemaName:  "schema1",
 				},

From 810da66809cf5771da20ce12926408c0cf72a2f3 Mon Sep 17 00:00:00 2001
From: Shreyas Goenka <shreyas.goenka@databricks.com>
Date: Wed, 16 Oct 2024 13:36:55 +0200
Subject: [PATCH 33/39] -

---
 bundle/config/mutator/apply_presets.go  | 2 +-
 bundle/deploy/terraform/convert_test.go | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/bundle/config/mutator/apply_presets.go b/bundle/config/mutator/apply_presets.go
index 4d5a4f51..9d79835d 100644
--- a/bundle/config/mutator/apply_presets.go
+++ b/bundle/config/mutator/apply_presets.go
@@ -208,7 +208,7 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos
 		// We only have to check for ${resources.schemas...} references because any
 		// other valid reference (like ${var.foo}) would have been interpolated by this point.
 		if p, ok := dynvar.PureReferenceToPath(v.SchemaName); ok && p.HasPrefix(dyn.Path{dyn.Key("resources"), dyn.Key("schemas")}) {
-			log.Debugf(ctx, "Skipping prefix for volume %s because schema name %s is defined in the bundle and will be interpolated at runtime", v.Name, v.SchemaName)
+			log.Debugf(ctx, "Skipping prefix for volume %s because schema %s is defined in the bundle and the schema name will be interpolated at runtime", v.Name, v.SchemaName)
 			continue
 
 		}
diff --git a/bundle/deploy/terraform/convert_test.go b/bundle/deploy/terraform/convert_test.go
index bd36c84a..6808076b 100644
--- a/bundle/deploy/terraform/convert_test.go
+++ b/bundle/deploy/terraform/convert_test.go
@@ -1110,7 +1110,8 @@ func TestTerraformToBundleModifiedResources(t *testing.T) {
 					{Attributes: stateInstanceAttributes{ID: "2"}},
 				},
 			},
-			{Type: "databricks_cluster",
+			{
+				Type: "databricks_cluster",
 				Mode: "managed",
 				Name: "test_cluster_old",
 				Instances: []stateResourceInstance{

From 1a961eb19c71c0ef33483428d97b701e7bff6458 Mon Sep 17 00:00:00 2001
From: Shreyas Goenka <shreyas.goenka@databricks.com>
Date: Wed, 16 Oct 2024 13:46:45 +0200
Subject: [PATCH 34/39] -

---
 .../bundle/bundles/uc_volume/databricks_template_schema.json    | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/internal/bundle/bundles/uc_volume/databricks_template_schema.json b/internal/bundle/bundles/uc_volume/databricks_template_schema.json
index 762f4470..d849dacf 100644
--- a/internal/bundle/bundles/uc_volume/databricks_template_schema.json
+++ b/internal/bundle/bundles/uc_volume/databricks_template_schema.json
@@ -2,7 +2,7 @@
     "properties": {
         "unique_id": {
             "type": "string",
-            "description": "Unique ID for the schema and pipeline names"
+            "description": "Unique ID for the schema names"
         }
     }
 }

From e32ebd0b480fe8c3a506df04ac079d359312962c Mon Sep 17 00:00:00 2001
From: Shreyas Goenka <shreyas.goenka@databricks.com>
Date: Thu, 31 Oct 2024 15:24:51 +0100
Subject: [PATCH 35/39] use IsVolumesPath

---
 bundle/libraries/filer.go | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/bundle/libraries/filer.go b/bundle/libraries/filer.go
index 74f8175d..8cbf1a51 100644
--- a/bundle/libraries/filer.go
+++ b/bundle/libraries/filer.go
@@ -2,7 +2,6 @@ package libraries
 
 import (
 	"context"
-	"strings"
 
 	"github.com/databricks/cli/bundle"
 	"github.com/databricks/cli/libs/diag"
@@ -20,7 +19,7 @@ func GetFilerForLibraries(ctx context.Context, b *bundle.Bundle) (filer.Filer, s
 	}
 
 	switch {
-	case strings.HasPrefix(artifactPath, "/Volumes/"):
+	case IsVolumesPath(artifactPath):
 		return filerForVolume(ctx, b)
 
 	default:

From 6b122348adf1726a7ca7fc11d53615386ce5f671 Mon Sep 17 00:00:00 2001
From: Shreyas Goenka <shreyas.goenka@databricks.com>
Date: Thu, 31 Oct 2024 15:57:45 +0100
Subject: [PATCH 36/39] better message

---
 bundle/phases/deploy.go        | 7 ++++---
 internal/bundle/deploy_test.go | 7 ++++---
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/bundle/phases/deploy.go b/bundle/phases/deploy.go
index 28b78f28..f5fe42b0 100644
--- a/bundle/phases/deploy.go
+++ b/bundle/phases/deploy.go
@@ -96,9 +96,10 @@ properties such as the 'catalog' or 'storage' are changed:`
 	// One or more volumes is being recreated.
 	if len(volumeActions) != 0 {
 		msg := `
-This action will result in the deletion or recreation of the following Volumes. For managed volumes,
-this typically results in a deletion of the upstream data in the cloud tenant in ~30 days. For external
-volumes the upstream data in the cloud tenant is not affected:`
+This action will result in the deletion or recreation of the following Volumes.
+For managed volumes, the files stored in the volume are also deleted from your
+cloud tenant within 30 days. For external volumes, the metadata about the volume
+is removed from the catalog, but the underlying files are not deleted:`
 		cmdio.LogString(ctx, msg)
 		for _, action := range volumeActions {
 			cmdio.Log(ctx, action)
diff --git a/internal/bundle/deploy_test.go b/internal/bundle/deploy_test.go
index 9399c648..1956139a 100644
--- a/internal/bundle/deploy_test.go
+++ b/internal/bundle/deploy_test.go
@@ -284,9 +284,10 @@ func TestAccDeployUcVolume(t *testing.T) {
 	t.Setenv("BUNDLE_ROOT", bundleRoot)
 	stdout, stderr, err := internal.NewCobraTestRunnerWithContext(t, ctx, "bundle", "deploy", "--var=schema_name=${resources.schemas.schema2.name}").Run()
 	assert.Error(t, err)
-	assert.Contains(t, stderr.String(), `This action will result in the deletion or recreation of the following Volumes. For managed volumes,
-this typically results in a deletion of the upstream data in the cloud tenant in ~30 days. For external
-volumes the upstream data in the cloud tenant is not affected:
+	assert.Contains(t, stderr.String(), `This action will result in the deletion or recreation of the following Volumes.
+For managed volumes, the files stored in the volume are also deleted from your
+cloud tenant within 30 days. For external volumes, the metadata about the volume
+is removed from the catalog, but the underlying files are not deleted:
   recreate volume foo`)
 	assert.Contains(t, stdout.String(), "the deployment requires destructive actions, but current console does not support prompting. Please specify --auto-approve if you would like to skip prompts and proceed")
 

From f9287e01015719c5fd8b6d17794bbb099ff9b474 Mon Sep 17 00:00:00 2001
From: Shreyas Goenka <shreyas.goenka@databricks.com>
Date: Thu, 31 Oct 2024 17:52:45 +0100
Subject: [PATCH 37/39] address comments

---
 bundle/artifacts/expand_globs_test.go         |  2 +-
 bundle/{internal => }/bundletest/location.go  |  0
 bundle/{internal => }/bundletest/mutate.go    |  0
 .../configure_dashboard_defaults_test.go      |  2 +-
 .../expand_pipeline_glob_paths_test.go        |  2 +-
 .../config/mutator/rewrite_sync_paths_test.go |  2 +-
 bundle/config/mutator/sync_infer_root_test.go |  2 +-
 bundle/config/mutator/translate_paths_test.go |  2 +-
 bundle/deploy/metadata/compute_test.go        |  2 +-
 .../libraries/expand_glob_references_test.go  |  2 +-
 bundle/libraries/filer_volume_test.go         |  2 +-
 internal/bundle/artifacts_test.go             | 22 +++++++++----------
 12 files changed, 20 insertions(+), 20 deletions(-)
 rename bundle/{internal => }/bundletest/location.go (100%)
 rename bundle/{internal => }/bundletest/mutate.go (100%)

diff --git a/bundle/artifacts/expand_globs_test.go b/bundle/artifacts/expand_globs_test.go
index dc7c77de..af048f8e 100644
--- a/bundle/artifacts/expand_globs_test.go
+++ b/bundle/artifacts/expand_globs_test.go
@@ -7,8 +7,8 @@ import (
 	"testing"
 
 	"github.com/databricks/cli/bundle"
+	"github.com/databricks/cli/bundle/bundletest"
 	"github.com/databricks/cli/bundle/config"
-	"github.com/databricks/cli/bundle/internal/bundletest"
 	"github.com/databricks/cli/internal/testutil"
 	"github.com/databricks/cli/libs/dyn"
 	"github.com/stretchr/testify/assert"
diff --git a/bundle/internal/bundletest/location.go b/bundle/bundletest/location.go
similarity index 100%
rename from bundle/internal/bundletest/location.go
rename to bundle/bundletest/location.go
diff --git a/bundle/internal/bundletest/mutate.go b/bundle/bundletest/mutate.go
similarity index 100%
rename from bundle/internal/bundletest/mutate.go
rename to bundle/bundletest/mutate.go
diff --git a/bundle/config/mutator/configure_dashboard_defaults_test.go b/bundle/config/mutator/configure_dashboard_defaults_test.go
index 4804b715..d34c35bc 100644
--- a/bundle/config/mutator/configure_dashboard_defaults_test.go
+++ b/bundle/config/mutator/configure_dashboard_defaults_test.go
@@ -5,10 +5,10 @@ import (
 	"testing"
 
 	"github.com/databricks/cli/bundle"
+	"github.com/databricks/cli/bundle/bundletest"
 	"github.com/databricks/cli/bundle/config"
 	"github.com/databricks/cli/bundle/config/mutator"
 	"github.com/databricks/cli/bundle/config/resources"
-	"github.com/databricks/cli/bundle/internal/bundletest"
 	"github.com/databricks/cli/libs/dyn"
 	"github.com/databricks/databricks-sdk-go/service/dashboards"
 	"github.com/stretchr/testify/assert"
diff --git a/bundle/config/mutator/expand_pipeline_glob_paths_test.go b/bundle/config/mutator/expand_pipeline_glob_paths_test.go
index 9f70b74a..063c76b3 100644
--- a/bundle/config/mutator/expand_pipeline_glob_paths_test.go
+++ b/bundle/config/mutator/expand_pipeline_glob_paths_test.go
@@ -7,9 +7,9 @@ import (
 	"testing"
 
 	"github.com/databricks/cli/bundle"
+	"github.com/databricks/cli/bundle/bundletest"
 	"github.com/databricks/cli/bundle/config"
 	"github.com/databricks/cli/bundle/config/resources"
-	"github.com/databricks/cli/bundle/internal/bundletest"
 	"github.com/databricks/cli/libs/dyn"
 	"github.com/databricks/databricks-sdk-go/service/compute"
 	"github.com/databricks/databricks-sdk-go/service/pipelines"
diff --git a/bundle/config/mutator/rewrite_sync_paths_test.go b/bundle/config/mutator/rewrite_sync_paths_test.go
index 0e4dfc4c..90da8792 100644
--- a/bundle/config/mutator/rewrite_sync_paths_test.go
+++ b/bundle/config/mutator/rewrite_sync_paths_test.go
@@ -6,9 +6,9 @@ import (
 	"testing"
 
 	"github.com/databricks/cli/bundle"
+	"github.com/databricks/cli/bundle/bundletest"
 	"github.com/databricks/cli/bundle/config"
 	"github.com/databricks/cli/bundle/config/mutator"
-	"github.com/databricks/cli/bundle/internal/bundletest"
 	"github.com/databricks/cli/libs/dyn"
 	"github.com/stretchr/testify/assert"
 )
diff --git a/bundle/config/mutator/sync_infer_root_test.go b/bundle/config/mutator/sync_infer_root_test.go
index f507cbc7..98c9d362 100644
--- a/bundle/config/mutator/sync_infer_root_test.go
+++ b/bundle/config/mutator/sync_infer_root_test.go
@@ -6,9 +6,9 @@ import (
 	"testing"
 
 	"github.com/databricks/cli/bundle"
+	"github.com/databricks/cli/bundle/bundletest"
 	"github.com/databricks/cli/bundle/config"
 	"github.com/databricks/cli/bundle/config/mutator"
-	"github.com/databricks/cli/bundle/internal/bundletest"
 	"github.com/databricks/cli/libs/dyn"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
diff --git a/bundle/config/mutator/translate_paths_test.go b/bundle/config/mutator/translate_paths_test.go
index 9d655b27..04325b68 100644
--- a/bundle/config/mutator/translate_paths_test.go
+++ b/bundle/config/mutator/translate_paths_test.go
@@ -8,11 +8,11 @@ import (
 	"testing"
 
 	"github.com/databricks/cli/bundle"
+	"github.com/databricks/cli/bundle/bundletest"
 	"github.com/databricks/cli/bundle/config"
 	"github.com/databricks/cli/bundle/config/mutator"
 	"github.com/databricks/cli/bundle/config/resources"
 	"github.com/databricks/cli/bundle/config/variable"
-	"github.com/databricks/cli/bundle/internal/bundletest"
 	"github.com/databricks/cli/libs/diag"
 	"github.com/databricks/cli/libs/dyn"
 	"github.com/databricks/cli/libs/vfs"
diff --git a/bundle/deploy/metadata/compute_test.go b/bundle/deploy/metadata/compute_test.go
index 2c2c7237..2d852bf7 100644
--- a/bundle/deploy/metadata/compute_test.go
+++ b/bundle/deploy/metadata/compute_test.go
@@ -5,9 +5,9 @@ import (
 	"testing"
 
 	"github.com/databricks/cli/bundle"
+	"github.com/databricks/cli/bundle/bundletest"
 	"github.com/databricks/cli/bundle/config"
 	"github.com/databricks/cli/bundle/config/resources"
-	"github.com/databricks/cli/bundle/internal/bundletest"
 	"github.com/databricks/cli/bundle/metadata"
 	"github.com/databricks/cli/libs/dyn"
 	"github.com/databricks/databricks-sdk-go/service/jobs"
diff --git a/bundle/libraries/expand_glob_references_test.go b/bundle/libraries/expand_glob_references_test.go
index 2dfbddb7..bdd04216 100644
--- a/bundle/libraries/expand_glob_references_test.go
+++ b/bundle/libraries/expand_glob_references_test.go
@@ -6,9 +6,9 @@ import (
 	"testing"
 
 	"github.com/databricks/cli/bundle"
+	"github.com/databricks/cli/bundle/bundletest"
 	"github.com/databricks/cli/bundle/config"
 	"github.com/databricks/cli/bundle/config/resources"
-	"github.com/databricks/cli/bundle/internal/bundletest"
 	"github.com/databricks/cli/internal/testutil"
 	"github.com/databricks/cli/libs/dyn"
 	"github.com/databricks/databricks-sdk-go/service/compute"
diff --git a/bundle/libraries/filer_volume_test.go b/bundle/libraries/filer_volume_test.go
index 047a009a..987c57cb 100644
--- a/bundle/libraries/filer_volume_test.go
+++ b/bundle/libraries/filer_volume_test.go
@@ -7,9 +7,9 @@ import (
 	"testing"
 
 	"github.com/databricks/cli/bundle"
+	"github.com/databricks/cli/bundle/bundletest"
 	"github.com/databricks/cli/bundle/config"
 	"github.com/databricks/cli/bundle/config/resources"
-	"github.com/databricks/cli/bundle/internal/bundletest"
 	"github.com/databricks/cli/libs/diag"
 	"github.com/databricks/cli/libs/dyn"
 	"github.com/databricks/cli/libs/filer"
diff --git a/internal/bundle/artifacts_test.go b/internal/bundle/artifacts_test.go
index d2938fa7..3cc731f4 100644
--- a/internal/bundle/artifacts_test.go
+++ b/internal/bundle/artifacts_test.go
@@ -9,6 +9,7 @@ import (
 	"testing"
 
 	"github.com/databricks/cli/bundle"
+	"github.com/databricks/cli/bundle/bundletest"
 	"github.com/databricks/cli/bundle/config"
 	"github.com/databricks/cli/bundle/config/resources"
 	"github.com/databricks/cli/bundle/libraries"
@@ -244,6 +245,11 @@ func TestAccUploadArtifactFileToInvalidVolume(t *testing.T) {
 	})
 	require.NoError(t, err)
 
+	t.Cleanup(func() {
+		err = w.Schemas.DeleteByFullName(ctx, "main."+schemaName)
+		require.NoError(t, err)
+	})
+
 	t.Run("volume not in DAB", func(t *testing.T) {
 		volumePath := fmt.Sprintf("/Volumes/main/%s/doesnotexist", schemaName)
 		dir := t.TempDir()
@@ -307,17 +313,11 @@ func TestAccUploadArtifactFileToInvalidVolume(t *testing.T) {
 		}
 
 		// set location of volume definition in config.
-		b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) {
-			return dyn.Map(v, "resources.volumes.foo", func(p dyn.Path, volume dyn.Value) (dyn.Value, error) {
-				return volume.WithLocations([]dyn.Location{
-					{
-						File:   filepath.Join(dir, "databricks.yml"),
-						Line:   1,
-						Column: 2,
-					},
-				}), nil
-			})
-		})
+		bundletest.SetLocation(b, "resources.volumes.foo", []dyn.Location{{
+			File:   filepath.Join(dir, "databricks.yml"),
+			Line:   1,
+			Column: 2,
+		}})
 
 		diags := bundle.Apply(ctx, b, bundle.Seq(libraries.ExpandGlobReferences(), libraries.Upload()))
 		assert.Contains(t, diags, diag.Diagnostic{

From 250d4265ceac562b8a41870c38164c311a8ee553 Mon Sep 17 00:00:00 2001
From: Shreyas Goenka <shreyas.goenka@databricks.com>
Date: Thu, 31 Oct 2024 18:00:31 +0100
Subject: [PATCH 38/39] rename to volume

---
 .../{uc_volume => volume}/databricks_template_schema.json       | 0
 .../bundles/{uc_volume => volume}/template/databricks.yml.tmpl  | 0
 internal/bundle/bundles/{uc_volume => volume}/template/nb.sql   | 0
 internal/bundle/deploy_test.go                                  | 2 +-
 4 files changed, 1 insertion(+), 1 deletion(-)
 rename internal/bundle/bundles/{uc_volume => volume}/databricks_template_schema.json (100%)
 rename internal/bundle/bundles/{uc_volume => volume}/template/databricks.yml.tmpl (100%)
 rename internal/bundle/bundles/{uc_volume => volume}/template/nb.sql (100%)

diff --git a/internal/bundle/bundles/uc_volume/databricks_template_schema.json b/internal/bundle/bundles/volume/databricks_template_schema.json
similarity index 100%
rename from internal/bundle/bundles/uc_volume/databricks_template_schema.json
rename to internal/bundle/bundles/volume/databricks_template_schema.json
diff --git a/internal/bundle/bundles/uc_volume/template/databricks.yml.tmpl b/internal/bundle/bundles/volume/template/databricks.yml.tmpl
similarity index 100%
rename from internal/bundle/bundles/uc_volume/template/databricks.yml.tmpl
rename to internal/bundle/bundles/volume/template/databricks.yml.tmpl
diff --git a/internal/bundle/bundles/uc_volume/template/nb.sql b/internal/bundle/bundles/volume/template/nb.sql
similarity index 100%
rename from internal/bundle/bundles/uc_volume/template/nb.sql
rename to internal/bundle/bundles/volume/template/nb.sql
diff --git a/internal/bundle/deploy_test.go b/internal/bundle/deploy_test.go
index 1956139a..0ddc79f3 100644
--- a/internal/bundle/deploy_test.go
+++ b/internal/bundle/deploy_test.go
@@ -249,7 +249,7 @@ func TestAccDeployUcVolume(t *testing.T) {
 	w := wt.W
 
 	uniqueId := uuid.New().String()
-	bundleRoot, err := initTestTemplate(t, ctx, "uc_volume", map[string]any{
+	bundleRoot, err := initTestTemplate(t, ctx, "volume", map[string]any{
 		"unique_id": uniqueId,
 	})
 	require.NoError(t, err)

From 1218178e64844a091fb06293b6fc14552273942b Mon Sep 17 00:00:00 2001
From: Shreyas Goenka <shreyas.goenka@databricks.com>
Date: Thu, 31 Oct 2024 18:01:37 +0100
Subject: [PATCH 39/39] -

---
 bundle/config/mutator/apply_presets.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bundle/config/mutator/apply_presets.go b/bundle/config/mutator/apply_presets.go
index ffe9fc93..2b60f52e 100644
--- a/bundle/config/mutator/apply_presets.go
+++ b/bundle/config/mutator/apply_presets.go
@@ -195,7 +195,7 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos
 		// the Databricks UI and via the SQL API.
 	}
 
-	// Apply the prefix to volumes
+	// Volumes: Prefix
 	for _, v := range r.Volumes {
 		if containsUserIdentity(v.CatalogName, b.Config.Workspace.CurrentUser) {
 			log.Debugf(ctx, "Skipping prefix for volume %s because catalog %s contains the current user's identity", v.Name, v.CatalogName)