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

Fix static IP not set if IP is already present in any way inside dhcpcd.conf #4995

Merged

Conversation

StephanPillhofer
Copy link
Contributor

@StephanPillhofer StephanPillhofer commented Oct 29, 2022

Signed-off-by: Stephan Pillhofer 43667664+StephanPillhofer@users.noreply.github.com

  • What does this PR aim to accomplish?:

This PR solves a bug which prevents the dhcpcd.conf file from being changed when the user wants to assign a static IP address to the RaspberryPi. The bug occurs because the target static IP address may already be present in any form in the (default) dhcpcd.conf file. (e.g. the IP address may be mentioned in another setting than "static ip_address" or it may be present in a commented example setting)

  • How does this PR accomplish the above?:

The bug is resolved by utilizing a regular expression for matching a valid non-commented static IP addess entry (e.g. static ip_address=192.168.10.2/24)

  • What documentation changes (if any) are needed to support this PR?:

none


By submitting this pull request, I confirm the following:

  1. I have read and understood the contributors guide, as well as this entire template. I understand which branch to base my commits and Pull Requests against.
  2. I have commented my proposed changes within the code and I have tested my changes.
  3. I am willing to help maintain this change if there are issues with it later.
  4. It is compatible with the EUPL 1.2 license
  5. I have squashed any insignificant commits. (git rebase)

  • I have read the above and my PR is ready for review. Check this box to confirm

@StephanPillhofer
Copy link
Contributor Author

StephanPillhofer commented Oct 29, 2022

A certificate error (cert expired) causes a centos_9 test to fail. Addidtionally the mirror links return a 404. Shouldn't be related to the PR.

E           [mirror] dbus-libs-1.12.20-6.el9.x86_64.rpm: curl error (60): ssl peer certificate or ssh remote key was not ok for https://mirror.grid.uchicago.edu/pub/linux/centos-stream/9-stream/baseos/x86_64/os/packages/dbus-libs-1.12.20-6.el9.x86_64.rpm [ssl certificate problem: certificate has expired]
E         ?                                                   +++++
E           [mirror] dnf-plugins-core-4.1.0-3.el9.noarch.rpm: curl error (60): ssl peer certificate or ssh remote key was not ok for https://mirror.grid.uchicago.edu/pub/linux/centos-stream/9-stream/baseos/x86_64/os/packages/dnf-plugins-core-4.1.0-3.el9.noarch.rpm [ssl certificate problem: certificate has expired]
E           [mirror] python3-dateutil-2.8.1-6.el9.noarch.rpm: curl error (60): ssl peer certificate or ssh remote key was not ok for https://mirror.grid.uchicago.edu/pub/linux/centos-stream/9-stream/baseos/x86_64/os/packages/python3-dateutil-2.8.1-6.el9.noarch.rpm [ssl certificate problem: certificate has expired]

yubiuser
yubiuser previously approved these changes Oct 29, 2022
automated install/basic-install.sh Outdated Show resolved Hide resolved
Signed-off-by: Stephan Pillhofer <43667664+StephanPillhofer@users.noreply.github.com>
@dschaper dschaper merged commit 871067a into pi-hole:development Oct 31, 2022
@yubiuser
Copy link
Member

yubiuser commented Nov 1, 2022

With the anchors, the -x option should not be necessary anymore.
__

The current code with the ending anchor will fail with something like this

static ip_address = 192.168.0.5 #comment

I think the trailing anchor should be removed.

@StephanPillhofer
Copy link
Contributor Author

StephanPillhofer commented Nov 1, 2022

You are right, didn't think about the possibility of adding a comment at the end. Also the -x can be removed. Should I create a new PR to fix it?

New Regex will be: regex="^[ \t]*static ip_address[ \t]*=[ \t]*${IPV4_ADDRESS}" (tested)

@yubiuser
Copy link
Member

yubiuser commented Nov 1, 2022

I also tested with different regex/grep combinations and came to the same conclusion. Please open a new PR if you want :)

@dschaper
Copy link
Member

dschaper commented Nov 1, 2022

Is an inline trailing comment a valid configuration?

@yubiuser
Copy link
Member

yubiuser commented Nov 1, 2022

The manual says

Comments may be placed anywhere within the file (except within quotes). Comments begin with the # character and end at the end of the line.


Ok, this was for dhcpd.conf. Sorry.

@dschaper
Copy link
Member

dschaper commented Nov 1, 2022

Can you link to that please?

The only thing I have found is

The first word on the line is the option and the rest of the line is the value. Leading and trailing whitespace for the option and value are trimmed. You can escape characters in the value using the \ character. Comments can be prefixed with the # character. String values should be quoted with the " character.

From https://manpages.debian.org/testing/dhcpcd5/dhcpcd.conf.5.en.html

@StephanPillhofer
Copy link
Contributor Author

So in the worst case the comment may be part of the "value" part which would not be good.

@yubiuser
Copy link
Member

yubiuser commented Nov 1, 2022

@StephanPillhofer
Copy link
Contributor Author

Apparently it seems to be valid:

noipv6rs                 # disable routing solicitation
denyinterfaces eth2      # Don't touch eth2 at all
interface eth0
  ipv6rs                 # enable routing solicitation for eth0
  ia_na 1                # request an IPv6 address
  ia_pd 2 eth1/0         # request a PD and assign it to eth1
  ia_pd 3 eth2/1 eth3/2  # req a PD and assign it to eth2 and eth3

From: https://www.daemon-systems.org/man/dhcpcd.conf.5.html (scroll a bit down)

@StephanPillhofer
Copy link
Contributor Author

The relevant code section should be

https://github.com/NetworkConfiguration/dhcpcd/blob/6a9994bbf00712ace77a835cb7e45558383be673/src/common.c#L181-L195

My c is not the best but I would also see the code as an indication that it is valid to do inline comments.

@dschaper
Copy link
Member

dschaper commented Nov 1, 2022

Yeah, the c code that Yubi linked to does truncate the lines at the first #, so trailing line comments are perfectly valid.

@dschaper
Copy link
Member

dschaper commented Nov 1, 2022

My c is not great either but my Stack Overflow is Ninja level.

@StephanPillhofer
Copy link
Contributor Author

New PR created: #4998

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants