edge, supernode: allow -t 0 to disable the management UDP port#1242
Open
larsborn wants to merge 5 commits into
Open
edge, supernode: allow -t 0 to disable the management UDP port#1242larsborn wants to merge 5 commits into
larsborn wants to merge 5 commits into
Conversation
Member
|
Many tests are failing, please fix them first |
Author
|
Happy to, but if I'm reading this correctly, those tests would fail even without my changes: Should I just push the fix to this MR here or would you rather have a separate one and then we rebase this one? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Passing
-t 0toedge(default mgmt port 5644) orsupernode(default 5645) now skips opening the management UDP socket entirely. The socket field stays at its-1init value and theselect()-loopFD_SET/FD_ISSETcalls are guarded with>= 0.Motivation: operators who don't use the JSON-over-UDP management API and want one fewer loopback socket bound (e.g. for hardening) had no way to suppress it. The mgmt port also exposes a small UDP parser and request dispatcher that some deployments would rather not have running.
Changes
src/supernode.c,src/sn_utils.c— gate the mgmt-socket open and the loop'sFD_SET/FD_ISSETcalls; remove the misleadingbad management port format, defaulting to %uwarning that fired on-t 0(it never actually restored the default — the// default is made sure in sn_init()comment was wrong;sn_initruns before CLI parsing).src/edge.c,src/edge_utils.c— same gating for the edge.supernode.1,edge.8— one-line note under the-tentry: "A value of 0 disables the management API entirely (no UDP socket is bound)."-tin both binaries appended `; 0 disables the management API`.The existing `MAX(...)` expressions in both `select()` loops absorb a `-1` correctly because the macro is a signed comparison, so they're left untouched.
No tests added. The existing `tests-*` unit suite doesn't cover socket lifecycle. The shell-based integration harness exercises the management API on default ports; extending it to cover the disabled case would require spinning a second daemon instance. Happy to add either on request.