Merge pull request #3162 from keszybz/alias-refusal

Refuse Alias, DefaultInstance, templated units in install (as appropriate)

12 files changed, 75 insertions, 29 deletions

diff --git a/src/core/automount.c b/src/core/automount.c
index 7c55d7bc49..1239a0efc6 100644
--- a/src/core/automount.c
+++ b/src/core/automount.c
@@ -1050,9 +1050,6 @@ const UnitVTable automount_vtable = {
                 "Automount\0"
                 "Install\0",
 
-        .no_alias = true,
-        .no_instances = true,
-
         .init = automount_init,
         .load = automount_load,
         .done = automount_done,
diff --git a/src/core/busname.c b/src/core/busname.c
index f4f433340c..e7b7b5c012 100644
--- a/src/core/busname.c
+++ b/src/core/busname.c
@@ -1028,9 +1028,6 @@ const UnitVTable busname_vtable = {
                 "Install\0",
         .private_section = "BusName",
 
-        .no_alias = true,
-        .no_instances = true,
-
         .init = busname_init,
         .done = busname_done,
         .load = busname_load,
diff --git a/src/core/device.c b/src/core/device.c
index d01bec53d8..16e56efcc3 100644
--- a/src/core/device.c
+++ b/src/core/device.c
@@ -841,8 +841,6 @@ const UnitVTable device_vtable = {
                 "Device\0"
                 "Install\0",
 
-        .no_instances = true,
-
         .init = device_init,
         .done = device_done,
         .load = unit_load_fragment_and_dropin_optional,
diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c
index 31b995aa6a..1a8c03904c 100644
--- a/src/core/load-fragment.c
+++ b/src/core/load-fragment.c
@@ -3612,7 +3612,7 @@ static int load_from_path(Unit *u, const char *path) {
                 /* Hmm, no suitable file found? */
                 return 0;
 
-        if (UNIT_VTABLE(u)->no_alias && set_size(symlink_names) > 1) {
+        if (!unit_type_may_alias(u->type) && set_size(symlink_names) > 1) {
                 log_unit_warning(u, "Unit type of %s does not support alias names, refusing loading via symlink.", u->id);
                 return -ELOOP;
         }
diff --git a/src/core/mount.c b/src/core/mount.c
index cc07873b24..c8a898e4dc 100644
--- a/src/core/mount.c
+++ b/src/core/mount.c
@@ -1839,9 +1839,6 @@ const UnitVTable mount_vtable = {
                 "Install\0",
         .private_section = "Mount",
 
-        .no_alias = true,
-        .no_instances = true,
-
         .init = mount_init,
         .load = mount_load,
         .done = mount_done,
diff --git a/src/core/scope.c b/src/core/scope.c
index 7078d1f7e9..238f63a729 100644
--- a/src/core/scope.c
+++ b/src/core/scope.c
@@ -569,8 +569,6 @@ const UnitVTable scope_vtable = {
                 "Install\0",
         .private_section = "Scope",
 
-        .no_alias = true,
-        .no_instances = true,
         .can_transient = true,
 
         .init = scope_init,
diff --git a/src/core/slice.c b/src/core/slice.c
index 63a77c9bca..c7700b8857 100644
--- a/src/core/slice.c
+++ b/src/core/slice.c
@@ -309,8 +309,6 @@ const UnitVTable slice_vtable = {
                 "Install\0",
         .private_section = "Slice",
 
-        .no_alias = true,
-        .no_instances = true,
         .can_transient = true,
 
         .init = slice_init,
diff --git a/src/core/swap.c b/src/core/swap.c
index d8802470d2..c018648d87 100644
--- a/src/core/swap.c
+++ b/src/core/swap.c
@@ -1465,9 +1465,6 @@ const UnitVTable swap_vtable = {
                 "Install\0",
         .private_section = "Swap",
 
-        .no_alias = true,
-        .no_instances = true,
-
         .init = swap_init,
         .load = swap_load,
         .done = swap_done,
diff --git a/src/core/unit.c b/src/core/unit.c
index 64466e4fb4..4ace6b075b 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -193,7 +193,7 @@ int unit_add_name(Unit *u, const char *text) {
         if (r < 0)
                 return r;
 
-        if (i && unit_vtable[t]->no_instances)
+        if (i && !unit_type_may_template(t))
                 return -EINVAL;
 
         /* Ensure that this unit is either instanced or not instanced,
@@ -202,7 +202,7 @@ int unit_add_name(Unit *u, const char *text) {
         if (u->type != _UNIT_TYPE_INVALID && !u->instance != !i)
                 return -EINVAL;
 
-        if (unit_vtable[t]->no_alias && !set_isempty(u->names))
+        if (!unit_type_may_alias(t) && !set_isempty(u->names))
                 return -EEXIST;
 
         if (hashmap_size(u->manager->units) >= MANAGER_MAX_NAMES)
@@ -720,7 +720,7 @@ int unit_merge(Unit *u, Unit *other) {
         if (!u->instance != !other->instance)
                 return -EINVAL;
 
-        if (UNIT_VTABLE(u)->no_alias) /* Merging only applies to unit names that support aliases */
+        if (!unit_type_may_alias(u->type)) /* Merging only applies to unit names that support aliases */
                 return -EEXIST;
 
         if (other->load_state != UNIT_STUB &&
diff --git a/src/core/unit.h b/src/core/unit.h
index 5909652976..be62e88421 100644
--- a/src/core/unit.h
+++ b/src/core/unit.h
@@ -416,12 +416,6 @@ struct UnitVTable {
         /* The strings to print in status messages */
         UnitStatusMessageFormats status_message_formats;
 
-        /* Can units of this type have multiple names? */
-        bool no_alias:1;
-
-        /* Instances make no sense for this type */
-        bool no_instances:1;
-
         /* True if transient units of this type are OK */
         bool can_transient:1;
 };
diff --git a/src/shared/install.c b/src/shared/install.c
index 931d3e2907..f02d81504f 100644
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -68,6 +68,25 @@ typedef struct {
 
 static int unit_file_lookup_state(UnitFileScope scope, const LookupPaths *paths, const char *name, UnitFileState *ret);
 
+bool unit_type_may_alias(UnitType type) {
+        return IN_SET(type,
+                      UNIT_SERVICE,
+                      UNIT_SOCKET,
+                      UNIT_TARGET,
+                      UNIT_DEVICE,
+                      UNIT_TIMER,
+                      UNIT_PATH);
+}
+
+bool unit_type_may_template(UnitType type) {
+        return IN_SET(type,
+                      UNIT_SERVICE,
+                      UNIT_SOCKET,
+                      UNIT_TARGET,
+                      UNIT_TIMER,
+                      UNIT_PATH);
+}
+
 static int in_search_path(const LookupPaths *p, const char *path) {
         _cleanup_free_ char *parent = NULL;
         char **i;
@@ -898,6 +917,36 @@ fail:
         return r;
 }
 
+static int config_parse_alias(
+                const char *unit,
+                const char *filename,
+                unsigned line,
+                const char *section,
+                unsigned section_line,
+                const char *lvalue,
+                int ltype,
+                const char *rvalue,
+                void *data,
+                void *userdata) {
+
+        const char *name;
+        UnitType type;
+
+        assert(filename);
+        assert(lvalue);
+        assert(rvalue);
+
+        name = basename(filename);
+        type = unit_name_to_type(name);
+        if (!unit_type_may_alias(type))
+                return log_syntax(unit, LOG_WARNING, filename, line, 0,
+                                  "Aliases are not allowed for %s units, ignoring.",
+                                  unit_type_to_string(type));
+
+        return config_parse_strv(unit, filename, line, section, section_line,
+                                 lvalue, ltype, rvalue, data, userdata);
+}
+
 static int config_parse_also(
                 const char *unit,
                 const char *filename,
@@ -954,6 +1003,7 @@ static int config_parse_default_instance(
                 void *userdata) {
 
         UnitFileInstallInfo *i = data;
+        const char *name;
         char *printed;
         int r;
 
@@ -961,6 +1011,15 @@ static int config_parse_default_instance(
         assert(lvalue);
         assert(rvalue);
 
+        name = basename(filename);
+        if (unit_name_is_valid(name, UNIT_NAME_INSTANCE))
+                /* When enabling an instance, we might be using a template unit file,
+                 * but we should ignore DefaultInstance silently. */
+                return 0;
+        if (!unit_name_is_valid(name, UNIT_NAME_TEMPLATE))
+                return log_syntax(unit, LOG_WARNING, filename, line, 0,
+                                  "DefaultInstance only makes sense for template units, ignoring.");
+
         r = install_full_printf(i, rvalue, &printed);
         if (r < 0)
                 return r;
@@ -983,7 +1042,7 @@ static int unit_file_load(
                 SearchFlags flags) {
 
         const ConfigTableItem items[] = {
-                { "Install", "Alias",           config_parse_strv,             0, &info->aliases           },
+                { "Install", "Alias",           config_parse_alias,            0, &info->aliases           },
                 { "Install", "WantedBy",        config_parse_strv,             0, &info->wanted_by         },
                 { "Install", "RequiredBy",      config_parse_strv,             0, &info->required_by       },
                 { "Install", "DefaultInstance", config_parse_default_instance, 0, info                     },
@@ -991,6 +1050,8 @@ static int unit_file_load(
                 {}
         };
 
+        const char *name;
+        UnitType type;
         _cleanup_fclose_ FILE *f = NULL;
         _cleanup_close_ int fd = -1;
         struct stat st;
@@ -1000,6 +1061,12 @@ static int unit_file_load(
         assert(info);
         assert(path);
 
+        name = basename(path);
+        type = unit_name_to_type(name);
+        if (unit_name_is_valid(name, UNIT_NAME_TEMPLATE|UNIT_NAME_INSTANCE) &&
+            !unit_type_may_template(type))
+                return log_error_errno(EINVAL, "Unit type %s cannot be templated.", unit_type_to_string(type));
+
         if (!(flags & SEARCH_LOAD)) {
                 r = lstat(path, &st);
                 if (r < 0)
diff --git a/src/shared/install.h b/src/shared/install.h
index 4ffc5a21f2..5812447c5b 100644
--- a/src/shared/install.h
+++ b/src/shared/install.h
@@ -138,6 +138,9 @@ static inline bool UNIT_FILE_INSTALL_INFO_HAS_ALSO(UnitFileInstallInfo *i) {
         return !strv_isempty(i->also);
 }
 
+bool unit_type_may_alias(UnitType type) _const_;
+bool unit_type_may_template(UnitType type) _const_;
+
 int unit_file_enable(
                 UnitFileScope scope,
                 bool runtime,