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

interfaces/builtin/hostname_control.go: fix call to hostnamectl in core24 #15196

Merged

Conversation

valentindavid
Copy link
Contributor

Since systemd/systemd#27573 we need to allow binding on the client side of the dbus socket.

Also hostnamectl seems to need to read properties on /org/freedesktop/systemd1 which should be fine.

To reproduce the issue, try to call hostnamectl from a snap on Ubuntu Core 24.

…re24

Since systemd/systemd#27573 we need to allow
binding on the client side of the dbus socket.

Also hostnamectl seems to need to read properties on
/org/freedesktop/systemd1 which should be fine.

To reproduce the issue, try to call `hostnamectl` from a snap
on Ubuntu Core 24.
@Meulengracht Meulengracht added the Needs security review Can only be merged once security gave a :+1: label Mar 12, 2025
Copy link

codecov bot commented Mar 12, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.08%. Comparing base (d6d95f0) to head (9723c65).
Report is 11 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #15196      +/-   ##
==========================================
- Coverage   78.09%   78.08%   -0.02%     
==========================================
  Files        1190     1194       +4     
  Lines      158458   158585     +127     
==========================================
+ Hits       123746   123825      +79     
- Misses      27017    27059      +42     
- Partials     7695     7701       +6     
Flag Coverage Δ
unittests 78.08% <ø> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@bboozzoo bboozzoo left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -65,6 +71,9 @@ dbus(receive, send)
interface=org.freedesktop.hostname1
member=Set{,Pretty,Static}Hostname,

# hostnamectl needs to bind the client side of the socket
unix (bind) type=stream addr="@*/bus/hostnamectl/system",
Copy link
Contributor

Choose a reason for hiding this comment

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

for those left wondering:

$ strace -vf -e bind,connect,socket,recvmsg,sendmsg hostnamectl
socket(AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC|SOCK_NONBLOCK, 0) = 3
bind(3, {sa_family=AF_UNIX, sun_path=@"8ad6795a6c6318c/bus/hostnamectl/system"}, 41) = 0
connect(3, {sa_family=AF_UNIX, sun_path="/run/dbus/system_bus_socket"}, 30) = 0

@bboozzoo bboozzoo requested review from alexmurray and jslarraz March 12, 2025 14:27
Copy link

github-actions bot commented Mar 12, 2025

Fri Mar 14 08:10:05 UTC 2025
The following results are from: https://github.com/canonical/snapd/actions/runs/13812181137

Failures:

Executing:

  • openstack:centos-9-64:tests/main/degraded
  • openstack:opensuse-tumbleweed-64:tests/main/auto-refresh-pre-download:close_mid_restart
  • openstack:opensuse-tumbleweed-64:tests/main/auto-refresh-pre-download:close
  • openstack:opensuse-tumbleweed-64:tests/main/auto-refresh-pre-download:restart
  • openstack:opensuse-tumbleweed-64:tests/main/auto-refresh-gating-from-snap
  • openstack:opensuse-tumbleweed-64:tests/main/auto-refresh:parallel
  • openstack:opensuse-tumbleweed-64:tests/main/auto-refresh:regular
  • openstack:opensuse-tumbleweed-64:tests/main/auto-refresh-gating
  • openstack:opensuse-tumbleweed-64:tests/main/auto-refresh-backoff
  • openstack:opensuse-tumbleweed-64:tests/main/auto-refresh-retry
  • openstack:opensuse-tumbleweed-64:tests/main/snap-refresh-hold
  • openstack:opensuse-tumbleweed-64:tests/main/auto-refresh-pre-download:ignore
  • openstack:opensuse-tumbleweed-64:tests/main/refresh-app-awareness-notify
  • google:ubuntu-25.04-64:tests/main/security-device-cgroups-self-manage
  • google:ubuntu-25.04-64:tests/main/security-device-cgroups-serial-port
  • google:ubuntu-25.04-64:tests/main/security-device-cgroups-helper
  • google:ubuntu-25.04-64:tests/main/security-device-cgroups:uinput
  • google:ubuntu-25.04-64:tests/main/security-device-cgroups:kmsg
  • google:ubuntu-25.04-64:tests/main/snap-user-service-socket-activation
  • google:ubuntu-25.04-64:tests/main/cgroup-devices-v2
  • google:ubuntu-25.04-64:tests/main/snap-ns-forward-compat
  • google:ubuntu-25.04-64:tests/main/security-device-cgroups-strict-enforced
  • google:ubuntu-25.04-64:tests/main/security-device-cgroups-required-or-optional
  • google:ubuntu-25.04-64:tests/main/microk8s-smoke:edge

Restoring:

  • openstack:debian-sid-64:tests/unit/
  • openstack:debian-sid-64:tests/unit/
  • openstack:debian-sid-64:tests/main/interfaces-timeserver-control
  • openstack:opensuse-tumbleweed-64:tests/main/refresh-app-awareness-notify
  • google:ubuntu-25.04-64:tests/main/security-device-cgroups-strict-enforced

Copy link
Contributor

@alexmurray alexmurray left a comment

Choose a reason for hiding this comment

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

LGTM!

@valentindavid valentindavid merged commit 2d2a0d1 into canonical:master Mar 14, 2025
73 of 77 checks passed
@valentindavid valentindavid deleted the valentindavid/hostnamectl-core24 branch March 14, 2025 07:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs security review Can only be merged once security gave a :+1:
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants