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

Leaving network interface and resolved configuration to the system #364

Closed
wants to merge 2 commits into from

Conversation

Yayg
Copy link

@Yayg Yayg commented May 2, 2024

Hi,

First thanks for maintaining this package which is really helpful for custom installation.
I have a proposal here because I ran into the same problems as people in issues:

Thus the proposal is to leave network interface and resolved configuration to the system administrator.
Now that being said most of this configuration is handled by Debian installer so it should not be a problem for anyone.

Sincerely

Yayg added 2 commits May 2, 2024 11:51
Instead of overwriting etc/network/interfaces just add a custom file in
etc/network/interfaces.d/ for home-assistant purposes if necessary.
This was conflicting with basic Debian installation.
While installing debian comes with a configuration for resolving and
resolved is not necessary and causing problems in some cases (no IPv6,
custom network setup, etc...)
Thus it might be better to leave this configuration to the system
administrator while installing the supervised way.
Copy link

@home-assistant home-assistant bot left a comment

Choose a reason for hiding this comment

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

Hi @Yayg

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

@home-assistant home-assistant bot marked this pull request as draft May 2, 2024 10:04
@home-assistant
Copy link

home-assistant bot commented May 2, 2024

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

@Yayg Yayg marked this pull request as ready for review May 2, 2024 10:05
@agners
Copy link
Member

agners commented Jun 21, 2024

We do mention that systemd-resolved must be installed in the README, so this commands should work in general. Also, systemd-resolved is mandatory for Supervisor, as the DNS plug-in uses it to resolve things via mDNS for the Core/Supervisor/Add-ons etc.

But if we can get rid of unnecessary interference, I am not against it. Is installing systemd-resolved automatically enabling it on Debian 12, so that manually enabling it no longer necessary?

@Yayg
Copy link
Author

Yayg commented Jun 21, 2024

We do mention that systemd-resolved must be installed in the README, so this commands should work in general. Also, systemd-resolved is mandatory for Supervisor, as the DNS plug-in uses it to resolve things via mDNS for the Core/Supervisor/Add-ons etc.

Systemd-resolved is installed and the configuration should not be directly changed because it will conflict with the maintainer's default. Also it creates situations where the URL becomes unresolved because the configuration given is not correct ATM.

But if we can get rid of unnecessary interference, I am not against it. Is installing systemd-resolved automatically enabling it on Debian 12, so that manually enabling it no longer necessary?

Yes the necessary is already there in the base package. Also, as you mentioned, it is explicitly said in the supervised guide to install the dependencies, so I guess leave it all to the system is the best way to go.

Same for interfaces config, they broke the network for me.

@ikifar2012
Copy link
Member

ikifar2012 commented Jun 21, 2024

While I do agree the NetworkManager config has caused issues in some scenarios and I am looking into possible solutions to that, I do not think removing systemd-resolved is necessary

@Yayg
Copy link
Author

Yayg commented Jun 22, 2024

While I do agree the NetworkManager config has caused issues in some scenarios and I am looking into possible solutions to that, I do not think removing systemd-resolved is necessary

As seen and detailed in #339 systemd-resolved is problematic with the same consequences.

Depending on the network setup the user may have to specify manually in the configuration it's internal DNS.

Moreover, any reinstallation of the package wipes the configuration handwritten if necessary.

@Yayg
Copy link
Author

Yayg commented Jun 22, 2024

Btw, if any configuration should be added by HA or it should be in etc/systemd/resolved.conf.d/. Not directly in the main configuration file.

@litinoveweedle
Copy link

Depending on the network setup the user may have to specify manually in the configuration it's internal DNS.

Moreover, any reinstallation of the package wipes the configuration handwritten if necessary.

I can confirm this, I reported also in #374. During each package upgrade, install or re-install my DNS setting are lost and manual recovery is required. At least now I already know what to do, as for few first times I had to go through systemd DNS implementation rabbit hole. Again my understanding is, that supervised is for skilled admin users, who probably already have working system setup including networking and can maintain it (if the decide even without NetworkManager).

Copy link
Member

@agners agners left a comment

Choose a reason for hiding this comment

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

While I do agree the NetworkManager config has caused issues in some scenarios and I am looking into possible solutions to that, I do not think removing systemd-resolved is necessary

From what I can see, this PR does not remove the dependency to systemd-resolved (no changes to the control file). And this is good, since we use systemd-resolved to do mDNS resolution in the Supervisor (add-ons/Core) ecosystem via CoreDNS plug-in. So systemd-resolved is indeed a hard dependency of the Supervised installation method.

From my standpoint it makes sense to follow Debian best practice and not overwrite or mess otherwise with other packages. From what I can tell, at least today, the systemd-resolved package should enable systemd-resolved by default (at least this is what the package description says: "Installing this package automatically overwrites /etc/resolv.conf and switches it to be managed by systemd-resolved.").

Comment on lines -18 to -19
DNSSEC=no
DNSOverTLS=no
Copy link
Member

Choose a reason for hiding this comment

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

These two configs got most likely copied over from the HAOS config at home-assistant/operating-system@e3120df.

Buildroot (used to build HAOS) by default sets DNSOverTLS to opportunistic mode, which did cause issues in certain cases. Hence we opt out of it. Debian uses the systemd default configuration which is off.

For DNSSEC, Debian explicitly turns it off by default.

So it is fairly safe to not set these two options, as their default is no already.

#MulticastDNS=yes
#LLMNR=yes
#Cache=yes
DNSStubListener=no
Copy link
Member

Choose a reason for hiding this comment

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

This setting most likely came from HAOS too, particularly this commit: home-assistant/operating-system@7c25f7c

To be safe, we should keep this config by default, but as @Yayg mentions, in a file at etc/systemd/resolved.conf.d/.

Copy link

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days.
Thank you for your contributions.

@github-actions github-actions bot added the stale label Oct 26, 2024
@github-actions github-actions bot closed this Nov 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants