Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 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
1 change: 1 addition & 0 deletions NEXT_CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,6 @@
* Added support for clusters in deployment bind/unbind commands ([#2536](https://github.com/databricks/cli/pull/2536))
* Added support for volumes in deployment bind/unbind commands ([#2527](https://github.com/databricks/cli/pull/2527))
* Added support for dashboards in deployment bind/unbind commands ([#2516](https://github.com/databricks/cli/pull/2516))
* Added a mismatch check when host is defined in config and as an env variable ([#2549](https://github.com/databricks/cli/pull/2549))

### API Changes
2 changes: 1 addition & 1 deletion acceptance/bundle/debug/out.stderr.txt
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@
10:07:59 Debug: Apply pid=12345 mutator=validate:unique_resource_keys
10:07:59 Debug: Apply pid=12345 mutator=SelectDefaultTarget
10:07:59 Debug: Apply pid=12345 mutator=SelectDefaultTarget mutator=SelectTarget(default)
10:07:59 Debug: Apply pid=12345 mutator=validate:interpolation_in_auth_config
10:07:59 Debug: Apply pid=12345 mutator=<func>
10:07:59 Info: Phase: initialize pid=12345
10:07:59 Debug: Apply pid=12345 mutator=validate:AllResourcesHaveValues
10:07:59 Debug: Apply pid=12345 mutator=validate:interpolation_in_auth_config
10:07:59 Debug: Apply pid=12345 mutator=RewriteSyncPaths
10:07:59 Debug: Apply pid=12345 mutator=SyncDefaultPath
10:07:59 Debug: Apply pid=12345 mutator=SyncInferRoot
Expand Down
9 changes: 2 additions & 7 deletions acceptance/bundle/variables/host/output.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,14 @@ Warning: Variable interpolation is not supported for fields that configure authe
Interpolation is not supported for the field workspace.host. Please set
the DATABRICKS_HOST environment variable if you wish to configure this field at runtime.

Error: failed during request visitor: parse "https://${var.host}": invalid character "{" in host name
Error: cannot resolve bundle auth configuration: config host mismatch: DATABRICKS_HOST is defined as [DATABRICKS_URL], but CLI configured to use ${var.host}
Copy link
Copy Markdown
Contributor Author

@ilyakuz-db ilyakuz-db Mar 22, 2025

Choose a reason for hiding this comment

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

Just in case - we have a warning displayed above that explains incorrect usage of variables in auth-related fields


{
"bundle": {
"environment": "default",
"name": "host",
"target": "default"
},
"sync": {
"paths": [
"."
]
},
"targets": null,
"variables": {
"host": {
Expand All @@ -41,7 +36,7 @@ Warning: Variable interpolation is not supported for fields that configure authe
Interpolation is not supported for the field workspace.host. Please set
the DATABRICKS_HOST environment variable if you wish to configure this field at runtime.

Error: failed during request visitor: parse "https://${var.host}": invalid character "{" in host name
Error: cannot resolve bundle auth configuration: config host mismatch: DATABRICKS_HOST is defined as [DATABRICKS_URL], but CLI configured to use ${var.host}

Name: host
Target: default
Expand Down
27 changes: 27 additions & 0 deletions bundle/config/workspace.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
package config

import (
"fmt"
"os"
"path/filepath"

"github.com/databricks/cli/libs/databrickscfg"
"github.com/databricks/cli/libs/workspace"
"github.com/databricks/databricks-sdk-go"
"github.com/databricks/databricks-sdk-go/config"
"github.com/databricks/databricks-sdk-go/marshal"
Expand Down Expand Up @@ -148,9 +150,34 @@ func (w *Workspace) Client() (*databricks.WorkspaceClient, error) {
}
}

if w.Host != "" && w.Profile == "" {
err := validateConfigAndEnvHost(cfg)
if err != nil {
return nil, err
}
}

return databricks.NewWorkspaceClient((*databricks.Config)(cfg))
}

func validateConfigAndEnvHost(cfg *config.Config) error {
var hostEnvName, hostEnvVal string
for _, attr := range config.ConfigAttributes {
if attr.Name == "host" {
hostEnvVal, hostEnvName = attr.ReadEnv()
break
}
}
if hostEnvName == "" || hostEnvVal == "" {
return nil
}

if !workspace.MatchHost(hostEnvVal, cfg.Host) {
return fmt.Errorf("config host mismatch: %s is defined as %s, but CLI configured to use %s", hostEnvName, hostEnvVal, cfg.Host)
}
return nil
}

func init() {
arg0 := os.Args[0]

Expand Down
31 changes: 31 additions & 0 deletions bundle/config/workspace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,3 +149,34 @@ func TestWorkspaceVerifyProfileForHost(t *testing.T) {
assert.ErrorContains(t, err, "config host mismatch")
})
}

func TestWorkspaceValidateConfigAndEnvHost(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.

Can you instead convert this to acceptance test?

t.Run("host from env mismatch", func(t *testing.T) {
w := Workspace{
Host: "https://abc.cloud.databricks.com",
}
setupWorkspaceTest(t)
t.Setenv("DATABRICKS_HOST", "https://def.cloud.databricks.com")
_, err := w.Client()
assert.ErrorContains(t, err, "config host mismatch: DATABRICKS_HOST is defined as https://def.cloud.databricks.com, but CLI configured to use https://abc.cloud.databricks.com")
})

t.Run("with profile defined", func(t *testing.T) {
w := Workspace{
Host: "https://abc.cloud.databricks.com",
Profile: "abc",
}

setupWorkspaceTest(t)
// This works if there is a config file with a matching profile.
err := databrickscfg.SaveToProfile(context.Background(), &config.Config{
Profile: "abc",
Host: "https://abc.cloud.databricks.com",
})
require.NoError(t, err)

t.Setenv("DATABRICKS_HOST", "https://def.cloud.databricks.com")
_, err = w.Client()
require.NoError(t, err)
})
}
1 change: 0 additions & 1 deletion bundle/phases/initialize.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ func Initialize(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {

return bundle.ApplySeq(ctx, b,
validate.AllResourcesHaveValues(),
validate.NoInterpolationInAuthConfig(),

// Update all path fields in the sync block to be relative to the bundle root path.
mutator.RewriteSyncPaths(),
Expand Down
3 changes: 3 additions & 0 deletions cmd/root/bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"

"github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/config/validate"
"github.com/databricks/cli/bundle/env"
"github.com/databricks/cli/bundle/phases"
"github.com/databricks/cli/libs/cmdctx"
Expand Down Expand Up @@ -94,6 +95,8 @@ func configureBundle(cmd *cobra.Command, b *bundle.Bundle) (*bundle.Bundle, diag
return b, diags
}

diags = diags.Extend(bundle.Apply(ctx, b, validate.NoInterpolationInAuthConfig()))
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.

You can actually convert the whole NoInterpolationInAuthConfig mutator in a usual Go function and don't use bundle.Apply here at all because this mutator does not actually mutate anything


// Set the auth configuration in the command context. This can be used
// downstream to initialize a API client.
//
Expand Down
22 changes: 0 additions & 22 deletions libs/databrickscfg/host.go

This file was deleted.

26 changes: 0 additions & 26 deletions libs/databrickscfg/host_test.go

This file was deleted.

5 changes: 3 additions & 2 deletions libs/databrickscfg/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"strings"

"github.com/databricks/cli/libs/log"
"github.com/databricks/cli/libs/workspace"
"github.com/databricks/databricks-sdk-go/config"
"gopkg.in/ini.v1"
)
Expand Down Expand Up @@ -74,7 +75,7 @@ func (l profileFromHostLoader) Configure(cfg *config.Config) error {
return fmt.Errorf("cannot parse config file: %w", err)
}
// Normalized version of the configured host.
host := normalizeHost(cfg.Host)
host := workspace.NormalizeHost(cfg.Host)
match, err := findMatchingProfile(configFile, func(s *ini.Section) bool {
key, err := s.GetKey("host")
if err != nil {
Expand All @@ -83,7 +84,7 @@ func (l profileFromHostLoader) Configure(cfg *config.Config) error {
}

// Check if this section matches the normalized host
return normalizeHost(key.Value()) == host
return workspace.NormalizeHost(key.Value()) == host
})
if err == errNoMatchingProfiles {
return nil
Expand Down
9 changes: 5 additions & 4 deletions libs/databrickscfg/ops.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"strings"

"github.com/databricks/cli/libs/log"
"github.com/databricks/cli/libs/workspace"
"github.com/databricks/databricks-sdk-go/config"
"gopkg.in/ini.v1"
)
Expand Down Expand Up @@ -67,7 +68,7 @@ func matchOrCreateSection(ctx context.Context, configFile *config.File, cfg *con
return false
}
// Check if this section matches the normalized host
return normalizeHost(host) == normalizeHost(cfg.Host)
return workspace.MatchHost(host, cfg.Host)
})
if err == errNoMatchingProfiles {
section, err = configFile.NewSection(cfg.Profile)
Expand Down Expand Up @@ -138,16 +139,16 @@ func ValidateConfigAndProfileHost(cfg *config.Config, profile string) error {
}

// Normalized version of the configured host.
host := normalizeHost(cfg.Host)
host := workspace.NormalizeHost(cfg.Host)
match, err := findMatchingProfile(configFile, func(s *ini.Section) bool {
return profile == s.Name()
})
if err != nil {
return err
}

hostFromProfile := normalizeHost(match.Key("host").Value())
if hostFromProfile != "" && host != "" && hostFromProfile != host {
hostFromProfile := workspace.NormalizeHost(match.Key("host").Value())
if hostFromProfile != "" && host != "" && !workspace.MatchHost(hostFromProfile, host) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added same ignore-scheme-behavior for config-defined hosts

return fmt.Errorf("config host mismatch: profile uses host %s, but CLI configured to use %s", hostFromProfile, host)
}

Expand Down
48 changes: 48 additions & 0 deletions libs/workspace/host.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package workspace

import (
"net/url"
"strings"
)

// NormalizeHost returns the string representation of only
// the scheme and host part of the specified host.
func NormalizeHost(host string) string {
u, err := url.Parse(host)
if err != nil {
return host
}
if u.Scheme == "" || u.Host == "" {
return host
}

normalized := &url.URL{
Scheme: u.Scheme,
Host: u.Host,
}

return normalized.String()
}

// Match hosts using only Host part of the URL to allow cases when scheme is not specified
func MatchHost(host1, host2 string) bool {
if host1 == "" || host2 == "" {
return false
}
u1, err := url.Parse(fixUrlIfNeeded(host1))
if err != nil {
return false
}
u2, err := url.Parse(fixUrlIfNeeded(host2))
if err != nil {
return false
}
return u1.Host == u2.Host
}

func fixUrlIfNeeded(s string) string {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Apparently u := url.Parse('foo.com') will put foo.com in u.Path instead of u.Host so we're handling such cases explicitly

if !strings.HasPrefix(s, "http") {
return "https://" + s
}
return s
}
35 changes: 35 additions & 0 deletions libs/workspace/host_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package workspace

import (
"testing"

"github.com/stretchr/testify/assert"
)

func TestNormalizeHost(t *testing.T) {
assert.Equal(t, "invalid", NormalizeHost("invalid"))

// With port.
assert.Equal(t, "http://foo:123", NormalizeHost("http://foo:123"))

// With trailing slash.
assert.Equal(t, "http://foo", NormalizeHost("http://foo/"))

// With path.
assert.Equal(t, "http://foo", NormalizeHost("http://foo/bar"))

// With query string.
assert.Equal(t, "http://foo", NormalizeHost("http://foo?bar"))

// With anchor.
assert.Equal(t, "http://foo", NormalizeHost("http://foo#bar"))
}

func TestMatchHost(t *testing.T) {
assert.True(t, MatchHost("https://foo.com", "https://foo.com"))
assert.True(t, MatchHost("https://foo.com", "foo.com"))

assert.False(t, MatchHost("https://foo.com", "bar.com"))
assert.False(t, MatchHost("https://foo.com", "::invalid"))
assert.False(t, MatchHost("foo", "bar"))
}