diff --git a/man/org.freedesktop.hostname1.xml b/man/org.freedesktop.hostname1.xml
index 09097145a1d..2ce3e87bee1 100644
--- a/man/org.freedesktop.hostname1.xml
+++ b/man/org.freedesktop.hostname1.xml
@@ -390,13 +390,15 @@ node /org/freedesktop/hostname1 {
the TAGS= field from /etc/machine-info.
AddAndRemoveTags() incrementally updates the Tags
- property instead of replacing it. It takes two lists of machine tags: the tags in add
- are added to the current list, and the tags in remove are dropped from it (the
- resulting list is (current ∪ add) \ remove, sorted and deduplicated). A
+ property instead of replacing it. It takes two lists of machine tags: the tags in
+ add are added to the current list, and any tags matching the patterns in
+ remove are dropped from it (the resulting list is (current ∪ add) \
+ remove, sorted and deduplicated). A
key=value tag in
- add replaces any assignment of the same key in the current list. This is useful
- when several independent agents each manage a subset of the tags without racing on a read-modify-write
- of the full list. Each tag must be a valid machine tag, as for SetTags().
+ add replaces any assignment of the same key in the current list. This is useful when
+ several independent agents each manage a subset of the tags without racing on a read-modify-write of
+ the full list. Each tag to add must be a valid machine tag, as for
+ SetTags().
PrettyHostname, IconName, Chassis,
Deployment, Location, and Tags are stored in
diff --git a/src/hostname/hostnamed.c b/src/hostname/hostnamed.c
index 5ad8c0f0862..5093484eed1 100644
--- a/src/hostname/hostnamed.c
+++ b/src/hostname/hostnamed.c
@@ -1677,7 +1677,7 @@ static int method_set_tags(sd_bus_message *m, void *userdata, sd_bus_error *erro
if (!j)
return log_oom();
- r = machine_tag_list_is_valid(tags);
+ r = machine_tag_list_is_valid(tags, /* to_add= */ true);
if (r < 0)
return r;
if (r == 0)
@@ -1728,7 +1728,7 @@ static int method_add_and_remove_tags(sd_bus_message *m, void *userdata, sd_bus_
if (r < 0)
return r;
- r = machine_tag_list_is_valid(add);
+ r = machine_tag_list_is_valid(add, /* to_add= */ true);
if (r < 0)
return r;
if (r == 0) {
@@ -1736,13 +1736,11 @@ static int method_add_and_remove_tags(sd_bus_message *m, void *userdata, sd_bus_
return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid tags to add '%s'", strna(j));
}
- r = machine_tag_list_is_valid(remove);
+ r = machine_tag_list_is_valid(remove, /* to_add= */ false);
if (r < 0)
return r;
- if (r == 0) {
- _cleanup_free_ char *j = strv_join(remove, ":");
- return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid tags to remove '%s'", strna(j));
- }
+ if (r == 0)
+ return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Too many tags patterns to remove.");
context_read_machine_info(c);
@@ -2534,20 +2532,20 @@ static int vl_method_set_tags(sd_varlink *link, sd_json_variant *parameters, sd_
bool reset = sd_json_variant_by_key(parameters, "set");
if (reset) {
- r = machine_tag_list_is_valid(p.set);
+ r = machine_tag_list_is_valid(p.set, /* to_add= */ true);
if (r < 0)
return r;
if (r == 0)
return sd_varlink_error_invalid_parameter_name(link, "set");
}
- r = machine_tag_list_is_valid(p.add);
+ r = machine_tag_list_is_valid(p.add, /* to_add= */ true);
if (r < 0)
return r;
if (r == 0)
return sd_varlink_error_invalid_parameter_name(link, "add");
- r = machine_tag_list_is_valid(p.remove);
+ r = machine_tag_list_is_valid(p.remove, /* to_add= */ false);
if (r < 0)
return r;
if (r == 0)
diff --git a/src/shared/machine-tags.c b/src/shared/machine-tags.c
index 1e7d5446e91..b0883b76b54 100644
--- a/src/shared/machine-tags.c
+++ b/src/shared/machine-tags.c
@@ -59,18 +59,40 @@ static int machine_tag_key(const char *tag, char **ret) {
return 0;
}
-int machine_tag_list_is_valid(char **l) {
+int machine_tag_list_is_valid(char **l, bool to_add) {
int r;
- r = machine_tags_from_strv(l, /* graceful= */ false, /* ret= */ NULL);
- switch (r) {
- case -EINVAL:
- case -E2BIG:
- return false;
- case 0:
+ if (to_add) {
+ r = machine_tags_from_strv(l, /* graceful= */ false, /* ret= */ NULL);
+ switch (r) {
+ case -EINVAL:
+ case -E2BIG:
+ return false;
+ case 0:
+ return true;
+ default:
+ return r;
+ }
+ } else {
+ size_t n = 0, n_globs = 0;
+
+ STRV_FOREACH(i, l) {
+ if (++n > MACHINE_TAGS_MAX)
+ return false;
+ if (!in_charset(*i, ALPHANUMERICAL "-.=[]?*"))
+ return false;
+
+ size_t len = strlen(*i);
+ if (len <= 0 || len >= 256)
+ return false;
+
+ /* Globs are potentially quite expensive to evaluate. Use a stricter limit for them. */
+ n_globs += string_is_glob(*i);
+ if (n_globs > MACHINE_TAG_GLOBS_MAX)
+ return false;
+ }
+
return true;
- default:
- return r;
}
}
@@ -242,21 +264,24 @@ int machine_tags_edit(char* const *base, char* const *add, char* const *remove,
_cleanup_hashmap_free_ Hashmap *h = NULL;
int r;
+ /* The removal is done first. It can contain globs which are used to remove
+ * existing entries. The additions are done after the removals. */
+
r = tags_make_hashmap(base, &h);
if (r < 0) {
assert(r == -ENOMEM);
return log_oom();
}
+ STRV_FOREACH(i, remove)
+ tags_remove_matching(h, *i);
+
STRV_FOREACH(i, add) {
r = tags_add(&h, *i);
if (r < 0)
return r;
}
- STRV_FOREACH(i, remove)
- tags_remove_matching(h, *i);
-
return tags_hashmap_to_strv(h, ret);
}
diff --git a/src/shared/machine-tags.h b/src/shared/machine-tags.h
index 6215bb59541..9ccdeac1c69 100644
--- a/src/shared/machine-tags.h
+++ b/src/shared/machine-tags.h
@@ -4,9 +4,10 @@
#include "forward.h"
#define MACHINE_TAGS_MAX 1024U
+#define MACHINE_TAG_GLOBS_MAX 64U
bool machine_tag_is_valid(const char *s);
-int machine_tag_list_is_valid(char **l);
+int machine_tag_list_is_valid(char **l, bool to_add);
int machine_tags_from_string(const char *s, bool graceful, char ***ret);
int machine_tags_from_strv(char **l, bool graceful, char ***ret);
diff --git a/src/shared/varlink-io.systemd.Hostname.c b/src/shared/varlink-io.systemd.Hostname.c
index b3323215ac8..f0c72c230ee 100644
--- a/src/shared/varlink-io.systemd.Hostname.c
+++ b/src/shared/varlink-io.systemd.Hostname.c
@@ -121,11 +121,11 @@ static SD_VARLINK_DEFINE_METHOD(
static SD_VARLINK_DEFINE_METHOD(
SetTags,
- SD_VARLINK_FIELD_COMMENT("If specified, the machine tag list is first reset to exactly these tags, before the 'add' and 'remove' fields are applied. If null, the current tag list is used as basis instead."),
+ SD_VARLINK_FIELD_COMMENT("If specified, the machine tag list is first reset to exactly these tags, before the 'remove' and 'add' fields are applied. If null, the current tag list is used as basis instead."),
SD_VARLINK_DEFINE_INPUT(set, SD_VARLINK_STRING, SD_VARLINK_NULLABLE|SD_VARLINK_ARRAY),
SD_VARLINK_FIELD_COMMENT("Machine tags to add to the list. A key=value tag replaces any assignment of the same key already in the list."),
SD_VARLINK_DEFINE_INPUT(add, SD_VARLINK_STRING, SD_VARLINK_NULLABLE|SD_VARLINK_ARRAY),
- SD_VARLINK_FIELD_COMMENT("Machine tags to remove from the list."),
+ SD_VARLINK_FIELD_COMMENT("Machine tags to remove from the list. The removals are processed before the additions. Removals may include globs."),
SD_VARLINK_DEFINE_INPUT(remove, SD_VARLINK_STRING, SD_VARLINK_NULLABLE|SD_VARLINK_ARRAY),
VARLINK_DEFINE_POLKIT_INPUT);
diff --git a/src/test/test-machine-tags.c b/src/test/test-machine-tags.c
index 628c5cd7f3c..9573cb2e10d 100644
--- a/src/test/test-machine-tags.c
+++ b/src/test/test-machine-tags.c
@@ -58,21 +58,41 @@ TEST(machine_tag_is_valid) {
}
TEST(machine_tag_list_is_valid) {
- ASSERT_OK_POSITIVE(machine_tag_list_is_valid(NULL)); /* empty list is valid */
- ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("a")));
- ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("foo", "bar", "c-d.e")));
- ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("foo=uuu", "bar=qqqq", "c-d.e")));
- ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("foo", "foo=aa"))); /* bare key and assignment coexist */
- ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("foo=1", "foobar=2"))); /* one key is a prefix of the other */
- ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("ab=1", "a=2"))); /* ... and the other way around */
+ ASSERT_OK_POSITIVE(machine_tag_list_is_valid(NULL, /* to_add= */ true)); /* empty list is valid */
+ ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("a"), true));
+ ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("foo", "bar", "c-d.e"), true));
+ ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("foo=uuu", "bar=qqqq", "c-d.e"), true));
+ ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("foo", "foo=aa"), true)); /* bare key and assignment coexist */
+ ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("foo=1", "foobar=2"), true)); /* one key is a prefix of the other */
+ ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("ab=1", "a=2"), true)); /* ... and the other way around */
- ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("foo=aa", "foo=aa"))); /* same key + same value is OK */
- ASSERT_OK_ZERO(machine_tag_list_is_valid(STRV_MAKE("foo", "b:c")));
- ASSERT_OK_ZERO(machine_tag_list_is_valid(STRV_MAKE("foo", "")));
- ASSERT_OK_ZERO(machine_tag_list_is_valid(STRV_MAKE("foo=aa", "foo=b"))); /* same key, different value */
- ASSERT_OK_ZERO(machine_tag_list_is_valid(STRV_MAKE("a=1", "b=2", "a=3"))); /* ... also when not adjacent */
- ASSERT_OK_ZERO(machine_tag_list_is_valid(STRV_MAKE("foo=aa", "bar", "foo=aa", "foo=b")));
- ASSERT_OK_ZERO(machine_tag_list_is_valid(STRV_MAKE("=aa")));
+ ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("foo=aa", "foo=aa"), true)); /* same key + same value is OK */
+ ASSERT_OK_ZERO(machine_tag_list_is_valid(STRV_MAKE("foo", "b:c"), true));
+ ASSERT_OK_ZERO(machine_tag_list_is_valid(STRV_MAKE("foo", ""), true));
+ ASSERT_OK_ZERO(machine_tag_list_is_valid(STRV_MAKE("foo=aa", "foo=b"), true)); /* same key, different value */
+ ASSERT_OK_ZERO(machine_tag_list_is_valid(STRV_MAKE("a=1", "b=2", "a=3"), true)); /* ... also when not adjacent */
+ ASSERT_OK_ZERO(machine_tag_list_is_valid(STRV_MAKE("foo=aa", "bar", "foo=aa", "foo=b"), true));
+ ASSERT_OK_ZERO(machine_tag_list_is_valid(STRV_MAKE("=aa"), true));
+ ASSERT_OK_ZERO(machine_tag_list_is_valid(STRV_MAKE("ab=1*"), true)); /* globs */
+ ASSERT_OK_ZERO(machine_tag_list_is_valid(STRV_MAKE("ab=2[345]"), true));
+
+ ASSERT_OK_POSITIVE(machine_tag_list_is_valid(NULL, /* to_add= */ false)); /* empty list is valid */
+ ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("a"), false));
+ ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("foo", "bar", "c-d.e"), false));
+ ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("foo=uuu", "bar=qqqq", "c-d.e"), false));
+ ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("foo", "foo=aa"), false)); /* bare key and assignment coexist */
+ ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("foo=1", "foobar=2"), false)); /* one key is a prefix of the other */
+ ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("ab=1", "a=2"), false)); /* ... and the other way around */
+
+ ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("foo=aa", "foo=aa"), false)); /* same key + same value is OK */
+ ASSERT_OK_ZERO(machine_tag_list_is_valid(STRV_MAKE("foo", "b:c"), false));
+ ASSERT_OK_ZERO(machine_tag_list_is_valid(STRV_MAKE("foo", ""), false));
+ ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("foo=aa", "foo=b"), false)); /* same key, different value */
+ ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("a=1", "b=2", "a=3"), false)); /* ... also when not adjacent */
+ ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("foo=aa", "bar", "foo=aa", "foo=b"), false));
+ ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("=aa"), false)); /* this is not a valid pattern, but we don't check */
+ ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("ab=1*"), false)); /* globs */
+ ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("ab=2[345]"), false));
}
TEST(machine_tags_from_string) {
diff --git a/test/units/TEST-71-HOSTNAME.sh b/test/units/TEST-71-HOSTNAME.sh
index 3c3bcd98401..414f3ff6011 100755
--- a/test/units/TEST-71-HOSTNAME.sh
+++ b/test/units/TEST-71-HOSTNAME.sh
@@ -492,6 +492,11 @@ testcase_tags() {
AddAndRemoveTags asas 1 three 1 one
assert_eq "$(hostnamectl tags)" "three:two"
+ # Check removal by glob
+ busctl call org.freedesktop.hostname1 /org/freedesktop/hostname1 org.freedesktop.hostname1 \
+ AddAndRemoveTags asas 2 three one 1 'tw*'
+ assert_eq "$(hostnamectl tags)" "one:three"
+
# The SetTags() Varlink method: 'set' resets the list, then 'add'/'remove' are applied on top.
varlinkctl call /run/systemd/io.systemd.Hostname io.systemd.Hostname.SetTags \
'{"set":["x","y"],"add":["z"],"remove":["x"]}'
@@ -511,6 +516,17 @@ testcase_tags() {
varlinkctl call /run/systemd/io.systemd.Hostname io.systemd.Hostname.SetTags '{"set":["bbb"]}'
# Invalid tags are refused.
(! varlinkctl call /run/systemd/io.systemd.Hostname io.systemd.Hostname.SetTags '{"add":["in valid"]}')
+ assert_eq "$(hostnamectl tags)" "bbb"
+
+ # Remove by glob
+ varlinkctl call /run/systemd/io.systemd.Hostname io.systemd.Hostname.SetTags '{"add":["bbb=33"],"remove":["bb*"]}'
+ assert_eq "$(hostnamectl tags)" "bbb=33"
+ varlinkctl call /run/systemd/io.systemd.Hostname io.systemd.Hostname.SetTags '{"add":["aaa=33"],"remove":["bbb","bbb=34"]}'
+ assert_eq "$(hostnamectl tags)" "aaa=33:bbb=33"
+
+ # Removals precede additions
+ varlinkctl call /run/systemd/io.systemd.Hostname io.systemd.Hostname.SetTags '{"add":["aaa=34","bbb=34"],"remove":["aaa=*","bbb=*"]}'
+ assert_eq "$(hostnamectl tags)" "aaa=34:bbb=34"
# Describe() exposes the parsed tag list as MachineTags, on both transports.
varlinkctl call /run/systemd/io.systemd.Hostname io.systemd.Hostname.SetTags '{"set":["foo","bar"]}'