tests: use setfacl to give $SUDO_USER read permissions on artifacts

We have to invoke the tests as superuser, and not being able to read
the journal as the invoking user is annoying. I don't think there are
any security considerations here, since the invoking user can already
put arbitrary code in the Makefile and test scripts which get executed
with root privileges.

src/fstab-generator/fstab-generator.c

@@ -29,14 +29,14 @@
 #include "virt.h"
 #include "volatile-util.h"
 
-typedef enum MountpointFlags {
-        NOAUTO    = 1 << 0,
-        NOFAIL    = 1 << 1,
-        AUTOMOUNT = 1 << 2,
-        MAKEFS    = 1 << 3,
-        GROWFS    = 1 << 4,
-        RWONLY    = 1 << 5,
-} MountpointFlags;
+typedef enum MountPointFlags {
+        MOUNT_NOAUTO    = 1 << 0,
+        MOUNT_NOFAIL    = 1 << 1,
+        MOUNT_AUTOMOUNT = 1 << 2,
+        MOUNT_MAKEFS    = 1 << 3,
+        MOUNT_GROWFS    = 1 << 4,
+        MOUNT_RW_ONLY   = 1 << 5,
+} MountPointFlags;
 
 static const char *arg_dest = NULL;
 static const char *arg_dest_late = NULL;
@@ -91,7 +91,7 @@ static int write_what(FILE *f, const char *what) {
 static int add_swap(
                 const char *what,
                 struct mntent *me,
-                MountpointFlags flags) {
+                MountPointFlags flags) {
 
         _cleanup_free_ char *name = NULL;
         _cleanup_fclose_ FILE *f = NULL;
@@ -154,19 +154,19 @@ static int add_swap(
         if (r < 0)
                 return r;
 
-        if (flags & MAKEFS) {
+        if (flags & MOUNT_MAKEFS) {
                 r = generator_hook_up_mkswap(arg_dest, what);
                 if (r < 0)
                         return r;
         }
 
-        if (flags & GROWFS)
+        if (flags & MOUNT_GROWFS)
                 /* TODO: swap devices must be wiped and recreated */
                 log_warning("%s: growing swap devices is currently unsupported.", what);
 
-        if (!(flags & NOAUTO)) {
+        if (!(flags & MOUNT_NOAUTO)) {
                 r = generator_add_symlink(arg_dest, SPECIAL_SWAP_TARGET,
-                                          (flags & NOFAIL) ? "wants" : "requires", name);
+                                          (flags & MOUNT_NOFAIL) ? "wants" : "requires", name);
                 if (r < 0)
                         return r;
         }
@@ -345,7 +345,7 @@ static int add_mount(
                 const char *fstype,
                 const char *opts,
                 int passno,
-                MountpointFlags flags,
+                MountPointFlags flags,
                 const char *post,
                 const char *source) {
 
@@ -385,20 +385,20 @@ static int add_mount(
                 return r;
 
         if (path_equal(where, "/")) {
-                if (flags & NOAUTO)
-                        log_warning("Ignoring \"noauto\" for root device");
-                if (flags & NOFAIL)
-                        log_warning("Ignoring \"nofail\" for root device");
-                if (flags & AUTOMOUNT)
-                        log_warning("Ignoring automount option for root device");
+                if (flags & MOUNT_NOAUTO)
+                        log_warning("Ignoring \"noauto\" option for root device");
+                if (flags & MOUNT_NOFAIL)
+                        log_warning("Ignoring \"nofail\" option for root device");
+                if (flags & MOUNT_AUTOMOUNT)
+                        log_warning("Ignoring \"automount\" option for root device");
                 if (!strv_isempty(wanted_by))
-                        log_warning("Ignoring \"x-systemd.wanted-by=\" for root device");
+                        log_warning("Ignoring \"x-systemd.wanted-by=\" option for root device");
                 if (!strv_isempty(required_by))
-                        log_warning("Ignoring \"x-systemd.required-by=\" for root device");
+                        log_warning("Ignoring \"x-systemd.required-by=\" option for root device");
 
                 required_by = strv_free(required_by);
                 wanted_by = strv_free(wanted_by);
-                SET_FLAG(flags, NOAUTO | NOFAIL | AUTOMOUNT, false);
+                SET_FLAG(flags, MOUNT_NOAUTO | MOUNT_NOFAIL | MOUNT_AUTOMOUNT, false);
         }
 
         r = unit_name_from_path(where, ".mount", &name);
@@ -415,7 +415,7 @@ static int add_mount(
                 "SourcePath=%s\n",
                 source);
 
-        if (STRPTR_IN_SET(fstype, "nfs", "nfs4") && !(flags & AUTOMOUNT) &&
+        if (STRPTR_IN_SET(fstype, "nfs", "nfs4") && !(flags & MOUNT_AUTOMOUNT) &&
             fstab_test_yes_no_option(opts, "bg\0" "fg\0")) {
                 /* The default retry timeout that mount.nfs uses for 'bg' mounts
                  * is 10000 minutes, where as it uses 2 minutes for 'fg' mounts.
@@ -426,7 +426,7 @@ static int add_mount(
                  * By placing these options first, they can be overridden by
                  * settings in /etc/fstab. */
                 opts = strjoina("x-systemd.mount-timeout=infinity,retry=10000,nofail,", opts, ",fg");
-                SET_FLAG(flags, NOFAIL, true);
+                SET_FLAG(flags, MOUNT_NOFAIL, true);
         }
 
         r = write_extra_dependencies(f, opts);
@@ -435,7 +435,7 @@ static int add_mount(
 
         /* Order the mount unit we generate relative to the post unit, so that DefaultDependencies= on the
          * target unit won't affect us. */
-        if (post && !FLAGS_SET(flags, AUTOMOUNT) && !FLAGS_SET(flags, NOAUTO))
+        if (post && !FLAGS_SET(flags, MOUNT_AUTOMOUNT) && !FLAGS_SET(flags, MOUNT_NOAUTO))
                 fprintf(f, "Before=%s\n", post);
 
         if (passno != 0) {
@@ -490,29 +490,29 @@ static int add_mount(
         if (r < 0)
                 return r;
 
-        if (flags & RWONLY)
+        if (flags & MOUNT_RW_ONLY)
                 fprintf(f, "ReadWriteOnly=yes\n");
 
         r = fflush_and_check(f);
         if (r < 0)
                 return log_error_errno(r, "Failed to write unit file %s: %m", name);
 
-        if (flags & MAKEFS) {
+        if (flags & MOUNT_MAKEFS) {
                 r = generator_hook_up_mkfs(dest, what, where, fstype);
                 if (r < 0)
                         return r;
         }
 
-        if (flags & GROWFS) {
+        if (flags & MOUNT_GROWFS) {
                 r = generator_hook_up_growfs(dest, where, post);
                 if (r < 0)
                         return r;
         }
 
-        if (!FLAGS_SET(flags, AUTOMOUNT)) {
-                if (!FLAGS_SET(flags, NOAUTO) && strv_isempty(wanted_by) && strv_isempty(required_by)) {
+        if (!FLAGS_SET(flags, MOUNT_AUTOMOUNT)) {
+                if (!FLAGS_SET(flags, MOUNT_NOAUTO) && strv_isempty(wanted_by) && strv_isempty(required_by)) {
                         r = generator_add_symlink(dest, post,
-                                                  (flags & NOFAIL) ? "wants" : "requires", name);
+                                                  (flags & MOUNT_NOFAIL) ? "wants" : "requires", name);
                         if (r < 0)
                                 return r;
                 } else {
@@ -562,7 +562,7 @@ static int add_mount(
                         return log_error_errno(r, "Failed to write unit file %s: %m", automount_name);
 
                 r = generator_add_symlink(dest, post,
-                                          (flags & NOFAIL) ? "wants" : "requires", automount_name);
+                                          (flags & MOUNT_NOFAIL) ? "wants" : "requires", automount_name);
                 if (r < 0)
                         return r;
         }
@@ -589,7 +589,8 @@ static int parse_fstab(bool initrd) {
 
         while ((me = getmntent(f))) {
                 _cleanup_free_ char *where = NULL, *what = NULL, *canonical_where = NULL;
-                bool makefs, growfs, noauto, nofail, rwonly;
+                bool makefs, growfs, noauto, nofail;
+                MountPointFlags flags;
                 int k;
 
                 if (initrd && !mount_in_initrd(me))
@@ -629,7 +630,6 @@ static int parse_fstab(bool initrd) {
 
                 makefs = fstab_test_option(me->mnt_opts, "x-systemd.makefs\0");
                 growfs = fstab_test_option(me->mnt_opts, "x-systemd.growfs\0");
-                rwonly = fstab_test_option(me->mnt_opts, "x-systemd.rw-only\0");
                 noauto = fstab_test_yes_no_option(me->mnt_opts, "noauto\0" "auto\0");
                 nofail = fstab_test_yes_no_option(me->mnt_opts, "nofail\0" "fail\0");
 
@@ -638,16 +638,25 @@ static int parse_fstab(bool initrd) {
                           yes_no(makefs), yes_no(growfs),
                           yes_no(noauto), yes_no(nofail));
 
+                flags = makefs * MOUNT_MAKEFS |
+                        growfs * MOUNT_GROWFS |
+                        noauto * MOUNT_NOAUTO |
+                        nofail * MOUNT_NOFAIL;
+
                 if (streq(me->mnt_type, "swap"))
-                        k = add_swap(what, me,
-                                     makefs*MAKEFS | growfs*GROWFS | noauto*NOAUTO | nofail*NOFAIL);
+                        k = add_swap(what, me, flags);
                 else {
-                        bool automount;
+                        bool rw_only, automount;
                         const char *post;
 
+                        rw_only = fstab_test_option(me->mnt_opts, "x-systemd.rw-only\0");
                         automount = fstab_test_option(me->mnt_opts,
                                                       "comment=systemd.automount\0"
                                                       "x-systemd.automount\0");
+
+                        flags |= rw_only * MOUNT_RW_ONLY |
+                                 automount * MOUNT_AUTOMOUNT;
+
                         if (initrd)
                                 post = SPECIAL_INITRD_FS_TARGET;
                         else if (mount_is_network(me))
@@ -662,7 +671,7 @@ static int parse_fstab(bool initrd) {
                                       me->mnt_type,
                                       me->mnt_opts,
                                       me->mnt_passno,
-                                      makefs*MAKEFS | growfs*GROWFS | noauto*NOAUTO | nofail*NOFAIL | automount*AUTOMOUNT | rwonly*RWONLY,
+                                      flags,
                                       post,
                                       fstab);
                 }

src/shared/install.c

@@ -350,6 +350,11 @@ void unit_file_dump_changes(int r, const char *verb, const UnitFileChange *chang
                                 log_info("Unit %s is an alias to a unit that is not present, ignoring.",
                                          changes[i].path);
                         break;
+                case UNIT_FILE_DESTINATION_NOT_PRESENT:
+                        if (!quiet)
+                                log_warning("Unit %s is added as a dependency to a non-existent unit %s.",
+                                            changes[i].source, changes[i].path);
+                        break;
                 case -EEXIST:
                         if (changes[i].source)
                                 log_error_errno(changes[i].type_or_errno,
@@ -1832,6 +1837,7 @@ static int install_info_symlink_alias(
 }
 
 static int install_info_symlink_wants(
+                UnitFileScope scope,
                 UnitFileInstallInfo *i,
                 const LookupPaths *paths,
                 const char *config_path,
@@ -1902,6 +1908,9 @@ static int install_info_symlink_wants(
                 q = create_symlink(paths, i->path, path, true, changes, n_changes);
                 if (r == 0)
                         r = q;
+
+                if (unit_file_exists(scope, paths, dst) == 0)
+                        unit_file_changes_add(changes, n_changes, UNIT_FILE_DESTINATION_NOT_PRESENT, dst, i->path);
         }
 
         return r;
@@ -1937,6 +1946,7 @@ static int install_info_symlink_link(
 }
 
 static int install_info_apply(
+                UnitFileScope scope,
                 UnitFileInstallInfo *i,
                 const LookupPaths *paths,
                 const char *config_path,
@@ -1955,11 +1965,11 @@ static int install_info_apply(
 
         r = install_info_symlink_alias(i, paths, config_path, force, changes, n_changes);
 
-        q = install_info_symlink_wants(i, paths, config_path, i->wanted_by, ".wants/", changes, n_changes);
+        q = install_info_symlink_wants(scope, i, paths, config_path, i->wanted_by, ".wants/", changes, n_changes);
         if (r == 0)
                 r = q;
 
-        q = install_info_symlink_wants(i, paths, config_path, i->required_by, ".requires/", changes, n_changes);
+        q = install_info_symlink_wants(scope, i, paths, config_path, i->required_by, ".requires/", changes, n_changes);
         if (r == 0)
                 r = q;
 
@@ -2023,7 +2033,7 @@ static int install_context_apply(
                 if (i->type != UNIT_FILE_TYPE_REGULAR)
                         continue;
 
-                q = install_info_apply(i, paths, config_path, force, changes, n_changes);
+                q = install_info_apply(scope, i, paths, config_path, force, changes, n_changes);
                 if (r >= 0) {
                         if (q < 0)
                                 r = q;
@@ -3457,10 +3467,11 @@ static const char* const unit_file_state_table[_UNIT_FILE_STATE_MAX] = {
 DEFINE_STRING_TABLE_LOOKUP(unit_file_state, UnitFileState);
 
 static const char* const unit_file_change_type_table[_UNIT_FILE_CHANGE_TYPE_MAX] = {
-        [UNIT_FILE_SYMLINK]     = "symlink",
-        [UNIT_FILE_UNLINK]      = "unlink",
-        [UNIT_FILE_IS_MASKED]   = "masked",
-        [UNIT_FILE_IS_DANGLING] = "dangling",
+        [UNIT_FILE_SYMLINK]                 = "symlink",
+        [UNIT_FILE_UNLINK]                  = "unlink",
+        [UNIT_FILE_IS_MASKED]               = "masked",
+        [UNIT_FILE_IS_DANGLING]             = "dangling",
+        [UNIT_FILE_DESTINATION_NOT_PRESENT] = "destination not present",
 };
 
 DEFINE_STRING_TABLE_LOOKUP(unit_file_change_type, int);

src/shared/install.h

@@ -32,6 +32,7 @@ enum {
         UNIT_FILE_UNLINK,
         UNIT_FILE_IS_MASKED,
         UNIT_FILE_IS_DANGLING,
+        UNIT_FILE_DESTINATION_NOT_PRESENT,
         _UNIT_FILE_CHANGE_TYPE_MAX,
         _UNIT_FILE_CHANGE_TYPE_INVALID = -EINVAL,
 };

src/test/test-install-root.c

@@ -25,6 +25,7 @@ static void test_basic_mask_and_enable(const char *root) {
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "c.service", NULL) == -ENOENT);
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "d.service", NULL) == -ENOENT);
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "e.service", NULL) == -ENOENT);
+        assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "f.service", NULL) == -ENOENT);
 
         p = strjoina(root, "/usr/lib/systemd/system/a.service");
         assert_se(write_string_file(p,
@@ -168,6 +169,31 @@ static void test_basic_mask_and_enable(const char *root) {
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "e.service", &state) >= 0 && state == UNIT_FILE_MASKED);
 
         assert_se(unlink(p) == 0);
+
+        /* Test enabling with unknown dependency target */
+
+        p = strjoina(root, "/usr/lib/systemd/system/f.service");
+        assert_se(write_string_file(p,
+                                    "[Install]\n"
+                                    "WantedBy=x.target\n", WRITE_STRING_FILE_CREATE) >= 0);
+
+        assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "f.service", NULL) >= 0);
+        assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "f.service", &state) >= 0 && state == UNIT_FILE_DISABLED);
+
+        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("f.service"), &changes, &n_changes) == 1);
+        assert_se(n_changes == 2);
+        assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
+        assert_se(streq(changes[0].source, "/usr/lib/systemd/system/f.service"));
+        p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/x.target.wants/f.service");
+        assert_se(streq(changes[0].path, p));
+        assert_se(changes[1].type_or_errno == UNIT_FILE_DESTINATION_NOT_PRESENT);
+        p = strjoina(root, "/usr/lib/systemd/system/f.service");
+        assert_se(streq(changes[1].source, p));
+        assert_se(streq(changes[1].path, "x.target"));
+        unit_file_changes_free(changes, n_changes);
+        changes = NULL; n_changes = 0;
+
+        assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "f.service", &state) >= 0 && state == UNIT_FILE_ENABLED);
 }
 
 static void test_linked_units(const char *root) {
@@ -1244,6 +1270,12 @@ int main(int argc, char *argv[]) {
         p = strjoina(root, "/usr/lib/systemd/system-preset/");
         assert_se(mkdir_p(p, 0755) >= 0);
 
+        p = strjoina(root, "/usr/lib/systemd/system/multi-user.target");
+        assert_se(write_string_file(p, "# pretty much empty", WRITE_STRING_FILE_CREATE) >= 0);
+
+        p = strjoina(root, "/usr/lib/systemd/system/graphical.target");
+        assert_se(write_string_file(p, "# pretty much empty", WRITE_STRING_FILE_CREATE) >= 0);
+
         test_basic_mask_and_enable(root);
         test_linked_units(root);
         test_default(root);

test/TEST-55-OOMD/test.sh

@@ -2,10 +2,23 @@
 set -e
 
 TEST_DESCRIPTION="systemd-oomd Memory Pressure Test"
+IMAGE_NAME="oomd"
 
 # shellcheck source=test/test-functions
 . "${TEST_BASE_DIR:?}/test-functions"
 
+test_append_files() {
+    # Create a swap device
+    (
+        mkswap "${LOOPDEV:?}p2"
+        dracut_install swapon swapoff
+
+        cat >>"${initdir:?}/etc/fstab" <<EOF
+UUID=$(blkid -o value -s UUID "${LOOPDEV}p2")    none    swap    defaults 0 0
+EOF
+    )
+}
+
 check_result_nspawn() {
     local workspace="${1:?}"
     local ret=1

test/test-functions

@@ -2393,6 +2393,12 @@ do_test() {
     import_testdir
     import_initdir
 
+    if [ -n "${SUDO_USER}" ]; then
+        ddebug "Making ${TESTDIR:?} readable for ${SUDO_USER} (acquired from sudo)"
+        setfacl -m "user:${SUDO_USER:?}:r-X" "${TESTDIR:?}"
+        setfacl -d -m "user:${SUDO_USER:?}:r-X" "${TESTDIR:?}"
+    fi
+
     testname="$(basename "$PWD")"
 
     while (($# > 0)); do

test/units/testsuite-55-slowgrowth.sh

@@ -12,9 +12,7 @@ PID="$$"
 function bloat {
         local set_size mem_usage target_mem_size
 
-        # Following `| cat` weirdness is intentional to generate some reclaim
-        # activity in case there's no swap available.
-        set_size=$(cut -d " " -f2 "/proc/$PID/statm" | cat)
+        set_size=$(cut -d " " -f2 "/proc/$PID/statm")
         mem_usage=$((set_size * PAGE_SIZE))
         target_mem_size=$((mem_usage + $1))
 
@@ -23,7 +21,7 @@ function bloat {
                 echo "target $target_mem_size"
                 echo "mem usage $mem_usage"
                 BLOAT_HOLDER+=("$(printf "=%0.s" {1..1000000})")
-                set_size=$(cut -d " " -f2 "/proc/$PID/statm" | cat)
+                set_size=$(cut -d " " -f2 "/proc/$PID/statm")
                 mem_usage=$((set_size * PAGE_SIZE))
         done
 }