[RFC] Every service should have an explicit Enable/Disable toggle

Started by doktornotor, September 02, 2024, 12:24:43 PM

Previous topic - Next topic
Willing to post PRs on Github eventually but want some comments first.

The worst offender here being the netflow thing, that is completely confusing with lots of threads about disabling the feature.

Another one that comes to mind is the legacy ntpd stuff. Wanting to disable something temporarily does not mean I want to ruin my configuration by removing all the pools / timeservers and redo it again when finished with testing chrony or whatever.

I'm probably missing some others here, again - please comment.

Disabling NTP was a workaround for a lack of a knob in the first place and introducing a "disableenable" type would have been more work and also not sure where to put it (system general vs. network time settings). Both being static pages doesn't help the cause. If you want to do it why not. Just be aware of repercussions of model migration and enable/disable toggles causing model validations to fail because of cross-reference between enable settings and other (required) fields in the future... like this one https://github.com/opnsense/core/commit/54ccb747cd

NetFlow is certainly a good target but also needs a model migration figuring out the implicit logic first to have users arrive in their correct state WRT the enable flag.

I've always felt that items from Reporting: Settings don't have good visilbilty but given your requirement they are already considered feature complete. Still, they both feel a bit out of sight on that particular page.


Cheers,
Franco

PS: maybe a link to the settings page where to enable/disable would be an interesting addition to the services API?

Quote from: franco on September 02, 2024, 12:40:11 PM
Disabling NTP was a workaround for a lack of a knob in the first place and introducing a "disableenable" type would have been more work and also not sure where to put it (system general vs. network time settings).

The ntpd "logic" is particularly horrible, as noticed recently.  :-X

Quote from: franco on September 02, 2024, 12:40:11 PM
Just be aware of repercussions of model migration and enable/disable toggles causing model validations to fail because of cross-reference between enable settings and other (required) fields in the future... like this one https://github.com/opnsense/core/commit/54ccb747cd

Thanks, will check.

Quote from: franco on September 02, 2024, 12:40:11 PM
PS: maybe a link to the settings page where to enable/disable would be an interesting addition to the services API?

Does not sound bad.

And, a completely off-topic before I forget - the dashboard widgets' headers. Please, link that to appropriate menu items.

> And, a completely off-topic before I forget - the dashboard widgets' headers. Please, link that to appropriate menu items.

Yes, already on Stephan's TODO list. Found out recently while testing the new WOL widget.


Cheers,
Franco