-
Notifications
You must be signed in to change notification settings - Fork 810
Allow setting UniFi Controller IP in Kea DHCP. #7361
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
base: master
Are you sure you want to change the base?
Conversation
We need something more generic and better maintainable for this, option 43 equals vendor-encapsulated-options (https://kea.readthedocs.io/en/latest/arm/dhcp4-srv.html) and can be used for multiple purposes, when 10 different vendors use the same option, we rather don't want to add all of them (and possibly cause incompatible variations of options). Leaving this open as feature request and room for further discussion. |
Its the same as my previous request, option 43 should be some kind of grid as there could be multiple 43 option entries |
@mimugmail but if I'm not mistaken, they are mutually exclusive (so only one applies for that specific configuration) |
Based on 9.3 of https://www.isc.org/docs/2023kea_custom_options.pdf, there is a way to apply different options for different clients. |
But I would like to keep it as simple as possible.... these type of constructs are almost impossible to generalize. |
@reitermarkus although it feels a bit like using a cannon to kill a mosquito, maybe it's something we can't prevent for these custom options (I was hoping to keep this simple). I'll take a look as soon as I can. |
@reitermarkus what if we merge the definitions and the data? from a data perspective the split might be cleaner, but from a user input perspective the question is how often this relation is actually |
I have merged the definitions and data. Can you let me know how to hide e.g. the record types field based on the type field? |
I don't mind changing that for you, but if you want to give it a shot, usually we add a style to the field and hook an event. For example in OpenVPN:
core/src/opnsense/mvc/app/views/OPNsense/OpenVPN/instances.volt Lines 63 to 76 in a8e329b
But if we agree on the direction, I'm more than fine polishing the final bits when merging. |
I have now also added a field to specify a client class test condition. I'll leave the polishing up to you. Ideally |
@reitermarkus I don't think we should add the client class now as the relation between both entries is not the same (usually a client class contains a set of options). If in the future we do want to add client-classes, we should be able to reuse the custom options. |
6586a65
to
607e32a
Compare
@AdSchellevis I just tried "make mount" with a current system and I can set the values. Is this way to go to test? |
@mimugmail this probably needs a rebase, the ticket has been stale for months I'm afraid. The branch I opened is https://github.com/opnsense/core/tree/kea_custom_opt_pr7361, I can try to rebase to master, but that has a lot of other breaking changes. |
I did:
Sure, Dashboard is messy but for testing it's fine. No need to rebase right now, let me test first :) |
why not "make upgrade" or if it complains "make upgrade CORE_NAME=opnsense" ;) (ok I couldn't resist) |
Shawn would say: Because reasons :) |
Hi, I spent some time on this turning some rounds and it seems the PR of @reitermarkus is correct. Kea requirest the use of client-classes for code 43, as sub classes are intended with this one. This is working when editing the file directly:
|
Btw. space name needs to be ubnt, vendor-encapsulated-options-space_43 will be ignored by the dhcp device. |
@mimugmail it has been quite some time since I made the branch, not sure what it needs to offer this functionality without offering impossible to validate input. Can you ping me in a couple of weeks? With the example you offered I can have another look when I have some time |
Since our efforts to implement #7361 hasn't reached a functional state, lets move the code into a separate branch to keep master clean.
I did rebase https://github.com/opnsense/core/tree/kea_custom_opt_pr7361 , but realistically this doesn't bring it much further at the moment. It doesn't look very feasible to offer an interface to |
bfdf0d3
to
968e5f9
Compare
Came to this issue while checking the state of vendor option support.
Experience at least firmly says yes, although that might depend on how "intelligent" the client devices are. DHCP provided unifi controller address confused the heck out of our SIP phones here which tried to parse it as their firmware bootstrap URL (also option 43). We ended up keeping the unifi option in place and configuring the phones statically through their UI. Or maybe it's supposed to be a server side job to detect the type of client device and serve the matching option? Any ETA/target release for at least a rudimentary implementation? I need that unifi thing before ISC DHCP is removed ;) Given how far this issue has gone I will not try to hack my own solution and submit it. Kind regards, |
no, not likely to appear either for reasons mentioned earlier. dnsmasq might be a better target to aim for or use a completely custom kea config (see also https://opnsense.org/roadmap/) |
What do you think of "prepackaged" UI options for the more common vendor options? Given the ubiquity (grin) of Unifi gear we could start with that and leave it to users to request more - mutually exclusive and with easy fail safe setup and proper constraints like "valid IP address" for Unifi for example. Precedence: you did take my "Observium support" option for SNMP, for example. |
Isnt there a config include folder for such setups with kea? |
Unfortunately kea doesn't support includes, which either ends up in some hacky construct or not offering it at all, we choose the later for good reasons (see also #7822). Somewhere down the line we came to the conclusion kea might not be the best option for small and medium sized networks (which often has other requirements than provider grade ones), which is what triggered addition of dnsmasq for dhcp on our end. I'm open for maintainable pull-requests, the time we have spend so-far on kea is a bit staggering to be honest, there are just limits in the amount of time I am willing to invest. (maybe the best example so-far is about adding kea for v6, implementing the example as documented https://kea.readthedocs.io/en/stable/arm/dhcp6-srv.html#introduction, leading to the conclusion this can't work for "reasons" https://lists.isc.org/pipermail/kea-users/2018-January/001581.html) |
Hi, i don't know if this is a stupid idea or not but could the inclusions be done trough "slicing" the the config files into multiple little files such as client-classes, custom-options-def, etc and including them when needed in the right sections. Example: kea-dhcp4.conf (sorry if this a stupid idea and or improper but i thought this might be useful) |
It won't offer a consistent state, you can merge json files, but the outcome is unpredictable (increasing the chances kea won't start, which as a result triggers unsolvable reports). kea in my experience is quite fragile, which means data validation is crucial. When people want custom configurations, just ship the full config, which is possible since #7822 |
Going forward, it seems like this is the far easier route to take than to try making a fully configurable template for generic options. This gets even more complex if such options are made dynamic / conditional, like only for a specfic vendor class. That mechanism was originally designed to keep DHCP/BOOTP packets small as to not overflow the available space. If instead, such options are made available one-after-another on demand, afterwards, users can indivdually choose if they need them and also, validations are easy, like in this case, a single IPv4 address. Considering the recent discussions on how to correctly specify the Unifi opton for ISC DHCP and DNSmasq suggests, that this would be a good idea. Also, implementing a specific variant for Unifi does not stand in the way of other vendor-specific option 43 use cases for other manufacturers, AFAIK. |
Correct. As I found out with vendor classes you can have as many of them as you need. Will give unifi a try over the long weekend. |
https://help.ui.com/hc/en-us/articles/204909754-Remote-Adoption-Layer-3 https://forum.opnsense.org/index.php?topic=47370.msg238856#msg238856
Using this dhcp option 43 is optional for unifiy, not a strict requirement. Just leaving this here for those who come after. |
Yep, but there are other business solutions around too, using option 43 to send configuration to clients like VoIP phones |
newer equipment is probably less likely to use option 43, but with dnsmasq it's likely a lot easier to match clients option 60 request to a proper 43 response anyway. |
Yes, for DNSmasq, we already have the full IANA list and means to do this rule-based. Basically, the question is if you like to go the same route with Kea. For standard options, we have specific inputs already. As I have said for DNSmasq already, I like a less error-prone, typed approach to have better checks for specific purposes, which create the needed output for daemon configurations. Of course, that comes with less flexibility. Now, that I have learned how to modify the MVC model to generate non-generic output, I could try a pull request to add the Unifi option with an optional, standard IPv4 input field that does this. I do not think that a rule-based output would even be needed if the user can choose if he needs it, anyway. I would just plainly generate option 43 - and also, in the global section, since it is used for layer 3 traversal, anyway. |
I'm just not a fan of adding vendor specific items which aren't bound to any standard and can only be used for the vendor in question. There's just a high risk of creating a maintenance nightmare as we have no idea about the actual scope of the change. Looking at other vendors, the most common method seems to be to just send raw data (optionally upon request), which is pretty similar to what dnsmasq on our end is currently offering. A similar approach for KEA seems to be challenging and very time consuming to develop due to how the configuration file is modelled (and how poorly documentation seems to fit reality). It's just not something I plan to spend my time on at the moment, also given the limited amount of valuable use-cases. In theory, for dnsmasq, we could eventually add validators for common types to protect agains common input mistakes. |
Tested using WireShark for my controller IP, 10.0.1.77 (
0x0a00014d
):