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

Create a shorter symlink for Unix socket in mountoptions package #346

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

unexge
Copy link
Contributor

@unexge unexge commented Jan 16, 2025

Go returns invalid argument if you try to net.{Dial, Listen} on a Unix socket if the path is longer than 108 characters.

This limitation is only to the argument passed to those functions, if we receive a Unix socket path longer than 108 characters, we create a symlink for that path in temporary dir for that platform and use that path in network operations to work around this limitation.

We were just omitting a warning before, but it'd be hard to work around this limitation on the caller side, so that warning wouldn't be much actionable.


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Go returns `invalid argument` if you try to `net.{Dial, Listen}` on a
Unix socket if the path is longer than 108 characters.

This limitation is only to the argument passed to those functions, if
we receive a Unix socket path longer than 108 characters, we create a
symlink for that path in tempdir for that platform and use that path
in network operations to workaround this limitation.

Signed-off-by: Burak Varlı <[email protected]>
@unexge unexge requested a review from a team as a code owner January 16, 2025 17:03
@unexge unexge enabled auto-merge January 17, 2025 11:24
@muddyfish
Copy link
Contributor

Can we add a test for this?

@unexge
Copy link
Contributor Author

unexge commented Jan 17, 2025

Originally there were tests for this functionality in PodMounter, but now since this is a separate change I think it's fair to add tests here as well.

@unexge
Copy link
Contributor Author

unexge commented Jan 17, 2025

Added tests with 8f044d4

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.

2 participants