Added validation for usage of /Workspace pattern

This commit is contained in:
Andrew Nester 2024-08-28 14:00:47 +02:00
parent 1862b731d9
commit c842eb3c12
No known key found for this signature in database
GPG Key ID: 12BC628A44B7DA57
11 changed files with 148 additions and 17 deletions

View File

@ -24,7 +24,6 @@ func (m *prependWorkspacePrefix) Name() string {
var skipPrefixes = []string{ var skipPrefixes = []string{
"/Workspace", "/Workspace",
"/Volumes", "/Volumes",
"/Shared",
} }
func (m *prependWorkspacePrefix) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { func (m *prependWorkspacePrefix) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
@ -44,12 +43,6 @@ func (m *prependWorkspacePrefix) Apply(ctx context.Context, b *bundle.Bundle) di
return dyn.InvalidValue, fmt.Errorf("expected string, got %s", v.Kind()) return dyn.InvalidValue, fmt.Errorf("expected string, got %s", v.Kind())
} }
for _, prefix := range skipPrefixes {
if path == prefix {
return pv, nil
}
}
for _, prefix := range skipPrefixes { for _, prefix := range skipPrefixes {
if strings.HasPrefix(path, prefix) { if strings.HasPrefix(path, prefix) {
return pv, nil return pv, nil

View File

@ -21,7 +21,7 @@ func TestPrependWorkspacePrefix(t *testing.T) {
}, },
{ {
path: "/Shared/test", path: "/Shared/test",
expected: "/Shared/test", expected: "/Workspace/Shared/test",
}, },
{ {
path: "/Workspace/Users/test", path: "/Workspace/Users/test",

View File

@ -0,0 +1,58 @@
package validate
import (
"context"
"fmt"
"strings"
"github.com/databricks/cli/bundle"
"github.com/databricks/cli/libs/diag"
"github.com/databricks/cli/libs/dyn"
)
type noWorkspacePrefixUsed struct{}
// NoWorkspacePrefixUsed ensures that no workspace prefix plus workspace path variable is used in the remote path.
func NoWorkspacePrefixUsed() bundle.Mutator {
return &noWorkspacePrefixUsed{}
}
func (m *noWorkspacePrefixUsed) Name() string {
return "validate:no_workspace_prefix_used"
}
func (m *noWorkspacePrefixUsed) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
diags := diag.Diagnostics{}
prefixes := []string{
"/Workspace/${workspace.root_path}",
"/Workspace/${workspace.file_path}",
"/Workspace/${workspace.artifact_path}",
"/Workspace/${workspace.state_path}",
}
_, err := dyn.Walk(b.Config.Value(), func(p dyn.Path, v dyn.Value) (dyn.Value, error) {
vv, ok := v.AsString()
if !ok {
return v, nil
}
for _, prefix := range prefixes {
if strings.HasPrefix(vv, prefix) {
diags = append(diags, diag.Diagnostic{
Severity: diag.Error,
Summary: fmt.Sprintf("%s used in the remote path %s. Please change to use %s instead", prefix, vv, strings.ReplaceAll(vv, "/Workspace/", "")),
Locations: v.Locations(),
Paths: []dyn.Path{p},
})
}
}
return v, nil
})
if err != nil {
return diag.FromErr(err)
}
return diags
}

View File

@ -0,0 +1,78 @@
package validate
import (
"context"
"testing"
"github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/config"
"github.com/databricks/cli/bundle/config/resources"
"github.com/databricks/cli/libs/diag"
"github.com/databricks/databricks-sdk-go/service/compute"
"github.com/databricks/databricks-sdk-go/service/jobs"
"github.com/stretchr/testify/require"
)
func TestNoWorkspacePrefixUsed(t *testing.T) {
b := &bundle.Bundle{
Config: config.Root{
Workspace: config.Workspace{
RootPath: "/Workspace/Users/test",
ArtifactPath: "/Workspace/Users/test/artifacts",
FilePath: "/Workspace/Users/test/files",
StatePath: "/Workspace/Users/test/state",
},
Resources: config.Resources{
Jobs: map[string]*resources.Job{
"test_job": {
JobSettings: &jobs.JobSettings{
Tasks: []jobs.Task{
{
SparkPythonTask: &jobs.SparkPythonTask{
PythonFile: "/Workspace/${workspace.root_path}/file1.py",
},
},
{
NotebookTask: &jobs.NotebookTask{
NotebookPath: "/Workspace/${workspace.file_path}/notebook1",
},
Libraries: []compute.Library{
{
Jar: "/Workspace/${workspace.artifact_path}/jar1.jar",
},
},
},
{
NotebookTask: &jobs.NotebookTask{
NotebookPath: "${workspace.file_path}/notebook2",
},
Libraries: []compute.Library{
{
Jar: "${workspace.artifact_path}/jar2.jar",
},
},
},
},
},
},
},
},
},
}
diags := bundle.Apply(context.Background(), b, NoWorkspacePrefixUsed())
require.Len(t, diags, 3)
expectedErrors := map[string]bool{
"/Workspace/${workspace.root_path} used in the remote path /Workspace/${workspace.root_path}/file1.py. Please change to use ${workspace.root_path}/file1.py instead": true,
"/Workspace/${workspace.file_path} used in the remote path /Workspace/${workspace.file_path}/notebook1. Please change to use ${workspace.file_path}/notebook1 instead": true,
"/Workspace/${workspace.artifact_path} used in the remote path /Workspace/${workspace.artifact_path}/jar1.jar. Please change to use ${workspace.artifact_path}/jar1.jar instead": true,
}
for _, d := range diags {
require.Equal(t, d.Severity, diag.Error)
require.Contains(t, expectedErrors, d.Summary)
delete(expectedErrors, d.Summary)
}
}

View File

@ -47,7 +47,7 @@ type Workspace struct {
// Remote workspace base path for deployment state, for artifacts, as synchronization target. // Remote workspace base path for deployment state, for artifacts, as synchronization target.
// This defaults to "~/.bundle/${bundle.name}/${bundle.target}" where "~" expands to // This defaults to "~/.bundle/${bundle.name}/${bundle.target}" where "~" expands to
// the current user's home directory in the workspace (e.g. `/Users/jane@doe.com`). // the current user's home directory in the workspace (e.g. `/Workspace/Users/jane@doe.com`).
RootPath string `json:"root_path,omitempty"` RootPath string `json:"root_path,omitempty"`
// Remote workspace path to synchronize local files to. // Remote workspace path to synchronize local files to.

View File

@ -45,6 +45,8 @@ func Initialize() bundle.Mutator {
mutator.DefineDefaultWorkspacePaths(), mutator.DefineDefaultWorkspacePaths(),
mutator.PrependWorkspacePrefix(), mutator.PrependWorkspacePrefix(),
validate.NoWorkspacePrefixUsed(),
mutator.SetVariables(), mutator.SetVariables(),
// Intentionally placed before ResolveVariableReferencesInLookup, ResolveResourceReferences, // Intentionally placed before ResolveVariableReferencesInLookup, ResolveResourceReferences,
// ResolveVariableReferencesInComplexVariables and ResolveVariableReferences. // ResolveVariableReferencesInComplexVariables and ResolveVariableReferences.

View File

@ -236,7 +236,7 @@ func TestAccDeployBasicBundleLogs(t *testing.T) {
stdout, stderr := blackBoxRun(t, root, "bundle", "deploy") stdout, stderr := blackBoxRun(t, root, "bundle", "deploy")
assert.Equal(t, strings.Join([]string{ assert.Equal(t, strings.Join([]string{
fmt.Sprintf("Uploading bundle files to /Users/%s/.bundle/%s/files...", currentUser.UserName, uniqueId), fmt.Sprintf("Uploading bundle files to /Workspace/Users/%s/.bundle/%s/files...", currentUser.UserName, uniqueId),
"Deploying resources...", "Deploying resources...",
"Updating deployment state...", "Updating deployment state...",
"Deployment complete!\n", "Deployment complete!\n",

View File

@ -24,8 +24,8 @@ targets:
mode: production mode: production
workspace: workspace:
host: {{workspace_host}} host: {{workspace_host}}
# We explicitly specify /Users/{{user_name}} to make sure we only have a single copy. # We explicitly specify /Workspace/Users/{{user_name}} to make sure we only have a single copy.
root_path: /Users/{{user_name}}/.bundle/${bundle.name}/${bundle.target} root_path: /Workspace/Users/{{user_name}}/.bundle/${bundle.name}/${bundle.target}
permissions: permissions:
- {{if is_service_principal}}service_principal{{else}}user{{end}}_name: {{user_name}} - {{if is_service_principal}}service_principal{{else}}user{{end}}_name: {{user_name}}
level: CAN_MANAGE level: CAN_MANAGE

View File

@ -21,8 +21,8 @@ targets:
mode: production mode: production
workspace: workspace:
host: {{workspace_host}} host: {{workspace_host}}
# We explicitly specify /Users/{{user_name}} to make sure we only have a single copy. # We explicitly specify /Workspace/Users/{{user_name}} to make sure we only have a single copy.
root_path: /Users/{{user_name}}/.bundle/${bundle.name}/${bundle.target} root_path: /Workspace/Users/{{user_name}}/.bundle/${bundle.name}/${bundle.target}
permissions: permissions:
- {{if is_service_principal}}service_principal{{else}}user{{end}}_name: {{user_name}} - {{if is_service_principal}}service_principal{{else}}user{{end}}_name: {{user_name}}
level: CAN_MANAGE level: CAN_MANAGE

View File

@ -15,4 +15,4 @@ resources:
path: ../src/dlt_pipeline.ipynb path: ../src/dlt_pipeline.ipynb
configuration: configuration:
bundle.sourcePath: /Workspace/${workspace.file_path}/src bundle.sourcePath: ${workspace.file_path}/src

View File

@ -41,8 +41,8 @@ targets:
mode: production mode: production
workspace: workspace:
host: {{workspace_host}} host: {{workspace_host}}
# We explicitly specify /Users/{{user_name}} to make sure we only have a single copy. # We explicitly specify /Workspace/Users/{{user_name}} to make sure we only have a single copy.
root_path: /Users/{{user_name}}/.bundle/${bundle.name}/${bundle.target} root_path: /Workspace/Users/{{user_name}}/.bundle/${bundle.name}/${bundle.target}
variables: variables:
warehouse_id: {{index ((regexp "[^/]+$").FindStringSubmatch .http_path) 0}} warehouse_id: {{index ((regexp "[^/]+$").FindStringSubmatch .http_path) 0}}
catalog: {{.default_catalog}} catalog: {{.default_catalog}}