1
1
mirror of https://github.com/systemd/systemd-stable.git synced 2024-12-27 03:21:32 +03:00
Commit Graph

22 Commits

Author SHA1 Message Date
Zbigniew Jędrzejewski-Szmek
9aa3d6b41e Revert "shared/install: create relative symlinks for enablement and aliasing"
This reverts commit d6c9411072.

I still think this is something that needs to be done, but we're hitting some
unexplained failures, e.g. https://github.com/systemd/systemd/issues/22920.
So let's revert this for now, so -rc2 can be released, with a plan to return
to this after a release.

Closes #22920.
2022-05-05 19:02:26 +02:00
Zbigniew Jędrzejewski-Szmek
c3a053c241 test-systemctl-enable: skip test for %v if kver is not a valid instance
On arm, we'd fail with:
target@v:5.16.8-200.fc35.armv7hl+lpae.socket: not a valid unit name "target@v:5.16.8-200.fc35.armv7hl+lpae.socket": Invalid argument
2022-03-29 22:39:32 +02:00
Zbigniew Jędrzejewski-Szmek
5c29de29b4 test-systemctl-enable: disable the test for %a for now 2022-03-29 16:17:57 +02:00
Zbigniew Jędrzejewski-Szmek
2a2d002fb0 test-systemctl-enable: also use freshly-built systemd-id128
Tests were failing on centos7 because systemd-id128 is not in path.
2022-03-29 16:17:57 +02:00
Zbigniew Jędrzejewski-Szmek
d6c51c485a test-systemctl-enable: use magic syntax to allow inverted tests
Inspired by 7910ec3bcd.
'! true' passes, because it's a conditional expression.
But '( ! true )' fails, because '( … )' creates a subshell, i.e. a separate
program, and '! true' becomes the return value of that program, and the whole
thing apparently is not a conditional expression for the outer shell.

This is shorter, so let's just do this.
2022-03-29 16:17:57 +02:00
Zbigniew Jędrzejewski-Szmek
3fc53351dc shared/install: when creating symlinks, accept different but equivalent symlinks
We would only accept "identical" links, but having e.g. a symlink
/usr/lib/systemd/system/foo-alias.service → /usr/lib/systemd/system/foo.service
when we're trying to create /usr/lib/systemd/system/foo-alias.service →
./foo.service is OK. This fixes an issue found in ubuntuautopkg package
installation, where we'd fail when enabling systemd-resolved.service, because
the existing alias was absolute, and (with the recent patches) we were trying
to create a relative one.

A test is added.
(For .wants/.requires symlinks we were already doing OK. A test is also
added, to verify.)
2022-03-29 16:17:57 +02:00
Zbigniew Jędrzejewski-Szmek
84fdced62c test-systemctl-enable: make shellcheck happy
Quoting is not necessary in many places, but I think it's nicer
to use it consistently.
2022-03-29 16:17:57 +02:00
Zbigniew Jędrzejewski-Szmek
48eadb9d9b shared/install: fix handling of a linked unit file
When we have a symlink that goes outside of our search path, we should just
ignore the target file name. But we were verifying it, and rejecting in
the case where a symlink was created manually.
2022-03-29 16:17:57 +02:00
Zbigniew Jędrzejewski-Szmek
d6c9411072 shared/install: create relative symlinks for enablement and aliasing
This is a fairly noticable change, but I think it needs to be done.
So far we'd create an absolute symlink to the target unit file:
  .wants/foo.service → /usr/lib/systemd/system/foo.service
or
  alias.service → /etc/systemd/system/aliased.service.

