Skip to content

fix(cli): preserve external TLS for SSH forwarding - #4344

Open
danehans wants to merge 2 commits into
NVIDIA:mainfrom
danehans:fix/3674-preserve-external-tls/danehans
Open

fix(cli): preserve external TLS for SSH forwarding#4344
danehans wants to merge 2 commits into
NVIDIA:mainfrom
danehans:fix/3674-preserve-external-tls/danehans

Conversation

@danehans

@danehans danehans commented Oct 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

Preserve the registered external HTTPS scheme when SSH forwarding replaces an internal gateway bind address. This fix was ported from #3714 after that PR was closed.

Related Issue

Fixes: #3674

Changes

  • Resolve the effective gateway scheme together with its host and port.
  • Apply the shared result in CLI and TUI SSH paths.
  • Cover loopback, unspecified, invalid, IPv6, and HTTP(S) transitions.
  • Document HTTPS endpoint registration for SSH connect, exec, and forwarding.

Testing

  • Checks appropriate to the affected code and behavior pass
  • Unit tests added/updated (if applicable)
  • E2E tests added/updated (existing workflow coverage requested with test:e2e)
  • cargo fmt --all -- --check
  • cargo test -p openshell-core resolve_ssh_gateway
  • cargo test -p openshell-cli -p openshell-tui
  • mise run docs
  • Repository commit hooks

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture docs updated (not applicable)

@danehans
fix(cli): preserve external TLS for SSH forwarding
ce8e85e 
Resolve the external scheme together with its authority when replacing an
internal bind address with the registered gateway endpoint.

Ported from NVIDIA#3714.

Fixes: NVIDIA#3674

Signed-off-by: Daneyon Hansen <daneyon.hansen@solo.io>
@copy-pr-bot

copy-pr-bot Bot commented Oct 8, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@danehans danehans mentioned this pull request Oct 8, 2026
Open
5 tasks
@purp

purp commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator

/ok to test ce8e85e

@purp purp added the test:e2e Requires end-to-end coverage label Oct 8, 2026
@github-actions

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown

Label test:e2e applied for ce8e85e. Open the existing run and click Re-run all jobs to execute with the label set. The run will execute the standard E2E suite after building the required gateway, sandbox, and supervisor images once. The matching required CI gate status on this PR will flip green automatically once the run finishes.

@purp

purp commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator

gator-agent

PR Review Status

The independent code review found no blocking defects. The shared resolver now carries the external scheme through the CLI and all three TUI SSH paths, while retaining the reported endpoint for non-loopback hosts.

@purp, your /ok to test command successfully enabled the current-head mirror checks; Branch Checks, Helm Lint, Trivy, DCO, and vouch pass. I applied test:e2e because the shared SSH transport affects connect, exec, and forwarding. The existing E2E run skipped its runtime suites before that label was present, so it does not satisfy the newly required coverage.

Action required: A maintainer must select Re-run all jobs on Branch E2E Checks. @danehans, add a small published docs note about SSH connect/exec/forwarding using the registered HTTPS endpoint behind TLS termination, or have a maintainer explain in this PR why docs are unnecessary.

Blocking code findings: None.

Carried findings: None.

Non-blocking suggestion: Track the linked issue's dedicated Kubernetes regression coverage for SSH through frontend TLS termination with a plaintext backend, including the exact http://0.0.0.0:8080 to https://localhost:18443 case.

