Add support for ssh tunnel to remote hosts #2
Conversation
* Initial plan * Add SSH tunnel support for MySQL connections Co-authored-by: tkuhlengel <897141+tkuhlengel@users.noreply.github.com> * Add SSH tunnel documentation and tests Co-authored-by: tkuhlengel <897141+tkuhlengel@users.noreply.github.com> * Address code review feedback - improve error handling and validation Co-authored-by: tkuhlengel <897141+tkuhlengel@users.noreply.github.com> * Improve exception handling and documentation Co-authored-by: tkuhlengel <897141+tkuhlengel@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: tkuhlengel <897141+tkuhlengel@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
This PR adds SSH tunnel support to the guacalib library, enabling secure remote MySQL database access through an SSH gateway. This feature is particularly useful for connecting to databases that are not directly accessible from the local network.
Key changes:
- Adds
sshtunnelpackage dependency (version >=0.4.0) - Implements SSH tunnel configuration reading from environment variables or config files
- Integrates SSH tunnel establishment in the database connection flow with automatic cleanup
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| requirements.txt | Adds sshtunnel>=0.4.0 dependency |
| pyproject.toml | Adds sshtunnel>=0.4.0 to project dependencies |
| guacalib/db.py | Implements SSH tunnel configuration, connection logic, and cleanup handling |
| guacalib.ini.example | Documents SSH tunnel configuration options in example config file |
| README.md | Adds comprehensive documentation for SSH tunnel feature with usage examples |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ssh_config = { | ||
| "enabled": True, | ||
| "host": os.environ.get("GUACALIB_SSH_TUNNEL_HOST"), | ||
| "port": port, | ||
| "user": os.environ.get("GUACALIB_SSH_TUNNEL_USER"), | ||
| "password": os.environ.get("GUACALIB_SSH_TUNNEL_PASSWORD"), | ||
| "private_key": os.environ.get("GUACALIB_SSH_TUNNEL_PRIVATE_KEY"), | ||
| "private_key_passphrase": os.environ.get( | ||
| "GUACALIB_SSH_TUNNEL_PRIVATE_KEY_PASSPHRASE" | ||
| ), | ||
| } |
There was a problem hiding this comment.
[nitpick] The SSH configuration dictionary includes both password and private_key even though they are mutually exclusive authentication methods. This could lead to confusion about which method is being used. Consider restructuring to make the authentication method explicit, or add a comment explaining that the SSHTunnelForwarder library will handle the priority between these methods.
| ssh_config = { | ||
| "enabled": True, | ||
| "host": config["mysql"].get("ssh_tunnel_host"), | ||
| "port": port, | ||
| "user": config["mysql"].get("ssh_tunnel_user"), | ||
| "password": config["mysql"].get("ssh_tunnel_password"), | ||
| "private_key": config["mysql"].get("ssh_tunnel_private_key"), | ||
| "private_key_passphrase": config["mysql"].get( | ||
| "ssh_tunnel_private_key_passphrase" | ||
| ), | ||
| } |
There was a problem hiding this comment.
This duplicates the SSH configuration dictionary structure from lines 318-328. Consider extracting this into a helper function to reduce code duplication and improve maintainability.
| if self.ssh_tunnel_config.get("private_key"): | ||
| ssh_kwargs["ssh_pkey"] = self.ssh_tunnel_config["private_key"] |
There was a problem hiding this comment.
The private_key value from config is a file path string, but SSHTunnelForwarder's ssh_pkey parameter expects a paramiko key object or the path needs to be passed to ssh_pkey parameter which handles both. This may work if SSHTunnelForwarder accepts string paths, but if it expects a key object, this will fail. Verify that the sshtunnel library accepts file paths as strings for the ssh_pkey parameter, or load the key using paramiko before passing it.
|
Interesting idea. The trouble is, I’m in the middle of a serious db.py rewrite -- db.py has gotten huge, LLMs started doing weird things with it, and I’m splitting it into multiple files while leaving db.py as a facade. You can take a peek at the feature/modular branch to see what’s actually going on right now. I expect to finish this within a few weeks, and then we can look into the MySQL-over-SSH feature. |
2 participants
Interesting project! I've been working on a related private task for a while, and this is a lovely clean project.
While I was checking to see if this would be useful for my project, I added support for using SSH Tunnels, which is necessary for my use case, and easy to support from inside.
Take a look and see if this is something you're interested in.
Happy to make changes if you see fit.