From af64b375746a287ce3cfce08d2998d0299f6da72 Mon Sep 17 00:00:00 2001 From: Kirill Date: Sat, 12 Aug 2023 18:32:55 +0600 Subject: [PATCH 1/6] fix(DB): change backend to `postgresql` from `postgres` --- opensipscli/db.py | 26 +++++++++++++------------- opensipscli/modules/database.py | 10 +++++----- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/opensipscli/db.py b/opensipscli/db.py index cff1eb5..e2897c3 100644 --- a/opensipscli/db.py +++ b/opensipscli/db.py @@ -45,7 +45,7 @@ SUPPORTED_BACKENDS = [ "mysql", - "postgres", + "postgresql", "sqlite", "oracle", ] @@ -225,7 +225,7 @@ def __init__(self, db_url, db_name): raise except: logger.error("unexpected parsing exception") - elif self.dialect == "postgres" and \ + elif self.dialect == "postgresql" and \ (("authentication" in se.args[0] and "failed" in se.args[0]) or \ ("no password supplied" in se.args[0])): raise osdbAccessDeniedError @@ -243,7 +243,7 @@ def alter_role(self, role_name, role_options=None, role_password=None): alter attributes of a role object """ # TODO: is any other dialect using the "role" concept? - if self.dialect != "postgres": + if self.dialect != "postgresql": return False # TODO: do this only for SQLAlchemy @@ -277,7 +277,7 @@ def connect(self, db_name=None): # TODO: do this only for SQLAlchemy try: - if self.dialect == "postgres": + if self.dialect == "postgresql": self.db_url = self.set_url_db(self.db_url, self.db_name) if sqlalchemy_utils.database_exists(self.db_url) is True: engine = sqlalchemy.create_engine(self.db_url, isolation_level='AUTOCOMMIT') @@ -312,7 +312,7 @@ def create(self, db_name=None): self.db_name, self.dialect) # all good - it's time to create the database - if self.dialect == "postgres": + if self.dialect == "postgresql": self.__conn.connection.connection.set_isolation_level(0) try: self.__conn.execute("CREATE DATABASE {}".format(self.db_name)) @@ -408,7 +408,7 @@ def ensure_user(self, db_url): logger.exception("failed to flush privileges") return False - elif url.drivername.lower() == "postgres": + elif url.drivername.lower() == "postgresql": if not self.exists_role(url.username): logger.info("creating role %s", url.username) if not self.create_role(url.username, url.password): @@ -437,7 +437,7 @@ def create_role(self, role_name, role_password, update=False, create a role object (PostgreSQL secific) """ # TODO: is any other dialect using the "role" concept? - if self.dialect != "postgres": + if self.dialect != "postgresql": return False # TODO: do this only for SQLAlchemy @@ -517,7 +517,7 @@ def drop_role(self, role_name): drop a role object (PostgreSQL specific) """ # TODO: is any other dialect using the "role" concept? - if self.dialect != "postgres": + if self.dialect != "postgresql": return False # TODO: do this only for SQLAlchemy @@ -615,7 +615,7 @@ def exists_role(self, role_name=None): check for existence of a role object (PostgreSQL specific) """ # TODO: is any other dialect using the "role" concept? - if self.dialect != "postgres": + if self.dialect != "postgresql": return False # TODO: do this only for SQLAlchemy @@ -696,7 +696,7 @@ def get_role(self, role_name="opensips"): get attibutes of a role object (PostgreSQL specific) """ # TODO: is any other dialect using the "role" concept? - if self.dialect != "postgres": + if self.dialect != "postgresql": return False # TODO: do this only for SQLAlchemy @@ -721,7 +721,7 @@ def grant_db_options(self, role_name="opensips", role_options="ALL PRIVILEGES"): assign attibutes to a role object (PostgreSQL specific) """ # TODO: is any other dialect using the "role" concept? - if self.dialect != "postgres": + if self.dialect != "postgresql": return False # TODO: do this only for SQLAlchemy @@ -731,7 +731,7 @@ def grant_db_options(self, role_name="opensips", role_options="ALL PRIVILEGES"): return True def grant_table_options(self, role, table, privs="ALL PRIVILEGES"): - if self.dialect != "postgres": + if self.dialect != "postgresql": return False if not self.__conn: @@ -963,7 +963,7 @@ def get_url_driver(url, capitalize=False): driver = make_url(url).drivername.lower() capitalized = { 'mysql': 'MySQL', - 'postgres': 'PostgreSQL', + 'postgresql': 'PostgreSQL', 'sqlite': 'SQLite', 'oracle': 'Oracle', } diff --git a/opensipscli/modules/database.py b/opensipscli/modules/database.py index 5653287..ade596b 100644 --- a/opensipscli/modules/database.py +++ b/opensipscli/modules/database.py @@ -499,12 +499,12 @@ def get_admin_db_url(self, db_name): if cfg.exists('database_admin_url'): admin_url = cfg.get("database_admin_url") - if engine == "postgres": + if engine == "postgresql": admin_url = osdb.set_url_db(admin_url, 'postgres') else: admin_url = osdb.set_url_db(admin_url, None) else: - if engine == 'postgres': + if engine == 'postgresql': if getuser() != "postgres": logger.error("Command must be run as 'postgres' user: " "sudo -u postgres opensips-cli ...") @@ -513,7 +513,7 @@ def get_admin_db_url(self, db_name): """ For PG, do the initial setup using 'postgres' as role + DB """ - admin_url = "postgres://postgres@localhost/postgres" + admin_url = "postgresql://postgres@localhost/postgres" else: admin_url = "{}://root@localhost".format(engine) @@ -712,7 +712,7 @@ def create_tables(self, db_name, db_url, admin_db, tables=[], logger.info("Running {}...".format(os.path.basename(table_file))) try: db.create_module(table_file) - if db.dialect == "postgres": + if db.dialect == "postgresql": self.pg_grant_table_access(table_file, username, admin_db) except osdbModuleAlreadyExistsError: logger.error("{} table(s) are already created!".format(module)) @@ -772,7 +772,7 @@ def do_drop(self, params=None, modifiers=None): if db_url is None: return -1 - if db_url.lower().startswith("postgres"): + if db_url.lower().startswith("postgresql"): db_url = osdb.set_url_db(db_url, 'postgres') else: db_url = self.get_db_url(db_name) From f39929492da0252c99dfc8cd22059abac3a65bf5 Mon Sep 17 00:00:00 2001 From: Kirill Date: Sat, 12 Aug 2023 18:52:21 +0600 Subject: [PATCH 2/6] feat(DB): use either postgres or postgresql schema path --- opensipscli/modules/database.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/opensipscli/modules/database.py b/opensipscli/modules/database.py index ade596b..ad3c9da 100644 --- a/opensipscli/modules/database.py +++ b/opensipscli/modules/database.py @@ -931,8 +931,20 @@ def get_schema_path(self, backend="mysql"): logger.error("path '{}' to OpenSIPS DB scripts is not a directory!". format(db_path)) return None - - schema_path = os.path.join(db_path, backend) + + def build_schema_path(db_path, backend): + """ + Replaces schema path of postgresql to old postgre schema path if exists. + Should be deleted after opensips main repo refactors folder name to the new backend name. + """ + if backend == "postgresql": + old_postgre_path = os.path.join(db_path, "postgres") + if os.path.isdir(old_postgre_path): + return old_postgre_path + schema_path = os.path.join(db_path, backend) + return schema_path + + schema_path = build_schema_path(db_path, backend) if not os.path.isdir(schema_path): logger.error("invalid OpenSIPS DB scripts dir: '{}'!". format(schema_path)) From 4af815567db8d7984565a5b442151d5566efa1dd Mon Sep 17 00:00:00 2001 From: Kirill Date: Sat, 12 Aug 2023 19:02:57 +0600 Subject: [PATCH 3/6] refactor(DB): generalise `grant_db_options()` and use it --- opensipscli/db.py | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/opensipscli/db.py b/opensipscli/db.py index e2897c3..da82750 100644 --- a/opensipscli/db.py +++ b/opensipscli/db.py @@ -716,7 +716,7 @@ def get_role(self, role_name="opensips"): print (key + ": " + dict[key]) logger.debug("role_elements: %s", dict) - def grant_db_options(self, role_name="opensips", role_options="ALL PRIVILEGES"): + def grant_db_options(self, role_name, on_statement, privs="ALL PRIVILEGES"): """ assign attibutes to a role object (PostgreSQL specific) """ @@ -727,29 +727,22 @@ def grant_db_options(self, role_name="opensips", role_options="ALL PRIVILEGES"): # TODO: do this only for SQLAlchemy if not self.__conn: raise osdbError("connection not available") - - return True - - def grant_table_options(self, role, table, privs="ALL PRIVILEGES"): - if self.dialect != "postgresql": - return False - - if not self.__conn: - raise osdbError("connection not available") - - sqlcmd = "GRANT {} ON TABLE {} TO {}".format(privs, table, role) + + sqlcmd = "GRANT {} {} TO {}".format(privs, on_statement, role_name) logger.info(sqlcmd) try: - result = self.__conn.execute(sqlcmd) + self.__conn.execute(sqlcmd) except Exception as e: logger.exception(e) - logger.error("failed to grant '%s' to '%s' on table '%s'", - privs, role, table) + logger.error("failed to grant '%s' '%s' to '%s'", privs, on_statement, role_name) return False return True + def grant_table_options(self, role, table, privs="ALL PRIVILEGES"): + self.grant_db_options(role, "ON TABLE {}".format(table)) + def has_sqlalchemy(): """ check for usability of the SQLAlchemy modules From ffe3d38c7fa7067bb9911ec3023da0390e935118 Mon Sep 17 00:00:00 2001 From: Kirill Date: Sat, 12 Aug 2023 19:09:28 +0600 Subject: [PATCH 4/6] feat(DB): grant database public schema to user --- opensipscli/db.py | 3 +++ opensipscli/modules/database.py | 9 +++++++++ 2 files changed, 12 insertions(+) diff --git a/opensipscli/db.py b/opensipscli/db.py index da82750..67ea46d 100644 --- a/opensipscli/db.py +++ b/opensipscli/db.py @@ -739,6 +739,9 @@ def grant_db_options(self, role_name, on_statement, privs="ALL PRIVILEGES"): return False return True + + def grant_public_schema(self, role_name): + self.grant_db_options(role_name, "ON SCHEMA public") def grant_table_options(self, role, table, privs="ALL PRIVILEGES"): self.grant_db_options(role, "ON TABLE {}".format(table)) diff --git a/opensipscli/modules/database.py b/opensipscli/modules/database.py index ad3c9da..a3e89cd 100644 --- a/opensipscli/modules/database.py +++ b/opensipscli/modules/database.py @@ -707,6 +707,9 @@ def create_tables(self, db_name, db_url, admin_db, tables=[], username = osdb.get_url_user(db_url) admin_db.connect(db_name) + if db.dialect == "postgresql": + self.pg_grant_schema(username, admin_db) + # create tables from SQL schemas for module, table_file in table_files.items(): logger.info("Running {}...".format(os.path.basename(table_file))) @@ -974,3 +977,9 @@ def pg_grant_table_access(self, sql_file, username, admin_db): if res: seq = res.group(1) admin_db.grant_table_options(username, seq) + + def pg_grant_schema(self, username, admin_db): + """ + Grant access to public schema of DB + """ + admin_db.grant_public_schema(username) From 62edb25892b20075c3bb5725fd9ded7585ff3204 Mon Sep 17 00:00:00 2001 From: Kirill Date: Sat, 12 Aug 2023 21:33:37 +0600 Subject: [PATCH 5/6] docs: changed `postgres` scheme to `postgresql` --- docs/modules/database.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/modules/database.md b/docs/modules/database.md index ceb92b1..510f66d 100644 --- a/docs/modules/database.md +++ b/docs/modules/database.md @@ -67,7 +67,7 @@ Consider the following configuration file: #database_modules: acc clusterer dialog dialplan dispatcher domain rtpproxy usrloc database_modules: ALL -#database_admin_url: postgres://root@localhost +#database_admin_url: postgresql://root@localhost database_admin_url: mysql://root@localhost ``` From 16ba5fd06fcecec07c64ff16f43f4c9a7c9f7541 Mon Sep 17 00:00:00 2001 From: Kirill Date: Sat, 12 Aug 2023 21:35:09 +0600 Subject: [PATCH 6/6] fix(test): changed `PGSQL_URL` backend to `postgresql` --- test/test-database.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test-database.sh b/test/test-database.sh index 302e66d..596c2a2 100644 --- a/test/test-database.sh +++ b/test/test-database.sh @@ -21,7 +21,7 @@ CLI_CFG=/tmp/.__cli.cfg DB_NAME=_opensips_cli_test MYSQL_URL=mysql://opensips:opensipsrw@localhost -PGSQL_URL=postgres://opensips:opensipsrw@localhost +PGSQL_URL=postgresql://opensips:opensipsrw@localhost TESTS=( test_mysql_drop_1_prompt