-
Notifications
You must be signed in to change notification settings - Fork 108
Add "ssh server" command #3475
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add "ssh server" command #3475
Conversation
|
8c90ef8
to
e6fb1d0
Compare
a0f94b7
to
fbbc3db
Compare
fbbc3db
to
17c3059
Compare
b222dae
to
421c153
Compare
50eeb53
to
48eaf73
Compare
421c153
to
7d74402
Compare
48eaf73
to
de12be3
Compare
7d74402
to
a3e7567
Compare
de12be3
to
6f8a52e
Compare
a3e7567
to
33d5bda
Compare
6f8a52e
to
febaca3
Compare
5d7a9f1
to
15e2eb7
Compare
## Changes This is the first PR that introduces the ssh-tunnel functionality into the CLI See README for an overview of the SSH tunnel feature and its usage. This PR only adds `ssh setup` command, `connect` and `server` are here: - #3471 - #3475 The whole `ssh` command group is hidden (not available in `--help` output, and in this PR it's not even added to the root cmd), and the usage text has private preview disclaimer. Hidden flag will be removed when we move to public preview phase (no timeline yet). `setup` command updates local ssh config (`~/.ssh/config`). The config entry relies on `ProxyCommand` option calling `databricks ssh connect`, which is implemented in a follow up PR. ## Why We've agreed to move the implementation form the separate repo to the main databricks CLI ## Tests Manually and unit tests. --------- Co-authored-by: Pieter Noordhuis <[email protected]>
2f17742
to
da4953f
Compare
15e2eb7
to
ee55b90
Compare
da4953f
to
9bdf95a
Compare
ee55b90
to
a13a6e4
Compare
a13a6e4
to
9e595af
Compare
9e595af
to
60fc8a3
Compare
60fc8a3
to
8be1123
Compare
9bdf95a
to
2030553
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stamp to unblock review of the #3544.
Long: `Run SSH tunnel server. | ||
|
||
This command starts an SSH tunnel server that accepts WebSocket connections | ||
and proxies them to local SSH daemon processes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack. The failure happens somewhere deep in the stack. It would be useful to make this error more explicit. If a user runs it and gets some error about a secret missing, that's confusing. If instead they see "This command is supposed to be run on Databricks compute. Please find usage instructions at <>.", that's better.
## Changes See the base PR for the context: - #3470 This PR adds `databricks ssh connect` subcommand and all related utilities. The main logic here is in `libs/ssh/client.go` and `libs/ssh/proxy.go` files. See `proxy_test.go` for an overview of how client and server interact with each other through the proxy. Overview of the `ssh client` logic: - Generate local ssh keys if necessary (`~/.databricks/ssh-tunnel-keys/<cluster-id>`) - Save the public key in the secret scope (`<username>-<cluster-id>-ssh-tunnel-keys`) - Upload databricks releases (linux arm and amd) with the marching version to the /Workspace - Get /Workspace/metadata.json file with the server port info - If the metadata is not there, execute `ssh-server-bootsrap.py` file as a job (it runs `databricks server` command that's implemented in the follow up PR) - Get server metadata by sending a driver-proxy request to the known port, it will return a user name - Spawn `ssh client` with the right User and a ProxyCommand that executed `ssh connect --proxy --metadata` - New instance of the `connect` command can now start a proxy over websocket connection backed by Driver Proxy API Overview of the Proxy logic: - The main interesting part is that the proxy does automatic re-connects every 30 minutes (configurable) to avoid auth token expiration problems. In all the tricky places (mostly related to concurrency and locks) there are comments explaining the logic. The final follow up PR: - #3475 ## Tests Manual and unit
8be1123
to
7ebfe7e
Compare
## Changes Previous PRs in the stack: - #3471 - #3475 Move from flat structure to cmd and internal client/server/proxy/keys/workspace sub packages. Moved a bunch of hardcoded values to constants (some hardcoded stuff remains in the `keys` and `workspace` sub packages, will be improved in a follow up) Added unit tests for servers connection manager logic and updated proxy tests. The coverage isn't great yet, the goal is to avoid unit tests with excessive mocking and focus on integration tests from now on. ## Why `/experimental` already has the databricks-bundles python project, seems like a good place to put ssh stuff too ## Tests Unit and manual
Changes
This PR is based on the above, it adds "ssh server" command and related logic and finishes the "ssh" implementation by adding
ssh
command group to the rootcmd
(it's still hidden from the --help output).Server overview:
ssh-server-bootsrap.py
jobsetup_subreaper
method, which ensures no child process leave the parent process group. This is necessary in order to prevent background processes being reparented to pid 1 when their parent ssh server session terminates, after which they loose access to wsfs and dbfs fuse mounts (as access is based on process groups). All major IDEs usually try to spawn their server processes as a background processes (by double forking for example).-i
flag, making it run as in non-daemon mode and communicating with it through stdin and our (and so sshd doesn't listen on any ports)$HOME/.ipython/profile_default/startup
. This file makes jupyter kernels a bit more similar to webapp notebooks - it provides spark and dbutils globals, plus handles %sql and %pip magic commandsWIP: unit tests
Tests
Manual and unit