diff --git a/CHANGELOG.md b/CHANGELOG.md index 639270e32..9b08d7514 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,26 @@ # Version changelog +## [Release] Release v0.233.0 + +CLI: + * Clean host URL in the `auth login` command ([#1879](https://github.com/databricks/cli/pull/1879)). + +Bundles: + * Fix bundle run when run interactively ([#1880](https://github.com/databricks/cli/pull/1880)). + * Fix relative path resolution for dashboards on Windows ([#1881](https://github.com/databricks/cli/pull/1881)). + +Internal: + * Address goreleaser deprecation warning ([#1872](https://github.com/databricks/cli/pull/1872)). + * Update actions/github-script to v7 ([#1873](https://github.com/databricks/cli/pull/1873)). + * Use Go 1.23 ([#1871](https://github.com/databricks/cli/pull/1871)). + * [Internal] Always write message for manual integration test trigger ([#1874](https://github.com/databricks/cli/pull/1874)). + * Add `cmd-exec-id` to user agent ([#1808](https://github.com/databricks/cli/pull/1808)). + * Added E2E test to run Python wheels on interactive cluster created in bundle ([#1864](https://github.com/databricks/cli/pull/1864)). + + +Dependency updates: + * Bump github.com/hashicorp/terraform-json from 0.22.1 to 0.23.0 ([#1877](https://github.com/databricks/cli/pull/1877)). + ## [Release] Release v0.232.1 This patch release fixes the following error observed when deploying to /Shared root folder diff --git a/bundle/config/mutator/translate_paths.go b/bundle/config/mutator/translate_paths.go index 82b0b3caa..321fa5b30 100644 --- a/bundle/config/mutator/translate_paths.go +++ b/bundle/config/mutator/translate_paths.go @@ -163,7 +163,7 @@ func (t *translateContext) translateNoOp(literal, localFullPath, localRelPath, r } func (t *translateContext) retainLocalAbsoluteFilePath(literal, localFullPath, localRelPath, remotePath string) (string, error) { - info, err := t.b.SyncRoot.Stat(localRelPath) + info, err := t.b.SyncRoot.Stat(filepath.ToSlash(localRelPath)) if errors.Is(err, fs.ErrNotExist) { return "", fmt.Errorf("file %s not found", literal) } diff --git a/bundle/config/mutator/translate_paths_dashboards_test.go b/bundle/config/mutator/translate_paths_dashboards_test.go new file mode 100644 index 000000000..c386f1bbe --- /dev/null +++ b/bundle/config/mutator/translate_paths_dashboards_test.go @@ -0,0 +1,54 @@ +package mutator_test + +import ( + "context" + "path/filepath" + "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/cli/libs/vfs" + "github.com/databricks/databricks-sdk-go/service/dashboards" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestTranslatePathsDashboards_FilePathRelativeSubDirectory(t *testing.T) { + dir := t.TempDir() + touchEmptyFile(t, filepath.Join(dir, "src", "my_dashboard.lvdash.json")) + + b := &bundle.Bundle{ + SyncRootPath: dir, + SyncRoot: vfs.MustNew(dir), + Config: config.Root{ + Resources: config.Resources{ + Dashboards: map[string]*resources.Dashboard{ + "dashboard": { + CreateDashboardRequest: &dashboards.CreateDashboardRequest{ + DisplayName: "My Dashboard", + }, + FilePath: "../src/my_dashboard.lvdash.json", + }, + }, + }, + }, + } + + bundletest.SetLocation(b, "resources.dashboards", []dyn.Location{{ + File: filepath.Join(dir, "resources/dashboard.yml"), + }}) + + diags := bundle.Apply(context.Background(), b, mutator.TranslatePaths()) + require.NoError(t, diags.Error()) + + // Assert that the file path for the dashboard has been converted to its local absolute path. + assert.Equal( + t, + filepath.Join(dir, "src", "my_dashboard.lvdash.json"), + b.Config.Resources.Dashboards["dashboard"].FilePath, + ) +} diff --git a/cmd/bundle/destroy.go b/cmd/bundle/destroy.go index cd7e63062..711abbcd7 100644 --- a/cmd/bundle/destroy.go +++ b/cmd/bundle/destroy.go @@ -6,6 +6,7 @@ import ( "os" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config/mutator" "github.com/databricks/cli/bundle/phases" "github.com/databricks/cli/cmd/bundle/utils" "github.com/databricks/cli/cmd/root" @@ -62,7 +63,12 @@ func newDestroyCommand() *cobra.Command { diags = bundle.Apply(ctx, b, bundle.Seq( phases.Initialize(), - phases.Build(), + // We need to resolve artifact variable (how we do it in build phase) + // because some of the to-be-destroyed resource might use this variable. + // Not resolving might lead to terraform "Reference to undeclared resource" error + mutator.ResolveVariableReferences( + "artifacts", + ), phases.Destroy(), )) if err := diags.Error(); err != nil { diff --git a/cmd/bundle/run.go b/cmd/bundle/run.go index 96851d0c0..7a92766d9 100644 --- a/cmd/bundle/run.go +++ b/cmd/bundle/run.go @@ -35,17 +35,23 @@ func promptRunArgument(ctx context.Context, b *bundle.Bundle) (string, error) { return key, nil } -func resolveRunArgument(ctx context.Context, b *bundle.Bundle, args []string) (string, error) { +// resolveRunArgument resolves the resource key to run. +// It returns the remaining arguments to pass to the runner, if applicable. +func resolveRunArgument(ctx context.Context, b *bundle.Bundle, args []string) (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) + key, err := promptRunArgument(ctx, b) + if err != nil { + return "", nil, err + } + return key, args, nil } if len(args) < 1 { - return "", fmt.Errorf("expected a KEY of the resource to run") + return "", nil, fmt.Errorf("expected a KEY of the resource to run") } - return args[0], nil + return args[0], args[1:], nil } func keyToRunner(b *bundle.Bundle, arg string) (run.Runner, error) { @@ -109,7 +115,7 @@ task or a Python wheel task, the second example applies. return err } - arg, err := resolveRunArgument(ctx, b, args) + key, args, err := resolveRunArgument(ctx, b, args) if err != nil { return err } @@ -124,13 +130,13 @@ task or a Python wheel task, the second example applies. return err } - runner, err := keyToRunner(b, arg) + runner, err := keyToRunner(b, key) if err != nil { return err } // Parse additional positional arguments. - err = runner.ParseArgs(args[1:], &runOptions) + err = runner.ParseArgs(args, &runOptions) if err != nil { return err } diff --git a/libs/auth/oauth.go b/libs/auth/oauth.go index 7c1cb9576..026c45468 100644 --- a/libs/auth/oauth.go +++ b/libs/auth/oauth.go @@ -9,6 +9,7 @@ import ( "errors" "fmt" "net" + "net/url" "strings" "time" @@ -143,6 +144,26 @@ func (a *PersistentAuth) Challenge(ctx context.Context) error { return nil } +// This function cleans up the host URL by only retaining the scheme and the host. +// This function thus removes any path, query arguments, or fragments from the URL. +func (a *PersistentAuth) cleanHost() { + parsedHost, err := url.Parse(a.Host) + if err != nil { + return + } + // when either host or scheme is empty, we don't want to clean it. This is because + // the Go url library parses a raw "abc" string as the path of a URL and cleaning + // it will return thus return an empty string. + if parsedHost.Host == "" || parsedHost.Scheme == "" { + return + } + host := url.URL{ + Scheme: parsedHost.Scheme, + Host: parsedHost.Host, + } + a.Host = host.String() +} + func (a *PersistentAuth) init(ctx context.Context) error { if a.Host == "" && a.AccountID == "" { return ErrFetchCredentials @@ -156,6 +177,9 @@ func (a *PersistentAuth) init(ctx context.Context) error { if a.browser == nil { a.browser = browser.OpenURL } + + a.cleanHost() + // try acquire listener, which we also use as a machine-local // exclusive lock to prevent token cache corruption in the scope // of developer machine, where this command runs. diff --git a/libs/auth/oauth_test.go b/libs/auth/oauth_test.go index ea6a8061e..fdf0d04bf 100644 --- a/libs/auth/oauth_test.go +++ b/libs/auth/oauth_test.go @@ -228,3 +228,37 @@ func TestChallengeFailed(t *testing.T) { assert.EqualError(t, err, "authorize: access_denied: Policy evaluation failed for this request") }) } + +func TestPersistentAuthCleanHost(t *testing.T) { + for _, tcases := range []struct { + in string + out string + }{ + {"https://example.com", "https://example.com"}, + {"https://example.com/", "https://example.com"}, + {"https://example.com/path", "https://example.com"}, + {"https://example.com/path/subpath", "https://example.com"}, + {"https://example.com/path?query=1", "https://example.com"}, + {"https://example.com/path?query=1&other=2", "https://example.com"}, + {"https://example.com/path#fragment", "https://example.com"}, + {"https://example.com/path?query=1#fragment", "https://example.com"}, + {"https://example.com/path?query=1&other=2#fragment", "https://example.com"}, + {"https://example.com/path/subpath?query=1", "https://example.com"}, + {"https://example.com/path/subpath?query=1&other=2", "https://example.com"}, + {"https://example.com/path/subpath#fragment", "https://example.com"}, + {"https://example.com/path/subpath?query=1#fragment", "https://example.com"}, + {"https://example.com/path/subpath?query=1&other=2#fragment", "https://example.com"}, + {"https://example.com/path?query=1%20value&other=2%20value", "https://example.com"}, + {"http://example.com/path/subpath?query=1%20value&other=2%20value", "http://example.com"}, + + // URLs without scheme should be left as is + {"abc", "abc"}, + {"abc.com/def", "abc.com/def"}, + } { + p := &PersistentAuth{ + Host: tcases.in, + } + p.cleanHost() + assert.Equal(t, tcases.out, p.Host) + } +}