From e589861c61b378e9a6a57ec53fec618492b91ac7 Mon Sep 17 00:00:00 2001 From: Jesper Beltman Date: Thu, 17 Oct 2024 21:43:35 +0200 Subject: [PATCH 1/5] Fix list_ for presense detection --- src/saltext/mysql/cache/mysql_cache.py | 156 +++++++++---------------- tests/functional/cache/test_mysql.py | 12 +- 2 files changed, 58 insertions(+), 110 deletions(-) diff --git a/src/saltext/mysql/cache/mysql_cache.py b/src/saltext/mysql/cache/mysql_cache.py index 1e935ce8..35eb3b73 100644 --- a/src/saltext/mysql/cache/mysql_cache.py +++ b/src/saltext/mysql/cache/mysql_cache.py @@ -8,13 +8,9 @@ .. warning:: - The mysql.database and mysql.table_name will be directly added into certain + The mysql_cache.database and mysql_cache.table_name will be directly added into certain queries. Salt treats these as trusted input. -The module requires the database (default ``salt_cache``) to exist but creates -its own table if needed. The keys are indexed using the ``bank`` and -``etcd_key`` columns. - To enable this cache plugin, the master will need the python client for MySQL installed. This can be easily installed with pip: @@ -27,14 +23,31 @@ .. code-block:: yaml - mysql.host: 127.0.0.1 - mysql.port: 2379 - mysql.user: None - mysql.password: None - mysql.database: salt_cache - mysql.table_name: cache + mysql_cache.host: 127.0.0.1 + mysql_cache.port: 3306 + mysql_cache.user: None + mysql_cache.password: None + mysql_cache.database: salt_cache + mysql_cache.table_name: cache # This may be enabled to create a fresh connection on every call - mysql.fresh_connection: false + mysql_cache.fresh_connection: false + +Use the following mysql database schema: + +.. code-block:: sql + + CREATE DATABASE salt_cache; + USE salt_cache; + + CREATE TABLE IF NOT EXISTS cache ( + bank VARCHAR(255), + cache_key VARCHAR(255), + data MEDIUMBLOB, + last_update TIMESTAMP NOT NULL + DEFAULT CURRENT_TIMESTAMP + ON UPDATE CURRENT_TIMESTAMP, + PRIMARY KEY(bank, cache_key) + ); To use the mysql as a minion data cache backend, set the master ``cache`` config value to ``mysql``: @@ -43,7 +56,6 @@ cache: mysql - .. _`MySQL documentation`: https://github.com/coreos/mysql """ @@ -82,8 +94,6 @@ class InterfaceError(Exception): MySQLdb = None -_DEFAULT_DATABASE_NAME = "salt_cache" -_DEFAULT_CACHE_TABLE_NAME = "cache" _RECONNECT_INTERVAL_SEC = 0.050 log = logging.getLogger(__name__) @@ -91,7 +101,7 @@ class InterfaceError(Exception): # Module properties __virtualname__ = "mysql" -__func_alias__ = {"ls": "list"} +__func_alias__ = {"list_": "list"} def __virtual__(): @@ -157,69 +167,6 @@ def run_query(conn, query, args=None, retries=3): ) from e -def _create_table(): - """ - Create table if needed - """ - # Explicitly check if the table already exists as the library logs a - # warning on CREATE TABLE - query = """SELECT COUNT(TABLE_NAME) FROM information_schema.tables - WHERE table_schema = %s AND table_name = %s""" - cur, _ = run_query( - __context__.get("mysql_client"), - query, - args=(__context__["mysql_kwargs"]["db"], __context__["mysql_table_name"]), - ) - r = cur.fetchone() - cur.close() - if r[0] == 1: - query = """ - SELECT COUNT(TABLE_NAME) - FROM - information_schema.columns - WHERE - table_schema = %s - AND table_name = %s - AND column_name = 'last_update' - """ - cur, _ = run_query( - __context__["mysql_client"], - query, - args=(__context__["mysql_kwargs"]["db"], __context__["mysql_table_name"]), - ) - r = cur.fetchone() - cur.close() - if r[0] == 1: - return - else: - query = """ - ALTER TABLE {}.{} - ADD COLUMN last_update TIMESTAMP NOT NULL - DEFAULT CURRENT_TIMESTAMP - ON UPDATE CURRENT_TIMESTAMP - """.format( - __context__["mysql_kwargs"]["db"], __context__["mysql_table_name"] - ) - cur, _ = run_query(__context__["mysql_client"], query) - cur.close() - return - - query = """CREATE TABLE IF NOT EXISTS {} ( - bank CHAR(255), - etcd_key CHAR(255), - data MEDIUMBLOB, - last_update TIMESTAMP NOT NULL - DEFAULT CURRENT_TIMESTAMP - ON UPDATE CURRENT_TIMESTAMP, - PRIMARY KEY(bank, etcd_key) - );""".format( - __context__["mysql_table_name"] - ) - log.info("mysql_cache: creating table %s", __context__["mysql_table_name"]) - cur, _ = run_query(__context__.get("mysql_client"), query) - cur.close() - - def _init_client(): """Initialize connection and create table if needed""" if __context__.get("mysql_client") is not None: @@ -228,22 +175,22 @@ def _init_client(): opts = copy.deepcopy(__opts__) mysql_kwargs = { "autocommit": True, - "host": opts.pop("mysql.host", "127.0.0.1"), - "user": opts.pop("mysql.user", None), - "passwd": opts.pop("mysql.password", None), - "db": opts.pop("mysql.database", _DEFAULT_DATABASE_NAME), - "port": opts.pop("mysql.port", 3306), - "unix_socket": opts.pop("mysql.unix_socket", None), - "connect_timeout": opts.pop("mysql.connect_timeout", None), + "host": opts.pop("mysql_cache.host", "127.0.0.1"), + "user": opts.pop("mysql_cache.user", None), + "passwd": opts.pop("mysql_cache.password", None), + "db": opts.pop("mysql_cache.database", "salt_cache"), + "port": opts.pop("mysql_cache.port", 3306), + "unix_socket": opts.pop("mysql_cache.unix_socket", None), + "connect_timeout": opts.pop("mysql_cache.connect_timeout", None), } mysql_kwargs["autocommit"] = True - __context__["mysql_table_name"] = opts.pop("mysql.table_name", "salt") - __context__["mysql_fresh_connection"] = opts.pop("mysql.fresh_connection", False) + __context__["mysql_table_name"] = opts.pop("mysql_cache.table_name", "cache") + __context__["mysql_fresh_connection"] = opts.pop("mysql_cache.fresh_connection", False) # Gather up any additional MySQL configuration options for k in opts: - if k.startswith("mysql."): + if k.startswith("mysql_cache."): _key = k.split(".")[1] mysql_kwargs[_key] = opts.get(k) @@ -254,10 +201,8 @@ def _init_client(): mysql_kwargs.pop(k) kwargs_copy = mysql_kwargs.copy() kwargs_copy["passwd"] = "" - log.info("mysql_cache: Setting up client with params: %r", kwargs_copy) + log.info("Cache mysql: Setting up client with params: %r", kwargs_copy) __context__["mysql_kwargs"] = mysql_kwargs - # The MySQL client is created later on by run_query - _create_table() def store(bank, key, data): @@ -266,7 +211,7 @@ def store(bank, key, data): """ _init_client() data = salt.payload.dumps(data) - query = "REPLACE INTO {} (bank, etcd_key, data) values(%s,%s,%s)".format( + query = "REPLACE INTO {} (bank, cache_key, data) values(%s,%s,%s)".format( __context__["mysql_table_name"] ) args = (bank, key, data) @@ -282,7 +227,7 @@ def fetch(bank, key): Fetch a key value. """ _init_client() - query = "SELECT data FROM {} WHERE bank=%s AND etcd_key=%s".format( + query = "SELECT data FROM {} WHERE bank=%s AND cache_key=%s".format( __context__["mysql_table_name"] ) cur, _ = run_query(__context__.get("mysql_client"), query, args=(bank, key)) @@ -303,23 +248,26 @@ def flush(bank, key=None): data = (bank,) else: data = (bank, key) - query += " AND etcd_key=%s" + query += " AND cache_key=%s" cur, _ = run_query(__context__.get("mysql_client"), query, args=data) cur.close() -def ls(bank): +def list_(bank): """ - Return an iterable object containing all entries stored in the specified - bank. + Return an iterable object containing all entries stored in the specified bank. """ _init_client() - query = "SELECT etcd_key FROM {} WHERE bank=%s".format(__context__["mysql_table_name"]) - cur, _ = run_query(__context__.get("mysql_client"), query, args=(bank,)) + query = "SELECT bank FROM {}".format(__context__["mysql_table_name"]) + cur, _ = run_query(__context__.get("mysql_client"), query) out = [row[0] for row in cur.fetchall()] cur.close() - return out + minions = [] + for entry in out: + minion = entry.replace('minions/', '') + minions.append(minion) + return minions def contains(bank, key): @@ -332,7 +280,7 @@ def contains(bank, key): query = "SELECT COUNT(data) FROM {} WHERE bank=%s".format(__context__["mysql_table_name"]) else: data = (bank, key) - query = "SELECT COUNT(data) FROM {} WHERE bank=%s AND etcd_key=%s".format( + query = "SELECT COUNT(data) FROM {} WHERE bank=%s AND cache_key=%s".format( __context__["mysql_table_name"] ) cur, _ = run_query(__context__.get("mysql_client"), query, args=data) @@ -347,11 +295,11 @@ def updated(bank, key): key. """ _init_client() - query = "SELECT UNIX_TIMESTAMP(last_update) FROM {} WHERE bank=%s " "AND etcd_key=%s".format( + query = "SELECT UNIX_TIMESTAMP(last_update) FROM {} WHERE bank=%s " "AND cache_key=%s".format( __context__["mysql_table_name"] ) data = (bank, key) cur, _ = run_query(__context__.get("mysql_client"), query=query, args=data) r = cur.fetchone() cur.close() - return int(r[0]) if r else r + return int(r[0]) if r else r \ No newline at end of file diff --git a/tests/functional/cache/test_mysql.py b/tests/functional/cache/test_mysql.py index f59d51c3..cf4b4c06 100644 --- a/tests/functional/cache/test_mysql.py +++ b/tests/functional/cache/test_mysql.py @@ -26,12 +26,12 @@ def mysql_combo(create_mysql_combo): # pylint: disable=function-redefined def cache(minion_opts, mysql_container): opts = minion_opts.copy() opts["cache"] = "mysql" - opts["mysql.host"] = "127.0.0.1" - opts["mysql.port"] = mysql_container.mysql_port - opts["mysql.user"] = mysql_container.mysql_user - opts["mysql.password"] = mysql_container.mysql_passwd - opts["mysql.database"] = mysql_container.mysql_database - opts["mysql.table_name"] = "cache" + opts["mysql_cache.host"] = "127.0.0.1" + opts["mysql_cache.port"] = mysql_container.mysql_port + opts["mysql_cache.user"] = mysql_container.mysql_user + opts["mysql_cache.password"] = mysql_container.mysql_passwd + opts["mysql_cache.database"] = mysql_container.mysql_database + opts["mysql_cache.table_name"] = "cache" cache = salt.cache.factory(opts) return cache From d1a8090e04763d18a22e8b26d1b36e383d4010cc Mon Sep 17 00:00:00 2001 From: Jesper Beltman Date: Thu, 24 Oct 2024 15:29:15 +0200 Subject: [PATCH 2/5] docs: Add changelog issue 9 --- changelog/9.fixed.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog/9.fixed.md diff --git a/changelog/9.fixed.md b/changelog/9.fixed.md new file mode 100644 index 00000000..56bc1c7d --- /dev/null +++ b/changelog/9.fixed.md @@ -0,0 +1 @@ +Fixed presence detection with using the mysql_cache module and removal of create table function \ No newline at end of file From 7b5ceba5776df970805c9d71de8b7de0369b2e8e Mon Sep 17 00:00:00 2001 From: Jesper Beltman Date: Tue, 12 Nov 2024 16:26:59 +0100 Subject: [PATCH 3/5] Revert and adjust list function --- src/saltext/mysql/cache/mysql_cache.py | 152 +++++++++++++++++-------- tests/functional/cache/test_mysql.py | 12 +- 2 files changed, 111 insertions(+), 53 deletions(-) diff --git a/src/saltext/mysql/cache/mysql_cache.py b/src/saltext/mysql/cache/mysql_cache.py index 35eb3b73..2c415f40 100644 --- a/src/saltext/mysql/cache/mysql_cache.py +++ b/src/saltext/mysql/cache/mysql_cache.py @@ -8,9 +8,13 @@ .. warning:: - The mysql_cache.database and mysql_cache.table_name will be directly added into certain + The mysql.database and mysql.table_name will be directly added into certain queries. Salt treats these as trusted input. +The module requires the database (default ``salt_cache``) to exist but creates +its own table if needed. The keys are indexed using the ``bank`` and +``etcd_key`` columns. + To enable this cache plugin, the master will need the python client for MySQL installed. This can be easily installed with pip: @@ -23,31 +27,14 @@ .. code-block:: yaml - mysql_cache.host: 127.0.0.1 - mysql_cache.port: 3306 - mysql_cache.user: None - mysql_cache.password: None - mysql_cache.database: salt_cache - mysql_cache.table_name: cache + mysql.host: 127.0.0.1 + mysql.port: 2379 + mysql.user: None + mysql.password: None + mysql.database: salt_cache + mysql.table_name: cache # This may be enabled to create a fresh connection on every call - mysql_cache.fresh_connection: false - -Use the following mysql database schema: - -.. code-block:: sql - - CREATE DATABASE salt_cache; - USE salt_cache; - - CREATE TABLE IF NOT EXISTS cache ( - bank VARCHAR(255), - cache_key VARCHAR(255), - data MEDIUMBLOB, - last_update TIMESTAMP NOT NULL - DEFAULT CURRENT_TIMESTAMP - ON UPDATE CURRENT_TIMESTAMP, - PRIMARY KEY(bank, cache_key) - ); + mysql.fresh_connection: false To use the mysql as a minion data cache backend, set the master ``cache`` config value to ``mysql``: @@ -56,6 +43,7 @@ cache: mysql + .. _`MySQL documentation`: https://github.com/coreos/mysql """ @@ -94,6 +82,8 @@ class InterfaceError(Exception): MySQLdb = None +_DEFAULT_DATABASE_NAME = "salt_cache" +_DEFAULT_CACHE_TABLE_NAME = "cache" _RECONNECT_INTERVAL_SEC = 0.050 log = logging.getLogger(__name__) @@ -167,6 +157,69 @@ def run_query(conn, query, args=None, retries=3): ) from e +def _create_table(): + """ + Create table if needed + """ + # Explicitly check if the table already exists as the library logs a + # warning on CREATE TABLE + query = """SELECT COUNT(TABLE_NAME) FROM information_schema.tables + WHERE table_schema = %s AND table_name = %s""" + cur, _ = run_query( + __context__.get("mysql_client"), + query, + args=(__context__["mysql_kwargs"]["db"], __context__["mysql_table_name"]), + ) + r = cur.fetchone() + cur.close() + if r[0] == 1: + query = """ + SELECT COUNT(TABLE_NAME) + FROM + information_schema.columns + WHERE + table_schema = %s + AND table_name = %s + AND column_name = 'last_update' + """ + cur, _ = run_query( + __context__["mysql_client"], + query, + args=(__context__["mysql_kwargs"]["db"], __context__["mysql_table_name"]), + ) + r = cur.fetchone() + cur.close() + if r[0] == 1: + return + else: + query = """ + ALTER TABLE {}.{} + ADD COLUMN last_update TIMESTAMP NOT NULL + DEFAULT CURRENT_TIMESTAMP + ON UPDATE CURRENT_TIMESTAMP + """.format( + __context__["mysql_kwargs"]["db"], __context__["mysql_table_name"] + ) + cur, _ = run_query(__context__["mysql_client"], query) + cur.close() + return + + query = """CREATE TABLE IF NOT EXISTS {} ( + bank CHAR(255), + etcd_key CHAR(255), + data MEDIUMBLOB, + last_update TIMESTAMP NOT NULL + DEFAULT CURRENT_TIMESTAMP + ON UPDATE CURRENT_TIMESTAMP, + PRIMARY KEY(bank, etcd_key) + );""".format( + __context__["mysql_table_name"] + ) + log.info("mysql_cache: creating table %s", __context__["mysql_table_name"]) + cur, _ = run_query(__context__.get("mysql_client"), query) + cur.close() + + def _init_client(): """Initialize connection and create table if needed""" if __context__.get("mysql_client") is not None: @@ -175,22 +228,22 @@ def _init_client(): opts = copy.deepcopy(__opts__) mysql_kwargs = { "autocommit": True, - "host": opts.pop("mysql_cache.host", "127.0.0.1"), - "user": opts.pop("mysql_cache.user", None), - "passwd": opts.pop("mysql_cache.password", None), - "db": opts.pop("mysql_cache.database", "salt_cache"), - "port": opts.pop("mysql_cache.port", 3306), - "unix_socket": opts.pop("mysql_cache.unix_socket", None), - "connect_timeout": opts.pop("mysql_cache.connect_timeout", None), + "host": opts.pop("mysql.host", "127.0.0.1"), + "user": opts.pop("mysql.user", None), + "passwd": opts.pop("mysql.password", None), + "db": opts.pop("mysql.database", _DEFAULT_DATABASE_NAME), + "port": opts.pop("mysql.port", 3306), + "unix_socket": opts.pop("mysql.unix_socket", None), + "connect_timeout": opts.pop("mysql.connect_timeout", None), } mysql_kwargs["autocommit"] = True - __context__["mysql_table_name"] = opts.pop("mysql_cache.table_name", "cache") - __context__["mysql_fresh_connection"] = opts.pop("mysql_cache.fresh_connection", False) + __context__["mysql_table_name"] = opts.pop("mysql.table_name", "salt") + __context__["mysql_fresh_connection"] = opts.pop("mysql.fresh_connection", False) # Gather up any additional MySQL configuration options for k in opts: - if k.startswith("mysql_cache."): + if k.startswith("mysql."): _key = k.split(".")[1] mysql_kwargs[_key] = opts.get(k) @@ -201,8 +254,10 @@ def _init_client(): mysql_kwargs.pop(k) kwargs_copy = mysql_kwargs.copy() kwargs_copy["passwd"] = "" - log.info("Cache mysql: Setting up client with params: %r", kwargs_copy) + log.info("mysql_cache: Setting up client with params: %r", kwargs_copy) __context__["mysql_kwargs"] = mysql_kwargs + # The MySQL client is created later on by run_query + _create_table() def store(bank, key, data): @@ -211,7 +266,7 @@ def store(bank, key, data): """ _init_client() data = salt.payload.dumps(data) - query = "REPLACE INTO {} (bank, cache_key, data) values(%s,%s,%s)".format( + query = "REPLACE INTO {} (bank, etcd_key, data) values(%s,%s,%s)".format( __context__["mysql_table_name"] ) args = (bank, key, data) @@ -227,7 +282,7 @@ def fetch(bank, key): Fetch a key value. """ _init_client() - query = "SELECT data FROM {} WHERE bank=%s AND cache_key=%s".format( + query = "SELECT data FROM {} WHERE bank=%s AND etcd_key=%s".format( __context__["mysql_table_name"] ) cur, _ = run_query(__context__.get("mysql_client"), query, args=(bank, key)) @@ -248,7 +303,7 @@ def flush(bank, key=None): data = (bank,) else: data = (bank, key) - query += " AND cache_key=%s" + query += " AND etcd_key=%s" cur, _ = run_query(__context__.get("mysql_client"), query, args=data) cur.close() @@ -258,16 +313,19 @@ def list_(bank): """ Return an iterable object containing all entries stored in the specified bank. """ + bank_like = bank + '%' _init_client() - query = "SELECT bank FROM {}".format(__context__["mysql_table_name"]) - cur, _ = run_query(__context__.get("mysql_client"), query) + banks_query = "SELECT bank FROM {} WHERE bank LIKE %s".format( + __context__["mysql_table_name"] + ) + cur, _ = run_query(__context__.get("mysql_client"), banks_query, args=(bank_like,)) out = [row[0] for row in cur.fetchall()] cur.close() - minions = [] + data = [] for entry in out: - minion = entry.replace('minions/', '') - minions.append(minion) - return minions + filtered_entry = entry.split('/')[-1] + data.append(filtered_entry) + return data def contains(bank, key): @@ -280,7 +338,7 @@ def contains(bank, key): query = "SELECT COUNT(data) FROM {} WHERE bank=%s".format(__context__["mysql_table_name"]) else: data = (bank, key) - query = "SELECT COUNT(data) FROM {} WHERE bank=%s AND cache_key=%s".format( + query = "SELECT COUNT(data) FROM {} WHERE bank=%s AND etcd_key=%s".format( __context__["mysql_table_name"] ) cur, _ = run_query(__context__.get("mysql_client"), query, args=data) @@ -295,7 +353,7 @@ def updated(bank, key): key. """ _init_client() - query = "SELECT UNIX_TIMESTAMP(last_update) FROM {} WHERE bank=%s " "AND cache_key=%s".format( + query = "SELECT UNIX_TIMESTAMP(last_update) FROM {} WHERE bank=%s " "AND etcd_key=%s".format( __context__["mysql_table_name"] ) data = (bank, key) diff --git a/tests/functional/cache/test_mysql.py b/tests/functional/cache/test_mysql.py index cf4b4c06..f59d51c3 100644 --- a/tests/functional/cache/test_mysql.py +++ b/tests/functional/cache/test_mysql.py @@ -26,12 +26,12 @@ def mysql_combo(create_mysql_combo): # pylint: disable=function-redefined def cache(minion_opts, mysql_container): opts = minion_opts.copy() opts["cache"] = "mysql" - opts["mysql_cache.host"] = "127.0.0.1" - opts["mysql_cache.port"] = mysql_container.mysql_port - opts["mysql_cache.user"] = mysql_container.mysql_user - opts["mysql_cache.password"] = mysql_container.mysql_passwd - opts["mysql_cache.database"] = mysql_container.mysql_database - opts["mysql_cache.table_name"] = "cache" + opts["mysql.host"] = "127.0.0.1" + opts["mysql.port"] = mysql_container.mysql_port + opts["mysql.user"] = mysql_container.mysql_user + opts["mysql.password"] = mysql_container.mysql_passwd + opts["mysql.database"] = mysql_container.mysql_database + opts["mysql.table_name"] = "cache" cache = salt.cache.factory(opts) return cache From 8ad5fa2fe84941b0472b01cc34c3b95503146a43 Mon Sep 17 00:00:00 2001 From: Jesper Beltman Date: Tue, 12 Nov 2024 16:42:17 +0100 Subject: [PATCH 4/5] fix: No newline at end of file --- src/saltext/mysql/cache/mysql_cache.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/saltext/mysql/cache/mysql_cache.py b/src/saltext/mysql/cache/mysql_cache.py index 2c415f40..52d2f44d 100644 --- a/src/saltext/mysql/cache/mysql_cache.py +++ b/src/saltext/mysql/cache/mysql_cache.py @@ -360,4 +360,4 @@ def updated(bank, key): cur, _ = run_query(__context__.get("mysql_client"), query=query, args=data) r = cur.fetchone() cur.close() - return int(r[0]) if r else r \ No newline at end of file + return int(r[0]) if r else r From ea4a06924944a28c1fab32c3d4e98506f4d6e7d7 Mon Sep 17 00:00:00 2001 From: Jesper Beltman <129941616+JesperBeltman@users.noreply.github.com> Date: Tue, 12 Nov 2024 21:00:46 +0100 Subject: [PATCH 5/5] Update changelog/9.fixed.md Co-authored-by: jeanluc <2163936+lkubb@users.noreply.github.com> --- changelog/9.fixed.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/9.fixed.md b/changelog/9.fixed.md index 56bc1c7d..e3b1d779 100644 --- a/changelog/9.fixed.md +++ b/changelog/9.fixed.md @@ -1 +1 @@ -Fixed presence detection with using the mysql_cache module and removal of create table function \ No newline at end of file +Fixed `mysql_cache.list` not listing nested banks, thereby fixed presence detection when using the `mysql_cache` module \ No newline at end of file