diff --git a/bundle/run/job.go b/bundle/run/job.go index de5457c26e9..98f059e5784 100644 --- a/bundle/run/job.go +++ b/bundle/run/job.go @@ -5,9 +5,7 @@ import ( "encoding/json" "errors" "fmt" - "net/url" "strconv" - "strings" "time" "github.com/databricks/cli/bundle" @@ -99,7 +97,7 @@ func (m *jobRunMonitor) onProgress(info *jobs.Run) { // First time we see this run. if m.prevState == nil { - runURL := runPageURL(m.ctx, info.RunPageUrl) + runURL := workspaceurls.ModernizeJobRunPageURL(info.RunPageUrl) log.Infof(m.ctx, "Run available at %s", runURL) cmdio.Log(m.ctx, progress.NewJobRunUrlEvent(runURL)) } @@ -126,47 +124,6 @@ func (m *jobRunMonitor) onProgress(info *jobs.Run) { log.Info(m.ctx, event.String()) } -// runPageURL converts the legacy run URL returned by the Jobs API -// -// https:///?o=#job//run/ -// -// into the modern path form -// -// https:///jobs//runs/?o= -// -// so that non-admin users permitted to view the run are not redirected to the -// workspace homepage. See https://github.com/databricks/cli/issues/5142. The -// workspace selector query param (o) is preserved as-is. The conversion is -// cosmetic, so the original URL is returned on the rare chance the format is -// unexpected. -func runPageURL(ctx context.Context, raw string) string { - u, err := url.Parse(raw) - if err != nil { - log.Debugf(ctx, "could not parse run URL %q: %v", raw, err) - return raw - } - - jobID, runID, ok := parseLegacyRunFragment(u.Fragment) - if !ok { - log.Debugf(ctx, "unexpected run URL fragment %q", u.Fragment) - return raw - } - - u.Fragment = "" - u.Path = "/" + workspaceurls.JobRunPath(jobID, runID) - return u.String() -} - -// parseLegacyRunFragment extracts the job and run IDs from a legacy run URL -// fragment of the form "job//run/". -func parseLegacyRunFragment(fragment string) (jobID, runID string, ok bool) { - parts := strings.Split(fragment, "/") - if len(parts) != 4 || parts[0] != "job" || parts[2] != "run" || parts[1] == "" || parts[3] == "" { - return "", "", false - } - return parts[1], parts[3], true -} - func (r *jobRunner) Run(ctx context.Context, opts *Options) (output.RunOutput, error) { jobID, err := strconv.ParseInt(r.job.ID, 10, 64) if err != nil { @@ -205,7 +162,7 @@ func (r *jobRunner) Run(ctx context.Context, opts *Options) (output.RunOutput, e if err != nil { return nil, err } - cmdio.Log(ctx, progress.NewJobRunUrlEvent(runPageURL(ctx, details.RunPageUrl))) + cmdio.Log(ctx, progress.NewJobRunUrlEvent(workspaceurls.ModernizeJobRunPageURL(details.RunPageUrl))) return nil, nil } diff --git a/bundle/run/job_test.go b/bundle/run/job_test.go index 73fb68757c2..4307f2fae74 100644 --- a/bundle/run/job_test.go +++ b/bundle/run/job_test.go @@ -11,7 +11,6 @@ import ( "github.com/databricks/cli/libs/cmdio" "github.com/databricks/databricks-sdk-go/experimental/mocks" "github.com/databricks/databricks-sdk-go/service/jobs" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" ) @@ -292,50 +291,3 @@ func TestJobRunnerRestartForContinuousUnpausedJobs(t *testing.T) { _, err := runner.Restart(ctx, &Options{}) require.NoError(t, err) } - -func TestRunPageURL(t *testing.T) { - ctx := t.Context() - tests := []struct { - name string - raw string - expected string - }{ - { - "legacy fragment form preserves workspace selector", - "https://myworkspace.databricks.test/?o=900800700600#job/123/run/456", - "https://myworkspace.databricks.test/jobs/123/runs/456?o=900800700600", - }, - { - "no workspace selector", - "https://myworkspace.databricks.test/#job/123/run/456", - "https://myworkspace.databricks.test/jobs/123/runs/456", - }, - { - "http host with port", - "http://127.0.0.1:8080/?o=900800700600#job/1/run/2", - "http://127.0.0.1:8080/jobs/1/runs/2?o=900800700600", - }, - // Unexpected formats are returned unchanged because the conversion is cosmetic. - { - "already modern path is left as-is", - "https://myworkspace.databricks.test/jobs/123/runs/456?o=900800700600", - "https://myworkspace.databricks.test/jobs/123/runs/456?o=900800700600", - }, - { - "incomplete fragment is left as-is", - "https://myworkspace.databricks.test/?o=900800700600#job/123", - "https://myworkspace.databricks.test/?o=900800700600#job/123", - }, - { - "empty job id is left as-is", - "https://myworkspace.databricks.test/?o=900800700600#job//run/456", - "https://myworkspace.databricks.test/?o=900800700600#job//run/456", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - assert.Equal(t, tt.expected, runPageURL(ctx, tt.raw)) - }) - } -} diff --git a/libs/workspaceurls/urls.go b/libs/workspaceurls/urls.go index a1bf973801f..db5a817567b 100644 --- a/libs/workspaceurls/urls.go +++ b/libs/workspaceurls/urls.go @@ -90,6 +90,45 @@ func JobRunURL(baseURL url.URL, jobID, runID string) string { return baseURL.String() } +// ModernizeJobRunPageURL converts the legacy run URL returned by the Jobs API +// +// https:///?o=#job//run/ +// +// into the modern path form +// +// https:///jobs//runs/?o= +// +// so that non-admin users permitted to view the run are not redirected to the +// workspace homepage. See https://github.com/databricks/cli/issues/5142. The +// workspace selector query param (o) is preserved as-is. The conversion is +// cosmetic, so the original URL is returned on the rare chance the format is +// unexpected. +func ModernizeJobRunPageURL(raw string) string { + u, err := url.Parse(raw) + if err != nil { + return raw + } + + jobID, runID, ok := parseLegacyRunFragment(u.Fragment) + if !ok { + return raw + } + + u.Fragment = "" + u.Path = "/" + JobRunPath(jobID, runID) + return u.String() +} + +// parseLegacyRunFragment extracts the job and run IDs from a legacy run URL +// fragment of the form "job//run/". +func parseLegacyRunFragment(fragment string) (jobID, runID string, ok bool) { + parts := strings.Split(fragment, "/") + if len(parts) != 4 || parts[0] != "job" || parts[2] != "run" || parts[1] == "" || parts[3] == "" { + return "", "", false + } + return parts[1], parts[3], true +} + // ResourceURL constructs a workspace URL for a named resource type and ID. func ResourceURL(baseURL url.URL, resourceType, id string) string { resourceType = resolveAlias(resourceType) diff --git a/libs/workspaceurls/urls_test.go b/libs/workspaceurls/urls_test.go index e39d28d9aaf..a398167e027 100644 --- a/libs/workspaceurls/urls_test.go +++ b/libs/workspaceurls/urls_test.go @@ -166,3 +166,49 @@ func TestHasWorkspaceIDInHostname(t *testing.T) { }) } } + +func TestModernizeJobRunPageURL(t *testing.T) { + tests := []struct { + name string + raw string + expected string + }{ + { + "legacy fragment form preserves workspace selector", + "https://myworkspace.databricks.test/?o=900800700600#job/123/run/456", + "https://myworkspace.databricks.test/jobs/123/runs/456?o=900800700600", + }, + { + "no workspace selector", + "https://myworkspace.databricks.test/#job/123/run/456", + "https://myworkspace.databricks.test/jobs/123/runs/456", + }, + { + "http host with port", + "http://127.0.0.1:8080/?o=900800700600#job/1/run/2", + "http://127.0.0.1:8080/jobs/1/runs/2?o=900800700600", + }, + // The conversion is cosmetic, so any other format passes through unchanged. + { + "already modern path is left as-is", + "https://myworkspace.databricks.test/jobs/123/runs/456?o=900800700600", + "https://myworkspace.databricks.test/jobs/123/runs/456?o=900800700600", + }, + { + "incomplete fragment is left as-is", + "https://myworkspace.databricks.test/?o=900800700600#job/123", + "https://myworkspace.databricks.test/?o=900800700600#job/123", + }, + { + "empty job id is left as-is", + "https://myworkspace.databricks.test/?o=900800700600#job//run/456", + "https://myworkspace.databricks.test/?o=900800700600#job//run/456", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, ModernizeJobRunPageURL(tt.raw)) + }) + } +}