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 handling of resolv.conf in the chroot #134

Merged
merged 1 commit into from
Jul 14, 2023
Merged

Conversation

sil2100
Copy link
Collaborator

@sil2100 sil2100 commented Jul 12, 2023

We have some logic in place that backs-up the resolv.conf in the chroot, puts the host one in place for package installation and then restores the original resolv.conf so that we don't leak the host config into the images. Sadly, it seems that debootstrap basically copies the resolv.conf from the host system into the chroot it generates. Looking into what live-build was doing, it seems that live-build was always truncating the resolv.conf that we were getting from debootstrap, so let's do the same.
While checking what live-build was doing, I also noticed a nifty conditional that checks if during package installation resolv.conf wasn't substituted with a symlink (example has been given of systemd-resolved or resolvconf), and making sure we leave the new resolv.conf symlink in place. So I added it here as well.

All changes come with testing. Had to add a new test suite for the helper functions.

@sil2100 sil2100 requested a review from mwhudson July 12, 2023 11:03
Copy link
Contributor

@mwhudson mwhudson left a comment

Choose a reason for hiding this comment

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

Looks OK. I wonder how we feel about suggesting to glibc that the default behaviour when resolv.conf is not present should be to assume it contains "nameserver 127.0.0.53"!

@sil2100
Copy link
Collaborator Author

sil2100 commented Jul 13, 2023

The CI is not looking great but that's also present in trunk. I'll try to address that before merging.

@sil2100
Copy link
Collaborator Author

sil2100 commented Jul 14, 2023

Sadly, with the current CI failures which I'm still investigating, we can't reliably get full coverage report. I'll merge this as-is and perform some manual tests before pushing it to later channels.

@sil2100 sil2100 merged commit acb18a7 into main Jul 14, 2023
8 of 10 checks passed
@sil2100 sil2100 deleted the resolv-conf-fixes branch July 14, 2023 10:47
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.

2 participants