diff --git a/.codegen.json b/.codegen.json index 077d072b..4524ab55 100644 --- a/.codegen.json +++ b/.codegen.json @@ -11,6 +11,7 @@ "toolchain": { "required": ["go"], "post_generate": [ + "go test -timeout 240s -run TestConsistentDatabricksSdkVersion github.com/databricks/cli/internal/build", "go run ./bundle/internal/schema/*.go ./bundle/schema/jsonschema.json", "echo 'bundle/internal/tf/schema/\\*.go linguist-generated=true' >> ./.gitattributes", "echo 'go.sum linguist-generated=true' >> ./.gitattributes", diff --git a/.codegen/_openapi_sha b/.codegen/_openapi_sha index 303c7855..2d9cb6d8 100644 --- a/.codegen/_openapi_sha +++ b/.codegen/_openapi_sha @@ -1 +1 @@ -0c86ea6dbd9a730c24ff0d4e509603e476955ac5 \ No newline at end of file +cf9c61453990df0f9453670f2fe68e1b128647a2 \ No newline at end of file diff --git a/.gitattributes b/.gitattributes index 2470eb33..ae10198b 100755 --- a/.gitattributes +++ b/.gitattributes @@ -54,6 +54,7 @@ cmd/workspace/dashboards/dashboards.go linguist-generated=true cmd/workspace/data-sources/data-sources.go linguist-generated=true cmd/workspace/default-namespace/default-namespace.go linguist-generated=true cmd/workspace/disable-legacy-access/disable-legacy-access.go linguist-generated=true +cmd/workspace/disable-legacy-dbfs/disable-legacy-dbfs.go linguist-generated=true cmd/workspace/enhanced-security-monitoring/enhanced-security-monitoring.go linguist-generated=true cmd/workspace/experiments/experiments.go linguist-generated=true cmd/workspace/external-locations/external-locations.go linguist-generated=true diff --git a/.github/workflows/integration-tests.yml b/.github/workflows/integration-tests.yml new file mode 100644 index 00000000..a40cdf32 --- /dev/null +++ b/.github/workflows/integration-tests.yml @@ -0,0 +1,60 @@ +name: integration + +on: + + pull_request: + types: [opened, synchronize] + + merge_group: + + +jobs: + trigger-tests: + if: github.event_name == 'pull_request' + runs-on: ubuntu-latest + environment: "test-trigger-is" + + steps: + - uses: actions/checkout@v4 + + - name: Generate GitHub App Token + id: generate-token + uses: actions/create-github-app-token@v1 + with: + app-id: ${{ secrets.DECO_WORKFLOW_TRIGGER_APP_ID }} + private-key: ${{ secrets.DECO_WORKFLOW_TRIGGER_PRIVATE_KEY }} + owner: ${{ secrets.ORG_NAME }} + repositories: ${{secrets.REPO_NAME}} + + - name: Trigger Workflow in Another Repo + env: + GH_TOKEN: ${{ steps.generate-token.outputs.token }} + run: | + gh workflow run cli-isolated-pr.yml -R ${{ secrets.ORG_NAME }}/${{secrets.REPO_NAME}} \ + --ref main \ + -f pull_request_number=${{ github.event.pull_request.number }} \ + -f commit_sha=${{ github.event.pull_request.head.sha }} + + + + # Statuses and checks apply to specific commits (by hash). + # Enforcement of required checks is done both at the PR level and the merge queue level. + # In case of multiple commits in a single PR, the hash of the squashed commit + # will not match the one for the latest (approved) commit in the PR. + # We auto approve the check for the merge queue for two reasons: + # * Queue times out due to duration of tests. + # * Avoid running integration tests twice, since it was already run at the tip of the branch before squashing. + auto-approve: + if: github.event_name == 'merge_group' + runs-on: ubuntu-latest + steps: + - name: Mark Check + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + shell: bash + run: | + gh api -X POST -H "Accept: application/vnd.github+json" \ + -H "X-GitHub-Api-Version: 2022-11-28" \ + /repos/${{ github.repository }}/statuses/${{ github.sha }} \ + -f 'state=success' \ + -f 'context=Integration Tests Check' diff --git a/CHANGELOG.md b/CHANGELOG.md index f31bb10b..86347493 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,43 @@ # Version changelog +## [Release] Release v0.231.0 + +CLI: + * Added JSON input validation for CLI commands ([#1771](https://github.com/databricks/cli/pull/1771)). + * Support Git worktrees for `sync` ([#1831](https://github.com/databricks/cli/pull/1831)). + +Bundles: + * Add `bundle summary` to display URLs for deployed resources ([#1731](https://github.com/databricks/cli/pull/1731)). + * Added a warning when incorrect permissions used for `/Workspace/Shared` bundle root ([#1821](https://github.com/databricks/cli/pull/1821)). + * Show actionable errors for collaborative deployment scenarios ([#1386](https://github.com/databricks/cli/pull/1386)). + * Fix path to repository-wide exclude file ([#1837](https://github.com/databricks/cli/pull/1837)). + * Fixed typo in converting cluster permissions ([#1826](https://github.com/databricks/cli/pull/1826)). + * Ignore metastore permission error during template generation ([#1819](https://github.com/databricks/cli/pull/1819)). + * Handle normalization of `dyn.KindTime` into an any type ([#1836](https://github.com/databricks/cli/pull/1836)). + * Added support for pip options in environment dependencies ([#1842](https://github.com/databricks/cli/pull/1842)). + * Fix race condition when restarting continuous jobs ([#1849](https://github.com/databricks/cli/pull/1849)). + * Fix pipeline in default-python template not working for certain workspaces ([#1854](https://github.com/databricks/cli/pull/1854)). + * Add "output" flag to the bundle sync command ([#1853](https://github.com/databricks/cli/pull/1853)). + +Internal: + * Move utility functions dealing with IAM to libs/iamutil ([#1820](https://github.com/databricks/cli/pull/1820)). + * Remove unused `IS_OWNER` constant ([#1823](https://github.com/databricks/cli/pull/1823)). + * Assert SDK version is consistent in the CLI generation process ([#1814](https://github.com/databricks/cli/pull/1814)). + * Fixed unmarshalling json input into `interface{}` type ([#1832](https://github.com/databricks/cli/pull/1832)). + * Fix `TestAccFsMkdirWhenFileExistsAtPath` in isolated Azure environments ([#1833](https://github.com/databricks/cli/pull/1833)). + * Add behavioral tests for examples from the YAML spec ([#1835](https://github.com/databricks/cli/pull/1835)). + * Remove Terraform conversion function that's no longer used ([#1840](https://github.com/databricks/cli/pull/1840)). + * Encode assumptions about the dashboards API in a test ([#1839](https://github.com/databricks/cli/pull/1839)). + * Add script to make testing of code on branches easier ([#1844](https://github.com/databricks/cli/pull/1844)). + +API Changes: + * Added `databricks disable-legacy-dbfs` command group. + +OpenAPI commit cf9c61453990df0f9453670f2fe68e1b128647a2 (2024-10-14) +Dependency updates: + * Upgrade TF provider to 1.54.0 ([#1852](https://github.com/databricks/cli/pull/1852)). + * Bump github.com/databricks/databricks-sdk-go from 0.48.0 to 0.49.0 ([#1843](https://github.com/databricks/cli/pull/1843)). + ## [Release] Release v0.230.0 Notable changes for Databricks Asset Bundles: diff --git a/README.md b/README.md index 51780d0f..3c238702 100644 --- a/README.md +++ b/README.md @@ -35,3 +35,6 @@ docker run -e DATABRICKS_HOST=$YOUR_HOST_URL -e DATABRICKS_TOKEN=$YOUR_TOKEN ghc This CLI follows the Databricks Unified Authentication principles. You can find a detailed description at https://github.com/databricks/databricks-sdk-go#authentication. + +## Privacy Notice +Databricks CLI use is subject to the [Databricks License](https://github.com/databricks/cli/blob/main/LICENSE) and [Databricks Privacy Notice](https://www.databricks.com/legal/privacynotice), including any Usage Data provisions. diff --git a/bundle/config/mutator/apply_presets.go b/bundle/config/mutator/apply_presets.go index 9d79835d..ffe9fc93 100644 --- a/bundle/config/mutator/apply_presets.go +++ b/bundle/config/mutator/apply_presets.go @@ -238,6 +238,15 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos } } + // Dashboards: Prefix + for key, dashboard := range r.Dashboards { + if dashboard == nil || dashboard.CreateDashboardRequest == nil { + diags = diags.Extend(diag.Errorf("dashboard %s s is not defined", key)) + continue + } + dashboard.DisplayName = prefix + dashboard.DisplayName + } + return diags } diff --git a/bundle/config/mutator/configure_dashboard_defaults.go b/bundle/config/mutator/configure_dashboard_defaults.go new file mode 100644 index 00000000..36ec279d --- /dev/null +++ b/bundle/config/mutator/configure_dashboard_defaults.go @@ -0,0 +1,70 @@ +package mutator + +import ( + "context" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/dyn" +) + +type configureDashboardDefaults struct{} + +func ConfigureDashboardDefaults() bundle.Mutator { + return &configureDashboardDefaults{} +} + +func (m *configureDashboardDefaults) Name() string { + return "ConfigureDashboardDefaults" +} + +func (m *configureDashboardDefaults) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + var diags diag.Diagnostics + + pattern := dyn.NewPattern( + dyn.Key("resources"), + dyn.Key("dashboards"), + dyn.AnyKey(), + ) + + // Configure defaults for all dashboards. + err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { + return dyn.MapByPattern(v, pattern, func(p dyn.Path, v dyn.Value) (dyn.Value, error) { + var err error + v, err = setIfNotExists(v, dyn.NewPath(dyn.Key("parent_path")), dyn.V(b.Config.Workspace.ResourcePath)) + if err != nil { + return dyn.InvalidValue, err + } + v, err = setIfNotExists(v, dyn.NewPath(dyn.Key("embed_credentials")), dyn.V(false)) + if err != nil { + return dyn.InvalidValue, err + } + return v, nil + }) + }) + + diags = diags.Extend(diag.FromErr(err)) + return diags +} + +func setIfNotExists(v dyn.Value, path dyn.Path, defaultValue dyn.Value) (dyn.Value, error) { + // Get the field at the specified path (if set). + _, err := dyn.GetByPath(v, path) + switch { + case dyn.IsNoSuchKeyError(err): + // OK, we'll set the default value. + break + case dyn.IsCannotTraverseNilError(err): + // Cannot traverse the value, skip it. + return v, nil + case err == nil: + // The field is set, skip it. + return v, nil + default: + // Return the error. + return v, err + } + + // Set the field at the specified path. + return dyn.SetByPath(v, path, defaultValue) +} diff --git a/bundle/config/mutator/configure_dashboard_defaults_test.go b/bundle/config/mutator/configure_dashboard_defaults_test.go new file mode 100644 index 00000000..4804b715 --- /dev/null +++ b/bundle/config/mutator/configure_dashboard_defaults_test.go @@ -0,0 +1,130 @@ +package mutator_test + +import ( + "context" + "testing" + + "github.com/databricks/cli/bundle" + "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" + "github.com/stretchr/testify/require" +) + +func TestConfigureDashboardDefaultsParentPath(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Workspace: config.Workspace{ + ResourcePath: "/foo/bar", + }, + Resources: config.Resources{ + Dashboards: map[string]*resources.Dashboard{ + "d1": { + // Empty string is skipped. + // See below for how it is set. + CreateDashboardRequest: &dashboards.CreateDashboardRequest{ + ParentPath: "", + }, + }, + "d2": { + // Non-empty string is skipped. + CreateDashboardRequest: &dashboards.CreateDashboardRequest{ + ParentPath: "already-set", + }, + }, + "d3": { + // No parent path set. + }, + "d4": nil, + }, + }, + }, + } + + // We can't set an empty string in the typed configuration. + // Do it on the dyn.Value directly. + bundletest.Mutate(t, b, func(v dyn.Value) (dyn.Value, error) { + return dyn.Set(v, "resources.dashboards.d1.parent_path", dyn.V("")) + }) + + diags := bundle.Apply(context.Background(), b, mutator.ConfigureDashboardDefaults()) + require.NoError(t, diags.Error()) + + var v dyn.Value + var err error + + // Set to empty string; unchanged. + v, err = dyn.Get(b.Config.Value(), "resources.dashboards.d1.parent_path") + if assert.NoError(t, err) { + assert.Equal(t, "", v.MustString()) + } + + // Set to "already-set"; unchanged. + v, err = dyn.Get(b.Config.Value(), "resources.dashboards.d2.parent_path") + if assert.NoError(t, err) { + assert.Equal(t, "already-set", v.MustString()) + } + + // Not set; now set to the workspace resource path. + v, err = dyn.Get(b.Config.Value(), "resources.dashboards.d3.parent_path") + if assert.NoError(t, err) { + assert.Equal(t, "/foo/bar", v.MustString()) + } + + // No valid dashboard; no change. + _, err = dyn.Get(b.Config.Value(), "resources.dashboards.d4.parent_path") + assert.True(t, dyn.IsCannotTraverseNilError(err)) +} + +func TestConfigureDashboardDefaultsEmbedCredentials(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + Dashboards: map[string]*resources.Dashboard{ + "d1": { + EmbedCredentials: true, + }, + "d2": { + EmbedCredentials: false, + }, + "d3": { + // No parent path set. + }, + "d4": nil, + }, + }, + }, + } + + diags := bundle.Apply(context.Background(), b, mutator.ConfigureDashboardDefaults()) + require.NoError(t, diags.Error()) + + var v dyn.Value + var err error + + // Set to true; still true. + v, err = dyn.Get(b.Config.Value(), "resources.dashboards.d1.embed_credentials") + if assert.NoError(t, err) { + assert.Equal(t, true, v.MustBool()) + } + + // Set to false; still false. + v, err = dyn.Get(b.Config.Value(), "resources.dashboards.d2.embed_credentials") + if assert.NoError(t, err) { + assert.Equal(t, false, v.MustBool()) + } + + // Not set; now false. + v, err = dyn.Get(b.Config.Value(), "resources.dashboards.d3.embed_credentials") + if assert.NoError(t, err) { + assert.Equal(t, false, v.MustBool()) + } + + // No valid dashboard; no change. + _, err = dyn.Get(b.Config.Value(), "resources.dashboards.d4.embed_credentials") + assert.True(t, dyn.IsCannotTraverseNilError(err)) +} diff --git a/bundle/config/mutator/initialize_urls.go b/bundle/config/mutator/initialize_urls.go new file mode 100644 index 00000000..31930591 --- /dev/null +++ b/bundle/config/mutator/initialize_urls.go @@ -0,0 +1,65 @@ +package mutator + +import ( + "context" + "net/url" + "strconv" + "strings" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" +) + +type initializeURLs struct { +} + +// InitializeURLs makes sure the URL field of each resource is configured. +// NOTE: since this depends on an extra API call, this mutator adds some extra +// latency. As such, it should only be used when needed. +// This URL field is used for the output of the 'bundle summary' CLI command. +func InitializeURLs() bundle.Mutator { + return &initializeURLs{} +} + +func (m *initializeURLs) Name() string { + return "InitializeURLs" +} + +func (m *initializeURLs) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + workspaceId, err := b.WorkspaceClient().CurrentWorkspaceID(ctx) + if err != nil { + return diag.FromErr(err) + } + orgId := strconv.FormatInt(workspaceId, 10) + host := b.WorkspaceClient().Config.CanonicalHostName() + initializeForWorkspace(b, orgId, host) + return nil +} + +func initializeForWorkspace(b *bundle.Bundle, orgId string, host string) error { + baseURL, err := url.Parse(host) + if err != nil { + return err + } + + // Add ?o= only if wasn't in the subdomain already. + // The ?o= is needed when vanity URLs / legacy workspace URLs are used. + // If it's not needed we prefer to leave it out since these URLs are rather + // long for most terminals. + // + // See https://docs.databricks.com/en/workspace/workspace-details.html for + // further reading about the '?o=' suffix. + if !strings.Contains(baseURL.Hostname(), orgId) { + values := baseURL.Query() + values.Add("o", orgId) + baseURL.RawQuery = values.Encode() + } + + for _, group := range b.Config.Resources.AllResources() { + for _, r := range group.Resources { + r.InitializeURL(*baseURL) + } + } + + return nil +} diff --git a/bundle/config/mutator/initialize_urls_test.go b/bundle/config/mutator/initialize_urls_test.go new file mode 100644 index 00000000..61103de8 --- /dev/null +++ b/bundle/config/mutator/initialize_urls_test.go @@ -0,0 +1,140 @@ +package mutator + +import ( + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/databricks-sdk-go/service/catalog" + "github.com/databricks/databricks-sdk-go/service/compute" + "github.com/databricks/databricks-sdk-go/service/dashboards" + "github.com/databricks/databricks-sdk-go/service/jobs" + "github.com/databricks/databricks-sdk-go/service/ml" + "github.com/databricks/databricks-sdk-go/service/pipelines" + "github.com/databricks/databricks-sdk-go/service/serving" + "github.com/stretchr/testify/require" +) + +func TestInitializeURLs(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Workspace: config.Workspace{ + Host: "https://mycompany.databricks.com/", + }, + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "job1": { + ID: "1", + JobSettings: &jobs.JobSettings{Name: "job1"}, + }, + }, + Pipelines: map[string]*resources.Pipeline{ + "pipeline1": { + ID: "3", + PipelineSpec: &pipelines.PipelineSpec{Name: "pipeline1"}, + }, + }, + Experiments: map[string]*resources.MlflowExperiment{ + "experiment1": { + ID: "4", + Experiment: &ml.Experiment{Name: "experiment1"}, + }, + }, + Models: map[string]*resources.MlflowModel{ + "model1": { + ID: "a model uses its name for identifier", + Model: &ml.Model{Name: "a model uses its name for identifier"}, + }, + }, + ModelServingEndpoints: map[string]*resources.ModelServingEndpoint{ + "servingendpoint1": { + ID: "my_serving_endpoint", + CreateServingEndpoint: &serving.CreateServingEndpoint{ + Name: "my_serving_endpoint", + }, + }, + }, + RegisteredModels: map[string]*resources.RegisteredModel{ + "registeredmodel1": { + ID: "8", + CreateRegisteredModelRequest: &catalog.CreateRegisteredModelRequest{ + Name: "my_registered_model", + }, + }, + }, + QualityMonitors: map[string]*resources.QualityMonitor{ + "qualityMonitor1": { + CreateMonitor: &catalog.CreateMonitor{ + TableName: "catalog.schema.qualityMonitor1", + }, + }, + }, + Schemas: map[string]*resources.Schema{ + "schema1": { + ID: "catalog.schema", + CreateSchema: &catalog.CreateSchema{ + Name: "schema", + }, + }, + }, + Clusters: map[string]*resources.Cluster{ + "cluster1": { + ID: "1017-103929-vlr7jzcf", + ClusterSpec: &compute.ClusterSpec{ + ClusterName: "cluster1", + }, + }, + }, + Dashboards: map[string]*resources.Dashboard{ + "dashboard1": { + ID: "01ef8d56871e1d50ae30ce7375e42478", + CreateDashboardRequest: &dashboards.CreateDashboardRequest{ + DisplayName: "My special dashboard", + }, + }, + }, + }, + }, + } + + expectedURLs := map[string]string{ + "job1": "https://mycompany.databricks.com/jobs/1?o=123456", + "pipeline1": "https://mycompany.databricks.com/pipelines/3?o=123456", + "experiment1": "https://mycompany.databricks.com/ml/experiments/4?o=123456", + "model1": "https://mycompany.databricks.com/ml/models/a%20model%20uses%20its%20name%20for%20identifier?o=123456", + "servingendpoint1": "https://mycompany.databricks.com/ml/endpoints/my_serving_endpoint?o=123456", + "registeredmodel1": "https://mycompany.databricks.com/explore/data/models/8?o=123456", + "qualityMonitor1": "https://mycompany.databricks.com/explore/data/catalog/schema/qualityMonitor1?o=123456", + "schema1": "https://mycompany.databricks.com/explore/data/catalog/schema?o=123456", + "cluster1": "https://mycompany.databricks.com/compute/clusters/1017-103929-vlr7jzcf?o=123456", + "dashboard1": "https://mycompany.databricks.com/dashboardsv3/01ef8d56871e1d50ae30ce7375e42478/published?o=123456", + } + + initializeForWorkspace(b, "123456", "https://mycompany.databricks.com/") + + for _, group := range b.Config.Resources.AllResources() { + for key, r := range group.Resources { + require.Equal(t, expectedURLs[key], r.GetURL(), "Unexpected URL for "+key) + } + } +} + +func TestInitializeURLsWithoutOrgId(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "job1": { + ID: "1", + JobSettings: &jobs.JobSettings{Name: "job1"}, + }, + }, + }, + }, + } + + initializeForWorkspace(b, "123456", "https://adb-123456.azuredatabricks.net/") + + require.Equal(t, "https://adb-123456.azuredatabricks.net/jobs/1", b.Config.Resources.Jobs["job1"].URL) +} diff --git a/bundle/config/mutator/process_target_mode_test.go b/bundle/config/mutator/process_target_mode_test.go index 5dcc7ba1..cddc8758 100644 --- a/bundle/config/mutator/process_target_mode_test.go +++ b/bundle/config/mutator/process_target_mode_test.go @@ -14,6 +14,7 @@ import ( sdkconfig "github.com/databricks/databricks-sdk-go/config" "github.com/databricks/databricks-sdk-go/service/catalog" "github.com/databricks/databricks-sdk-go/service/compute" + "github.com/databricks/databricks-sdk-go/service/dashboards" "github.com/databricks/databricks-sdk-go/service/iam" "github.com/databricks/databricks-sdk-go/service/jobs" "github.com/databricks/databricks-sdk-go/service/ml" @@ -126,6 +127,13 @@ func mockBundle(mode config.Mode) *bundle.Bundle { Clusters: map[string]*resources.Cluster{ "cluster1": {ClusterSpec: &compute.ClusterSpec{ClusterName: "cluster1", SparkVersion: "13.2.x", NumWorkers: 1}}, }, + Dashboards: map[string]*resources.Dashboard{ + "dashboard1": { + CreateDashboardRequest: &dashboards.CreateDashboardRequest{ + DisplayName: "dashboard1", + }, + }, + }, }, }, // Use AWS implementation for testing. @@ -187,6 +195,9 @@ func TestProcessTargetModeDevelopment(t *testing.T) { // Clusters assert.Equal(t, "[dev lennart] cluster1", b.Config.Resources.Clusters["cluster1"].ClusterName) + + // Dashboards + assert.Equal(t, "[dev lennart] dashboard1", b.Config.Resources.Dashboards["dashboard1"].DisplayName) } func TestProcessTargetModeDevelopmentTagNormalizationForAws(t *testing.T) { diff --git a/bundle/config/mutator/run_as.go b/bundle/config/mutator/run_as.go index 6b3069d4..0ca71e28 100644 --- a/bundle/config/mutator/run_as.go +++ b/bundle/config/mutator/run_as.go @@ -110,6 +110,16 @@ func validateRunAs(b *bundle.Bundle) diag.Diagnostics { )) } + // Dashboards do not support run_as in the API. + if len(b.Config.Resources.Dashboards) > 0 { + diags = diags.Extend(reportRunAsNotSupported( + "dashboards", + b.Config.GetLocation("resources.dashboards"), + b.Config.Workspace.CurrentUser.UserName, + identity, + )) + } + return diags } diff --git a/bundle/config/mutator/run_as_test.go b/bundle/config/mutator/run_as_test.go index 0ed968e2..dbf4bf80 100644 --- a/bundle/config/mutator/run_as_test.go +++ b/bundle/config/mutator/run_as_test.go @@ -33,6 +33,7 @@ func allResourceTypes(t *testing.T) []string { // also update this check when adding a new resource require.Equal(t, []string{ "clusters", + "dashboards", "experiments", "jobs", "model_serving_endpoints", @@ -190,6 +191,7 @@ func TestRunAsErrorForUnsupportedResources(t *testing.T) { Config: *r, } diags := bundle.Apply(context.Background(), b, SetRunAs()) + require.Error(t, diags.Error()) assert.Contains(t, diags.Error().Error(), "do not support a setting a run_as user that is different from the owner.\n"+ "Current identity: alice. Run as identity: bob.\n"+ "See https://docs.databricks.com/dev-tools/bundles/run-as.html to learn more about the run_as property.", rt) diff --git a/bundle/config/mutator/translate_paths.go b/bundle/config/mutator/translate_paths.go index 5f22570e..82b0b3ca 100644 --- a/bundle/config/mutator/translate_paths.go +++ b/bundle/config/mutator/translate_paths.go @@ -162,6 +162,20 @@ func (t *translateContext) translateNoOp(literal, localFullPath, localRelPath, r return localRelPath, nil } +func (t *translateContext) retainLocalAbsoluteFilePath(literal, localFullPath, localRelPath, remotePath string) (string, error) { + info, err := t.b.SyncRoot.Stat(localRelPath) + if errors.Is(err, fs.ErrNotExist) { + return "", fmt.Errorf("file %s not found", literal) + } + if err != nil { + return "", fmt.Errorf("unable to determine if %s is a file: %w", localFullPath, err) + } + if info.IsDir() { + return "", fmt.Errorf("expected %s to be a file but found a directory", literal) + } + return localFullPath, nil +} + func (t *translateContext) translateNoOpWithPrefix(literal, localFullPath, localRelPath, remotePath string) (string, error) { if !strings.HasPrefix(localRelPath, ".") { localRelPath = "." + string(filepath.Separator) + localRelPath @@ -215,6 +229,7 @@ func (m *translatePaths) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnos t.applyJobTranslations, t.applyPipelineTranslations, t.applyArtifactTranslations, + t.applyDashboardTranslations, } { v, err = fn(v) if err != nil { diff --git a/bundle/config/mutator/translate_paths_dashboards.go b/bundle/config/mutator/translate_paths_dashboards.go new file mode 100644 index 00000000..93822a59 --- /dev/null +++ b/bundle/config/mutator/translate_paths_dashboards.go @@ -0,0 +1,28 @@ +package mutator + +import ( + "fmt" + + "github.com/databricks/cli/libs/dyn" +) + +func (t *translateContext) applyDashboardTranslations(v dyn.Value) (dyn.Value, error) { + // Convert the `file_path` field to a local absolute path. + // We load the file at this path and use its contents for the dashboard contents. + pattern := dyn.NewPattern( + dyn.Key("resources"), + dyn.Key("dashboards"), + dyn.AnyKey(), + dyn.Key("file_path"), + ) + + return dyn.MapByPattern(v, pattern, func(p dyn.Path, v dyn.Value) (dyn.Value, error) { + key := p[2].Key() + dir, err := v.Location().Directory() + if err != nil { + return dyn.InvalidValue, fmt.Errorf("unable to determine directory for dashboard %s: %w", key, err) + } + + return t.rewriteRelativeTo(p, v, t.retainLocalAbsoluteFilePath, dir, "") + }) +} diff --git a/bundle/config/mutator/translate_paths_test.go b/bundle/config/mutator/translate_paths_test.go index c03cee73..9d655b27 100644 --- a/bundle/config/mutator/translate_paths_test.go +++ b/bundle/config/mutator/translate_paths_test.go @@ -699,6 +699,9 @@ func TestTranslatePathJobEnvironments(t *testing.T) { "../dist/env2.whl", "simplejson", "/Workspace/Users/foo@bar.com/test.whl", + "--extra-index-url https://name:token@gitlab.com/api/v4/projects/9876/packages/pypi/simple foobar", + "foobar --extra-index-url https://name:token@gitlab.com/api/v4/projects/9876/packages/pypi/simple", + "https://foo@bar.com/packages/pypi/simple", }, }, }, @@ -719,6 +722,9 @@ func TestTranslatePathJobEnvironments(t *testing.T) { assert.Equal(t, strings.Join([]string{".", "dist", "env2.whl"}, string(os.PathSeparator)), b.Config.Resources.Jobs["job"].JobSettings.Environments[0].Spec.Dependencies[1]) assert.Equal(t, "simplejson", b.Config.Resources.Jobs["job"].JobSettings.Environments[0].Spec.Dependencies[2]) assert.Equal(t, "/Workspace/Users/foo@bar.com/test.whl", b.Config.Resources.Jobs["job"].JobSettings.Environments[0].Spec.Dependencies[3]) + assert.Equal(t, "--extra-index-url https://name:token@gitlab.com/api/v4/projects/9876/packages/pypi/simple foobar", b.Config.Resources.Jobs["job"].JobSettings.Environments[0].Spec.Dependencies[4]) + assert.Equal(t, "foobar --extra-index-url https://name:token@gitlab.com/api/v4/projects/9876/packages/pypi/simple", b.Config.Resources.Jobs["job"].JobSettings.Environments[0].Spec.Dependencies[5]) + assert.Equal(t, "https://foo@bar.com/packages/pypi/simple", b.Config.Resources.Jobs["job"].JobSettings.Environments[0].Spec.Dependencies[6]) } func TestTranslatePathWithComplexVariables(t *testing.T) { diff --git a/bundle/config/resources.go b/bundle/config/resources.go index d3ddde89..2e3f4dbf 100644 --- a/bundle/config/resources.go +++ b/bundle/config/resources.go @@ -3,6 +3,7 @@ package config import ( "context" "fmt" + "net/url" "github.com/databricks/cli/bundle/config/resources" "github.com/databricks/databricks-sdk-go" @@ -21,6 +22,7 @@ type Resources struct { Schemas map[string]*resources.Schema `json:"schemas,omitempty"` Volumes map[string]*resources.Volume `json:"volumes,omitempty"` Clusters map[string]*resources.Cluster `json:"clusters,omitempty"` + Dashboards map[string]*resources.Dashboard `json:"dashboards,omitempty"` } type ConfigResource interface { @@ -31,6 +33,54 @@ type ConfigResource interface { // Terraform equivalent name of the resource. For example "databricks_job" // for jobs and "databricks_pipeline" for pipelines. TerraformResourceName() string + + // GetName returns the in-product name of the resource. + GetName() string + + // GetURL returns the URL of the resource. + GetURL() string + + // InitializeURL initializes the URL field of the resource. + InitializeURL(baseURL url.URL) +} + +// ResourceGroup represents a group of resources of the same type. +// It includes a description of the resource type and a map of resources. +type ResourceGroup struct { + Description ResourceDescription + Resources map[string]ConfigResource +} + +// collectResourceMap collects resources of a specific type into a ResourceGroup. +func collectResourceMap[T ConfigResource]( + description ResourceDescription, + input map[string]T, +) ResourceGroup { + resources := make(map[string]ConfigResource) + for key, resource := range input { + resources[key] = resource + } + return ResourceGroup{ + Description: description, + Resources: resources, + } +} + +// AllResources returns all resources in the bundle grouped by their resource type. +func (r *Resources) AllResources() []ResourceGroup { + descriptions := SupportedResources() + return []ResourceGroup{ + collectResourceMap(descriptions["jobs"], r.Jobs), + collectResourceMap(descriptions["pipelines"], r.Pipelines), + collectResourceMap(descriptions["models"], r.Models), + collectResourceMap(descriptions["experiments"], r.Experiments), + collectResourceMap(descriptions["model_serving_endpoints"], r.ModelServingEndpoints), + collectResourceMap(descriptions["registered_models"], r.RegisteredModels), + collectResourceMap(descriptions["quality_monitors"], r.QualityMonitors), + collectResourceMap(descriptions["schemas"], r.Schemas), + collectResourceMap(descriptions["clusters"], r.Clusters), + collectResourceMap(descriptions["dashboards"], r.Dashboards), + } } func (r *Resources) FindResourceByConfigKey(key string) (ConfigResource, error) { @@ -62,21 +112,77 @@ func (r *Resources) FindResourceByConfigKey(key string) (ConfigResource, error) } type ResourceDescription struct { + // Singular and plural name when used to refer to the configuration. SingularName string + PluralName string + + // Singular and plural title when used in summaries / terminal UI. + SingularTitle string + PluralTitle string } // The keys of the map corresponds to the resource key in the bundle configuration. func SupportedResources() map[string]ResourceDescription { return map[string]ResourceDescription{ - "jobs": {SingularName: "job"}, - "pipelines": {SingularName: "pipeline"}, - "models": {SingularName: "model"}, - "experiments": {SingularName: "experiment"}, - "model_serving_endpoints": {SingularName: "model_serving_endpoint"}, - "registered_models": {SingularName: "registered_model"}, - "quality_monitors": {SingularName: "quality_monitor"}, - "schemas": {SingularName: "schema"}, - "volumes": {SingularName: "volume"}, - "clusters": {SingularName: "cluster"}, + "jobs": { + SingularName: "job", + PluralName: "jobs", + SingularTitle: "Job", + PluralTitle: "Jobs", + }, + "pipelines": { + SingularName: "pipeline", + PluralName: "pipelines", + SingularTitle: "Pipeline", + PluralTitle: "Pipelines", + }, + "models": { + SingularName: "model", + PluralName: "models", + SingularTitle: "Model", + PluralTitle: "Models", + }, + "experiments": { + SingularName: "experiment", + PluralName: "experiments", + SingularTitle: "Experiment", + PluralTitle: "Experiments", + }, + "model_serving_endpoints": { + SingularName: "model_serving_endpoint", + PluralName: "model_serving_endpoints", + SingularTitle: "Model Serving Endpoint", + PluralTitle: "Model Serving Endpoints", + }, + "registered_models": { + SingularName: "registered_model", + PluralName: "registered_models", + SingularTitle: "Registered Model", + PluralTitle: "Registered Models", + }, + "quality_monitors": { + SingularName: "quality_monitor", + PluralName: "quality_monitors", + SingularTitle: "Quality Monitor", + PluralTitle: "Quality Monitors", + }, + "schemas": { + SingularName: "schema", + PluralName: "schemas", + SingularTitle: "Schema", + PluralTitle: "Schemas", + }, + "clusters": { + SingularName: "cluster", + PluralName: "clusters", + SingularTitle: "Cluster", + PluralTitle: "Clusters", + }, + "dashboards": { + SingularName: "dashboard", + PluralName: "dashboards", + SingularTitle: "Dashboard", + PluralTitle: "Dashboards", + }, } } diff --git a/bundle/config/resources/clusters.go b/bundle/config/resources/clusters.go index 63234566..eb0247c6 100644 --- a/bundle/config/resources/clusters.go +++ b/bundle/config/resources/clusters.go @@ -2,6 +2,8 @@ package resources import ( "context" + "fmt" + "net/url" "github.com/databricks/cli/libs/log" "github.com/databricks/databricks-sdk-go" @@ -13,6 +15,7 @@ type Cluster struct { ID string `json:"id,omitempty" bundle:"readonly"` Permissions []Permission `json:"permissions,omitempty"` ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"internal"` + URL string `json:"url,omitempty" bundle:"internal"` *compute.ClusterSpec } @@ -37,3 +40,19 @@ func (s *Cluster) Exists(ctx context.Context, w *databricks.WorkspaceClient, id func (s *Cluster) TerraformResourceName() string { return "databricks_cluster" } + +func (s *Cluster) InitializeURL(baseURL url.URL) { + if s.ID == "" { + return + } + baseURL.Path = fmt.Sprintf("compute/clusters/%s", s.ID) + s.URL = baseURL.String() +} + +func (s *Cluster) GetName() string { + return s.ClusterName +} + +func (s *Cluster) GetURL() string { + return s.URL +} diff --git a/bundle/config/resources/dashboard.go b/bundle/config/resources/dashboard.go new file mode 100644 index 00000000..462dbc56 --- /dev/null +++ b/bundle/config/resources/dashboard.go @@ -0,0 +1,81 @@ +package resources + +import ( + "context" + "fmt" + "net/url" + + "github.com/databricks/cli/libs/log" + "github.com/databricks/databricks-sdk-go" + "github.com/databricks/databricks-sdk-go/marshal" + "github.com/databricks/databricks-sdk-go/service/dashboards" +) + +type Dashboard struct { + ID string `json:"id,omitempty" bundle:"readonly"` + Permissions []Permission `json:"permissions,omitempty"` + ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"internal"` + URL string `json:"url,omitempty" bundle:"internal"` + + *dashboards.CreateDashboardRequest + + // ========================= + // === Additional fields === + // ========================= + + // SerializedDashboard holds the contents of the dashboard in serialized JSON form. + // We override the field's type from the SDK struct here to allow for inlining as YAML. + // If the value is a string, it is used as is. + // If it is not a string, its contents is marshalled as JSON. + SerializedDashboard any `json:"serialized_dashboard,omitempty"` + + // EmbedCredentials is a flag to indicate if the publisher's credentials should + // be embedded in the published dashboard. These embedded credentials will be used + // to execute the published dashboard's queries. + // + // Defaults to false if not set. + EmbedCredentials bool `json:"embed_credentials,omitempty"` + + // FilePath points to the local `.lvdash.json` file containing the dashboard definition. + FilePath string `json:"file_path,omitempty"` +} + +func (r *Dashboard) UnmarshalJSON(b []byte) error { + return marshal.Unmarshal(b, r) +} + +func (r Dashboard) MarshalJSON() ([]byte, error) { + return marshal.Marshal(r) +} + +func (*Dashboard) Exists(ctx context.Context, w *databricks.WorkspaceClient, id string) (bool, error) { + _, err := w.Lakeview.Get(ctx, dashboards.GetDashboardRequest{ + DashboardId: id, + }) + if err != nil { + log.Debugf(ctx, "dashboard %s does not exist", id) + return false, err + } + return true, nil +} + +func (*Dashboard) TerraformResourceName() string { + return "databricks_dashboard" +} + +func (r *Dashboard) InitializeURL(baseURL url.URL) { + if r.ID == "" { + return + } + + baseURL.Path = fmt.Sprintf("dashboardsv3/%s/published", r.ID) + r.URL = baseURL.String() +} + +func (r *Dashboard) GetName() string { + return r.DisplayName +} + +func (r *Dashboard) GetURL() string { + return r.URL +} diff --git a/bundle/config/resources/job.go b/bundle/config/resources/job.go index d8f97a2d..98db1ec5 100644 --- a/bundle/config/resources/job.go +++ b/bundle/config/resources/job.go @@ -2,6 +2,8 @@ package resources import ( "context" + "fmt" + "net/url" "strconv" "github.com/databricks/cli/libs/log" @@ -14,6 +16,7 @@ type Job struct { ID string `json:"id,omitempty" bundle:"readonly"` Permissions []Permission `json:"permissions,omitempty"` ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"internal"` + URL string `json:"url,omitempty" bundle:"internal"` *jobs.JobSettings } @@ -44,3 +47,19 @@ func (j *Job) Exists(ctx context.Context, w *databricks.WorkspaceClient, id stri func (j *Job) TerraformResourceName() string { return "databricks_job" } + +func (j *Job) InitializeURL(baseURL url.URL) { + if j.ID == "" { + return + } + baseURL.Path = fmt.Sprintf("jobs/%s", j.ID) + j.URL = baseURL.String() +} + +func (j *Job) GetName() string { + return j.Name +} + +func (j *Job) GetURL() string { + return j.URL +} diff --git a/bundle/config/resources/mlflow_experiment.go b/bundle/config/resources/mlflow_experiment.go index 0ab48643..a5871468 100644 --- a/bundle/config/resources/mlflow_experiment.go +++ b/bundle/config/resources/mlflow_experiment.go @@ -2,6 +2,8 @@ package resources import ( "context" + "fmt" + "net/url" "github.com/databricks/cli/libs/log" "github.com/databricks/databricks-sdk-go" @@ -13,6 +15,7 @@ type MlflowExperiment struct { ID string `json:"id,omitempty" bundle:"readonly"` Permissions []Permission `json:"permissions,omitempty"` ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"internal"` + URL string `json:"url,omitempty" bundle:"internal"` *ml.Experiment } @@ -39,3 +42,19 @@ func (s *MlflowExperiment) Exists(ctx context.Context, w *databricks.WorkspaceCl func (s *MlflowExperiment) TerraformResourceName() string { return "databricks_mlflow_experiment" } + +func (s *MlflowExperiment) InitializeURL(baseURL url.URL) { + if s.ID == "" { + return + } + baseURL.Path = fmt.Sprintf("ml/experiments/%s", s.ID) + s.URL = baseURL.String() +} + +func (s *MlflowExperiment) GetName() string { + return s.Name +} + +func (s *MlflowExperiment) GetURL() string { + return s.URL +} diff --git a/bundle/config/resources/mlflow_model.go b/bundle/config/resources/mlflow_model.go index 300474e3..9ead254d 100644 --- a/bundle/config/resources/mlflow_model.go +++ b/bundle/config/resources/mlflow_model.go @@ -2,6 +2,8 @@ package resources import ( "context" + "fmt" + "net/url" "github.com/databricks/cli/libs/log" "github.com/databricks/databricks-sdk-go" @@ -13,6 +15,7 @@ type MlflowModel struct { ID string `json:"id,omitempty" bundle:"readonly"` Permissions []Permission `json:"permissions,omitempty"` ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"internal"` + URL string `json:"url,omitempty" bundle:"internal"` *ml.Model } @@ -39,3 +42,19 @@ func (s *MlflowModel) Exists(ctx context.Context, w *databricks.WorkspaceClient, func (s *MlflowModel) TerraformResourceName() string { return "databricks_mlflow_model" } + +func (s *MlflowModel) InitializeURL(baseURL url.URL) { + if s.ID == "" { + return + } + baseURL.Path = fmt.Sprintf("ml/models/%s", s.ID) + s.URL = baseURL.String() +} + +func (s *MlflowModel) GetName() string { + return s.Name +} + +func (s *MlflowModel) GetURL() string { + return s.URL +} diff --git a/bundle/config/resources/model_serving_endpoint.go b/bundle/config/resources/model_serving_endpoint.go index 5efb7ea2..7f3ae00c 100644 --- a/bundle/config/resources/model_serving_endpoint.go +++ b/bundle/config/resources/model_serving_endpoint.go @@ -2,6 +2,8 @@ package resources import ( "context" + "fmt" + "net/url" "github.com/databricks/cli/libs/log" "github.com/databricks/databricks-sdk-go" @@ -23,6 +25,7 @@ type ModelServingEndpoint struct { Permissions []Permission `json:"permissions,omitempty"` ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"internal"` + URL string `json:"url,omitempty" bundle:"internal"` } func (s *ModelServingEndpoint) UnmarshalJSON(b []byte) error { @@ -47,3 +50,19 @@ func (s *ModelServingEndpoint) Exists(ctx context.Context, w *databricks.Workspa func (s *ModelServingEndpoint) TerraformResourceName() string { return "databricks_model_serving" } + +func (s *ModelServingEndpoint) InitializeURL(baseURL url.URL) { + if s.ID == "" { + return + } + baseURL.Path = fmt.Sprintf("ml/endpoints/%s", s.ID) + s.URL = baseURL.String() +} + +func (s *ModelServingEndpoint) GetName() string { + return s.Name +} + +func (s *ModelServingEndpoint) GetURL() string { + return s.URL +} diff --git a/bundle/config/resources/permission.go b/bundle/config/resources/permission.go index fa2d8796..62e18a09 100644 --- a/bundle/config/resources/permission.go +++ b/bundle/config/resources/permission.go @@ -1,5 +1,7 @@ package resources +import "fmt" + // Permission holds the permission level setting for a single principal. // Multiple of these can be defined on any resource. type Permission struct { @@ -9,3 +11,19 @@ type Permission struct { ServicePrincipalName string `json:"service_principal_name,omitempty"` GroupName string `json:"group_name,omitempty"` } + +func (p Permission) String() string { + if p.UserName != "" { + return fmt.Sprintf("level: %s, user_name: %s", p.Level, p.UserName) + } + + if p.ServicePrincipalName != "" { + return fmt.Sprintf("level: %s, service_principal_name: %s", p.Level, p.ServicePrincipalName) + } + + if p.GroupName != "" { + return fmt.Sprintf("level: %s, group_name: %s", p.Level, p.GroupName) + } + + return fmt.Sprintf("level: %s", p.Level) +} diff --git a/bundle/config/resources/pipeline.go b/bundle/config/resources/pipeline.go index 55270be6..b3311d8e 100644 --- a/bundle/config/resources/pipeline.go +++ b/bundle/config/resources/pipeline.go @@ -2,6 +2,8 @@ package resources import ( "context" + "fmt" + "net/url" "github.com/databricks/cli/libs/log" "github.com/databricks/databricks-sdk-go" @@ -13,6 +15,7 @@ type Pipeline struct { ID string `json:"id,omitempty" bundle:"readonly"` Permissions []Permission `json:"permissions,omitempty"` ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"internal"` + URL string `json:"url,omitempty" bundle:"internal"` *pipelines.PipelineSpec } @@ -39,3 +42,19 @@ func (p *Pipeline) Exists(ctx context.Context, w *databricks.WorkspaceClient, id func (p *Pipeline) TerraformResourceName() string { return "databricks_pipeline" } + +func (p *Pipeline) InitializeURL(baseURL url.URL) { + if p.ID == "" { + return + } + baseURL.Path = fmt.Sprintf("pipelines/%s", p.ID) + p.URL = baseURL.String() +} + +func (p *Pipeline) GetName() string { + return p.Name +} + +func (s *Pipeline) GetURL() string { + return s.URL +} diff --git a/bundle/config/resources/quality_monitor.go b/bundle/config/resources/quality_monitor.go index 9160782c..3c823e62 100644 --- a/bundle/config/resources/quality_monitor.go +++ b/bundle/config/resources/quality_monitor.go @@ -2,6 +2,9 @@ package resources import ( "context" + "fmt" + "net/url" + "strings" "github.com/databricks/cli/libs/log" "github.com/databricks/databricks-sdk-go" @@ -20,6 +23,7 @@ type QualityMonitor struct { ID string `json:"id,omitempty" bundle:"readonly"` ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"internal"` + URL string `json:"url,omitempty" bundle:"internal"` } func (s *QualityMonitor) UnmarshalJSON(b []byte) error { @@ -44,3 +48,19 @@ func (s *QualityMonitor) Exists(ctx context.Context, w *databricks.WorkspaceClie func (s *QualityMonitor) TerraformResourceName() string { return "databricks_quality_monitor" } + +func (s *QualityMonitor) InitializeURL(baseURL url.URL) { + if s.TableName == "" { + return + } + baseURL.Path = fmt.Sprintf("explore/data/%s", strings.ReplaceAll(s.TableName, ".", "/")) + s.URL = baseURL.String() +} + +func (s *QualityMonitor) GetName() string { + return s.TableName +} + +func (s *QualityMonitor) GetURL() string { + return s.URL +} diff --git a/bundle/config/resources/registered_model.go b/bundle/config/resources/registered_model.go index 6033ffdf..c44526d0 100644 --- a/bundle/config/resources/registered_model.go +++ b/bundle/config/resources/registered_model.go @@ -2,6 +2,9 @@ package resources import ( "context" + "fmt" + "net/url" + "strings" "github.com/databricks/cli/libs/log" "github.com/databricks/databricks-sdk-go" @@ -24,6 +27,7 @@ type RegisteredModel struct { *catalog.CreateRegisteredModelRequest ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"internal"` + URL string `json:"url,omitempty" bundle:"internal"` } func (s *RegisteredModel) UnmarshalJSON(b []byte) error { @@ -48,3 +52,19 @@ func (s *RegisteredModel) Exists(ctx context.Context, w *databricks.WorkspaceCli func (s *RegisteredModel) TerraformResourceName() string { return "databricks_registered_model" } + +func (s *RegisteredModel) InitializeURL(baseURL url.URL) { + if s.ID == "" { + return + } + baseURL.Path = fmt.Sprintf("explore/data/models/%s", strings.ReplaceAll(s.ID, ".", "/")) + s.URL = baseURL.String() +} + +func (s *RegisteredModel) GetName() string { + return s.Name +} + +func (s *RegisteredModel) GetURL() string { + return s.URL +} diff --git a/bundle/config/resources/schema.go b/bundle/config/resources/schema.go index 7ab00495..a9f905cf 100644 --- a/bundle/config/resources/schema.go +++ b/bundle/config/resources/schema.go @@ -1,6 +1,12 @@ package resources import ( + "context" + "fmt" + "net/url" + "strings" + + "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/marshal" "github.com/databricks/databricks-sdk-go/service/catalog" ) @@ -16,6 +22,31 @@ type Schema struct { *catalog.CreateSchema ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"internal"` + URL string `json:"url,omitempty" bundle:"internal"` +} + +func (s *Schema) Exists(ctx context.Context, w *databricks.WorkspaceClient, id string) (bool, error) { + return false, fmt.Errorf("schema.Exists() is not supported") +} + +func (s *Schema) TerraformResourceName() string { + return "databricks_schema" +} + +func (s *Schema) InitializeURL(baseURL url.URL) { + if s.ID == "" { + return + } + baseURL.Path = fmt.Sprintf("explore/data/%s", strings.ReplaceAll(s.ID, ".", "/")) + s.URL = baseURL.String() +} + +func (s *Schema) GetURL() string { + return s.URL +} + +func (s *Schema) GetName() string { + return s.Name } func (s *Schema) UnmarshalJSON(b []byte) error { diff --git a/bundle/config/resources_test.go b/bundle/config/resources_test.go index c1b76118..9ae73b22 100644 --- a/bundle/config/resources_test.go +++ b/bundle/config/resources_test.go @@ -63,17 +63,37 @@ func TestCustomMarshallerIsImplemented(t *testing.T) { } } +func TestResourcesAllResourcesCompleteness(t *testing.T) { + r := Resources{} + rt := reflect.TypeOf(r) + + // Collect set of includes resource types + var types []string + for _, group := range r.AllResources() { + types = append(types, group.Description.PluralName) + } + + for i := 0; i < rt.NumField(); i++ { + field := rt.Field(i) + jsonTag := field.Tag.Get("json") + + if idx := strings.Index(jsonTag, ","); idx != -1 { + jsonTag = jsonTag[:idx] + } + + assert.Contains(t, types, jsonTag, "Field %s is missing in AllResources", field.Name) + } +} + func TestSupportedResources(t *testing.T) { - expected := map[string]ResourceDescription{} + // Please add your resource to the SupportedResources() function in resources.go if you add a new resource. + actual := SupportedResources() + typ := reflect.TypeOf(Resources{}) for i := 0; i < typ.NumField(); i++ { field := typ.Field(i) jsonTags := strings.Split(field.Tag.Get("json"), ",") - singularName := strings.TrimSuffix(jsonTags[0], "s") - expected[jsonTags[0]] = ResourceDescription{SingularName: singularName} + pluralName := jsonTags[0] + assert.Equal(t, actual[pluralName].PluralName, pluralName) } - - // Please add your resource to the SupportedResources() function in resources.go - // if you are adding a new resource. - assert.Equal(t, expected, SupportedResources()) } diff --git a/bundle/config/validate/folder_permissions.go b/bundle/config/validate/folder_permissions.go new file mode 100644 index 00000000..a376bd77 --- /dev/null +++ b/bundle/config/validate/folder_permissions.go @@ -0,0 +1,126 @@ +package validate + +import ( + "context" + "fmt" + "path" + "strings" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/libraries" + "github.com/databricks/cli/bundle/permissions" + "github.com/databricks/cli/libs/diag" + "github.com/databricks/databricks-sdk-go/apierr" + "github.com/databricks/databricks-sdk-go/service/workspace" + "golang.org/x/sync/errgroup" +) + +type folderPermissions struct { +} + +// Apply implements bundle.ReadOnlyMutator. +func (f *folderPermissions) Apply(ctx context.Context, b bundle.ReadOnlyBundle) diag.Diagnostics { + if len(b.Config().Permissions) == 0 { + return nil + } + + rootPath := b.Config().Workspace.RootPath + paths := []string{} + if !libraries.IsVolumesPath(rootPath) && !libraries.IsWorkspaceSharedPath(rootPath) { + paths = append(paths, rootPath) + } + + if !strings.HasSuffix(rootPath, "/") { + rootPath += "/" + } + + for _, p := range []string{ + b.Config().Workspace.ArtifactPath, + b.Config().Workspace.FilePath, + b.Config().Workspace.StatePath, + b.Config().Workspace.ResourcePath, + } { + if libraries.IsWorkspaceSharedPath(p) || libraries.IsVolumesPath(p) { + continue + } + + if strings.HasPrefix(p, rootPath) { + continue + } + + paths = append(paths, p) + } + + var diags diag.Diagnostics + g, ctx := errgroup.WithContext(ctx) + results := make([]diag.Diagnostics, len(paths)) + for i, p := range paths { + g.Go(func() error { + results[i] = checkFolderPermission(ctx, b, p) + return nil + }) + } + + if err := g.Wait(); err != nil { + return diag.FromErr(err) + } + + for _, r := range results { + diags = diags.Extend(r) + } + + return diags +} + +func checkFolderPermission(ctx context.Context, b bundle.ReadOnlyBundle, folderPath string) diag.Diagnostics { + w := b.WorkspaceClient().Workspace + obj, err := getClosestExistingObject(ctx, w, folderPath) + if err != nil { + return diag.FromErr(err) + } + + objPermissions, err := w.GetPermissions(ctx, workspace.GetWorkspaceObjectPermissionsRequest{ + WorkspaceObjectId: fmt.Sprint(obj.ObjectId), + WorkspaceObjectType: "directories", + }) + if err != nil { + return diag.FromErr(err) + } + + p := permissions.ObjectAclToResourcePermissions(folderPath, objPermissions.AccessControlList) + return p.Compare(b.Config().Permissions) +} + +func getClosestExistingObject(ctx context.Context, w workspace.WorkspaceInterface, folderPath string) (*workspace.ObjectInfo, error) { + for { + obj, err := w.GetStatusByPath(ctx, folderPath) + if err == nil { + return obj, nil + } + + if !apierr.IsMissing(err) { + return nil, err + } + + parent := path.Dir(folderPath) + // If the parent is the same as the current folder, then we have reached the root + if folderPath == parent { + break + } + + folderPath = parent + } + + return nil, fmt.Errorf("folder %s and its parent folders do not exist", folderPath) +} + +// Name implements bundle.ReadOnlyMutator. +func (f *folderPermissions) Name() string { + return "validate:folder_permissions" +} + +// ValidateFolderPermissions validates that permissions for the folders in Workspace file system matches +// the permissions in the top-level permissions section of the bundle. +func ValidateFolderPermissions() bundle.ReadOnlyMutator { + return &folderPermissions{} +} diff --git a/bundle/config/validate/folder_permissions_test.go b/bundle/config/validate/folder_permissions_test.go new file mode 100644 index 00000000..8e68c9fb --- /dev/null +++ b/bundle/config/validate/folder_permissions_test.go @@ -0,0 +1,208 @@ +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/bundle/permissions" + "github.com/databricks/cli/libs/diag" + "github.com/databricks/databricks-sdk-go/apierr" + "github.com/databricks/databricks-sdk-go/experimental/mocks" + "github.com/databricks/databricks-sdk-go/service/workspace" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +func TestFolderPermissionsInheritedWhenRootPathDoesNotExist(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Workspace: config.Workspace{ + RootPath: "/Workspace/Users/foo@bar.com", + ArtifactPath: "/Workspace/Users/otherfoo@bar.com/artifacts", + FilePath: "/Workspace/Users/foo@bar.com/files", + StatePath: "/Workspace/Users/foo@bar.com/state", + ResourcePath: "/Workspace/Users/foo@bar.com/resources", + }, + Permissions: []resources.Permission{ + {Level: permissions.CAN_MANAGE, UserName: "foo@bar.com"}, + }, + }, + } + m := mocks.NewMockWorkspaceClient(t) + api := m.GetMockWorkspaceAPI() + api.EXPECT().GetStatusByPath(mock.Anything, "/Workspace/Users/otherfoo@bar.com/artifacts").Return(nil, &apierr.APIError{ + StatusCode: 404, + ErrorCode: "RESOURCE_DOES_NOT_EXIST", + }) + api.EXPECT().GetStatusByPath(mock.Anything, "/Workspace/Users/otherfoo@bar.com").Return(nil, &apierr.APIError{ + StatusCode: 404, + ErrorCode: "RESOURCE_DOES_NOT_EXIST", + }) + api.EXPECT().GetStatusByPath(mock.Anything, "/Workspace/Users/foo@bar.com").Return(nil, &apierr.APIError{ + StatusCode: 404, + ErrorCode: "RESOURCE_DOES_NOT_EXIST", + }) + api.EXPECT().GetStatusByPath(mock.Anything, "/Workspace/Users").Return(nil, &apierr.APIError{ + StatusCode: 404, + ErrorCode: "RESOURCE_DOES_NOT_EXIST", + }) + api.EXPECT().GetStatusByPath(mock.Anything, "/Workspace").Return(&workspace.ObjectInfo{ + ObjectId: 1234, + }, nil) + + api.EXPECT().GetPermissions(mock.Anything, workspace.GetWorkspaceObjectPermissionsRequest{ + WorkspaceObjectId: "1234", + WorkspaceObjectType: "directories", + }).Return(&workspace.WorkspaceObjectPermissions{ + ObjectId: "1234", + AccessControlList: []workspace.WorkspaceObjectAccessControlResponse{ + { + UserName: "foo@bar.com", + AllPermissions: []workspace.WorkspaceObjectPermission{ + {PermissionLevel: "CAN_MANAGE"}, + }, + }, + }, + }, nil) + + b.SetWorkpaceClient(m.WorkspaceClient) + rb := bundle.ReadOnly(b) + + diags := bundle.ApplyReadOnly(context.Background(), rb, ValidateFolderPermissions()) + require.Empty(t, diags) +} + +func TestValidateFolderPermissionsFailsOnMissingBundlePermission(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Workspace: config.Workspace{ + RootPath: "/Workspace/Users/foo@bar.com", + ArtifactPath: "/Workspace/Users/foo@bar.com/artifacts", + FilePath: "/Workspace/Users/foo@bar.com/files", + StatePath: "/Workspace/Users/foo@bar.com/state", + ResourcePath: "/Workspace/Users/foo@bar.com/resources", + }, + Permissions: []resources.Permission{ + {Level: permissions.CAN_MANAGE, UserName: "foo@bar.com"}, + }, + }, + } + m := mocks.NewMockWorkspaceClient(t) + api := m.GetMockWorkspaceAPI() + api.EXPECT().GetStatusByPath(mock.Anything, "/Workspace/Users/foo@bar.com").Return(&workspace.ObjectInfo{ + ObjectId: 1234, + }, nil) + + api.EXPECT().GetPermissions(mock.Anything, workspace.GetWorkspaceObjectPermissionsRequest{ + WorkspaceObjectId: "1234", + WorkspaceObjectType: "directories", + }).Return(&workspace.WorkspaceObjectPermissions{ + ObjectId: "1234", + AccessControlList: []workspace.WorkspaceObjectAccessControlResponse{ + { + UserName: "foo@bar.com", + AllPermissions: []workspace.WorkspaceObjectPermission{ + {PermissionLevel: "CAN_MANAGE"}, + }, + }, + { + UserName: "foo2@bar.com", + AllPermissions: []workspace.WorkspaceObjectPermission{ + {PermissionLevel: "CAN_MANAGE"}, + }, + }, + }, + }, nil) + + b.SetWorkpaceClient(m.WorkspaceClient) + rb := bundle.ReadOnly(b) + + diags := bundle.ApplyReadOnly(context.Background(), rb, ValidateFolderPermissions()) + require.Len(t, diags, 1) + require.Equal(t, "untracked permissions apply to target workspace path", diags[0].Summary) + require.Equal(t, diag.Warning, diags[0].Severity) + require.Equal(t, "The following permissions apply to the workspace folder at \"/Workspace/Users/foo@bar.com\" but are not configured in the bundle:\n- level: CAN_MANAGE, user_name: foo2@bar.com\n", diags[0].Detail) +} + +func TestValidateFolderPermissionsFailsOnPermissionMismatch(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Workspace: config.Workspace{ + RootPath: "/Workspace/Users/foo@bar.com", + ArtifactPath: "/Workspace/Users/foo@bar.com/artifacts", + FilePath: "/Workspace/Users/foo@bar.com/files", + StatePath: "/Workspace/Users/foo@bar.com/state", + ResourcePath: "/Workspace/Users/foo@bar.com/resources", + }, + Permissions: []resources.Permission{ + {Level: permissions.CAN_MANAGE, UserName: "foo@bar.com"}, + }, + }, + } + m := mocks.NewMockWorkspaceClient(t) + api := m.GetMockWorkspaceAPI() + api.EXPECT().GetStatusByPath(mock.Anything, "/Workspace/Users/foo@bar.com").Return(&workspace.ObjectInfo{ + ObjectId: 1234, + }, nil) + + api.EXPECT().GetPermissions(mock.Anything, workspace.GetWorkspaceObjectPermissionsRequest{ + WorkspaceObjectId: "1234", + WorkspaceObjectType: "directories", + }).Return(&workspace.WorkspaceObjectPermissions{ + ObjectId: "1234", + AccessControlList: []workspace.WorkspaceObjectAccessControlResponse{ + { + UserName: "foo2@bar.com", + AllPermissions: []workspace.WorkspaceObjectPermission{ + {PermissionLevel: "CAN_MANAGE"}, + }, + }, + }, + }, nil) + + b.SetWorkpaceClient(m.WorkspaceClient) + rb := bundle.ReadOnly(b) + + diags := bundle.ApplyReadOnly(context.Background(), rb, ValidateFolderPermissions()) + require.Len(t, diags, 1) + require.Equal(t, "untracked permissions apply to target workspace path", diags[0].Summary) + require.Equal(t, diag.Warning, diags[0].Severity) +} + +func TestValidateFolderPermissionsFailsOnNoRootFolder(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Workspace: config.Workspace{ + RootPath: "/NotExisting", + ArtifactPath: "/NotExisting/artifacts", + FilePath: "/NotExisting/files", + StatePath: "/NotExisting/state", + ResourcePath: "/NotExisting/resources", + }, + Permissions: []resources.Permission{ + {Level: permissions.CAN_MANAGE, UserName: "foo@bar.com"}, + }, + }, + } + m := mocks.NewMockWorkspaceClient(t) + api := m.GetMockWorkspaceAPI() + api.EXPECT().GetStatusByPath(mock.Anything, "/NotExisting").Return(nil, &apierr.APIError{ + StatusCode: 404, + ErrorCode: "RESOURCE_DOES_NOT_EXIST", + }) + api.EXPECT().GetStatusByPath(mock.Anything, "/").Return(nil, &apierr.APIError{ + StatusCode: 404, + ErrorCode: "RESOURCE_DOES_NOT_EXIST", + }) + + b.SetWorkpaceClient(m.WorkspaceClient) + rb := bundle.ReadOnly(b) + + diags := bundle.ApplyReadOnly(context.Background(), rb, ValidateFolderPermissions()) + require.Len(t, diags, 1) + require.Equal(t, "folder / and its parent folders do not exist", diags[0].Summary) + require.Equal(t, diag.Error, diags[0].Severity) +} diff --git a/bundle/config/validate/validate.go b/bundle/config/validate/validate.go index 79f42bd2..440477e6 100644 --- a/bundle/config/validate/validate.go +++ b/bundle/config/validate/validate.go @@ -35,6 +35,7 @@ func (v *validate) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics FilesToSync(), ValidateSyncPatterns(), JobTaskClusterSpec(), + ValidateFolderPermissions(), )) } diff --git a/bundle/deploy/terraform/check_dashboards_modified_remotely.go b/bundle/deploy/terraform/check_dashboards_modified_remotely.go new file mode 100644 index 00000000..c884bcb9 --- /dev/null +++ b/bundle/deploy/terraform/check_dashboards_modified_remotely.go @@ -0,0 +1,117 @@ +package terraform + +import ( + "context" + "fmt" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/dyn" + tfjson "github.com/hashicorp/terraform-json" +) + +type dashboardState struct { + Name string + ID string + ETag string +} + +func collectDashboardsFromState(ctx context.Context, b *bundle.Bundle) ([]dashboardState, error) { + state, err := ParseResourcesState(ctx, b) + if err != nil && state == nil { + return nil, err + } + + var dashboards []dashboardState + for _, resource := range state.Resources { + if resource.Mode != tfjson.ManagedResourceMode { + continue + } + for _, instance := range resource.Instances { + switch resource.Type { + case "databricks_dashboard": + dashboards = append(dashboards, dashboardState{ + Name: resource.Name, + ID: instance.Attributes.ID, + ETag: instance.Attributes.ETag, + }) + } + } + } + + return dashboards, nil +} + +type checkDashboardsModifiedRemotely struct { +} + +func (l *checkDashboardsModifiedRemotely) Name() string { + return "CheckDashboardsModifiedRemotely" +} + +func (l *checkDashboardsModifiedRemotely) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + // This mutator is relevant only if the bundle includes dashboards. + if len(b.Config.Resources.Dashboards) == 0 { + return nil + } + + // If the user has forced the deployment, skip this check. + if b.Config.Bundle.Force { + return nil + } + + dashboards, err := collectDashboardsFromState(ctx, b) + if err != nil { + return diag.FromErr(err) + } + + var diags diag.Diagnostics + for _, dashboard := range dashboards { + // Skip dashboards that are not defined in the bundle. + // These will be destroyed upon deployment. + if _, ok := b.Config.Resources.Dashboards[dashboard.Name]; !ok { + continue + } + + path := dyn.MustPathFromString(fmt.Sprintf("resources.dashboards.%s", dashboard.Name)) + loc := b.Config.GetLocation(path.String()) + actual, err := b.WorkspaceClient().Lakeview.GetByDashboardId(ctx, dashboard.ID) + if err != nil { + diags = diags.Append(diag.Diagnostic{ + Severity: diag.Error, + Summary: fmt.Sprintf("failed to get dashboard %q", dashboard.Name), + Detail: err.Error(), + Paths: []dyn.Path{path}, + Locations: []dyn.Location{loc}, + }) + continue + } + + // If the ETag is the same, the dashboard has not been modified. + if actual.Etag == dashboard.ETag { + continue + } + + diags = diags.Append(diag.Diagnostic{ + Severity: diag.Error, + Summary: fmt.Sprintf("dashboard %q has been modified remotely", dashboard.Name), + Detail: "" + + "This dashboard has been modified remotely since the last bundle deployment.\n" + + "These modifications are untracked and will be overwritten on deploy.\n" + + "\n" + + "Make sure that the local dashboard definition matches what you intend to deploy\n" + + "before proceeding with the deployment.\n" + + "\n" + + "Run `databricks bundle deploy --force` to bypass this error." + + "", + Paths: []dyn.Path{path}, + Locations: []dyn.Location{loc}, + }) + } + + return diags +} + +func CheckDashboardsModifiedRemotely() *checkDashboardsModifiedRemotely { + return &checkDashboardsModifiedRemotely{} +} diff --git a/bundle/deploy/terraform/check_dashboards_modified_remotely_test.go b/bundle/deploy/terraform/check_dashboards_modified_remotely_test.go new file mode 100644 index 00000000..c13f800f --- /dev/null +++ b/bundle/deploy/terraform/check_dashboards_modified_remotely_test.go @@ -0,0 +1,191 @@ +package terraform + +import ( + "context" + "fmt" + "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/internal/testutil" + "github.com/databricks/cli/libs/diag" + "github.com/databricks/databricks-sdk-go/experimental/mocks" + "github.com/databricks/databricks-sdk-go/service/dashboards" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +func mockDashboardBundle(t *testing.T) *bundle.Bundle { + dir := t.TempDir() + b := &bundle.Bundle{ + BundleRootPath: dir, + Config: config.Root{ + Bundle: config.Bundle{ + Target: "test", + }, + Resources: config.Resources{ + Dashboards: map[string]*resources.Dashboard{ + "dash1": { + CreateDashboardRequest: &dashboards.CreateDashboardRequest{ + DisplayName: "My Special Dashboard", + }, + }, + }, + }, + }, + } + return b +} + +func TestCheckDashboardsModifiedRemotely_NoDashboards(t *testing.T) { + dir := t.TempDir() + b := &bundle.Bundle{ + BundleRootPath: dir, + Config: config.Root{ + Bundle: config.Bundle{ + Target: "test", + }, + Resources: config.Resources{}, + }, + } + + diags := bundle.Apply(context.Background(), b, CheckDashboardsModifiedRemotely()) + assert.Empty(t, diags) +} + +func TestCheckDashboardsModifiedRemotely_FirstDeployment(t *testing.T) { + b := mockDashboardBundle(t) + diags := bundle.Apply(context.Background(), b, CheckDashboardsModifiedRemotely()) + assert.Empty(t, diags) +} + +func TestCheckDashboardsModifiedRemotely_ExistingStateNoChange(t *testing.T) { + ctx := context.Background() + + b := mockDashboardBundle(t) + writeFakeDashboardState(t, ctx, b) + + // Mock the call to the API. + m := mocks.NewMockWorkspaceClient(t) + dashboardsAPI := m.GetMockLakeviewAPI() + dashboardsAPI.EXPECT(). + GetByDashboardId(mock.Anything, "id1"). + Return(&dashboards.Dashboard{ + DisplayName: "My Special Dashboard", + Etag: "1000", + }, nil). + Once() + b.SetWorkpaceClient(m.WorkspaceClient) + + // No changes, so no diags. + diags := bundle.Apply(ctx, b, CheckDashboardsModifiedRemotely()) + assert.Empty(t, diags) +} + +func TestCheckDashboardsModifiedRemotely_ExistingStateChange(t *testing.T) { + ctx := context.Background() + + b := mockDashboardBundle(t) + writeFakeDashboardState(t, ctx, b) + + // Mock the call to the API. + m := mocks.NewMockWorkspaceClient(t) + dashboardsAPI := m.GetMockLakeviewAPI() + dashboardsAPI.EXPECT(). + GetByDashboardId(mock.Anything, "id1"). + Return(&dashboards.Dashboard{ + DisplayName: "My Special Dashboard", + Etag: "1234", + }, nil). + Once() + b.SetWorkpaceClient(m.WorkspaceClient) + + // The dashboard has changed, so expect an error. + diags := bundle.Apply(ctx, b, CheckDashboardsModifiedRemotely()) + if assert.Len(t, diags, 1) { + assert.Equal(t, diag.Error, diags[0].Severity) + assert.Equal(t, `dashboard "dash1" has been modified remotely`, diags[0].Summary) + } +} + +func TestCheckDashboardsModifiedRemotely_ExistingStateFailureToGet(t *testing.T) { + ctx := context.Background() + + b := mockDashboardBundle(t) + writeFakeDashboardState(t, ctx, b) + + // Mock the call to the API. + m := mocks.NewMockWorkspaceClient(t) + dashboardsAPI := m.GetMockLakeviewAPI() + dashboardsAPI.EXPECT(). + GetByDashboardId(mock.Anything, "id1"). + Return(nil, fmt.Errorf("failure")). + Once() + b.SetWorkpaceClient(m.WorkspaceClient) + + // Unable to get the dashboard, so expect an error. + diags := bundle.Apply(ctx, b, CheckDashboardsModifiedRemotely()) + if assert.Len(t, diags, 1) { + assert.Equal(t, diag.Error, diags[0].Severity) + assert.Equal(t, `failed to get dashboard "dash1"`, diags[0].Summary) + } +} + +func writeFakeDashboardState(t *testing.T, ctx context.Context, b *bundle.Bundle) { + tfDir, err := Dir(ctx, b) + require.NoError(t, err) + + // Write fake state file. + testutil.WriteFile(t, ` + { + "version": 4, + "terraform_version": "1.5.5", + "resources": [ + { + "mode": "managed", + "type": "databricks_dashboard", + "name": "dash1", + "instances": [ + { + "schema_version": 0, + "attributes": { + "etag": "1000", + "id": "id1" + } + } + ] + }, + { + "mode": "managed", + "type": "databricks_job", + "name": "job", + "instances": [ + { + "schema_version": 0, + "attributes": { + "id": "1234" + } + } + ] + }, + { + "mode": "managed", + "type": "databricks_dashboard", + "name": "dash2", + "instances": [ + { + "schema_version": 0, + "attributes": { + "etag": "1001", + "id": "id2" + } + } + ] + } + ] + } + `, filepath.Join(tfDir, TerraformStateFileName)) +} diff --git a/bundle/deploy/terraform/convert.go b/bundle/deploy/terraform/convert.go index b42f3c0d..b710c690 100644 --- a/bundle/deploy/terraform/convert.go +++ b/bundle/deploy/terraform/convert.go @@ -2,9 +2,7 @@ package terraform import ( "context" - "encoding/json" "fmt" - "sort" "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/config/resources" @@ -14,244 +12,6 @@ import ( tfjson "github.com/hashicorp/terraform-json" ) -func conv(from any, to any) { - buf, _ := json.Marshal(from) - json.Unmarshal(buf, &to) -} - -func convPermissions(acl []resources.Permission) *schema.ResourcePermissions { - if len(acl) == 0 { - return nil - } - - resource := schema.ResourcePermissions{} - for _, ac := range acl { - resource.AccessControl = append(resource.AccessControl, convPermission(ac)) - } - - return &resource -} - -func convPermission(ac resources.Permission) schema.ResourcePermissionsAccessControl { - dst := schema.ResourcePermissionsAccessControl{ - PermissionLevel: ac.Level, - } - if ac.UserName != "" { - dst.UserName = ac.UserName - } - if ac.GroupName != "" { - dst.GroupName = ac.GroupName - } - if ac.ServicePrincipalName != "" { - dst.ServicePrincipalName = ac.ServicePrincipalName - } - return dst -} - -func convGrants(acl []resources.Grant) *schema.ResourceGrants { - if len(acl) == 0 { - return nil - } - - resource := schema.ResourceGrants{} - for _, ac := range acl { - resource.Grant = append(resource.Grant, schema.ResourceGrantsGrant{ - Privileges: ac.Privileges, - Principal: ac.Principal, - }) - } - - return &resource -} - -// BundleToTerraform converts resources in a bundle configuration -// to the equivalent Terraform JSON representation. -// -// Note: This function is an older implementation of the conversion logic. It is -// no longer used in any code paths. It is kept around to be used in tests. -// New resources do not need to modify this function and can instead can define -// the conversion login in the tfdyn package. -func BundleToTerraform(config *config.Root) *schema.Root { - tfroot := schema.NewRoot() - tfroot.Provider = schema.NewProviders() - tfroot.Resource = schema.NewResources() - noResources := true - - for k, src := range config.Resources.Jobs { - noResources = false - var dst schema.ResourceJob - conv(src, &dst) - - if src.JobSettings != nil { - sort.Slice(src.JobSettings.Tasks, func(i, j int) bool { - return src.JobSettings.Tasks[i].TaskKey < src.JobSettings.Tasks[j].TaskKey - }) - - for _, v := range src.Tasks { - var t schema.ResourceJobTask - conv(v, &t) - - for _, v_ := range v.Libraries { - var l schema.ResourceJobTaskLibrary - conv(v_, &l) - t.Library = append(t.Library, l) - } - - // Convert for_each_task libraries - if v.ForEachTask != nil { - for _, v_ := range v.ForEachTask.Task.Libraries { - var l schema.ResourceJobTaskForEachTaskTaskLibrary - conv(v_, &l) - t.ForEachTask.Task.Library = append(t.ForEachTask.Task.Library, l) - } - - } - - dst.Task = append(dst.Task, t) - } - - for _, v := range src.JobClusters { - var t schema.ResourceJobJobCluster - conv(v, &t) - dst.JobCluster = append(dst.JobCluster, t) - } - - // Unblock downstream work. To be addressed more generally later. - if git := src.GitSource; git != nil { - dst.GitSource = &schema.ResourceJobGitSource{ - Url: git.GitUrl, - Branch: git.GitBranch, - Commit: git.GitCommit, - Provider: string(git.GitProvider), - Tag: git.GitTag, - } - } - - for _, v := range src.Parameters { - var t schema.ResourceJobParameter - conv(v, &t) - dst.Parameter = append(dst.Parameter, t) - } - } - - tfroot.Resource.Job[k] = &dst - - // Configure permissions for this resource. - if rp := convPermissions(src.Permissions); rp != nil { - rp.JobId = fmt.Sprintf("${databricks_job.%s.id}", k) - tfroot.Resource.Permissions["job_"+k] = rp - } - } - - for k, src := range config.Resources.Pipelines { - noResources = false - var dst schema.ResourcePipeline - conv(src, &dst) - - if src.PipelineSpec != nil { - for _, v := range src.Libraries { - var l schema.ResourcePipelineLibrary - conv(v, &l) - dst.Library = append(dst.Library, l) - } - - for _, v := range src.Clusters { - var l schema.ResourcePipelineCluster - conv(v, &l) - dst.Cluster = append(dst.Cluster, l) - } - - for _, v := range src.Notifications { - var l schema.ResourcePipelineNotification - conv(v, &l) - dst.Notification = append(dst.Notification, l) - } - } - - tfroot.Resource.Pipeline[k] = &dst - - // Configure permissions for this resource. - if rp := convPermissions(src.Permissions); rp != nil { - rp.PipelineId = fmt.Sprintf("${databricks_pipeline.%s.id}", k) - tfroot.Resource.Permissions["pipeline_"+k] = rp - } - } - - for k, src := range config.Resources.Models { - noResources = false - var dst schema.ResourceMlflowModel - conv(src, &dst) - tfroot.Resource.MlflowModel[k] = &dst - - // Configure permissions for this resource. - if rp := convPermissions(src.Permissions); rp != nil { - rp.RegisteredModelId = fmt.Sprintf("${databricks_mlflow_model.%s.registered_model_id}", k) - tfroot.Resource.Permissions["mlflow_model_"+k] = rp - } - } - - for k, src := range config.Resources.Experiments { - noResources = false - var dst schema.ResourceMlflowExperiment - conv(src, &dst) - tfroot.Resource.MlflowExperiment[k] = &dst - - // Configure permissions for this resource. - if rp := convPermissions(src.Permissions); rp != nil { - rp.ExperimentId = fmt.Sprintf("${databricks_mlflow_experiment.%s.id}", k) - tfroot.Resource.Permissions["mlflow_experiment_"+k] = rp - } - } - - for k, src := range config.Resources.ModelServingEndpoints { - noResources = false - var dst schema.ResourceModelServing - conv(src, &dst) - tfroot.Resource.ModelServing[k] = &dst - - // Configure permissions for this resource. - if rp := convPermissions(src.Permissions); rp != nil { - rp.ServingEndpointId = fmt.Sprintf("${databricks_model_serving.%s.serving_endpoint_id}", k) - tfroot.Resource.Permissions["model_serving_"+k] = rp - } - } - - for k, src := range config.Resources.RegisteredModels { - noResources = false - var dst schema.ResourceRegisteredModel - conv(src, &dst) - tfroot.Resource.RegisteredModel[k] = &dst - - // Configure permissions for this resource. - if rp := convGrants(src.Grants); rp != nil { - rp.Function = fmt.Sprintf("${databricks_registered_model.%s.id}", k) - tfroot.Resource.Grants["registered_model_"+k] = rp - } - } - - for k, src := range config.Resources.QualityMonitors { - noResources = false - var dst schema.ResourceQualityMonitor - conv(src, &dst) - tfroot.Resource.QualityMonitor[k] = &dst - } - - for k, src := range config.Resources.Clusters { - noResources = false - var dst schema.ResourceCluster - conv(src, &dst) - tfroot.Resource.Cluster[k] = &dst - } - - // We explicitly set "resource" to nil to omit it from a JSON encoding. - // This is required because the terraform CLI requires >= 1 resources defined - // if the "resource" property is used in a .tf.json file. - if noResources { - tfroot.Resource = nil - } - return tfroot -} - // BundleToTerraformWithDynValue converts resources in a bundle configuration // to the equivalent Terraform JSON representation. func BundleToTerraformWithDynValue(ctx context.Context, root dyn.Value) (*schema.Root, error) { @@ -426,6 +186,16 @@ func TerraformToBundle(state *resourcesState, config *config.Root) error { } cur.ID = instance.Attributes.ID config.Resources.Clusters[resource.Name] = cur + case "databricks_dashboard": + if config.Resources.Dashboards == nil { + config.Resources.Dashboards = make(map[string]*resources.Dashboard) + } + cur := config.Resources.Dashboards[resource.Name] + if cur == nil { + cur = &resources.Dashboard{ModifiedStatus: resources.ModifiedStatusDeleted} + } + cur.ID = instance.Attributes.ID + config.Resources.Dashboards[resource.Name] = cur case "databricks_permissions": case "databricks_grants": // Ignore; no need to pull these back into the configuration. @@ -485,6 +255,11 @@ func TerraformToBundle(state *resourcesState, config *config.Root) error { src.ModifiedStatus = resources.ModifiedStatusCreated } } + for _, src := range config.Resources.Dashboards { + 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 6808076b..d40f5514 100644 --- a/bundle/deploy/terraform/convert_test.go +++ b/bundle/deploy/terraform/convert_test.go @@ -2,7 +2,6 @@ package terraform import ( "context" - "encoding/json" "reflect" "testing" @@ -13,6 +12,7 @@ import ( "github.com/databricks/cli/libs/dyn/convert" "github.com/databricks/databricks-sdk-go/service/catalog" "github.com/databricks/databricks-sdk-go/service/compute" + "github.com/databricks/databricks-sdk-go/service/dashboards" "github.com/databricks/databricks-sdk-go/service/jobs" "github.com/databricks/databricks-sdk-go/service/ml" "github.com/databricks/databricks-sdk-go/service/pipelines" @@ -21,6 +21,27 @@ import ( "github.com/stretchr/testify/require" ) +func produceTerraformConfiguration(t *testing.T, config config.Root) *schema.Root { + vin, err := convert.FromTyped(config, dyn.NilValue) + require.NoError(t, err) + out, err := BundleToTerraformWithDynValue(context.Background(), vin) + require.NoError(t, err) + return out +} + +func convertToResourceStruct[T any](t *testing.T, resource *T, data any) { + require.NotNil(t, resource) + require.NotNil(t, data) + + // Convert data to a dyn.Value. + vin, err := convert.FromTyped(data, dyn.NilValue) + require.NoError(t, err) + + // Convert the dyn.Value to a struct. + err = convert.ToTyped(resource, vin) + require.NoError(t, err) +} + func TestBundleToTerraformJob(t *testing.T) { var src = resources.Job{ JobSettings: &jobs.JobSettings{ @@ -58,8 +79,9 @@ func TestBundleToTerraformJob(t *testing.T) { }, } - out := BundleToTerraform(&config) - resource := out.Resource.Job["my_job"].(*schema.ResourceJob) + var resource schema.ResourceJob + out := produceTerraformConfiguration(t, config) + convertToResourceStruct(t, &resource, out.Resource.Job["my_job"]) assert.Equal(t, "my job", resource.Name) assert.Len(t, resource.JobCluster, 1) @@ -68,8 +90,6 @@ func TestBundleToTerraformJob(t *testing.T) { assert.Equal(t, "param1", resource.Parameter[0].Name) assert.Equal(t, "param2", resource.Parameter[1].Name) assert.Nil(t, out.Data) - - bundleToTerraformEquivalenceTest(t, &config) } func TestBundleToTerraformJobPermissions(t *testing.T) { @@ -90,15 +110,14 @@ func TestBundleToTerraformJobPermissions(t *testing.T) { }, } - out := BundleToTerraform(&config) - resource := out.Resource.Permissions["job_my_job"].(*schema.ResourcePermissions) + var resource schema.ResourcePermissions + out := produceTerraformConfiguration(t, config) + convertToResourceStruct(t, &resource, out.Resource.Permissions["job_my_job"]) assert.NotEmpty(t, resource.JobId) assert.Len(t, resource.AccessControl, 1) assert.Equal(t, "jane@doe.com", resource.AccessControl[0].UserName) assert.Equal(t, "CAN_VIEW", resource.AccessControl[0].PermissionLevel) - - bundleToTerraformEquivalenceTest(t, &config) } func TestBundleToTerraformJobTaskLibraries(t *testing.T) { @@ -128,15 +147,14 @@ func TestBundleToTerraformJobTaskLibraries(t *testing.T) { }, } - out := BundleToTerraform(&config) - resource := out.Resource.Job["my_job"].(*schema.ResourceJob) + var resource schema.ResourceJob + out := produceTerraformConfiguration(t, config) + convertToResourceStruct(t, &resource, out.Resource.Job["my_job"]) assert.Equal(t, "my job", resource.Name) require.Len(t, resource.Task, 1) require.Len(t, resource.Task[0].Library, 1) assert.Equal(t, "mlflow", resource.Task[0].Library[0].Pypi.Package) - - bundleToTerraformEquivalenceTest(t, &config) } func TestBundleToTerraformForEachTaskLibraries(t *testing.T) { @@ -172,15 +190,14 @@ func TestBundleToTerraformForEachTaskLibraries(t *testing.T) { }, } - out := BundleToTerraform(&config) - resource := out.Resource.Job["my_job"].(*schema.ResourceJob) + var resource schema.ResourceJob + out := produceTerraformConfiguration(t, config) + convertToResourceStruct(t, &resource, out.Resource.Job["my_job"]) assert.Equal(t, "my job", resource.Name) require.Len(t, resource.Task, 1) require.Len(t, resource.Task[0].ForEachTask.Task.Library, 1) assert.Equal(t, "mlflow", resource.Task[0].ForEachTask.Task.Library[0].Pypi.Package) - - bundleToTerraformEquivalenceTest(t, &config) } func TestBundleToTerraformPipeline(t *testing.T) { @@ -230,8 +247,9 @@ func TestBundleToTerraformPipeline(t *testing.T) { }, } - out := BundleToTerraform(&config) - resource := out.Resource.Pipeline["my_pipeline"].(*schema.ResourcePipeline) + var resource schema.ResourcePipeline + out := produceTerraformConfiguration(t, config) + convertToResourceStruct(t, &resource, out.Resource.Pipeline["my_pipeline"]) assert.Equal(t, "my pipeline", resource.Name) assert.Len(t, resource.Library, 2) @@ -241,8 +259,6 @@ func TestBundleToTerraformPipeline(t *testing.T) { assert.Equal(t, resource.Notification[1].Alerts, []string{"on-update-failure", "on-flow-failure"}) assert.Equal(t, resource.Notification[1].EmailRecipients, []string{"jane@doe.com", "john@doe.com"}) assert.Nil(t, out.Data) - - bundleToTerraformEquivalenceTest(t, &config) } func TestBundleToTerraformPipelinePermissions(t *testing.T) { @@ -263,15 +279,14 @@ func TestBundleToTerraformPipelinePermissions(t *testing.T) { }, } - out := BundleToTerraform(&config) - resource := out.Resource.Permissions["pipeline_my_pipeline"].(*schema.ResourcePermissions) + var resource schema.ResourcePermissions + out := produceTerraformConfiguration(t, config) + convertToResourceStruct(t, &resource, out.Resource.Permissions["pipeline_my_pipeline"]) assert.NotEmpty(t, resource.PipelineId) assert.Len(t, resource.AccessControl, 1) assert.Equal(t, "jane@doe.com", resource.AccessControl[0].UserName) assert.Equal(t, "CAN_VIEW", resource.AccessControl[0].PermissionLevel) - - bundleToTerraformEquivalenceTest(t, &config) } func TestBundleToTerraformModel(t *testing.T) { @@ -300,8 +315,9 @@ func TestBundleToTerraformModel(t *testing.T) { }, } - out := BundleToTerraform(&config) - resource := out.Resource.MlflowModel["my_model"].(*schema.ResourceMlflowModel) + var resource schema.ResourceMlflowModel + out := produceTerraformConfiguration(t, config) + convertToResourceStruct(t, &resource, out.Resource.MlflowModel["my_model"]) assert.Equal(t, "name", resource.Name) assert.Equal(t, "description", resource.Description) @@ -311,8 +327,6 @@ func TestBundleToTerraformModel(t *testing.T) { assert.Equal(t, "k2", resource.Tags[1].Key) assert.Equal(t, "v2", resource.Tags[1].Value) assert.Nil(t, out.Data) - - bundleToTerraformEquivalenceTest(t, &config) } func TestBundleToTerraformModelPermissions(t *testing.T) { @@ -336,15 +350,14 @@ func TestBundleToTerraformModelPermissions(t *testing.T) { }, } - out := BundleToTerraform(&config) - resource := out.Resource.Permissions["mlflow_model_my_model"].(*schema.ResourcePermissions) + var resource schema.ResourcePermissions + out := produceTerraformConfiguration(t, config) + convertToResourceStruct(t, &resource, out.Resource.Permissions["mlflow_model_my_model"]) assert.NotEmpty(t, resource.RegisteredModelId) assert.Len(t, resource.AccessControl, 1) assert.Equal(t, "jane@doe.com", resource.AccessControl[0].UserName) assert.Equal(t, "CAN_READ", resource.AccessControl[0].PermissionLevel) - - bundleToTerraformEquivalenceTest(t, &config) } func TestBundleToTerraformExperiment(t *testing.T) { @@ -362,13 +375,12 @@ func TestBundleToTerraformExperiment(t *testing.T) { }, } - out := BundleToTerraform(&config) - resource := out.Resource.MlflowExperiment["my_experiment"].(*schema.ResourceMlflowExperiment) + var resource schema.ResourceMlflowExperiment + out := produceTerraformConfiguration(t, config) + convertToResourceStruct(t, &resource, out.Resource.MlflowExperiment["my_experiment"]) assert.Equal(t, "name", resource.Name) assert.Nil(t, out.Data) - - bundleToTerraformEquivalenceTest(t, &config) } func TestBundleToTerraformExperimentPermissions(t *testing.T) { @@ -392,15 +404,14 @@ func TestBundleToTerraformExperimentPermissions(t *testing.T) { }, } - out := BundleToTerraform(&config) - resource := out.Resource.Permissions["mlflow_experiment_my_experiment"].(*schema.ResourcePermissions) + var resource schema.ResourcePermissions + out := produceTerraformConfiguration(t, config) + convertToResourceStruct(t, &resource, out.Resource.Permissions["mlflow_experiment_my_experiment"]) assert.NotEmpty(t, resource.ExperimentId) assert.Len(t, resource.AccessControl, 1) assert.Equal(t, "jane@doe.com", resource.AccessControl[0].UserName) assert.Equal(t, "CAN_READ", resource.AccessControl[0].PermissionLevel) - - bundleToTerraformEquivalenceTest(t, &config) } func TestBundleToTerraformModelServing(t *testing.T) { @@ -436,8 +447,9 @@ func TestBundleToTerraformModelServing(t *testing.T) { }, } - out := BundleToTerraform(&config) - resource := out.Resource.ModelServing["my_model_serving_endpoint"].(*schema.ResourceModelServing) + var resource schema.ResourceModelServing + out := produceTerraformConfiguration(t, config) + convertToResourceStruct(t, &resource, out.Resource.ModelServing["my_model_serving_endpoint"]) assert.Equal(t, "name", resource.Name) assert.Equal(t, "model_name", resource.Config.ServedModels[0].ModelName) @@ -447,8 +459,6 @@ func TestBundleToTerraformModelServing(t *testing.T) { assert.Equal(t, "model_name-1", resource.Config.TrafficConfig.Routes[0].ServedModelName) assert.Equal(t, 100, resource.Config.TrafficConfig.Routes[0].TrafficPercentage) assert.Nil(t, out.Data) - - bundleToTerraformEquivalenceTest(t, &config) } func TestBundleToTerraformModelServingPermissions(t *testing.T) { @@ -490,15 +500,14 @@ func TestBundleToTerraformModelServingPermissions(t *testing.T) { }, } - out := BundleToTerraform(&config) - resource := out.Resource.Permissions["model_serving_my_model_serving_endpoint"].(*schema.ResourcePermissions) + var resource schema.ResourcePermissions + out := produceTerraformConfiguration(t, config) + convertToResourceStruct(t, &resource, out.Resource.Permissions["model_serving_my_model_serving_endpoint"]) assert.NotEmpty(t, resource.ServingEndpointId) assert.Len(t, resource.AccessControl, 1) assert.Equal(t, "jane@doe.com", resource.AccessControl[0].UserName) assert.Equal(t, "CAN_VIEW", resource.AccessControl[0].PermissionLevel) - - bundleToTerraformEquivalenceTest(t, &config) } func TestBundleToTerraformRegisteredModel(t *testing.T) { @@ -519,16 +528,15 @@ func TestBundleToTerraformRegisteredModel(t *testing.T) { }, } - out := BundleToTerraform(&config) - resource := out.Resource.RegisteredModel["my_registered_model"].(*schema.ResourceRegisteredModel) + var resource schema.ResourceRegisteredModel + out := produceTerraformConfiguration(t, config) + convertToResourceStruct(t, &resource, out.Resource.RegisteredModel["my_registered_model"]) assert.Equal(t, "name", resource.Name) assert.Equal(t, "catalog", resource.CatalogName) assert.Equal(t, "schema", resource.SchemaName) assert.Equal(t, "comment", resource.Comment) assert.Nil(t, out.Data) - - bundleToTerraformEquivalenceTest(t, &config) } func TestBundleToTerraformRegisteredModelGrants(t *testing.T) { @@ -554,15 +562,14 @@ func TestBundleToTerraformRegisteredModelGrants(t *testing.T) { }, } - out := BundleToTerraform(&config) - resource := out.Resource.Grants["registered_model_my_registered_model"].(*schema.ResourceGrants) + var resource schema.ResourceGrants + out := produceTerraformConfiguration(t, config) + convertToResourceStruct(t, &resource, out.Resource.Grants["registered_model_my_registered_model"]) assert.NotEmpty(t, resource.Function) assert.Len(t, resource.Grant, 1) assert.Equal(t, "jane@doe.com", resource.Grant[0].Principal) assert.Equal(t, "EXECUTE", resource.Grant[0].Privileges[0]) - - bundleToTerraformEquivalenceTest(t, &config) } func TestBundleToTerraformDeletedResources(t *testing.T) { @@ -679,6 +686,14 @@ func TestTerraformToBundleEmptyLocalResources(t *testing.T) { {Attributes: stateInstanceAttributes{ID: "1"}}, }, }, + { + Type: "databricks_dashboard", + Mode: "managed", + Name: "test_dashboard", + Instances: []stateResourceInstance{ + {Attributes: stateInstanceAttributes{ID: "1"}}, + }, + }, }, } err := TerraformToBundle(&tfState, &config) @@ -714,6 +729,9 @@ func TestTerraformToBundleEmptyLocalResources(t *testing.T) { assert.Equal(t, "1", config.Resources.Clusters["test_cluster"].ID) assert.Equal(t, resources.ModifiedStatusDeleted, config.Resources.Clusters["test_cluster"].ModifiedStatus) + assert.Equal(t, "1", config.Resources.Dashboards["test_dashboard"].ID) + assert.Equal(t, resources.ModifiedStatusDeleted, config.Resources.Dashboards["test_dashboard"].ModifiedStatus) + AssertFullResourceCoverage(t, &config) } @@ -790,6 +808,13 @@ func TestTerraformToBundleEmptyRemoteResources(t *testing.T) { }, }, }, + Dashboards: map[string]*resources.Dashboard{ + "test_dashboard": { + CreateDashboardRequest: &dashboards.CreateDashboardRequest{ + DisplayName: "test_dashboard", + }, + }, + }, }, } var tfState = resourcesState{ @@ -828,6 +853,9 @@ func TestTerraformToBundleEmptyRemoteResources(t *testing.T) { assert.Equal(t, "", config.Resources.Clusters["test_cluster"].ID) assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.Clusters["test_cluster"].ModifiedStatus) + assert.Equal(t, "", config.Resources.Dashboards["test_dashboard"].ID) + assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.Dashboards["test_dashboard"].ModifiedStatus) + AssertFullResourceCoverage(t, &config) } @@ -954,6 +982,18 @@ func TestTerraformToBundleModifiedResources(t *testing.T) { }, }, }, + Dashboards: map[string]*resources.Dashboard{ + "test_dashboard": { + CreateDashboardRequest: &dashboards.CreateDashboardRequest{ + DisplayName: "test_dashboard", + }, + }, + "test_dashboard_new": { + CreateDashboardRequest: &dashboards.CreateDashboardRequest{ + DisplayName: "test_dashboard_new", + }, + }, + }, }, } var tfState = resourcesState{ @@ -1118,6 +1158,22 @@ func TestTerraformToBundleModifiedResources(t *testing.T) { {Attributes: stateInstanceAttributes{ID: "2"}}, }, }, + { + Type: "databricks_dashboard", + Mode: "managed", + Name: "test_dashboard", + Instances: []stateResourceInstance{ + {Attributes: stateInstanceAttributes{ID: "1"}}, + }, + }, + { + Type: "databricks_dashboard", + Mode: "managed", + Name: "test_dashboard_old", + Instances: []stateResourceInstance{ + {Attributes: stateInstanceAttributes{ID: "2"}}, + }, + }, }, } err := TerraformToBundle(&tfState, &config) @@ -1193,6 +1249,13 @@ func TestTerraformToBundleModifiedResources(t *testing.T) { assert.Equal(t, "", config.Resources.Clusters["test_cluster_new"].ID) assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.Clusters["test_cluster_new"].ModifiedStatus) + assert.Equal(t, "1", config.Resources.Dashboards["test_dashboard"].ID) + assert.Equal(t, "", config.Resources.Dashboards["test_dashboard"].ModifiedStatus) + assert.Equal(t, "2", config.Resources.Dashboards["test_dashboard_old"].ID) + assert.Equal(t, resources.ModifiedStatusDeleted, config.Resources.Dashboards["test_dashboard_old"].ModifiedStatus) + assert.Equal(t, "", config.Resources.Dashboards["test_dashboard_new"].ID) + assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.Dashboards["test_dashboard_new"].ModifiedStatus) + AssertFullResourceCoverage(t, &config) } @@ -1210,25 +1273,3 @@ func AssertFullResourceCoverage(t *testing.T, config *config.Root) { } } } - -func assertEqualTerraformRoot(t *testing.T, a, b *schema.Root) { - ba, err := json.Marshal(a) - require.NoError(t, err) - bb, err := json.Marshal(b) - require.NoError(t, err) - assert.JSONEq(t, string(ba), string(bb)) -} - -func bundleToTerraformEquivalenceTest(t *testing.T, config *config.Root) { - t.Run("dyn equivalence", func(t *testing.T) { - tf1 := BundleToTerraform(config) - - vin, err := convert.FromTyped(config, dyn.NilValue) - require.NoError(t, err) - tf2, err := BundleToTerraformWithDynValue(context.Background(), vin) - require.NoError(t, err) - - // Compare roots - assertEqualTerraformRoot(t, tf1, tf2) - }) -} diff --git a/bundle/deploy/terraform/interpolate.go b/bundle/deploy/terraform/interpolate.go index 36074aad..9c2126ae 100644 --- a/bundle/deploy/terraform/interpolate.go +++ b/bundle/deploy/terraform/interpolate.go @@ -62,6 +62,8 @@ func (m *interpolateMutator) Apply(ctx context.Context, b *bundle.Bundle) diag.D path = dyn.NewPath(dyn.Key("databricks_volume")).Append(path[2:]...) case dyn.Key("clusters"): path = dyn.NewPath(dyn.Key("databricks_cluster")).Append(path[2:]...) + case dyn.Key("dashboards"): + path = dyn.NewPath(dyn.Key("databricks_dashboard")).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 4a14e0f6..fc5c4d18 100644 --- a/bundle/deploy/terraform/interpolate_test.go +++ b/bundle/deploy/terraform/interpolate_test.go @@ -33,6 +33,7 @@ func TestInterpolate(t *testing.T) { "other_schema": "${resources.schemas.other_schema.id}", "other_volume": "${resources.volumes.other_volume.id}", "other_cluster": "${resources.clusters.other_cluster.id}", + "other_dashboard": "${resources.dashboards.other_dashboard.id}", }, Tasks: []jobs.Task{ { @@ -71,6 +72,7 @@ func TestInterpolate(t *testing.T) { assert.Equal(t, "${databricks_schema.other_schema.id}", j.Tags["other_schema"]) assert.Equal(t, "${databricks_volume.other_volume.id}", j.Tags["other_volume"]) assert.Equal(t, "${databricks_cluster.other_cluster.id}", j.Tags["other_cluster"]) + assert.Equal(t, "${databricks_dashboard.other_dashboard.id}", j.Tags["other_dashboard"]) m := b.Config.Resources.Models["my_model"] assert.Equal(t, "my_model", m.Model.Name) diff --git a/bundle/deploy/terraform/tfdyn/convert_dashboard.go b/bundle/deploy/terraform/tfdyn/convert_dashboard.go new file mode 100644 index 00000000..3ba7e19a --- /dev/null +++ b/bundle/deploy/terraform/tfdyn/convert_dashboard.go @@ -0,0 +1,109 @@ +package tfdyn + +import ( + "context" + "encoding/json" + "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" +) + +const ( + filePathFieldName = "file_path" + serializedDashboardFieldName = "serialized_dashboard" +) + +// Marshal "serialized_dashboard" as JSON if it is set in the input but not in the output. +func marshalSerializedDashboard(vin dyn.Value, vout dyn.Value) (dyn.Value, error) { + // Skip if the "serialized_dashboard" field is already set. + if v := vout.Get(serializedDashboardFieldName); v.IsValid() { + return vout, nil + } + + // Skip if the "serialized_dashboard" field on the input is not set. + v := vin.Get(serializedDashboardFieldName) + if !v.IsValid() { + return vout, nil + } + + // Marshal the "serialized_dashboard" field as JSON. + data, err := json.Marshal(v.AsAny()) + if err != nil { + return dyn.InvalidValue, fmt.Errorf("failed to marshal serialized_dashboard: %w", err) + } + + // Set the "serialized_dashboard" field on the output. + return dyn.Set(vout, serializedDashboardFieldName, dyn.V(string(data))) +} + +func convertDashboardResource(ctx context.Context, vin dyn.Value) (dyn.Value, error) { + var err error + + // Normalize the output value to the target schema. + vout, diags := convert.Normalize(schema.ResourceDashboard{}, vin) + for _, diag := range diags { + log.Debugf(ctx, "dashboard normalization diagnostic: %s", diag.Summary) + } + + // Include "serialized_dashboard" field if "file_path" is set. + // Note: the Terraform resource supports "file_path" natively, but its + // change detection mechanism doesn't work as expected at the time of writing (Sep 30). + if path, ok := vout.Get(filePathFieldName).AsString(); ok { + vout, err = dyn.Set(vout, serializedDashboardFieldName, dyn.V(fmt.Sprintf("${file(%q)}", path))) + if err != nil { + return dyn.InvalidValue, fmt.Errorf("failed to set serialized_dashboard: %w", err) + } + // Drop the "file_path" field. It is mutually exclusive with "serialized_dashboard". + vout, err = dyn.Walk(vout, func(p dyn.Path, v dyn.Value) (dyn.Value, error) { + switch len(p) { + case 0: + return v, nil + case 1: + if p[0] == dyn.Key(filePathFieldName) { + return v, dyn.ErrDrop + } + } + + // Skip everything else. + return v, dyn.ErrSkip + }) + if err != nil { + return dyn.InvalidValue, fmt.Errorf("failed to drop file_path: %w", err) + } + } + + // Marshal "serialized_dashboard" as JSON if it is set in the input but not in the output. + vout, err = marshalSerializedDashboard(vin, vout) + if err != nil { + return dyn.InvalidValue, err + } + + return vout, nil +} + +type dashboardConverter struct{} + +func (dashboardConverter) Convert(ctx context.Context, key string, vin dyn.Value, out *schema.Resources) error { + vout, err := convertDashboardResource(ctx, vin) + if err != nil { + return err + } + + // Add the converted resource to the output. + out.Dashboard[key] = vout.AsAny() + + // Configure permissions for this resource. + if permissions := convertPermissionsResource(ctx, vin); permissions != nil { + permissions.DashboardId = fmt.Sprintf("${databricks_dashboard.%s.id}", key) + out.Permissions["dashboard_"+key] = permissions + } + + return nil +} + +func init() { + registerConverter("dashboards", dashboardConverter{}) +} diff --git a/bundle/deploy/terraform/tfdyn/convert_dashboard_test.go b/bundle/deploy/terraform/tfdyn/convert_dashboard_test.go new file mode 100644 index 00000000..9cefbc10 --- /dev/null +++ b/bundle/deploy/terraform/tfdyn/convert_dashboard_test.go @@ -0,0 +1,153 @@ +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/dashboards" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestConvertDashboard(t *testing.T) { + var src = resources.Dashboard{ + CreateDashboardRequest: &dashboards.CreateDashboardRequest{ + DisplayName: "my dashboard", + WarehouseId: "f00dcafe", + ParentPath: "/some/path", + }, + + EmbedCredentials: true, + + Permissions: []resources.Permission{ + { + Level: "CAN_VIEW", + UserName: "jane@doe.com", + }, + }, + } + + vin, err := convert.FromTyped(src, dyn.NilValue) + require.NoError(t, err) + + ctx := context.Background() + out := schema.NewResources() + err = dashboardConverter{}.Convert(ctx, "my_dashboard", vin, out) + require.NoError(t, err) + + // Assert equality on the job + assert.Equal(t, map[string]any{ + "display_name": "my dashboard", + "warehouse_id": "f00dcafe", + "parent_path": "/some/path", + "embed_credentials": true, + }, out.Dashboard["my_dashboard"]) + + // Assert equality on the permissions + assert.Equal(t, &schema.ResourcePermissions{ + DashboardId: "${databricks_dashboard.my_dashboard.id}", + AccessControl: []schema.ResourcePermissionsAccessControl{ + { + PermissionLevel: "CAN_VIEW", + UserName: "jane@doe.com", + }, + }, + }, out.Permissions["dashboard_my_dashboard"]) +} + +func TestConvertDashboardFilePath(t *testing.T) { + var src = resources.Dashboard{ + FilePath: "some/path", + } + + vin, err := convert.FromTyped(src, dyn.NilValue) + require.NoError(t, err) + + ctx := context.Background() + out := schema.NewResources() + err = dashboardConverter{}.Convert(ctx, "my_dashboard", vin, out) + require.NoError(t, err) + + // Assert that the "serialized_dashboard" is included. + assert.Subset(t, out.Dashboard["my_dashboard"], map[string]any{ + "serialized_dashboard": "${file(\"some/path\")}", + }) + + // Assert that the "file_path" doesn't carry over. + assert.NotSubset(t, out.Dashboard["my_dashboard"], map[string]any{ + "file_path": "some/path", + }) +} + +func TestConvertDashboardFilePathQuoted(t *testing.T) { + var src = resources.Dashboard{ + FilePath: `C:\foo\bar\baz\dashboard.lvdash.json`, + } + + vin, err := convert.FromTyped(src, dyn.NilValue) + require.NoError(t, err) + + ctx := context.Background() + out := schema.NewResources() + err = dashboardConverter{}.Convert(ctx, "my_dashboard", vin, out) + require.NoError(t, err) + + // Assert that the "serialized_dashboard" is included. + assert.Subset(t, out.Dashboard["my_dashboard"], map[string]any{ + "serialized_dashboard": `${file("C:\\foo\\bar\\baz\\dashboard.lvdash.json")}`, + }) + + // Assert that the "file_path" doesn't carry over. + assert.NotSubset(t, out.Dashboard["my_dashboard"], map[string]any{ + "file_path": `C:\foo\bar\baz\dashboard.lvdash.json`, + }) +} + +func TestConvertDashboardSerializedDashboardString(t *testing.T) { + var src = resources.Dashboard{ + SerializedDashboard: `{ "json": true }`, + } + + vin, err := convert.FromTyped(src, dyn.NilValue) + require.NoError(t, err) + + ctx := context.Background() + out := schema.NewResources() + err = dashboardConverter{}.Convert(ctx, "my_dashboard", vin, out) + require.NoError(t, err) + + // Assert that the "serialized_dashboard" is included. + assert.Subset(t, out.Dashboard["my_dashboard"], map[string]any{ + "serialized_dashboard": `{ "json": true }`, + }) +} + +func TestConvertDashboardSerializedDashboardAny(t *testing.T) { + var src = resources.Dashboard{ + SerializedDashboard: map[string]any{ + "pages": []map[string]any{ + { + "displayName": "New Page", + "layout": []map[string]any{}, + }, + }, + }, + } + + vin, err := convert.FromTyped(src, dyn.NilValue) + require.NoError(t, err) + + ctx := context.Background() + out := schema.NewResources() + err = dashboardConverter{}.Convert(ctx, "my_dashboard", vin, out) + require.NoError(t, err) + + // Assert that the "serialized_dashboard" is included. + assert.Subset(t, out.Dashboard["my_dashboard"], map[string]any{ + "serialized_dashboard": `{"pages":[{"displayName":"New Page","layout":[]}]}`, + }) +} diff --git a/bundle/deploy/terraform/util.go b/bundle/deploy/terraform/util.go index 64d667b5..4da015c2 100644 --- a/bundle/deploy/terraform/util.go +++ b/bundle/deploy/terraform/util.go @@ -13,7 +13,7 @@ import ( // Partial representation of the Terraform state file format. // We are only interested global version and serial numbers, -// plus resource types, names, modes, and ids. +// plus resource types, names, modes, IDs, and ETags (for dashboards). type resourcesState struct { Version int `json:"version"` Resources []stateResource `json:"resources"` @@ -33,7 +33,8 @@ type stateResourceInstance struct { } type stateInstanceAttributes struct { - ID string `json:"id"` + ID string `json:"id"` + ETag string `json:"etag,omitempty"` } func ParseResourcesState(ctx context.Context, b *bundle.Bundle) (*resourcesState, error) { diff --git a/bundle/internal/bundletest/mutate.go b/bundle/internal/bundletest/mutate.go new file mode 100644 index 00000000..c0ac630c --- /dev/null +++ b/bundle/internal/bundletest/mutate.go @@ -0,0 +1,20 @@ +package bundletest + +import ( + "context" + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/dyn" + "github.com/stretchr/testify/require" +) + +func Mutate(t *testing.T, b *bundle.Bundle, f func(v dyn.Value) (dyn.Value, error)) { + diags := bundle.ApplyFunc(context.Background(), b, func(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + err := b.Config.Mutate(f) + require.NoError(t, err) + return nil + }) + require.NoError(t, diags.Error()) +} diff --git a/bundle/internal/tf/codegen/schema/version.go b/bundle/internal/tf/codegen/schema/version.go index 49e48a6e..0c424408 100644 --- a/bundle/internal/tf/codegen/schema/version.go +++ b/bundle/internal/tf/codegen/schema/version.go @@ -1,3 +1,3 @@ package schema -const ProviderVersion = "1.53.0" +const ProviderVersion = "1.54.0" diff --git a/bundle/internal/tf/schema/data_source_notification_destinations.go b/bundle/internal/tf/schema/data_source_notification_destinations.go new file mode 100644 index 00000000..c95ad6db --- /dev/null +++ b/bundle/internal/tf/schema/data_source_notification_destinations.go @@ -0,0 +1,15 @@ +// Generated from Databricks Terraform provider schema. DO NOT EDIT. + +package schema + +type DataSourceNotificationDestinationsNotificationDestinations struct { + DestinationType string `json:"destination_type,omitempty"` + DisplayName string `json:"display_name,omitempty"` + Id string `json:"id,omitempty"` +} + +type DataSourceNotificationDestinations struct { + DisplayNameContains string `json:"display_name_contains,omitempty"` + Type string `json:"type,omitempty"` + NotificationDestinations []DataSourceNotificationDestinationsNotificationDestinations `json:"notification_destinations,omitempty"` +} diff --git a/bundle/internal/tf/schema/data_source_registered_model.go b/bundle/internal/tf/schema/data_source_registered_model.go new file mode 100644 index 00000000..e19e0849 --- /dev/null +++ b/bundle/internal/tf/schema/data_source_registered_model.go @@ -0,0 +1,32 @@ +// Generated from Databricks Terraform provider schema. DO NOT EDIT. + +package schema + +type DataSourceRegisteredModelModelInfoAliases struct { + AliasName string `json:"alias_name,omitempty"` + VersionNum int `json:"version_num,omitempty"` +} + +type DataSourceRegisteredModelModelInfo struct { + BrowseOnly bool `json:"browse_only,omitempty"` + CatalogName string `json:"catalog_name,omitempty"` + Comment string `json:"comment,omitempty"` + CreatedAt int `json:"created_at,omitempty"` + CreatedBy string `json:"created_by,omitempty"` + FullName string `json:"full_name,omitempty"` + MetastoreId string `json:"metastore_id,omitempty"` + Name string `json:"name,omitempty"` + Owner string `json:"owner,omitempty"` + SchemaName string `json:"schema_name,omitempty"` + StorageLocation string `json:"storage_location,omitempty"` + UpdatedAt int `json:"updated_at,omitempty"` + UpdatedBy string `json:"updated_by,omitempty"` + Aliases []DataSourceRegisteredModelModelInfoAliases `json:"aliases,omitempty"` +} + +type DataSourceRegisteredModel struct { + FullName string `json:"full_name"` + IncludeAliases bool `json:"include_aliases,omitempty"` + IncludeBrowse bool `json:"include_browse,omitempty"` + ModelInfo []DataSourceRegisteredModelModelInfo `json:"model_info,omitempty"` +} diff --git a/bundle/internal/tf/schema/data_sources.go b/bundle/internal/tf/schema/data_sources.go index 10829b99..050e0bc1 100644 --- a/bundle/internal/tf/schema/data_sources.go +++ b/bundle/internal/tf/schema/data_sources.go @@ -36,7 +36,9 @@ type DataSources struct { NodeType map[string]any `json:"databricks_node_type,omitempty"` Notebook map[string]any `json:"databricks_notebook,omitempty"` NotebookPaths map[string]any `json:"databricks_notebook_paths,omitempty"` + NotificationDestinations map[string]any `json:"databricks_notification_destinations,omitempty"` Pipelines map[string]any `json:"databricks_pipelines,omitempty"` + RegisteredModel map[string]any `json:"databricks_registered_model,omitempty"` Schema map[string]any `json:"databricks_schema,omitempty"` Schemas map[string]any `json:"databricks_schemas,omitempty"` ServicePrincipal map[string]any `json:"databricks_service_principal,omitempty"` @@ -92,7 +94,9 @@ func NewDataSources() *DataSources { NodeType: make(map[string]any), Notebook: make(map[string]any), NotebookPaths: make(map[string]any), + NotificationDestinations: make(map[string]any), Pipelines: make(map[string]any), + RegisteredModel: make(map[string]any), Schema: make(map[string]any), Schemas: make(map[string]any), ServicePrincipal: make(map[string]any), diff --git a/bundle/internal/tf/schema/resource_job.go b/bundle/internal/tf/schema/resource_job.go index 42b648b0..c89eafab 100644 --- a/bundle/internal/tf/schema/resource_job.go +++ b/bundle/internal/tf/schema/resource_job.go @@ -1448,6 +1448,7 @@ type ResourceJobWebhookNotifications struct { type ResourceJob struct { AlwaysRunning bool `json:"always_running,omitempty"` + BudgetPolicyId string `json:"budget_policy_id,omitempty"` ControlRunState bool `json:"control_run_state,omitempty"` Description string `json:"description,omitempty"` EditMode string `json:"edit_mode,omitempty"` diff --git a/bundle/internal/tf/schema/resource_online_table.go b/bundle/internal/tf/schema/resource_online_table.go index de671ead..58d6f4ba 100644 --- a/bundle/internal/tf/schema/resource_online_table.go +++ b/bundle/internal/tf/schema/resource_online_table.go @@ -19,9 +19,10 @@ type ResourceOnlineTableSpec struct { } type ResourceOnlineTable struct { - Id string `json:"id,omitempty"` - Name string `json:"name"` - Status []any `json:"status,omitempty"` - TableServingUrl string `json:"table_serving_url,omitempty"` - Spec *ResourceOnlineTableSpec `json:"spec,omitempty"` + Id string `json:"id,omitempty"` + Name string `json:"name"` + Status []any `json:"status,omitempty"` + TableServingUrl string `json:"table_serving_url,omitempty"` + UnityCatalogProvisioningState string `json:"unity_catalog_provisioning_state,omitempty"` + Spec *ResourceOnlineTableSpec `json:"spec,omitempty"` } diff --git a/bundle/internal/tf/schema/resource_pipeline.go b/bundle/internal/tf/schema/resource_pipeline.go index 1bed91fc..2cb459ab 100644 --- a/bundle/internal/tf/schema/resource_pipeline.go +++ b/bundle/internal/tf/schema/resource_pipeline.go @@ -142,10 +142,26 @@ type ResourcePipelineGatewayDefinition struct { GatewayStorageSchema string `json:"gateway_storage_schema,omitempty"` } +type ResourcePipelineIngestionDefinitionObjectsReportTableConfiguration struct { + PrimaryKeys []string `json:"primary_keys,omitempty"` + SalesforceIncludeFormulaFields bool `json:"salesforce_include_formula_fields,omitempty"` + ScdType string `json:"scd_type,omitempty"` + SequenceBy []string `json:"sequence_by,omitempty"` +} + +type ResourcePipelineIngestionDefinitionObjectsReport struct { + DestinationCatalog string `json:"destination_catalog,omitempty"` + DestinationSchema string `json:"destination_schema,omitempty"` + DestinationTable string `json:"destination_table,omitempty"` + SourceUrl string `json:"source_url,omitempty"` + TableConfiguration *ResourcePipelineIngestionDefinitionObjectsReportTableConfiguration `json:"table_configuration,omitempty"` +} + type ResourcePipelineIngestionDefinitionObjectsSchemaTableConfiguration struct { PrimaryKeys []string `json:"primary_keys,omitempty"` SalesforceIncludeFormulaFields bool `json:"salesforce_include_formula_fields,omitempty"` ScdType string `json:"scd_type,omitempty"` + SequenceBy []string `json:"sequence_by,omitempty"` } type ResourcePipelineIngestionDefinitionObjectsSchema struct { @@ -160,6 +176,7 @@ type ResourcePipelineIngestionDefinitionObjectsTableTableConfiguration struct { PrimaryKeys []string `json:"primary_keys,omitempty"` SalesforceIncludeFormulaFields bool `json:"salesforce_include_formula_fields,omitempty"` ScdType string `json:"scd_type,omitempty"` + SequenceBy []string `json:"sequence_by,omitempty"` } type ResourcePipelineIngestionDefinitionObjectsTable struct { @@ -173,6 +190,7 @@ type ResourcePipelineIngestionDefinitionObjectsTable struct { } type ResourcePipelineIngestionDefinitionObjects struct { + Report *ResourcePipelineIngestionDefinitionObjectsReport `json:"report,omitempty"` Schema *ResourcePipelineIngestionDefinitionObjectsSchema `json:"schema,omitempty"` Table *ResourcePipelineIngestionDefinitionObjectsTable `json:"table,omitempty"` } @@ -181,6 +199,7 @@ type ResourcePipelineIngestionDefinitionTableConfiguration struct { PrimaryKeys []string `json:"primary_keys,omitempty"` SalesforceIncludeFormulaFields bool `json:"salesforce_include_formula_fields,omitempty"` ScdType string `json:"scd_type,omitempty"` + SequenceBy []string `json:"sequence_by,omitempty"` } type ResourcePipelineIngestionDefinition struct { diff --git a/bundle/internal/tf/schema/root.go b/bundle/internal/tf/schema/root.go index 7a0cc01f..bf4283c9 100644 --- a/bundle/internal/tf/schema/root.go +++ b/bundle/internal/tf/schema/root.go @@ -21,7 +21,7 @@ type Root struct { const ProviderHost = "registry.terraform.io" const ProviderSource = "databricks/databricks" -const ProviderVersion = "1.53.0" +const ProviderVersion = "1.54.0" func NewRoot() *Root { return &Root{ diff --git a/bundle/libraries/local_path.go b/bundle/libraries/local_path.go index 6d60d56b..53b71410 100644 --- a/bundle/libraries/local_path.go +++ b/bundle/libraries/local_path.go @@ -57,6 +57,12 @@ func IsLibraryLocal(dep string) bool { } } + // If the dependency starts with --, it's a pip flag option which is a valid + // entry for environment dependencies but not a local path + if containsPipFlag(dep) { + return false + } + // If the dependency is a requirements file, it's not a valid local path if strings.HasPrefix(dep, "-r") { return false @@ -70,6 +76,11 @@ func IsLibraryLocal(dep string) bool { return IsLocalPath(dep) } +func containsPipFlag(input string) bool { + re := regexp.MustCompile(`--[a-zA-Z0-9-]+`) + return re.MatchString(input) +} + // ^[a-zA-Z0-9\-_]+: Matches the package name, allowing alphanumeric characters, dashes (-), and underscores (_). // \[.*\])?: Optionally matches any extras specified in square brackets, e.g., [security]. // ((==|!=|<=|>=|~=|>|<)\d+(\.\d+){0,2}(\.\*)?): Optionally matches version specifiers, supporting various operators (==, !=, etc.) followed by a version number (e.g., 2.25.1). diff --git a/bundle/libraries/workspace_path.go b/bundle/libraries/path.go similarity index 78% rename from bundle/libraries/workspace_path.go rename to bundle/libraries/path.go index 126ad3f1..3bad40fa 100644 --- a/bundle/libraries/workspace_path.go +++ b/bundle/libraries/path.go @@ -36,3 +36,12 @@ func IsWorkspaceLibrary(library *compute.Library) bool { return IsWorkspacePath(path) } + +// IsVolumesPath returns true if the specified path indicates that +func IsVolumesPath(path string) bool { + return strings.HasPrefix(path, "/Volumes/") +} + +func IsWorkspaceSharedPath(path string) bool { + return strings.HasPrefix(path, "/Workspace/Shared/") +} diff --git a/bundle/libraries/workspace_path_test.go b/bundle/libraries/path_test.go similarity index 77% rename from bundle/libraries/workspace_path_test.go rename to bundle/libraries/path_test.go index feaaab7f..90fe187a 100644 --- a/bundle/libraries/workspace_path_test.go +++ b/bundle/libraries/path_test.go @@ -31,3 +31,13 @@ func TestIsWorkspaceLibrary(t *testing.T) { // Empty. assert.False(t, IsWorkspaceLibrary(&compute.Library{})) } + +func TestIsVolumesPath(t *testing.T) { + // Absolute paths with particular prefixes. + assert.True(t, IsVolumesPath("/Volumes/path/to/package")) + + // Relative paths. + assert.False(t, IsVolumesPath("myfile.txt")) + assert.False(t, IsVolumesPath("./myfile.txt")) + assert.False(t, IsVolumesPath("../myfile.txt")) +} diff --git a/bundle/permissions/mutator.go b/bundle/permissions/mutator.go index 7787bc04..bc1392d9 100644 --- a/bundle/permissions/mutator.go +++ b/bundle/permissions/mutator.go @@ -39,6 +39,10 @@ var levelsMap = map[string](map[string]string){ CAN_VIEW: "CAN_VIEW", CAN_RUN: "CAN_QUERY", }, + "dashboards": { + CAN_MANAGE: "CAN_MANAGE", + CAN_VIEW: "CAN_READ", + }, } type bundlePermissions struct{} diff --git a/bundle/permissions/validate.go b/bundle/permissions/validate.go new file mode 100644 index 00000000..f1a18f43 --- /dev/null +++ b/bundle/permissions/validate.go @@ -0,0 +1,52 @@ +package permissions + +import ( + "context" + "fmt" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/libraries" + "github.com/databricks/cli/libs/diag" +) + +type validateSharedRootPermissions struct { +} + +func ValidateSharedRootPermissions() bundle.Mutator { + return &validateSharedRootPermissions{} +} + +func (*validateSharedRootPermissions) Name() string { + return "ValidateSharedRootPermissions" +} + +func (*validateSharedRootPermissions) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + if libraries.IsWorkspaceSharedPath(b.Config.Workspace.RootPath) { + return isUsersGroupPermissionSet(b) + } + + return nil +} + +// isUsersGroupPermissionSet checks that top-level permissions set for bundle contain group_name: users with CAN_MANAGE permission. +func isUsersGroupPermissionSet(b *bundle.Bundle) diag.Diagnostics { + var diags diag.Diagnostics + + allUsers := false + for _, p := range b.Config.Permissions { + if p.GroupName == "users" && p.Level == CAN_MANAGE { + allUsers = true + break + } + } + + if !allUsers { + diags = diags.Append(diag.Diagnostic{ + Severity: diag.Warning, + Summary: fmt.Sprintf("the bundle root path %s is writable by all workspace users", b.Config.Workspace.RootPath), + Detail: "The bundle is configured to use /Workspace/Shared, which will give read/write access to all users. If this is intentional, add CAN_MANAGE for 'group_name: users' permission to your bundle configuration. If the deployment should be restricted, move it to a restricted folder such as /Workspace/Users/.", + }) + } + + return diags +} diff --git a/bundle/permissions/validate_test.go b/bundle/permissions/validate_test.go new file mode 100644 index 00000000..ff132b4e --- /dev/null +++ b/bundle/permissions/validate_test.go @@ -0,0 +1,66 @@ +package permissions + +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/experimental/mocks" + "github.com/databricks/databricks-sdk-go/service/jobs" + "github.com/stretchr/testify/require" +) + +func TestValidateSharedRootPermissionsForShared(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Workspace: config.Workspace{ + RootPath: "/Workspace/Shared/foo/bar", + }, + Permissions: []resources.Permission{ + {Level: CAN_MANAGE, GroupName: "users"}, + }, + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "job_1": {JobSettings: &jobs.JobSettings{Name: "job_1"}}, + "job_2": {JobSettings: &jobs.JobSettings{Name: "job_2"}}, + }, + }, + }, + } + + m := mocks.NewMockWorkspaceClient(t) + b.SetWorkpaceClient(m.WorkspaceClient) + + diags := bundle.Apply(context.Background(), b, bundle.Seq(ValidateSharedRootPermissions())) + require.Empty(t, diags) +} + +func TestValidateSharedRootPermissionsForSharedError(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Workspace: config.Workspace{ + RootPath: "/Workspace/Shared/foo/bar", + }, + Permissions: []resources.Permission{ + {Level: CAN_MANAGE, UserName: "foo@bar.com"}, + }, + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "job_1": {JobSettings: &jobs.JobSettings{Name: "job_1"}}, + "job_2": {JobSettings: &jobs.JobSettings{Name: "job_2"}}, + }, + }, + }, + } + + m := mocks.NewMockWorkspaceClient(t) + b.SetWorkpaceClient(m.WorkspaceClient) + + diags := bundle.Apply(context.Background(), b, bundle.Seq(ValidateSharedRootPermissions())) + require.Len(t, diags, 1) + require.Equal(t, "the bundle root path /Workspace/Shared/foo/bar is writable by all workspace users", diags[0].Summary) + require.Equal(t, diag.Warning, diags[0].Severity) +} diff --git a/bundle/permissions/workspace_path_permissions.go b/bundle/permissions/workspace_path_permissions.go new file mode 100644 index 00000000..a3b4424c --- /dev/null +++ b/bundle/permissions/workspace_path_permissions.go @@ -0,0 +1,89 @@ +package permissions + +import ( + "fmt" + "strings" + + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/libs/diag" + "github.com/databricks/databricks-sdk-go/service/workspace" +) + +type WorkspacePathPermissions struct { + Path string + Permissions []resources.Permission +} + +func ObjectAclToResourcePermissions(path string, acl []workspace.WorkspaceObjectAccessControlResponse) *WorkspacePathPermissions { + permissions := make([]resources.Permission, 0) + for _, a := range acl { + // Skip the admin group because it's added to all resources by default. + if a.GroupName == "admins" { + continue + } + + for _, pl := range a.AllPermissions { + permissions = append(permissions, resources.Permission{ + Level: convertWorkspaceObjectPermissionLevel(pl.PermissionLevel), + GroupName: a.GroupName, + UserName: a.UserName, + ServicePrincipalName: a.ServicePrincipalName, + }) + } + } + + return &WorkspacePathPermissions{Permissions: permissions, Path: path} +} + +func (p WorkspacePathPermissions) Compare(perms []resources.Permission) diag.Diagnostics { + var diags diag.Diagnostics + + // Check the permissions in the workspace and see if they are all set in the bundle. + ok, missing := containsAll(p.Permissions, perms) + if !ok { + diags = diags.Append(diag.Diagnostic{ + Severity: diag.Warning, + Summary: "untracked permissions apply to target workspace path", + Detail: fmt.Sprintf("The following permissions apply to the workspace folder at %q but are not configured in the bundle:\n%s", p.Path, toString(missing)), + }) + } + + return diags +} + +// containsAll checks if permA contains all permissions in permB. +func containsAll(permA []resources.Permission, permB []resources.Permission) (bool, []resources.Permission) { + missing := make([]resources.Permission, 0) + for _, a := range permA { + found := false + for _, b := range permB { + if a == b { + found = true + break + } + } + if !found { + missing = append(missing, a) + } + } + return len(missing) == 0, missing +} + +// convertWorkspaceObjectPermissionLevel converts matching object permission levels to bundle ones. +// If there is no matching permission level, it returns permission level as is, for example, CAN_EDIT. +func convertWorkspaceObjectPermissionLevel(level workspace.WorkspaceObjectPermissionLevel) string { + switch level { + case workspace.WorkspaceObjectPermissionLevelCanRead: + return CAN_VIEW + default: + return string(level) + } +} + +func toString(p []resources.Permission) string { + var sb strings.Builder + for _, perm := range p { + sb.WriteString(fmt.Sprintf("- %s\n", perm.String())) + } + return sb.String() +} diff --git a/bundle/permissions/workspace_path_permissions_test.go b/bundle/permissions/workspace_path_permissions_test.go new file mode 100644 index 00000000..0bb00474 --- /dev/null +++ b/bundle/permissions/workspace_path_permissions_test.go @@ -0,0 +1,121 @@ +package permissions + +import ( + "testing" + + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/libs/diag" + "github.com/databricks/databricks-sdk-go/service/workspace" + "github.com/stretchr/testify/require" +) + +func TestWorkspacePathPermissionsCompare(t *testing.T) { + testCases := []struct { + perms []resources.Permission + acl []workspace.WorkspaceObjectAccessControlResponse + expected diag.Diagnostics + }{ + { + perms: []resources.Permission{ + {Level: CAN_MANAGE, UserName: "foo@bar.com"}, + }, + acl: []workspace.WorkspaceObjectAccessControlResponse{ + { + UserName: "foo@bar.com", + AllPermissions: []workspace.WorkspaceObjectPermission{ + {PermissionLevel: "CAN_MANAGE"}, + }, + }, + }, + expected: nil, + }, + { + perms: []resources.Permission{ + {Level: CAN_MANAGE, UserName: "foo@bar.com"}, + }, + acl: []workspace.WorkspaceObjectAccessControlResponse{ + { + UserName: "foo@bar.com", + AllPermissions: []workspace.WorkspaceObjectPermission{ + {PermissionLevel: "CAN_MANAGE"}, + }, + }, + { + GroupName: "admins", + AllPermissions: []workspace.WorkspaceObjectPermission{ + {PermissionLevel: "CAN_MANAGE"}, + }, + }, + }, + expected: nil, + }, + { + perms: []resources.Permission{ + {Level: CAN_VIEW, UserName: "foo@bar.com"}, + {Level: CAN_MANAGE, ServicePrincipalName: "sp.com"}, + }, + acl: []workspace.WorkspaceObjectAccessControlResponse{ + { + UserName: "foo@bar.com", + AllPermissions: []workspace.WorkspaceObjectPermission{ + {PermissionLevel: "CAN_READ"}, + }, + }, + }, + expected: nil, + }, + { + perms: []resources.Permission{ + {Level: CAN_MANAGE, UserName: "foo@bar.com"}, + }, + acl: []workspace.WorkspaceObjectAccessControlResponse{ + { + UserName: "foo@bar.com", + AllPermissions: []workspace.WorkspaceObjectPermission{ + {PermissionLevel: "CAN_MANAGE"}, + }, + }, + { + GroupName: "foo", + AllPermissions: []workspace.WorkspaceObjectPermission{ + {PermissionLevel: "CAN_MANAGE"}, + }, + }, + }, + expected: diag.Diagnostics{ + { + Severity: diag.Warning, + Summary: "untracked permissions apply to target workspace path", + Detail: "The following permissions apply to the workspace folder at \"path\" but are not configured in the bundle:\n- level: CAN_MANAGE, group_name: foo\n", + }, + }, + }, + { + perms: []resources.Permission{ + {Level: CAN_MANAGE, UserName: "foo@bar.com"}, + }, + acl: []workspace.WorkspaceObjectAccessControlResponse{ + { + UserName: "foo2@bar.com", + AllPermissions: []workspace.WorkspaceObjectPermission{ + {PermissionLevel: "CAN_MANAGE"}, + }, + }, + }, + expected: diag.Diagnostics{ + { + Severity: diag.Warning, + Summary: "untracked permissions apply to target workspace path", + Detail: "The following permissions apply to the workspace folder at \"path\" but are not configured in the bundle:\n- level: CAN_MANAGE, user_name: foo2@bar.com\n", + }, + }, + }, + } + + for _, tc := range testCases { + wp := ObjectAclToResourcePermissions("path", tc.acl) + diags := wp.Compare(tc.perms) + require.Equal(t, tc.expected, diags) + } + +} diff --git a/bundle/permissions/workspace_root.go b/bundle/permissions/workspace_root.go index a59a039f..93a90ed9 100644 --- a/bundle/permissions/workspace_root.go +++ b/bundle/permissions/workspace_root.go @@ -16,6 +16,10 @@ func ApplyWorkspaceRootPermissions() bundle.Mutator { return &workspaceRootPermissions{} } +func (*workspaceRootPermissions) Name() string { + return "ApplyWorkspaceRootPermissions" +} + // Apply implements bundle.Mutator. func (*workspaceRootPermissions) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { err := giveAccessForWorkspaceRoot(ctx, b) @@ -26,15 +30,11 @@ func (*workspaceRootPermissions) Apply(ctx context.Context, b *bundle.Bundle) di return nil } -func (*workspaceRootPermissions) Name() string { - return "ApplyWorkspaceRootPermissions" -} - func giveAccessForWorkspaceRoot(ctx context.Context, b *bundle.Bundle) error { permissions := make([]workspace.WorkspaceObjectAccessControlRequest, 0) for _, p := range b.Config.Permissions { - level, err := getWorkspaceObjectPermissionLevel(p.Level) + level, err := GetWorkspaceObjectPermissionLevel(p.Level) if err != nil { return err } @@ -65,7 +65,7 @@ func giveAccessForWorkspaceRoot(ctx context.Context, b *bundle.Bundle) error { return err } -func getWorkspaceObjectPermissionLevel(bundlePermission string) (workspace.WorkspaceObjectPermissionLevel, error) { +func GetWorkspaceObjectPermissionLevel(bundlePermission string) (workspace.WorkspaceObjectPermissionLevel, error) { switch bundlePermission { case CAN_MANAGE: return workspace.WorkspaceObjectPermissionLevelCanManage, nil diff --git a/bundle/permissions/workspace_root_test.go b/bundle/permissions/workspace_root_test.go index 5e23a1da..6b37b2c4 100644 --- a/bundle/permissions/workspace_root_test.go +++ b/bundle/permissions/workspace_root_test.go @@ -69,6 +69,6 @@ func TestApplyWorkspaceRootPermissions(t *testing.T) { WorkspaceObjectType: "directories", }).Return(nil, nil) - diags := bundle.Apply(context.Background(), b, ApplyWorkspaceRootPermissions()) - require.NoError(t, diags.Error()) + diags := bundle.Apply(context.Background(), b, bundle.Seq(ValidateSharedRootPermissions(), ApplyWorkspaceRootPermissions())) + require.Empty(t, diags) } diff --git a/bundle/phases/deploy.go b/bundle/phases/deploy.go index f6349baf..28b78f28 100644 --- a/bundle/phases/deploy.go +++ b/bundle/phases/deploy.go @@ -146,6 +146,7 @@ func Deploy(outputHandler sync.OutputHandler) bundle.Mutator { bundle.Defer( bundle.Seq( terraform.StatePull(), + terraform.CheckDashboardsModifiedRemotely(), deploy.StatePull(), mutator.ValidateGitDetails(), artifacts.CleanUp(), diff --git a/bundle/phases/initialize.go b/bundle/phases/initialize.go index da5b2eff..3d5ad5e8 100644 --- a/bundle/phases/initialize.go +++ b/bundle/phases/initialize.go @@ -66,6 +66,7 @@ func Initialize() bundle.Mutator { permissions.PermissionDiagnostics(), mutator.SetRunAs(), mutator.OverrideCompute(), + mutator.ConfigureDashboardDefaults(), mutator.ProcessTargetMode(), mutator.ApplyPresets(), mutator.DefaultQueueing(), @@ -76,8 +77,11 @@ func Initialize() bundle.Mutator { mutator.TranslatePaths(), trampoline.WrapperWarning(), + + permissions.ValidateSharedRootPermissions(), permissions.ApplyBundlePermissions(), permissions.FilterCurrentUser(), + metadata.AnnotateJobs(), metadata.AnnotatePipelines(), terraform.Initialize(), diff --git a/bundle/render/render_text_output.go b/bundle/render/render_text_output.go index 3e52d5f1..2f7affbf 100644 --- a/bundle/render/render_text_output.go +++ b/bundle/render/render_text_output.go @@ -1,9 +1,11 @@ package render import ( + "context" "fmt" "io" "path/filepath" + "sort" "strings" "text/template" @@ -29,7 +31,7 @@ var renderFuncMap = template.FuncMap{ }, } -const summaryTemplate = `{{- if .Name -}} +const summaryHeaderTemplate = `{{- if .Name -}} Name: {{ .Name | bold }} {{- if .Target }} Target: {{ .Target | bold }} @@ -46,12 +48,30 @@ Workspace: Path: {{ .Path | bold }} {{- end }} {{- end }} +{{ end -}}` -{{ end -}} - -{{ .Trailer }} +const resourcesTemplate = `Resources: +{{- range . }} + {{ .GroupName }}: + {{- range .Resources }} + {{ .Key | bold }}: + Name: {{ .Name }} + URL: {{ if .URL }}{{ .URL | cyan }}{{ else }}{{ "(not deployed)" | cyan }}{{ end }} + {{- end }} +{{- end }} ` +type ResourceGroup struct { + GroupName string + Resources []ResourceInfo +} + +type ResourceInfo struct { + Key string + Name string + URL string +} + func pluralize(n int, singular, plural string) string { if n == 1 { return fmt.Sprintf("%d %s", n, singular) @@ -74,20 +94,20 @@ func buildTrailer(diags diag.Diagnostics) string { case len(parts) >= 3: first := strings.Join(parts[:len(parts)-1], ", ") last := parts[len(parts)-1] - return fmt.Sprintf("Found %s, and %s", first, last) + return fmt.Sprintf("Found %s, and %s\n", first, last) case len(parts) == 2: - return fmt.Sprintf("Found %s and %s", parts[0], parts[1]) + return fmt.Sprintf("Found %s and %s\n", parts[0], parts[1]) case len(parts) == 1: - return fmt.Sprintf("Found %s", parts[0]) + return fmt.Sprintf("Found %s\n", parts[0]) default: // No diagnostics to print. - return color.GreenString("Validation OK!") + return color.GreenString("Validation OK!\n") } } -func renderSummaryTemplate(out io.Writer, b *bundle.Bundle, diags diag.Diagnostics) error { +func renderSummaryHeaderTemplate(out io.Writer, b *bundle.Bundle) error { if b == nil { - return renderSummaryTemplate(out, &bundle.Bundle{}, diags) + return renderSummaryHeaderTemplate(out, &bundle.Bundle{}) } var currentUser = &iam.User{} @@ -98,20 +118,19 @@ func renderSummaryTemplate(out io.Writer, b *bundle.Bundle, diags diag.Diagnosti } } - t := template.Must(template.New("summary").Funcs(renderFuncMap).Parse(summaryTemplate)) + t := template.Must(template.New("summary").Funcs(renderFuncMap).Parse(summaryHeaderTemplate)) err := t.Execute(out, map[string]any{ - "Name": b.Config.Bundle.Name, - "Target": b.Config.Bundle.Target, - "User": currentUser.UserName, - "Path": b.Config.Workspace.RootPath, - "Host": b.Config.Workspace.Host, - "Trailer": buildTrailer(diags), + "Name": b.Config.Bundle.Name, + "Target": b.Config.Bundle.Target, + "User": currentUser.UserName, + "Path": b.Config.Workspace.RootPath, + "Host": b.Config.Workspace.Host, }) return err } -func renderDiagnostics(out io.Writer, b *bundle.Bundle, diags diag.Diagnostics) error { +func renderDiagnosticsOnly(out io.Writer, b *bundle.Bundle, diags diag.Diagnostics) error { for _, d := range diags { for i := range d.Locations { if b == nil { @@ -139,19 +158,73 @@ type RenderOptions struct { RenderSummaryTable bool } -// RenderTextOutput renders the diagnostics in a human-readable format. -func RenderTextOutput(out io.Writer, b *bundle.Bundle, diags diag.Diagnostics, opts RenderOptions) error { - err := renderDiagnostics(out, b, diags) +// RenderDiagnostics renders the diagnostics in a human-readable format. +func RenderDiagnostics(out io.Writer, b *bundle.Bundle, diags diag.Diagnostics, opts RenderOptions) error { + err := renderDiagnosticsOnly(out, b, diags) if err != nil { return fmt.Errorf("failed to render diagnostics: %w", err) } if opts.RenderSummaryTable { - err = renderSummaryTemplate(out, b, diags) - if err != nil { - return fmt.Errorf("failed to render summary: %w", err) + if b != nil { + err = renderSummaryHeaderTemplate(out, b) + if err != nil { + return fmt.Errorf("failed to render summary: %w", err) + } + io.WriteString(out, "\n") } + trailer := buildTrailer(diags) + io.WriteString(out, trailer) } return nil } + +func RenderSummary(ctx context.Context, out io.Writer, b *bundle.Bundle) error { + if err := renderSummaryHeaderTemplate(out, b); err != nil { + return err + } + + var resourceGroups []ResourceGroup + + for _, group := range b.Config.Resources.AllResources() { + resources := make([]ResourceInfo, 0, len(group.Resources)) + for key, resource := range group.Resources { + resources = append(resources, ResourceInfo{ + Key: key, + Name: resource.GetName(), + URL: resource.GetURL(), + }) + } + + if len(resources) > 0 { + resourceGroups = append(resourceGroups, ResourceGroup{ + GroupName: group.Description.PluralTitle, + Resources: resources, + }) + } + } + + if err := renderResourcesTemplate(out, resourceGroups); err != nil { + return fmt.Errorf("failed to render resources template: %w", err) + } + + return nil +} + +// Helper function to sort and render resource groups using the template +func renderResourcesTemplate(out io.Writer, resourceGroups []ResourceGroup) error { + // Sort everything to ensure consistent output + sort.Slice(resourceGroups, func(i, j int) bool { + return resourceGroups[i].GroupName < resourceGroups[j].GroupName + }) + for _, group := range resourceGroups { + sort.Slice(group.Resources, func(i, j int) bool { + return group.Resources[i].Key < group.Resources[j].Key + }) + } + + t := template.Must(template.New("resources").Funcs(renderFuncMap).Parse(resourcesTemplate)) + + return t.Execute(out, resourceGroups) +} diff --git a/bundle/render/render_text_output_test.go b/bundle/render/render_text_output_test.go index 1a41fa01..cd9e7723 100644 --- a/bundle/render/render_text_output_test.go +++ b/bundle/render/render_text_output_test.go @@ -2,14 +2,21 @@ package render import ( "bytes" + "context" + "io" "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/cli/libs/dyn" - assert "github.com/databricks/cli/libs/dyn/dynassert" + "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/databricks/databricks-sdk-go/service/pipelines" + "github.com/databricks/databricks-sdk-go/service/serving" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -326,7 +333,7 @@ func TestRenderTextOutput(t *testing.T) { t.Run(tc.name, func(t *testing.T) { writer := &bytes.Buffer{} - err := RenderTextOutput(writer, tc.bundle, tc.diags, tc.opts) + err := RenderDiagnostics(writer, tc.bundle, tc.diags, tc.opts) require.NoError(t, err) assert.Equal(t, tc.expected, writer.String()) @@ -468,7 +475,7 @@ func TestRenderDiagnostics(t *testing.T) { t.Run(tc.name, func(t *testing.T) { writer := &bytes.Buffer{} - err := renderDiagnostics(writer, bundle, tc.diags) + err := renderDiagnosticsOnly(writer, bundle, tc.diags) require.NoError(t, err) assert.Equal(t, tc.expected, writer.String()) @@ -479,8 +486,105 @@ func TestRenderDiagnostics(t *testing.T) { func TestRenderSummaryTemplate_nilBundle(t *testing.T) { writer := &bytes.Buffer{} - err := renderSummaryTemplate(writer, nil, nil) + err := renderSummaryHeaderTemplate(writer, nil) require.NoError(t, err) + io.WriteString(writer, buildTrailer(nil)) + assert.Equal(t, "Validation OK!\n", writer.String()) } + +func TestRenderSummary(t *testing.T) { + ctx := context.Background() + + // Create a mock bundle with various resources + b := &bundle.Bundle{ + Config: config.Root{ + Bundle: config.Bundle{ + Name: "test-bundle", + Target: "test-target", + }, + Workspace: config.Workspace{ + Host: "https://mycompany.databricks.com/", + }, + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "job1": { + ID: "1", + URL: "https://url1", + JobSettings: &jobs.JobSettings{Name: "job1-name"}, + }, + "job2": { + ID: "2", + URL: "https://url2", + JobSettings: &jobs.JobSettings{Name: "job2-name"}, + }, + }, + Pipelines: map[string]*resources.Pipeline{ + "pipeline2": { + ID: "4", + // no URL + PipelineSpec: &pipelines.PipelineSpec{Name: "pipeline2-name"}, + }, + "pipeline1": { + ID: "3", + URL: "https://url3", + PipelineSpec: &pipelines.PipelineSpec{Name: "pipeline1-name"}, + }, + }, + Schemas: map[string]*resources.Schema{ + "schema1": { + ID: "catalog.schema", + CreateSchema: &catalog.CreateSchema{ + Name: "schema", + }, + // no URL + }, + }, + ModelServingEndpoints: map[string]*resources.ModelServingEndpoint{ + "endpoint1": { + ID: "7", + CreateServingEndpoint: &serving.CreateServingEndpoint{ + Name: "my_serving_endpoint", + }, + URL: "https://url4", + }, + }, + }, + }, + } + + writer := &bytes.Buffer{} + err := RenderSummary(ctx, writer, b) + require.NoError(t, err) + + expectedSummary := `Name: test-bundle +Target: test-target +Workspace: + Host: https://mycompany.databricks.com/ +Resources: + Jobs: + job1: + Name: job1-name + URL: https://url1 + job2: + Name: job2-name + URL: https://url2 + Model Serving Endpoints: + endpoint1: + Name: my_serving_endpoint + URL: https://url4 + Pipelines: + pipeline1: + Name: pipeline1-name + URL: https://url3 + pipeline2: + Name: pipeline2-name + URL: (not deployed) + Schemas: + schema1: + Name: schema + URL: (not deployed) +` + assert.Equal(t, expectedSummary, writer.String()) +} diff --git a/bundle/resources/completion.go b/bundle/resources/completion.go new file mode 100644 index 00000000..c1bcd022 --- /dev/null +++ b/bundle/resources/completion.go @@ -0,0 +1,17 @@ +package resources + +import "github.com/databricks/cli/bundle" + +// Completions returns the same as [References] except +// that every key maps directly to a single reference. +func Completions(b *bundle.Bundle, filters ...Filter) map[string]Reference { + out := make(map[string]Reference) + keyOnlyRefs, _ := References(b, filters...) + for k, refs := range keyOnlyRefs { + if len(refs) != 1 { + continue + } + out[k] = refs[0] + } + return out +} diff --git a/bundle/resources/completion_test.go b/bundle/resources/completion_test.go new file mode 100644 index 00000000..2f7942aa --- /dev/null +++ b/bundle/resources/completion_test.go @@ -0,0 +1,58 @@ +package resources + +import ( + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/bundle/config/resources" + "github.com/stretchr/testify/assert" +) + +func TestCompletions_SkipDuplicates(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "foo": {}, + "bar": {}, + }, + Pipelines: map[string]*resources.Pipeline{ + "foo": {}, + }, + }, + }, + } + + // Test that this skips duplicates and only includes unambiguous completions. + out := Completions(b) + if assert.Len(t, out, 1) { + assert.Contains(t, out, "bar") + } +} + +func TestCompletions_Filter(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "foo": {}, + }, + Pipelines: map[string]*resources.Pipeline{ + "bar": {}, + }, + }, + }, + } + + includeJobs := func(ref Reference) bool { + _, ok := ref.Resource.(*resources.Job) + return ok + } + + // Test that this does not include the pipeline. + out := Completions(b, includeJobs) + if assert.Len(t, out, 1) { + assert.Contains(t, out, "foo") + } +} diff --git a/bundle/resources/lookup.go b/bundle/resources/lookup.go new file mode 100644 index 00000000..48d86223 --- /dev/null +++ b/bundle/resources/lookup.go @@ -0,0 +1,98 @@ +package resources + +import ( + "fmt" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" +) + +// Reference is a reference to a resource. +// It includes the resource type description, and a reference to the resource itself. +type Reference struct { + // Key is the unique key of the resource, e.g. "my_job". + Key string + + // KeyWithType is the unique key of the resource, including the resource type, e.g. "jobs.my_job". + KeyWithType string + + // Description is the resource type description. + Description config.ResourceDescription + + // Resource is the resource itself. + Resource config.ConfigResource +} + +// Map is the core type for resource lookup and completion. +type Map map[string][]Reference + +// Filter defines the function signature for filtering resources. +type Filter func(Reference) bool + +// includeReference checks if the specified reference passes all filters. +// If the list of filters is empty, the reference is always included. +func includeReference(filters []Filter, ref Reference) bool { + for _, filter := range filters { + if !filter(ref) { + return false + } + } + return true +} + +// References returns maps of resource keys to a slice of [Reference]. +// +// The first map is indexed by the resource key only. +// The second map is indexed by the resource type name and its key. +// +// While the return types allows for multiple resources to share the same key, +// this is confirmed not to happen in the [validate.UniqueResourceKeys] mutator. +func References(b *bundle.Bundle, filters ...Filter) (Map, Map) { + keyOnly := make(Map) + keyWithType := make(Map) + + // Collect map of resource references indexed by their keys. + for _, group := range b.Config.Resources.AllResources() { + for k, v := range group.Resources { + ref := Reference{ + Key: k, + KeyWithType: fmt.Sprintf("%s.%s", group.Description.PluralName, k), + Description: group.Description, + Resource: v, + } + + // Skip resources that do not pass all filters. + if !includeReference(filters, ref) { + continue + } + + keyOnly[ref.Key] = append(keyOnly[ref.Key], ref) + keyWithType[ref.KeyWithType] = append(keyWithType[ref.KeyWithType], ref) + } + } + + return keyOnly, keyWithType +} + +// Lookup returns the resource with the specified key. +// If the key maps to more than one resource, an error is returned. +// If the key does not map to any resource, an error is returned. +func Lookup(b *bundle.Bundle, key string, filters ...Filter) (Reference, error) { + keyOnlyRefs, keyWithTypeRefs := References(b, filters...) + refs, ok := keyOnlyRefs[key] + if !ok { + refs, ok = keyWithTypeRefs[key] + if !ok { + return Reference{}, fmt.Errorf("resource with key %q not found", key) + } + } + + switch { + case len(refs) == 1: + return refs[0], nil + case len(refs) > 1: + return Reference{}, fmt.Errorf("multiple resources with key %q found", key) + default: + panic("unreachable") + } +} diff --git a/bundle/resources/lookup_test.go b/bundle/resources/lookup_test.go new file mode 100644 index 00000000..b2eaafd1 --- /dev/null +++ b/bundle/resources/lookup_test.go @@ -0,0 +1,117 @@ +package resources + +import ( + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/databricks-sdk-go/service/jobs" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestLookup_EmptyBundle(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{}, + }, + } + + _, err := Lookup(b, "foo") + require.Error(t, err) + assert.ErrorContains(t, err, "resource with key \"foo\" not found") +} + +func TestLookup_NotFound(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "foo": {}, + "bar": {}, + }, + }, + }, + } + + _, err := Lookup(b, "qux") + require.Error(t, err) + assert.ErrorContains(t, err, `resource with key "qux" not found`) +} + +func TestLookup_MultipleFound(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "foo": {}, + }, + Pipelines: map[string]*resources.Pipeline{ + "foo": {}, + }, + }, + }, + } + + _, err := Lookup(b, "foo") + require.Error(t, err) + assert.ErrorContains(t, err, `multiple resources with key "foo" found`) +} + +func TestLookup_Nominal(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "foo": { + JobSettings: &jobs.JobSettings{ + Name: "Foo job", + }, + }, + }, + }, + }, + } + + // Lookup by key only. + out, err := Lookup(b, "foo") + if assert.NoError(t, err) { + assert.Equal(t, "Foo job", out.Resource.GetName()) + } + + // Lookup by type and key. + out, err = Lookup(b, "jobs.foo") + if assert.NoError(t, err) { + assert.Equal(t, "Foo job", out.Resource.GetName()) + } +} + +func TestLookup_NominalWithFilters(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "foo": {}, + }, + Pipelines: map[string]*resources.Pipeline{ + "bar": {}, + }, + }, + }, + } + + includeJobs := func(ref Reference) bool { + _, ok := ref.Resource.(*resources.Job) + return ok + } + + // This should succeed because the filter includes jobs. + _, err := Lookup(b, "foo", includeJobs) + require.NoError(t, err) + + // This should fail because the filter excludes pipelines. + _, err = Lookup(b, "bar", includeJobs) + require.Error(t, err) + assert.ErrorContains(t, err, `resource with key "bar" not found`) +} diff --git a/bundle/run/job.go b/bundle/run/job.go index 8003c7d2..340af961 100644 --- a/bundle/run/job.go +++ b/bundle/run/job.go @@ -317,6 +317,29 @@ func (r *jobRunner) Cancel(ctx context.Context) error { return errGroup.Wait() } +func (r *jobRunner) Restart(ctx context.Context, opts *Options) (output.RunOutput, error) { + // We don't need to cancel existing runs if the job is continuous and unpaused. + // the /jobs/run-now API will automatically cancel any existing runs before starting a new one. + // + // /jobs/run-now will not cancel existing runs if the job is continuous and paused. + // New job runs will be queued instead and will wait for existing runs to finish. + // In this case, we need to cancel the existing runs before starting a new one. + continuous := r.job.JobSettings.Continuous + if continuous != nil && continuous.PauseStatus == jobs.PauseStatusUnpaused { + return r.Run(ctx, opts) + } + + s := cmdio.Spinner(ctx) + s <- "Cancelling all active job runs" + err := r.Cancel(ctx) + close(s) + if err != nil { + return nil, err + } + + return r.Run(ctx, opts) +} + func (r *jobRunner) ParseArgs(args []string, opts *Options) error { return r.posArgsHandler().ParseArgs(args, opts) } diff --git a/bundle/run/job_test.go b/bundle/run/job_test.go index be189306..369c546a 100644 --- a/bundle/run/job_test.go +++ b/bundle/run/job_test.go @@ -1,6 +1,7 @@ package run import ( + "bytes" "context" "testing" "time" @@ -8,6 +9,8 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/flags" "github.com/databricks/databricks-sdk-go/experimental/mocks" "github.com/databricks/databricks-sdk-go/service/jobs" "github.com/stretchr/testify/mock" @@ -126,3 +129,132 @@ func TestJobRunnerCancelWithNoActiveRuns(t *testing.T) { err := runner.Cancel(context.Background()) require.NoError(t, err) } + +func TestJobRunnerRestart(t *testing.T) { + for _, jobSettings := range []*jobs.JobSettings{ + {}, + { + Continuous: &jobs.Continuous{ + PauseStatus: jobs.PauseStatusPaused, + }, + }, + } { + job := &resources.Job{ + ID: "123", + JobSettings: jobSettings, + } + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "test_job": job, + }, + }, + }, + } + + runner := jobRunner{key: "test", bundle: b, job: job} + + m := mocks.NewMockWorkspaceClient(t) + b.SetWorkpaceClient(m.WorkspaceClient) + ctx := context.Background() + ctx = cmdio.InContext(ctx, cmdio.NewIO(flags.OutputText, &bytes.Buffer{}, &bytes.Buffer{}, &bytes.Buffer{}, "", "")) + ctx = cmdio.NewContext(ctx, cmdio.NewLogger(flags.ModeAppend)) + + jobApi := m.GetMockJobsAPI() + jobApi.EXPECT().ListRunsAll(mock.Anything, jobs.ListRunsRequest{ + ActiveOnly: true, + JobId: 123, + }).Return([]jobs.BaseRun{ + {RunId: 1}, + {RunId: 2}, + }, nil) + + // Mock the runner cancelling existing job runs. + mockWait := &jobs.WaitGetRunJobTerminatedOrSkipped[struct{}]{ + Poll: func(time time.Duration, f func(j *jobs.Run)) (*jobs.Run, error) { + return nil, nil + }, + } + jobApi.EXPECT().CancelRun(mock.Anything, jobs.CancelRun{ + RunId: 1, + }).Return(mockWait, nil) + jobApi.EXPECT().CancelRun(mock.Anything, jobs.CancelRun{ + RunId: 2, + }).Return(mockWait, nil) + + // Mock the runner triggering a job run + mockWaitForRun := &jobs.WaitGetRunJobTerminatedOrSkipped[jobs.RunNowResponse]{ + Poll: func(d time.Duration, f func(*jobs.Run)) (*jobs.Run, error) { + return &jobs.Run{ + State: &jobs.RunState{ + ResultState: jobs.RunResultStateSuccess, + }, + }, nil + }, + } + jobApi.EXPECT().RunNow(mock.Anything, jobs.RunNow{ + JobId: 123, + }).Return(mockWaitForRun, nil) + + // Mock the runner getting the job output + jobApi.EXPECT().GetRun(mock.Anything, jobs.GetRunRequest{}).Return(&jobs.Run{}, nil) + + _, err := runner.Restart(ctx, &Options{}) + require.NoError(t, err) + } +} + +func TestJobRunnerRestartForContinuousUnpausedJobs(t *testing.T) { + job := &resources.Job{ + ID: "123", + JobSettings: &jobs.JobSettings{ + Continuous: &jobs.Continuous{ + PauseStatus: jobs.PauseStatusUnpaused, + }, + }, + } + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "test_job": job, + }, + }, + }, + } + + runner := jobRunner{key: "test", bundle: b, job: job} + + m := mocks.NewMockWorkspaceClient(t) + b.SetWorkpaceClient(m.WorkspaceClient) + ctx := context.Background() + ctx = cmdio.InContext(ctx, cmdio.NewIO(flags.OutputText, &bytes.Buffer{}, &bytes.Buffer{}, &bytes.Buffer{}, "", "...")) + ctx = cmdio.NewContext(ctx, cmdio.NewLogger(flags.ModeAppend)) + + jobApi := m.GetMockJobsAPI() + + // The runner should not try and cancel existing job runs for unpaused continuous jobs. + jobApi.AssertNotCalled(t, "ListRunsAll") + jobApi.AssertNotCalled(t, "CancelRun") + + // Mock the runner triggering a job run + mockWaitForRun := &jobs.WaitGetRunJobTerminatedOrSkipped[jobs.RunNowResponse]{ + Poll: func(d time.Duration, f func(*jobs.Run)) (*jobs.Run, error) { + return &jobs.Run{ + State: &jobs.RunState{ + ResultState: jobs.RunResultStateSuccess, + }, + }, nil + }, + } + jobApi.EXPECT().RunNow(mock.Anything, jobs.RunNow{ + JobId: 123, + }).Return(mockWaitForRun, nil) + + // Mock the runner getting the job output + jobApi.EXPECT().GetRun(mock.Anything, jobs.GetRunRequest{}).Return(&jobs.Run{}, nil) + + _, err := runner.Restart(ctx, &Options{}) + require.NoError(t, err) +} diff --git a/bundle/run/keys.go b/bundle/run/keys.go deleted file mode 100644 index 76ec50ac..00000000 --- a/bundle/run/keys.go +++ /dev/null @@ -1,69 +0,0 @@ -package run - -import ( - "fmt" - - "github.com/databricks/cli/bundle" - "golang.org/x/exp/maps" -) - -// RunnerLookup maps identifiers to a list of workloads that match that identifier. -// The list can have more than 1 entry if resources of different types use the -// same key. When this happens, the user should disambiguate between them. -type RunnerLookup map[string][]Runner - -// ResourceKeys computes a map with -func ResourceKeys(b *bundle.Bundle) (keyOnly RunnerLookup, keyWithType RunnerLookup) { - keyOnly = make(RunnerLookup) - keyWithType = make(RunnerLookup) - - r := b.Config.Resources - for k, v := range r.Jobs { - kt := fmt.Sprintf("jobs.%s", k) - w := jobRunner{key: key(kt), bundle: b, job: v} - keyOnly[k] = append(keyOnly[k], &w) - keyWithType[kt] = append(keyWithType[kt], &w) - } - for k, v := range r.Pipelines { - kt := fmt.Sprintf("pipelines.%s", k) - w := pipelineRunner{key: key(kt), bundle: b, pipeline: v} - keyOnly[k] = append(keyOnly[k], &w) - keyWithType[kt] = append(keyWithType[kt], &w) - } - return -} - -// ResourceCompletionMap returns a map of resource keys to their respective names. -func ResourceCompletionMap(b *bundle.Bundle) map[string]string { - out := make(map[string]string) - keyOnly, keyWithType := ResourceKeys(b) - - // Keep track of resources we have seen by their fully qualified key. - seen := make(map[string]bool) - - // First add resources that can be identified by key alone. - for k, v := range keyOnly { - // Invariant: len(v) >= 1. See [ResourceKeys]. - if len(v) == 1 { - seen[v[0].Key()] = true - out[k] = v[0].Name() - } - } - - // Then add resources that can only be identified by their type and key. - for k, v := range keyWithType { - // Invariant: len(v) == 1. See [ResourceKeys]. - _, ok := seen[v[0].Key()] - if ok { - continue - } - out[k] = v[0].Name() - } - - return out -} - -// ResourceCompletions returns a list of keys that unambiguously reference resources in the bundle. -func ResourceCompletions(b *bundle.Bundle) []string { - return maps.Keys(ResourceCompletionMap(b)) -} diff --git a/bundle/run/keys_test.go b/bundle/run/keys_test.go deleted file mode 100644 index 5ab73b13..00000000 --- a/bundle/run/keys_test.go +++ /dev/null @@ -1,25 +0,0 @@ -package run - -import ( - "testing" - - "github.com/databricks/cli/bundle" - "github.com/databricks/cli/bundle/config" - "github.com/databricks/cli/bundle/config/resources" - "github.com/stretchr/testify/assert" -) - -func TestResourceCompletionsUnique(t *testing.T) { - b := &bundle.Bundle{ - Config: config.Root{ - Resources: config.Resources{ - Jobs: map[string]*resources.Job{ - "foo": {}, - "bar": {}, - }, - }, - }, - } - - assert.ElementsMatch(t, []string{"foo", "bar"}, ResourceCompletions(b)) -} diff --git a/bundle/run/pipeline.go b/bundle/run/pipeline.go index d684f838..ffe01284 100644 --- a/bundle/run/pipeline.go +++ b/bundle/run/pipeline.go @@ -183,6 +183,18 @@ func (r *pipelineRunner) Cancel(ctx context.Context) error { return err } +func (r *pipelineRunner) Restart(ctx context.Context, opts *Options) (output.RunOutput, error) { + s := cmdio.Spinner(ctx) + s <- "Cancelling the active pipeline update" + err := r.Cancel(ctx) + close(s) + if err != nil { + return nil, err + } + + return r.Run(ctx, opts) +} + func (r *pipelineRunner) ParseArgs(args []string, opts *Options) error { if len(args) == 0 { return nil diff --git a/bundle/run/pipeline_test.go b/bundle/run/pipeline_test.go index 29b57ffd..e4608061 100644 --- a/bundle/run/pipeline_test.go +++ b/bundle/run/pipeline_test.go @@ -1,6 +1,7 @@ package run import ( + "bytes" "context" "testing" "time" @@ -8,8 +9,12 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/flags" + sdk_config "github.com/databricks/databricks-sdk-go/config" "github.com/databricks/databricks-sdk-go/experimental/mocks" "github.com/databricks/databricks-sdk-go/service/pipelines" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" ) @@ -47,3 +52,68 @@ func TestPipelineRunnerCancel(t *testing.T) { err := runner.Cancel(context.Background()) require.NoError(t, err) } + +func TestPipelineRunnerRestart(t *testing.T) { + pipeline := &resources.Pipeline{ + ID: "123", + } + + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + Pipelines: map[string]*resources.Pipeline{ + "test_pipeline": pipeline, + }, + }, + }, + } + + runner := pipelineRunner{key: "test", bundle: b, pipeline: pipeline} + + m := mocks.NewMockWorkspaceClient(t) + m.WorkspaceClient.Config = &sdk_config.Config{ + Host: "https://test.com", + } + b.SetWorkpaceClient(m.WorkspaceClient) + ctx := context.Background() + ctx = cmdio.InContext(ctx, cmdio.NewIO(flags.OutputText, &bytes.Buffer{}, &bytes.Buffer{}, &bytes.Buffer{}, "", "...")) + ctx = cmdio.NewContext(ctx, cmdio.NewLogger(flags.ModeAppend)) + + mockWait := &pipelines.WaitGetPipelineIdle[struct{}]{ + Poll: func(time.Duration, func(*pipelines.GetPipelineResponse)) (*pipelines.GetPipelineResponse, error) { + return nil, nil + }, + } + + pipelineApi := m.GetMockPipelinesAPI() + pipelineApi.EXPECT().Stop(mock.Anything, pipelines.StopRequest{ + PipelineId: "123", + }).Return(mockWait, nil) + + pipelineApi.EXPECT().GetByPipelineId(mock.Anything, "123").Return(&pipelines.GetPipelineResponse{}, nil) + + // Mock runner starting a new update + pipelineApi.EXPECT().StartUpdate(mock.Anything, pipelines.StartUpdate{ + PipelineId: "123", + }).Return(&pipelines.StartUpdateResponse{ + UpdateId: "456", + }, nil) + + // Mock runner polling for events + pipelineApi.EXPECT().ListPipelineEventsAll(mock.Anything, pipelines.ListPipelineEventsRequest{ + Filter: `update_id = '456'`, + MaxResults: 100, + PipelineId: "123", + }).Return([]pipelines.PipelineEvent{}, nil) + + // Mock runner polling for update status + pipelineApi.EXPECT().GetUpdateByPipelineIdAndUpdateId(mock.Anything, "123", "456"). + Return(&pipelines.GetUpdateResponse{ + Update: &pipelines.UpdateInfo{ + State: pipelines.UpdateInfoStateCompleted, + }, + }, nil) + + _, err := runner.Restart(ctx, &Options{}) + require.NoError(t, err) +} diff --git a/bundle/run/runner.go b/bundle/run/runner.go index 0f202ce7..4c907d06 100644 --- a/bundle/run/runner.go +++ b/bundle/run/runner.go @@ -3,9 +3,10 @@ package run import ( "context" "fmt" - "strings" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config/resources" + refs "github.com/databricks/cli/bundle/resources" "github.com/databricks/cli/bundle/run/output" ) @@ -27,6 +28,10 @@ type Runner interface { // Run the underlying worklow. Run(ctx context.Context, opts *Options) (output.RunOutput, error) + // Restart the underlying workflow by cancelling any existing runs before + // starting a new one. + Restart(ctx context.Context, opts *Options) (output.RunOutput, error) + // Cancel the underlying workflow. Cancel(ctx context.Context) error @@ -34,34 +39,24 @@ type Runner interface { argsHandler } -// Find locates a runner matching the specified argument. -// -// Its behavior is as follows: -// 1. Try to find a resource with identical to the argument. -// 2. Try to find a resource with . identical to the argument. -// -// If an argument resolves to multiple resources, it returns an error. -func Find(b *bundle.Bundle, arg string) (Runner, error) { - keyOnly, keyWithType := ResourceKeys(b) - if len(keyWithType) == 0 { - return nil, fmt.Errorf("bundle defines no resources") +// IsRunnable returns a filter that only allows runnable resources. +func IsRunnable(ref refs.Reference) bool { + switch ref.Resource.(type) { + case *resources.Job, *resources.Pipeline: + return true + default: + return false + } +} + +// ToRunner converts a resource reference to a runnable resource. +func ToRunner(b *bundle.Bundle, ref refs.Reference) (Runner, error) { + switch resource := ref.Resource.(type) { + case *resources.Job: + return &jobRunner{key: key(ref.KeyWithType), bundle: b, job: resource}, nil + case *resources.Pipeline: + return &pipelineRunner{key: key(ref.KeyWithType), bundle: b, pipeline: resource}, nil + default: + return nil, fmt.Errorf("unsupported resource type: %T", resource) } - - runners, ok := keyOnly[arg] - if !ok { - runners, ok = keyWithType[arg] - if !ok { - return nil, fmt.Errorf("no such resource: %s", arg) - } - } - - if len(runners) != 1 { - var keys []string - for _, runner := range runners { - keys = append(keys, runner.Key()) - } - return nil, fmt.Errorf("ambiguous: %s (can resolve to all of %s)", arg, strings.Join(keys, ", ")) - } - - return runners[0], nil } diff --git a/bundle/run/runner_test.go b/bundle/run/runner_test.go index 85baa192..2fc5fa6f 100644 --- a/bundle/run/runner_test.go +++ b/bundle/run/runner_test.go @@ -3,82 +3,14 @@ package run import ( "testing" - "github.com/databricks/cli/bundle" - "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/config/resources" + refs "github.com/databricks/cli/bundle/resources" "github.com/stretchr/testify/assert" ) -func TestFindNoResources(t *testing.T) { - b := &bundle.Bundle{ - Config: config.Root{ - Resources: config.Resources{}, - }, - } - - _, err := Find(b, "foo") - assert.ErrorContains(t, err, "bundle defines no resources") -} - -func TestFindSingleArg(t *testing.T) { - b := &bundle.Bundle{ - Config: config.Root{ - Resources: config.Resources{ - Jobs: map[string]*resources.Job{ - "foo": {}, - }, - }, - }, - } - - _, err := Find(b, "foo") - assert.NoError(t, err) -} - -func TestFindSingleArgNotFound(t *testing.T) { - b := &bundle.Bundle{ - Config: config.Root{ - Resources: config.Resources{ - Jobs: map[string]*resources.Job{ - "foo": {}, - }, - }, - }, - } - - _, err := Find(b, "bar") - assert.ErrorContains(t, err, "no such resource: bar") -} - -func TestFindSingleArgAmbiguous(t *testing.T) { - b := &bundle.Bundle{ - Config: config.Root{ - Resources: config.Resources{ - Jobs: map[string]*resources.Job{ - "key": {}, - }, - Pipelines: map[string]*resources.Pipeline{ - "key": {}, - }, - }, - }, - } - - _, err := Find(b, "key") - assert.ErrorContains(t, err, "ambiguous: ") -} - -func TestFindSingleArgWithType(t *testing.T) { - b := &bundle.Bundle{ - Config: config.Root{ - Resources: config.Resources{ - Jobs: map[string]*resources.Job{ - "key": {}, - }, - }, - }, - } - - _, err := Find(b, "jobs.key") - assert.NoError(t, err) +func TestRunner_IsRunnable(t *testing.T) { + assert.True(t, IsRunnable(refs.Reference{Resource: &resources.Job{}})) + assert.True(t, IsRunnable(refs.Reference{Resource: &resources.Pipeline{}})) + assert.False(t, IsRunnable(refs.Reference{Resource: &resources.MlflowModel{}})) + assert.False(t, IsRunnable(refs.Reference{Resource: &resources.MlflowExperiment{}})) } diff --git a/bundle/schema/jsonschema.json b/bundle/schema/jsonschema.json index 06b9cc15..62e5fe6d 100644 --- a/bundle/schema/jsonschema.json +++ b/bundle/schema/jsonschema.json @@ -180,6 +180,48 @@ } ] }, + "resources.Dashboard": { + "anyOf": [ + { + "type": "object", + "properties": { + "display_name": { + "description": "The display name of the dashboard.", + "$ref": "#/$defs/string" + }, + "embed_credentials": { + "$ref": "#/$defs/bool" + }, + "file_path": { + "$ref": "#/$defs/string" + }, + "parent_path": { + "description": "The workspace path of the folder containing the dashboard. Includes leading slash and no\ntrailing slash.\nThis field is excluded in List Dashboards responses.", + "$ref": "#/$defs/string" + }, + "permissions": { + "$ref": "#/$defs/slice/github.com/databricks/cli/bundle/config/resources.Permission" + }, + "serialized_dashboard": { + "description": "The contents of the dashboard in serialized string form.\nThis field is excluded in List Dashboards responses.\nUse the [get dashboard API](https://docs.databricks.com/api/workspace/lakeview/get)\nto retrieve an example response, which includes the `serialized_dashboard` field.\nThis field provides the structure of the JSON string that represents the dashboard's\nlayout and components.", + "$ref": "#/$defs/interface" + }, + "warehouse_id": { + "description": "The warehouse ID used to run the dashboard.", + "$ref": "#/$defs/string" + } + }, + "additionalProperties": false, + "required": [ + "display_name" + ] + }, + { + "type": "string", + "pattern": "\\$\\{(var(\\.[a-zA-Z]+([-_]?[a-zA-Z0-9]+)*(\\[[0-9]+\\])*)+)\\}" + } + ] + }, "resources.Grant": { "anyOf": [ { @@ -209,6 +251,10 @@ { "type": "object", "properties": { + "budget_policy_id": { + "description": "The id of the user specified budget policy to use for this job.\nIf not specified, a default budget policy may be applied when creating or modifying the job.\nSee `effective_budget_policy_id` for the budget policy used by this workload.", + "$ref": "#/$defs/string" + }, "continuous": { "description": "An optional continuous property for this job. The continuous property will ensure that there is always one run executing. Only one of `schedule` and `continuous` can be used.", "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/jobs.Continuous" @@ -1050,6 +1096,9 @@ "clusters": { "$ref": "#/$defs/map/github.com/databricks/cli/bundle/config/resources.Cluster" }, + "dashboards": { + "$ref": "#/$defs/map/github.com/databricks/cli/bundle/config/resources.Dashboard" + }, "experiments": { "$ref": "#/$defs/map/github.com/databricks/cli/bundle/config/resources.MlflowExperiment" }, @@ -3901,6 +3950,10 @@ { "type": "object", "properties": { + "report": { + "description": "Select tables from a specific source report.", + "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/pipelines.ReportSpec" + }, "schema": { "description": "Select tables from a specific source schema.", "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/pipelines.SchemaSpec" @@ -4233,6 +4286,40 @@ } ] }, + "pipelines.ReportSpec": { + "anyOf": [ + { + "type": "object", + "properties": { + "destination_catalog": { + "description": "Required. Destination catalog to store table.", + "$ref": "#/$defs/string" + }, + "destination_schema": { + "description": "Required. Destination schema to store table.", + "$ref": "#/$defs/string" + }, + "destination_table": { + "description": "Required. Destination table name. The pipeline fails if a table with that name already exists.", + "$ref": "#/$defs/string" + }, + "source_url": { + "description": "Required. Report URL in the source system.", + "$ref": "#/$defs/string" + }, + "table_configuration": { + "description": "Configuration settings to control the ingestion of tables. These settings override the table_configuration defined in the IngestionPipelineDefinition object.", + "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/pipelines.TableSpecificConfig" + } + }, + "additionalProperties": false + }, + { + "type": "string", + "pattern": "\\$\\{(var(\\.[a-zA-Z]+([-_]?[a-zA-Z0-9]+)*(\\[[0-9]+\\])*)+)\\}" + } + ] + }, "pipelines.SchemaSpec": { "anyOf": [ { @@ -4281,7 +4368,7 @@ "$ref": "#/$defs/string" }, "destination_table": { - "description": "Optional. Destination table name. The pipeline fails If a table with that name already exists. If not set, the source table name is used.", + "description": "Optional. Destination table name. The pipeline fails if a table with that name already exists. If not set, the source table name is used.", "$ref": "#/$defs/string" }, "source_catalog": { @@ -4329,6 +4416,10 @@ "SCD_TYPE_1", "SCD_TYPE_2" ] + }, + "sequence_by": { + "description": "The column names specifying the logical order of events in the source data. Delta Live Tables uses this sequencing to handle change events that arrive out of order.", + "$ref": "#/$defs/slice/string" } }, "additionalProperties": false @@ -5246,6 +5337,20 @@ } ] }, + "resources.Dashboard": { + "anyOf": [ + { + "type": "object", + "additionalProperties": { + "$ref": "#/$defs/github.com/databricks/cli/bundle/config/resources.Dashboard" + } + }, + { + "type": "string", + "pattern": "\\$\\{(var(\\.[a-zA-Z]+([-_]?[a-zA-Z0-9]+)*(\\[[0-9]+\\])*)+)\\}" + } + ] + }, "resources.Job": { "anyOf": [ { diff --git a/bundle/tests/issue_1828/databricks.yml b/bundle/tests/issue_1828/databricks.yml new file mode 100644 index 00000000..d5f60ce7 --- /dev/null +++ b/bundle/tests/issue_1828/databricks.yml @@ -0,0 +1,33 @@ +bundle: + name: issue_1828 + +variables: + # One entry for each of the underlying YAML (or [dyn.Kind]) types. + # The test confirms we can convert to and from the typed configuration without losing information. + + map: + default: + foo: bar + + sequence: + default: + - foo + - bar + + string: + default: foo + + bool: + default: true + + int: + default: 42 + + float: + default: 3.14 + + time: + default: 2021-01-01 + + nil: + default: diff --git a/bundle/tests/issue_1828_test.go b/bundle/tests/issue_1828_test.go new file mode 100644 index 00000000..5f2becce --- /dev/null +++ b/bundle/tests/issue_1828_test.go @@ -0,0 +1,48 @@ +package config_tests + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestIssue1828(t *testing.T) { + b := load(t, "./issue_1828") + + if assert.Contains(t, b.Config.Variables, "map") { + assert.Equal(t, map[string]any{ + "foo": "bar", + }, b.Config.Variables["map"].Default) + } + + if assert.Contains(t, b.Config.Variables, "sequence") { + assert.Equal(t, []any{ + "foo", + "bar", + }, b.Config.Variables["sequence"].Default) + } + + if assert.Contains(t, b.Config.Variables, "string") { + assert.Equal(t, "foo", b.Config.Variables["string"].Default) + } + + if assert.Contains(t, b.Config.Variables, "bool") { + assert.Equal(t, true, b.Config.Variables["bool"].Default) + } + + if assert.Contains(t, b.Config.Variables, "int") { + assert.Equal(t, 42, b.Config.Variables["int"].Default) + } + + if assert.Contains(t, b.Config.Variables, "float") { + assert.Equal(t, 3.14, b.Config.Variables["float"].Default) + } + + if assert.Contains(t, b.Config.Variables, "time") { + assert.Equal(t, "2021-01-01", b.Config.Variables["time"].Default) + } + + if assert.Contains(t, b.Config.Variables, "nil") { + assert.Equal(t, nil, b.Config.Variables["nil"].Default) + } +} diff --git a/cmd/bundle/bundle.go b/cmd/bundle/bundle.go index 0880c9c4..fb88cd7d 100644 --- a/cmd/bundle/bundle.go +++ b/cmd/bundle/bundle.go @@ -27,5 +27,6 @@ func New() *cobra.Command { cmd.AddCommand(newGenerateCommand()) cmd.AddCommand(newDebugCommand()) cmd.AddCommand(deployment.NewDeploymentCommand()) + cmd.AddCommand(newOpenCommand()) return cmd } diff --git a/cmd/bundle/deploy.go b/cmd/bundle/deploy.go index f1c85cb3..a25e02f6 100644 --- a/cmd/bundle/deploy.go +++ b/cmd/bundle/deploy.go @@ -78,7 +78,7 @@ func newDeployCommand() *cobra.Command { } renderOpts := render.RenderOptions{RenderSummaryTable: false} - err := render.RenderTextOutput(cmd.OutOrStdout(), b, diags, renderOpts) + err := render.RenderDiagnostics(cmd.OutOrStdout(), b, diags, renderOpts) if err != nil { return fmt.Errorf("failed to render output: %w", err) } diff --git a/cmd/bundle/open.go b/cmd/bundle/open.go new file mode 100644 index 00000000..a2ad32fd --- /dev/null +++ b/cmd/bundle/open.go @@ -0,0 +1,144 @@ +package bundle + +import ( + "context" + "errors" + "fmt" + "os" + "path/filepath" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config/mutator" + "github.com/databricks/cli/bundle/deploy/terraform" + "github.com/databricks/cli/bundle/phases" + "github.com/databricks/cli/bundle/resources" + "github.com/databricks/cli/cmd/bundle/utils" + "github.com/databricks/cli/cmd/root" + "github.com/databricks/cli/libs/cmdio" + "github.com/spf13/cobra" + "golang.org/x/exp/maps" + + "github.com/pkg/browser" +) + +func promptOpenArgument(ctx context.Context, b *bundle.Bundle) (string, error) { + // Compute map of "Human readable name of resource" -> "resource key". + inv := make(map[string]string) + for k, ref := range resources.Completions(b) { + title := fmt.Sprintf("%s: %s", ref.Description.SingularTitle, ref.Resource.GetName()) + inv[title] = k + } + + key, err := cmdio.Select(ctx, inv, "Resource to open") + if err != nil { + return "", err + } + + return key, nil +} + +func resolveOpenArgument(ctx context.Context, b *bundle.Bundle, args []string) (string, error) { + // If no arguments are specified, prompt the user to select the resource to open. + if len(args) == 0 && cmdio.IsPromptSupported(ctx) { + return promptOpenArgument(ctx, b) + } + + if len(args) < 1 { + return "", fmt.Errorf("expected a KEY of the resource to open") + } + + return args[0], nil +} + +func newOpenCommand() *cobra.Command { + cmd := &cobra.Command{ + Use: "open", + Short: "Open a resource in the browser", + Args: root.MaximumNArgs(1), + } + + var forcePull bool + cmd.Flags().BoolVar(&forcePull, "force-pull", false, "Skip local cache and load the state from the remote workspace") + + cmd.RunE = func(cmd *cobra.Command, args []string) error { + ctx := cmd.Context() + b, diags := utils.ConfigureBundleWithVariables(cmd) + if err := diags.Error(); err != nil { + return diags.Error() + } + + diags = bundle.Apply(ctx, b, phases.Initialize()) + if err := diags.Error(); err != nil { + return err + } + + arg, err := resolveOpenArgument(ctx, b, args) + if err != nil { + return err + } + + cacheDir, err := terraform.Dir(ctx, b) + if err != nil { + return err + } + _, stateFileErr := os.Stat(filepath.Join(cacheDir, terraform.TerraformStateFileName)) + _, configFileErr := os.Stat(filepath.Join(cacheDir, terraform.TerraformConfigFileName)) + noCache := errors.Is(stateFileErr, os.ErrNotExist) || errors.Is(configFileErr, os.ErrNotExist) + + if forcePull || noCache { + diags = bundle.Apply(ctx, b, bundle.Seq( + terraform.StatePull(), + terraform.Interpolate(), + terraform.Write(), + )) + if err := diags.Error(); err != nil { + return err + } + } + + diags = bundle.Apply(ctx, b, bundle.Seq( + terraform.Load(), + mutator.InitializeURLs(), + )) + if err := diags.Error(); err != nil { + return err + } + + // Locate resource to open. + ref, err := resources.Lookup(b, arg) + if err != nil { + return err + } + + // Confirm that the resource has a URL. + url := ref.Resource.GetURL() + if url == "" { + return fmt.Errorf("resource does not have a URL associated with it (has it been deployed?)") + } + + return browser.OpenURL(url) + } + + cmd.ValidArgsFunction = func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { + b, diags := root.MustConfigureBundle(cmd) + if err := diags.Error(); err != nil { + cobra.CompErrorln(err.Error()) + return nil, cobra.ShellCompDirectiveError + } + + // No completion in the context of a bundle. + // Source and destination paths are taken from bundle configuration. + if b == nil { + return nil, cobra.ShellCompDirectiveNoFileComp + } + + if len(args) == 0 { + completions := resources.Completions(b) + return maps.Keys(completions), cobra.ShellCompDirectiveNoFileComp + } else { + return nil, cobra.ShellCompDirectiveNoFileComp + } + } + + return cmd +} diff --git a/cmd/bundle/run.go b/cmd/bundle/run.go index 9ef5eb8f..96851d0c 100644 --- a/cmd/bundle/run.go +++ b/cmd/bundle/run.go @@ -1,20 +1,69 @@ package bundle import ( + "context" "encoding/json" "fmt" "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/deploy/terraform" "github.com/databricks/cli/bundle/phases" + "github.com/databricks/cli/bundle/resources" "github.com/databricks/cli/bundle/run" + "github.com/databricks/cli/bundle/run/output" "github.com/databricks/cli/cmd/bundle/utils" "github.com/databricks/cli/cmd/root" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/flags" "github.com/spf13/cobra" + "golang.org/x/exp/maps" ) +func promptRunArgument(ctx context.Context, b *bundle.Bundle) (string, error) { + // Compute map of "Human readable name of resource" -> "resource key". + inv := make(map[string]string) + for k, ref := range resources.Completions(b, run.IsRunnable) { + title := fmt.Sprintf("%s: %s", ref.Description.SingularTitle, ref.Resource.GetName()) + inv[title] = k + } + + key, err := cmdio.Select(ctx, inv, "Resource to run") + if err != nil { + return "", err + } + + return key, nil +} + +func resolveRunArgument(ctx context.Context, b *bundle.Bundle, args []string) (string, error) { + // If no arguments are specified, prompt the user to select something to run. + if len(args) == 0 && cmdio.IsPromptSupported(ctx) { + return promptRunArgument(ctx, b) + } + + if len(args) < 1 { + return "", fmt.Errorf("expected a KEY of the resource to run") + } + + return args[0], nil +} + +func keyToRunner(b *bundle.Bundle, arg string) (run.Runner, error) { + // Locate the resource to run. + ref, err := resources.Lookup(b, arg, run.IsRunnable) + if err != nil { + return nil, err + } + + // Convert the resource to a runnable resource. + runner, err := run.ToRunner(b, ref) + if err != nil { + return nil, err + } + + return runner, nil +} + func newRunCommand() *cobra.Command { cmd := &cobra.Command{ Use: "run [flags] KEY", @@ -60,22 +109,9 @@ task or a Python wheel task, the second example applies. return err } - // If no arguments are specified, prompt the user to select something to run. - if len(args) == 0 && cmdio.IsPromptSupported(ctx) { - // Invert completions from KEY -> NAME, to NAME -> KEY. - inv := make(map[string]string) - for k, v := range run.ResourceCompletionMap(b) { - inv[v] = k - } - id, err := cmdio.Select(ctx, inv, "Resource to run") - if err != nil { - return err - } - args = append(args, id) - } - - if len(args) < 1 { - return fmt.Errorf("expected a KEY of the resource to run") + arg, err := resolveRunArgument(ctx, b, args) + if err != nil { + return err } diags = bundle.Apply(ctx, b, bundle.Seq( @@ -88,7 +124,7 @@ task or a Python wheel task, the second example applies. return err } - runner, err := run.Find(b, args[0]) + runner, err := keyToRunner(b, arg) if err != nil { return err } @@ -100,19 +136,16 @@ task or a Python wheel task, the second example applies. } runOptions.NoWait = noWait + var output output.RunOutput if restart { - s := cmdio.Spinner(ctx) - s <- "Cancelling all runs" - err := runner.Cancel(ctx) - close(s) - if err != nil { - return err - } + output, err = runner.Restart(ctx, &runOptions) + } else { + output, err = runner.Run(ctx, &runOptions) } - output, err := runner.Run(ctx, &runOptions) if err != nil { return err } + if output != nil { switch root.OutputType(cmd) { case flags.OutputText: @@ -148,10 +181,11 @@ task or a Python wheel task, the second example applies. } if len(args) == 0 { - return run.ResourceCompletions(b), cobra.ShellCompDirectiveNoFileComp + completions := resources.Completions(b, run.IsRunnable) + return maps.Keys(completions), cobra.ShellCompDirectiveNoFileComp } else { // If we know the resource to run, we can complete additional positional arguments. - runner, err := run.Find(b, args[0]) + runner, err := keyToRunner(b, args[0]) if err != nil { return nil, cobra.ShellCompDirectiveError } diff --git a/cmd/bundle/summary.go b/cmd/bundle/summary.go index 5a64b46c..8c34dd61 100644 --- a/cmd/bundle/summary.go +++ b/cmd/bundle/summary.go @@ -8,8 +8,10 @@ import ( "path/filepath" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config/mutator" "github.com/databricks/cli/bundle/deploy/terraform" "github.com/databricks/cli/bundle/phases" + "github.com/databricks/cli/bundle/render" "github.com/databricks/cli/cmd/bundle/utils" "github.com/databricks/cli/cmd/root" "github.com/databricks/cli/libs/flags" @@ -19,11 +21,8 @@ import ( func newSummaryCommand() *cobra.Command { cmd := &cobra.Command{ Use: "summary", - Short: "Describe the bundle resources and their deployment states", + Short: "Summarize resources deployed by this bundle", Args: root.NoArgs, - - // This command is currently intended for the Databricks VSCode extension only - Hidden: true, } var forcePull bool @@ -60,14 +59,15 @@ func newSummaryCommand() *cobra.Command { } } - diags = bundle.Apply(ctx, b, terraform.Load()) + diags = bundle.Apply(ctx, b, + bundle.Seq(terraform.Load(), mutator.InitializeURLs())) if err := diags.Error(); err != nil { return err } switch root.OutputType(cmd) { case flags.OutputText: - return fmt.Errorf("%w, only json output is supported", errors.ErrUnsupported) + return render.RenderSummary(ctx, cmd.OutOrStdout(), b) case flags.OutputJSON: buf, err := json.MarshalIndent(b.Config, "", " ") if err != nil { diff --git a/cmd/bundle/sync.go b/cmd/bundle/sync.go index df3e087c..274bba0e 100644 --- a/cmd/bundle/sync.go +++ b/cmd/bundle/sync.go @@ -1,7 +1,9 @@ package bundle import ( + "context" "fmt" + "io" "time" "github.com/databricks/cli/bundle" @@ -9,6 +11,7 @@ import ( "github.com/databricks/cli/bundle/phases" "github.com/databricks/cli/cmd/bundle/utils" "github.com/databricks/cli/cmd/root" + "github.com/databricks/cli/libs/flags" "github.com/databricks/cli/libs/log" "github.com/databricks/cli/libs/sync" "github.com/spf13/cobra" @@ -18,6 +21,7 @@ type syncFlags struct { interval time.Duration full bool watch bool + output flags.Output } func (f *syncFlags) syncOptionsFromBundle(cmd *cobra.Command, b *bundle.Bundle) (*sync.SyncOptions, error) { @@ -26,6 +30,21 @@ func (f *syncFlags) syncOptionsFromBundle(cmd *cobra.Command, b *bundle.Bundle) return nil, fmt.Errorf("cannot get sync options: %w", err) } + if f.output != "" { + var outputFunc func(context.Context, <-chan sync.Event, io.Writer) + switch f.output { + case flags.OutputText: + outputFunc = sync.TextOutput + case flags.OutputJSON: + outputFunc = sync.JsonOutput + } + if outputFunc != nil { + opts.OutputHandler = func(ctx context.Context, c <-chan sync.Event) { + outputFunc(ctx, c, cmd.OutOrStdout()) + } + } + } + opts.Full = f.full opts.PollInterval = f.interval return opts, nil @@ -42,6 +61,7 @@ func newSyncCommand() *cobra.Command { cmd.Flags().DurationVar(&f.interval, "interval", 1*time.Second, "file system polling interval (for --watch)") cmd.Flags().BoolVar(&f.full, "full", false, "perform full synchronization (default is incremental)") cmd.Flags().BoolVar(&f.watch, "watch", false, "watch local file system for changes") + cmd.Flags().Var(&f.output, "output", "type of the output format") cmd.RunE = func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() @@ -65,6 +85,7 @@ func newSyncCommand() *cobra.Command { if err != nil { return err } + defer s.Close() log.Infof(ctx, "Remote file sync location: %v", opts.RemotePath) diff --git a/cmd/bundle/validate.go b/cmd/bundle/validate.go index 496d5d2b..5331e7e7 100644 --- a/cmd/bundle/validate.go +++ b/cmd/bundle/validate.go @@ -54,7 +54,7 @@ func newValidateCommand() *cobra.Command { switch root.OutputType(cmd) { case flags.OutputText: renderOpts := render.RenderOptions{RenderSummaryTable: true} - err := render.RenderTextOutput(cmd.OutOrStdout(), b, diags, renderOpts) + err := render.RenderDiagnostics(cmd.OutOrStdout(), b, diags, renderOpts) if err != nil { return fmt.Errorf("failed to render output: %w", err) } diff --git a/cmd/workspace/apps/apps.go b/cmd/workspace/apps/apps.go index 4cee2f82..9331ddc2 100755 --- a/cmd/workspace/apps/apps.go +++ b/cmd/workspace/apps/apps.go @@ -28,9 +28,6 @@ func New() *cobra.Command { Annotations: map[string]string{ "package": "apps", }, - - // This service is being previewed; hide from help output. - Hidden: true, } // Add methods diff --git a/cmd/workspace/disable-legacy-dbfs/disable-legacy-dbfs.go b/cmd/workspace/disable-legacy-dbfs/disable-legacy-dbfs.go new file mode 100755 index 00000000..d0975537 --- /dev/null +++ b/cmd/workspace/disable-legacy-dbfs/disable-legacy-dbfs.go @@ -0,0 +1,220 @@ +// Code generated from OpenAPI specs by Databricks SDK Generator. DO NOT EDIT. + +package disable_legacy_dbfs + +import ( + "fmt" + + "github.com/databricks/cli/cmd/root" + "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/flags" + "github.com/databricks/databricks-sdk-go/service/settings" + "github.com/spf13/cobra" +) + +// Slice with functions to override default command behavior. +// Functions can be added from the `init()` function in manually curated files in this directory. +var cmdOverrides []func(*cobra.Command) + +func New() *cobra.Command { + cmd := &cobra.Command{ + Use: "disable-legacy-dbfs", + Short: `When this setting is on, access to DBFS root and DBFS mounts is disallowed (as well as creation of new mounts).`, + Long: `When this setting is on, access to DBFS root and DBFS mounts is disallowed (as + well as creation of new mounts). When the setting is off, all DBFS + functionality is enabled`, + + // This service is being previewed; hide from help output. + Hidden: true, + } + + // Add methods + cmd.AddCommand(newDelete()) + cmd.AddCommand(newGet()) + cmd.AddCommand(newUpdate()) + + // Apply optional overrides to this command. + for _, fn := range cmdOverrides { + fn(cmd) + } + + return cmd +} + +// start delete command + +// Slice with functions to override default command behavior. +// Functions can be added from the `init()` function in manually curated files in this directory. +var deleteOverrides []func( + *cobra.Command, + *settings.DeleteDisableLegacyDbfsRequest, +) + +func newDelete() *cobra.Command { + cmd := &cobra.Command{} + + var deleteReq settings.DeleteDisableLegacyDbfsRequest + + // TODO: short flags + + cmd.Flags().StringVar(&deleteReq.Etag, "etag", deleteReq.Etag, `etag used for versioning.`) + + cmd.Use = "delete" + cmd.Short = `Delete the disable legacy DBFS setting.` + cmd.Long = `Delete the disable legacy DBFS setting. + + Deletes the disable legacy DBFS setting for a workspace, reverting back to the + default.` + + cmd.Annotations = make(map[string]string) + + cmd.Args = func(cmd *cobra.Command, args []string) error { + check := root.ExactArgs(0) + return check(cmd, args) + } + + cmd.PreRunE = root.MustWorkspaceClient + cmd.RunE = func(cmd *cobra.Command, args []string) (err error) { + ctx := cmd.Context() + w := root.WorkspaceClient(ctx) + + response, err := w.Settings.DisableLegacyDbfs().Delete(ctx, deleteReq) + if err != nil { + return err + } + return cmdio.Render(ctx, response) + } + + // Disable completions since they are not applicable. + // Can be overridden by manual implementation in `override.go`. + cmd.ValidArgsFunction = cobra.NoFileCompletions + + // Apply optional overrides to this command. + for _, fn := range deleteOverrides { + fn(cmd, &deleteReq) + } + + return cmd +} + +// start get command + +// Slice with functions to override default command behavior. +// Functions can be added from the `init()` function in manually curated files in this directory. +var getOverrides []func( + *cobra.Command, + *settings.GetDisableLegacyDbfsRequest, +) + +func newGet() *cobra.Command { + cmd := &cobra.Command{} + + var getReq settings.GetDisableLegacyDbfsRequest + + // TODO: short flags + + cmd.Flags().StringVar(&getReq.Etag, "etag", getReq.Etag, `etag used for versioning.`) + + cmd.Use = "get" + cmd.Short = `Get the disable legacy DBFS setting.` + cmd.Long = `Get the disable legacy DBFS setting. + + Gets the disable legacy DBFS setting.` + + cmd.Annotations = make(map[string]string) + + cmd.Args = func(cmd *cobra.Command, args []string) error { + check := root.ExactArgs(0) + return check(cmd, args) + } + + cmd.PreRunE = root.MustWorkspaceClient + cmd.RunE = func(cmd *cobra.Command, args []string) (err error) { + ctx := cmd.Context() + w := root.WorkspaceClient(ctx) + + response, err := w.Settings.DisableLegacyDbfs().Get(ctx, getReq) + if err != nil { + return err + } + return cmdio.Render(ctx, response) + } + + // Disable completions since they are not applicable. + // Can be overridden by manual implementation in `override.go`. + cmd.ValidArgsFunction = cobra.NoFileCompletions + + // Apply optional overrides to this command. + for _, fn := range getOverrides { + fn(cmd, &getReq) + } + + return cmd +} + +// start update command + +// Slice with functions to override default command behavior. +// Functions can be added from the `init()` function in manually curated files in this directory. +var updateOverrides []func( + *cobra.Command, + *settings.UpdateDisableLegacyDbfsRequest, +) + +func newUpdate() *cobra.Command { + cmd := &cobra.Command{} + + var updateReq settings.UpdateDisableLegacyDbfsRequest + var updateJson flags.JsonFlag + + // TODO: short flags + cmd.Flags().Var(&updateJson, "json", `either inline JSON string or @path/to/file.json with request body`) + + cmd.Use = "update" + cmd.Short = `Update the disable legacy DBFS setting.` + cmd.Long = `Update the disable legacy DBFS setting. + + Updates the disable legacy DBFS setting for the workspace.` + + cmd.Annotations = make(map[string]string) + + cmd.PreRunE = root.MustWorkspaceClient + cmd.RunE = func(cmd *cobra.Command, args []string) (err error) { + ctx := cmd.Context() + w := root.WorkspaceClient(ctx) + + if cmd.Flags().Changed("json") { + diags := updateJson.Unmarshal(&updateReq) + if diags.HasError() { + return diags.Error() + } + if len(diags) > 0 { + err := cmdio.RenderDiagnosticsToErrorOut(ctx, diags) + if err != nil { + return err + } + } + } else { + return fmt.Errorf("please provide command input in JSON format by specifying the --json flag") + } + + response, err := w.Settings.DisableLegacyDbfs().Update(ctx, updateReq) + if err != nil { + return err + } + return cmdio.Render(ctx, response) + } + + // Disable completions since they are not applicable. + // Can be overridden by manual implementation in `override.go`. + cmd.ValidArgsFunction = cobra.NoFileCompletions + + // Apply optional overrides to this command. + for _, fn := range updateOverrides { + fn(cmd, &updateReq) + } + + return cmd +} + +// end service DisableLegacyDbfs diff --git a/cmd/workspace/jobs/jobs.go b/cmd/workspace/jobs/jobs.go index d4ceb0c2..9e8db43d 100755 --- a/cmd/workspace/jobs/jobs.go +++ b/cmd/workspace/jobs/jobs.go @@ -1557,6 +1557,7 @@ func newSubmit() *cobra.Command { cmd.Flags().Var(&submitJson, "json", `either inline JSON string or @path/to/file.json with request body`) // TODO: array: access_control_list + cmd.Flags().StringVar(&submitReq.BudgetPolicyId, "budget-policy-id", submitReq.BudgetPolicyId, `The user specified id of the budget policy to use for this one-time run.`) // TODO: complex arg: email_notifications // TODO: array: environments // TODO: complex arg: git_source diff --git a/cmd/workspace/settings/settings.go b/cmd/workspace/settings/settings.go index aaeecf41..31e6ceee 100755 --- a/cmd/workspace/settings/settings.go +++ b/cmd/workspace/settings/settings.go @@ -9,6 +9,7 @@ import ( compliance_security_profile "github.com/databricks/cli/cmd/workspace/compliance-security-profile" default_namespace "github.com/databricks/cli/cmd/workspace/default-namespace" disable_legacy_access "github.com/databricks/cli/cmd/workspace/disable-legacy-access" + disable_legacy_dbfs "github.com/databricks/cli/cmd/workspace/disable-legacy-dbfs" enhanced_security_monitoring "github.com/databricks/cli/cmd/workspace/enhanced-security-monitoring" restrict_workspace_admins "github.com/databricks/cli/cmd/workspace/restrict-workspace-admins" ) @@ -33,6 +34,7 @@ func New() *cobra.Command { cmd.AddCommand(compliance_security_profile.New()) cmd.AddCommand(default_namespace.New()) cmd.AddCommand(disable_legacy_access.New()) + cmd.AddCommand(disable_legacy_dbfs.New()) cmd.AddCommand(enhanced_security_monitoring.New()) cmd.AddCommand(restrict_workspace_admins.New()) diff --git a/go.mod b/go.mod index 697205f3..d8679fd6 100644 --- a/go.mod +++ b/go.mod @@ -7,8 +7,8 @@ toolchain go1.22.7 require ( github.com/Masterminds/semver/v3 v3.3.0 // MIT github.com/briandowns/spinner v1.23.1 // Apache 2.0 - github.com/databricks/databricks-sdk-go v0.48.0 // Apache 2.0 - github.com/fatih/color v1.17.0 // MIT + github.com/databricks/databricks-sdk-go v0.49.0 // Apache 2.0 + github.com/fatih/color v1.18.0 // MIT github.com/ghodss/yaml v1.0.0 // MIT + NOTICE github.com/google/uuid v1.6.0 // BSD-3-Clause github.com/hashicorp/go-version v1.7.0 // MPL 2.0 diff --git a/go.sum b/go.sum index 03698b20..c47ae769 100644 --- a/go.sum +++ b/go.sum @@ -32,8 +32,8 @@ github.com/cncf/udpa/go v0.0.0-20191209042840-269d4d468f6f/go.mod h1:M8M6+tZqaGX github.com/cpuguy83/go-md2man/v2 v2.0.4/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= github.com/cyphar/filepath-securejoin v0.2.4 h1:Ugdm7cg7i6ZK6x3xDF1oEu1nfkyfH53EtKeQYTC3kyg= github.com/cyphar/filepath-securejoin v0.2.4/go.mod h1:aPGpWjXOXUn2NCNjFvBE6aRxGGx79pTxQpKOJNYHHl4= -github.com/databricks/databricks-sdk-go v0.48.0 h1:46KtsnRo+FGhC3izUXbpL0PXBNomvsdignYDhJZlm9s= -github.com/databricks/databricks-sdk-go v0.48.0/go.mod h1:ds+zbv5mlQG7nFEU5ojLtgN/u0/9YzZmKQES/CfedzU= +github.com/databricks/databricks-sdk-go v0.49.0 h1:VBTeZZMLIuBSM4kxOCfUcW9z4FUQZY2QeNRD5qm9FUQ= +github.com/databricks/databricks-sdk-go v0.49.0/go.mod h1:ds+zbv5mlQG7nFEU5ojLtgN/u0/9YzZmKQES/CfedzU= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= @@ -44,8 +44,8 @@ github.com/envoyproxy/go-control-plane v0.9.1-0.20191026205805-5f8ba28d4473/go.m github.com/envoyproxy/go-control-plane v0.9.4/go.mod h1:6rpuAdCZL397s3pYoYcLgu1mIlRU8Am5FuJP05cCM98= github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c= github.com/fatih/color v1.9.0/go.mod h1:eQcE1qtQxscV5RaZvpXrrb8Drkc3/DdQ+uUYCNjL+zU= -github.com/fatih/color v1.17.0 h1:GlRw1BRJxkpqUCBKzKOw098ed57fEsKeNjpTe3cSjK4= -github.com/fatih/color v1.17.0/go.mod h1:YZ7TlrGPkiz6ku9fK3TLD/pl3CpsiFyu8N92HLgmosI= +github.com/fatih/color v1.18.0 h1:S8gINlzdQ840/4pfAwic/ZE0djQEH3wM94VfqLTZcOM= +github.com/fatih/color v1.18.0/go.mod h1:4FelSpRwEGDpQ12mAdzqdOukCy4u8WUtOY6lkT/6HfU= github.com/felixge/httpsnoop v1.0.4 h1:NFTV2Zj1bL4mc9sqWACXbQFVBBg2W3GPvqp8/ESS2Wg= github.com/felixge/httpsnoop v1.0.4/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U= github.com/ghodss/yaml v1.0.0 h1:wQHKEahhL6wmXdzwWG11gIVCkOv05bNOh+Rxn0yngAk= diff --git a/internal/acc/workspace.go b/internal/acc/workspace.go index 39374f22..69ab0e71 100644 --- a/internal/acc/workspace.go +++ b/internal/acc/workspace.go @@ -2,11 +2,14 @@ package acc import ( "context" + "fmt" "os" "testing" "github.com/databricks/databricks-sdk-go" + "github.com/databricks/databricks-sdk-go/apierr" "github.com/databricks/databricks-sdk-go/service/compute" + "github.com/databricks/databricks-sdk-go/service/workspace" "github.com/stretchr/testify/require" ) @@ -94,3 +97,30 @@ func (t *WorkspaceT) RunPython(code string) (string, error) { require.True(t, ok, "unexpected type %T", results.Data) return output, nil } + +func (t *WorkspaceT) TemporaryWorkspaceDir(name ...string) string { + ctx := context.Background() + me, err := t.W.CurrentUser.Me(ctx) + require.NoError(t, err) + + basePath := fmt.Sprintf("/Users/%s/%s", me.UserName, RandomName(name...)) + + t.Logf("Creating %s", basePath) + err = t.W.Workspace.MkdirsByPath(ctx, basePath) + require.NoError(t, err) + + // Remove test directory on test completion. + t.Cleanup(func() { + t.Logf("Removing %s", basePath) + err := t.W.Workspace.Delete(ctx, workspace.Delete{ + Path: basePath, + Recursive: true, + }) + if err == nil || apierr.IsMissing(err) { + return + } + t.Logf("Unable to remove temporary workspace directory %s: %#v", basePath, err) + }) + + return basePath +} diff --git a/internal/bugbash/README.md b/internal/bugbash/README.md new file mode 100644 index 00000000..941ab622 --- /dev/null +++ b/internal/bugbash/README.md @@ -0,0 +1,13 @@ +# Bugbash + +The script in this directory can be used to conveniently exec into a shell +where a CLI build for a specific branch is made available. + +## Usage + +This script prompts if you do NOT have at least Bash 5 installed, +but works without command completion with earlier versions. + +```shell +bash <(curl -fsSL https://raw.githubusercontent.com/databricks/cli/main/internal/bugbash/exec.sh) my-branch +``` diff --git a/internal/bugbash/exec.sh b/internal/bugbash/exec.sh new file mode 100755 index 00000000..ac25b16e --- /dev/null +++ b/internal/bugbash/exec.sh @@ -0,0 +1,139 @@ +#!/usr/bin/env bash + +set -euo pipefail + +# Set the GitHub repository for the Databricks CLI. +export GH_REPO="databricks/cli" + +# Synthesize the directory name for the snapshot build. +function cli_snapshot_directory() { + dir="cli" + + # Append OS + case "$(uname -s)" in + Linux) + dir="${dir}_linux" + ;; + Darwin) + dir="${dir}_darwin" + ;; + *) + echo "Unknown operating system: $os" + ;; + esac + + # Append architecture + case "$(uname -m)" in + x86_64) + dir="${dir}_amd64_v1" + ;; + i386|i686) + dir="${dir}_386" + ;; + arm64|aarch64) + dir="${dir}_arm64" + ;; + armv7l|armv8l) + dir="${dir}_arm_6" + ;; + *) + echo "Unknown architecture: $arch" + ;; + esac + + echo $dir +} + +BRANCH=$1 +shift + +# Default to main branch if branch is not specified. +if [ -z "$BRANCH" ]; then + BRANCH=main +fi + +if [ -z "$BRANCH" ]; then + echo "Please specify which branch to bugbash..." + exit 1 +fi + +# Check if the "gh" command is available. +if ! command -v gh &> /dev/null; then + echo "The GitHub CLI (gh) is required to download the snapshot build." + echo "Install and configure it with:" + echo "" + echo " brew install gh" + echo " gh auth login" + echo "" + exit 1 +fi + +echo "Looking for a snapshot build of the Databricks CLI on branch $BRANCH..." + +# Find last successful build on $BRANCH. +last_successful_run_id=$( + gh run list -b "$BRANCH" -w release-snapshot --json 'databaseId,conclusion' | + jq 'limit(1; .[] | select(.conclusion == "success")) | .databaseId' +) +if [ -z "$last_successful_run_id" ]; then + echo "Unable to find last successful build of the release-snapshot workflow for branch $BRANCH." + exit 1 +fi + +# Determine artifact name with the right binaries for this runner. +case "$(uname -s)" in +Linux) + artifact="cli_linux_snapshot" + ;; +Darwin) + artifact="cli_darwin_snapshot" + ;; +esac + +# Create a temporary directory to download the artifact. +dir=$(mktemp -d) + +# Download the artifact. +echo "Downloading the snapshot build..." +gh run download "$last_successful_run_id" -n "$artifact" -D "$dir/.bin" +dir="$dir/.bin/$(cli_snapshot_directory)" +if [ ! -d "$dir" ]; then + echo "Directory does not exist: $dir" + exit 1 +fi + +# Make CLI available on $PATH. +chmod +x "$dir/databricks" +export PATH="$dir:$PATH" + +# Set the prompt to indicate the bugbash environment and exec. +export PS1="(bugbash $BRANCH) \[\033[01;32m\]\u@\h\[\033[00m\]:\[\033[01;34m\]\w\[\033[00m\]\$ " + +# Display completion instructions. +echo "" +echo "==================================================================" + +if [[ ${BASH_VERSINFO[0]} -lt 5 ]]; then + echo -en "\033[31m" + echo "You have Bash version < 5 installed... completion won't work." + echo -en "\033[0m" + echo "" + echo "Install it with:" + echo "" + echo " brew install bash bash-completion" + echo "" + echo "==================================================================" +fi + +echo "" +echo "To load completions in your current shell session:" +echo "" +echo " source /opt/homebrew/etc/profile.d/bash_completion.sh" +echo " source <(databricks completion bash)" +echo "" +echo "==================================================================" +echo "" + +# Exec into a new shell. +# Note: don't use zsh because on macOS it _always_ overwrites PS1. +exec /usr/bin/env bash diff --git a/internal/build/sdk_consistency_test.go b/internal/build/sdk_consistency_test.go new file mode 100644 index 00000000..37f60250 --- /dev/null +++ b/internal/build/sdk_consistency_test.go @@ -0,0 +1,73 @@ +package build + +import ( + "encoding/json" + "fmt" + "io" + "net/http" + "os" + "os/exec" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/mod/modfile" +) + +// This test ensures that the OpenAPI SHA the CLI is being generated from matches +// the OpenAPI SHA of the Go SDK version used in the CLI. We should always upgrade +// the Go SDK version before generating the CLI because downstream generated assets +// like the bundle schema depend on the Go SDK itself. +func TestConsistentDatabricksSdkVersion(t *testing.T) { + // Read the go.mod file + b, err := os.ReadFile("../../go.mod") + require.NoError(t, err) + + // Parse the go.mod file to get the databricks-sdk version + modFile, err := modfile.Parse("../../go.mod", b, nil) + require.NoError(t, err) + + modulePath := "github.com/databricks/databricks-sdk-go" + var version string + for _, r := range modFile.Require { + if r.Mod.Path == modulePath { + version = r.Mod.Version + } + } + require.NotEmpty(t, version) + + // Full path of the package. For example: github.com/databricks/databricks-sdk-go@v0.47.1-0.20241002195128-6cecc224cbf7 + fullPath := fmt.Sprintf("%s@%s", modulePath, version) + + type goListResponse struct { + Origin struct { + Hash string + } + } + + // Using the go CLI query for the git hash corresponding to the databricks-sdk-go version + cmd := exec.Command("go", "list", "-m", "-json", "-mod=readonly", fullPath) + out, err := cmd.Output() + require.NoError(t, err) + parsedOutput := new(goListResponse) + err = json.Unmarshal(out, parsedOutput) + require.NoError(t, err) + hash := parsedOutput.Origin.Hash + require.NotEmpty(t, hash) + + // Read the OpenAPI SHA from the Go SDK. + url := fmt.Sprintf("https://raw.githubusercontent.com/databricks/databricks-sdk-go/%s/.codegen/_openapi_sha", hash) + resp, err := http.Get(url) + require.NoError(t, err) + defer resp.Body.Close() + require.Equal(t, http.StatusOK, resp.StatusCode) + + sdkSha, err := io.ReadAll(resp.Body) + require.NoError(t, err) + + cliSha, err := os.ReadFile("../../.codegen/_openapi_sha") + require.NoError(t, err) + + assert.Equal(t, strings.TrimSpace(string(cliSha)), strings.TrimSpace(string(sdkSha)), "please update the SDK version before generating the CLI") +} diff --git a/internal/bundle/bundles/dashboards/databricks_template_schema.json b/internal/bundle/bundles/dashboards/databricks_template_schema.json new file mode 100644 index 00000000..1aa5728f --- /dev/null +++ b/internal/bundle/bundles/dashboards/databricks_template_schema.json @@ -0,0 +1,12 @@ +{ + "properties": { + "unique_id": { + "type": "string", + "description": "Unique ID for job name" + }, + "warehouse_id": { + "type": "string", + "description": "The SQL warehouse ID to use for the dashboard" + } + } +} diff --git a/internal/bundle/bundles/dashboards/template/dashboard.lvdash.json b/internal/bundle/bundles/dashboards/template/dashboard.lvdash.json new file mode 100644 index 00000000..397a9a12 --- /dev/null +++ b/internal/bundle/bundles/dashboards/template/dashboard.lvdash.json @@ -0,0 +1,34 @@ +{ + "pages": [ + { + "displayName": "New Page", + "layout": [ + { + "position": { + "height": 2, + "width": 6, + "x": 0, + "y": 0 + }, + "widget": { + "name": "82eb9107", + "textbox_spec": "# I'm a title" + } + }, + { + "position": { + "height": 2, + "width": 6, + "x": 0, + "y": 2 + }, + "widget": { + "name": "ffa6de4f", + "textbox_spec": "Text" + } + } + ], + "name": "fdd21a3c" + } + ] +} diff --git a/internal/bundle/bundles/dashboards/template/databricks.yml.tmpl b/internal/bundle/bundles/dashboards/template/databricks.yml.tmpl new file mode 100644 index 00000000..e7771238 --- /dev/null +++ b/internal/bundle/bundles/dashboards/template/databricks.yml.tmpl @@ -0,0 +1,12 @@ +bundle: + name: dashboards + +workspace: + root_path: "~/.bundle/{{.unique_id}}" + +resources: + dashboards: + file_reference: + display_name: test-dashboard-{{.unique_id}} + file_path: ./dashboard.lvdash.json + warehouse_id: {{.warehouse_id}} diff --git a/internal/bundle/dashboards_test.go b/internal/bundle/dashboards_test.go new file mode 100644 index 00000000..b12cc040 --- /dev/null +++ b/internal/bundle/dashboards_test.go @@ -0,0 +1,63 @@ +package bundle + +import ( + "fmt" + "testing" + + "github.com/databricks/cli/internal/acc" + "github.com/databricks/databricks-sdk-go/service/dashboards" + "github.com/databricks/databricks-sdk-go/service/workspace" + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestAccDashboards(t *testing.T) { + ctx, wt := acc.WorkspaceTest(t) + + warehouseID := acc.GetEnvOrSkipTest(t, "TEST_DEFAULT_WAREHOUSE_ID") + uniqueID := uuid.New().String() + root, err := initTestTemplate(t, ctx, "dashboards", map[string]any{ + "unique_id": uniqueID, + "warehouse_id": warehouseID, + }) + require.NoError(t, err) + + t.Cleanup(func() { + err = destroyBundle(t, ctx, root) + require.NoError(t, err) + }) + + err = deployBundle(t, ctx, root) + require.NoError(t, err) + + // Load bundle configuration by running the validate command. + b := unmarshalConfig(t, mustValidateBundle(t, ctx, root)) + + // Assert that the dashboard exists at the expected path and is, indeed, a dashboard. + oi, err := wt.W.Workspace.GetStatusByPath(ctx, fmt.Sprintf("%s/test-dashboard-%s.lvdash.json", b.Config.Workspace.ResourcePath, uniqueID)) + require.NoError(t, err) + assert.EqualValues(t, workspace.ObjectTypeDashboard, oi.ObjectType) + + // Load the dashboard by its ID and confirm its display name. + dashboard, err := wt.W.Lakeview.GetByDashboardId(ctx, oi.ResourceId) + require.NoError(t, err) + assert.Equal(t, fmt.Sprintf("test-dashboard-%s", uniqueID), dashboard.DisplayName) + + // Make an out of band modification to the dashboard and confirm that it is detected. + _, err = wt.W.Lakeview.Update(ctx, dashboards.UpdateDashboardRequest{ + DashboardId: oi.ResourceId, + SerializedDashboard: dashboard.SerializedDashboard, + }) + require.NoError(t, err) + + // Try to redeploy the bundle and confirm that the out of band modification is detected. + stdout, _, err := deployBundleWithArgs(t, ctx, root) + require.Error(t, err) + assert.Contains(t, stdout, `Error: dashboard "file_reference" has been modified remotely`+"\n") + + // Redeploy the bundle with the --force flag and confirm that the out of band modification is ignored. + _, stderr, err := deployBundleWithArgs(t, ctx, root, "--force") + require.NoError(t, err) + assert.Contains(t, stderr, `Deployment complete!`+"\n") +} diff --git a/internal/bundle/helpers.go b/internal/bundle/helpers.go index b8c81a8d..8f1a866f 100644 --- a/internal/bundle/helpers.go +++ b/internal/bundle/helpers.go @@ -11,6 +11,7 @@ import ( "strings" "testing" + "github.com/databricks/cli/bundle" "github.com/databricks/cli/cmd/root" "github.com/databricks/cli/internal" "github.com/databricks/cli/libs/cmdio" @@ -66,6 +67,19 @@ func validateBundle(t *testing.T, ctx context.Context, path string) ([]byte, err return stdout.Bytes(), err } +func mustValidateBundle(t *testing.T, ctx context.Context, path string) []byte { + data, err := validateBundle(t, ctx, path) + require.NoError(t, err) + return data +} + +func unmarshalConfig(t *testing.T, data []byte) *bundle.Bundle { + bundle := &bundle.Bundle{} + err := json.Unmarshal(data, &bundle.Config) + require.NoError(t, err) + return bundle +} + func deployBundle(t *testing.T, ctx context.Context, path string) error { ctx = env.Set(ctx, "BUNDLE_ROOT", path) c := internal.NewCobraTestRunnerWithContext(t, ctx, "bundle", "deploy", "--force-lock", "--auto-approve") @@ -73,6 +87,14 @@ func deployBundle(t *testing.T, ctx context.Context, path string) error { return err } +func deployBundleWithArgs(t *testing.T, ctx context.Context, path string, args ...string) (string, string, error) { + ctx = env.Set(ctx, "BUNDLE_ROOT", path) + args = append([]string{"bundle", "deploy"}, args...) + c := internal.NewCobraTestRunnerWithContext(t, ctx, args...) + stdout, stderr, err := c.Run() + return stdout.String(), stderr.String(), err +} + func deployBundleWithFlags(t *testing.T, ctx context.Context, path string, flags []string) error { ctx = env.Set(ctx, "BUNDLE_ROOT", path) args := []string{"bundle", "deploy", "--force-lock"} diff --git a/internal/dashboard_assumptions_test.go b/internal/dashboard_assumptions_test.go new file mode 100644 index 00000000..912e046b --- /dev/null +++ b/internal/dashboard_assumptions_test.go @@ -0,0 +1,110 @@ +package internal + +import ( + "encoding/base64" + "testing" + + "github.com/databricks/cli/internal/acc" + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/dyn/convert" + "github.com/databricks/cli/libs/dyn/merge" + "github.com/databricks/databricks-sdk-go/apierr" + "github.com/databricks/databricks-sdk-go/service/dashboards" + "github.com/databricks/databricks-sdk-go/service/workspace" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Verify that importing a dashboard through the Workspace API retains the identity of the underying resource, +// as well as properties exclusively accessible through the dashboards API. +func TestAccDashboardAssumptions_WorkspaceImport(t *testing.T) { + ctx, wt := acc.WorkspaceTest(t) + + t.Parallel() + + dashboardName := "New Dashboard" + dashboardPayload := []byte(`{"pages":[{"name":"2506f97a","displayName":"New Page"}]}`) + warehouseId := acc.GetEnvOrSkipTest(t, "TEST_DEFAULT_WAREHOUSE_ID") + + dir := wt.TemporaryWorkspaceDir("dashboard-assumptions-") + + dashboard, err := wt.W.Lakeview.Create(ctx, dashboards.CreateDashboardRequest{ + DisplayName: dashboardName, + ParentPath: dir, + SerializedDashboard: string(dashboardPayload), + WarehouseId: warehouseId, + }) + require.NoError(t, err) + t.Logf("Dashboard ID (per Lakeview API): %s", dashboard.DashboardId) + + // Overwrite the dashboard via the workspace API. + { + err := wt.W.Workspace.Import(ctx, workspace.Import{ + Format: workspace.ImportFormatAuto, + Path: dashboard.Path, + Content: base64.StdEncoding.EncodeToString(dashboardPayload), + Overwrite: true, + }) + require.NoError(t, err) + } + + // Cross-check consistency with the workspace object. + { + obj, err := wt.W.Workspace.GetStatusByPath(ctx, dashboard.Path) + require.NoError(t, err) + + // Confirm that the resource ID included in the response is equal to the dashboard ID. + require.Equal(t, dashboard.DashboardId, obj.ResourceId) + t.Logf("Dashboard ID (per workspace object status): %s", obj.ResourceId) + } + + // Try to overwrite the dashboard via the Lakeview API (and expect failure). + { + _, err := wt.W.Lakeview.Create(ctx, dashboards.CreateDashboardRequest{ + DisplayName: dashboardName, + ParentPath: dir, + SerializedDashboard: string(dashboardPayload), + }) + require.ErrorIs(t, err, apierr.ErrResourceAlreadyExists) + } + + // Retrieve the dashboard object and confirm that only select fields were updated by the import. + { + previousDashboard := dashboard + currentDashboard, err := wt.W.Lakeview.Get(ctx, dashboards.GetDashboardRequest{ + DashboardId: dashboard.DashboardId, + }) + require.NoError(t, err) + + // Convert the dashboard object to a [dyn.Value] to make comparison easier. + previous, err := convert.FromTyped(previousDashboard, dyn.NilValue) + require.NoError(t, err) + current, err := convert.FromTyped(currentDashboard, dyn.NilValue) + require.NoError(t, err) + + // Collect updated paths. + var updatedFieldPaths []string + _, err = merge.Override(previous, current, merge.OverrideVisitor{ + VisitDelete: func(basePath dyn.Path, left dyn.Value) error { + assert.Fail(t, "unexpected delete operation") + return nil + }, + VisitInsert: func(basePath dyn.Path, right dyn.Value) (dyn.Value, error) { + assert.Fail(t, "unexpected insert operation") + return right, nil + }, + VisitUpdate: func(basePath dyn.Path, left dyn.Value, right dyn.Value) (dyn.Value, error) { + updatedFieldPaths = append(updatedFieldPaths, basePath.String()) + return right, nil + }, + }) + require.NoError(t, err) + + // Confirm that only the expected fields have been updated. + assert.ElementsMatch(t, []string{ + "etag", + "update_time", + }, updatedFieldPaths) + } +} diff --git a/internal/fs_mkdir_test.go b/internal/fs_mkdir_test.go index dd75c7c3..9191f614 100644 --- a/internal/fs_mkdir_test.go +++ b/internal/fs_mkdir_test.go @@ -112,8 +112,8 @@ func TestAccFsMkdirWhenFileExistsAtPath(t *testing.T) { // assert mkdir fails _, _, err = RequireErrorRun(t, "fs", "mkdir", path.Join(tmpDir, "hello")) - // Different cloud providers return different errors. - regex := regexp.MustCompile(`(^|: )Path is a file: .*$|(^|: )Cannot create directory .* because .* is an existing file\.$|(^|: )mkdirs\(hadoopPath: .*, permission: rwxrwxrwx\): failed$`) + // Different cloud providers or cloud configurations return different errors. + regex := regexp.MustCompile(`(^|: )Path is a file: .*$|(^|: )Cannot create directory .* because .* is an existing file\.$|(^|: )mkdirs\(hadoopPath: .*, permission: rwxrwxrwx\): failed$|(^|: )"The specified path already exists.".*$`) assert.Regexp(t, regex, err.Error()) }) diff --git a/internal/helpers.go b/internal/helpers.go index 9387706b..3bf38775 100644 --- a/internal/helpers.go +++ b/internal/helpers.go @@ -20,6 +20,7 @@ import ( "time" "github.com/databricks/cli/cmd/root" + "github.com/databricks/cli/internal/acc" "github.com/databricks/cli/libs/flags" "github.com/databricks/cli/cmd" @@ -591,13 +592,10 @@ func setupWsfsExtensionsFiler(t *testing.T) (filer.Filer, string) { } func setupDbfsFiler(t *testing.T) (filer.Filer, string) { - t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) + _, wt := acc.WorkspaceTest(t) - w, err := databricks.NewWorkspaceClient() - require.NoError(t, err) - - tmpDir := TemporaryDbfsDir(t, w) - f, err := filer.NewDbfsClient(w, tmpDir) + tmpDir := TemporaryDbfsDir(t, wt.W) + f, err := filer.NewDbfsClient(wt.W, tmpDir) require.NoError(t, err) return f, path.Join("dbfs:/", tmpDir) diff --git a/libs/dyn/convert/normalize.go b/libs/dyn/convert/normalize.go index bc80a150..106add35 100644 --- a/libs/dyn/convert/normalize.go +++ b/libs/dyn/convert/normalize.go @@ -398,6 +398,34 @@ func (n normalizeOptions) normalizeFloat(typ reflect.Type, src dyn.Value, path d return dyn.NewValue(out, src.Locations()), diags } -func (n normalizeOptions) normalizeInterface(typ reflect.Type, src dyn.Value, path dyn.Path) (dyn.Value, diag.Diagnostics) { +func (n normalizeOptions) normalizeInterface(_ reflect.Type, src dyn.Value, path dyn.Path) (dyn.Value, diag.Diagnostics) { + // Deal with every [dyn.Kind] here to ensure completeness. + switch src.Kind() { + case dyn.KindMap: + // Fall through + case dyn.KindSequence: + // Fall through + case dyn.KindString: + // Fall through + case dyn.KindBool: + // Fall through + case dyn.KindInt: + // Fall through + case dyn.KindFloat: + // Fall through + case dyn.KindTime: + // Conversion of a time value to an interface{}. + // The [dyn.Value.AsAny] equivalent for this kind is the [time.Time] struct. + // If we convert to a typed representation and back again, we cannot distinguish + // a [time.Time] struct from any other struct. + // + // Therefore, we normalize the time value to a string. + return dyn.NewValue(src.MustTime().String(), src.Locations()), nil + case dyn.KindNil: + // Fall through + default: + return dyn.InvalidValue, diag.Errorf("unsupported kind: %s", src.Kind()) + } + return src, nil } diff --git a/libs/dyn/convert/normalize_test.go b/libs/dyn/convert/normalize_test.go index 4b2a3c18..ab0a1cec 100644 --- a/libs/dyn/convert/normalize_test.go +++ b/libs/dyn/convert/normalize_test.go @@ -858,23 +858,7 @@ func TestNormalizeAnchors(t *testing.T) { }, vout.AsAny()) } -func TestNormalizeBoolToAny(t *testing.T) { - var typ any - vin := dyn.NewValue(false, []dyn.Location{{File: "file", Line: 1, Column: 1}}) - vout, err := Normalize(&typ, vin) - assert.Len(t, err, 0) - assert.Equal(t, dyn.NewValue(false, []dyn.Location{{File: "file", Line: 1, Column: 1}}), vout) -} - -func TestNormalizeIntToAny(t *testing.T) { - var typ any - vin := dyn.NewValue(10, []dyn.Location{{File: "file", Line: 1, Column: 1}}) - vout, err := Normalize(&typ, vin) - assert.Len(t, err, 0) - assert.Equal(t, dyn.NewValue(10, []dyn.Location{{File: "file", Line: 1, Column: 1}}), vout) -} - -func TestNormalizeSliceToAny(t *testing.T) { +func TestNormalizeAnyFromSlice(t *testing.T) { var typ any v1 := dyn.NewValue(1, []dyn.Location{{File: "file", Line: 1, Column: 1}}) v2 := dyn.NewValue(2, []dyn.Location{{File: "file", Line: 1, Column: 1}}) @@ -883,3 +867,35 @@ func TestNormalizeSliceToAny(t *testing.T) { assert.Len(t, err, 0) assert.Equal(t, dyn.NewValue([]dyn.Value{v1, v2}, []dyn.Location{{File: "file", Line: 1, Column: 1}}), vout) } + +func TestNormalizeAnyFromString(t *testing.T) { + var typ any + vin := dyn.NewValue("string", []dyn.Location{{File: "file", Line: 1, Column: 1}}) + vout, err := Normalize(&typ, vin) + assert.Len(t, err, 0) + assert.Equal(t, dyn.NewValue("string", []dyn.Location{{File: "file", Line: 1, Column: 1}}), vout) +} + +func TestNormalizeAnyFromBool(t *testing.T) { + var typ any + vin := dyn.NewValue(false, []dyn.Location{{File: "file", Line: 1, Column: 1}}) + vout, err := Normalize(&typ, vin) + assert.Len(t, err, 0) + assert.Equal(t, dyn.NewValue(false, []dyn.Location{{File: "file", Line: 1, Column: 1}}), vout) +} + +func TestNormalizeAnyFromInt(t *testing.T) { + var typ any + vin := dyn.NewValue(10, []dyn.Location{{File: "file", Line: 1, Column: 1}}) + vout, err := Normalize(&typ, vin) + assert.Len(t, err, 0) + assert.Equal(t, dyn.NewValue(10, []dyn.Location{{File: "file", Line: 1, Column: 1}}), vout) +} + +func TestNormalizeAnyFromTime(t *testing.T) { + var typ any + vin := dyn.NewValue(dyn.MustTime("2024-08-29"), []dyn.Location{{File: "file", Line: 1, Column: 1}}) + vout, err := Normalize(&typ, vin) + assert.Empty(t, err) + assert.Equal(t, dyn.NewValue("2024-08-29", vin.Locations()), vout) +} diff --git a/libs/dyn/dynassert/dump.go b/libs/dyn/dynassert/dump.go new file mode 100644 index 00000000..be0c7242 --- /dev/null +++ b/libs/dyn/dynassert/dump.go @@ -0,0 +1,60 @@ +package dynassert + +import ( + "fmt" + "strings" + + "github.com/databricks/cli/libs/dyn" +) + +// Dump returns the Go code to recreate the given value. +func Dump(v dyn.Value) string { + var sb strings.Builder + dump(v, &sb) + return sb.String() +} + +func dump(v dyn.Value, sb *strings.Builder) { + sb.WriteString("dyn.NewValue(\n") + + switch v.Kind() { + case dyn.KindMap: + sb.WriteString("map[string]dyn.Value{") + m := v.MustMap() + for _, p := range m.Pairs() { + sb.WriteString(fmt.Sprintf("\n%q: ", p.Key.MustString())) + dump(p.Value, sb) + sb.WriteByte(',') + } + sb.WriteString("\n},\n") + case dyn.KindSequence: + sb.WriteString("[]dyn.Value{\n") + for _, e := range v.MustSequence() { + dump(e, sb) + sb.WriteByte(',') + } + sb.WriteString("},\n") + case dyn.KindString: + sb.WriteString(fmt.Sprintf("%q,\n", v.MustString())) + case dyn.KindBool: + sb.WriteString(fmt.Sprintf("%t,\n", v.MustBool())) + case dyn.KindInt: + sb.WriteString(fmt.Sprintf("%d,\n", v.MustInt())) + case dyn.KindFloat: + sb.WriteString(fmt.Sprintf("%f,\n", v.MustFloat())) + case dyn.KindTime: + sb.WriteString(fmt.Sprintf("dyn.NewTime(%q),\n", v.MustTime().String())) + case dyn.KindNil: + sb.WriteString("nil,\n") + default: + panic(fmt.Sprintf("unhandled kind: %v", v.Kind())) + } + + // Add location + sb.WriteString("[]dyn.Location{") + for _, l := range v.Locations() { + sb.WriteString(fmt.Sprintf("{File: %q, Line: %d, Column: %d},", l.File, l.Line, l.Column)) + } + sb.WriteString("},\n") + sb.WriteString(")") +} diff --git a/libs/dyn/yamlloader/loader.go b/libs/dyn/yamlloader/loader.go index c3e8d081..b4aaf0a7 100644 --- a/libs/dyn/yamlloader/loader.go +++ b/libs/dyn/yamlloader/loader.go @@ -105,6 +105,9 @@ func (d *loader) loadMapping(node *yaml.Node, loc dyn.Location) (dyn.Value, erro switch st { case "!!str": // OK + case "!!null": + // A literal unquoted "null" is treated as a null value by the YAML parser. + // However, when used as a key, it is treated as the string "null". case "!!merge": if merge != nil { panic("merge node already set") @@ -115,10 +118,11 @@ func (d *loader) loadMapping(node *yaml.Node, loc dyn.Location) (dyn.Value, erro return dyn.InvalidValue, errorf(loc, "invalid key tag: %v", st) } - k, err := d.load(key) - if err != nil { - return dyn.InvalidValue, err - } + k := dyn.NewValue(key.Value, []dyn.Location{{ + File: d.path, + Line: key.Line, + Column: key.Column, + }}) v, err := d.load(val) if err != nil { @@ -173,6 +177,14 @@ func (d *loader) loadMapping(node *yaml.Node, loc dyn.Location) (dyn.Value, erro return dyn.NewValue(out, []dyn.Location{loc}), nil } +func newIntValue(i64 int64, loc dyn.Location) dyn.Value { + // Use regular int type instead of int64 if possible. + if i64 >= math.MinInt32 && i64 <= math.MaxInt32 { + return dyn.NewValue(int(i64), []dyn.Location{loc}) + } + return dyn.NewValue(i64, []dyn.Location{loc}) +} + func (d *loader) loadScalar(node *yaml.Node, loc dyn.Location) (dyn.Value, error) { st := node.ShortTag() switch st { @@ -188,18 +200,44 @@ func (d *loader) loadScalar(node *yaml.Node, loc dyn.Location) (dyn.Value, error return dyn.InvalidValue, errorf(loc, "invalid bool value: %v", node.Value) } case "!!int": - i64, err := strconv.ParseInt(node.Value, 10, 64) - if err != nil { - return dyn.InvalidValue, errorf(loc, "invalid int value: %v", node.Value) + // Try to parse the an integer value in base 10. + // We trim leading zeros to avoid octal parsing of the "0" prefix. + // See "testdata/spec_example_2.19.yml" for background. + i64, err := strconv.ParseInt(strings.TrimLeft(node.Value, "0"), 10, 64) + if err == nil { + return newIntValue(i64, loc), nil } - // Use regular int type instead of int64 if possible. - if i64 >= math.MinInt32 && i64 <= math.MaxInt32 { - return dyn.NewValue(int(i64), []dyn.Location{loc}), nil + // Let the [ParseInt] function figure out the base. + i64, err = strconv.ParseInt(node.Value, 0, 64) + if err == nil { + return newIntValue(i64, loc), nil } - return dyn.NewValue(i64, []dyn.Location{loc}), nil + return dyn.InvalidValue, errorf(loc, "invalid int value: %v", node.Value) case "!!float": f64, err := strconv.ParseFloat(node.Value, 64) if err != nil { + // Deal with infinity prefixes. + v := strings.ToLower(node.Value) + switch { + case strings.HasPrefix(v, "+"): + v = strings.TrimPrefix(v, "+") + f64 = math.Inf(1) + case strings.HasPrefix(v, "-"): + v = strings.TrimPrefix(v, "-") + f64 = math.Inf(-1) + default: + // No prefix. + f64 = math.Inf(1) + } + + // Deal with infinity and NaN values. + switch v { + case ".inf": + return dyn.NewValue(f64, []dyn.Location{loc}), nil + case ".nan": + return dyn.NewValue(math.NaN(), []dyn.Location{loc}), nil + } + return dyn.InvalidValue, errorf(loc, "invalid float value: %v", node.Value) } return dyn.NewValue(f64, []dyn.Location{loc}), nil diff --git a/libs/dyn/yamlloader/testdata/spec_example_2.1.yml b/libs/dyn/yamlloader/testdata/spec_example_2.1.yml new file mode 100644 index 00000000..c9e26274 --- /dev/null +++ b/libs/dyn/yamlloader/testdata/spec_example_2.1.yml @@ -0,0 +1,5 @@ +# Example 2.1 Sequence of Scalars (ball players) + +- Mark McGwire +- Sammy Sosa +- Ken Griffey diff --git a/libs/dyn/yamlloader/testdata/spec_example_2.10.yml b/libs/dyn/yamlloader/testdata/spec_example_2.10.yml new file mode 100644 index 00000000..a3459ded --- /dev/null +++ b/libs/dyn/yamlloader/testdata/spec_example_2.10.yml @@ -0,0 +1,10 @@ +# Example 2.10 Node for “Sammy Sosa” appears twice in this document + +--- +hr: +- Mark McGwire +# Following node labeled SS +- &SS Sammy Sosa +rbi: +- *SS # Subsequent occurrence +- Ken Griffey diff --git a/libs/dyn/yamlloader/testdata/spec_example_2.11.yml b/libs/dyn/yamlloader/testdata/spec_example_2.11.yml new file mode 100644 index 00000000..e3c5c115 --- /dev/null +++ b/libs/dyn/yamlloader/testdata/spec_example_2.11.yml @@ -0,0 +1,10 @@ +# Example 2.11 Mapping between Sequences + +? - Detroit Tigers + - Chicago cubs +: - 2001-07-23 + +? [ New York Yankees, + Atlanta Braves ] +: [ 2001-07-02, 2001-08-12, + 2001-08-14 ] diff --git a/libs/dyn/yamlloader/testdata/spec_example_2.12.yml b/libs/dyn/yamlloader/testdata/spec_example_2.12.yml new file mode 100644 index 00000000..eb4a526f --- /dev/null +++ b/libs/dyn/yamlloader/testdata/spec_example_2.12.yml @@ -0,0 +1,10 @@ +# Example 2.12 Compact Nested Mapping + +--- +# Products purchased +- item : Super Hoop + quantity: 1 +- item : Basketball + quantity: 4 +- item : Big Shoes + quantity: 1 diff --git a/libs/dyn/yamlloader/testdata/spec_example_2.13.yml b/libs/dyn/yamlloader/testdata/spec_example_2.13.yml new file mode 100644 index 00000000..e55abff1 --- /dev/null +++ b/libs/dyn/yamlloader/testdata/spec_example_2.13.yml @@ -0,0 +1,6 @@ +# Example 2.13 In literals, newlines are preserved + +# ASCII Art +--- | + \//||\/|| + // || ||__ diff --git a/libs/dyn/yamlloader/testdata/spec_example_2.14.yml b/libs/dyn/yamlloader/testdata/spec_example_2.14.yml new file mode 100644 index 00000000..439fca30 --- /dev/null +++ b/libs/dyn/yamlloader/testdata/spec_example_2.14.yml @@ -0,0 +1,6 @@ +# Example 2.14 In the folded scalars, newlines become spaces + +--- > + Mark McGwire's + year was crippled + by a knee injury. diff --git a/libs/dyn/yamlloader/testdata/spec_example_2.15.yml b/libs/dyn/yamlloader/testdata/spec_example_2.15.yml new file mode 100644 index 00000000..266e7ce4 --- /dev/null +++ b/libs/dyn/yamlloader/testdata/spec_example_2.15.yml @@ -0,0 +1,10 @@ +# Example 2.15 Folded newlines are preserved for “more indented” and blank lines + +--- > + Sammy Sosa completed another + fine season with great stats. + + 63 Home Runs + 0.288 Batting Average + + What a year! diff --git a/libs/dyn/yamlloader/testdata/spec_example_2.16.yml b/libs/dyn/yamlloader/testdata/spec_example_2.16.yml new file mode 100644 index 00000000..6db6b087 --- /dev/null +++ b/libs/dyn/yamlloader/testdata/spec_example_2.16.yml @@ -0,0 +1,9 @@ +# Example 2.16 Indentation determines scope + +name: Mark McGwire +accomplishment: > + Mark set a major league + home run record in 1998. +stats: | + 65 Home Runs + 0.278 Batting Average diff --git a/libs/dyn/yamlloader/testdata/spec_example_2.17.yml b/libs/dyn/yamlloader/testdata/spec_example_2.17.yml new file mode 100644 index 00000000..af0777ab --- /dev/null +++ b/libs/dyn/yamlloader/testdata/spec_example_2.17.yml @@ -0,0 +1,9 @@ +# Example 2.17 Quoted Scalars + +unicode: "Sosa did fine.\u263A" +control: "\b1998\t1999\t2000\n" +hex esc: "\x0d\x0a is \r\n" + +single: '"Howdy!" he cried.' +quoted: ' # Not a ''comment''.' +tie-fighter: '|\-*-/|' diff --git a/libs/dyn/yamlloader/testdata/spec_example_2.18.yml b/libs/dyn/yamlloader/testdata/spec_example_2.18.yml new file mode 100644 index 00000000..741bcd8c --- /dev/null +++ b/libs/dyn/yamlloader/testdata/spec_example_2.18.yml @@ -0,0 +1,8 @@ +# Example 2.18 Multi-line Flow Scalars + +plain: + This unquoted scalar + spans many lines. + +quoted: "So does this + quoted scalar.\n" diff --git a/libs/dyn/yamlloader/testdata/spec_example_2.19.yml b/libs/dyn/yamlloader/testdata/spec_example_2.19.yml new file mode 100644 index 00000000..6ed95e09 --- /dev/null +++ b/libs/dyn/yamlloader/testdata/spec_example_2.19.yml @@ -0,0 +1,15 @@ +# Example 2.19 Integers + +canonical: 12345 +decimal: +12345 +octal: 0o14 +hexadecimal: 0xC + +# Note: this example is not part of the spec but added for completeness. +# +# Octal numbers: +# - YAML 1.1: prefix is "0" +# - YAML 1.2: prefix is "0o" +# The "gopkg.in/yaml.v3" package accepts both for backwards compat. +# We accept only the YAML 1.2 prefix "0o". +octal11: 012345 diff --git a/libs/dyn/yamlloader/testdata/spec_example_2.2.yml b/libs/dyn/yamlloader/testdata/spec_example_2.2.yml new file mode 100644 index 00000000..29c16105 --- /dev/null +++ b/libs/dyn/yamlloader/testdata/spec_example_2.2.yml @@ -0,0 +1,5 @@ +# Example 2.2 Mapping Scalars to Scalars (player statistics) + +hr: 65 # Home runs +avg: 0.278 # Batting average +rbi: 147 # Runs Batted In diff --git a/libs/dyn/yamlloader/testdata/spec_example_2.20.yml b/libs/dyn/yamlloader/testdata/spec_example_2.20.yml new file mode 100644 index 00000000..77a79a0c --- /dev/null +++ b/libs/dyn/yamlloader/testdata/spec_example_2.20.yml @@ -0,0 +1,7 @@ +# Example 2.20 Floating Point + +canonical: 1.23015e+3 +exponential: 12.3015e+02 +fixed: 1230.15 +negative infinity: -.inf +not a number: .nan diff --git a/libs/dyn/yamlloader/testdata/spec_example_2.21.yml b/libs/dyn/yamlloader/testdata/spec_example_2.21.yml new file mode 100644 index 00000000..cdb423c5 --- /dev/null +++ b/libs/dyn/yamlloader/testdata/spec_example_2.21.yml @@ -0,0 +1,5 @@ +# Example 2.21 Miscellaneous + +null: +booleans: [ true, false ] +string: '012345' diff --git a/libs/dyn/yamlloader/testdata/spec_example_2.22.yml b/libs/dyn/yamlloader/testdata/spec_example_2.22.yml new file mode 100644 index 00000000..bef2addf --- /dev/null +++ b/libs/dyn/yamlloader/testdata/spec_example_2.22.yml @@ -0,0 +1,6 @@ +# Example 2.22 Timestamps + +canonical: 2001-12-15T02:59:43.1Z +iso8601: 2001-12-14t21:59:43.10-05:00 +spaced: 2001-12-14 21:59:43.10 -5 +date: 2002-12-14 diff --git a/libs/dyn/yamlloader/testdata/spec_example_2.23.yml b/libs/dyn/yamlloader/testdata/spec_example_2.23.yml new file mode 100644 index 00000000..56e9898e --- /dev/null +++ b/libs/dyn/yamlloader/testdata/spec_example_2.23.yml @@ -0,0 +1,15 @@ +# Example 2.23 Various Explicit Tags + +--- +not-date: !!str 2002-04-28 + +picture: !!binary | + R0lGODlhDAAMAIQAAP//9/X + 17unp5WZmZgAAAOfn515eXv + Pz7Y6OjuDg4J+fn5OTk6enp + 56enmleECcgggoBADs= + +application specific tag: !something | + The semantics of the tag + above may be different for + different documents. diff --git a/libs/dyn/yamlloader/testdata/spec_example_2.24.yml b/libs/dyn/yamlloader/testdata/spec_example_2.24.yml new file mode 100644 index 00000000..f7c11d0f --- /dev/null +++ b/libs/dyn/yamlloader/testdata/spec_example_2.24.yml @@ -0,0 +1,16 @@ +# Example 2.24 Global Tags + +%TAG ! tag:clarkevans.com,2002: +--- !shape + # Use the ! handle for presenting + # tag:clarkevans.com,2002:circle +- !circle + center: &ORIGIN {x: 73, y: 129} + radius: 7 +- !line + start: *ORIGIN + finish: { x: 89, y: 102 } +- !label + start: *ORIGIN + color: 0xFFEEBB + text: Pretty vector drawing. diff --git a/libs/dyn/yamlloader/testdata/spec_example_2.25.yml b/libs/dyn/yamlloader/testdata/spec_example_2.25.yml new file mode 100644 index 00000000..73bac862 --- /dev/null +++ b/libs/dyn/yamlloader/testdata/spec_example_2.25.yml @@ -0,0 +1,9 @@ +# Example 2.25 Unordered Sets + +# Sets are represented as a +# Mapping where each key is +# associated with a null value +--- !!set +? Mark McGwire +? Sammy Sosa +? Ken Griffey diff --git a/libs/dyn/yamlloader/testdata/spec_example_2.26.yml b/libs/dyn/yamlloader/testdata/spec_example_2.26.yml new file mode 100644 index 00000000..00863a6b --- /dev/null +++ b/libs/dyn/yamlloader/testdata/spec_example_2.26.yml @@ -0,0 +1,9 @@ +# Example 2.26 Ordered Mappings + +# Ordered maps are represented as +# A sequence of mappings, with +# each mapping having one key +--- !!omap +- Mark McGwire: 65 +- Sammy Sosa: 63 +- Ken Griffey: 58 diff --git a/libs/dyn/yamlloader/testdata/spec_example_2.27.yml b/libs/dyn/yamlloader/testdata/spec_example_2.27.yml new file mode 100644 index 00000000..fc9b460c --- /dev/null +++ b/libs/dyn/yamlloader/testdata/spec_example_2.27.yml @@ -0,0 +1,31 @@ +# Example 2.27 Invoice + +--- ! +invoice: 34843 +date : 2001-01-23 +bill-to: &id001 + given : Chris + family : Dumars + address: + lines: | + 458 Walkman Dr. + Suite #292 + city : Royal Oak + state : MI + postal : 48046 +ship-to: *id001 +product: +- sku : BL394D + quantity : 4 + description : Basketball + price : 450.00 +- sku : BL4438H + quantity : 1 + description : Super Hoop + price : 2392.00 +tax : 251.42 +total: 4443.52 +comments: + Late afternoon is best. + Backup contact is Nancy + Billsmer @ 338-4338. diff --git a/libs/dyn/yamlloader/testdata/spec_example_2.28.yml b/libs/dyn/yamlloader/testdata/spec_example_2.28.yml new file mode 100644 index 00000000..35369472 --- /dev/null +++ b/libs/dyn/yamlloader/testdata/spec_example_2.28.yml @@ -0,0 +1,28 @@ +# Example 2.28 Log File + +--- +Time: 2001-11-23 15:01:42 -5 +User: ed +Warning: + This is an error message + for the log file +--- +Time: 2001-11-23 15:02:31 -5 +User: ed +Warning: + A slightly different error + message. +--- +Date: 2001-11-23 15:03:17 -5 +User: ed +Fatal: + Unknown variable "bar" +Stack: +- file: TopClass.py + line: 23 + code: | + x = MoreObject("345\n") +- file: MoreClass.py + line: 58 + code: |- + foo = bar diff --git a/libs/dyn/yamlloader/testdata/spec_example_2.3.yml b/libs/dyn/yamlloader/testdata/spec_example_2.3.yml new file mode 100644 index 00000000..70cbe07d --- /dev/null +++ b/libs/dyn/yamlloader/testdata/spec_example_2.3.yml @@ -0,0 +1,10 @@ +# Example 2.3 Mapping Scalars to Sequences (ball clubs in each league) + +american: +- Boston Red Sox +- Detroit Tigers +- New York Yankees +national: +- New York Mets +- Chicago Cubs +- Atlanta Braves diff --git a/libs/dyn/yamlloader/testdata/spec_example_2.4.yml b/libs/dyn/yamlloader/testdata/spec_example_2.4.yml new file mode 100644 index 00000000..cce28625 --- /dev/null +++ b/libs/dyn/yamlloader/testdata/spec_example_2.4.yml @@ -0,0 +1,10 @@ +# Example 2.4 Sequence of Mappings (players’ statistics) + +- + name: Mark McGwire + hr: 65 + avg: 0.278 +- + name: Sammy Sosa + hr: 63 + avg: 0.288 diff --git a/libs/dyn/yamlloader/testdata/spec_example_2.5.yml b/libs/dyn/yamlloader/testdata/spec_example_2.5.yml new file mode 100644 index 00000000..a585faee --- /dev/null +++ b/libs/dyn/yamlloader/testdata/spec_example_2.5.yml @@ -0,0 +1,5 @@ +# Example 2.5 Sequence of Sequences + +- [name , hr, avg ] +- [Mark McGwire, 65, 0.278] +- [Sammy Sosa , 63, 0.288] diff --git a/libs/dyn/yamlloader/testdata/spec_example_2.6.yml b/libs/dyn/yamlloader/testdata/spec_example_2.6.yml new file mode 100644 index 00000000..cc137e5d --- /dev/null +++ b/libs/dyn/yamlloader/testdata/spec_example_2.6.yml @@ -0,0 +1,7 @@ +# Example 2.6 Mapping of Mappings + +Mark McGwire: {hr: 65, avg: 0.278} +Sammy Sosa: { + hr: 63, + avg: 0.288, + } diff --git a/libs/dyn/yamlloader/testdata/spec_example_2.7.yml b/libs/dyn/yamlloader/testdata/spec_example_2.7.yml new file mode 100644 index 00000000..35c2541d --- /dev/null +++ b/libs/dyn/yamlloader/testdata/spec_example_2.7.yml @@ -0,0 +1,12 @@ +# Example 2.7 Two Documents in a Stream (each with a leading comment) + +# Ranking of 1998 home runs +--- +- Mark McGwire +- Sammy Sosa +- Ken Griffey + +# Team ranking +--- +- Chicago Cubs +- St Louis Cardinals diff --git a/libs/dyn/yamlloader/testdata/spec_example_2.8.yml b/libs/dyn/yamlloader/testdata/spec_example_2.8.yml new file mode 100644 index 00000000..ae6e8bf2 --- /dev/null +++ b/libs/dyn/yamlloader/testdata/spec_example_2.8.yml @@ -0,0 +1,12 @@ +# Example 2.8 Play by Play Feed from a Game + +--- +time: 20:03:20 +player: Sammy Sosa +action: strike (miss) +... +--- +time: 20:03:47 +player: Sammy Sosa +action: grand slam +... diff --git a/libs/dyn/yamlloader/testdata/spec_example_2.9.yml b/libs/dyn/yamlloader/testdata/spec_example_2.9.yml new file mode 100644 index 00000000..75217b25 --- /dev/null +++ b/libs/dyn/yamlloader/testdata/spec_example_2.9.yml @@ -0,0 +1,10 @@ +# Example 2.9 Single Document with Two Comments + +--- +hr: # 1998 hr ranking +- Mark McGwire +- Sammy Sosa +# 1998 rbi ranking +rbi: +- Sammy Sosa +- Ken Griffey diff --git a/libs/dyn/yamlloader/yaml_spec_test.go b/libs/dyn/yamlloader/yaml_spec_test.go new file mode 100644 index 00000000..2a5ae817 --- /dev/null +++ b/libs/dyn/yamlloader/yaml_spec_test.go @@ -0,0 +1,821 @@ +package yamlloader_test + +import ( + "bytes" + "math" + "os" + "testing" + + "github.com/databricks/cli/libs/dyn" + assert "github.com/databricks/cli/libs/dyn/dynassert" + "github.com/databricks/cli/libs/dyn/yamlloader" + "github.com/stretchr/testify/require" +) + +const NL = "\n" + +func loadExample(t *testing.T, file string) dyn.Value { + input, err := os.ReadFile(file) + require.NoError(t, err) + self, err := yamlloader.LoadYAML(file, bytes.NewBuffer(input)) + require.NoError(t, err) + return self +} + +func TestYAMLSpecExample_2_1(t *testing.T) { + file := "testdata/spec_example_2.1.yml" + self := loadExample(t, file) + + assert.Equal(t, dyn.NewValue( + []dyn.Value{ + dyn.NewValue("Mark McGwire", []dyn.Location{{File: file, Line: 3, Column: 3}}), + dyn.NewValue("Sammy Sosa", []dyn.Location{{File: file, Line: 4, Column: 3}}), + dyn.NewValue("Ken Griffey", []dyn.Location{{File: file, Line: 5, Column: 3}}), + }, + []dyn.Location{{File: file, Line: 3, Column: 1}}, + ), self) +} + +func TestYAMLSpecExample_2_2(t *testing.T) { + file := "testdata/spec_example_2.2.yml" + self := loadExample(t, file) + + assert.Equal(t, dyn.NewValue( + map[string]dyn.Value{ + "hr": dyn.NewValue(65, []dyn.Location{{File: file, Line: 3, Column: 6}}), + "avg": dyn.NewValue(0.278, []dyn.Location{{File: file, Line: 4, Column: 6}}), + "rbi": dyn.NewValue(147, []dyn.Location{{File: file, Line: 5, Column: 6}}), + }, + []dyn.Location{{File: file, Line: 3, Column: 1}}, + ), self) +} + +func TestYAMLSpecExample_2_3(t *testing.T) { + file := "testdata/spec_example_2.3.yml" + self := loadExample(t, file) + + assert.Equal(t, dyn.NewValue( + map[string]dyn.Value{ + "american": dyn.NewValue( + []dyn.Value{ + dyn.NewValue("Boston Red Sox", []dyn.Location{{File: file, Line: 4, Column: 3}}), + dyn.NewValue("Detroit Tigers", []dyn.Location{{File: file, Line: 5, Column: 3}}), + dyn.NewValue("New York Yankees", []dyn.Location{{File: file, Line: 6, Column: 3}}), + }, + []dyn.Location{{File: file, Line: 4, Column: 1}}, + ), + "national": dyn.NewValue( + []dyn.Value{ + dyn.NewValue("New York Mets", []dyn.Location{{File: file, Line: 8, Column: 3}}), + dyn.NewValue("Chicago Cubs", []dyn.Location{{File: file, Line: 9, Column: 3}}), + dyn.NewValue("Atlanta Braves", []dyn.Location{{File: file, Line: 10, Column: 3}}), + }, + []dyn.Location{{File: file, Line: 8, Column: 1}}, + ), + }, + []dyn.Location{{File: file, Line: 3, Column: 1}}, + ), self) +} + +func TestYAMLSpecExample_2_4(t *testing.T) { + file := "testdata/spec_example_2.4.yml" + self := loadExample(t, file) + + assert.Equal(t, dyn.NewValue( + []dyn.Value{ + dyn.NewValue( + map[string]dyn.Value{ + "name": dyn.NewValue("Mark McGwire", []dyn.Location{{File: file, Line: 4, Column: 9}}), + "hr": dyn.NewValue(65, []dyn.Location{{File: file, Line: 5, Column: 9}}), + "avg": dyn.NewValue(0.278, []dyn.Location{{File: file, Line: 6, Column: 9}}), + }, + []dyn.Location{{File: file, Line: 4, Column: 3}}, + ), + dyn.NewValue( + map[string]dyn.Value{ + "name": dyn.NewValue("Sammy Sosa", []dyn.Location{{File: file, Line: 8, Column: 9}}), + "hr": dyn.NewValue(63, []dyn.Location{{File: file, Line: 9, Column: 9}}), + "avg": dyn.NewValue(0.288, []dyn.Location{{File: file, Line: 10, Column: 9}}), + }, + []dyn.Location{{File: file, Line: 8, Column: 3}}, + ), + }, + []dyn.Location{{File: file, Line: 3, Column: 1}}, + ), self) +} + +func TestYAMLSpecExample_2_5(t *testing.T) { + file := "testdata/spec_example_2.5.yml" + self := loadExample(t, file) + + assert.Equal(t, dyn.NewValue( + []dyn.Value{ + dyn.NewValue( + []dyn.Value{ + dyn.NewValue("name", []dyn.Location{{File: file, Line: 3, Column: 4}}), + dyn.NewValue("hr", []dyn.Location{{File: file, Line: 3, Column: 18}}), + dyn.NewValue("avg", []dyn.Location{{File: file, Line: 3, Column: 22}}), + }, + []dyn.Location{{File: file, Line: 3, Column: 3}}, + ), + dyn.NewValue( + []dyn.Value{ + dyn.NewValue("Mark McGwire", []dyn.Location{{File: file, Line: 4, Column: 4}}), + dyn.NewValue(65, []dyn.Location{{File: file, Line: 4, Column: 18}}), + dyn.NewValue(0.278, []dyn.Location{{File: file, Line: 4, Column: 22}}), + }, + []dyn.Location{{File: file, Line: 4, Column: 3}}, + ), + dyn.NewValue( + []dyn.Value{ + dyn.NewValue("Sammy Sosa", []dyn.Location{{File: file, Line: 5, Column: 4}}), + dyn.NewValue(63, []dyn.Location{{File: file, Line: 5, Column: 18}}), + dyn.NewValue(0.288, []dyn.Location{{File: file, Line: 5, Column: 22}}), + }, + []dyn.Location{{File: file, Line: 5, Column: 3}}, + ), + }, + []dyn.Location{{File: file, Line: 3, Column: 1}}, + ), self) +} + +func TestYAMLSpecExample_2_6(t *testing.T) { + file := "testdata/spec_example_2.6.yml" + self := loadExample(t, file) + + assert.Equal(t, dyn.NewValue( + map[string]dyn.Value{ + "Mark McGwire": dyn.NewValue( + map[string]dyn.Value{ + "hr": dyn.NewValue(65, []dyn.Location{{File: file, Line: 3, Column: 20}}), + "avg": dyn.NewValue(0.278, []dyn.Location{{File: file, Line: 3, Column: 29}}), + }, + []dyn.Location{{File: file, Line: 3, Column: 15}}, + ), + "Sammy Sosa": dyn.NewValue( + map[string]dyn.Value{ + "hr": dyn.NewValue(63, []dyn.Location{{File: file, Line: 5, Column: 9}}), + "avg": dyn.NewValue(0.288, []dyn.Location{{File: file, Line: 6, Column: 10}}), + }, + []dyn.Location{{File: file, Line: 4, Column: 13}}, + ), + }, + []dyn.Location{{File: file, Line: 3, Column: 1}}, + ), self) +} + +func TestYAMLSpecExample_2_7(t *testing.T) { + file := "testdata/spec_example_2.7.yml" + self := loadExample(t, file) + + // Note: we do not support multiple documents in a single YAML file. + + assert.Equal(t, dyn.NewValue( + []dyn.Value{ + dyn.NewValue( + "Mark McGwire", + []dyn.Location{{File: file, Line: 5, Column: 3}}, + ), + dyn.NewValue( + "Sammy Sosa", + []dyn.Location{{File: file, Line: 6, Column: 3}}, + ), + dyn.NewValue( + "Ken Griffey", + []dyn.Location{{File: file, Line: 7, Column: 3}}, + ), + }, + []dyn.Location{{File: file, Line: 5, Column: 1}}, + ), self) +} + +func TestYAMLSpecExample_2_8(t *testing.T) { + file := "testdata/spec_example_2.8.yml" + self := loadExample(t, file) + + // Note: we do not support multiple documents in a single YAML file. + + assert.Equal(t, dyn.NewValue( + map[string]dyn.Value{ + "time": dyn.NewValue("20:03:20", []dyn.Location{{File: file, Line: 4, Column: 7}}), + "player": dyn.NewValue("Sammy Sosa", []dyn.Location{{File: file, Line: 5, Column: 9}}), + "action": dyn.NewValue("strike (miss)", []dyn.Location{{File: file, Line: 6, Column: 9}}), + }, + []dyn.Location{{File: file, Line: 4, Column: 1}}, + ), self) +} + +func TestYAMLSpecExample_2_9(t *testing.T) { + file := "testdata/spec_example_2.9.yml" + self := loadExample(t, file) + + // Note: we do not support multiple documents in a single YAML file. + + assert.Equal(t, dyn.NewValue( + map[string]dyn.Value{ + "hr": dyn.NewValue( + []dyn.Value{ + dyn.NewValue("Mark McGwire", []dyn.Location{{File: file, Line: 5, Column: 3}}), + dyn.NewValue("Sammy Sosa", []dyn.Location{{File: file, Line: 6, Column: 3}}), + }, + []dyn.Location{{File: file, Line: 5, Column: 1}}, + ), + "rbi": dyn.NewValue( + []dyn.Value{ + dyn.NewValue("Sammy Sosa", []dyn.Location{{File: file, Line: 9, Column: 3}}), + dyn.NewValue("Ken Griffey", []dyn.Location{{File: file, Line: 10, Column: 3}}), + }, + []dyn.Location{{File: file, Line: 9, Column: 1}}, + ), + }, + []dyn.Location{{File: file, Line: 4, Column: 1}}, + ), self) +} + +func TestYAMLSpecExample_2_10(t *testing.T) { + file := "testdata/spec_example_2.10.yml" + self := loadExample(t, file) + + assert.Equal(t, dyn.NewValue( + map[string]dyn.Value{ + "hr": dyn.NewValue( + []dyn.Value{ + dyn.NewValue("Mark McGwire", []dyn.Location{{File: file, Line: 5, Column: 3}}), + dyn.NewValue("Sammy Sosa", []dyn.Location{{File: file, Line: 7, Column: 3}}), + }, + []dyn.Location{{File: file, Line: 5, Column: 1}}, + ), + "rbi": dyn.NewValue( + []dyn.Value{ + // The location for an anchored value refers to the anchor, not the reference. + // This is the same location as the anchor that appears in the "hr" mapping. + dyn.NewValue("Sammy Sosa", []dyn.Location{{File: file, Line: 7, Column: 3}}), + dyn.NewValue("Ken Griffey", []dyn.Location{{File: file, Line: 10, Column: 3}}), + }, + []dyn.Location{{File: file, Line: 9, Column: 1}}, + ), + }, + []dyn.Location{{File: file, Line: 4, Column: 1}}, + ), self) +} + +func TestYAMLSpecExample_2_11(t *testing.T) { + file := "testdata/spec_example_2.11.yml" + input, err := os.ReadFile(file) + require.NoError(t, err) + + // Note: non-string mapping keys are not supported by "gopkg.in/yaml.v3". + _, err = yamlloader.LoadYAML(file, bytes.NewBuffer(input)) + assert.ErrorContains(t, err, `: key is not a scalar`) +} + +func TestYAMLSpecExample_2_12(t *testing.T) { + file := "testdata/spec_example_2.12.yml" + self := loadExample(t, file) + + assert.Equal(t, dyn.NewValue( + []dyn.Value{ + dyn.NewValue( + map[string]dyn.Value{ + "item": dyn.NewValue("Super Hoop", []dyn.Location{{File: file, Line: 5, Column: 13}}), + "quantity": dyn.NewValue(1, []dyn.Location{{File: file, Line: 6, Column: 13}}), + }, + []dyn.Location{{File: file, Line: 5, Column: 3}}, + ), + dyn.NewValue( + map[string]dyn.Value{ + "item": dyn.NewValue("Basketball", []dyn.Location{{File: file, Line: 7, Column: 13}}), + "quantity": dyn.NewValue(4, []dyn.Location{{File: file, Line: 8, Column: 13}}), + }, + []dyn.Location{{File: file, Line: 7, Column: 3}}, + ), + dyn.NewValue( + map[string]dyn.Value{ + "item": dyn.NewValue("Big Shoes", []dyn.Location{{File: file, Line: 9, Column: 13}}), + "quantity": dyn.NewValue(1, []dyn.Location{{File: file, Line: 10, Column: 13}}), + }, + []dyn.Location{{File: file, Line: 9, Column: 3}}, + ), + }, + []dyn.Location{{File: file, Line: 5, Column: 1}}, + ), self) +} + +func TestYAMLSpecExample_2_13(t *testing.T) { + file := "testdata/spec_example_2.13.yml" + self := loadExample(t, file) + + assert.Equal(t, dyn.NewValue( + ``+ + `\//||\/||`+NL+ + "// || ||__"+NL, + []dyn.Location{{File: file, Line: 4, Column: 5}}, + ), self) +} + +func TestYAMLSpecExample_2_14(t *testing.T) { + file := "testdata/spec_example_2.14.yml" + self := loadExample(t, file) + + assert.Equal(t, dyn.NewValue( + `Mark McGwire's year was crippled by a knee injury.`+NL, + []dyn.Location{{File: file, Line: 3, Column: 5}}, + ), self) +} + +func TestYAMLSpecExample_2_15(t *testing.T) { + file := "testdata/spec_example_2.15.yml" + self := loadExample(t, file) + + assert.Equal(t, dyn.NewValue( + ``+ + `Sammy Sosa completed another fine season with great stats.`+NL+ + NL+ + ` 63 Home Runs`+NL+ + ` 0.288 Batting Average`+NL+ + NL+ + `What a year!`+NL, + []dyn.Location{{File: file, Line: 3, Column: 5}}, + ), self) +} + +func TestYAMLSpecExample_2_16(t *testing.T) { + file := "testdata/spec_example_2.16.yml" + self := loadExample(t, file) + + assert.Equal(t, dyn.NewValue( + map[string]dyn.Value{ + "name": dyn.NewValue( + "Mark McGwire", + []dyn.Location{{File: file, Line: 3, Column: 7}}, + ), + "accomplishment": dyn.NewValue( + `Mark set a major league home run record in 1998.`+NL, + []dyn.Location{{File: file, Line: 4, Column: 17}}, + ), + "stats": dyn.NewValue( + ``+ + `65 Home Runs`+NL+ + `0.278 Batting Average`+NL, + []dyn.Location{{File: file, Line: 7, Column: 8}}, + ), + }, + []dyn.Location{{File: file, Line: 3, Column: 1}}, + ), self) +} + +func TestYAMLSpecExample_2_17(t *testing.T) { + file := "testdata/spec_example_2.17.yml" + self := loadExample(t, file) + + assert.Equal(t, dyn.NewValue( + map[string]dyn.Value{ + "unicode": dyn.NewValue( + `Sosa did fine.`+"\u263A", + []dyn.Location{{File: file, Line: 3, Column: 10}}, + ), + "control": dyn.NewValue( + "\b1998\t1999\t2000\n", + []dyn.Location{{File: file, Line: 4, Column: 10}}, + ), + "hex esc": dyn.NewValue( + "\x0d\x0a is \r\n", + []dyn.Location{{File: file, Line: 5, Column: 10}}, + ), + "single": dyn.NewValue( + `"Howdy!" he cried.`, + []dyn.Location{{File: file, Line: 7, Column: 9}}, + ), + "quoted": dyn.NewValue( + ` # Not a 'comment'.`, + []dyn.Location{{File: file, Line: 8, Column: 9}}, + ), + "tie-fighter": dyn.NewValue( + `|\-*-/|`, + []dyn.Location{{File: file, Line: 9, Column: 14}}, + ), + }, + []dyn.Location{{File: file, Line: 3, Column: 1}}, + ), self) +} + +func TestYAMLSpecExample_2_18(t *testing.T) { + file := "testdata/spec_example_2.18.yml" + self := loadExample(t, file) + + assert.Equal(t, dyn.NewValue( + map[string]dyn.Value{ + "plain": dyn.NewValue( + `This unquoted scalar spans many lines.`, + []dyn.Location{{File: file, Line: 4, Column: 3}}, + ), + "quoted": dyn.NewValue( + `So does this quoted scalar.`+NL, + []dyn.Location{{File: file, Line: 7, Column: 9}}, + ), + }, + []dyn.Location{{File: file, Line: 3, Column: 1}}, + ), self) +} + +func TestYAMLSpecExample_2_19(t *testing.T) { + file := "testdata/spec_example_2.19.yml" + self := loadExample(t, file) + + assert.Equal(t, dyn.NewValue( + map[string]dyn.Value{ + "canonical": dyn.NewValue( + 12345, + []dyn.Location{{File: file, Line: 3, Column: 12}}, + ), + "decimal": dyn.NewValue( + 12345, + []dyn.Location{{File: file, Line: 4, Column: 10}}, + ), + "octal": dyn.NewValue( + 12, + []dyn.Location{{File: file, Line: 5, Column: 8}}, + ), + "hexadecimal": dyn.NewValue( + 12, + []dyn.Location{{File: file, Line: 6, Column: 14}}, + ), + "octal11": dyn.NewValue( + 12345, + []dyn.Location{{File: file, Line: 15, Column: 10}}, + ), + }, + []dyn.Location{{File: file, Line: 3, Column: 1}}, + ), self) +} + +func TestYAMLSpecExample_2_20(t *testing.T) { + file := "testdata/spec_example_2.20.yml" + self := loadExample(t, file) + + // Equality assertion doesn't work with NaNs. + // See https://github.com/stretchr/testify/issues/624. + // + // Remove the NaN entry. + self, _ = dyn.Walk(self, func(p dyn.Path, v dyn.Value) (dyn.Value, error) { + if f, ok := v.AsFloat(); ok && math.IsNaN(f) { + return dyn.InvalidValue, dyn.ErrDrop + } + return v, nil + }) + + assert.Equal(t, dyn.NewValue( + map[string]dyn.Value{ + "canonical": dyn.NewValue( + 1230.15, + []dyn.Location{{File: file, Line: 3, Column: 12}}, + ), + "exponential": dyn.NewValue( + 1230.15, + []dyn.Location{{File: file, Line: 4, Column: 14}}, + ), + "fixed": dyn.NewValue( + 1230.15, + []dyn.Location{{File: file, Line: 5, Column: 8}}, + ), + "negative infinity": dyn.NewValue( + math.Inf(-1), + []dyn.Location{{File: file, Line: 6, Column: 20}}, + ), + }, + []dyn.Location{{File: file, Line: 3, Column: 1}}, + ), self) +} + +func TestYAMLSpecExample_2_21(t *testing.T) { + file := "testdata/spec_example_2.21.yml" + self := loadExample(t, file) + + assert.Equal(t, dyn.NewValue( + map[string]dyn.Value{ + "null": dyn.NewValue( + nil, + []dyn.Location{{File: file, Line: 3, Column: 6}}, + ), + "booleans": dyn.NewValue( + []dyn.Value{ + dyn.NewValue(true, []dyn.Location{{File: file, Line: 4, Column: 13}}), + dyn.NewValue(false, []dyn.Location{{File: file, Line: 4, Column: 19}}), + }, + []dyn.Location{{File: file, Line: 4, Column: 11}}, + ), + "string": dyn.NewValue( + "012345", + []dyn.Location{{File: file, Line: 5, Column: 9}}, + ), + }, + []dyn.Location{{File: file, Line: 3, Column: 1}}, + ), self) +} + +func TestYAMLSpecExample_2_22(t *testing.T) { + file := "testdata/spec_example_2.22.yml" + self := loadExample(t, file) + + assert.Equal(t, dyn.NewValue( + map[string]dyn.Value{ + "canonical": dyn.NewValue( + dyn.MustTime("2001-12-15T02:59:43.1Z"), + []dyn.Location{{File: file, Line: 3, Column: 12}}, + ), + "iso8601": dyn.NewValue( + dyn.MustTime("2001-12-14t21:59:43.10-05:00"), + []dyn.Location{{File: file, Line: 4, Column: 10}}, + ), + "spaced": dyn.NewValue( + // This is parsed as a string, not a timestamp, + // both by "gopkg.in/yaml.v3" and by our implementation. + "2001-12-14 21:59:43.10 -5", + []dyn.Location{{File: file, Line: 5, Column: 9}}, + ), + "date": dyn.NewValue( + dyn.MustTime("2002-12-14"), + []dyn.Location{{File: file, Line: 6, Column: 7}}, + ), + }, + []dyn.Location{{File: file, Line: 3, Column: 1}}, + ), self) +} + +func TestYAMLSpecExample_2_23(t *testing.T) { + file := "testdata/spec_example_2.23.yml" + input, err := os.ReadFile(file) + require.NoError(t, err) + + // Note: the !!binary tag is not supported by us. + + _, err = yamlloader.LoadYAML(file, bytes.NewBuffer(input)) + assert.ErrorContains(t, err, `: unknown tag: !!binary`) +} + +func TestYAMLSpecExample_2_24(t *testing.T) { + file := "testdata/spec_example_2.24.yml" + self := loadExample(t, file) + + assert.Equal(t, dyn.NewValue( + []dyn.Value{ + dyn.NewValue( + map[string]dyn.Value{ + "center": dyn.NewValue( + map[string]dyn.Value{ + "x": dyn.NewValue(73, []dyn.Location{{File: file, Line: 8, Column: 23}}), + "y": dyn.NewValue(129, []dyn.Location{{File: file, Line: 8, Column: 30}}), + }, + []dyn.Location{{File: file, Line: 8, Column: 11}}, + ), + "radius": dyn.NewValue(7, []dyn.Location{{File: file, Line: 9, Column: 11}}), + }, + []dyn.Location{{File: file, Line: 7, Column: 3}}, + ), + dyn.NewValue( + map[string]dyn.Value{ + "start": dyn.NewValue( + map[string]dyn.Value{ + "x": dyn.NewValue(73, []dyn.Location{{File: file, Line: 8, Column: 23}}), + "y": dyn.NewValue(129, []dyn.Location{{File: file, Line: 8, Column: 30}}), + }, + []dyn.Location{{File: file, Line: 8, Column: 11}}, + ), + "finish": dyn.NewValue( + map[string]dyn.Value{ + "x": dyn.NewValue(89, []dyn.Location{{File: file, Line: 12, Column: 16}}), + "y": dyn.NewValue(102, []dyn.Location{{File: file, Line: 12, Column: 23}}), + }, + []dyn.Location{{File: file, Line: 12, Column: 11}}, + ), + }, + []dyn.Location{{File: file, Line: 10, Column: 3}}, + ), + dyn.NewValue( + map[string]dyn.Value{ + "start": dyn.NewValue( + map[string]dyn.Value{ + "x": dyn.NewValue(73, []dyn.Location{{File: file, Line: 8, Column: 23}}), + "y": dyn.NewValue(129, []dyn.Location{{File: file, Line: 8, Column: 30}}), + }, + []dyn.Location{{File: file, Line: 8, Column: 11}}, + ), + "color": dyn.NewValue(16772795, []dyn.Location{{File: file, Line: 15, Column: 10}}), + "text": dyn.NewValue("Pretty vector drawing.", []dyn.Location{{File: file, Line: 16, Column: 9}}), + }, + []dyn.Location{{File: file, Line: 13, Column: 3}}, + ), + }, + []dyn.Location{{File: file, Line: 4, Column: 5}}, + ), self) +} + +func TestYAMLSpecExample_2_25(t *testing.T) { + file := "testdata/spec_example_2.25.yml" + self := loadExample(t, file) + + assert.Equal(t, dyn.NewValue( + map[string]dyn.Value{ + "Mark McGwire": dyn.NewValue(nil, []dyn.Location{{File: file, Line: 8, Column: 1}}), + "Sammy Sosa": dyn.NewValue(nil, []dyn.Location{{File: file, Line: 9, Column: 1}}), + "Ken Griffey": dyn.NewValue(nil, []dyn.Location{{File: file, Line: 10, Column: 1}}), + }, + []dyn.Location{{File: file, Line: 6, Column: 5}}, + ), self) +} + +func TestYAMLSpecExample_2_26(t *testing.T) { + file := "testdata/spec_example_2.26.yml" + self := loadExample(t, file) + + assert.Equal(t, dyn.NewValue( + []dyn.Value{ + dyn.NewValue( + map[string]dyn.Value{ + "Mark McGwire": dyn.NewValue(65, []dyn.Location{{File: file, Line: 7, Column: 17}}), + }, + []dyn.Location{{File: file, Line: 7, Column: 3}}, + ), + dyn.NewValue( + map[string]dyn.Value{ + "Sammy Sosa": dyn.NewValue(63, []dyn.Location{{File: file, Line: 8, Column: 15}}), + }, + []dyn.Location{{File: file, Line: 8, Column: 3}}, + ), + dyn.NewValue( + map[string]dyn.Value{ + "Ken Griffey": dyn.NewValue(58, []dyn.Location{{File: file, Line: 9, Column: 16}}), + }, + []dyn.Location{{File: file, Line: 9, Column: 3}}, + ), + }, + []dyn.Location{{File: file, Line: 6, Column: 5}}, + ), self) +} + +func TestYAMLSpecExample_2_27(t *testing.T) { + file := "testdata/spec_example_2.27.yml" + self := loadExample(t, file) + + assert.Equal(t, dyn.NewValue( + map[string]dyn.Value{ + "invoice": dyn.NewValue( + 34843, + []dyn.Location{{File: file, Line: 4, Column: 10}}, + ), + "date": dyn.NewValue( + dyn.MustTime("2001-01-23"), + []dyn.Location{{File: file, Line: 5, Column: 10}}, + ), + "bill-to": dyn.NewValue( + map[string]dyn.Value{ + "given": dyn.NewValue( + "Chris", + []dyn.Location{{File: file, Line: 7, Column: 12}}, + ), + "family": dyn.NewValue( + "Dumars", + []dyn.Location{{File: file, Line: 8, Column: 12}}, + ), + "address": dyn.NewValue( + map[string]dyn.Value{ + "lines": dyn.NewValue( + "458 Walkman Dr.\nSuite #292\n", + []dyn.Location{{File: file, Line: 10, Column: 12}}, + ), + "city": dyn.NewValue( + "Royal Oak", + []dyn.Location{{File: file, Line: 13, Column: 15}}, + ), + "state": dyn.NewValue( + "MI", + []dyn.Location{{File: file, Line: 14, Column: 15}}, + ), + "postal": dyn.NewValue( + 48046, + []dyn.Location{{File: file, Line: 15, Column: 15}}, + ), + }, + []dyn.Location{{File: file, Line: 10, Column: 5}}, + ), + }, + []dyn.Location{{File: file, Line: 6, Column: 10}}, + ), + "ship-to": dyn.NewValue( + map[string]dyn.Value{ + "given": dyn.NewValue( + "Chris", + []dyn.Location{{File: file, Line: 7, Column: 12}}, + ), + "family": dyn.NewValue( + "Dumars", + []dyn.Location{{File: file, Line: 8, Column: 12}}, + ), + "address": dyn.NewValue( + map[string]dyn.Value{ + "lines": dyn.NewValue( + "458 Walkman Dr.\nSuite #292\n", + []dyn.Location{{File: file, Line: 10, Column: 12}}, + ), + "city": dyn.NewValue( + "Royal Oak", + []dyn.Location{{File: file, Line: 13, Column: 15}}, + ), + "state": dyn.NewValue( + "MI", + []dyn.Location{{File: file, Line: 14, Column: 15}}, + ), + "postal": dyn.NewValue( + 48046, + []dyn.Location{{File: file, Line: 15, Column: 15}}, + ), + }, + []dyn.Location{{File: file, Line: 10, Column: 5}}, + ), + }, + []dyn.Location{{File: file, Line: 6, Column: 10}}, + ), + "product": dyn.NewValue( + []dyn.Value{ + dyn.NewValue( + map[string]dyn.Value{ + "sku": dyn.NewValue( + "BL394D", + []dyn.Location{{File: file, Line: 18, Column: 17}}, + ), + "quantity": dyn.NewValue( + 4, + []dyn.Location{{File: file, Line: 19, Column: 17}}, + ), + "description": dyn.NewValue( + "Basketball", + []dyn.Location{{File: file, Line: 20, Column: 17}}, + ), + "price": dyn.NewValue( + 450.0, + []dyn.Location{{File: file, Line: 21, Column: 17}}, + ), + }, + []dyn.Location{{File: file, Line: 18, Column: 3}}, + ), dyn.NewValue( + map[string]dyn.Value{ + "sku": dyn.NewValue( + "BL4438H", + []dyn.Location{{File: file, Line: 22, Column: 17}}, + ), + "quantity": dyn.NewValue( + 1, + []dyn.Location{{File: file, Line: 23, Column: 17}}, + ), + "description": dyn.NewValue( + "Super Hoop", + []dyn.Location{{File: file, Line: 24, Column: 17}}, + ), + "price": dyn.NewValue( + 2392.0, + []dyn.Location{{File: file, Line: 25, Column: 17}}, + ), + }, + []dyn.Location{{File: file, Line: 22, Column: 3}}, + )}, + []dyn.Location{{File: file, Line: 18, Column: 1}}, + ), + "tax": dyn.NewValue( + 251.42, + []dyn.Location{{File: file, Line: 26, Column: 8}}, + ), + "total": dyn.NewValue( + 4443.52, + []dyn.Location{{File: file, Line: 27, Column: 8}}, + ), + "comments": dyn.NewValue( + "Late afternoon is best. Backup contact is Nancy Billsmer @ 338-4338.", + []dyn.Location{{File: file, Line: 29, Column: 3}}, + ), + }, + []dyn.Location{{File: file, Line: 3, Column: 5}}, + ), self) +} + +func TestYAMLSpecExample_2_28(t *testing.T) { + file := "testdata/spec_example_2.28.yml" + self := loadExample(t, file) + + assert.Equal(t, dyn.NewValue( + map[string]dyn.Value{ + "Time": dyn.NewValue( + "2001-11-23 15:01:42 -5", + []dyn.Location{{File: file, Line: 4, Column: 7}}, + ), + "User": dyn.NewValue( + "ed", + []dyn.Location{{File: file, Line: 5, Column: 7}}, + ), + "Warning": dyn.NewValue( + "This is an error message for the log file", + []dyn.Location{{File: file, Line: 7, Column: 3}}, + ), + }, + []dyn.Location{{File: file, Line: 4, Column: 1}}, + ), self) +} diff --git a/libs/flags/json_flag.go b/libs/flags/json_flag.go index 0ed8be74..0324f114 100644 --- a/libs/flags/json_flag.go +++ b/libs/flags/json_flag.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "os" + "reflect" "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn/convert" @@ -63,11 +64,24 @@ func (j *JsonFlag) Unmarshal(v any) diag.Diagnostics { return diags.Extend(diag.FromErr(err)) } - // Finally unmarshal the normalized data to the output. - // It will fill in the ForceSendFields field if the struct contains it. - err = marshal.Unmarshal(data, v) - if err != nil { - return diags.Extend(diag.FromErr(err)) + kind := reflect.ValueOf(v).Kind() + if kind == reflect.Ptr { + kind = reflect.ValueOf(v).Elem().Kind() + } + + if kind == reflect.Struct { + // Finally unmarshal the normalized data to the output. + // It will fill in the ForceSendFields field if the struct contains it. + err = marshal.Unmarshal(data, v) + if err != nil { + return diags.Extend(diag.FromErr(err)) + } + } else { + // If the output is not a struct, just unmarshal the data to the output. + err = json.Unmarshal(data, v) + if err != nil { + return diags.Extend(diag.FromErr(err)) + } } return diags diff --git a/libs/flags/json_flag_test.go b/libs/flags/json_flag_test.go index 16769f4e..77530086 100644 --- a/libs/flags/json_flag_test.go +++ b/libs/flags/json_flag_test.go @@ -13,10 +13,6 @@ import ( "github.com/stretchr/testify/require" ) -type requestType struct { - Foo string `json:"foo"` -} - func TestJsonFlagEmpty(t *testing.T) { var body JsonFlag @@ -35,13 +31,13 @@ func TestJsonFlagInline(t *testing.T) { err := body.Set(`{"foo": "bar"}`) assert.NoError(t, err) - var request requestType + var request any diags := body.Unmarshal(&request) assert.NoError(t, diags.Error()) assert.Empty(t, diags) assert.Equal(t, "JSON (14 bytes)", body.String()) - assert.Equal(t, requestType{"bar"}, request) + assert.Equal(t, map[string]any{"foo": "bar"}, request) } func TestJsonFlagError(t *testing.T) { @@ -50,7 +46,7 @@ func TestJsonFlagError(t *testing.T) { err := body.Set(`{"foo":`) assert.NoError(t, err) - var request requestType + var request any diags := body.Unmarshal(&request) assert.EqualError(t, diags.Error(), "error decoding JSON at (inline):1:8: unexpected end of JSON input") assert.Equal(t, "JSON (7 bytes)", body.String()) @@ -58,7 +54,7 @@ func TestJsonFlagError(t *testing.T) { func TestJsonFlagFile(t *testing.T) { var body JsonFlag - var request requestType + var request any var fpath string var payload = []byte(`{"foo": "bar"}`) @@ -78,7 +74,7 @@ func TestJsonFlagFile(t *testing.T) { assert.NoError(t, diags.Error()) assert.Empty(t, diags) - assert.Equal(t, requestType{"bar"}, request) + assert.Equal(t, map[string]any{"foo": "bar"}, request) } const jsonData = ` diff --git a/libs/git/repository.go b/libs/git/repository.go index 6940ddac..0bbd5786 100644 --- a/libs/git/repository.go +++ b/libs/git/repository.go @@ -23,8 +23,21 @@ type Repository struct { // directory where we process .gitignore files. real bool - // root is the absolute path to the repository root. - root vfs.Path + // rootDir is the path to the root of the repository checkout. + // This can be either the main repository checkout or a worktree checkout. + // For more information about worktrees, see: https://git-scm.com/docs/git-worktree#_description. + rootDir vfs.Path + + // gitDir is the equivalent of $GIT_DIR and points to the + // `.git` directory of a repository or a worktree directory. + // See https://git-scm.com/docs/git-worktree#_details for more information. + gitDir vfs.Path + + // gitCommonDir is the equivalent of $GIT_COMMON_DIR and points to the + // `.git` directory of the main working tree (common between worktrees). + // This is equivalent to [gitDir] if this is the main working tree. + // See https://git-scm.com/docs/git-worktree#_details for more information. + gitCommonDir vfs.Path // ignore contains a list of ignore patterns indexed by the // path prefix relative to the repository root. @@ -44,12 +57,11 @@ type Repository struct { // Root returns the absolute path to the repository root. func (r *Repository) Root() string { - return r.root.Native() + return r.rootDir.Native() } func (r *Repository) CurrentBranch() (string, error) { - // load .git/HEAD - ref, err := LoadReferenceFile(r.root, path.Join(GitDirectoryName, "HEAD")) + ref, err := LoadReferenceFile(r.gitDir, "HEAD") if err != nil { return "", err } @@ -65,8 +77,7 @@ func (r *Repository) CurrentBranch() (string, error) { } func (r *Repository) LatestCommit() (string, error) { - // load .git/HEAD - ref, err := LoadReferenceFile(r.root, path.Join(GitDirectoryName, "HEAD")) + ref, err := LoadReferenceFile(r.gitDir, "HEAD") if err != nil { return "", err } @@ -80,12 +91,12 @@ func (r *Repository) LatestCommit() (string, error) { return ref.Content, nil } - // read reference from .git/HEAD + // Read reference from $GIT_DIR/HEAD branchHeadPath, err := ref.ResolvePath() if err != nil { return "", err } - branchHeadRef, err := LoadReferenceFile(r.root, path.Join(GitDirectoryName, branchHeadPath)) + branchHeadRef, err := LoadReferenceFile(r.gitCommonDir, branchHeadPath) if err != nil { return "", err } @@ -125,7 +136,7 @@ func (r *Repository) loadConfig() error { if err != nil { return fmt.Errorf("unable to load user specific gitconfig: %w", err) } - err = config.loadFile(r.root, ".git/config") + err = config.loadFile(r.gitCommonDir, "config") if err != nil { return fmt.Errorf("unable to load repository specific gitconfig: %w", err) } @@ -133,12 +144,6 @@ func (r *Repository) loadConfig() error { return nil } -// newIgnoreFile constructs a new [ignoreRules] implementation backed by -// a file using the specified path relative to the repository root. -func (r *Repository) newIgnoreFile(relativeIgnoreFilePath string) ignoreRules { - return newIgnoreFile(r.root, relativeIgnoreFilePath) -} - // getIgnoreRules returns a slice of [ignoreRules] that apply // for the specified prefix. The prefix must be cleaned by the caller. // It lazily initializes an entry for the specified prefix if it @@ -149,7 +154,7 @@ func (r *Repository) getIgnoreRules(prefix string) []ignoreRules { return fs } - r.ignore[prefix] = append(r.ignore[prefix], r.newIgnoreFile(path.Join(prefix, gitIgnoreFileName))) + r.ignore[prefix] = append(r.ignore[prefix], newIgnoreFile(r.rootDir, path.Join(prefix, gitIgnoreFileName))) return r.ignore[prefix] } @@ -205,21 +210,30 @@ func (r *Repository) Ignore(relPath string) (bool, error) { func NewRepository(path vfs.Path) (*Repository, error) { real := true - rootPath, err := vfs.FindLeafInTree(path, GitDirectoryName) + rootDir, err := vfs.FindLeafInTree(path, GitDirectoryName) if err != nil { if !errors.Is(err, fs.ErrNotExist) { return nil, err } // Cannot find `.git` directory. - // Treat the specified path as a potential repository root. + // Treat the specified path as a potential repository root checkout. real = false - rootPath = path + rootDir = path + } + + // Derive $GIT_DIR and $GIT_COMMON_DIR paths if this is a real repository. + // If it isn't a real repository, they'll point to the (non-existent) `.git` directory. + gitDir, gitCommonDir, err := resolveGitDirs(rootDir) + if err != nil { + return nil, err } repo := &Repository{ - real: real, - root: rootPath, - ignore: make(map[string][]ignoreRules), + real: real, + rootDir: rootDir, + gitDir: gitDir, + gitCommonDir: gitCommonDir, + ignore: make(map[string][]ignoreRules), } err = repo.loadConfig() @@ -252,10 +266,10 @@ func NewRepository(path vfs.Path) (*Repository, error) { newStringIgnoreRules([]string{ ".git", }), - // Load repository-wide excludes file. - repo.newIgnoreFile(".git/info/excludes"), + // Load repository-wide exclude file. + newIgnoreFile(repo.gitCommonDir, "info/exclude"), // Load root gitignore file. - repo.newIgnoreFile(".gitignore"), + newIgnoreFile(repo.rootDir, ".gitignore"), } return repo, nil diff --git a/libs/git/view.go b/libs/git/view.go index 90eed0bb..2d2e39a6 100644 --- a/libs/git/view.go +++ b/libs/git/view.go @@ -80,7 +80,7 @@ func NewView(root vfs.Path) (*View, error) { // Target path must be relative to the repository root path. target := root.Native() - prefix := repo.root.Native() + prefix := repo.rootDir.Native() if !strings.HasPrefix(target, prefix) { return nil, fmt.Errorf("path %q is not within repository root %q", root.Native(), prefix) } diff --git a/libs/git/worktree.go b/libs/git/worktree.go new file mode 100644 index 00000000..964c1c95 --- /dev/null +++ b/libs/git/worktree.go @@ -0,0 +1,123 @@ +package git + +import ( + "bufio" + "errors" + "fmt" + "io/fs" + "path/filepath" + "strings" + + "github.com/databricks/cli/libs/vfs" +) + +func readLines(root vfs.Path, name string) ([]string, error) { + file, err := root.Open(name) + if err != nil { + return nil, err + } + + defer file.Close() + + var lines []string + scanner := bufio.NewScanner(file) + for scanner.Scan() { + lines = append(lines, scanner.Text()) + } + + return lines, scanner.Err() +} + +// readGitDir reads the value of the `.git` file in a worktree. +func readGitDir(root vfs.Path) (string, error) { + lines, err := readLines(root, GitDirectoryName) + if err != nil { + return "", err + } + + var gitDir string + for _, line := range lines { + parts := strings.SplitN(line, ": ", 2) + if len(parts) != 2 { + continue + } + + if parts[0] == "gitdir" { + gitDir = strings.TrimSpace(parts[1]) + } + } + + if gitDir == "" { + return "", fmt.Errorf(`expected %q to contain a line with "gitdir: [...]"`, filepath.Join(root.Native(), GitDirectoryName)) + } + + return gitDir, nil +} + +// readGitCommonDir reads the value of the `commondir` file in the `.git` directory of a worktree. +// This file typically contains "../.." to point to $GIT_COMMON_DIR. +func readGitCommonDir(gitDir vfs.Path) (string, error) { + lines, err := readLines(gitDir, "commondir") + if err != nil { + return "", err + } + + if len(lines) == 0 { + return "", errors.New("file is empty") + } + + return strings.TrimSpace(lines[0]), nil +} + +// resolveGitDirs resolves the paths for $GIT_DIR and $GIT_COMMON_DIR. +// The path argument is the root of the checkout where (supposedly) a `.git` file or directory exists. +func resolveGitDirs(root vfs.Path) (vfs.Path, vfs.Path, error) { + fileInfo, err := root.Stat(GitDirectoryName) + if err != nil { + // If the `.git` file or directory does not exist, then this is not a git repository. + // Return paths that we know don't exist, so we do not need to perform nil checks in the caller. + if errors.Is(err, fs.ErrNotExist) { + gitDir := vfs.MustNew(filepath.Join(root.Native(), GitDirectoryName)) + return gitDir, gitDir, nil + } + return nil, nil, err + } + + // If the path is a directory, then it is the main working tree. + // Both $GIT_DIR and $GIT_COMMON_DIR point to the same directory. + if fileInfo.IsDir() { + gitDir := vfs.MustNew(filepath.Join(root.Native(), GitDirectoryName)) + return gitDir, gitDir, nil + } + + // If the path is not a directory, then it is a worktree. + // Read value for $GIT_DIR. + gitDirValue, err := readGitDir(root) + if err != nil { + return nil, nil, err + } + + // Resolve $GIT_DIR. + var gitDir vfs.Path + if filepath.IsAbs(gitDirValue) { + gitDir = vfs.MustNew(gitDirValue) + } else { + gitDir = vfs.MustNew(filepath.Join(root.Native(), gitDirValue)) + } + + // Read value for $GIT_COMMON_DIR. + gitCommonDirValue, err := readGitCommonDir(gitDir) + if err != nil { + return nil, nil, fmt.Errorf(`expected "commondir" file in worktree git folder at %q: %w`, gitDir.Native(), err) + } + + // Resolve $GIT_COMMON_DIR. + var gitCommonDir vfs.Path + if filepath.IsAbs(gitCommonDirValue) { + gitCommonDir = vfs.MustNew(gitCommonDirValue) + } else { + gitCommonDir = vfs.MustNew(filepath.Join(gitDir.Native(), gitCommonDirValue)) + } + + return gitDir, gitCommonDir, nil +} diff --git a/libs/git/worktree_test.go b/libs/git/worktree_test.go new file mode 100644 index 00000000..3d620c48 --- /dev/null +++ b/libs/git/worktree_test.go @@ -0,0 +1,108 @@ +package git + +import ( + "fmt" + "os" + "path/filepath" + "testing" + + "github.com/databricks/cli/libs/vfs" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func setupWorktree(t *testing.T) string { + var err error + + tmpDir := t.TempDir() + + // Checkout path + err = os.MkdirAll(filepath.Join(tmpDir, "my_worktree"), os.ModePerm) + require.NoError(t, err) + + // Main $GIT_COMMON_DIR + err = os.MkdirAll(filepath.Join(tmpDir, ".git"), os.ModePerm) + require.NoError(t, err) + + // Worktree $GIT_DIR + err = os.MkdirAll(filepath.Join(tmpDir, ".git/worktrees/my_worktree"), os.ModePerm) + require.NoError(t, err) + + return tmpDir +} + +func writeGitDir(t *testing.T, dir, content string) { + err := os.WriteFile(filepath.Join(dir, "my_worktree/.git"), []byte(content), os.ModePerm) + require.NoError(t, err) +} + +func writeGitCommonDir(t *testing.T, dir, content string) { + err := os.WriteFile(filepath.Join(dir, ".git/worktrees/my_worktree/commondir"), []byte(content), os.ModePerm) + require.NoError(t, err) +} + +func verifyCorrectDirs(t *testing.T, dir string) { + gitDir, gitCommonDir, err := resolveGitDirs(vfs.MustNew(filepath.Join(dir, "my_worktree"))) + require.NoError(t, err) + assert.Equal(t, filepath.Join(dir, ".git/worktrees/my_worktree"), gitDir.Native()) + assert.Equal(t, filepath.Join(dir, ".git"), gitCommonDir.Native()) +} + +func TestWorktreeResolveGitDir(t *testing.T) { + dir := setupWorktree(t) + writeGitCommonDir(t, dir, "../..") + + t.Run("relative", func(t *testing.T) { + writeGitDir(t, dir, fmt.Sprintf("gitdir: %s", "../.git/worktrees/my_worktree")) + verifyCorrectDirs(t, dir) + }) + + t.Run("absolute", func(t *testing.T) { + writeGitDir(t, dir, fmt.Sprintf("gitdir: %s", filepath.Join(dir, ".git/worktrees/my_worktree"))) + verifyCorrectDirs(t, dir) + }) + + t.Run("additional spaces", func(t *testing.T) { + writeGitDir(t, dir, fmt.Sprintf("gitdir: %s \n\n\n", "../.git/worktrees/my_worktree")) + verifyCorrectDirs(t, dir) + }) + + t.Run("empty", func(t *testing.T) { + writeGitDir(t, dir, "") + + _, _, err := resolveGitDirs(vfs.MustNew(filepath.Join(dir, "my_worktree"))) + assert.ErrorContains(t, err, ` to contain a line with "gitdir: [...]"`) + }) +} + +func TestWorktreeResolveCommonDir(t *testing.T) { + dir := setupWorktree(t) + writeGitDir(t, dir, fmt.Sprintf("gitdir: %s", "../.git/worktrees/my_worktree")) + + t.Run("relative", func(t *testing.T) { + writeGitCommonDir(t, dir, "../..") + verifyCorrectDirs(t, dir) + }) + + t.Run("absolute", func(t *testing.T) { + writeGitCommonDir(t, dir, filepath.Join(dir, ".git")) + verifyCorrectDirs(t, dir) + }) + + t.Run("additional spaces", func(t *testing.T) { + writeGitCommonDir(t, dir, " ../.. \n\n\n") + verifyCorrectDirs(t, dir) + }) + + t.Run("empty", func(t *testing.T) { + writeGitCommonDir(t, dir, "") + + _, _, err := resolveGitDirs(vfs.MustNew(filepath.Join(dir, "my_worktree"))) + assert.ErrorContains(t, err, `expected "commondir" file in worktree git folder at `) + }) + + t.Run("missing", func(t *testing.T) { + _, _, err := resolveGitDirs(vfs.MustNew(filepath.Join(dir, "my_worktree"))) + assert.ErrorContains(t, err, `expected "commondir" file in worktree git folder at `) + }) +} diff --git a/libs/process/opts.go b/libs/process/opts.go index 9516e49b..dd066751 100644 --- a/libs/process/opts.go +++ b/libs/process/opts.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "os/exec" + "sync" ) type execOption func(context.Context, *exec.Cmd) error @@ -69,10 +70,24 @@ func WithStdoutWriter(dst io.Writer) execOption { } } +// safeWriter is a writer that is safe to use concurrently. +// It serializes writes to the underlying writer. +type safeWriter struct { + w io.Writer + m sync.Mutex +} + +func (s *safeWriter) Write(p []byte) (n int, err error) { + s.m.Lock() + defer s.m.Unlock() + return s.w.Write(p) +} + func WithCombinedOutput(buf *bytes.Buffer) execOption { + sw := &safeWriter{w: buf} return func(_ context.Context, c *exec.Cmd) error { - c.Stdout = io.MultiWriter(buf, c.Stdout) - c.Stderr = io.MultiWriter(buf, c.Stderr) + c.Stdout = io.MultiWriter(sw, c.Stdout) + c.Stderr = io.MultiWriter(sw, c.Stderr) return nil } } diff --git a/libs/template/templates/default-python/template/{{.project_name}}/resources/{{.project_name}}.pipeline.yml.tmpl b/libs/template/templates/default-python/template/{{.project_name}}/resources/{{.project_name}}.pipeline.yml.tmpl index 50e5ad97..1c6b8607 100644 --- a/libs/template/templates/default-python/template/{{.project_name}}/resources/{{.project_name}}.pipeline.yml.tmpl +++ b/libs/template/templates/default-python/template/{{.project_name}}/resources/{{.project_name}}.pipeline.yml.tmpl @@ -3,7 +3,7 @@ resources: pipelines: {{.project_name}}_pipeline: name: {{.project_name}}_pipeline - {{- if eq default_catalog ""}} + {{- if or (eq default_catalog "") (eq default_catalog "hive_metastore")}} ## Specify the 'catalog' field to configure this pipeline to make use of Unity Catalog: # catalog: catalog_name {{- else}}