feat: add forward for port-forwarding sessions - #26
Merged
Conversation
Every existing command is host-shaped: ensure auth, a MANAGED_SSH session and an SSH alias for a compute instance. That cannot reach things which are not compute -- a private OKE API endpoint, a database, an internal service -- because MANAGED_SSH requires an OS user and the Bastion plugin on the target. Those need a port-forwarding session, which is a different OCI session type. `oci-hop forward --private-ip X --target-port N` wraps `bastion-session forward`. It stays a thin pass-through on purpose: bastion-session owns session lifecycle, reuse, ssh option hardening and the readiness wait, and duplicating any of that here would let the two drift. --region/--profile are passed through because without a region the OCI CLI falls back to its own default and fails NotAuthorizedOrNotFound, which reads like a permissions problem rather than a wrong-region one. Requires bastion-session with forward support (adrianmross/bastion-session#28).
There was a problem hiding this comment.
🟡 Changes recommended
The new forward command has confirmed CLI contract inconsistencies (exit codes/flag forwarding/output handling) that can cause incorrect behavior or surprising automation differences.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a new oci-hop forward subcommand to support OCI Bastion port-forwarding sessions (non-host-shaped targets like private Kubernetes API endpoints, databases, and internal services) by delegating to bastion-session forward.
Changes:
- Registers a new
forwardcobra subcommand in the root CLI. - Introduces
cmd/oci-hop/forward.goimplementing a thin argv pass-through tobastion-session forward, including region/profile override flags and stdio streaming for long-lived tunnels.
File summaries
| File | Description |
|---|---|
| cmd/oci-hop/main.go | Wires the new forward command into the root command tree. |
| cmd/oci-hop/forward.go | Implements the oci-hop forward command and forwards flags/stdio to bastion-session forward. |
Review details
- Files reviewed: 2/3 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+44
to
+50
| RunE: func(cmd *cobra.Command, args []string) error { | ||
| if strings.TrimSpace(privateIP) == "" { | ||
| return fmt.Errorf("--private-ip is required") | ||
| } | ||
| if targetPort <= 0 { | ||
| return fmt.Errorf("--target-port is required") | ||
| } |
Comment on lines
+71
to
+73
| if strings.TrimSpace(identityFile) != "" { | ||
| argv = append(argv, "--ssh-private-key", identityFile) | ||
| } |
Comment on lines
+86
to
+88
| if f := strings.ToLower(strings.TrimSpace(format)); f == "json" || f == "yaml" || f == "yml" { | ||
| argv = append(argv, "-o", f) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Companion to adrianmross/bastion-session#28, which adds the underlying session type.
Why
Every existing command is host-shaped —
ensure,ssh,repair,trackall resolve a<host>and give it an SSH alias. That shape rests on MANAGED_SSH sessions, which need an OS user and the Bastion plugin enabled on a compute instance:Plenty of things worth reaching are not compute: a private OKE API endpoint, a database, an internal HTTP service. Those need a port-forwarding session — a different OCI session type, with no plugin requirement and no target user.
What
A deliberately thin pass-through to
bastion-session forward. bastion-session owns session lifecycle, reuse, ssh option hardening and the readiness wait; duplicating any of that here would let the two drift.--region/--profileare passed through because without a region the OCI CLI falls back to its own default and fails withNotAuthorizedOrNotFound— which reads like a permissions problem rather than a wrong-region one. I hit exactly that while testing.Measured outcome
Against a live bastion and a real private-endpoint OKE cluster:
Child failures surface correctly rather than being swallowed — verified accidentally, by running it against a released bastion-session that lacks the subcommand:
go build,go vet,go test ./...all clean;gofmtapplied.Dependency
Needs bastion-session with
forwardsupport, so this should land after #28 and after a bastion-session release. Until then the command exists but exits non-zero with the message above, which is at least self-explaining.