Skip to content

Commit d795f40

Browse files
authored
direct: Fix spurious recreate of external volumes with trailing-slash storage_location (#5145)
The UC API strips trailing slashes from storage_location on create, causing the direct engine to detect drift on every subsequent plan and trigger a full delete+create cycle. Fix: add OverrideChangeDesc to ResourceVolume that treats storage_location values differing only by trailing slashes as equivalent (alias), matching the Terraform provider's suppressLocationDiff behavior. Also update the testserver to accept storage_location on EXTERNAL volumes and strip the trailing slash, mirroring UC API behavior so the bug and fix are reproducible locally. Add an invariant no_drift test for external volumes. Co-authored-by: Isaac
1 parent e5fb6e4 commit d795f40

8 files changed

Lines changed: 63 additions & 5 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
bundle:
2+
name: test-bundle-$UNIQUE_NAME
3+
4+
resources:
5+
volumes:
6+
foo:
7+
name: test-volume-$UNIQUE_NAME
8+
catalog_name: main
9+
schema_name: default
10+
volume_type: EXTERNAL
11+
storage_location: s3://test-bucket/path/

acceptance/bundle/invariant/continue_293/out.test.toml

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

acceptance/bundle/invariant/migrate/out.test.toml

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

acceptance/bundle/invariant/no_drift/out.test.toml

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

acceptance/bundle/invariant/test.toml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ EnvMatrix.INPUT_CONFIG = [
5757
"synced_database_table.yml.tmpl",
5858
"vector_search_endpoint.yml.tmpl",
5959
"volume.yml.tmpl",
60+
"volume_external.yml.tmpl",
6061
]
6162

6263
[EnvMatrixExclude]
@@ -71,6 +72,9 @@ no_postgres_endpoint_on_cloud = ["CONFIG_Cloud=true", "INPUT_CONFIG=postgres_end
7172
# which are environment-specific, so we only test locally with the mock server
7273
no_external_location_on_cloud = ["CONFIG_Cloud=true", "INPUT_CONFIG=external_location.yml.tmpl"]
7374

75+
# External volumes reference external locations; excluded from cloud for the same reason
76+
no_external_volume_on_cloud = ["CONFIG_Cloud=true", "INPUT_CONFIG=volume_external.yml.tmpl"]
77+
7478
# Fake SQL endpoint for local tests
7579
[[Server]]
7680
Pattern = "POST /api/2.0/sql/statements/"

bundle/deployplan/plan.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ type ChangeDesc struct {
100100
const (
101101
ReasonBackendDefault = "backend_default"
102102
ReasonAlias = "alias"
103+
ReasonURLNormalization = "url_normalization"
103104
ReasonRemoteAlreadySet = "remote_already_set"
104105
ReasonEmpty = "empty"
105106
ReasonCustom = "custom"

bundle/direct/dresources/volume.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@ import (
66
"strings"
77

88
"github.com/databricks/cli/bundle/config/resources"
9+
"github.com/databricks/cli/bundle/deployplan"
910
"github.com/databricks/cli/libs/log"
11+
"github.com/databricks/cli/libs/structs/structpath"
1012
"github.com/databricks/cli/libs/utils"
1113
"github.com/databricks/databricks-sdk-go"
1214
"github.com/databricks/databricks-sdk-go/service/catalog"
@@ -112,6 +114,35 @@ func (r *ResourceVolume) DoDelete(ctx context.Context, id string) error {
112114
return r.client.Volumes.DeleteByName(ctx, id)
113115
}
114116

117+
// OverrideChangeDesc suppresses drift for storage_location when the only difference
118+
// is a trailing slash. The UC API strips trailing slashes on create, so remote returns
119+
// "s3://bucket/path" while the config may have "s3://bucket/path/".
120+
//
121+
// This matches the Terraform provider's suppressLocationDiff behavior.
122+
// https://github.com/databricks/terraform-provider-databricks/blob/v1.65.1/catalog/resource_volume.go#L25
123+
func (*ResourceVolume) OverrideChangeDesc(_ context.Context, path *structpath.PathNode, change *ChangeDesc, _ *catalog.VolumeInfo) error {
124+
if change.Action == deployplan.Skip {
125+
return nil
126+
}
127+
128+
if path.String() != "storage_location" {
129+
return nil
130+
}
131+
132+
newStr, newOk := change.New.(string)
133+
remoteStr, remoteOk := change.Remote.(string)
134+
if !newOk || !remoteOk {
135+
return nil
136+
}
137+
138+
if newStr != remoteStr && strings.TrimRight(newStr, "/") == strings.TrimRight(remoteStr, "/") {
139+
change.Action = deployplan.Skip
140+
change.Reason = deployplan.ReasonURLNormalization
141+
}
142+
143+
return nil
144+
}
145+
115146
func getNameFromID(id string) (string, error) {
116147
items := strings.Split(id, ".")
117148
if len(items) == 0 {

libs/testserver/volumes.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"encoding/json"
55
"fmt"
66
"net/http"
7+
"strings"
78

89
"github.com/databricks/databricks-sdk-go/service/catalog"
910
"github.com/databricks/databricks-sdk-go/service/workspace"
@@ -21,7 +22,7 @@ func (s *FakeWorkspace) VolumesCreate(req Request) Response {
2122

2223
volume.FullName = volume.CatalogName + "." + volume.SchemaName + "." + volume.Name
2324

24-
if volume.StorageLocation != "" {
25+
if volume.StorageLocation != "" && volume.VolumeType != catalog.VolumeTypeExternal {
2526
return Response{
2627
StatusCode: 400,
2728
Body: map[string]string{
@@ -30,8 +31,15 @@ func (s *FakeWorkspace) VolumesCreate(req Request) Response {
3031
},
3132
}
3233
}
34+
3335
volume.VolumeId = nextUUID()
34-
volume.StorageLocation = fmt.Sprintf("s3://%s/metastore/%s/volumes/%s", testMetastoreName, TestMetastore.MetastoreId, volume.VolumeId)
36+
37+
if volume.StorageLocation != "" {
38+
// Strip trailing slash to mimic UC API normalization behavior.
39+
volume.StorageLocation = strings.TrimRight(volume.StorageLocation, "/")
40+
} else {
41+
volume.StorageLocation = fmt.Sprintf("s3://%s/metastore/%s/volumes/%s", testMetastoreName, TestMetastore.MetastoreId, volume.VolumeId)
42+
}
3543

3644
volume.CreatedAt = nowMilli()
3745
volume.UpdatedAt = volume.CreatedAt

0 commit comments

Comments
 (0)