Added support for CNAME records add/remove #1278

marank · 2020-05-12T20:29:26Z

Signed-off-by: Matthias rank [email protected]

By submitting this pull request, I confirm the following: {please fill any appropriate checkboxes, e.g: [X]}

{Please ensure that your pull request is for the 'devel' branch!}

I have read and understood the contributors guide, as well as this entire template.
I have made only one major change in my proposed changes.
I have commented my proposed changes within the code.
I have tested my proposed changes.
I am willing to help maintain this change if there are issues with it later.
I give this submission freely and claim no ownership.
It is compatible with the EUPL 1.2 license
I have squashed any insignificant commits. (git rebase)
I have Signed Off all commits. (git commit --signoff)

What does this PR aim to accomplish?:
{A detailed description, screenshots (if necessary), as well as links to any relevant GitHub issues}
This PR enables adding/removing CNAME records via the admin interface.
To do so, the following PR needs to be accepted as well, because it allows editing the neccessary file using the pihole CLI: pi-hole/pi-hole#3351

How does this PR accomplish the above?:
{A detailed description (such as a changelog) and screenshots (if necessary) of the implemented fix}

I changed the menu accordingly to have both Local DNS records and CNAME records in one submenu.

What documentation changes (if any) are needed to support this PR?:
{A detailed list of any necessary changes}

Signed-off-by: Matthias rank <[email protected]>

PromoFaux · 2020-05-13T08:13:51Z

Hi there, thanks for the submission, I will try and find some time to review later on today

PromoFaux · 2020-05-14T21:44:39Z

Hi there,

OK, pulled a copy of this one and the accompanying core PR however I've fallen over at the first hurdle:

thus far clearing the cache has had no effect...

Signed-off-by: Matthias Rank <[email protected]>

marank · 2020-05-15T08:19:38Z

That's what I get for accidentally testing my changes on another branch... Should work now, sorry for that!
On a side note, should I move the functions for managing the CNAME records also to func.php? Personally I think it's cleaner to keep them separated.

DL6ER

I was thinking for a moment we should maybe fuse the DNS records and the CNAME records pages into one, however, it's likely clearer structured (esp. for the inexperienced) like you proposed it.

Some more thoughts:

I merged the core PR into development already.
There is a small merge conflict on this branch.

Yes, please move the routines over to func.php as you suggested. Just like I did in #1276 and add support for importing CNAMEs through teleporter. There should be anything you need in #1276 where I did this for the local DNS records page.

Also, please add authentication checking for the CNAME page to avoid anyone being able to submit configuration even without logging into the web interface. See 7b2d396 for reference.

marank · 2020-05-20T19:51:06Z

I'll update this PR tomorrow. Thanks for your review!

Moved CNAME functions to func.php Added teleporter support Fixed CNAME and DNS file declaration (filename was declarated in seperate file, therefore trying to read those files from func.php failed) Fixed error and success response functions in func.php (calling addCustomDNSEntry and addCustomCNAMEEntry repeatedly from teleporter failed because of error function defined inside of those functions) Signed-off-by: Matthias Rank <[email protected]>

marank · 2020-05-21T13:05:40Z

Okay, there were some hurdles while implementing the requested changes:

