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 TempDir option to UnixSocketConfig #282

Merged
merged 3 commits into from
Sep 21, 2023

Conversation

tomhjp
Copy link
Contributor

@tomhjp tomhjp commented Sep 21, 2023

When running plugins in containers with systemd's PrivateTmp=true setting, the containers are not sub-processes so they are not part of the plugin client's file system namespace - as such /tmp is different for the 2 sides of the connection and they can't establish communication.

As a workaround, TMPDIR can be set to something that is the same on both sides like /home/user/tmp, but that punctures the PrivateTmp setting for all other uses of /tmp as well. This lets us choose a single exception.

Copy link
Contributor

@swenson swenson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

The title of this PR confuses me a little though. TempDir is already a field in UnixSocketConfig? It's now we use the option (instead of ignoring, as it appears that it was unused before this change).

@tomhjp
Copy link
Contributor Author

tomhjp commented Sep 21, 2023

That's a fair comment, the field names were pretty confusing with this addition. TempDir is a new field, and we already tracked directory (now renamed to socketDir to try to improve disambiguation). TempDir is the dir that socketDir will be created in. socketDir is the dir that unix sockets will be created in. We have two layers because we want to both be able to:

  1. Specify a base directory that is visible to both the host and plugin
  2. And use a unique directory per-plugin so that plugins' unix sockets can be made not visible to each other.

I tried to improve the comments and field names a bit in bf6a95e, but lmk if I didn't succeed.

@tomhjp
Copy link
Contributor Author

tomhjp commented Sep 21, 2023

Thanks!

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.

3 participants