Repository navigation
Conversation
License Check Results🚀 The license check job ran with the Bazel command: bazel run --lockfile_mode=error //:license-checkStatus: Click to expand output |
|
The created documentation from the pull request is available at: docu-html |
|
please come to our FT meeting to discuss |
|
@arsibo Sorry for the late reply. Joining meeting is not the most practical thing for me. Is there a possibility/place where we could have this discussion asynchronously? |
|
@anmittag I'm a little confused about this MR being assigned to me. Isn't the assignee supposed to do something with this MR? |
|
Well @pypingou you have started a pr. I am expecting a pr author to follow up or accompany the pr till closure. |
|
@anmittag ok, for me the assignee would be more the reviewer rather than the author, but ok that's fine. I'm unclear what the next steps are though. I've asked if there is an asynchronous alternative to attending the logging meeting and so far without answer. Let's just discuss in this PR first and resort to a meeting only if we really need a higher-bandwidth mode of communication? Is there anything in this PR that is not clear to you? |
|
@pypingou no reviewers are mentioned separately |
56ac4e7 to
dcd86ee
Compare
rmaddikery
left a comment
There was a problem hiding this comment.
A review will continue after the checklist has been introduced for code changes. Additionally such a change would break the regular copybara exports, I would suggesest to wait until we have completed the migration (planned in score v0.10). If this is urgent please join the CFT and we will try to find an alternative.
dcd86ee to
18e2ced
Compare
Tried to address this in commit 1d65e83d. I added an integration test that requires a non-root test process, resolves the configured file-socket path and mode, and starts the real UnixDomainServer. It verifies socket creation with 0660 permissions, client connectivity, stale-socket cleanup on restart, and refusal to unlink a regular file. The test passes normally and under TSan and ASan/UBSan/LSan. |
6b473ce to
fef8cf4
Compare
| }; | ||
|
|
||
| const UnixDomainSockAddr addr(score::logging::config::kSocketAddress, true); | ||
| const UnixDomainSockAddr addr = score::logging::config::CreateSocketAddress(score::os::Stdlib::instance()); |
There was a problem hiding this comment.
Please use an instantiated Stdlib and not the instance() - its safer since you offload the lifetime handling to the language
| if (!unlink_ret.has_value()) | ||
| const char* path = static_cast<const char*>(addr.addr.sun_path); | ||
| score::os::StatBuffer st{}; | ||
| if (score::os::Stat::instance().stat(path, st).has_value()) |
There was a problem hiding this comment.
| if (!addr.IsAbstract()) | ||
| { | ||
| using Mode = score::os::Stat::Mode; | ||
| constexpr auto kSocketPerms = Mode::kReadUser | Mode::kWriteUser | Mode::kReadGroup | Mode::kWriteGroup; |
There was a problem hiding this comment.
Why should the group have write permissions?
There was a problem hiding this comment.
non-root clients sharing the socket GID would need write permission to connect. Do you want me to drop it?
fef8cf4 to
b8d578a
Compare
b8d578a to
fd17082
Compare
Make the datarouter Unix domain socket configurable via environment variables while maintaining backward compatibility (defaults to abstract namespace with "datarouter_socket" path). This enables containerized deployments where both the datarouter and applications run in separate containers and communicate via file-based sockets with shared volume mounts. Also supports running multiple datarouter instances with unique abstract namespace sockets. Changes: - Add socket_config module for centralized socket configuration - Support DATAROUTER_SOCKET_PATH and DATAROUTER_SOCKET_MODE env vars - Use score::os::Stdlib for env access (mockable via StdlibMock in tests) - Add S_ISSOCK check before unlinking to prevent removing non-socket files - Set restrictive permissions (0660) on file-based sockets after bind - Expose kMaxPathLength/kMaxAbstractPathLength from UnixDomainSockAddr - Platform-specific defaults: Linux uses abstract, QNX uses file-based Environment variables: - DATAROUTER_SOCKET_PATH: Socket path (default: "datarouter_socket") - DATAROUTER_SOCKET_MODE: "abstract" or "file" (default: "abstract") Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
fd17082 to
9bd797e
Compare

Make the datarouter Unix domain socket configurable via environment variables while maintaining backward compatibility (defaults to abstract namespace with "datarouter_socket" path).
This enables containerized deployments where both the datarouter and applications run in separate containers and communicate via file-based sockets with shared volume mounts. Also supports running multiple datarouter instances with unique abstract namespace sockets.
Changes:
Environment variables:
Example usage for containerized datarouter + application:
Both containers set:
export DATAROUTER_SOCKET_PATH=/var/run/datarouter.sock
export DATAROUTER_SOCKET_MODE=file
And mount the same volume at /var/run
Notes for Reviewer
Pre-Review Checklist for the PR Author
Checklist for the PR Reviewer
Post-review Checklist for the PR Author
References
Closes #