Skip to content
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

wizard: reimplement system setup #8378

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

AdSchellevis
Copy link
Member

closes #8352

This commit implements our replacement for the setup wizard. The questions are roughly the same as in the legacy version. Some less relevant options have been removed (pppoe ondemand for example) and isc-dhcpd has been replaced with dnsmasq.

Only standard tools have been used, a memory model to validate the data and simple input forms in tabs.

The in memory model acts as a wrapper around a legacy configuration data and a couple of component models to apply the requested settings.

Some legacy settings using isset() have been altered to use their empty() equivalent.

This commit implements our replacement for the setup wizard. The questions are roughly the same as in the legacy version.
Some less relevant options have been removed (pppoe ondemand for example) and isc-dhcpd has been replaced with dnsmasq.

Only standard tools have been used, a memory model to validate the data and simple input forms in tabs.

The in memory model acts as a wrapper around a legacy configuration data and a couple of component models to apply the requested settings.

Some legacy settings using isset() have been altered to use their empty() equivalent.
@AdSchellevis AdSchellevis added the feature Adding new functionality label Feb 26, 2025
@AdSchellevis AdSchellevis self-assigned this Feb 26, 2025
<dnsmasq>
<enable>1</enable>
<port>0</port>
<interface>lan</interface>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if people don't assign a "lan" interface in console assignment this can be a problem. we also need to make sure migrations do the normalize dance then on this field and the one in dhcp_ranges ;(

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fichtner but isn't this the same for isc at the moment?

<dhcpd>
<lan>
<enable/>
<range>
<from>192.168.1.100</from>
<to>192.168.1.199</to>
</range>
</lan>
</dhcpd>

In practice this likely means we need to replace the dhcpd sections in console.inc, which I haven't looked at yet (but should do indeed)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dhcp is disabled when lan is not used, yes, this ties back to the console.inc madness

as far as isc doing it, also yes, but it's not an mvc migration-breaks-on-validation case

@@ -328,7 +328,7 @@ function get_searchdomains()
$master_list[] = $syscfg['dnssearchdomain'];
}

if (isset($syscfg['dnsallowoverride'])) {
if (!empty($syscfg['dnsallowoverride'])) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we did have issues with one of these before

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fichtner not sure what it was, but for simplicity, I prefer the empty() variants, so if it doesn't work as it should, we might have to figure out why that is.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was #2751 and looks like it was fixed in the way we want, but then we should move all spots to !empty, not just this one

@@ -365,7 +365,8 @@
<page-wizard-system>
<name>System Setup Wizard</name>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Menu says System: Wizard, I wonder if can bury it in System: Settings

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have a strong preference, can change the acl description or the menu item.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Adding new functionality
Development

Successfully merging this pull request may close these issues.

wizard: reimplement system setup and remove openvpn server one
2 participants