Gator metadata
  • Validation: Concentrated client bug fix for accepted issue bug: SSH forwarding drops HTTPS behind TLS termination #3674; the source PR feat(helm): add agentgateway ingress support #3714 is closed.
  • Docs: Missing for user-visible SSH behavior; no maintainer-authored no-docs rationale is present.
  • Checks: Current-head Branch Checks, Helm Lint, Trivy, DCO, and vouch pass.
  • E2E: test:e2e applied; existing run's runtime suites were skipped. Maintainer rerun required; no workflow rerun was authorized in this invocation.
  • Windows: No Windows-specific changes; test:windows not required.
  • Head SHA: ce8e85e87ac6ba2e5da81e270a17052e3c080844
  • Base SHA: 0e981be906ef34b26464b39a7de49e1f7823b697
  • Merge base SHA: 0e981be906ef34b26464b39a7de49e1f7823b697
  • Patch ID: 8a2602719df53438dedd6633cbd8dd820853044c
  • Gator payload: 11
  • Review mode: initial
  • Previous reviewed SHA: none
  • Review budget exhausted: no
  • Maintainer decision required: no convergence decision required
  • Next state: gator:blocked
  • Blocked reason: test_dispatch_required

@purp purp added gator:blocked Gator is blocked by process or repository gates gator:in-review Gator is reviewing or awaiting PR review feedback and removed gator:blocked Gator is blocked by process or repository gates labels Oct 8, 2026
@danehans
docs(kubernetes): explain TLS SSH relay endpoint
c6dae2b 
Signed-off-by: Daneyon Hansen <daneyon.hansen@solo.io>
@danehans

danehans commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

@purp I added a note clarifying that connect/exec/port forwarding use the registered external HTTPS endpoint behind TLS termination, PTAL.

@purp

purp commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator

/ok to test c6dae2b

@purp

purp commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator

gator-agent

PR Review Status

Thanks @danehans, I checked your new SSH Relay note in docs/kubernetes/ingress.mdx. It documents the registered external HTTPS endpoint for connect, exec, and forwarding, resolving the documentation request. The independent follow-up review covered this docs-only delta and found no blocking defects; the previous code review remains applicable.

@purp, your current-head /ok to test command enabled the mirror, and Branch Checks, Helm Lint, and Branch E2E Checks are now queued or running. The older E2E rerun request is superseded by this new current-head run with test:e2e already applied.

Action required: A maintainer must approve Trivy Changes, which reports action_required and has no jobs. The sandbox policy denied the workflow approval endpoint, so Gator cannot release that gate. No workflow rerun is requested.

Blocking findings: None.

Carried findings: None.

Gator metadata
  • Validation: Concentrated client bug fix for issue bug: SSH forwarding drops HTTPS behind TLS termination #3674; source PR feat(helm): add agentgateway ingress support #3714 is closed.
  • Docs: Requested published SSH Relay note added; no navigation change needed.
  • Checks: DCO passes; Branch Checks and Helm Lint started; Trivy awaits workflow approval.
  • E2E: test:e2e present; current-head Branch E2E Checks started.
  • Windows: No Windows-specific changes; coverage label not required.
  • Head SHA: c6dae2bc553371dd99d9fec511d799b989607aac
  • Base SHA: 0e981be906ef34b26464b39a7de49e1f7823b697
  • Merge base SHA: 0e981be906ef34b26464b39a7de49e1f7823b697
  • Patch ID: e67421516fb457567b3808163a92377627741e90
  • Gator payload: 11
  • Review mode: follow_up
  • Previous reviewed SHA: ce8e85e87ac6ba2e5da81e270a17052e3c080844
  • Review budget exhausted: no
  • Maintainer decision required: no convergence decision required
  • Next state: gator:blocked
  • Blocked reason: workflow_approval_required

@purp purp added gator:blocked Gator is blocked by process or repository gates and removed gator:in-review Gator is reviewing or awaiting PR review feedback labels Oct 8, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mrunalp mrunalp Awaiting requested review from mrunalp mrunalp is a code owner
@sjenning sjenning Awaiting requested review from sjenning sjenning is a code owner
@derekwaynecarr derekwaynecarr Awaiting requested review from derekwaynecarr derekwaynecarr is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

gator:blocked Gator is blocked by process or repository gates test:e2e Requires end-to-end coverage

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

bug: SSH forwarding drops HTTPS behind TLS termination

2 participants

@danehans @purp