Make V6 Static DHCP Lease GUI More user friendly

[fellipec]

As discussed in Reddit: https://www.reddit.com/r/pihole/comments/1isvj5g/what_in_the_world_is_this_atrocious_new_static/

The older static lease for DHCP in V5 had a more user-friendly interface, while the new V6 expose a text field for the user write a config file-like text.

While one can argue that the new interface is more powerful, it lacks the ease of use that was a fundamental cornerstone of Pi-Hole.

I suggest reverting to the old GUI, leaving this new one as an "advanced" option for those who want.

[PromoFaux]

The following is a conversation that happened on the same topic shortly before release.

@Bucking_Horn

I'd expect that v5's three columns (MAC|IP|hostname) would be enough for the vast majority of v6 users as well, especially since that would be what they did configure in v5.

If v5 users needed some of the more exotic options, they probably have created a custom dnmasq conf or edited the file directly, so they'd be already somewhat familiar with dnsmasq's detailed options.

Instead of trying to fit all possible options into a table, how about making the existing table a Basic level interface, with a new, extra column 'expert options present' that shows as ticked when more options are set, and replacing that with the current v6 style view in the Expert panel?

Parsing the Expert options for the Basic UI shouldn't be overly difficult:
Strip or ignore options starting with set: or tag: as well as ignore, and then parse by strict order, the first being MAC or id, second IP, third hostname.

Sorting Basic should affect how Expert lines are sorted accordingly.

@DL6ER

The tricky part being not to wipe out possible "expert" setting when editing the line on the "basic" level. We could also consider making a much larger table having eight columns:

<hwaddr>   id:<client_id>   set:<tag>   tag:<tag>   <ipaddr>   <hostname>   <lease_time>   <ignore>

but this is probably nightmare in UI-design.

---

