Disable dhcp if DHCP_ACTIVE is set to false

[Lukinoh]

Description

In the current version of Pi-hole docker image, you can enable the DHCP using the following environment variables:

• DHCP_ACTIVE
• DHCP_START
• DHCP_END
• DHCP_ROUTER

It works fine.
However, if you have a volume that persists your Pi-hole/Dnsmasq data, and you decide to restart the container with DHCP_ACTIVE=false the Pi-hole DHCP will stay enable.

So, I updated the code of setup_FTL_dhcp to correctly disable the DHCP if DHCP_ACTIVE=false

Motivation and Context

I have a personal project where the user defines the Pi-hole DHCP configuration before running the container.
Hence he/she could enable it, and then disable it.

I found a workaround which consists on setting DHCP_ACTIVE=true, but not defining the other mandatory environment variables.
The workaround is a bit counter-intuitive, so I thought it could be nice to have a proper behaviour.

How Has This Been Tested?

I did one manual test on my personal project.
Moreover, I added unit tests to ensure the correct behaviour.
I do not really know Python, so I write the tests by mimicking the existing ones.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

I would define it as Bug fix. However, it changes a behaviour so maybe someone relies on this behaviour?

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.

I did not find any code style. If something is wrong let me know.
I think the current documentation still is fine.

[PromoFaux]

However, it changes a behaviour so maybe someone relies on this behaviour?

https://xkcd.com/1172/

I can't say for certain that anyone's install would or would not be affected by this change. If you are able, I'd appreciate if you could test your workflow/setup against the :development-v6 tag - as there are a number of breaking changes there already (see blog post) including how the environment variables are handled. A lot of the current v5 environment variables have been removed entirely, and instead everything is configured with an FTLCONF_ variable.

[Lukinoh]

Okey, thanks for the quick response.

I will give a look by the end of the next week-end.

[Lukinoh]

So here my findings:

DHCP

When starting the container the first time with the following environment variables, you will get an error.

• "FTLCONF_dhcp_active=true",
• "FTLCONF_dhcp_start=192.168.9.3",
• "FTLCONF_dhcp_end=192.168.9.33",
• "FTLCONF_dhcp_router=192.168.9.1",

2023-10-10 06:14:30 ==========Applying settings from environment variables==========
2023-10-10 06:14:30   [✗] Error Applying pihole-FTL setting dhcp.active=true
2023-10-10 06:14:30   [i]   dnsmasq test failed with signal 11 
2023-10-10 06:14:30 New dnsmasq configuration is not valid (nsmasq: bad dhcp-range at line 72 of /etc/pihole/dnsmasq.conf.temp), config remains unchanged
2023-10-10 06:14:30   [✓] Applied pihole-FTL setting dhcp.end=192.168.9.33
2023-10-10 06:14:30   [✓] Applied pihole-FTL setting dhcp.router=192.168.9.1
2023-10-10 06:14:30   [✓] Applied pihole-FTL setting dhcp.start=192.168.9.3
2023-10-10 06:14:30 ================================================================

Then, if you restart the container it will work fine.
I suppose it comes from the fact that to be able to enable dhcp.active, you need dhcp.start, dhcp.end and dhcp.router to be already set, and from the log they are set after

Permission denied

With the default configuration, I got several permissions denied.
By setting DNSMASQ_USER=root, it removes them.

2023-10-10 06:45:53 2023-10-10 04:45:52.111 [240M] INFO: FTL user: pihole
2023-10-10 06:45:53 2023-10-10 04:45:52.111 [240M] INFO: Compiled for linux/amd64 (compiled on CI) using cc (Alpine 12.2.1_git20220924-r10) 12.2.1 20220924
2023-10-10 06:45:53 2023-10-10 04:45:52.113 [240M] WARNING: copy_file(): Failed to open "/etc/pihole/config_backups/pihole.toml.1" for writing: Permission denied
2023-10-10 06:45:53 2023-10-10 04:45:52.113 [240M] WARNING: Rotation /etc/pihole/pihole.toml -(COPY)> /etc/pihole/config_backups/pihole.toml.1 failed
2023-10-10 06:45:53 2023-10-10 04:45:52.113 [240M] INFO: Writing config file
2023-10-10 06:45:53 2023-10-10 04:45:52.114 [240M] WARNING: copy_file(): Failed to open "/etc/pihole/config_backups/dnsmasq.conf.1" for writing: Permission denied
2023-10-10 06:45:53 2023-10-10 04:45:52.114 [240M] WARNING: Rotation /etc/pihole/dnsmasq.conf -(COPY)> /etc/pihole/config_backups/dnsmasq.conf.1 failed
2023-10-10 06:45:53 2023-10-10 04:45:52.115 [240M] WARNING: copy_file(): Failed to open "/etc/pihole/config_backups/custom.list.1" for writing: Permission denied
2023-10-10 06:45:53 2023-10-10 04:45:52.115 [240M] WARNING: Rotation /etc/pihole/custom.list -(COPY)> /etc/pihole/config_backups/custom.list.1 failed
2023-10-10 06:45:53 2023-10-10 04:45:52.115 [240M] WARNING: Cannot set process priority to -10: Permission denied. Process priority remains at 0

Password

If I do not set "FTLCONF_webserver_api_password", no random password is generated, and I can access Pi-hole UI directly without password.
Hence, it is not aligned with the behaviour described in the documentation.

---

Hope it helps.

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[github-actions]

Existing merge conflicts have not been addressed. This PR is considered abandoned.