Skip to content

experimental/ssh: validate dedicated access mode on direct ssh connect#5767

Closed
TanishqMaheshwari wants to merge 1 commit into
databricks:mainfrom
TanishqMaheshwari:ssh-connect-validate-dedicated-cluster
Closed

experimental/ssh: validate dedicated access mode on direct ssh connect#5767
TanishqMaheshwari wants to merge 1 commit into
databricks:mainfrom
TanishqMaheshwari:ssh-connect-validate-dedicated-cluster

Conversation

@TanishqMaheshwari

Copy link
Copy Markdown

Summary

databricks ssh connect --cluster <id> could be run without going through databricks ssh setup, which is where the cluster's access mode was validated. A user pointing the command directly at a non-dedicated cluster got no early error — the connection only failed later at runtime with an opaque message.

This change validates the cluster's access mode on the direct connect --cluster path as well:

  • The access-mode check (DataSecurityMode == SINGLE_USER) moves into the client package as the exported ValidateClusterAccess. setup now reuses it instead of its own private copy, so there is a single source of truth.
  • Run() calls it only on the direct, non-proxy connect --cluster path:
    • Proxy mode is skipped — its ProxyCommand was generated by setup, which already validated the cluster, so re-checking would add a Clusters.Get on every (re)connection.
    • Serverless is skipped — there is no cluster to inspect.
  • The error message now reports the access mode using the Databricks UI label ("Dedicated" / "Standard" / "No isolation") instead of the raw API enum (SINGLE_USER / USER_ISOLATION), so it matches what the user selected when creating the cluster. Legacy/auto/unknown modes fall back to the raw value.

Example

Before:

cluster '0605-...' does not have dedicated access mode. Current access mode: USER_ISOLATION. ...

After:

cluster '0605-...' does not have Dedicated access mode. Current access mode: Standard. Please ensure the cluster is configured with Dedicated (single user) access mode

Tests

  • Moved the three validateClusterAccess unit tests into client_internal_test.go (now testing the exported function) and updated assertions for the new wording.
  • Added TestAccessModeUILabel covering the enum → UI-label mapping, including the raw-value fallback for legacy modes.

This pull request and its description were written by Isaac.

`ssh connect --cluster` could be run without going through `ssh setup`,
which is where the cluster's access mode was validated. A user pointing
the command directly at a non-dedicated cluster got no early error and
the connection only failed later at runtime.

Move the access-mode check into the client package as the exported
ValidateClusterAccess and call it on the direct, non-proxy
`connect --cluster` path. Proxy mode is skipped (its ProxyCommand was
generated by `setup`, which already validated the cluster) as is
serverless (no cluster to inspect). `setup` now reuses the same function.

The error message now reports the access mode using the Databricks UI
label ("Dedicated"/"Standard"/"No isolation") instead of the raw API
enum (SINGLE_USER/USER_ISOLATION), so it matches what the user selected
when creating the cluster.

Co-authored-by: Isaac
@github-actions

Copy link
Copy Markdown
Contributor

Waiting for approval

Based on git history, these people are best suited to review:

  • @anton-107 -- recent work in experimental/ssh/internal/client/
  • @ilia-db -- recent work in experimental/ssh/internal/client/, experimental/ssh/internal/setup/

Eligible reviewers: @andrewnester, @denik, @pietern, @renaudhartert-db, @shreyas-goenka, @simonfaltum

Suggestions based on git history. See OWNERS for ownership rules.

@github-actions

Copy link
Copy Markdown
Contributor

An authorized user can trigger integration tests manually by following the instructions below:

Trigger:
go/deco-tests-run/cli

Inputs:

  • PR number: 5767
  • Commit SHA: 1f087ba272541e81058423bad4833f60cf31e07b

Checks will be approved automatically on success.

@TanishqDatabricks

Copy link
Copy Markdown
Collaborator

Superseded by #5768, which is opened directly from a databricks/cli branch instead of a personal fork. Same changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants