From 5b1f701e6a2c075278a7007a3c0174d1af17cc04 Mon Sep 17 00:00:00 2001 From: Sebastian Neubauer Date: Thu, 28 Sep 2017 14:25:45 +0200 Subject: [PATCH 1/2] fixed bug in correct exception handling in create db --- postgraas_server/backends/docker/__init__.py | 2 +- .../backends/postgres_cluster/__init__.py | 9 +- .../test_postgres_cluster_driver.py | 23 +- .../test_backend_behaviour.py | 197 ++++++++++++++++++ 4 files changed, 226 insertions(+), 5 deletions(-) create mode 100644 tests/test_integration/test_backend_behaviour.py diff --git a/postgraas_server/backends/docker/__init__.py b/postgraas_server/backends/docker/__init__.py index 7365adf..0277a66 100644 --- a/postgraas_server/backends/docker/__init__.py +++ b/postgraas_server/backends/docker/__init__.py @@ -11,7 +11,7 @@ def create(self, entity, connection_info): from . import postgres_instance_driver as pg try: return pg.create_postgres_instance(entity.postgraas_instance_name, connection_info) - except APIError as e: + except (APIError, ValueError) as e: raise PostgraasApiException(str(e)) def delete(self, entity): diff --git a/postgraas_server/backends/postgres_cluster/__init__.py b/postgraas_server/backends/postgres_cluster/__init__.py index 48d9085..a4db48a 100644 --- a/postgraas_server/backends/postgres_cluster/__init__.py +++ b/postgraas_server/backends/postgres_cluster/__init__.py @@ -1,13 +1,16 @@ from . import postgres_cluster_driver as pgcd - +from ..exceptions import PostgraasApiException class PGClusterBackend(object): def __init__(self, config): self.config = config def create(self, entity, connection_info): - pgcd.create_postgres_db(connection_info, self.config) - return entity.id + try: + pgcd.create_postgres_db(connection_info, self.config) + except ValueError as e: + raise PostgraasApiException(str(e)) + return None def delete(self, entity): pgcd.delete_database(entity.db_name, self.config) diff --git a/tests/test_integration/backends/postgres_cluster/test_postgres_cluster_driver.py b/tests/test_integration/backends/postgres_cluster/test_postgres_cluster_driver.py index d42af10..546667c 100644 --- a/tests/test_integration/backends/postgres_cluster/test_postgres_cluster_driver.py +++ b/tests/test_integration/backends/postgres_cluster/test_postgres_cluster_driver.py @@ -199,4 +199,25 @@ def test_create_postgres_instance_username_exists(self): headers={'Content-Type': 'application/json'}) delete_test_database_and_user(db_credentials['db_name'], db_credentials['db_username'], backend_config) delete_test_database_and_user(db_credentials_same_user['db_name'], db_credentials_same_user['db_username'], backend_config) - assert ("database or user already exists" in json.loads(response.data)['description']) is True \ No newline at end of file + assert ("database or user already exists" in json.loads(response.data)['description']) is True + + def test_create_postgres_instance_bad_username(self): + config = ConfigParser.ConfigParser() + + config.readfp(StringIO.StringIO(CONFIGS[self.backend])) + backend_config = dict(config.items('backend')) + print config + db_credentials = { + "postgraas_instance_name": "tests_postgraas_test_create_postgres_instance_api", + "db_name": 'test_create_postgres_instance_exists', + "db_username": 'test-invalid-username', + "db_pwd": 'test_db_pwd', + "host": backend_config['host'], + "port": backend_config['port'] + } + response = self.app_client.post('/api/v2/postgraas_instances', + data=json.dumps(db_credentials), + headers={'Content-Type': 'application/json'}) + print response.data + delete_test_database_and_user(db_credentials['db_name'], db_credentials['db_username'], backend_config) + assert ('syntax error at or near "-"' in json.loads(response.data)['msg']) is True diff --git a/tests/test_integration/test_backend_behaviour.py b/tests/test_integration/test_backend_behaviour.py new file mode 100644 index 0000000..df6c826 --- /dev/null +++ b/tests/test_integration/test_backend_behaviour.py @@ -0,0 +1,197 @@ +import os +import json +import uuid + +import docker +import pytest +from mock import patch, MagicMock, Mock + +import postgraas_server.backends.docker.postgres_instance_driver as pid +import postgraas_server.backends.postgres_cluster.postgres_cluster_driver as pgcd +import postgraas_server.configuration as configuration +from postgraas_server.backends.exceptions import PostgraasApiException +from postgraas_server.create_app import create_app +from postgraas_server.management_resources import DBInstance +from .utils import wait_for_postgres_listening + +DOCKER_CONFIG = """ +[metadb] +db_name = postgraas +db_username = postgraas +db_pwd = postgraas12 +host = localhost +port = 54321 + +[backend] +type = docker +""" + +CLUSTER_CONFIG = """ +[metadb] +db_name = postgraas +db_username = postgraas +db_pwd = postgraas12 +host = localhost +port = 54321 + +[backend] +type = pg_cluster +host = {host} +port = {port} +database = {database} +username = {username} +password = {password} +""".format( + database=os.environ.get('PGDATABASE', 'postgres'), + username=os.environ.get('PGUSER', 'postgres'), + password=os.environ.get('PGPASSWORD', 'postgres'), + port=os.environ.get('PGPORT', '5432'), + host=os.environ.get('PGHOST', 'localhost') +) + +CONFIGS = { + 'docker': DOCKER_CONFIG, + 'pg_cluster': CLUSTER_CONFIG, +} + + +def remove_digits(s): + return ''.join(c for c in s if not c.isdigit()) + + +def delete_all_test_postgraas_container(): + c = pid._docker_client() + for container in c.containers.list(): + if container.name.startswith("tests_postgraas_"): + container.remove(force=True) + + +def delete_all_test_database_and_user(config): + con = pgcd._create_pg_connection(config) + cur = con.cursor() + print "hier" + cur.execute( +'''SELECT d.datname, u.usename + FROM pg_database d + JOIN pg_user u ON (d.datdba = u.usesysid);''') + for db in cur: + print db[0] + if db[0].startswith("tests_postgraas_"): + print db + delete_test_database_and_user(db[0], db[1], config) + cur.execute( +'''SELECT u.usename + FROM pg_user u;''') + for db in cur: + print db[0] + if db[0].startswith("tests_postgraas_"): + print db + pgcd.delete_user(db[0], config) + + +def delete_test_database_and_user(db_name, username, config): + pgcd.delete_database(db_name, config) + pgcd.delete_user(username, config) + + +@pytest.fixture(params=['docker', 'pg_cluster']) +def parametrized_setup(request, tmpdir): + from postgraas_server.management_resources import db + cfg = tmpdir.join('config') + cfg.write(CONFIGS[request.param]) + config = configuration.get_config(cfg.strpath) + this_app = create_app(config) + this_app.config['SQLALCHEMY_DATABASE_URI'] = "sqlite://" + this_app.use_reloader = False + this_app.config['TESTING'] = True + ctx = this_app.app_context() + ctx.push() + db.create_all() + username, db_name = str(uuid.uuid4()).replace('-', '_'), str(uuid.uuid4()).replace('-', '_') + request.cls.this_app = this_app + request.cls.app_client = this_app.test_client() + request.cls.db_name = remove_digits(db_name) + request.cls.username = remove_digits(username) + request.cls.backend = request.param + try: + yield + except Exception: + pass + if request.param == 'docker': + delete_all_test_postgraas_container() + elif request.param == 'pg_cluster': + delete_all_test_database_and_user(dict(config.items('backend'))) + db.drop_all() + ctx.pop() + + +@pytest.mark.usefixtures('parametrized_setup') +class TestPostgraasApi(): + + def test_create_and_delete_postgres_instance(self): + db_credentials = { + "db_name": 'tests_postgraas_instance_name', + "db_username": 'tests_postgraas_db_username', + "db_pwd": 'test_db_pwd', + "host": pid.get_hostname(), + "port": pid.get_open_port() + } + db_entry = DBInstance( + postgraas_instance_name=db_credentials['db_name'], + db_name=db_credentials['db_name'], + username=db_credentials['db_username'], + password="", + hostname=db_credentials['host'], + port=db_credentials['port'] + ) + db_entry.container_id = self.this_app.postgraas_backend.create(db_entry, db_credentials) + self.this_app.postgraas_backend.delete(db_entry) + assert True + + def test_create_postgraas_twice(self): + db_credentials = { + "db_name": 'tests_postgraas_instance_name', + "db_username": 'tests_postgraas_db_username', + "db_pwd": 'test_db_pwd', + "host": pid.get_hostname(), + "port": pid.get_open_port() + } + db_entry = DBInstance( + postgraas_instance_name=db_credentials['db_name'], + db_name=db_credentials['db_name'], + username=db_credentials['db_username'], + password="", + hostname=db_credentials['host'], + port=db_credentials['port'] + ) + db_entry.container_id = self.this_app.postgraas_backend.create(db_entry, db_credentials) + with pytest.raises(PostgraasApiException) as excinfo: + db_entry.container_id = self.this_app.postgraas_backend.create(db_entry, db_credentials) + if self.backend == "pg_cluster": + assert excinfo.value.message == 'db or user already exists' + elif self.backend == "docker": + assert excinfo.value.message == 'Container exists already' + self.this_app.postgraas_backend.delete(db_entry) + assert True + + def test_create_postgraas_bad_username(self): + db_credentials = { + "db_name": 'tests_postgraas_instance_name', + "db_username": 'tests_postgraas_db-bad username', + "db_pwd": 'test_db_pwd', + "host": pid.get_hostname(), + "port": pid.get_open_port() + } + db_entry = DBInstance( + postgraas_instance_name=db_credentials['db_name'], + db_name=db_credentials['db_name'], + username=db_credentials['db_username'], + password="", + hostname=db_credentials['host'], + port=db_credentials['port'] + ) + if self.backend == "pg_cluster": + with pytest.raises(PostgraasApiException) as excinfo: + db_entry.container_id = self.this_app.postgraas_backend.create(db_entry, db_credentials) + self.this_app.postgraas_backend.delete(db_entry) + assert 'syntax error at or near "-"' in excinfo.value.message From b6d9fbaf35d31f26a8574a1febb620a0a23fbb9a Mon Sep 17 00:00:00 2001 From: Sebastian Neubauer Date: Thu, 28 Sep 2017 14:47:19 +0200 Subject: [PATCH 2/2] added correct exception handling in delete --- postgraas_server/backends/docker/__init__.py | 9 ++++++-- .../backends/postgres_cluster/__init__.py | 11 ++++++++-- .../test_backend_behaviour.py | 22 +++++++++++++++++++ 3 files changed, 38 insertions(+), 4 deletions(-) diff --git a/postgraas_server/backends/docker/__init__.py b/postgraas_server/backends/docker/__init__.py index 0277a66..f935f18 100644 --- a/postgraas_server/backends/docker/__init__.py +++ b/postgraas_server/backends/docker/__init__.py @@ -15,13 +15,18 @@ def create(self, entity, connection_info): raise PostgraasApiException(str(e)) def delete(self, entity): - from docker.errors import APIError + from docker.errors import APIError, NullResource, NotFound from . import postgres_instance_driver as pg + if not entity.container_id: + raise PostgraasApiException("container ID not provided") try: return pg.delete_postgres_instance(entity.container_id) - except APIError as e: + except NotFound as e: + raise PostgraasApiException("Could not delete, does not exist {}".format(entity.container_id)) + except (APIError, NullResource) as e: raise PostgraasApiException(str(e)) + def exists(self, entity): from . import postgres_instance_driver as pg if entity.postgraas_instance_name: diff --git a/postgraas_server/backends/postgres_cluster/__init__.py b/postgraas_server/backends/postgres_cluster/__init__.py index a4db48a..9173e19 100644 --- a/postgraas_server/backends/postgres_cluster/__init__.py +++ b/postgraas_server/backends/postgres_cluster/__init__.py @@ -2,6 +2,7 @@ from ..exceptions import PostgraasApiException class PGClusterBackend(object): + def __init__(self, config): self.config = config @@ -13,8 +14,14 @@ def create(self, entity, connection_info): return None def delete(self, entity): - pgcd.delete_database(entity.db_name, self.config) - pgcd.delete_user(entity.username, self.config) + try: + pgcd.delete_database(entity.db_name, self.config) + pgcd.delete_user(entity.username, self.config) + except ValueError as e: + if 'does not exist' in e.message: + raise PostgraasApiException("Could not delete, does not exist {}".format(entity.db_name)) + else: + raise PostgraasApiException(str(e)) def exists(self, entity): return pgcd.check_db_or_user_exists(entity.db_name, entity.username, self.config) diff --git a/tests/test_integration/test_backend_behaviour.py b/tests/test_integration/test_backend_behaviour.py index df6c826..737fc24 100644 --- a/tests/test_integration/test_backend_behaviour.py +++ b/tests/test_integration/test_backend_behaviour.py @@ -195,3 +195,25 @@ def test_create_postgraas_bad_username(self): db_entry.container_id = self.this_app.postgraas_backend.create(db_entry, db_credentials) self.this_app.postgraas_backend.delete(db_entry) assert 'syntax error at or near "-"' in excinfo.value.message + + def test_delete_nonexisting_db(self): + db_credentials = { + "db_name": 'tests_postgraas_instance_name', + "db_username": 'tests_postgraas_db-bad username', + "db_pwd": 'test_db_pwd', + "host": pid.get_hostname(), + "port": pid.get_open_port() + } + db_entry = DBInstance( + postgraas_instance_name=db_credentials['db_name'], + db_name=db_credentials['db_name'], + username=db_credentials['db_username'], + password="", + hostname=db_credentials['host'], + port=db_credentials['port'], + container_id="4n8nz48az49prdmdmprmr4doesnotexit" + + ) + with pytest.raises(PostgraasApiException) as excinfo: + db_entry.container_id = self.this_app.postgraas_backend.delete(db_entry) + assert 'does not exist' in excinfo.value.message \ No newline at end of file