Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for static unix socket forwarding over ssh #58

Merged
merged 2 commits into from
Oct 15, 2021

Conversation

n1hility
Copy link
Member

@n1hility n1hility commented Sep 30, 2021

Implements static Unix socket forward over ssh. The primary use case is forwarding docker API clients that do not support SSH and need to forward traffic to the VM.

This PR adds 4 command line parameters, which can be called out by podman machine to create a single unix socket forward:
--forward-sock, --forward-dest, --forward-user, and --forward-identity

As is the case with the previous podman specific prototype, this implementation supports auto-reconnecting in the case of connection failures and timeouts.

This implementation temporarily utilizes a modified version of the podman ssh client connection code. Ideally, this would be replaced with a future shared vendor module.

While this should function on Windows, properly handling win Docker API clients will require Named Pipe support. I will look at contributing that in a future PR after Mac is fully operational.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 30, 2021

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: n1hility

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@n1hility
Copy link
Member Author

n1hility commented Oct 4, 2021

PR rebased on #61 and #60

@n1hility n1hility marked this pull request as ready for review October 4, 2021 06:29
@guillaumerose
Copy link
Contributor

PTAL @baude @Luap99

@n1hility
Copy link
Member Author

@baude @Luap99 @rhatdan no rush but thought I would check and see if you had any initial feedback

@rhatdan
Copy link
Member

rhatdan commented Oct 12, 2021

Looks like you have to rebase.

@baude
Copy link
Member

baude commented Oct 12, 2021

LGTM

@guillaumerose
Copy link
Contributor

guillaumerose commented Oct 12, 2021

Minor comments for me, lgtm

Sorry for the conflicts..

@n1hility
Copy link
Member Author

@guillaumerose np, I moved it over. I did need to add one method to virtualnetwork (DialContextTCP), so the impact is minimal there. I have not had a chance to repeat manual testing/verification will do that today.

@n1hility
Copy link
Member Author

/hold

@n1hility
Copy link
Member Author

n1hility commented Oct 15, 2021

@guillaumerose just got a few mins to do some extra manual testing after the package change and all looks good.

@n1hility
Copy link
Member Author

/remove-hold

@guillaumerose
Copy link
Contributor

Thanks a lot! lgtm 👍

@guillaumerose guillaumerose merged commit be221fa into containers:main Oct 15, 2021
@rhatdan
Copy link
Member

rhatdan commented Oct 15, 2021

@n1hility what is the next step now.

@matejvasek
Copy link

@n1hility [Have you added | will you add] call to this new functionality in podman machine?

@n1hility
Copy link
Member Author

n1hility commented Jan 18, 2022

@matejvasek yes i am just doing the windows PR now, will next propose a change for mac consuming this pr. I will post an update comment to containers/podman#11462

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants