Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions acceptance/bundle/invariant/configs/volume_external.yml.tmpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
bundle:
name: test-bundle-$UNIQUE_NAME

resources:
volumes:
foo:
name: test-volume-$UNIQUE_NAME
catalog_name: main
schema_name: default
volume_type: EXTERNAL
storage_location: s3://test-bucket/path/

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider multiple / as suffix to ensure TrimRight does the expected thing

3 changes: 2 additions & 1 deletion acceptance/bundle/invariant/continue_293/out.test.toml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion acceptance/bundle/invariant/migrate/out.test.toml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion acceptance/bundle/invariant/no_drift/out.test.toml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions acceptance/bundle/invariant/test.toml
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ EnvMatrix.INPUT_CONFIG = [
"synced_database_table.yml.tmpl",
"vector_search_endpoint.yml.tmpl",
"volume.yml.tmpl",
"volume_external.yml.tmpl",
]

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

# External volumes reference external locations; excluded from cloud for the same reason
no_external_volume_on_cloud = ["CONFIG_Cloud=true", "INPUT_CONFIG=volume_external.yml.tmpl"]

# Fake SQL endpoint for local tests
[[Server]]
Pattern = "POST /api/2.0/sql/statements/"
Expand Down
1 change: 1 addition & 0 deletions bundle/deployplan/plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ type ChangeDesc struct {
const (
ReasonBackendDefault = "backend_default"
ReasonAlias = "alias"
ReasonURLNormalization = "url_normalization"
ReasonRemoteAlreadySet = "remote_already_set"
ReasonEmpty = "empty"
ReasonCustom = "custom"
Expand Down
31 changes: 31 additions & 0 deletions bundle/direct/dresources/volume.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import (
"strings"

"github.com/databricks/cli/bundle/config/resources"
"github.com/databricks/cli/bundle/deployplan"
"github.com/databricks/cli/libs/log"
"github.com/databricks/cli/libs/structs/structpath"
"github.com/databricks/cli/libs/utils"
"github.com/databricks/databricks-sdk-go"
"github.com/databricks/databricks-sdk-go/service/catalog"
Expand Down Expand Up @@ -112,6 +114,35 @@ func (r *ResourceVolume) DoDelete(ctx context.Context, id string) error {
return r.client.Volumes.DeleteByName(ctx, id)
}

// OverrideChangeDesc suppresses drift for storage_location when the only difference
// is a trailing slash. The UC API strips trailing slashes on create, so remote returns
// "s3://bucket/path" while the config may have "s3://bucket/path/".
//
// This matches the Terraform provider's suppressLocationDiff behavior.
// https://github.com/databricks/terraform-provider-databricks/blob/v1.65.1/catalog/resource_volume.go#L25
func (*ResourceVolume) OverrideChangeDesc(_ context.Context, path *structpath.PathNode, change *ChangeDesc, _ *catalog.VolumeInfo) error {
if change.Action == deployplan.Skip {
return nil
}

if path.String() != "storage_location" {
return nil
}

newStr, newOk := change.New.(string)
remoteStr, remoteOk := change.Remote.(string)
if !newOk || !remoteOk {
return nil
}

if newStr != remoteStr && strings.TrimRight(newStr, "/") == strings.TrimRight(remoteStr, "/") {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we store the storage_location in the state without the slash instead?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so, state is tracking the config. If user has slash in the config, we do that as well.

Normalized comparison is more explicit.

change.Action = deployplan.Skip
change.Reason = deployplan.ReasonURLNormalization
}

return nil
}

func getNameFromID(id string) (string, error) {
items := strings.Split(id, ".")
if len(items) == 0 {
Expand Down
12 changes: 10 additions & 2 deletions libs/testserver/volumes.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"encoding/json"
"fmt"
"net/http"
"strings"

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

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

if volume.StorageLocation != "" {
if volume.StorageLocation != "" && volume.VolumeType != catalog.VolumeTypeExternal {
return Response{
StatusCode: 400,
Body: map[string]string{
Expand All @@ -30,8 +31,15 @@ func (s *FakeWorkspace) VolumesCreate(req Request) Response {
},
}
}

volume.VolumeId = nextUUID()
volume.StorageLocation = fmt.Sprintf("s3://%s/metastore/%s/volumes/%s", testMetastoreName, TestMetastore.MetastoreId, volume.VolumeId)

if volume.StorageLocation != "" {
// Strip trailing slash to mimic UC API normalization behavior.
volume.StorageLocation = strings.TrimRight(volume.StorageLocation, "/")
} else {
volume.StorageLocation = fmt.Sprintf("s3://%s/metastore/%s/volumes/%s", testMetastoreName, TestMetastore.MetastoreId, volume.VolumeId)
}

volume.CreatedAt = nowMilli()
volume.UpdatedAt = volume.CreatedAt
Expand Down
Loading