froxlor / Froxlor

The server administration software for your needs - The official Froxlor development Git repository
http://www.froxlor.org
GNU General Public License v2.0
1.62k stars 455 forks source link

froxlor_bind.conf not updated when last domain is is removed form DNS #1230

Closed dtugend closed 7 months ago

dtugend commented 7 months ago

This probably doesn't happen often to most people, so it might sound like nit-picking, but it already bit me two times.

Describe the bug The froxlor_bind.conf not updated when last domain is is removed form DNS. As a result the DNS keeps serving the last contents of the file. Edit: actually the DNS fails due to serving an invalid config (missing zone files).

System information

To Reproduce Remove the last domain from DNS / nameserver.

Expected behavior An empty froxlor_bind.conf should be created instead, so no domains will be served from that config.

Additional context

Relevant bind9 code: https://github.com/Froxlor/Froxlor/blob/2629718b229fc7ef7518d38d32be5a55c5224086/lib/Froxlor/Cron/Dns/Bind.php#L57-L60

Relevant PowerDNS code (not sure if relevant): https://github.com/Froxlor/Froxlor/blob/2629718b229fc7ef7518d38d32be5a55c5224086/lib/Froxlor/Cron/Dns/PowerDNS.php#L47-L50

I am guessing removing the code snippets above should solve it correctly, but I am not sure, that's why I didn't make a pull-request.

d00p commented 7 months ago

Bind: please test the following patch, whether it resolves the issue

diff --git a/lib/Froxlor/Cron/Dns/Bind.php b/lib/Froxlor/Cron/Dns/Bind.php
index 882b3d99..d7cdbe2a 100644
--- a/lib/Froxlor/Cron/Dns/Bind.php
+++ b/lib/Froxlor/Cron/Dns/Bind.php
@@ -55,18 +55,17 @@ class Bind extends DnsBase
                $domains = $this->getDomainList();

                if (empty($domains)) {
-                       $this->logger->logAction(FroxlorLogger::CRON_ACTION, LOG_INFO, 'No domains found for nameserver-config, skipping...');
-                       return;
-               }
-
-               $this->bindconf_file = '# ' . Settings::Get('system.bindconf_directory') . 'froxlor_bind.conf' . "\n" . '# Created ' . date('d.m.Y H:i') . "\n" . '# Do NOT manually edit this file, all changes will be deleted after the next domain change at the panel.' . "\n\n";
-
-               foreach ($domains as $domain) {
-                       if ($domain['is_child']) {
-                               // domains that are subdomains to other main domains are handled by recursion within walkDomainList()
-                               continue;
+                       $this->logger->logAction(FroxlorLogger::CRON_ACTION, LOG_INFO, 'No domains found for nameserver-config, creating empty file...');
+                       $this->bindconf_file = '';
+               } else {
+                       $this->bindconf_file = '# ' . Settings::Get('system.bindconf_directory') . 'froxlor_bind.conf' . "\n" . '# Created ' . date('d.m.Y H:i') . "\n" . '# Do NOT manually edit this file, all changes will be deleted after the next domain change at the panel.' . "\n\n";
+                       foreach ($domains as $domain) {
+                               if ($domain['is_child']) {
+                                       // domains that are subdomains to other main domains are handled by recursion within walkDomainList()
+                                       continue;
+                               }
+                               $this->walkDomainList($domain, $domains);
                        }
-                       $this->walkDomainList($domain, $domains);
                }

                $bindconf_file_handler = fopen(FileDir::makeCorrectFile(Settings::Get('system.bindconf_directory') . '/froxlor_bind.conf'), 'w');

For PowerDNS, no changes are necessary, as it empties the corresponding database tables prior to regenerate them so if no domains are found, no entries are being added.

dtugend commented 7 months ago

Hi. Thank you very much for your reply.

I had problems applying the patch you sent, because of:

Thus I want to suggest to consider drag & drop the patch files into the issue text input box in the future if possible and an acceptable risk for you.

Otherwise works fine for bind9, tested it.

Regarding PowerDNS, I don't know your code as good as you, but be aware it skips $this->reloadDaemon(); in empty case: https://github.com/Froxlor/Froxlor/blob/2629718b229fc7ef7518d38d32be5a55c5224086/lib/Froxlor/Cron/Dns/PowerDNS.php#L61

d00p commented 7 months ago

Yes you're right, the reload should be done too in the case of empty domains