Project

General

Profile

Actions

Feature #13256

closed

Better handling of duplicate IP addresses in static DHCP assignments

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

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

100%

Estimated time:
Plus Target Version:
24.03
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

clipboard-202206081054-ol6ya.png (188 KB) clipboard-202206081054-ol6ya.png → luckman212, 06/08/2022 09:54 AM
clipboard-202206111450-srubn.png (89.6 KB) clipboard-202206111450-srubn.png → luckman212, 06/11/2022 01:50 PM
DHCP warning .png (7.14 KB) DHCP warning .png Alhusein Zawi, 02/04/2024 03:16 AM
Actions #1

Updated by Marcos M almost 2 years 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 almost 2 years 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 almost 2 years 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 almost 2 years ago

I pushed a revised version, looks like this now

Actions #5

Updated by Marcos M almost 2 years ago

Looks good to me.

Actions #6

Updated by Marcos M 5 months ago

  • Status changed from New to Feedback
  • Assignee set to Marcos M
  • Target version set to 2.8.0
  • Plus Target Version set to 24.03
Actions #7

Updated by Alhusein Zawi 3 months ago

the warning is added .

2.8.0.a.20240126.0600

Actions #8

Updated by Jim Pingle 2 months ago

  • Subject changed from Better handling of duplicate IPs in static DHCP assignments to Better handling of duplicate IP addresses in static DHCP assignments
Actions

Also available in: Atom PDF