Allow setting UniFi Controller IP in Kea DHCP.

[reitermarkus]

Tested using WireShark for my controller IP, 10.0.1.77 (0x0a00014d):

Option: (43) Vendor-Specific Information
  Length: 6
  Value: 01040a00014d

[AdSchellevis]

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.

[mimugmail]

Its the same as my previous request, option 43 should be some kind of grid as there could be multiple 43 option entries

[AdSchellevis]

@mimugmail but if I'm not mistaken, they are mutually exclusive (so only one applies for that specific configuration)

[reitermarkus]

they are mutually exclusive

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.

[AdSchellevis]

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]

I have made this more generic now, by adding option definitions

and options

and allowing to specify the options in subnets

[AdSchellevis]

@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.

[AdSchellevis]

@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 1-N. We can polish the input a bit to prevent non-relevant options being shown as well (e.g. the record type inputs). thoughts?

[reitermarkus]

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?

[AdSchellevis]

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/controllers/OPNsense/OpenVPN/forms/dialogInstance.xml

Line 130 in a8e329b

<style>selectpicker role role_server</style>

core/src/opnsense/mvc/app/views/OPNsense/OpenVPN/instances.volt

Lines 63 to 76 in a8e329b

	$("#instance\\.role, #instance\\.dev_type").change(function(){
	let show_advanced = $("#show_advanced_formDialogDialogInstance").hasClass("fa-toggle-on");
	let this_role = $("#instance\\.role").val();
	let this_dev_type = $("#instance\\.dev_type").val();
	$(".role").each(function(){
	let tr = $(this).closest("tr").hide();
	if ((tr.data('advanced') === true && show_advanced) || !tr.data('advanced')) {
	if ($(this).hasClass('role_' + this_role) || $(this).hasClass('role_' + this_role + '_' + this_dev_type)) {
	tr.show();
	}
	}
	});
	});

But if we agree on the direction, I'm more than fine polishing the final bits when merging.

[reitermarkus]

I have now also added a field to specify a client class test condition.

I'll leave the polishing up to you. Ideally record_types should work like select_multiple but with duplicates allowed instead of being a plain comma-separated string.

[AdSchellevis]

@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.

[reitermarkus]

I have tried to figure this out, but it doesn't seem that simple after all. I think the definitions have to be split again, as there are currently a bunch of problems:

• It is currently not possible to specify the same option with different data for different subnets.
• Pre-defined options cannot be re-defined, so these need to be specified without a definition.
• If client classes are added, sub-options need to be defined globally with a custom space in case they conflict with other client classes, while the parent-option needs to be defined in the client class, see the example here: https://kea.readthedocs.io/en/latest/arm/dhcp4-srv.html#dhcpv4-private-options

[AdSchellevis]

Maybe a stupid question, but can't we use uuids to bind the definitions and payload together? Personally I have a hard time finding definitions for kea fields in different area's, but something like this doesn't seem to crash:

         "option-def": [
                {
                        "name": "11635360-0eaf-4c63-9d30-a192c8e59168",
                        "code": 1,
                        "type": "string",
                        "space": "11635360-0eaf-4c63-9d30-a192c8e59168"
                }
        ],
....
        "option-data": [
                    {
                        "name": "11635360-0eaf-4c63-9d30-a192c8e59168",
                        "space": "11635360-0eaf-4c63-9d30-a192c8e59168",
                        "data": "0.0.0.0"
                    }
        ]

While writing this, it still feels like we're sending a rocket to the moon, where in practice, most issues only circle around code 43 as this option isn't well defined.

Personally I think offering fine grained controls with client classes adds way more complexity than 99% of our users are looking for, when someone needs almost programatic control of the dhcp process, a manual configuration might be a better alternative.

[reitermarkus]

but can't we use uuids to bind the definitions and payload together?

This will only work for sub-options. But then you need to also re-define the parent option with "encapsulate": "11635360-0eaf-4c63-9d30-a192c8e59168", which can only be done in a client class, i.e. you cannot re-define a pre-defined option globally.

[AdSchellevis]

hmm, that's annoying. Partly this is difficult in terms of templating (overcomplicated jinja templates are hard to maintain) and to some degree the user input is also problematic. We could opt for moving the json file generation to the model, which does make it easier to de-duplicate certain information into a single container.

If we change the config generation handling, the next question is, what should the user input look like and how do we prevent invalid entries being generated. Do you have thoughts about that?

I do think it makes sense to first figure out what we want to build and generate code after that, there are some loose ends here, currently I do have some difficulties seeing all constraints that apply.

I don't mind spending some time on this to ease future additions, this was also a bit of the goal from my end to not start with ipv6 yet until ew have enough feedback for the ipv6 variant.

[AdSchellevis]

@reitermarkus can you try the code currently in master? 3f184a6 adds the "vendor-encapsulated-options-space" with a minimal set of options. I haven't tested this myself, but the generated configuration on my end is valid and the ui offers validations to prevent duplicate sub-codes being inserted with different definitions.

[AdSchellevis]

@mimugmail tried it out, didn't seem to work, but config looks as expected. I've moved the feature into its own branch for now implementing 79f62cf (kea_custom_opt_pr7361)