I had to remove the definition of the error function inside of AddCustomDNSEntry because subsequent calls from teleporter failed (Cannot redeclare error() (previously declared in /var/www/html/admin/scripts/pi-hole/php/func.php:126) in /var/www/html/admin/scripts/pi-hole/php/func.php on line 126
I had to move the declaration of the $customDNSFile/$customCNAMEFile vars to func.php because they didn't exist when called from teleporter
When importing Local DNS Entries or Local CNAME Entries with teleporter and Clear existing data is set, every entry gets deleted seperately, always causing a restart of FTL (see webpage.sh). This has to be changed because FTL won't start if there were too many start attempts:

May 21 14:38:44 gradius systemd[1]: pihole-FTL.service: Start request repeated too quickly.
May 21 14:38:44 gradius systemd[1]: pihole-FTL.service: Failed with result 'start-limit-hit'.

I haven't used teleporter in the past, but all the tests I did were successfull.
On a side note, I'm not very familiar with resolving merge conflicts using git. I'd appreciate to get some help with that.

pralor-bot · 2020-05-30T20:34:33Z

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/add-cname-record-support-to-pi-hole-5s-local-dns-records/31346/7

pralor-bot · 2020-06-02T12:24:09Z

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/local-dns-management/6434/5

pralor-bot · 2020-06-06T11:46:11Z

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/dns-redirect/1555/5

pralor-bot · 2020-06-09T12:21:26Z

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/support-for-cname-next-to-host-record/25707/6

pralor-bot · 2020-06-09T13:07:16Z

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/create-custom-dns-srv-cname-entries-with-the-new-local-dns-tool/31992/3

Changes have been made

DL6ER · 2020-06-14T18:46:03Z

@marank Sorry for the delay on this. You can use the internal conflict resolution mechanism of Github. You should see it above the tests in this PR:

It will open a page showing what was added to the modified pages meanwhile so you can review the changes and decide what is the best when fusing your new changes with the ones made meanwhile, for instance:

pralor-bot · 2020-06-21T06:06:15Z

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/alias-cname-einrichten/34689/2

marank · 2020-06-21T11:09:37Z

Ok, I'll need some time to update this PR. There were some changes in the devel branch in the meantime which I want to add.

Signed-off-by: Matthias Rank <[email protected]>

marank · 2020-06-21T12:16:17Z

I think I'm done, looks good to me. The only issue remaining is the one with FTL restarting too often when restoring a backup via teleporter. Maybe we should add a flag to the pihole command which prevents restarting FTL after every relevant add/remove DNS/CNAME command?

PromoFaux · 2020-07-07T21:12:20Z

@DL6ER shall we change this to point at 5.1, or leave it til vNext?

DL6ER · 2020-07-07T21:14:26Z

Merge now for the next version after v5.1 gives us more time to test things properly.

XhmikosR · 2020-07-08T13:47:16Z

scripts/pi-hole/js/customcname.js

+var table;
+var token = $("#token").text();
+
+function showAlert(type, message) {


Why duplicate stuff here?

XhmikosR · 2020-07-08T13:47:35Z

scripts/pi-hole/js/customcname.js

+
+/* global utils:false */
+
+var table;


Please avoid global variables

pralor-bot · 2020-07-16T09:28:24Z

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/update-5-1-bar-checkbox-und-cname-fehlen/35637/2

pralor-bot · 2020-07-19T11:12:32Z

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/upgrade-from-5-0-5-1-doesnt-include-cname-admin/35868/1

pralor-bot · 2020-07-20T19:25:32Z

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/upgrade-from-5-0-5-1-doesnt-include-cname-admin/35868/4

pralor-bot · 2020-08-22T06:20:13Z

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/custom-redirects/37272/18

pralor-bot · 2020-09-18T18:31:27Z

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/change-the-ip-address-returned-for-blocked-websites-that-is-outside-of-the-machine/38458/10

mcnahum · 2020-10-19T16:40:57Z

I'm really waiting for that change to land.. any idea for when it is?

dschaper · 2020-10-19T18:58:05Z

When it's ready. Which is the same as "soon".

Addressing the issues that were brought up in #1278 (comment) would help.

pralor-bot · 2020-11-28T19:27:41Z

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/pi-hole-core-web-v5-2-and-ftl-v5-3-released/40909/1

pralor-bot · 2020-12-19T22:09:27Z

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/which-local-dns-list/41819/2

Added support for CNAME records add/remove

4e07ae2

Signed-off-by: Matthias rank <[email protected]>

Removed already defined functions

e380d92

Signed-off-by: Matthias Rank <[email protected]>

DL6ER previously requested changes May 20, 2020

View reviewed changes

PromoFaux mentioned this pull request May 22, 2020

Local DNS Records not parsing domains correctly if formatted as "ipaddress fqdn hostname" pi-hole/pi-hole#3427

Closed

3 tasks

Merge branch 'devel' into devel

b597a0f

Incorporate upstream changes

773fed0

Signed-off-by: Matthias Rank <[email protected]>

marank force-pushed the devel branch from 94f7f51 to 773fed0 Compare June 21, 2020 11:56

DL6ER approved these changes Jul 7, 2020

View reviewed changes

DL6ER merged commit cb5b9c6 into pi-hole:devel Jul 7, 2020

XhmikosR reviewed Jul 8, 2020

View reviewed changes

scripts/pi-hole/js/customcname.js

/* global utils:false */

var table;

Copy link

Contributor

XhmikosR Jul 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please avoid global variables

yubiuser mentioned this pull request Jul 16, 2020

Remove duplicated sidebar entry for Local DNS Records #1500

Merged

9 tasks

yubiuser mentioned this pull request Jul 27, 2020

CNAME resolution doesn't check local DNS records pi-hole/pi-hole#3619

Closed

3 tasks

dschaper added this to the v5.2 milestone Oct 19, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added support for CNAME records add/remove #1278

Added support for CNAME records add/remove #1278

marank commented May 12, 2020

PromoFaux commented May 13, 2020

PromoFaux commented May 14, 2020

marank commented May 15, 2020

DL6ER left a comment •

edited by PromoFaux

Loading

marank commented May 20, 2020

marank commented May 21, 2020 •

edited

Loading

pralor-bot commented May 30, 2020

pralor-bot commented Jun 2, 2020

pralor-bot commented Jun 6, 2020

pralor-bot commented Jun 9, 2020

pralor-bot commented Jun 9, 2020

DL6ER commented Jun 14, 2020

pralor-bot commented Jun 21, 2020

marank commented Jun 21, 2020

marank commented Jun 21, 2020

PromoFaux commented Jul 7, 2020

DL6ER commented Jul 7, 2020

XhmikosR Jul 8, 2020

XhmikosR Jul 8, 2020

pralor-bot commented Jul 16, 2020

pralor-bot commented Jul 19, 2020

pralor-bot commented Jul 20, 2020

pralor-bot commented Aug 22, 2020

pralor-bot commented Sep 18, 2020

mcnahum commented Oct 19, 2020

dschaper commented Oct 19, 2020

pralor-bot commented Nov 28, 2020

pralor-bot commented Dec 19, 2020

Added support for CNAME records add/remove #1278

Added support for CNAME records add/remove #1278

Conversation

marank commented May 12, 2020

PromoFaux commented May 13, 2020

PromoFaux commented May 14, 2020

marank commented May 15, 2020

DL6ER left a comment • edited by PromoFaux Loading

Choose a reason for hiding this comment

marank commented May 20, 2020

marank commented May 21, 2020 • edited Loading

pralor-bot commented May 30, 2020

pralor-bot commented Jun 2, 2020

pralor-bot commented Jun 6, 2020

pralor-bot commented Jun 9, 2020

pralor-bot commented Jun 9, 2020

DL6ER commented Jun 14, 2020

pralor-bot commented Jun 21, 2020

marank commented Jun 21, 2020

marank commented Jun 21, 2020

PromoFaux commented Jul 7, 2020

DL6ER commented Jul 7, 2020

XhmikosR Jul 8, 2020

Choose a reason for hiding this comment

XhmikosR Jul 8, 2020

Choose a reason for hiding this comment

pralor-bot commented Jul 16, 2020

pralor-bot commented Jul 19, 2020

pralor-bot commented Jul 20, 2020

pralor-bot commented Aug 22, 2020

pralor-bot commented Sep 18, 2020

mcnahum commented Oct 19, 2020

dschaper commented Oct 19, 2020

pralor-bot commented Nov 28, 2020

pralor-bot commented Dec 19, 2020

DL6ER left a comment •

edited by PromoFaux

Loading

marank commented May 21, 2020 •

edited

Loading