openwisp / openwisp-config

OpenWRT configuration agent for OpenWISP Controller
https://openwisp.io/docs/dev/openwrt-config-agent/
GNU General Public License v3.0
374 stars 59 forks source link

[change] Possible conflicts with hotplug events #185

Closed nemesifier closed 1 year ago

nemesifier commented 2 years ago

While reviewing https://github.com/openwisp/openwrt-openwisp-monitoring/pull/104 something has dawned on me: it seems possible to me that any other OpenWrt package may use the same hotplug event names as we do (eg: restart), in that case hotplug listener scripts would yield unpredictable results, which sounds like something very undesirable to me!

If that's the case, we should surely change the names of the hotplug events to be prefixed with openwisp-config, eg:

Since this feature hasn't been included in any tagged release yet and has been available only in the development version, I believe we shouldn't be concerned with backward compatibility because most likely very few users are using this.

CC: @okraits.

devkapilbansal commented 2 years ago

@nemesisdesign I don't think there is a conflict. Notice below that the call mentions 'openwisp'. That ensures only scripts in openwisp folder are executed. https://github.com/openwisp/openwisp-config/blob/5798e714fb4229a757cee5cbc006a7139308a9f2/openwisp-config/files/openwisp.agent#L333

Even if the event name is same, say restart, why the event will point to openwisp?

nemesifier commented 2 years ago

@nemesisdesign I don't think there is a conflict. Notice below that the call mentions 'openwisp'. That ensures only scripts in openwisp folder are executed.

https://github.com/openwisp/openwisp-config/blob/5798e714fb4229a757cee5cbc006a7139308a9f2/openwisp-config/files/openwisp.agent#L333

Even if the event name is same, say restart, why the event will point to openwisp?

Ah ok, so that should be fine! I will triple check before closing, thanks! :blush:

okraits commented 2 years ago

As @devkapilbansal said, /sbin/hotplug-call has openwisp as argument and thus only affects scripts in the the openwisp subdirectory.