feat: add support for docker over ssh tunnel #549
Conversation
|
Note Reviews pausedUse the following commands to manage reviews:
WalkthroughThe PR introduces SSH tunneling support for Docker client connections. An SSH tunnel mechanism forwards local connections through SSH to the remote Docker daemon socket, enabling secure Docker operations over SSH with automatic resource cleanup and bidirectional data forwarding. Changes
Sequence DiagramsequenceDiagram
participant Client as Docker Client
participant LocalSocket as Local Unix Socket<br/>(Tunnel)
participant SSH as SSH Connection
participant Remote as Remote Docker Socket
Client->>LocalSocket: Connect (docker request)
LocalSocket->>SSH: Accept connection
SSH->>Remote: Establish SSH session
Remote-->>SSH: Connected to /var/run/docker.sock
SSH->>SSH: Start bidirectional<br/>data forwarding
Client->>LocalSocket: Send request data
LocalSocket->>SSH: Forward data
SSH->>Remote: Send to remote socket
Remote-->>SSH: Response data
SSH->>LocalSocket: Forward response
LocalSocket-->>Client: Response data
Client->>LocalSocket: Close
LocalSocket->>SSH: Close SSH channel
SSH->>Remote: Close remote connection
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api/internal/features/deploy/docker/init.go (1)
358-396: Inconsistent SSH usage: Compose methods bypass the established tunnel.
ComposeUp(line 359),ComposeDown(line 374), andComposeBuild(line 385) each instantiate a freshssh.NewSSH()client and execute shell commands directly over SSH, ignoring the Docker client and SSH tunnel already configured inDockerService. This creates multiple SSH connections, bypasses the unified client abstraction, and prevents centralized error handling or connection pooling.Refactor these methods to either:
- Use the existing
s.CliDocker client (preferred if the Docker SDK supports Compose operations), or- Reuse
s.sshTunnel.sshClientfor SSH commands to avoid redundant SSH sessions.Example for option 2:
func (s *DockerService) ComposeUp(composeFilePath string, envVars map[string]string) error { - client := ssh.NewSSH() + client := s.sshTunnel.sshClient + if client == nil { + client = ssh.NewSSH() + } envVarsStr := "" ...Repeat for
ComposeDownandComposeBuild.
🧹 Nitpick comments (2)
api/internal/features/deploy/docker/init.go (1)
108-119: Simplify the conditional by removing redundant nil check.Line 112 checks both
err != nilandtunnel == nil, but ifCreateSSHTunnelreturns an error,tunnelis alreadynilby Go convention. The second condition adds no value and can confuse readers.Apply this diff:
- if err != nil || tunnel == nil { - if err != nil { - lgr.Log(logger.Info, "SSH tunnel not established, using local docker socket", err.Error()) - } + if err != nil { + lgr.Log(logger.Info, "SSH tunnel not established, using local docker socket", err.Error())api/internal/features/deploy/docker/ssh_tunnel.go (1)
47-57: Improve shutdown handling: distinguish listener closure from errors.Line 50–52: When
t.listener.Accept()returns an error, the goroutine logs it as an error and exits. However, if the error is due to intentional listener closure (e.g., duringClose()), logging it as an error is misleading and may trigger false alerts in production monitoring.Check if the error is due to listener closure and log accordingly:
func (t *SSHTunnel) handleConnections(lgr logger.Logger) { for { localConn, err := t.listener.Accept() if err != nil { - lgr.Log(logger.Error, "SSH tunnel listener error", err.Error()) + if !errors.Is(err, net.ErrClosed) { + lgr.Log(logger.Error, "SSH tunnel listener error", err.Error()) + } return } go t.forwardConnection(localConn, lgr) } }Add the
errorsimport:import ( + "errors" "fmt" "io" "net"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
api/internal/features/deploy/docker/init.go(3 hunks)api/internal/features/deploy/docker/ssh_tunnel.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
api/internal/features/deploy/docker/ssh_tunnel.go (2)
api/internal/features/deploy/docker/init.go (2)
SSHTunnel(30-35)DockerService(23-28)api/internal/features/logger/init.go (1)
Error(22-22)
api/internal/features/deploy/docker/init.go (3)
api/internal/features/logger/init.go (4)
NewLogger(35-42)Warning(21-21)Error(22-22)Info(20-20)api/internal/features/ssh/init.go (1)
NewSSH(24-33)api/internal/features/deploy/docker/ssh_tunnel.go (1)
CreateSSHTunnel(19-44)
🔇 Additional comments (1)
api/internal/features/deploy/docker/ssh_tunnel.go (1)
93-98: LGTM, but document the Close requirement.The
Close()method correctly invokes the cleanup function if a tunnel exists. However, as noted in the review ofinit.go, there is no visible call toClose()in the provided code, leading to a resource leak.Ensure that the lifecycle owner of
DockerServicecallsClose()during shutdown (see the comment oninit.golines 90–106).
|
@coderabbitai pause |
✅ Actions performedReviews paused. |
Issue
Link to related issue(s):
Description
Short summary of what this PR changes or introduces.
Scope of Change
Select all applicable areas impacted by this PR:
Screenshot / Video / GIF (if applicable)
Attach or embed screenshots, screen recordings, or GIFs demonstrating the feature or fix.
Related PRs (if any)
Link any related or dependent PRs across repos.
Additional Notes for Reviewers (optional)
Anything reviewers should know before testing or merging (e.g., environment variables, setup steps).
Developer Checklist
To be completed by the developer who raised the PR.
Reviewer Checklist
To be completed by the reviewer before merge.
Summary by CodeRabbit
New Features
Refactor