-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Implement publishing API UNIX socket on Windows platforms #23409
Conversation
I can't find any tests for sockets or pipes in e2e tests of the machine, so, I can't extend these tests for additional verification. There is limited value trying to test gvproxy or win-sshproxy command builders as it would be close to testing getters/setters. |
57c22a2
to
b4b807b
Compare
@l0rd any opinions on this? |
I am fine with the original issue. I think that's useful. Now a few considerations:
|
I will work on this. If you someone can share a good example of a feature toggle I can use as inspiration this would speed up things definitely.
I will try to sketch some tests and then they could be improved during review process. Having this accepted as an idea for a feature is good enough for me now to continue improving this changeset. |
b4b807b
to
0bf51cc
Compare
Added tests for the API. Client for Unix sockets and Named Pipes. Curl for Unix sockets. To not skip tests of Client on Windows it is needed to have a fix in go-connections utilities first docker/go-connections#116 Will work on the config changes for the feature toggle. |
gvproxy and win-sshproxy have capabilities to serve this type of enpoint. This change only adds one additional API enpoint publishing by appending proxy command lines. Signed-off-by: Arthur Sengileyev <[email protected]>
0bf51cc
to
1732338
Compare
@arixmkii thank you for updating the PR. I didn't had the chance to review it yet but I plan to do it tomorrow. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was able to manually test the Unix socket on Windows and the tests looks good (I don't think docker/go-connections#116 is a blocker). Looking forward for the config toggle.
code changes LGTM ... @l0rd are you good with merging this? |
There is no way to disable the exposure of the Unix API socket yet but in the current state the PR already adds some value so +1 for me to merge if @arixmkii is ok. |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: arixmkii, l0rd The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Fixes #23408
gvproxy and win-sshproxy have capabilities to serve this type of enpoint. This change only adds one additional API enpoint publishing by appending proxy command lines.
Originally developed within #13006 but could be generalized to any VM using
gvproxy
orwin-sshproxy
. This has been verified to work for Hyper-V and WSL machines.Example how it looks for WSL machine.
commands
Does this PR introduce a user-facing change?