unbound: migrate overrides support to MVC - #5488
Conversation
AdSchellevis
left a comment
AdSchellevis
left a comment
There was a problem hiding this comment.
@swhite2 some small remarks, but looks good. I haven't tried it on my end yet, but looking at the code I don't expect huge issues to be honest
|
@AdSchellevis Thanks for the review, the latest commit addresses all of the above remarks. |
| @@ -0,0 +1,17 @@ | |||
| {% if not helpers.empty('OPNsense.unboundplus.domains.domain') %} | |||
There was a problem hiding this comment.
personally would prefer leaving the "unboundplus" namespace for default "unbound" if we are going to migrate anyway
There was a problem hiding this comment.
Wouldn't this require a larger change beyond the scope of this PR? Just my 2 cents.
There was a problem hiding this comment.
I'm not sure if model renames are a good idea for general model migration behaviour reasons (@AdSchellevis would know better) but except this the change would entail changing a few configuration template lines and that isn't unreasonable to do so during the migration taking place here anyway. M1_0_0.php already contains the code to be used for this. It takes the contents from the old config location and moves it to the new model.
There was a problem hiding this comment.
Agreed. Impact doesn't seem to be that high. I'd also like to know what @AdSchellevis thinks about model renaming.
There was a problem hiding this comment.
Only concern is that templates need to be changed after adding more of them to migrate but the end result is still I can hope for 😊
|
@swhite2 just tried it on my end, seems to work fine. thanks! |
* unbound: overrides: migrate to mvc model * unbound: overrides: generate host_entries via model, revert template generation * unbound: overrides migration: fix missing include * unbound: overrides: clean up Co-authored-by: Stephan de Wit <stephan.de.wit@deciso.com>
fichtner
changed the title
fichtner
changed the title
3 participants
This PR moves the legacy Unbound->Overrides section to the new MVC model, enabling it for use with the API.
A couple of things to note:
HostnameFieldto now also accept wildcard entries (*). Default for this is false, so no impact.Adds a template helper which retrieves the current configured IP Address of an interface, useful for non-static assignments as they are not stored in config.src/opnsense/service/modules/template.pyto allow for regex checking in a template.While most of the relevant code fromunbound.inchas been deleted, checking the DHCP configured static map (both Ipv4 and Ipv6) and converting this to the relevant local-data sections for Unbound is still done by theunbound_add_host_entries_dhcp()function due to stringent IPv6 address-checking. It now writes this to a seperate include (host_entries_dhcp.conf).To avoid a huge template with too much logic in it, generating the config is still done by
unbound.inc, it, however, now uses the base model methods to achieve this.