Commit e1ccdbb
refactor: replace worker subprocesses with goroutines (#88)
* refactor: replace worker subprocesses with goroutines
Major architectural refactor to improve performance and simplify codebase:
Performance improvements:
- Remove IPC overhead between SPOA and workers (~1200 lines deleted)
- Direct shared memory access via pointers instead of socket communication
- Eliminate JSON encoding/decoding per request
- Reduce latency and CPU usage
Architecture changes:
- New goroutine-based worker manager (internal/worker/manager.go)
- SPOA workers now share dataset, host manager, and geo database via pointers
- Removed worker API client/server and all IPC infrastructure
- Metrics now tracked directly in SPOA handlers (upstream proxy compatible)
Security improvements:
- Systemd service runs as crowdsec-spoa user/group (privilege separation)
- Admin socket now optional and disabled by default
- SO_PEERCRED verification for root-only admin access
- Security hardening in systemd unit files
Bug fixes:
- Fix ServeUnix using wrong listener (ListenAddr vs ListenSocket)
- Fix worker shutdown timeout (5ns -> 5s)
- Fix potential metrics double-counting for upstream proxy mode
Testing:
- Comprehensive test suite for new worker manager
- All existing tests passing
- Build verified successful
Packaging:
- Added optional admin socket systemd unit
- Updated Debian and RPM packaging
- Docker container runs as root (standard practice)
Breaking changes: None (configuration compatible)
* fix: address golangci-lint issues
- Fix containedctx: Add nolint comment for errgroup context (legitimate use case)
- Fix deferInLoop: Wrap defer cancel() in anonymous function to properly scope
- Fix hugeParam: Pass SpoaConfig by pointer instead of value (80 bytes)
- Fix testifylint: Use assert.Empty instead of assert.Len for empty check
- Fix testifylint: Use require.NoError for error assertions
All tests passing, build verified.
* fix: add admin socket installation to install/uninstall scripts
- Add ADMIN_SOCKET and SYSTEMD_ADMIN_SOCKET_FILE variables to _bouncer.sh
- Install optional admin socket unit in install.sh (disabled by default)
- Add installation instructions for optional admin socket
- Update uninstall.sh to stop, disable, and remove admin socket unit
- Fix documentation URL to include trailing slash
- Add systemctl daemon-reload to uninstall.sh
The admin socket is installed but remains disabled by default, users must
manually enable it with: systemctl enable --now crowdsec-spoa-bouncer-admin.socket
* fix: use dedicated logging directory with proper permissions
- Change log_dir from /var/log/ to /var/log/crowdsec-spoa/
- Add LogsDirectory=crowdsec-spoa to systemd service (auto-creates with correct ownership)
- Set LogsDirectoryMode=0750 for security
- Update uninstall script to clean up both legacy and new log locations
This fixes 'read-only file system' errors when using ProtectSystem=strict.
Systemd automatically creates /var/log/crowdsec-spoa/ owned by crowdsec-spoa:crowdsec-spoa.
For existing installations, users need to update log_dir in their config to the new path.
Migration instructions will be provided in the release changelog.
* fix: config file permissions for unprivileged service user
- Change config file permissions from 0600 to 0640 (owner rw, group r)
- Set config file group to crowdsec-spoa for read access
- Ensure crowdsec-spoa group is created in all install paths:
* Manual install script (groupadd/addgroup fallback)
* RPM %post (groupadd --system)
* Debian postinst (already creates group)
- Update packaging scripts to set group ownership:
* install.sh: -g crowdsec-spoa flag
* _bouncer.sh: set_config_var_value() with -g crowdsec-spoa
* RPM spec: chgrp in %post
* Debian postinst: chgrp after user creation
* debian/rules: chmod 0640
* RPM spec: install -m 640
This fixes permission denied errors when the systemd service (running as
crowdsec-spoa user) tries to read the config file owned by root.
* fix: add Service directive to admin socket unit
- Add Service=crowdsec-spoa-bouncer.service to admin socket unit
- This tells systemd to activate the main bouncer service when admin socket receives connection
Without this directive, systemd looks for crowdsec-spoa-bouncer-admin.service which doesn't exist,
causing 'Socket service not loaded' error when enabling the admin socket.
* fix: admin socket systemd unit configuration
- Remove Service= directive (causes conflict when service already running)
- Add PartOf/Before to make socket part of the service lifecycle
- Socket fd is passed to service on startup via systemd activation
- Update instructions: enable socket, then restart service (not --now)
The admin socket uses systemd socket activation but the service must be restarted
to receive the socket file descriptor. This prevents 'already active, refusing' errors.
Correct activation flow:
1. Uncomment admin_socket in config
2. systemctl enable crowdsec-spoa-bouncer-admin.socket
3. systemctl restart crowdsec-spoa-bouncer.service
* fix: ensure systemd socket activation works for admin socket
- Add After=crowdsec-spoa-bouncer-admin.socket to service unit
- Service now waits for admin socket if it's enabled
- If socket unit is enabled, systemd creates it as root:root
- If socket unit is disabled, service creates it as crowdsec-spoa:crowdsec-spoa
This ensures proper socket ownership (root:root) when using systemd socket activation,
while maintaining fallback for non-systemd environments.
To verify:
systemctl stop crowdsec-spoa-bouncer
systemctl enable crowdsec-spoa-bouncer-admin.socket
systemctl start crowdsec-spoa-bouncer-admin.socket
systemctl start crowdsec-spoa-bouncer
ls -la /run/crowdsec-spoa/admin.sock # Should show root:root
* fix: properly configure optional systemd socket activation
Socket unit configuration:
- Service=crowdsec-spoa-bouncer.service (passes fd to correct service)
- FileDescriptorName=admin (identifies the fd)
- PartOf/Before for lifecycle management
Service unit configuration:
- After=crowdsec-spoa-bouncer-admin.socket (waits if socket enabled)
- NO Wants= directive (keeps socket truly optional)
The socket is only activated if explicitly enabled by user:
systemctl enable crowdsec-spoa-bouncer-admin.socket
systemctl restart crowdsec-spoa-bouncer
If not enabled, service creates socket itself as fallback (crowdsec-spoa:crowdsec-spoa)
* refactor: consolidate admin API and fix logger inheritance
- Merge pkg/server + internal/api -> internal/admin (single package)
- Remove GOB encoding, use simple string-based protocol
- Remove unused GetHostUnsetCookie command (worker API only)
- Fix logger inheritance: cmd/root.go -> manager -> workers
- Simplify types: keep only used error codes and response types
- Net reduction: -602 lines of code
This completes the worker-to-goroutine refactor by cleaning up
the admin interface and ensuring proper logger propagation.
* refactor: simplify admin server to use single listener
The admin server only ever uses one listener (either from systemd
socket activation or manually created), so there's no need for a
slice. This simplifies the code and makes the logic clearer.
* fix: make admin socket truly optional with proper systemd integration
Changes to socket unit:
- Remove Service= directive (service runs independently, not socket-activated)
- Remove PartOf= directive (socket is independent/optional)
- Remove FileDescriptorName= (not needed for basic activation)
- Keep Before= to ensure socket is ready if enabled
- Add usage instructions in comments
Changes to service unit:
- Add Sockets= directive to explicitly request admin socket FD if available
- Note: Sockets= is a hint, not a hard dependency - service starts fine without it
- Remove admin socket from After= (it's optional, shouldn't block startup)
How it works:
1. By default, socket is disabled - service runs without admin interface
2. To enable: systemctl enable --now crowdsec-spoa-bouncer-admin.socket
3. When service starts/restarts, systemd passes the socket FD via LISTEN_FDS
4. Service detects FD and uses it for root-only admin access
* fix: add Service directive back to socket unit
Without Service=, systemd looks for crowdsec-spoa-bouncer-admin.service
which doesn't exist. Need to explicitly specify which service the socket
is for: crowdsec-spoa-bouncer.service
The socket is still optional - Service= just tells systemd which service
to pass the FD to when the socket is enabled.
* fix: remove Sockets= to make admin socket truly optional
The Sockets= directive was causing the socket to auto-start when the
service started, even when the socket unit was disabled. This made it
impossible to truly disable the admin socket.
Removed Sockets= from service unit. Now the socket is only active if
explicitly enabled by the administrator.
How it works now:
- Socket disabled (default): No admin interface
- Socket enabled: systemd passes FD to service via LISTEN_FDS when service starts
- To enable: systemctl enable --now crowdsec-spoa-bouncer-admin.socket
- To disable: systemctl stop + disable the socket unit
* docs: clarify admin socket configuration for Docker vs systemd
Updated the config file comments to make it clear:
- Docker users: Uncomment admin_socket line in config
- Systemd users: Do NOT uncomment config, use the socket unit instead
This prevents confusion where systemd users might uncomment the config
option, which would create the socket as crowdsec-spoa user instead of
root, defeating the security model.
* fix: preserve runtime directory across service restarts
Add RuntimeDirectoryPreserve=yes to systemd service unit.
Without this, /run/crowdsec-spoa/ is removed when the service stops
and recreated with default ownership (crowdsec-spoa:crowdsec-spoa)
when it starts.
With this setting, the directory persists across restarts, allowing
administrators to set custom group ownership that survives restarts:
chgrp haproxy /run/crowdsec-spoa/
systemctl restart crowdsec-spoa-bouncer
The directory group is preserved, and Unix sockets created within it
will inherit the group ownership (with 0660 permissions).
* fix: use setgid directory for socket group inheritance
Non-root process can't chown() to change socket group. Use setgid
directory instead so sockets automatically inherit directory group.
Changes:
- Remove chown() call (was failing with 'operation not permitted')
- Set RuntimeDirectoryMode=2750 (setgid bit + 0750 permissions)
- Sockets inherit directory group, get 0660 permissions from umask
Usage: chgrp haproxy /run/crowdsec-spoa && systemctl restart
Result: sockets are crowdsec-spoa:haproxy with 0660 permissions
* fix: admin server logging to inherit root logger and log level
- Remove Logger field from admin.Config (not needed)
- Use log.WithField() directly to inherit from standard logger
- Admin server now respects log level configuration like other components
* test: use random ports in worker tests to avoid conflicts
- Add getFreePort helper function for dynamic port allocation
- Update all TCP worker tests to use random available ports
- Makes tests more robust and prevents port conflicts
* fix: SPOA logging to use inherited logger instead of global logger
- Replace all log. calls in SPOA with s.logger. calls
- Fixed handleHTTPRequest and handleIPRequest functions
- Ensures worker-specific logging with proper context
- All log messages now include worker name for better debugging
* fix: filter out net.ErrClosed errors during graceful server shutdown
Prevent spurious 'use of closed network connection' errors from being logged
when SPOA TCP and Unix servers shut down gracefully. Use errors.Is() to check
for net.ErrClosed and treat it as a normal shutdown rather than an error.
* refactor: unify TCP and Unix server handling into single Serve method
Replace separate ServeTCP and ServeUnix methods with a single Serve method
that handles both TCP and Unix socket serving using a shared error channel
and helper function, following the same pattern as CrowdSec's APIServer.
- Remove duplicate logging by using Spoa logger with worker context
- Simplify worker manager to launch single goroutine per worker
- Maintain same error filtering for net.ErrClosed during shutdown
* chore: remove accidentally committed binary (crowdsec-spoa)
* chore(lint): fix revive issues on workers-to-goroutines
- manager_test: check TCPAddr type assertion
- spoa: remove useless break in Serve switch
- spoa: add temporary nolint for handleHTTPRequest length (to be split with AppSec)
* test(worker): make TCPAddr assertion safe in tests
* chore: remove accidentally committed binary (crowdsec-spoa) from workers-to-goroutines
* refactor: single spoa listener (#98)
* refactor: simplify SPOA shutdown logic
- Remove unnecessary goroutine for context cancellation
- Always call Shutdown() after errgroup completes
- Ensure graceful shutdown even on unexpected errors
* refactor: remove worker manager files
* refactor(spoa): extract captcha flow, add HTTPRequestData for AppSec
- Move captcha remediation logic to handleCaptchaRemediation
- Return parsed URL/Method/Headers/Body to avoid reparsing later
- Add parseHTTPData helper and wire through allow/ban paths
- Cleanup logging and use matchedHost.Host instead of hoststring
- Keep behavior for metrics and remediation unchanged
* refactor: remove worker-only settings and remnants
- Drop SpoaConfig.LogLevel and level override in spoa
- Remove internal/worker package and tests
- Ensure build/lint/tests pass under single SPOA architecture
* fix: captcha logic and reduce calls to session when not needed
* Refactor IP remediation to use netip
* Enforce listener requirement in config
* Run go mod tidy after merge
* Remove admin socket functionality (#102)
- Remove internal/admin package and all admin socket code
- Remove AdminSocket configuration option
- Remove admin socket systemd unit file
- Remove admin socket references from scripts and Dockerfile
- Clean up configuration comments
* Run go mod tidy
* Run Docker container as crowdsec-spoa user
* Remove socat from Dockerfile
* Fix origin variable initialization in metrics counting
- Initialize origin when checking IP remediation for metrics
- Only count metrics when IP extraction succeeds
- Remove unused checkIPRemediation function
* Update debian/postinst
Co-authored-by: Copilot <[email protected]>
* Remove unnecessary directory creation from Docker startup script
Directory is already created in Dockerfile with proper permissions before user switch
* Update comment to clarify setgid bit handling
systemd RuntimeDirectoryMode=2750 already sets setgid bit,
manual chmod only needed for non-systemd deployments
* Set config file permissions to 0640 and group ownership
- Change config file permissions from 0600 to 0640 (allow group read)
- Set group ownership to crowdsec-spoa in RPM post-install script
- Match Debian and RPM packaging for consistent permissions
---------
Co-authored-by: Copilot <[email protected]>1 parent 742df38 commit e1ccdbb
File tree
25 files changed
+653
-2981
lines changed- cmd
- config
- debian
- internal
- api
- messages
- perms
- types
- worker
- pkg
- cfg
- dataset
- metrics
- server
- spoa
- rpm/SPECS
- scripts
25 files changed
+653
-2981
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | | - | |
| 1 | + | |
2 | 2 | | |
3 | 3 | | |
4 | 4 | | |
| |||
14 | 14 | | |
15 | 15 | | |
16 | 16 | | |
17 | | - | |
18 | | - | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
19 | 21 | | |
20 | 22 | | |
21 | 23 | | |
| |||
37 | 39 | | |
38 | 40 | | |
39 | 41 | | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
40 | 45 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2 | 2 | | |
3 | 3 | | |
4 | 4 | | |
5 | | - | |
6 | 5 | | |
7 | 6 | | |
8 | 7 | | |
| |||
19 | 18 | | |
20 | 19 | | |
21 | 20 | | |
22 | | - | |
23 | | - | |
24 | 21 | | |
25 | 22 | | |
26 | 23 | | |
27 | 24 | | |
28 | | - | |
29 | 25 | | |
30 | 26 | | |
31 | 27 | | |
| |||
63 | 59 | | |
64 | 60 | | |
65 | 61 | | |
66 | | - | |
67 | | - | |
68 | | - | |
69 | 62 | | |
70 | 63 | | |
71 | 64 | | |
| |||
74 | 67 | | |
75 | 68 | | |
76 | 69 | | |
77 | | - | |
78 | | - | |
79 | | - | |
80 | | - | |
81 | | - | |
82 | | - | |
83 | | - | |
84 | | - | |
85 | | - | |
86 | | - | |
87 | | - | |
88 | | - | |
89 | | - | |
90 | 70 | | |
91 | 71 | | |
92 | 72 | | |
| |||
158 | 138 | | |
159 | 139 | | |
160 | 140 | | |
161 | | - | |
| 141 | + | |
162 | 142 | | |
163 | 143 | | |
164 | 144 | | |
| |||
220 | 200 | | |
221 | 201 | | |
222 | 202 | | |
223 | | - | |
224 | | - | |
225 | | - | |
226 | | - | |
227 | | - | |
228 | | - | |
229 | | - | |
230 | | - | |
231 | | - | |
232 | | - | |
233 | | - | |
234 | | - | |
235 | | - | |
236 | | - | |
237 | | - | |
238 | | - | |
239 | | - | |
240 | | - | |
241 | | - | |
242 | | - | |
243 | | - | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
244 | 214 | | |
245 | 215 | | |
246 | | - | |
247 | | - | |
248 | | - | |
249 | | - | |
250 | | - | |
251 | | - | |
252 | | - | |
253 | | - | |
254 | | - | |
255 | | - | |
256 | | - | |
257 | | - | |
258 | | - | |
259 | | - | |
260 | | - | |
261 | | - | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
262 | 219 | | |
263 | 220 | | |
| 221 | + | |
264 | 222 | | |
265 | | - | |
266 | | - | |
267 | | - | |
268 | | - | |
269 | | - | |
270 | | - | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
271 | 227 | | |
272 | 228 | | |
273 | | - | |
274 | | - | |
275 | | - | |
276 | | - | |
277 | | - | |
278 | | - | |
279 | | - | |
280 | 229 | | |
281 | 230 | | |
282 | | - | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
283 | 236 | | |
284 | 237 | | |
285 | 238 | | |
| 239 | + | |
286 | 240 | | |
287 | 241 | | |
288 | | - | |
289 | | - | |
| 242 | + | |
290 | 243 | | |
291 | 244 | | |
292 | 245 | | |
293 | | - | |
294 | | - | |
295 | | - | |
296 | | - | |
297 | | - | |
298 | | - | |
299 | | - | |
300 | | - | |
301 | | - | |
302 | | - | |
303 | | - | |
304 | | - | |
305 | | - | |
306 | | - | |
307 | | - | |
308 | | - | |
309 | | - | |
310 | | - | |
311 | | - | |
312 | | - | |
313 | | - | |
314 | | - | |
315 | | - | |
316 | | - | |
317 | | - | |
318 | | - | |
319 | | - | |
320 | | - | |
321 | | - | |
322 | | - | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
323 | 250 | | |
324 | | - | |
325 | | - | |
326 | | - | |
327 | | - | |
328 | | - | |
329 | | - | |
330 | | - | |
| 251 | + | |
| 252 | + | |
331 | 253 | | |
332 | 254 | | |
333 | | - | |
334 | | - | |
335 | | - | |
336 | | - | |
337 | | - | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
338 | 258 | | |
339 | 259 | | |
340 | 260 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
| 3 | + | |
3 | 4 | | |
4 | 5 | | |
5 | 6 | | |
6 | 7 | | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
7 | 12 | | |
8 | 13 | | |
9 | | - | |
10 | | - | |
11 | 14 | | |
12 | 15 | | |
13 | 16 | | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
14 | 27 | | |
15 | 28 | | |
16 | 29 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | | - | |
| 3 | + | |
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
| |||
13 | 13 | | |
14 | 14 | | |
15 | 15 | | |
16 | | - | |
17 | | - | |
18 | | - | |
19 | | - | |
20 | | - | |
21 | | - | |
22 | | - | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
23 | 20 | | |
24 | 21 | | |
25 | 22 | | |
26 | | - | |
27 | | - | |
28 | | - | |
29 | | - | |
30 | 23 | | |
31 | 24 | | |
32 | 25 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
22 | 22 | | |
23 | 23 | | |
24 | 24 | | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
25 | 30 | | |
26 | 31 | | |
27 | 32 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
37 | 37 | | |
38 | 38 | | |
39 | 39 | | |
40 | | - | |
| 40 | + | |
41 | 41 | | |
0 commit comments