diff --git a/bundle/config/validate/validate_artifact_path.go b/bundle/config/validate/validate_artifact_path.go index f6b33a83c..ab6a733a3 100644 --- a/bundle/config/validate/validate_artifact_path.go +++ b/bundle/config/validate/validate_artifact_path.go @@ -8,9 +8,11 @@ import ( "strings" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/libraries" "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/dyn/dynvar" "github.com/databricks/databricks-sdk-go/apierr" "github.com/databricks/databricks-sdk-go/service/catalog" ) @@ -50,6 +52,29 @@ func extractVolumeFromPath(artifactPath string) (string, string, string, error) return catalogName, schemaName, volumeName, nil } +func findVolumeInBundle(r config.Root, catalogName, schemaName, volumeName string) (dyn.Path, []dyn.Location, bool) { + volumes := r.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.} 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. + // 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) + return dyn.MustPathFromString(pathString), r.GetLocations(pathString), true + } + return nil, nil, false +} + func (v *validateArtifactPath) Apply(ctx context.Context, rb bundle.ReadOnlyBundle) diag.Diagnostics { // We only validate UC Volumes paths right now. if !libraries.IsVolumesPath(rb.Config().Workspace.ArtifactPath) { @@ -79,7 +104,7 @@ func (v *validateArtifactPath) Apply(ctx context.Context, rb bundle.ReadOnlyBund return wrapErrorMsg(fmt.Sprintf("cannot access volume %s: %s", volumeFullName, err)) } if errors.Is(err, apierr.ErrNotFound) { - path, locations, ok := libraries.FindVolumeInBundle(rb.Config(), catalogName, schemaName, volumeName) + path, locations, ok := findVolumeInBundle(rb.Config(), catalogName, schemaName, volumeName) if !ok { return wrapErrorMsg(fmt.Sprintf("volume %s does not exist", volumeFullName)) } diff --git a/bundle/config/validate/validate_artifact_path_test.go b/bundle/config/validate/validate_artifact_path_test.go index 76e347513..a7a033156 100644 --- a/bundle/config/validate/validate_artifact_path_test.go +++ b/bundle/config/validate/validate_artifact_path_test.go @@ -207,3 +207,68 @@ func TestValidateArtifactPathWithInvalidPaths(t *testing.T) { }}) } } + +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.Config, "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.Config, "main", "my_schema", "doesnotexist") + assert.False(t, ok) + + // wrong schema name + _, _, ok = findVolumeInBundle(b.Config, "main", "doesnotexist", "my_volume") + assert.False(t, ok) + + // wrong catalog name + _, _, ok = findVolumeInBundle(b.Config, "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.Config, "main", "my_schema", "my_volume") + assert.False(t, ok) + + // schema name is interpolated. + b.Config.Resources.Volumes["foo"].SchemaName = "${resources.schemas.my_schema.name}" + path, locations, ok = findVolumeInBundle(b.Config, "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) +} diff --git a/bundle/libraries/filer_volume.go b/bundle/libraries/filer_volume.go index e356d9b82..4ef574628 100644 --- a/bundle/libraries/filer_volume.go +++ b/bundle/libraries/filer_volume.go @@ -2,14 +2,10 @@ package libraries import ( "context" - "fmt" "path" "github.com/databricks/cli/bundle" - "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/filer" ) @@ -19,26 +15,3 @@ func filerForVolume(ctx context.Context, b *bundle.Bundle) (filer.Filer, string, f, err := filer.NewFilesClient(w, uploadPath) return f, uploadPath, diag.FromErr(err) } - -func FindVolumeInBundle(r config.Root, catalogName, schemaName, volumeName string) (dyn.Path, []dyn.Location, bool) { - volumes := r.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.} 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. - // 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) - return dyn.MustPathFromString(pathString), r.GetLocations(pathString), true - } - return nil, nil, false -} diff --git a/bundle/libraries/filer_volume_test.go b/bundle/libraries/filer_volume_test.go index c788ac919..ee3523737 100644 --- a/bundle/libraries/filer_volume_test.go +++ b/bundle/libraries/filer_volume_test.go @@ -22,71 +22,6 @@ import ( "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.Config, "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.Config, "main", "my_schema", "doesnotexist") - assert.False(t, ok) - - // wrong schema name - _, _, ok = FindVolumeInBundle(b.Config, "main", "doesnotexist", "my_volume") - assert.False(t, ok) - - // wrong catalog name - _, _, ok = FindVolumeInBundle(b.Config, "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.Config, "main", "my_schema", "my_volume") - assert.False(t, ok) - - // schema name is interpolated. - b.Config.Resources.Volumes["foo"].SchemaName = "${resources.schemas.my_schema.name}" - path, locations, ok = FindVolumeInBundle(b.Config, "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 TestFilerForVolumeForErrorFromAPI(t *testing.T) { b := &bundle.Bundle{ Config: config.Root{