How about a middle ground? Showing both the table and the text field below it? One automatically updates the other (I did the same for the DNS server selection where you could add extra or special servers below in the (usually collapsed) text area?
When the script detects a non-basic configuration in any line, it could simply disable the table with a small note that advanced stuff has been detected and, hence, editing needs to happen below?

Just generating a few idea here...

@Bucking_Horn

@DL6ER

The tricky part being not to wipe out possible "expert" setting when editing the line on the "basic" level.

How about keeping a copy of those 3 entries per line when populating the UI table, in addition to the entry field values?
Updating a dnsmasq configuration line would then just be a search for the copied values to be replaced by the values entered via UI.

@rdwebdesign

@DL6ER

but this is probably nightmare in UI-design.

Maybe in smaller screens, but there is another possibility, similar to your table + text box idea.

We could create a simple table (4 columns = 3 values + action buttons) and an "Advanced" button to show the full table (with 8, 9 or 10 columns).

The "auto disable" feature can be used to disable the simple table as well.

@Bucking_Horn

I don't think that a fixed column layout is a good fit for the full set/table, as there can be no fixed number of columns.

set:, tag: and IPv6 addresses can occur multiple times.

@rdwebdesign

@Bucking_Horn

and IPv6 addresses can occur multiple times.

All IPv6 will be added in the same field/column. The the fixed number of columns is not the issue.

The issue would be the table width.

@rdwebdesign

@Bucking_Horn

How about keeping a copy of those 3 entries per line when populating the UI table, in addition to the entry field values?

Do you mean a table with 4 columns? Mac Address + IP + Hostname + "Advanced (containing the raw line text)"?

@Bucking_Horn

@rdwebdesign

All IPv6 will be added in the same field/column. The the fixed number of columns is not the issue.

Allowing comma-separated lists of values in one column still wouldn't seem to be a good fit to me.

Also, showing those additional options as columns may prompt users to fill them in, even when there would be no obvious use (e.g. aforementioned IPv6 addresses will never get assigned as Pi-hole won't operate as DHCPv6 server anyway (hmm, maybe we could cut down on options with that approach)).
I'd rather have the v5 table for backwards compatibilty, and leave the flat file format for expert use only (which it probably should be).

@rdwebdesign

Do you mean a table with 4 columns? Mac Address + IP + Hostname + "Advanced (containing the raw line text)"?

No, I thought of it as a purely informational tick box.
But yours looks similar to DL6ER's suggestion, only that his idea is to show the full line under the three fields, if I understand him correctly.

But combining fields and full line editing in one UI could make UI updating more challenging than when you keep them separate (fields for basic, flat file editing for expert).

@rdwebdesign

Yes. Similar to DL6ER's idea.

---

If we are going to use separate input fields for each option, there is no way to avoid comma separated entries (some options will need to accept more than one value).

@DL6ER

Sorry, I missed the

@rdwebdesign

I was testing the page and (unintentionally) I entered a line containing spaces and tabs. I noticed FTL didn't remove these characters.

...

Do we need to clean up the entries to avoid dnsmasq issues?

before: Yes, they can cause issues. The IP address is fed through inet_pton which may trim spaces around addresses, I have not seen any details about this in man inet_ntop but it may be possible. The extra spaces and tabs in the hostname are probably an issue but dnsmasq validates the hostnames you define here for validity, i.e., only a-z A-Z 0-9 - _ are allowed here. It should fail hard with bad DHCP host name otherwise,

@rdwebdesign

Thanks for the detailed explanation.

@DL6ER

It should fail hard with bad DHCP host name otherwise,

FTL and the web interface didn't generate any error messages when entering the values containing spaces and tabs. If later dnsmasq generates an error, this will cause user confusion.

Maybe we need to validate the values sent via web interface / API.

@DL6ER

I did just try something useless and get an error as expected:

Note that the DHCP server needs to be enabled to get the error as - otherwise - the dhcp.hosts array is not added to dnsmasq's configuration and, accordingly, also is not tested.

@rdwebdesign

@DL6ER

Note that the DHCP server needs to be enabled to get the error

Hmmm... that was the issue.
I tested simply adding values to the field, but with the DHCP disabled.

We probably should not accept static leases if DHCP is disabled to avoid issues.

If an user enter many invalid static leases while the DHCP server is disabled (like I did) and later enable it, there will be issues.

@rdwebdesign

For completeness:

I enabled DHCP and added an static lease (ÄÖÄ,192.168.1.4,fake).
It failed with an error message (as expected), but I also added a line using the format "MAC,IP,hostname" containing spaces and tabs in the IP and hostname.
FTL accepted them without errors:

A4:EE:57:30:0E:0F, 	192.168.1.2, 	printer
#                 ^^            ^ ^                 
#           space_||__tab space_| |__tab            

Note:
The spaces and tabs are preserved in the API return. Not sure if they will cause some issues with dnsmasq.

@DL6ER

@rdwebdesign

If an user enter many invalid static leases while the DHCP server is disabled (like I did) and later enable it, there will be issues.

But how realistic of a scenario is this? It would need quite some extra code as FTL is currently not aware of any kind of dependency between settings. Even without it, the issues should be small. Once you try to enable it, FTL will validate the entire config and detect that at least one dhcp-host is invalid in your example. It would then reject the config change (in this case the try to enable the DHCP server) and print the warning about the offending line (the first invalid dhcp-host line). The user can then try and fix it and repeat this as long as all invalid entries are gone. There should be no time without a running dnsmasq as it should never really be restarted if at least one invalid entry remains.

But we could change this to always adding dhcp-host to the config even when the server is disabled. dhcp-host lines without enabled DHCP server should be possible, they will just not do anything useful (but they'd be checked). However, there is an entire block of settings which are only applied when dhcp.active = true, changing this may mean a bit of a mess in the config file and seems not the best option at first sight.

@rdwebdesign

but I also added a line using the format "MAC,IP,hostname" containing spaces and tabs in the IP and hostname.

@rdwebdesign

The spaces and tabs are preserved in the API return. Not sure if they will cause some issues with dnsmasq.

They also end up in the used dnsmasq configuration so that's consistent.

I did just check that and can confirm inet_ntop is pretty robust when it comes to detecting and analyzing IP addresses plus dnsmasq feeds the specified host names through libidn which strips away the spaces/tabs on the hostname.
TL;DR: Your spaces/tab example works because dnsmasq is graceful. This does not have to apply to everything, though. The source code processing dhcp-host is rather difficult to read because it has some rather strange backwards-compatibility stuff, but, e.g., spaces even inside MAC addresses are tolerable as well.

@yubiuser

Regarding the basic/advanced input field. The first idea that came to my mind is exactly this

@DL6ER

howing both the table and the text field below it? One automatically updates the other (I did the same for the DNS server selection where you could add extra or special servers below in the (usually collapsed) text area?

90% of users will be happy with the current 3 column input fields. If users need more advanced configurations, they could use the text input field then.

@rdwebdesign

@DL6ER

TL;DR: Your spaces/tab example works because dnsmasq is graceful.

OK.
That was my main concern, because it is allowed by the web interface and FTL, but I wasn't sure if dnsmasq would accept or complain.

@DL6ER

But how realistic of a scenario is this?

The scenario I imagined was a case were an user import or create many static invalid leases while Pi-hole DHCP is disabled. Later (after all invalid addresses are already in Pi-hole options), the user decides to turn the DHCP on, and the user will receive many warnings during dnsmaq. Users will say "The web interface accepted the values, but now it is saying this is invalid".

I'm not sure how realistic is this. Maybe I'm just imagining a very crazy/unlikely scenario, but apparently it is possible.

I think we don't need to change anything for now.
At least we discussed this possibility and now we know where to look if some one starts to complain.

@rdwebdesign

I like this idea.

• table with the original 3 fields;
• textbox;
• "auto disable" (or auto hide) feature, to disable the table when there are advanced options;
• show note about the presence of advanced options

[CSM]

Wow, the new v6 DHCP lease GUI is quite a step back. Took me awhile to even see the Expert mode to set the static leases. Why not have a simple interface in the top Active Lease section to just have a check-box and save as static. Then just show the ones on the top that have a static designation, and the ones that don't they can just show that too. Maybe a column that says "save static" and you can see the ones saved. Just one list, and it can simply show the previous info from the 2 lists. One list is good, just keep it simple to show the static IP's that are set.

[nero355]

This guy got it right : https://old.reddit.com/r/pihole/comments/1isvj5g/what_in_the_world_is_this_atrocious_new_static/mdpm0jd/

The way it is in Pi-Hole v6 is how many admins would like to see it in other software !!!

[CSM]

Should at least be able to sort columns such as IP address etc. That was nice in V5 where you could sort by IP easily. You could always export the text data, mass edit, and then import, as a way for the editing management. Then you've have best of both worlds.

[perzaklie]

Why change it? I very much dislike the v6 DHCP static lease method. As for v5, "If it works, don't fix it!"

[Username0504]

I concur that the v5 GUI was a much friendlier interface. The simple ability to click on a DHCP lease and load it into the reservations window made things a snap.

[perzaklie]

Agree 1000%!!!

[smithbill]

A slight aside but connected to Static DHCP from v5 to v6:

I just discovered when I Teleported my v5 Static DHCP IP's, that ONE of the MAC addresses was defined as MA-CA-DD-RE-SS-ES as opposed to MA:CA:DD:RE:SS:ES (ie. using '-' instead of ':').

I'm presuming when I entered it in Pi-hole v5, this is how I entered it & v5 either was happy with it, or ignored that particular Static reservation.

When I used Teleporter to export my v5 settings, upgraded to v6 (with various issues) & then used Teleporter to import my settings into my new Pi-hole v6, DNS wouldn't work & every time I tried updating Pi-hole settings, it generated an update error. Eventually I worked out Pi-Hole v6 wasn't happy with the MAC address with '-' which had been read in by Teleporter from v5.

I guess either Teleporter needs to verify export/import such that only ':' are used in MAC addresses or else Pi-hole v6 Static DHCP MAC addresses needs to cope with either '-' or ':'

[smithbill]

Another slight aside but connected to Pi-hole v6 Static DHCP management:

I kept getting this error in my upgraded Pi-hole v6:

Warning in dnsmasq core:
not using configured address 192.168.68.110 because it is in use by the server or relay

I think this was because my Static DHCP leases included the MAC address & IP assignment for my Pi-hole host which was on 192.168.68.110.

Again, this was something that was fine in my Pi-hole v5 & came over in the Teleport from v5 to v6.

I liked having my v5 Static DHCP leases 'complete' with everything in there & v5 seemed happy with that, but v6 Static DHCP lease management seems totally unforgiving

[Havok115]

Agree, make it cleaner!

[thescinema]

agree 100%
v6 interface feels like a step back, not forward.

[davistw]

Yep just discovered the UGLY way to update static leases. I was able to update but it was UGLY. I much prefered the V5 way...

[DanSchaper]

And how would you feel if we called the kid in your picture UGLY?

[davistw]

Well he was kind of an ugly baby. Luckily he took after his PAWPAW and is very handsome now...

[davistw]

Well, on a vote for going back to the way dhcp was before, I realize that dhcp is not pi-hole's first and formost job but i don't think the devolpers realize how many acutally use that function. BTW I LOVE PI-Hole.....

[smithbill]

Management of Static IP leases may not be Pi-holes 'primary' role, but personally I see it as a very high secondary role. I would like to see restoring of the previous UI or even something better, prioritised above all other fixes & updates.

I've really struggled with the current massive text-box functionality over the last few week's when needing to make changes & tweaks to my DHCP leases. So much so, I'm considering whether I should abandon Pi-Hole DHCP server & go back to my router or use something else.

[smithbill]

Earlier, it was suggested that:

"We probably should not accept static leases if DHCP is disabled to avoid issues.

If a user enters many invalid static leases while the DHCP server is disabled and later enables it, there will be issues."

I disagree with this. Even if the static DHCP leases are complete gibberish, they should still be stored (or imported by Teleporter), but it should not be possible to enable DHCP until the problems are sorted out (and obviously the errors/issues need to be highlighted so that the user can work through & fix them in order to enable DHCP).

[Macenstine]

I'm not exactly sure how to "vote" on this but I too am extremely disappointed in how the new DHCP reservations system UI/UX works on v6. I have had Pi Hole running my DHCP and reservations for over a year now and it's been fantastic. The whole reason I configured it in the first place was because my router's DHCP/Reservations interace is painfully slow and Pi Hole's is lighting fast and simple. Please please please bring back the old v5 interface. I also really like the suggestion to have the old interface be on the simple view and the new interface be on the expert view for the power users that prefer it.

[Bucking_Horn]

@Macenstine

I'm not exactly sure how to "vote" on this

You should be able to do so via the Vote button next to the topic title: