Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The rsync transfer path selects SSH options incorrectly for local→remote vs remote→local transfers and needs conditional logic to pick the actual remote endpoint.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR aims to complete SSH portal support for multi-cluster setups by allowing SSH token retrieval to use a different host/port than the SSH tunnel endpoint, and by adjusting how per-environment SSH options are selected during transfers.
Changes:
- Extend
utils.ApiConn.Initto accept optionaltokenHost/tokenPortoverrides for SSH token retrieval. - Fix an environment selection error when resolving per-environment SSH options for rsync transfers.
- Add support for
LAGOON_CONFIG_TOKEN_HOST/LAGOON_CONFIG_TOKEN_PORTwhen building the SSH portal option wrapper.
File summaries
| File | Description |
|---|---|
| utils/sshportal.go | Adds token host/port override support for SSH token retrieval during API connection initialization. |
| synchers/syncutils.go | Changes which environment name is used to resolve SSH options for rsync transfers (needs conditional handling to support both transfer directions). |
| cmd/syncSharedFuncs.go | Reads token host/port env vars and passes them into the SSH token retrieval initialization path when portal mode is enabled. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| sourceEnvSshOptions := sshOptionWrapper.GetSSHOptionsForEnvironment(sourceEnvironment.EnvironmentName) | ||
| rsyncArgs := sshOptions.RsyncArgs | ||
|
|
||
| execString := fmt.Sprintf("%s %s --rsync-path=%s %s -e \"ssh%s -o LogLevel=FATAL -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no -p %s -l %s %s service=%s\" %s %s %s", |
| // Resolve token host/port from env vars when using the SSH portal. | ||
| // These are only used for token retrieval and do not affect the SSH tunnel endpoint. | ||
| tokenHost, _ := os.LookupEnv("LAGOON_CONFIG_TOKEN_HOST") | ||
| tokenPort, _ := os.LookupEnv("LAGOON_CONFIG_TOKEN_PORT") |
There was a problem hiding this comment.
These aren't user facing, and shouldn't be overridden, so no docs required.
SSH portal support was only partially supported up until now, this PR completes the implementation