From aad53ffb741c8e65a91c3583f26faafef3f3f0c3 Mon Sep 17 00:00:00 2001 From: Maksym Naboka Date: Wed, 6 Jul 2016 17:43:11 -0700 Subject: [PATCH] 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 https://github.com/coreos/bugs/issues/312 However if we try to change a user's primary group with -g flag all works as expected. --- packages/3dt/buildinfo.json | 1 + .../extra/integration_test.py | 23 ++++++ pkgpanda/__init__.py | 81 ++++++++++++++++--- pkgpanda/build/__init__.py | 12 +++ pkgpanda/tests/test_util.py | 8 ++ 5 files changed, 114 insertions(+), 11 deletions(-) diff --git a/packages/3dt/buildinfo.json b/packages/3dt/buildinfo.json index 4c99cde929..c72afac960 100644 --- a/packages/3dt/buildinfo.json +++ b/packages/3dt/buildinfo.json @@ -8,5 +8,6 @@ "ref_origin": "master" }, "username": "dcos_3dt", + "group": "systemd-journal", "state_directory": true } diff --git a/packages/dcos-integration-test/extra/integration_test.py b/packages/dcos-integration-test/extra/integration_test.py index ffb294c4d8..3eb85bf546 100644 --- a/packages/dcos-integration-test/extra/integration_test.py +++ b/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)) diff --git a/pkgpanda/__init__.py b/pkgpanda/__init__.py index cb33852f74..dd2db38406 100644 --- a/pkgpanda/__init__.py +++ b/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? diff --git a/pkgpanda/build/__init__.py b/pkgpanda/build/__init__.py index 6d4f25892e..d36660ac97 100644 --- a/pkgpanda/build/__init__.py +++ b/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. diff --git a/pkgpanda/tests/test_util.py b/pkgpanda/tests/test_util.py index c111369184..fd6d66d945 100644 --- a/pkgpanda/tests/test_util.py +++ b/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')