Samba: Add configuration option to disable WSDD - #4738
Conversation
Samba: add wsdd config option
Samba: add changelog entry for 12.9.0
Samba: document wsdd option
Samba: gate wsdd service behind config option
Samba: add UI strings for wsdd option
There was a problem hiding this comment.
It seems you haven't yet signed a CLA. Please do so here.
Once you do that we will be able to review and accept this pull request.
Thanks!
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughSamba 12.9.0 adds a ChangesSamba network discovery
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
What is the use case here? Is it a problem when it's discoverable on the network? The amount of options gets a bit out of hand. And I guess once we support mDNS (e.g. via #4742), yet another flag needs to be introduced 😢 . |
|
The primary use case is preventing the host from advertising itself on networks you don't fully control, where you'd rather it not broadcast itself to every device on the LAN. It's also restoring previous behavior rather than introducing a new feature. Before 12.7.0, the add-on didn't advertise itself this way, so servers that were intentionally hidden suddenly became discoverable with no way to disable it. That's the issue being addressed in #4722. I also agree that adding a separate boolean for every discovery protocol doesn't scale, NetBIOS today, WSDD now, mDNS next. Would a single option, something like discoverable (defaulting to true), make more sense? It could control WSDD now and the upcoming mDNS work in #4742, rather than introducing a new flag for each protocol. NetBIOS is a bit of an exception since it already shipped as a standalone option in 12.8.0, but newer discovery mechanisms could share a common switch. I'm happy to rework the PR in that direction if you think that's the better approach. Just let me know your preference. |
I also didn't understand the use case for disabling discovery. The reason I added a flag for mDNS in #4742 is I had taken note of #4722 and decided to just do the work to give the user a choice. I think NetBIOS needs separate control as (like SMB1) it's now considered insecure and should not be used unless absolutely needed. As far as bundling WSDD and mDNS into a single option I'm neutral either way. |
NetBIOS has an option since #4705. Ultimately we also want to flip the switch to default off, I intend to make a breaking change to the Samba app, I started to collect them in #4750. |
Co-authored-by: Stefan Agner <stefan@agner.ch>
samba: rename option to network_discovery
samba: gate service on network_discovery
samba: update docs for network_discovery
I renamed to |
Fixes #4722
WSDD has run unconditionally since 12.7.0, so the host is advertised in the
Windows Network view with no way to opt out.
compatibility_mode: falseandnetbios: falsehave no effect on it, sinceenable-check.shonly gatesnmbd.This adds a
wsddboolean option following the exact pattern of thenetbiosoption added in 12.8.0: the flag is checked inenable-check.shand the service file is removed from
user/contents.dwhen disabled.Defaults to
true, so existing installations see no change in behaviour.Disabling it does not affect share availability, only discovery.
Includes the schema entry, DOCS.md documentation, the
en.yamlUI strings,a version bump to 12.9.0, and a changelog entry.
Summary by CodeRabbit
New Features
network_discoveryconfiguration option to control WSDD-based advertising for Windows File Explorer.Documentation
network_discovery.