Skip to content

wrappers,tests: add ExecCondition=snap routine user-service-precondition - #17380

Open
olivercalder wants to merge 10 commits into
canonical:masterfrom
olivercalder:user-service-exec-condition
Open

wrappers,tests: add ExecCondition=snap routine user-service-precondition#17380
olivercalder wants to merge 10 commits into
canonical:masterfrom
olivercalder:user-service-exec-condition

Conversation

@olivercalder

Copy link
Copy Markdown
Member

This PR is based on #17379. It's the third and final in a chain to implement ExecCondition checks to prevent user daemons from running within sessions with class greeter. Spec here: https://docs.google.com/document/d/1PeWQrLDExIb5I7HqDYm1dn9WzQcqksKBn5a6V5ASvoI/edit

We want to ensure that user daemon services do not run for user sessions with class greeter. To do this, we add an ExecCondition stanza to user daemon service files which calls snap routine user-service-precondition.

However, services may also have SuccessExitStatus values set, so we need to ensure that snap routine user-service-precondition knows not to exit with an error code which would be misunderstood to be a success. Thus, the service unit generator considers SuccessExitStatus, and if 1 is a success code, it finds some non-success code and passes that to the snap routine user-service-precondition call via the optional --error-exit-code argument.

CC @aleasto

This work is tracked internally by https://warthogs.atlassian.net/browse/SNAPDENG-37245

Add a new `logind` package similar to the existing `systemd` package.
Add a private function to invoke `loginctl`, and expose a `SessionClass`
method which calls `loginctl show-session auto -p Class` and parses the
resulting `Class=foo` to return `foo`.

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@github-actions

github-actions Bot commented Jul 16, 2026

Copy link
Copy Markdown

Wed Jul 22 15:26:24 UTC 2026
The following results are from: https://github.com/canonical/snapd/actions/runs/29927458414

No spread failures reported

@olivercalder
olivercalder force-pushed the user-service-exec-condition branch from 0f02775 to 2dae8ae Compare July 16, 2026 23:57
@codecov

codecov Bot commented Jul 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.45977% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.92%. Comparing base (91bc64b) to head (c41d019).
⚠️ Report is 5 commits behind head on master.

Files with missing lines Patch % Lines
systemd/logind/logind.go 65.85% 13 Missing and 1 partial ⚠️
...snapd/cli/cmd_routine_user_service_precondition.go 86.36% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #17380      +/-   ##
==========================================
- Coverage   78.92%   78.92%   -0.01%     
==========================================
  Files        1395     1393       -2     
  Lines      195977   196016      +39     
  Branches     2466     2466              
==========================================
+ Hits       154680   154705      +25     
- Misses      32018    32026       +8     
- Partials     9279     9285       +6     
Flag Coverage Δ
unittests 78.92% <80.45%> (-0.01%) ⬇️

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

☔ View full report in Codecov by Harness.
📢 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.

Comment thread wrappers/internal/service_unit_gen.go Outdated
Comment thread wrappers/internal/service_unit_gen.go
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
Add a new `snap routine` subcommand designed to exit with an error code
if invoked from within a systemd session with class which is`greeter`.
The intent is for this to be called from `ExecCondition` stanzas in
systemd unit files. Thus, the subcommand is hidden.

Because this will be invoked by `systemd` via `ExecCondition` when
deciding whether to start the service, the `SuccessExitStatus` also
applies. Thus, we need to be sure not to exit with an error code which
happens to match one of the success codes. To accomplish this, the new
`snap routine user-service-precondition` command includes an optional
argument `--error-exit-code` which takes an integer as an argument,
defaulting to 1. The caller must ensure that the exit code passed in is
not a success exit status.

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
In order to exit with a particular error code, a panic was used, which
was then caught and the code used to exit cleanly. However, this is
unnecessary for `snap routine user-service-precondition`, we can instead
simply add a new error type and handle it like other special error cases
in `exitCodeFromError`.

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
@olivercalder
olivercalder force-pushed the user-service-exec-condition branch from 2dae8ae to c41d019 Compare July 21, 2026 03:32
@olivercalder
olivercalder requested a review from bboozzoo July 21, 2026 03:33
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
We want to ensure that user daemon services do not run for user sessions
with class greeter. To do this, we add an ExecCondition stanza to user
daemon service files which calls `snap routine user-service-precondition`.

However, services may also have `SuccessExitStatus` values set, so we
need to ensure that `snap routine user-service-precondition` knows not
to exit with an error code which would be misunderstood to be a success.
Thus, the service unit generator considers `SuccessExitStatus`, and if
`1` is a success code, it finds some non-success code and passes that to
the `snap routine user-service-precondition` call via the optional
`--error-exit-code` argument.

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
@olivercalder
olivercalder force-pushed the user-service-exec-condition branch from c41d019 to 5efedb6 Compare July 22, 2026 14:12

@pedronis pedronis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do we understand what happens with the services with the ExitCondition on a snapd revert?

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.

4 participants