Re: [PATCH v2] conf: Don't generate machine names with a dot

[Ján Tomko <jtomko@xxxxxxxxxx>]

On a Thursday in 2020, Michal Privoznik wrote:
> According to the linked BZ, machined expects either valid
> hostname or valid FQDN (see systemd commit
> v239-3092-gd65652f1f2). While in case of multiple dots, a
> trailing one doesn't violate FQDN, it does violate the rule in
> case of something simple, like "domain.". But it's safe to remove
> it in both cases.
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1808499
> Fixes: 45464db8ba502764cf37ec9335770248bdb3d9a8
>
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
>
> v2 of:
>
> https://www.redhat.com/archives/libvir-list/2020-February/msg01138.html
>
> diff to v1:
> - Adjusted commit message as suggested by Jano
> - Fixed ".-" and "-." occurrences too, again suggested by Jano
>
> src/conf/domain_conf.c | 6 +++---
> tests/virsystemdtest.c | 3 ++-
> 2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index d2d97daf80..246a78d39b 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -30933,7 +30933,7 @@ virDomainMachineNameAppendValid(virBufferPtr buf,
>         if (strlen(virBufferCurrentContent(buf)) >= 64)
>             break;
>
> -        if (*name == '.') {
> +        if (*name == '.' || *name == '-') {
>             if (!skip_dot)
>                 virBufferAddChar(buf, *name);
>             skip_dot = true;
> @@ -30948,8 +30948,8 @@ virDomainMachineNameAppendValid(virBufferPtr buf,
>         virBufferAddChar(buf, *name);
>     }
>
> -    /* trailing dashes are not allowed */
> -    virBufferTrimChars(buf, "-");
> +    /* trailing dashes or dots are not allowed */
> +    virBufferTrimChars(buf, "-.");
> }

Since we use '-' as a delimiter, a domain name of .demo could still result in
an invalid name. All that's needed is initializing skip_dot to true.
(Feel free to rename it to 'skip' too, since it also affects dashes now.

We already have a test for that, but it needs
adjustment to test valid results:

diff --git a/tests/virsystemdtest.c b/tests/virsystemdtest.c
index f48a714cca..1a041d0212 100644
--- a/tests/virsystemdtest.c
+++ b/tests/virsystemdtest.c
@@ -736,7 +736,7 @@ mymain(void)
     TEST_MACHINE("demo", 1, "qemu-1-demo");
     TEST_MACHINE("demo-name", 2, "qemu-2-demo-name");
     TEST_MACHINE("demo!name", 3, "qemu-3-demoname");
-    TEST_MACHINE(".demo", 4, "qemu-4-.demo");
+    TEST_MACHINE(".demo", 4, "qemu-4-demo");
     TEST_MACHINE("bull\U0001f4a9", 5, "qemu-5-bull");
     TEST_MACHINE("demo..name", 6, "qemu-6-demo.name");
     TEST_MACHINE("12345678901234567890123456789012345678901234567890123456789", 7,

With that:
Reviewed-by: Ján Tomko <jtomko@xxxxxxxxxx>

Jano
> #undef HOSTNAME_CHARS
> diff --git a/tests/virsystemdtest.c b/tests/virsystemdtest.c
> index 1e36298189..f48a714cca 100644
> --- a/tests/virsystemdtest.c
> +++ b/tests/virsystemdtest.c
> @@ -746,7 +746,8 @@ mymain(void)
>     TEST_MACHINE("kstest-network-device-default-httpks_(c9eed63e-981e-48ec-acdc-56b3f8c5f678)", 100,
>                  "qemu-100-kstest-network-device-default-httpksc9eed63e-981e-48ec");
>     TEST_MACHINE("kstest-network-device-default-httpks_(c9eed63e-981e-48ec--cdc-56b3f8c5f678)", 10,
> -                 "qemu-10-kstest-network-device-default-httpksc9eed63e-981e-48ec");
> +                 "qemu-10-kstest-network-device-default-httpksc9eed63e-981e-48ec-c");
> +    TEST_MACHINE("demo.-.test.", 11, "qemu-11-demo.test");
>
> # define TESTS_PM_SUPPORT_HELPER(name, function) \
>     do { \
> --
> 2.24.1

Attachment: signature.asc
Description: PGP signature