Project

General

Profile

Actions

Feature #13256

open

Better handling of duplicate IPs in static DHCP assignments

Added by → luckman212 2 months ago. Updated 2 months ago.

Status:
New
Priority:
Normal
Assignee:
-
Category:
DHCP (IPv4)
Target version:
-
Start date:
Due date:
% Done:

100%

Estimated time:
Plus Target Version:
Release Notes:
Default

Description

summary:

In 2018 code that prevented duplicate IPs from being used as static DHCP mappings was removed. There are some cases where this may be desireable, but this PR updates the feature to bring the warning back and allow it to be overridden. This should satisfy all use cases and prevent accidental misconfiguration.

related: https://redmine.pfsense.org/issues/8220

PR: https://github.com/pfsense/pfsense/pull/4594


Files

Actions #1

Updated by Marcos M 2 months ago

Thank you for the contributions!

In general, it's best to avoid first/second person perspective. A yellowish warning box would be preferred over the red error box as well.

However, there are many instances in the GUI where the user is able to save a valid configuration that can have unintended behavior. In these cases, details are left to documentation (either online or in field descriptions). I don't believe this situation warrants a checkbox confirmation in this way.

Actions #2

Updated by Jim Pingle 2 months ago

I don't think we should change the default behavior/add extra steps to reach the current behavior.

Something that might be more palatable is a non-fatal warning handled in JavaScript. Build a list of existing IP addresses in mappings for the interface and if the users enters one that matches, display a JS box under the field that states it's already in use.

That way the user could be warned but not required to jump through hoops to override it.

Actions #3

Updated by → luckman212 2 months ago

I wanted to make the warning display in a "Yellow Box" too but I looked through the code and couldn't see an easy way to re-use what we have to make that happen. If it's important I will try again.

As for rewriting the whole thing as a JS/AJAX function, that's not my strongest area so Jim Pingle if you feel strongly that it needs to be done, it might take me more time or I'd leave it up to someone else. Either way thanks for the consideration.

Actions #4

Updated by → luckman212 2 months ago

I pushed a revised version, looks like this now

Actions #5

Updated by Marcos M 2 months ago

Looks good to me.

Actions

Also available in: Atom PDF