-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
listeners: Add support for named socket activation #7243
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
listeners: Add support for named socket activation #7243
Conversation
|
Hmm. I don't know much about socket activation (not something I've ever used/needed) but generally we try to avoid having configuration via env vars. Are you sure that's necessary? Why can't it be in the config itself? |
|
From the issue description:
Without LISTEN_FDNAMES, there's no way to know which FD number corresponds to which socket - the ordering is unpredictable. This follows the same pattern as systemd's standard |
|
Referring to #6792 (comment), it's possible (even likely) to have multiple sockets with the same name by defining those sockets within the same Additionally, the current sock activation pattern (using To fully support named socket activation, we need to be able to express to caddy the "index" of a socket with a given name. The current impl in this PR always returns the first socket with a given name. To allow full utilization of named socket activation, it could be adjusted to provide a way to return the Nth socket with a given name. |
|
I've added full support for indexed named sockets. Now supports: |
|
Ah, so |
|
This is just a matter of personal taste but I think the default syntax could be dropped Intiutively, no number could also be understood as all sockets named "web". I would instead always require a number when an fdname is provided. |
|
I prefer keeping the default for single sockets (fdname/web). In practice, I think socket files define one socket with a given name in most cases. Requiring :0 always would make the simple case unnecessarily verbose. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Simple and narrow change, with tests. Thanks!
a819b73 to
00e68aa
Compare
|
did you see http://github.com/MayCXC/caddy-systemd-socket-activation ? it supports the indices from systemd.socket. for this to go into main, I would like to take your test cases and use the implementation from the extension and add them to the _linux target. are you able to test that the extension works for you? |
This PR implements support for named socket activation in systemd, allowing users to reference sockets by name using
fdname/nameandfdgramname/namesyntax instead of only numericfd/Nandfdgram/Nreferences.Solution Logic
The implementation uses an early normalization approach:
New function
getFdByName()- reads systemd environment variableLISTEN_FDNAMESand maps socket names to file descriptor numbersExtended
ParseNetworkAddressWithDefaults()- detectsfdname/andfdgramname/prefixes and converts them to standardfd/Nandfdgram/Nsyntax internallyBackward compatibility - existing
fd/Nsyntax continues to work unchangedThe conversion happens at parse time, so all downstream code continues to work with the standard fd syntax without modifications.
Testing Results
Unit Tests
Manual Testing
fd/3andfdgram/4syntaxfdname/httpandfdgramname/dnssyntax with environment variablesUsage Examples
Before (numeric references)
After (named references)
Assistance Disclosure
I consulted Claude to understand the project architecture, but I authored/coded the fix myself
Resolves #6792