diff --git a/src/shared/install.c b/src/shared/install.c index df9f92bddc..711638bd16 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -1708,36 +1708,67 @@ static int install_info_discover_and_check( return install_info_may_process(ret ? *ret : NULL, paths, changes, n_changes); } -int unit_file_verify_alias(const UnitFileInstallInfo *i, const char *dst) { +int unit_file_verify_alias(const UnitFileInstallInfo *i, const char *dst, char **ret_dst) { + _cleanup_free_ char *dst_updated = NULL; int r; - if (unit_name_is_valid(i->name, UNIT_NAME_TEMPLATE) && - !unit_name_is_valid(dst, UNIT_NAME_TEMPLATE) && - !i->default_instance) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), - "Aliases for templates without DefaultInstance must be templates."); + /* Verify that dst is a valid either a valid alias or a valid .wants/.requires symlink for the target + * unit *i. Return negative on error or if not compatible, zero on success. + * + * ret_dst is set in cases where "instance propagation" happens, i.e. when the instance part is + * inserted into dst. It is not normally set, even on success, so that the caller can easily + * distinguish the case where instance propagation occured. + */ - if (unit_name_is_valid(i->name, UNIT_NAME_INSTANCE | UNIT_NAME_TEMPLATE) && - !unit_name_is_valid(dst, UNIT_NAME_TEMPLATE | UNIT_NAME_INSTANCE)) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), - "Aliases for template instances must be a templates or template instances containing the instance name."); + const char *path_alias = strrchr(dst, '/'); + if (path_alias) { + /* This branch covers legacy Alias= function of creating .wants and .requires symlinks. */ + _cleanup_free_ char *dir = NULL; - if (unit_name_is_valid(i->name, UNIT_NAME_INSTANCE | UNIT_NAME_TEMPLATE) && - unit_name_is_valid(dst, UNIT_NAME_INSTANCE)) { - _cleanup_free_ char *src_inst = NULL, *dst_inst = NULL; + path_alias ++; /* skip over slash */ - r = unit_name_to_instance(i->name, &src_inst); + dir = dirname_malloc(dst); + if (!dir) + return log_oom(); + + if (!endswith(dir, ".wants") && !endswith(dir, ".requires")) + return log_warning_errno(SYNTHETIC_ERRNO(EXDEV), + "Invalid path \"%s\" in alias.", dir); + + /* That's the name we want to use for verification. */ + r = unit_symlink_name_compatible(path_alias, i->name); + if (r < 0) + return log_error_errno(r, "Failed to verify alias validity: %m"); + if (r == 0) + return log_warning_errno(SYNTHETIC_ERRNO(EXDEV), + "Invalid unit %s symlink %s.", + i->name, dst); + + } else { + /* If the symlink target has an instance set and the symlink source doesn't, we "propagate + * the instance", i.e. instantiate the symlink source with the target instance. */ + if (unit_name_is_valid(dst, UNIT_NAME_TEMPLATE)) { + _cleanup_free_ char *inst = NULL; + + r = unit_name_to_instance(i->name, &inst); + if (r < 0) + return log_error_errno(r, "Failed to extract instance name from %s: %m", i->name); + + if (r == UNIT_NAME_INSTANCE) { + r = unit_name_replace_instance(dst, inst, &dst_updated); + if (r < 0) + return log_error_errno(r, "Failed to build unit name from %s+%s: %m", + dst, inst); + } + } + + r = unit_validate_alias_symlink_and_warn(dst_updated ?: dst, i->name); if (r < 0) return r; - r = unit_name_to_instance(dst, &dst_inst); - if (r < 0) - return r; - if (!strstr(dst_inst, src_inst?src_inst:i->default_instance)) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), - "Aliases for template instances must be a templates or template instances containing the instance name."); } + *ret_dst = TAKE_PTR(dst_updated); return 0; } @@ -1757,31 +1788,17 @@ static int install_info_symlink_alias( assert(config_path); STRV_FOREACH(s, i->aliases) { - _cleanup_free_ char *alias_path = NULL, *dst = NULL; + _cleanup_free_ char *alias_path = NULL, *dst = NULL, *dst_updated = NULL; q = install_full_printf(i, *s, &dst); if (q < 0) return q; - q = unit_file_verify_alias(i, dst); + q = unit_file_verify_alias(i, dst, &dst_updated); if (q < 0) continue; - if (unit_name_is_valid(i->name, UNIT_NAME_INSTANCE) && - unit_name_is_valid(dst, UNIT_NAME_TEMPLATE)) { - - _cleanup_free_ char *s_copy = NULL, *instance = NULL; - q = unit_name_to_instance(i->name, &instance); - if (q < 0) - return q; - - q = unit_name_replace_instance(dst, instance, &s_copy); - if (q < 0) - return q; - free_and_replace(dst, s_copy); - } - - alias_path = path_make_absolute(dst, config_path); + alias_path = path_make_absolute(dst_updated ?: dst, config_path); if (!alias_path) return -ENOMEM; diff --git a/src/shared/install.h b/src/shared/install.h index 12de2c6609..54d22a45d3 100644 --- a/src/shared/install.h +++ b/src/shared/install.h @@ -187,7 +187,7 @@ int unit_file_changes_add(UnitFileChange **changes, size_t *n_changes, UnitFileC void unit_file_changes_free(UnitFileChange *changes, size_t n_changes); void unit_file_dump_changes(int r, const char *verb, const UnitFileChange *changes, size_t n_changes, bool quiet); -int unit_file_verify_alias(const UnitFileInstallInfo *i, const char *dst); +int unit_file_verify_alias(const UnitFileInstallInfo *i, const char *dst, char **ret_dst); int unit_file_query_preset(UnitFileScope scope, const char *root_dir, const char *name); diff --git a/src/test/test-install-root.c b/src/test/test-install-root.c index 323e1124ba..5b50f28118 100644 --- a/src/test/test-install-root.c +++ b/src/test/test-install-root.c @@ -1043,6 +1043,152 @@ static void test_preset_multiple_instances(const char *root) { unit_file_changes_free(changes, n_changes); } +static void verify_one( + const UnitFileInstallInfo *i, + const char *alias, + int expected, + const char *updated_name) { + int r; + static const UnitFileInstallInfo *last_info = NULL; + _cleanup_free_ char *alias2 = NULL; + + if (i != last_info) + log_info("-- %s --", (last_info = i)->name); + + r = unit_file_verify_alias(i, alias, &alias2); + log_info_errno(r, "alias %s ← %s: %d/%m (expected %d)%s%s%s", + i->name, alias, r, expected, + alias2 ? " [" : "", alias2 ?: "", alias2 ? "]" : ""); + assert(r == expected); + + /* This is is test for "instance propagation". This propagation matters mostly for WantedBy= and + * RequiredBy= settings, and less so for Alias=. The only case where it should happen is when we have + * an Alias=alias@.service an instantiated template template@instance. In that case the instance name + * should be propagated into the alias as alias@instance. */ + assert(streq_ptr(alias2, updated_name)); +} + +static void test_verify_alias(void) { + const UnitFileInstallInfo + plain_service = { .name = (char*) "plain.service" }, + bare_template = { .name = (char*) "template1@.service" }, + di_template = { .name = (char*) "template2@.service", + .default_instance = (char*) "di" }, + inst_template = { .name = (char*) "template3@inst.service" }, + di_inst_template = { .name = (char*) "template4@inst.service", + .default_instance = (char*) "di" }; + + verify_one(&plain_service, "alias.service", 0, NULL); + verify_one(&plain_service, "alias.socket", -EXDEV, NULL); + verify_one(&plain_service, "alias@.service", -EXDEV, NULL); + verify_one(&plain_service, "alias@inst.service", -EXDEV, NULL); + verify_one(&plain_service, "foo.target.wants/plain.service", 0, NULL); + verify_one(&plain_service, "foo.target.wants/plain.socket", -EXDEV, NULL); + verify_one(&plain_service, "foo.target.wants/plain@.service", -EXDEV, NULL); + verify_one(&plain_service, "foo.target.wants/service", -EXDEV, NULL); + verify_one(&plain_service, "foo.target.requires/plain.service", 0, NULL); + verify_one(&plain_service, "foo.target.requires/plain.socket", -EXDEV, NULL); + verify_one(&plain_service, "foo.target.requires/plain@.service", -EXDEV, NULL); + verify_one(&plain_service, "foo.target.requires/service", -EXDEV, NULL); + verify_one(&plain_service, "foo.target.conf/plain.service", -EXDEV, NULL); + + verify_one(&bare_template, "alias.service", -EXDEV, NULL); + verify_one(&bare_template, "alias.socket", -EXDEV, NULL); + verify_one(&bare_template, "alias@.socket", -EXDEV, NULL); + verify_one(&bare_template, "alias@inst.socket", -EXDEV, NULL); + /* A general alias alias@.service → template1@.service. */ + verify_one(&bare_template, "alias@.service", 0, NULL); + /* Only a specific instance is aliased, see the discussion in https://github.com/systemd/systemd/pull/13119. */ + verify_one(&bare_template, "alias@inst.service", 0, NULL); + verify_one(&bare_template, "foo.target.wants/plain.service", -EXDEV, NULL); + verify_one(&bare_template, "foo.target.wants/plain.socket", -EXDEV, NULL); + verify_one(&bare_template, "foo.target.wants/plain@.service", -EXDEV, NULL); + /* Name mistmatch: we cannot allow this, because plain@foo.service would be pulled in by foo.taget, + * but would not be resolvable on its own, since systemd doesn't know how to load the fragment. */ + verify_one(&bare_template, "foo.target.wants/plain@foo.service", -EXDEV, NULL); + verify_one(&bare_template, "foo.target.wants/template1@foo.service", 0, NULL); + verify_one(&bare_template, "foo.target.wants/service", -EXDEV, NULL); + verify_one(&bare_template, "foo.target.requires/plain.service", -EXDEV, NULL); + verify_one(&bare_template, "foo.target.requires/plain.socket", -EXDEV, NULL); + verify_one(&bare_template, "foo.target.requires/plain@.service", -EXDEV, NULL); /* instance missing */ + verify_one(&bare_template, "foo.target.requires/template1@inst.service", 0, NULL); + verify_one(&bare_template, "foo.target.requires/service", -EXDEV, NULL); + verify_one(&bare_template, "foo.target.conf/plain.service", -EXDEV, NULL); + + verify_one(&di_template, "alias.service", -EXDEV, NULL); + verify_one(&di_template, "alias.socket", -EXDEV, NULL); + verify_one(&di_template, "alias@.socket", -EXDEV, NULL); + verify_one(&di_template, "alias@inst.socket", -EXDEV, NULL); + verify_one(&di_template, "alias@inst.service", 0, NULL); + verify_one(&di_template, "alias@.service", 0, NULL); + verify_one(&di_template, "alias@di.service", 0, NULL); + verify_one(&di_template, "foo.target.wants/plain.service", -EXDEV, NULL); + verify_one(&di_template, "foo.target.wants/plain.socket", -EXDEV, NULL); + verify_one(&di_template, "foo.target.wants/plain@.service", -EXDEV, NULL); + verify_one(&di_template, "foo.target.wants/plain@di.service", -EXDEV, NULL); + verify_one(&di_template, "foo.target.wants/template2@di.service", 0, NULL); + verify_one(&di_template, "foo.target.wants/service", -EXDEV, NULL); + verify_one(&di_template, "foo.target.requires/plain.service", -EXDEV, NULL); + verify_one(&di_template, "foo.target.requires/plain.socket", -EXDEV, NULL); + verify_one(&di_template, "foo.target.requires/plain@.service", -EXDEV, NULL); + verify_one(&di_template, "foo.target.requires/plain@di.service", -EXDEV, NULL); + verify_one(&di_template, "foo.target.requires/plain@foo.service", -EXDEV, NULL); + verify_one(&di_template, "foo.target.requires/template2@di.service", 0, NULL); + verify_one(&di_template, "foo.target.requires/service", -EXDEV, NULL); + verify_one(&di_template, "foo.target.conf/plain.service", -EXDEV, NULL); + + verify_one(&inst_template, "alias.service", -EXDEV, NULL); + verify_one(&inst_template, "alias.socket", -EXDEV, NULL); + verify_one(&inst_template, "alias@.socket", -EXDEV, NULL); + verify_one(&inst_template, "alias@inst.socket", -EXDEV, NULL); + verify_one(&inst_template, "alias@inst.service", 0, NULL); + verify_one(&inst_template, "alias@.service", 0, "alias@inst.service"); + verify_one(&inst_template, "alias@di.service", -EXDEV, NULL); + verify_one(&inst_template, "bar.target.wants/plain.service", -EXDEV, NULL); + verify_one(&inst_template, "bar.target.wants/plain.socket", -EXDEV, NULL); + verify_one(&inst_template, "bar.target.wants/plain@.service", -EXDEV, NULL); + verify_one(&inst_template, "bar.target.wants/plain@di.service", -EXDEV, NULL); + verify_one(&inst_template, "bar.target.wants/plain@inst.service", -EXDEV, NULL); + verify_one(&inst_template, "bar.target.wants/template3@foo.service", -EXDEV, NULL); + verify_one(&inst_template, "bar.target.wants/template3@inst.service", 0, NULL); + verify_one(&inst_template, "bar.target.wants/service", -EXDEV, NULL); + verify_one(&inst_template, "bar.target.requires/plain.service", -EXDEV, NULL); + verify_one(&inst_template, "bar.target.requires/plain.socket", -EXDEV, NULL); + verify_one(&inst_template, "bar.target.requires/plain@.service", -EXDEV, NULL); + verify_one(&inst_template, "bar.target.requires/plain@di.service", -EXDEV, NULL); + verify_one(&inst_template, "bar.target.requires/plain@inst.service", -EXDEV, NULL); + verify_one(&inst_template, "bar.target.requires/template3@foo.service", -EXDEV, NULL); + verify_one(&inst_template, "bar.target.requires/template3@inst.service", 0, NULL); + verify_one(&inst_template, "bar.target.requires/service", -EXDEV, NULL); + verify_one(&inst_template, "bar.target.conf/plain.service", -EXDEV, NULL); + + /* explicit alias overrides DefaultInstance */ + verify_one(&di_inst_template, "alias.service", -EXDEV, NULL); + verify_one(&di_inst_template, "alias.socket", -EXDEV, NULL); + verify_one(&di_inst_template, "alias@.socket", -EXDEV, NULL); + verify_one(&di_inst_template, "alias@inst.socket", -EXDEV, NULL); + verify_one(&di_inst_template, "alias@inst.service", 0, NULL); + verify_one(&di_inst_template, "alias@.service", 0, "alias@inst.service"); + verify_one(&di_inst_template, "alias@di.service", -EXDEV, NULL); + verify_one(&di_inst_template, "goo.target.wants/plain.service", -EXDEV, NULL); + verify_one(&di_inst_template, "goo.target.wants/plain.socket", -EXDEV, NULL); + verify_one(&di_inst_template, "goo.target.wants/plain@.service", -EXDEV, NULL); + verify_one(&di_inst_template, "goo.target.wants/plain@di.service", -EXDEV, NULL); + verify_one(&di_inst_template, "goo.target.wants/template4@foo.service", -EXDEV, NULL); + verify_one(&di_inst_template, "goo.target.wants/template4@inst.service", 0, NULL); + verify_one(&di_inst_template, "goo.target.wants/template4@di.service", -EXDEV, NULL); + verify_one(&di_inst_template, "goo.target.wants/service", -EXDEV, NULL); + verify_one(&di_inst_template, "goo.target.requires/plain.service", -EXDEV, NULL); + verify_one(&di_inst_template, "goo.target.requires/plain.socket", -EXDEV, NULL); + verify_one(&di_inst_template, "goo.target.requires/plain@.service", -EXDEV, NULL); + verify_one(&di_inst_template, "goo.target.requires/plain@di.service", -EXDEV, NULL); + verify_one(&di_inst_template, "goo.target.requires/plain@inst.service", -EXDEV, NULL); + verify_one(&di_inst_template, "goo.target.requires/template4@foo.service", -EXDEV, NULL); + verify_one(&di_inst_template, "goo.target.requires/template4@inst.service", 0, NULL); + verify_one(&di_inst_template, "goo.target.requires/service", -EXDEV, NULL); + verify_one(&di_inst_template, "goo.target.conf/plain.service", -EXDEV, NULL); +} + int main(int argc, char *argv[]) { char root[] = "/tmp/rootXXXXXX"; const char *p; @@ -1080,5 +1226,7 @@ int main(int argc, char *argv[]) { assert_se(rm_rf(root, REMOVE_ROOT|REMOVE_PHYSICAL) >= 0); + test_verify_alias(); + return 0; }