When reloading, if something is wrong in the geoip module, the module fails to load.

Do you think it's a better idea to restore the old geoip config instead of failed to reload directly? Because if geoip failed, then the zone became totally unavailable. And this has caused a fatal problem in our product environment.

You mean typos in the geoip configuration file? Or some other problems?

Thanks for your reply. My point is that if the geoip configuration is malformed when reloading, like I meaningly wrote a "ABCD" in it:

foo.example.com.:
  - net: 127.0.0.0/24
    ABCD: [ 192.168.0.1 ]
    AAAA: [ 2001:beef::1 ]

Then knot will refuse to load this module, and the zone example.com will be totally unavailable. (which may cause fatal problem) Will you possibly modify the logic so that if the geo module fails, knot will continue to use the old geo config before reloading?

Switching to the previous configuration isn't easy. I'm considering adding geoip config file check to knotc conf-check phase. Let me investigate what is possible...

Ok, thanks a lot. We are currently working on this problem. If there's any progress, we'll touch you.

Hi Daniel, Is is ok to modify the func void conf_activate_modules() to int conf_activate_modules(). If the func fails due to a module then reload fails. And the config is still the one in memory. Will there be any potential risk doing so?

Generally it's not ok as modules can depend on the actual configuration or other components (DNSSEC). I cannot surely say if it's safe for you at the moment, sorry.

Thanks. We'll consider doing some check before reload.

It's the optimal approach. Still I'm considering how to improve the module but it's quite a lot of work :-/

@yfgogogo please try this out https://gitlab.nic.cz/knot/knot-dns/-/commits/geoip-checks It requires a review and more testing.

Also consider https://donations.nic.cz/cs/donate/?project=knot-dns

Thank you Daniel. Will review and test it.

mentioned in merge request !1407 (merged)

closed via merge request !1407 (merged)

mentioned in commit d485f24b

@dsalzman Hey, I ran into a similar problem. When we set '-s sock', reload/zone-reload won't work for this change, due to the geoip config path is loaded from confdb. Maybe we could change the function 'conf_activate_modules' to let it return an error and stop the reloading process. Here's my PR. Looking Forward to your opinion.

https://github.com/CZ-NIC/knot/pull/34

@solidcc2 Hello, thanks for spotting the bug and for the bugfix. As for the second commit, we have think about it more thoroughly.

Thank you for your review. Hope the commit is helpful for this project. :)

@dvasek I understand that it requires more thorough evaluation, and I am happy to provide any additional information or clarification that might be needed. Please let me know if there are any updates or if further discussion is required.