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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* Error out when a configured `workspace_id` does not match the connected workspace, instead of silently using it in resource URLs emitted by `bundle summary`. ([#6754](https://github.com/databricks/cli/pull/6754))
2 changes: 2 additions & 0 deletions acceptance/bundle/user_agent/output.txt
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,12 @@ MISS run.terraform /.well-known/databricks-config 'cli/[CLI_VERSION] databricks-
MISS summary.direct /api/2.0/preview/scim/v2/Me 'cli/[CLI_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/[OS] cmd/bundle_summary cmd-exec-id/[UUID] interactive/none auth/pat'
MISS summary.direct /api/2.0/workspace/get-status 'cli/[CLI_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/[OS] cmd/bundle_summary cmd-exec-id/[UUID] interactive/none auth/pat'
MISS summary.direct /api/2.0/workspace/get-status 'cli/[CLI_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/[OS] cmd/bundle_summary cmd-exec-id/[UUID] interactive/none auth/pat'
OK summary.direct /api/2.0/preview/scim/v2/Me engine/direct
MISS summary.direct /.well-known/databricks-config 'cli/[CLI_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/[OS]'
MISS summary.terraform /api/2.0/preview/scim/v2/Me 'cli/[CLI_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/[OS] cmd/bundle_summary cmd-exec-id/[UUID] interactive/none auth/pat'
MISS summary.terraform /api/2.0/workspace/get-status 'cli/[CLI_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/[OS] cmd/bundle_summary cmd-exec-id/[UUID] interactive/none auth/pat'
MISS summary.terraform /api/2.0/workspace/get-status 'cli/[CLI_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/[OS] cmd/bundle_summary cmd-exec-id/[UUID] interactive/none auth/pat'
OK summary.terraform /api/2.0/preview/scim/v2/Me engine/terraform
MISS summary.terraform /.well-known/databricks-config 'cli/[CLI_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/[OS]'
MISS validate.direct /api/2.0/preview/scim/v2/Me 'cli/[CLI_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/[OS] cmd/bundle_validate cmd-exec-id/[UUID] interactive/none auth/pat'
MISS validate.direct /api/2.0/workspace/get-status 'cli/[CLI_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/[OS] cmd/bundle_validate cmd-exec-id/[UUID] interactive/none auth/pat'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,18 @@
"return_export_info": "true"
}
}
{
"headers": {
"User-Agent": [
"cli/[CLI_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/[OS] cmd/bundle_summary cmd-exec-id/[UUID] interactive/none engine/direct auth/pat"
]
},
"method": "GET",
"path": "/api/2.0/preview/scim/v2/Me",
Comment thread
andrewnester marked this conversation as resolved.
"q": {
"excludedAttributes": "entitlements"
}
}
{
"headers": {
"User-Agent": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,18 @@
"return_export_info": "true"
}
}
{
"headers": {
"User-Agent": [
"cli/[CLI_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/[OS] cmd/bundle_summary cmd-exec-id/[UUID] interactive/none engine/terraform auth/pat"
]
},
"method": "GET",
"path": "/api/2.0/preview/scim/v2/Me",
"q": {
"excludedAttributes": "entitlements"
}
}
{
"headers": {
"User-Agent": [
Expand Down
40 changes: 36 additions & 4 deletions bundle/config/mutator/initialize_urls.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package mutator
import (
"context"
"net/url"
"strconv"
"strings"

"github.com/databricks/cli/bundle"
Expand All @@ -25,13 +26,44 @@ func (m *initializeURLs) Name() string {
}

func (m *initializeURLs) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
workspaceID, err := auth.ResolveWorkspaceID(ctx, b.WorkspaceClient(ctx))
client := b.WorkspaceClient(ctx)
host := client.Config.CanonicalHostName()

// A configured, non-numeric workspace ID (a UUID connection-style
// identifier) can't be validated against CurrentWorkspaceID, which
// strictly parses the org ID header as an integer and would error on a
// UUID. Skip the API call entirely and pass it through unchanged.
cfgID := client.Config.WorkspaceID
if cfgID != "" && cfgID != auth.WorkspaceIDNone {
if _, err := strconv.ParseInt(cfgID, 10, 64); err != nil {
if err := initializeForWorkspace(b, cfgID, host); err != nil {
return diag.FromErr(err)
}
return nil
}
}

// Otherwise, always resolve the workspace ID from the API. This is the
// only way to detect a numeric workspace ID that is stale or mis-scoped
// (e.g. a leftover value from a different profile or an old pasted SPOG
// URL): if the configured value disagrees with the org ID the workspace
// actually reports, embedding it in ?w= would silently navigate to the
// wrong workspace, so we error out instead.
apiID, err := client.CurrentWorkspaceID(ctx)
if err != nil {
return diag.FromErr(err)
}
host := b.WorkspaceClient(ctx).Config.CanonicalHostName()
err = initializeForWorkspace(b, workspaceID, host)
if err != nil {
orgID := strconv.FormatInt(apiID, 10)

if cfgID != "" && cfgID != auth.WorkspaceIDNone && cfgID != orgID {
return diag.Errorf(
"workspace_id %s in your configuration does not match the connected workspace (ID: %s); "+
"remove or correct workspace_id in your profile or bundle config to disambiguate",
cfgID, orgID,
)
}

if err := initializeForWorkspace(b, orgID, host); err != nil {
return diag.FromErr(err)
}
return nil
Expand Down
117 changes: 117 additions & 0 deletions bundle/config/mutator/initialize_urls_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,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/testserver"
"github.com/databricks/databricks-sdk-go"
"github.com/databricks/databricks-sdk-go/service/catalog"
"github.com/databricks/databricks-sdk-go/service/compute"
"github.com/databricks/databricks-sdk-go/service/jobs"
Expand Down Expand Up @@ -149,3 +151,118 @@ func TestInitializeURLsWithoutOrgId(t *testing.T) {

require.Equal(t, "https://adb-123456.azuredatabricks.net/jobs/1", b.Config.Resources.Jobs["job1"].URL)
}

// TestInitializeURLsApplyNonNumericConfigPassedThrough verifies that a
// non-numeric Config.WorkspaceID (e.g. a UUID connection-style identifier) is
// passed through unchanged into the ?w= parameter. The numeric-mismatch check
// is skipped because such IDs cannot be compared against an integer org ID.
func TestInitializeURLsApplyNonNumericConfigPassedThrough(t *testing.T) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

make this and the case below an acceptance test? Would be simpler?

server := testserver.New(t)
testserver.AddDefaultHandlers(server)

w, err := databricks.NewWorkspaceClient(&databricks.Config{
Host: server.URL,
Token: "testtoken",
WorkspaceID: "some-uuid-style-id",
})
require.NoError(t, err)

b := &bundle.Bundle{
Config: config.Root{
Resources: config.Resources{
Jobs: map[string]*resources.Job{
"job1": {
ID: "1",
JobSettings: jobs.JobSettings{Name: "job1"},
},
},
},
},
}
b.SetWorkpaceClient(w)

diags := InitializeURLs().Apply(t.Context(), b)
require.NoError(t, diags.Error())

// UUID flows through into ?w= unchanged — no numeric comparison is possible.
require.Equal(t,
server.URL+"/jobs/1?w=some-uuid-style-id",
b.Config.Resources.Jobs["job1"].URL,
)
}

// TestInitializeURLsApplyNoneSentinelResolvesViaAPI verifies that the "none"
// sentinel (persisted to .databrickscfg when a user explicitly skips
// workspace selection) is treated as unset rather than as a literal value: it
// must not end up as ?w=none, and it must not be compared against the API
// result (both would be wrong; the sentinel means "no preference").
func TestInitializeURLsApplyNoneSentinelResolvesViaAPI(t *testing.T) {
server := testserver.New(t)
testserver.AddDefaultHandlers(server)
// /Me returns X-Databricks-Org-Id: 900800700600.

w, err := databricks.NewWorkspaceClient(&databricks.Config{
Host: server.URL,
Token: "testtoken",
WorkspaceID: "none",
})
require.NoError(t, err)

b := &bundle.Bundle{
Config: config.Root{
Resources: config.Resources{
Jobs: map[string]*resources.Job{
"job1": {
ID: "1",
JobSettings: jobs.JobSettings{Name: "job1"},
},
},
},
},
}
b.SetWorkpaceClient(w)

diags := InitializeURLs().Apply(t.Context(), b)
require.NoError(t, diags.Error())

require.Equal(t,
server.URL+"/jobs/1?w=900800700600",
b.Config.Resources.Jobs["job1"].URL,
)
}

// TestInitializeURLsApplyErrorsOnNumericWorkspaceIDMismatch verifies that Apply
// returns an error when Config.WorkspaceID is a numeric value that differs from
// the workspace org ID returned by the API. This prevents silently embedding a
// wrong ?w= parameter in resource URLs that would navigate to an unexpected workspace.
func TestInitializeURLsApplyErrorsOnNumericWorkspaceIDMismatch(t *testing.T) {
server := testserver.New(t)
testserver.AddDefaultHandlers(server)
// /Me returns X-Databricks-Org-Id: 900800700600.

w, err := databricks.NewWorkspaceClient(&databricks.Config{
Host: server.URL,
Token: "testtoken",
WorkspaceID: "12345", // numeric but does not match 900800700600
})
require.NoError(t, err)

b := &bundle.Bundle{
Config: config.Root{
Resources: config.Resources{
Jobs: map[string]*resources.Job{
"job1": {
ID: "1",
JobSettings: jobs.JobSettings{Name: "job1"},
},
},
},
},
}
b.SetWorkpaceClient(w)

diags := InitializeURLs().Apply(t.Context(), b)
require.ErrorContains(t, diags.Error(), "12345")
require.ErrorContains(t, diags.Error(), "900800700600")
require.ErrorContains(t, diags.Error(), "disambiguate")
}
Loading