Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
dtugend opened this issue Jan 23, 2024 · 3 comments
Closed

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

dtugend opened this issue Jan 23, 2024 · 3 comments
Assignees
Milestone

Comments

@dtugend
Copy link

dtugend commented Jan 23, 2024

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

  • Froxlor version: 2.1.4
  • Web server: apache2
  • DNS server: Bind9
  • POP/IMAP server: Dovecot
  • SMTP server: postfix
  • FTP server: proftpd
  • OS/Version: Debian 12 (fully updated with sury.org PHP)

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
Copy link
Member

d00p commented Jan 23, 2024

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
Copy link
Author

dtugend commented Jan 23, 2024

Hi. Thank you very much for your reply.

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

  • missing newline at the end of the copy & paste
  • white space being different from my whites pace (had to use --ignore-whitespace)

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
Copy link
Member

d00p commented Jan 23, 2024

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

@d00p d00p closed this as completed in a7ee5e0 Jan 24, 2024
@d00p d00p added this to the 2.2.x milestone Jan 24, 2024
@d00p d00p self-assigned this Jan 24, 2024
d00p added a commit that referenced this issue Jan 26, 2024
…ed; fixes #1230

Signed-off-by: Michael Kaufmann <d00p@froxlor.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Status: Done
Development

No branches or pull requests

2 participants