openwrt
crowdsec-firewall-bouncer: add `retry_initial_connect` option support
Maintainer: Gerald Kerma @erdoukki Compile tested: n/a Run tested: 23.05.2 r23630-842932a63d, x86_64, I made those changes on my device and all config settings were correctly parsed and used by crowdsec-firewall-bouncer
Description:
This PR introduces support for retry_initial_connect config option in crowdsec-firewall-bouncer. The .initd file simply sources the option from package config and adds it to the generated config.yaml that is used by the bouncer.
In some cases Crowdsec firewall bouncer is running before Crowdsec engine finished initialization and LAIP started. In such case the bouncer will fatal, even though everything is configured correctly. In OpenWRT this issue can arise for example when engine and bouncer are running on the same device - during startup bouncer starts (and fatals) before LAPI is active.
Crowdsec firewall bouncer developers mitigated this problem by adding retry functionality to the bouncer and introducing the retry_initial_connect option in config. Both changes are present in v0.0.13 used in OpenWRT.
However, this option right now cannot be used in crowdsec-firewall-bouncer package because the initd script overwrites config.yaml every time the service is started (to account for changes made with UCI). Proposed changes make the service use retry_initial_connect option and allow user to configure it from uci.
I have also added a short comment about the use of this option in config file, as I have no idea how to change OpenWRT documentation and because this option is not straightforward to wind in Crowdsec documentation.
Thank you for your contribution. @erdoukki is the official maintainer but I'm not sure that he is still doing this. I would take the role but don't know how. ;)
The only issue I see is that the PR includes three commits. Is is best practice for OpenWrt to update commits with --amend so that there is a single commit in the PR (learned it myself).
I also believe that we can set the default to 1. As far as I understood this mitigates problems when the bouncer is starting before the local api is avaliable which does not only apply to lapi running on the same device but also if the lapi runs on another device that is not ready at startup of the router.
Thank you for your answer.
The only issue I see is that the PR includes three commits. Is is best practice for OpenWrt to update commits with --amend so that there is a single commit in the PR (learned it myself).
Okay, my bad, I will fix that.
As far as I understood this mitigates problems when the bouncer is starting before the local api is avaliable which does not only apply to lapi running on the same device but also if the lapi runs on another device that is not ready at startup of the router.
That's exactly the case, eg. after power loss when router starts before server running the engine.
I also believe that we can set the default to 1.
I think that's good idea as well, but I wasn't sure how more experienced people than me would feel about (theoretically) changing the default package behaviour.
Hi @Quba1 I see two more points:
- The commit message is empty. There is a few formal rules to follow for OpenWrt. You may look for my last commit messages. Also, start the commit and PR title with the package name. Signed-off-by should be last line in commit message (you may also add a Co-authored-by for me) ;).
- Please update the Makefile as well by increasing PKG_RELEASE. This is needed to get a new package built.
Hi @ne20002!
I think the latest changes address all your points.
If I can, I would like to suggest that in future you could add your points as a new PR comment instead of editing, because this way I will receive email notification about PR activity.
Thanks a lot for your work on this PR.
Hi @Quba1 I believe this PR should be closed. The changes have been included in the latest version update PR.