This works reasonably well, except in one case: where the unit file
is linked. When we look at a file link, the name of the physical file
isn't used, and we only take the account the symlink source name.
(In fact, the destination filename may not even be a well-formed unit name,
so we couldn't use it, even if we wanted to.) But this means that if
a file is linked, and specifies aliases, we'd create absolute links for
those aliases, and systemd would consider each "alias" to be a separate
unit. This isn't checked by the tests here, because we don't have a running
systemd instance, but it is easy enough to check manually.

The most reasonable way to fix this is to create relative links to the
unit file:
  .wants/foo.service → ../foo.service
  alias.service → aliased.service.

I opted to use no prefix for aliases, both normal and 'default.target',
and to add "../" for .wants/ and .requires/. Note that the link that is
created doesn't necessarily point to the file. E.g. if we're enabling
a file under /usr/lib/systemd/system, and create a symlink in /etc/systemd/system,
it'll still be "../foo.service", not "../../usr/lib/systemd/system/foo.service".
For our unit loading logic this doesn't matter, and figuring out a path
that actually leads somewhere would be more work. Since the user is allowed
to move the unit file, or add a new unit file in a different location, and
we don't actually follow the symlink, I think it's OK to create a dangling
symlink. The prefix of "../" is useful to give a hint that the link points
to files that are conceptually "one level up" in the directory hierarchy.

With the relative symlinks, systemd knows that those are aliases.

The tests are adjusted to use the new forms. There were a few tests that
weren't really testing something useful: 'test -e x' fails if 'x' is a
a dangling symlink. Absolute links in the chroot would be dangling, even
though the target existed in the expected path, but become non-dangling
when made relative and the test fails.

This should be described in NEWS, but I'm not adding that here, because
it'd likely result in conflicts.
2022-03-29 16:17:56 +02:00
Zbigniew Jędrzejewski-Szmek
9f61c9f79e shared/install: also remove symlinks like .wants/foo@one.service → ../foo@one.service
So far 'systemctl enable' would create absolute links to the target template
name. And we would remove such symlinks just fine. But the user may create
symlinks manually in a different form. In particular, symlinks for instanced
units *must* have the instance in the source name, and then it is natural to
also include it in the target name (.wants/foo@one.service../foo@one.service
rather than .wants/foo@one.service → ../foo@.service). We would choke on such
links, or not remove them at all. A test is added:

before:

+ build-rawhide/systemctl --root=/tmp/systemctl-test.001xda disable templ1@.service
Removed "/tmp/systemctl-test.001xda/etc/systemd/system/services.target.wants/templ1@seven.service".
Removed "/tmp/systemctl-test.001xda/etc/systemd/system/services.target.wants/templ1@six.service".
Removed "/tmp/systemctl-test.001xda/etc/systemd/system/services.target.wants/templ1@five.service".
Removed "/tmp/systemctl-test.001xda/etc/systemd/system/services.target.wants/templ1@four.service".
Removed "/tmp/systemctl-test.001xda/etc/systemd/system/services.target.wants/templ1@three.service".
Failed to disable unit, refusing to operate on linked unit file /tmp/systemctl-test.001xda/etc/systemd/system/services.target.wants/templ1@two.service.
Failed to disable unit, refusing to operate on linked unit file /tmp/systemctl-test.001xda/etc/systemd/system/services.target.wants/templ1@two.service.

after:

+ build-rawhide/systemctl --root=/tmp/systemctl-test.QVP0ev disable templ1@.service
Removed "/tmp/systemctl-test.QVP0ev/etc/systemd/system/services.target.wants/templ1@seven.service".
Removed "/tmp/systemctl-test.QVP0ev/etc/systemd/system/services.target.wants/templ1@six.service".
Removed "/tmp/systemctl-test.QVP0ev/etc/systemd/system/services.target.wants/templ1@five.service".
Removed "/tmp/systemctl-test.QVP0ev/etc/systemd/system/services.target.wants/templ1@four.service".
Removed "/tmp/systemctl-test.QVP0ev/etc/systemd/system/services.target.wants/templ1@three.service".
Removed "/tmp/systemctl-test.QVP0ev/etc/systemd/system/services.target.wants/templ1@two.service".
Removed "/tmp/systemctl-test.QVP0ev/etc/systemd/system/services.target.wants/templ1@one.service".
+ test '!' -h /tmp/systemctl-test.QVP0ev/etc/systemd/system/services.target.wants/templ1@one.service
+ test '!' -h /tmp/systemctl-test.QVP0ev/etc/systemd/system/services.target.wants/templ1@two.service
+ test '!' -h /tmp/systemctl-test.QVP0ev/etc/systemd/system/services.target.wants/templ1@three.service
+ test '!' -h /tmp/systemctl-test.QVP0ev/etc/systemd/system/services.target.wants/templ1@four.service
+ test '!' -h /tmp/systemctl-test.QVP0ev/etc/systemd/system/services.target.wants/templ1@five.service
+ test '!' -h /tmp/systemctl-test.QVP0ev/etc/systemd/system/services.target.wants/templ1@six.service
+ test '!' -h /tmp/systemctl-test.QVP0ev/etc/systemd/system/services.target.wants/templ1@seven.service
2022-03-29 16:17:56 +02:00
Zbigniew Jędrzejewski-Szmek
85516075a2 test-systemctl-enable: enhance the test for unit file linking
Current behaviour is wrong, but it cannot be shown in this test, because we
don't have a running systemd instance here.
2022-03-29 16:17:56 +02:00
Zbigniew Jędrzejewski-Szmek
0c003e8305 test-systemctl-enable: extend the test for repeated WantedBy/RequiredBy
I was considering deduplicating the list of target units in
WantedBy/RequiredBy. But to do this meaningfully, we'd need to do alias
expansion first, i.e. after the initial parsing is done. This seems to be
more trouble than it would be worth.

Instead, I added tests that we're doing the right thing and creating symlinks
as expected. For duplicate links, we create the link, and on the second time we
see that the link is already there, so the output is correct.
2022-03-29 16:17:56 +02:00
Zbigniew Jędrzejewski-Szmek
29a7c59abb shared/install: fix reenable on linked unit files 2022-03-29 16:17:56 +02:00
Zbigniew Jędrzejewski-Szmek
20d68b3aec install: when linking a file, create the link first or abort
We'd create aliases and other symlinks first, and only then try to create
the main link. Since that can fail, let's do things in opposite order, and
abort immediately if we can't link the file itself.
2022-03-29 16:17:56 +02:00
Zbigniew Jędrzejewski-Szmek
17a2679e99 man: fix invalid description of template handling in WantedBy=
We don't need to talk about Alias=. The approach of using Alias= to enable
units is still supported, but hasn't been advertised as the way to do thing
for many years. Using it as an explanation is just confusing.

Also, the description of templated units did not take DefaultInstance=
into account. It is updated and extended.
2022-03-29 16:17:56 +02:00
Zbigniew Jędrzejewski-Szmek
f663e6468f shared/install: also check for self-aliases during installation and ignore them
We had a check that was done in unit_file_resolve_symlink(). Let's move
the check to unit_validate_alias_symlink_or_warn(), which makes it available
to the code in install.c.

With this, unit_file_resolve_symlink() behaves almost the same. The warning
about "suspicious symlink" is done a bit later. I think this should be OK.
2022-03-29 16:17:56 +02:00
Zbigniew Jędrzejewski-Szmek
0d11db5982 shared/install: return failure when enablement fails, but process as much as possible
So far we'd issue a warning (before this series, just in the logs on the server
side, and before this commit, on stderr on the caller's side), but return
success. It seems that successfull return was introduced by mistake in
aa0f357fd8 (my fault :( ), which was supposed to
be a refactoring without a functional change. I think it's better to fail,
because if enablement fails, the user will most likely want to diagnose the
issue.

Note that we still do partial enablement, as far as that is possible. So if
e.g. we have [Install] Alias=foo.service foobar, we'll create the symlink
'foo.service', but not 'foobar', since that's not a valid unit name. We'll
print info about the action taken, and about 'foobar' being invalid, and return
failure.
2022-03-29 16:17:56 +02:00
Zbigniew Jędrzejewski-Szmek
172e9cc3ee shared/specifier: fix %u/%U/%g/%G when called as unprivileged user
We would resolve those specifiers to the calling user/group. This is mostly OK
when done in the manager, because the manager generally operates as root
in system mode, and a non-root in user mode. It would still be wrong if
called with --test though. But in systemctl, this would be generally wrong,
since we can call 'systemctl --system' as a normal user, either for testing
or even for actual operation with '--root=…'.

When operating in --global mode, %u/%U/%g/%G should return an error.

The information whether we're operating in system mode, user mode, or global
mode is passed as the data pointer to specifier_group_name(), specifier_user_name(),
specifier_group_id(), specifier_user_id(). We can't use userdata, because
it's already used for other things.
2022-03-29 16:17:56 +02:00
Zbigniew Jędrzejewski-Szmek
19b9d5d0d1 shared/install: provide proper error messages when invalid specifiers are used
$ build/systemctl --root=/tmp/systemctl-test.KXY8fu enable some-some-link6@.socket
Failed to enable unit, invalid specifier in "target@C:%C.socket".
2022-03-29 16:17:56 +02:00
Zbigniew Jędrzejewski-Szmek
6ec4c852c9 shared/specifier: provide proper error messages when specifiers fail to read files
ENOENT is easily confused with the file that we're working on not being
present, e.g. when the file contains %o or something else that requires
os-release to be present. Let's use -EUNATCH instead to reduce that chances of
confusion if the context of the error is lost.

And once we have pinpointed the reason, let's provide a proper error message:

+ build/systemctl --root=/tmp/systemctl-test.TO7Mcb enable some-some-link6@.socket
/tmp/systemctl-test.TO7Mcb/etc/systemd/system/some-some-link6@.socket: Failed to resolve alias "target@A:%A.socket": Protocol driver not attached
Failed to enable unit, cannot resolve specifiers in "target@A:%A.socket".
2022-03-29 16:17:56 +02:00
Zbigniew Jędrzejewski-Szmek
df78419d10 basic: add new variable $SYSTEMD_OS_RELEASE to override location of os-release
The test for the variable is added in test-systemctl-enable because there we
can do it almost for free, and the variable is most likely to be used with
'systemctl enable --root' anyway.
2022-03-29 16:17:56 +02:00
Zbigniew Jędrzejewski-Szmek
50c5f5a3d9 test: add test for systemctl link & enable
This test has overlap with test-install-root, but it tests things at a
different level, so I think it's useful to add. It immediately shows various
bugs which will be fixed in later patches.
2022-03-29 16:17:56 +02:00