valkey icon indicating copy to clipboard operation
valkey copied to clipboard

Add support for setting the group on a unix domain socket

Open ayush933 opened this issue 1 year ago • 4 comments

Add new optional, immutable string config called unixsocketgroup. Change the group of the unix socket to unixsocketgroup after bind() if specified.

Manually verified that group is modified as expected and fails on illegal arguments (like non existent group name).

Fixes #873.

ayush933 avatar Aug 12 '24 21:08 ayush933

Hi, @madolson can you please take a look?

ayush933 avatar Aug 12 '24 21:08 ayush933

Codecov Report

Attention: Patch coverage is 68.42105% with 6 lines in your changes missing coverage. Please review.

Project coverage is 70.33%. Comparing base (109cc21) to head (c8aeaae). Report is 37 commits behind head on unstable.

Files Patch % Lines
src/anet.c 57.14% 6 Missing :warning:
Additional details and impacted files
@@             Coverage Diff              @@
##           unstable     #901      +/-   ##
============================================
- Coverage     70.40%   70.33%   -0.07%     
============================================
  Files           112      112              
  Lines         61465    61518      +53     
============================================
- Hits          43275    43271       -4     
- Misses        18190    18247      +57     
Files Coverage Δ
src/config.c 78.69% <ø> (ø)
src/connection.h 86.74% <ø> (ø)
src/server.c 88.55% <100.00%> (-0.02%) :arrow_down:
src/server.h 100.00% <ø> (ø)
src/unix.c 76.31% <100.00%> (+0.31%) :arrow_up:
src/anet.c 74.09% <57.14%> (-1.06%) :arrow_down:

... and 23 files with indirect coverage changes

codecov[bot] avatar Aug 12 '24 22:08 codecov[bot]

Addressed reviews and added UT to test that permissions are set correctly. Not sure which group to use in tests since using the default root group or creating new group would require sudo.

ayush933 avatar Aug 15 '24 22:08 ayush933

@valkey-io/core-team Please vote on this small feature, we can optionally include it in Valkey 8.0 if it's ready for rc2, but not after.

madolson avatar Aug 19 '24 20:08 madolson

Since the new test is a bit odd, running the daily against a bunch of different platforms. https://github.com/valkey-io/valkey/actions/runs/10530596918

madolson avatar Aug 23 '24 18:08 madolson