Repository :
http://git.fedorahosted.org/cgit/copr.git
On branch : master
---------------------------------------------------------------
commit efad5eb925d405caed1da1312156313ad1a0690e
Author: Valentin Gologuzov <vgologuz(a)redhat.com>
Date: Thu Sep 25 15:51:52 2014 +0200
[keygen] python3 bugfix: decode Popen().communicate output
[keygen] added logging, with default location /var/lib/copr-keygen/main.log
---------------------------------------------------------------
keygen/configs/local_settings.py.example | 3 +
keygen/copr-keygen.spec | 5 ++
keygen/run/dev_run.py | 8 ++++
keygen/src/copr_keygen/__init__.py | 27 ++++++++++++
keygen/src/copr_keygen/default_settings.py | 6 ++-
keygen/src/copr_keygen/logic.py | 60 ++++++++++++++++++++++-----
keygen/tests/test_logic.py | 43 ++++++++++++++++----
7 files changed, 131 insertions(+), 21 deletions(-)
diff --git a/keygen/configs/local_settings.py.example
b/keygen/configs/local_settings.py.example
index ce83b96..bf26d68 100644
--- a/keygen/configs/local_settings.py.example
+++ b/keygen/configs/local_settings.py.example
@@ -7,3 +7,6 @@ GNUPG_HOMEDIR = "/var/lib/copr-keygen/gnupg"
GPG_KEY_LENGTH = 2048
GPG_EXPIRE = "5y"
+LOG_DIR = "/var/log/copr-keygen"
+import logging
+LOG_LEVEL = logging.INFO
diff --git a/keygen/copr-keygen.spec b/keygen/copr-keygen.spec
index 638ac3d..01fef65 100644
--- a/keygen/copr-keygen.spec
+++ b/keygen/copr-keygen.spec
@@ -117,6 +117,7 @@ install -d %{buildroot}%{_datadir}/copr-keygen
install -d %{buildroot}%{_bindir}
install -d -m 500 %{buildroot}%{_sharedstatedir}/copr-keygen/phrases
install -d -m 500 %{buildroot}%{_sharedstatedir}/copr-keygen/gnupg
+install -d %{buildroot}%{_localstatedir}/log/copr-keygen
%{__install} -p -m 0644 run/gpg_copr.sh %{buildroot}/%{_bindir}/gpg_copr.sh
@@ -158,9 +159,13 @@ service httpd condrestart
%{_bindir}/gpg_copr.sh
+
%defattr(600, copr-signer, copr-signer, 700)
%{_sharedstatedir}/copr-keygen
%config(noreplace) %{_sysconfdir}/copr-keygen
+%{_localstatedir}/log/copr-keygen/
+%ghost %{_localstatedir}/log/copr-keygen/main.log
+
%defattr(600, root, root)
%config(noreplace) %{_sysconfdir}/sudoers.d/copr_signer
diff --git a/keygen/run/dev_run.py b/keygen/run/dev_run.py
index d6b1761..3511bc5 100755
--- a/keygen/run/dev_run.py
+++ b/keygen/run/dev_run.py
@@ -4,5 +4,13 @@ import sys
sys.path.append("../src/")
from copr_keygen import app
+import logging
+
+logging.basicConfig(
+ level=logging.INFO,
+ format= '[%(asctime)s] {%(pathname)s:%(lineno)d} %(levelname)s -
%(message)s',
+ datefmt='%H:%M:%S'
+)
+
if __name__ == '__main__':
app.run()
diff --git a/keygen/src/copr_keygen/__init__.py b/keygen/src/copr_keygen/__init__.py
index 232f12d..41f19b3 100644
--- a/keygen/src/copr_keygen/__init__.py
+++ b/keygen/src/copr_keygen/__init__.py
@@ -5,8 +5,12 @@ from __future__ import division
from __future__ import absolute_import
import json
+import logging
+from logging import getLogger
+from logging.handlers import RotatingFileHandler
from flask import Flask, request, Response
+import os
from copr_keygen.exceptions import BadRequestException, \
KeygenServiceBaseException
@@ -15,8 +19,30 @@ app.config.from_object("copr_keygen.default_settings")
app.config.from_envvar("COPR_KEYGEN_CONFIG", silent=True)
+# setup logger
+if not app.config["DEBUG"]:
+ filename = os.path.join(app.config["LOG_DIR"], "main.log")
+ if os.path.exists(app.config["LOG_DIR"]):
+ handler = RotatingFileHandler(filename,
+ maxBytes=50*1024*1024,
+ backupCount=5)
+ handler.setLevel(app.config["LOG_LEVEL"])
+ handler.setFormatter(logging.Formatter(
+ '%(asctime)s %(levelname)s'
+ '[%(module)s:%(pathname)s:%(lineno)d]'
+ ': %(message)s '
+ ))
+ logger = getLogger(__name__)
+ logger.addHandler(handler)
+ logger.setLevel(app.config["LOG_LEVEL"])
+
+# end setup logger
+
+
from .logic import create_new_key, user_exists
+log = logging.getLogger(__name__)
+
@app.route('/ping')
def ping():
@@ -112,3 +138,4 @@ def handle_invalid_usage(error):
# awk -F: '$1 == "fpr" {print $10;}'"
# #TODO: complete implementation
+
diff --git a/keygen/src/copr_keygen/default_settings.py
b/keygen/src/copr_keygen/default_settings.py
index 92f1f4a..db44249 100644
--- a/keygen/src/copr_keygen/default_settings.py
+++ b/keygen/src/copr_keygen/default_settings.py
@@ -1,4 +1,4 @@
-DEBUG = True
+DEBUG = False
PHRASES_DIR = "/var/lib/copr-keygen/phrases/"
GPG_BINARY = "/bin/gpg2"
@@ -6,3 +6,7 @@ GNUPG_HOMEDIR = "/var/lib/copr-keygen/gnupg"
GPG_KEY_LENGTH = 2048
GPG_EXPIRE = "5y"
+
+LOG_DIR = "/var/log/copr-keygen"
+import logging
+LOG_LEVEL = logging.INFO
diff --git a/keygen/src/copr_keygen/logic.py b/keygen/src/copr_keygen/logic.py
index 779be8e..a19210c 100644
--- a/keygen/src/copr_keygen/logic.py
+++ b/keygen/src/copr_keygen/logic.py
@@ -1,19 +1,26 @@
+import traceback
import os
+import logging
from subprocess import PIPE, Popen
import tempfile
+import sys
-from .exceptions import GpgErrorException
+from .exceptions import GpgErrorException, KeygenServiceBaseException
+
+log = logging.getLogger(__name__)
def ensure_passphrase_exist(app, name_email):
""" Need this to tell signd server that `name_email` available in
keyring
Key not protected by passphrase, so we write *something* to passphrase file.
"""
+
def create():
with open(location, "w") as handle:
handle.write("1")
handle.write(os.linesep)
+ log.debug("created passphrase file for {}".format(name_email))
location = os.path.join(app.config["PHRASES_DIR"], name_email)
try:
@@ -40,18 +47,23 @@ def user_exists(app, mail):
handle = Popen(cmd, stdout=PIPE, stderr=PIPE)
stdout, stderr = handle.communicate()
except Exception as e:
+ log.exception(e)
raise GpgErrorException(msg="unhandled exception during gpg call",
cmd=" ".join(cmd), err=e)
if handle.returncode == 0:
# TODO: validate that we really got exactly one line in stdout
+ log.debug("user {} has keys in keyring".format(mail))
ensure_passphrase_exist(app, mail)
return True
- elif "error reading key" in stderr:
+ elif "error reading key" in stderr.decode():
+ log.debug("user {} not found in keyring".format(mail))
return False
else:
- raise GpgErrorException(msg="unhandled error", cmd=cmd, stdout=stdout,
- stderr=stderr)
+ err = GpgErrorException(msg="unhandled error", cmd=cmd,
+ stdout=stdout.decode(), stderr=stderr.decode())
+ log.error(err)
+ raise err
template = """
@@ -81,12 +93,20 @@ def create_new_key(
:return: (stdout, stderr) from `gpg` invocation
"""
- #TODO with file lock based on usermail !!!
+ # TODO with file lock based on usermail !!!
if user_exists(app, name_email):
return
- with tempfile.NamedTemporaryFile() as out:
+ try:
+ # ! Don't use context manager with delete=True
+ # TemporaryFile deletes file on .close() not on __exit__()
+ out = tempfile.NamedTemporaryFile(delete=False)
+ except Exception as e:
+ raise KeygenServiceBaseException(
+ msg="Failed to create tmp file for gen_key",
+ err=e)
+ try:
out.write(template.format(
key_type="RSA",
key_length=key_length or 2048,
@@ -94,25 +114,43 @@ def create_new_key(
comment=name_comment,
name_email=name_email,
expire=expire or 0).encode('utf-8'))
+ out.close()
+ except Exception as e:
+ raise GpgErrorException(msg="Failed to write tmp file for gen_key",
+ err=e)
cmd = [
- app.config["GPG_BINARY"], "-v", "--batch",
+ app.config["GPG_BINARY"], "--batch",
"--homedir", app.config["GNUPG_HOMEDIR"],
"--gen-key", out.name
]
+ log.debug("CMD: {}".format(' '.join(map(str, cmd))))
try:
handle = Popen(cmd, stdout=PIPE, stderr=PIPE)
stdout, stderr = handle.communicate()
except Exception as e:
- raise GpgErrorException(msg="unhandled exception during gpg call",
- cmd=" ".join(cmd), err=e)
-
+ log.exception(e)
+ err = GpgErrorException(msg="unhandled exception during gpg call",
+ cmd=" ".join(map(str, cmd)), err=e)
+ log.error(err)
+ raise err
+
+ log.debug("returncode: {}".format(handle.returncode))
+ log.debug("stdout: {}".format(stdout))
+ log.debug("stderr: {}".format(stderr))
if handle.returncode == 0:
# TODO: validate that we really got armored gpg key
if not user_exists(app, name_email):
raise GpgErrorException(
msg="Key was created, but not found in keyring"
"this shouldn't be possible")
+ log.info("Created key-pair for: {} ".format(name_email))
else:
- raise GpgErrorException(msg=stderr)
+ raise GpgErrorException(msg=stderr.decode())
+
+ try:
+ os.remove(out.name)
+ except Exception as e:
+ log.error(e)
+
diff --git a/keygen/tests/test_logic.py b/keygen/tests/test_logic.py
index 1c80113..6cabb4f 100644
--- a/keygen/tests/test_logic.py
+++ b/keygen/tests/test_logic.py
@@ -7,13 +7,15 @@ import six
if six.PY3:
from unittest import mock
+ from unittest.mock import patch
else:
import mock
+ from mock import patch
import pytest
from copr_keygen import app
-from copr_keygen.exceptions import GpgErrorException
+from copr_keygen.exceptions import GpgErrorException, KeygenServiceBaseException
from copr_keygen.logic import ensure_passphrase_exist
import copr_keygen.logic as logic
@@ -92,7 +94,7 @@ class MockPopenHandle(object):
self.stderr = stderr or "mock stderr"
def communicate(self):
- return self.stdout, self.stderr
+ return self.stdout.encode(), self.stderr.encode()
@mock.patch("copr_keygen.logic.ensure_passphrase_exist")
@@ -129,13 +131,19 @@ class TestGenKey(TestCase):
it exists
"""
- user_exists_returns = [False, True]
- user_exists.side_effect = \
- lambda *args, **kwargs: user_exists_returns.pop(0)
- popen.return_value = MockPopenHandle(0)
+ with mock.patch("tempfile.NamedTemporaryFile") as tmpfile:
+ user_exists_returns = [False, True]
+ user_exists.side_effect = \
+ lambda *args, **kwargs: user_exists_returns.pop(0)
+
+ def check_gpg_genkey_file_exists(*args, **kwargs):
+ assert tmpfile.called
+ return MockPopenHandle(0)
+
+ popen.side_effect = check_gpg_genkey_file_exists
- res = logic.create_new_key(app, TEST_NAME, TEST_EMAIL, TEST_KEYLENGTH)
- assert res is None
+ res = logic.create_new_key(app, TEST_NAME, TEST_EMAIL, TEST_KEYLENGTH)
+ assert res is None
def test_strange_situation_create(self, popen, user_exists):
"""
@@ -168,4 +176,21 @@ class TestGenKey(TestCase):
popen.return_value = MockPopenHandle(1, stderr=err_msg)
with pytest.raises(GpgErrorException) as e:
logic.create_new_key(app, TEST_NAME, TEST_EMAIL, TEST_KEYLENGTH)
- assert e.msg == err_msg
+ assert e.message == err_msg
+
+ def test_tmpfiles_errors(self, popen, user_exists):
+ user_exists.return_value = False
+
+ with mock.patch("tempfile.NamedTemporaryFile") as tmpfile:
+ tmpfile.side_effect = OSError()
+ with pytest.raises(KeygenServiceBaseException) as e:
+ logic.create_new_key(app, TEST_NAME, TEST_EMAIL, TEST_KEYLENGTH)
+
+ assert not popen.called
+
+ with mock.patch("tempfile.NamedTemporaryFile") as tmpfile:
+ tmpfile.return_value.write.side_effect = OSError()
+ with pytest.raises(KeygenServiceBaseException) as e:
+ logic.create_new_key(app, TEST_NAME, TEST_EMAIL, TEST_KEYLENGTH)
+
+ assert not popen.called