jenkins-bot has submitted this change and it was merged.
Change subject: Write logs to the tool's homedir more securely
......................................................................
Write logs to the tool's homedir more securely
- Uses seteuid / setegid with a context manager
to do this
- Store gid of tool in Tool object as well
- Write log to service.log rather than services.log
to match service.manifest
- Do not crash on permission errors, but keep going
Change-Id: I9e4364bd23a8271870453ec87d8b2f775b8cd1c6
---
M tools/manifest/tool.py
A tools/manifest/utils.py
M tools/manifest/webservicemonitor.py
3 files changed, 34 insertions(+), 17 deletions(-)
Approvals:
Yuvipanda: Looks good to me, approved
jenkins-bot: Verified
diff --git a/tools/manifest/tool.py b/tools/manifest/tool.py
index c402743..d80edfe 100644
--- a/tools/manifest/tool.py
+++ b/tools/manifest/tool.py
@@ -1,7 +1,8 @@
import os
import time
import pwd
-import subprocess
+
+from .utils import effective_user
class Tool(object):
@@ -10,9 +11,10 @@
class InvalidToolException(Exception):
pass
- def __init__(self, name, username, uid, home):
+ def __init__(self, name, username, uid, gid, home):
self.name = name
self.uid = uid
+ self.gid = gid
self.username = username
self.home = home
@@ -29,21 +31,15 @@
raise Tool.InvalidToolException("No tool with name %s" % (name, ))
if user_info.pw_uid < 50000:
raise Tool.InvalidToolException("uid of tools should be < 50000,
%s has uid %s" % (name, user_info.pw_uid))
- return cls(name, user_info.pw_name, user_info.pw_uid, user_info.pw_dir)
+ return cls(name, user_info.pw_name, user_info.pw_uid,
user_info.pw_gid, user_info.pw_dir)
def log(self, message):
"""
Write to a log file in the tool's homedir
"""
- # use ugly sudo and whatnot here instead of 'proper' file stuff
because unsure how to
- # preserve permissions in atomic way when writing to a file that may
not exist already
log_line = "%s %s" % (time.asctime(), message)
- log_path = os.path.join(self.home, 'services.log')
- # Ensure that the file exists already and is owned appropriately by
the tool
- subprocess.check_output([
- '/usr/bin/sudo',
- '-i', '-u', self.username,
- '/usr/bin/touch', log_path
- ])
- with open(log_path, 'a') as f:
- f.write(log_line)
+ log_path = os.path.join(self.home, 'service.log')
+
+ with effective_user(self.uid, self.gid):
+ with open(log_path, 'a') as f:
+ f.write(log_line + '\n')
diff --git a/tools/manifest/utils.py b/tools/manifest/utils.py
new file mode 100644
index 0000000..e4530a5
--- /dev/null
+++ b/tools/manifest/utils.py
@@ -0,0 +1,16 @@
+import os
+from contextlib import contextmanager
+
+
+@contextmanager
+def effective_user(uid, gid):
+ """
+ A ContextManager that executes code in the with block with effective uid /
gid given
+ """
+ original_uid = os.geteuid()
+ original_gid = os.getegid()
+ os.setegid(gid)
+ os.seteuid(uid)
+ yield
+ os.setegid(original_gid)
+ os.setuid(original_uid)
diff --git a/tools/manifest/webservicemonitor.py
b/tools/manifest/webservicemonitor.py
index ad55556..8f05048 100644
--- a/tools/manifest/webservicemonitor.py
+++ b/tools/manifest/webservicemonitor.py
@@ -36,9 +36,14 @@
continue
job = qstat_xml.find('.//job_list[JB_name="%s-%s"]' %
(manifest.webservice_server, manifest.tool.name))
if job is None or 'r' not in job.findtext('.//state'):
- manifest.tool.log('No running webservice job found, starting
it')
- if self._start_webservice(manifest):
- restarts_count += 1
+ try:
+ manifest.tool.log('No running webservice job found,
attempting to start it')
+ if self._start_webservice(manifest):
+ restarts_count += 1
+ except Exception:
+ # More specific exceptions are already caught elsewhere,
so this should catch the rest
+ self.log.exception('Starting webservice for tool %s
failed', manifest.tool.name)
+ self.stats.incr('startfailed')
self.log.info('Service monitor run completed, %s webservices
restarted', restarts_count)
self.stats.incr('startsuccess', restarts_count)
--
To view, visit https://gerrit.wikimedia.org/r/202472
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I9e4364bd23a8271870453ec87d8b2f775b8cd1c6
Gerrit-PatchSet: 7
Gerrit-Project: operations/software/tools-manifest
Gerrit-Branch: master
Gerrit-Owner: Yuvipanda <[email protected]>
Gerrit-Reviewer: Ori.livneh <[email protected]>
Gerrit-Reviewer: Yuvipanda <[email protected]>
Gerrit-Reviewer: coren <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
