core,install: allow one more case of "instance propagation"

If we have a template unit template@.service, it should be allowed to specify a
dependency on a unit without an instance, bar@.service. When the unit is created,
the instance will be propagated into the target, so template@inst.service will
depend on bar@inst.service.

This commit changes unit_dependency_name_compatible(), which makes the manager
accept links like that, and unit_file_verify_alias(), so that the installation
function will agree to create a symlink like that, and finally the tests are
adjusted to pass.

diff --git a/src/core/load-dropin.c b/src/core/load-dropin.c
index 492e816e91aeb5d7908e14cf0ffe71fe9aa26340..f61e9da6f281d2deaad55f11ba869424ed555a09 100644 (file)
--- a/src/core/load-dropin.c
+++ b/src/core/load-dropin.c
@@ -65,7 +65,7 @@ static int process_deps(Unit *u, UnitDependency dependency, const char *dir_suff
 
                 /* We don't treat this as an error, especially because we didn't check this for a
                  * long time. Nevertheless, we warn, because such mismatch can be mighty confusing. */
-                r = unit_symlink_name_compatible(entry, basename(target));
+                r = unit_symlink_name_compatible(entry, basename(target), u->instance);
                 if (r < 0) {
                         log_unit_warning_errno(u, r, "Can't check if names %s and %s are compatible, ignoring: %m",
                                                entry, basename(target));

diff --git a/src/shared/install.c b/src/shared/install.c
index 711638bd16b31125c17536aeb39034c2e9b18ed2..2e3f2d565a6ada39515b0a6b5d1b4f08bc6fce66 100644 (file)
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -1724,6 +1724,7 @@ int unit_file_verify_alias(const UnitFileInstallInfo *i, const char *dst, char *
         if (path_alias) {
                 /* This branch covers legacy Alias= function of creating .wants and .requires symlinks. */
                 _cleanup_free_ char *dir = NULL;
+                char *p;
 
                 path_alias ++; /* skip over slash */
 
@@ -1731,12 +1732,23 @@ int unit_file_verify_alias(const UnitFileInstallInfo *i, const char *dst, char *
                 if (!dir)
                         return log_oom();
 
-                if (!endswith(dir, ".wants") && !endswith(dir, ".requires"))
+                p = endswith(dir, ".wants");
+                if (!p)
+                        p = endswith(dir, ".requires");
+                if (!p)
                         return log_warning_errno(SYNTHETIC_ERRNO(EXDEV),
                                                  "Invalid path \"%s\" in alias.", dir);
+                *p = '\0'; /* dir should now be a unit name */
+
+                r = unit_name_classify(dir);
+                if (r < 0)
+                        return log_warning_errno(SYNTHETIC_ERRNO(EXDEV),
+                                                 "Invalid unit name component \"%s\" in alias.", dir);
+
+                const bool instance_propagation = r == UNIT_NAME_TEMPLATE;
 
                 /* That's the name we want to use for verification. */
-                r = unit_symlink_name_compatible(path_alias, i->name);
+                r = unit_symlink_name_compatible(path_alias, i->name, instance_propagation);
                 if (r < 0)
                         return log_error_errno(r, "Failed to verify alias validity: %m");
                 if (r == 0)

diff --git a/src/shared/unit-file.c b/src/shared/unit-file.c
index dc0ec784640691528bf32d2acc3a4ca69576ff99..7ad0ecc8fdca4f340d6b38bb339e81cd57e541a2 100644 (file)
--- a/src/shared/unit-file.c
+++ b/src/shared/unit-file.c
@@ -31,12 +31,15 @@ bool unit_type_may_template(UnitType type) {
                       UNIT_PATH);
 }
 
-int unit_symlink_name_compatible(const char *symlink, const char *target) {
+int unit_symlink_name_compatible(const char *symlink, const char *target, bool instance_propagation) {
         _cleanup_free_ char *template = NULL;
-        int r;
+        int r, un_type1, un_type2;
+
+        un_type1 = unit_name_classify(symlink);
 
-        /* The straightforward case: the symlink name matches the target */
-        if (streq(symlink, target))
+        /* The straightforward case: the symlink name matches the target and we have a valid unit */
+        if (streq(symlink, target) &&
+            (un_type1 & (UNIT_NAME_PLAIN | UNIT_NAME_INSTANCE)))
                 return 1;
 
         r = unit_name_template(symlink, &template);
@@ -45,8 +48,22 @@ int unit_symlink_name_compatible(const char *symlink, const char *target) {
         if (r < 0)
                 return r;
 
+        un_type2 = unit_name_classify(target);
+
         /* An instance name points to a target that is just the template name */
-        return streq(template, target);
+        if (un_type1 == UNIT_NAME_INSTANCE &&
+            un_type2 == UNIT_NAME_TEMPLATE &&
+            streq(template, target))
+                return 1;
+
+        /* foo@.target.requires/bar@.service: instance will be propagated */
+        if (instance_propagation &&
+            un_type1 == UNIT_NAME_TEMPLATE &&
+            un_type2 == UNIT_NAME_TEMPLATE &&
+            streq(template, target))
+                return 1;
+
+        return 0;
 }
 
 int unit_validate_alias_symlink_and_warn(const char *filename, const char *target) {

diff --git a/src/shared/unit-file.h b/src/shared/unit-file.h
index 83f78618d3073a2d11d5daadfea57a24e527efce..a44ba5b05182030f54b4e5e904581df1320a4011 100644 (file)
--- a/src/shared/unit-file.h
+++ b/src/shared/unit-file.h
@@ -39,7 +39,7 @@ enum UnitFileScope {
 bool unit_type_may_alias(UnitType type) _const_;
 bool unit_type_may_template(UnitType type) _const_;
 
-int unit_symlink_name_compatible(const char *symlink, const char *target);
+int unit_symlink_name_compatible(const char *symlink, const char *target, bool instance_propagation);
 int unit_validate_alias_symlink_and_warn(const char *filename, const char *target);
 
 int unit_file_build_name_map(

diff --git a/src/test/test-install-root.c b/src/test/test-install-root.c
index 5b50f28118b4aefc955894ed2ff8f666ffbebb66..25498916f1136ee586f51090bad15e2a851e0a4b 100644 (file)
--- a/src/test/test-install-root.c
+++ b/src/test/test-install-root.c
@@ -1091,6 +1091,8 @@ static void test_verify_alias(void) {
         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(&plain_service, "foo.service/plain.service", -EXDEV, NULL); /* missing dir suffix */
+        verify_one(&plain_service, "asdf.requires/plain.service", -EXDEV, NULL); /* invalid unit name component */
 
         verify_one(&bare_template, "alias.service", -EXDEV, NULL);
         verify_one(&bare_template, "alias.socket", -EXDEV, NULL);
@@ -1114,6 +1116,12 @@ static void test_verify_alias(void) {
         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(&bare_template, "FOO@.target.requires/plain@.service", -EXDEV, NULL); /* template name mistatch */
+        verify_one(&bare_template, "FOO@inst.target.requires/plain@.service", -EXDEV, NULL);
+        verify_one(&bare_template, "FOO@inst.target.requires/plain@inst.service", -EXDEV, NULL);
+        verify_one(&bare_template, "FOO@.target.requires/template1@.service", 0, NULL); /* instance propagated */
+        verify_one(&bare_template, "FOO@inst.target.requires/template1@.service", -EXDEV, NULL); /* instance missing */
+        verify_one(&bare_template, "FOO@inst.target.requires/template1@inst.service", 0, NULL); /* instance provided */
 
         verify_one(&di_template, "alias.service", -EXDEV, NULL);
         verify_one(&di_template, "alias.socket", -EXDEV, NULL);
@@ -1133,6 +1141,7 @@ static void test_verify_alias(void) {
         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@.service", -EXDEV, NULL); /* instance missing */
         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);
@@ -1161,6 +1170,13 @@ static void test_verify_alias(void) {
         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);
+        verify_one(&inst_template, "BAR@.target.requires/plain@.service", -EXDEV, NULL); /* template name mistatch */
+        verify_one(&inst_template, "BAR@inst.target.requires/plain@.service", -EXDEV, NULL);
+        verify_one(&inst_template, "BAR@inst.target.requires/plain@inst.service", -EXDEV, NULL);
+        verify_one(&inst_template, "BAR@.target.requires/template3@.service", -EXDEV, NULL); /* instance missing */
+        verify_one(&inst_template, "BAR@inst.target.requires/template3@.service", -EXDEV, NULL); /* instance missing */
+        verify_one(&inst_template, "BAR@inst.target.requires/template3@inst.service", 0, NULL); /* instance provided */
+        verify_one(&inst_template, "BAR@inst.target.requires/template3@ins2.service", -EXDEV, NULL); /* instance mismatch */
 
         /* explicit alias overrides DefaultInstance */
         verify_one(&di_inst_template, "alias.service", -EXDEV, NULL);