From 93eb56dfc8ec54b12e968b2b60c3b915ebb08955 Mon Sep 17 00:00:00 2001 From: David Shrewsbury Date: Wed, 10 Jan 2018 11:50:03 -0500 Subject: [PATCH] Remove need to start executor as root Now that we have a finger gateway, we no longer need to start the executor as root so that the finger streamer on the executor can bind to port 79 (default port for the finger streamer is changed from 79 to 7900). Remove that requirement. Change-Id: I6df685044c4ce81fd263043adba832609da100af --- doc/source/admin/components.rst | 9 +-------- tests/unit/test_streaming.py | 16 ++++++++-------- zuul/cmd/executor.py | 23 +---------------------- zuul/executor/server.py | 2 +- zuul/lib/log_streamer.py | 3 +-- zuul/lib/streamer_utils.py | 2 +- 6 files changed, 13 insertions(+), 42 deletions(-) diff --git a/doc/source/admin/components.rst b/doc/source/admin/components.rst index 3bec28afd6..18bbfa3f44 100644 --- a/doc/source/admin/components.rst +++ b/doc/source/admin/components.rst @@ -408,7 +408,7 @@ The following sections of ``zuul.conf`` are used by the executor: Path to command socket file for the executor process. .. attr:: finger_port - :default: 79 + :default: 7900 Port to use for finger log streamer. @@ -451,13 +451,6 @@ The following sections of ``zuul.conf`` are used by the executor: SSH private key file to be used when logging into worker nodes. - .. attr:: user - :default: zuul - - User ID for the zuul-executor process. In normal operation as a - daemon, the executor should be started as the ``root`` user, but - it will drop privileges to this user during startup. - .. _admin_sitewide_variables: .. attr:: variables diff --git a/tests/unit/test_streaming.py b/tests/unit/test_streaming.py index 59dd8b0169..b999106c87 100644 --- a/tests/unit/test_streaming.py +++ b/tests/unit/test_streaming.py @@ -41,13 +41,13 @@ class TestLogStreamer(tests.base.BaseTestCase): def startStreamer(self, port, root=None): if not root: root = tempfile.gettempdir() - return zuul.lib.log_streamer.LogStreamer(None, self.host, port, root) + return zuul.lib.log_streamer.LogStreamer(self.host, port, root) def test_start_stop(self): - port = 7900 - streamer = self.startStreamer(port) + streamer = self.startStreamer(0) self.addCleanup(streamer.stop) + port = streamer.server.socket.getsockname()[1] s = socket.create_connection((self.host, port)) s.close() @@ -77,8 +77,9 @@ class TestStreaming(tests.base.AnsibleZuulTestCase): def startStreamer(self, port, build_uuid, root=None): if not root: root = tempfile.gettempdir() - self.streamer = zuul.lib.log_streamer.LogStreamer(None, self.host, + self.streamer = zuul.lib.log_streamer.LogStreamer(self.host, port, root) + port = self.streamer.server.socket.getsockname()[1] s = socket.create_connection((self.host, port)) self.addCleanup(s.close) @@ -129,10 +130,9 @@ class TestStreaming(tests.base.AnsibleZuulTestCase): # Create a thread to stream the log. We need this to be happening # before we create the flag file to tell the job to complete. - port = 7901 streamer_thread = threading.Thread( target=self.startStreamer, - args=(port, build.uuid, self.executor_server.jobdir_root,) + args=(0, build.uuid, self.executor_server.jobdir_root,) ) streamer_thread.start() self.addCleanup(self.stopStreamer) @@ -209,7 +209,7 @@ class TestStreaming(tests.base.AnsibleZuulTestCase): def test_websocket_streaming(self): # Start the finger streamer daemon streamer = zuul.lib.log_streamer.LogStreamer( - None, self.host, 0, self.executor_server.jobdir_root) + self.host, 0, self.executor_server.jobdir_root) self.addCleanup(streamer.stop) # Need to set the streaming port before submitting the job @@ -294,7 +294,7 @@ class TestStreaming(tests.base.AnsibleZuulTestCase): def test_finger_gateway(self): # Start the finger streamer daemon streamer = zuul.lib.log_streamer.LogStreamer( - None, self.host, 0, self.executor_server.jobdir_root) + self.host, 0, self.executor_server.jobdir_root) self.addCleanup(streamer.stop) finger_port = streamer.server.socket.getsockname()[1] diff --git a/zuul/cmd/executor.py b/zuul/cmd/executor.py index ade9715c2e..ad7aaa8373 100755 --- a/zuul/cmd/executor.py +++ b/zuul/cmd/executor.py @@ -14,10 +14,8 @@ # License for the specific language governing permissions and limitations # under the License. -import grp import logging import os -import pwd import sys import signal import tempfile @@ -64,7 +62,7 @@ class Executor(zuul.cmd.ZuulDaemonApp): self.log.info("Starting log streamer") streamer = zuul.lib.log_streamer.LogStreamer( - self.user, '::', self.finger_port, self.job_dir) + '::', self.finger_port, self.job_dir) # Keep running until the parent dies: pipe_read = os.fdopen(pipe_read) @@ -76,22 +74,6 @@ class Executor(zuul.cmd.ZuulDaemonApp): os.close(pipe_read) self.log_streamer_pid = child_pid - def change_privs(self): - ''' - Drop our privileges to the zuul user. - ''' - if os.getuid() != 0: - return - pw = pwd.getpwnam(self.user) - # get a list of supplementary groups for the target user, and make sure - # we set them when dropping privileges. - groups = [g.gr_gid for g in grp.getgrall() if self.user in g.gr_mem] - os.setgroups(groups) - os.setgid(pw.pw_gid) - os.setuid(pw.pw_uid) - os.chdir(pw.pw_dir) - os.umask(0o022) - def run(self): if self.args.command in zuul.executor.server.COMMANDS: self.send_command(self.args.command) @@ -99,8 +81,6 @@ class Executor(zuul.cmd.ZuulDaemonApp): self.configure_connections(source_only=True) - self.user = get_default(self.config, 'executor', 'user', 'zuul') - if self.config.has_option('executor', 'job_dir'): self.job_dir = os.path.expanduser( self.config.get('executor', 'job_dir')) @@ -120,7 +100,6 @@ class Executor(zuul.cmd.ZuulDaemonApp): ) self.start_log_streamer() - self.change_privs() ExecutorServer = zuul.executor.server.ExecutorServer self.executor = ExecutorServer(self.config, self.connections, diff --git a/zuul/executor/server.py b/zuul/executor/server.py index 904d6e2667..a8ab8c45e6 100644 --- a/zuul/executor/server.py +++ b/zuul/executor/server.py @@ -44,7 +44,7 @@ from zuul.lib import commandsocket BUFFER_LINES_FOR_SYNTAX = 200 COMMANDS = ['stop', 'pause', 'unpause', 'graceful', 'verbose', 'unverbose', 'keep', 'nokeep'] -DEFAULT_FINGER_PORT = 79 +DEFAULT_FINGER_PORT = 7900 BLACKLISTED_ANSIBLE_CONNECTION_TYPES = ['network_cli'] diff --git a/zuul/lib/log_streamer.py b/zuul/lib/log_streamer.py index c778812a6f..f96f442794 100644 --- a/zuul/lib/log_streamer.py +++ b/zuul/lib/log_streamer.py @@ -157,12 +157,11 @@ class LogStreamer(object): Class implementing log streaming over the finger daemon port. ''' - def __init__(self, user, host, port, jobdir_root): + def __init__(self, host, port, jobdir_root): self.log = logging.getLogger('zuul.log_streamer') self.log.debug("LogStreamer starting on port %s", port) self.server = LogStreamerServer((host, port), RequestHandler, - user=user, jobdir_root=jobdir_root) # We start the actual serving within a thread so we can return to diff --git a/zuul/lib/streamer_utils.py b/zuul/lib/streamer_utils.py index 43bc28626f..3d2d561b9b 100644 --- a/zuul/lib/streamer_utils.py +++ b/zuul/lib/streamer_utils.py @@ -74,7 +74,7 @@ class CustomThreadingTCPServer(socketserver.ThreadingTCPServer): address_family = socket.AF_INET6 def __init__(self, *args, **kwargs): - self.user = kwargs.pop('user') + self.user = kwargs.pop('user', None) self.pid_file = kwargs.pop('pid_file', None) socketserver.ThreadingTCPServer.__init__(self, *args, **kwargs)