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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants