add group to make 3dt read journal

3dt needs to read the journal. In order to do that it needs to be in
systemd-journal group. The issue is that coreos uses several backends for
users and groups. One is a regular /etc/passwd, /etc/groups and the other is
located under /usr/share/baselayout. If we try to append dcos_3dt user with
flag -G this will not work. (Tested on CoreOS beta (1068.3.0))
This seems to be a long running issue with CoreOS
coreos/bugs#312
However if we try to change a user's primary group with -g flag all works
as expected.

packages/3dt/buildinfo.json

@@ -8,5 +8,6 @@
     "ref_origin": "master"
   },
   "username": "dcos_3dt",
+  "group": "systemd-journal",
   "state_directory": true
 }

packages/dcos-integration-test/extra/integration_test.py

@@ -1,4 +1,5 @@
 import collections
+import gzip
 import json
 import logging
 import os
@@ -1725,6 +1726,16 @@ def wait_for_job():
     assert bundles[0] == response['extra']['bundle_name']
 
 
+def verify_unit_response(zip_ext_file):
+    assert isinstance(zip_ext_file, zipfile.ZipExtFile)
+    unit_output = gzip.decompress(zip_ext_file.read())
+
+    # TODO: This seems like a really fragile string to be searching for. This might need to be changed for
+    # different localizations.
+    assert 'Hint: You are currently not seeing messages from other users and the system' not in str(unit_output), (
+        '3dt does not have permission to run `journalctl`')
+
+
 def test_3dt_bundle_download_and_extract(cluster):
     """
     test bundle download and validate zip file
@@ -1769,6 +1780,10 @@ def test_3dt_bundle_download_and_extract(cluster):
                 assert 'ip' in health_report
                 assert health_report['ip'] == master_ip
 
+                # make sure systemd unit output is correct and does not contain error message
+                gzipped_unit_output = z.open(master_folder + 'dcos-mesos-master.service.gz')
+                verify_unit_response(gzipped_unit_output)
+
                 for expected_master_file in expected_master_files:
                     expected_file = master_folder + expected_master_file
                     assert expected_file in archived_items, 'expecting {} in {}'.format(expected_file, archived_items)
@@ -1782,6 +1797,10 @@ def test_3dt_bundle_download_and_extract(cluster):
                 assert 'ip' in health_report
                 assert health_report['ip'] == slave_ip
 
+                # make sure systemd unit output is correct and does not contain error message
+                gzipped_unit_output = z.open(agent_folder + 'dcos-mesos-slave.service.gz')
+                verify_unit_response(gzipped_unit_output)
+
                 for expected_agent_file in expected_agent_files:
                     expected_file = agent_folder + expected_agent_file
                     assert expected_file in archived_items, 'expecting {} in {}'.format(expected_file, archived_items)
@@ -1795,6 +1814,10 @@ def test_3dt_bundle_download_and_extract(cluster):
                 assert 'ip' in health_report
                 assert health_report['ip'] == public_slave_ip
 
+                # make sure systemd unit output is correct and does not contain error message
+                gzipped_unit_output = z.open(agent_public_folder + 'dcos-mesos-slave-public.service.gz')
+                verify_unit_response(gzipped_unit_output)
+
                 for expected_public_agent_file in expected_public_agent_files:
                     expected_file = agent_public_folder + expected_public_agent_file
                     assert expected_file in archived_items, ('expecting {} in {}'.format(expected_file, archived_items))

pkgpanda/__init__.py

@@ -12,6 +12,7 @@
 envrionment variables from the package.
 
 """
+import grp
 import json
 import os
 import os.path
@@ -41,6 +42,7 @@
 name_regex = "^[a-zA-Z0-9@_+][a-zA-Z0-9@._+\-]*$"
 version_regex = "^[a-zA-Z0-9@_+:.]+$"
 username_regex = "^dcos_[a-z0-9_]+$"
+linux_group_regex = "^[a-z_][a-z0-9_-]*$"  # https://github.com/shadow-maint/shadow/blob/master/libmisc/chkname.c#L52
 
 
 # Manage starting/stopping all systemd services inside a folder.
@@ -172,6 +174,10 @@ def state_directory(self):
     def username(self):
         return self.__pkginfo.get('username', None)
 
+    @property
+    def group(self):
+        return self.__pkginfo.get('group', None)
+
     def __repr__(self):
         return str(self.__id)
 
@@ -211,6 +217,10 @@ def validate_compatible(packages, roles):
             raise ValidationError(
                 "Repeated name {0} in set of packages {1}".format(
                     package.name, ' '.join(map(lambda x: str(x.id), packages))))
+
+        if package.username is None and package.group is not None:
+            raise ValidationError("`group` cannot be used without `username`")
+
         names.add(package.name)
         ids.add(str(package.id))
         tuples.add((package.name, package.variant))
@@ -442,15 +452,54 @@ def validate_username(username):
         if not re.match(username_regex, username):
             raise ValidationError("Username must begin with `dcos_` and only have a-z and underscore after that")
 
-    def add_user(self, username):
+    @staticmethod
+    def validate_group(group):
+        # Empty group is allowed.
+        if not group:
+            return
+
+        UserManagement.validate_group_name(group)
+
+        try:
+            grp.getgrnam(group)
+        except KeyError:
+            raise ValidationError("Group {} does not exist on the system".format(group))
+
+    @staticmethod
+    def validate_group_name(group_name):
+        if not group_name:
+            return
+
+        if not re.match(linux_group_regex, group_name):
+            raise ValidationError("Group {} has invalid name, must match the following regex: {}".format(
+                group_name, linux_group_regex))
+
+    @staticmethod
+    def validate_user_group(username, group_name):
+        user = pwd.getpwnam(username)
+        if not group_name:
+            return
+
+        group = grp.getgrnam(group_name)
+        if user.pw_gid != group.gr_gid:
+
+            # check if the user is the right group, but the group is not primary.
+            if username in group.gr_mem:
+                return
+
+            raise ValidationError(
+                "User {} exists with current UID {}, however he should be assigned to group {} with {} UID, please "
+                "check `buildinfo.json`".format(username, user.pw_gid, group_name, group.gr_gid))
+
+    def add_user(self, username, group):
         UserManagement.validate_username(username)
 
         if not self._manage_users:
             return
 
-        # Check if hte user already exists and exit.
+        # Check if the user already exists and exit.
         try:
-            pwd.getpwnam(username)
+            UserManagement.validate_user_group(username, group)
             self._users.add(username)
             return
         except KeyError as ex:
@@ -463,14 +512,24 @@ def add_user(self, username):
                                   "automatic user addition is disabled".format(username))
 
         # Add the user:
+        add_user_cmd = [
+            'useradd',
+            '--system',
+            '--home-dir', '/opt/mesosphere',
+            '--shell', '/sbin/nologin',
+            '-c', 'DCOS System User',
+        ]
+
+        if group is not None:
+            UserManagement.validate_group(group)
+            add_user_cmd += [
+                '-g', group
+            ]
+
+        add_user_cmd += [username]
+
         try:
-            check_output([
-                'useradd',
-                '--system',
-                '--home-dir', '/opt/mesosphere',
-                '--shell', '/sbin/nologin',
-                '-c', 'DCOS System User',
-                username])
+            check_output(add_user_cmd)
             self._users.add(username)
         except CalledProcessError as ex:
             raise ValidationError("User {} doesn't exist and couldn't be created because of: {}"
@@ -681,7 +740,7 @@ def symlink_all(src, dest):
             # to something incompatible. We survive the first upgrade because everything goes from
             # root to specific users, and root can access all user files.
             if package.username is not None:
-                sysusers.add_user(package.username)
+                sysusers.add_user(package.username, package.group)
 
             # Ensure the state directory in `/var/lib/dcos` exists
             # TODO(cmaloney): On upgrade take a snapshot?

pkgpanda/build/__init__.py

@@ -858,6 +858,18 @@ def cache_abs(filename):
             raise BuildError("username in buildinfo.json didn't meet the validation rules. {}".format(ex))
         pkginfo['username'] = username
 
+    group = None
+    if builder.has('group'):
+        group = builder.take('group')
+        if not isinstance(group, str):
+            raise BuildError("group in buildinfo.json must be either not set (use default group for this user)"
+                             ", or group must be a string")
+        try:
+            pkgpanda.UserManagement.validate_group_name(group)
+        except ValidationError as ex:
+            raise BuildError("group in buildinfo.json didn't meet the validation rules. {}".format(ex))
+        pkginfo['group'] = group
+
     # Packages need directories inside the fake install root (otherwise docker
     # will try making the directories on a readonly filesystem), so build the
     # install root now, and make the package directories in it as we go.

pkgpanda/tests/test_util.py

@@ -42,3 +42,11 @@ def bad(name):
     bad('dcos_foo:bar')
     bad('3dcos_foobar')
     bad('dcos3_foobar')
+
+
+def test_validate_group():
+    # assuming linux distributions have `root` group.
+    UserManagement.validate_group('root')
+
+    with pytest.raises(ValidationError):
+        UserManagement.validate_group('group-should-not-exist')