From c392a77fe22262183cef40ff25452c0b87efdcfb Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Sat, 16 Dec 2023 16:30:59 -0700 Subject: [PATCH 1/5] make mac assistive tests more stable --- salt/modules/mac_assistive.py | 119 +++++++++++++++++----------------- 1 file changed, 58 insertions(+), 61 deletions(-) diff --git a/salt/modules/mac_assistive.py b/salt/modules/mac_assistive.py index 8583c40efba..b43b8022c8d 100644 --- a/salt/modules/mac_assistive.py +++ b/salt/modules/mac_assistive.py @@ -175,22 +175,21 @@ class TccDB: def _check_table_digest(self): # This logic comes from https://github.com/jacobsalmela/tccutil which is # Licensed under GPL-2.0 - with self.connection as conn: - cursor = conn.execute( - "SELECT sql FROM sqlite_master WHERE name='access' and type='table'" - ) - for row in cursor.fetchall(): - digest = hashlib.sha1(row["sql"].encode()).hexdigest()[:10] - if digest in ("ecc443615f", "80a4bb6912"): - # Mojave and Catalina - self.ge_mojave_and_catalina = True - elif digest in ("3d1c2a0e97", "cef70648de"): - # BigSur and later - self.ge_bigsur_and_later = True - else: - raise CommandExecutionError( - "TCC Database structure unknown for digest '{}'".format(digest) - ) + cursor = self.connection.execute( + "SELECT sql FROM sqlite_master WHERE name='access' and type='table'" + ) + for row in cursor.fetchall(): + digest = hashlib.sha1(row["sql"].encode()).hexdigest()[:10] + if digest in ("ecc443615f", "80a4bb6912"): + # Mojave and Catalina + self.ge_mojave_and_catalina = True + elif digest in ("3d1c2a0e97", "cef70648de"): + # BigSur and later + self.ge_bigsur_and_later = True + else: + raise CommandExecutionError( + "TCC Database structure unknown for digest '{}'".format(digest) + ) def _get_client_type(self, app_id): if app_id[0] == "/": @@ -200,14 +199,13 @@ class TccDB: return 0 def installed(self, app_id): - with self.connection as conn: - cursor = conn.execute( - "SELECT * from access WHERE client=? and service='kTCCServiceAccessibility'", - (app_id,), - ) - for row in cursor.fetchall(): - if row: - return True + cursor = self.connection.execute( + "SELECT * from access WHERE client=? and service='kTCCServiceAccessibility'", + (app_id,), + ) + for row in cursor.fetchall(): + if row: + return True return False def install(self, app_id, enable=True): @@ -234,9 +232,8 @@ class TccDB: # indirect_object_identifier # ), # FOREIGN KEY (policy_id) REFERENCES policies(id) ON DELETE CASCADE ON UPDATE CASCADE); - with self.connection as conn: - conn.execute( - """ + self.connection.execute( + """ INSERT or REPLACE INTO access VALUES ( 'kTCCServiceAccessibility', ?, @@ -253,8 +250,9 @@ class TccDB: 0 ) """, - (app_id, client_type, auth_value), - ) + (app_id, client_type, auth_value), + ) + self.connection.commit() elif self.ge_mojave_and_catalina: # CREATE TABLE IF NOT EXISTS "access" ( # service TEXT NOT NULL, @@ -276,9 +274,8 @@ class TccDB: # indirect_object_identifier # ), # FOREIGN KEY (policy_id) REFERENCES policies(id) ON DELETE CASCADE ON UPDATE CASCADE); - with self.connection as conn: - conn.execute( - """ + self.connection.execute( + """ INSERT or REPLACE INTO access VALUES( 'kTCCServiceAccessibility', ?, @@ -294,8 +291,9 @@ class TccDB: 0 ) """, - (app_id, client_type, auth_value), - ) + (app_id, client_type, auth_value), + ) + self.connection.commit() return True def enabled(self, app_id): @@ -303,14 +301,13 @@ class TccDB: column = "auth_value" elif self.ge_mojave_and_catalina: column = "allowed" - with self.connection as conn: - cursor = conn.execute( - "SELECT * from access WHERE client=? and service='kTCCServiceAccessibility'", - (app_id,), - ) - for row in cursor.fetchall(): - if row[column]: - return True + cursor = self.connection.execute( + "SELECT * from access WHERE client=? and service='kTCCServiceAccessibility'", + (app_id,), + ) + for row in cursor.fetchall(): + if row[column]: + return True return False def enable(self, app_id): @@ -320,13 +317,13 @@ class TccDB: column = "auth_value" elif self.ge_mojave_and_catalina: column = "allowed" - with self.connection as conn: - conn.execute( - "UPDATE access SET {} = ? WHERE client=? AND service IS 'kTCCServiceAccessibility'".format( - column - ), - (1, app_id), - ) + self.connection.execute( + "UPDATE access SET {} = ? WHERE client=? AND service IS 'kTCCServiceAccessibility'".format( + column + ), + (1, app_id), + ) + self.connection.commit() return True def disable(self, app_id): @@ -336,23 +333,23 @@ class TccDB: column = "auth_value" elif self.ge_mojave_and_catalina: column = "allowed" - with self.connection as conn: - conn.execute( - "UPDATE access SET {} = ? WHERE client=? AND service IS 'kTCCServiceAccessibility'".format( - column - ), - (0, app_id), - ) + self.connection.execute( + "UPDATE access SET {} = ? WHERE client=? AND service IS 'kTCCServiceAccessibility'".format( + column + ), + (0, app_id), + ) + self.connection.commit() return True def remove(self, app_id): if not self.installed(app_id): return False - with self.connection as conn: - conn.execute( - "DELETE from access where client IS ? AND service IS 'kTCCServiceAccessibility'", - (app_id,), - ) + self.connection.execute( + "DELETE from access where client IS ? AND service IS 'kTCCServiceAccessibility'", + (app_id,), + ) + self.connection.commit() return True def __enter__(self): From 01c194f53f6512275a0d068aa0af99ed49b1f80d Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Sat, 16 Dec 2023 18:04:31 -0700 Subject: [PATCH 2/5] Even more reliable pillar timeout test --- tests/pytests/integration/minion/test_return_retries.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/pytests/integration/minion/test_return_retries.py b/tests/pytests/integration/minion/test_return_retries.py index 24be4c39695..5e44d17317c 100644 --- a/tests/pytests/integration/minion/test_return_retries.py +++ b/tests/pytests/integration/minion/test_return_retries.py @@ -64,6 +64,7 @@ def test_pillar_timeout(salt_master_factory): "auto_accept": True, "worker_threads": 2, "peer": True, + "minion_data_cache": False, } minion_overrides = { "auth_timeout": 20, @@ -77,7 +78,7 @@ def test_pillar_timeout(salt_master_factory): - name: example - changes: True - result: True - - comment: "Nothing has actually been changed" + - comment: "Nothing has actually been changed {{ pillar['foo'] }}" """ master = salt_master_factory.salt_master_daemon( "pillar-timeout-master", @@ -105,6 +106,7 @@ def test_pillar_timeout(salt_master_factory): ) with master.started(), minion1.started(), minion2.started(), minion3.started(), minion4.started(), sls_tempfile: proc = cli.run("state.sls", sls_name, minion_tgt="*") + print(proc) # At least one minion should have a Pillar timeout assert proc.returncode == 1 minion_timed_out = False From ac90b8455ec5e1317eb2852757ab4a3fd203bda2 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Sun, 17 Dec 2023 09:54:45 -0700 Subject: [PATCH 3/5] Try install multiple times --- salt/modules/mac_assistive.py | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/salt/modules/mac_assistive.py b/salt/modules/mac_assistive.py index b43b8022c8d..78999052994 100644 --- a/salt/modules/mac_assistive.py +++ b/salt/modules/mac_assistive.py @@ -54,13 +54,18 @@ def install(app_id, enable=True): salt '*' assistive.install /usr/bin/osascript salt '*' assistive.install com.smileonmymac.textexpander """ - with TccDB() as db: - try: - return db.install(app_id, enable=enable) - except sqlite3.Error as exc: - raise CommandExecutionError( - "Error installing app({}): {}".format(app_id, exc) - ) + tires = 1 + while True: + with TccDB() as db: + try: + return db.install(app_id, enable=enable) + except sqlite3.Error as exc: + if tries <= 2: + time.sleep(10) + else: + raise CommandExecutionError( + "Error installing app({}): {}".format(app_id, exc) + ) def installed(app_id): From 62350b1dcfaf2ea204dc9d7405b761a124765171 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Sun, 17 Dec 2023 10:09:52 -0700 Subject: [PATCH 4/5] Fix lint --- salt/modules/mac_assistive.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/salt/modules/mac_assistive.py b/salt/modules/mac_assistive.py index 78999052994..132cfe70ea4 100644 --- a/salt/modules/mac_assistive.py +++ b/salt/modules/mac_assistive.py @@ -11,6 +11,7 @@ This module allows you to manage assistive access on macOS minions with 10.9+ import hashlib import logging import sqlite3 +import time import salt.utils.platform import salt.utils.stringutils @@ -54,7 +55,7 @@ def install(app_id, enable=True): salt '*' assistive.install /usr/bin/osascript salt '*' assistive.install com.smileonmymac.textexpander """ - tires = 1 + tries = 1 while True: with TccDB() as db: try: @@ -62,6 +63,7 @@ def install(app_id, enable=True): except sqlite3.Error as exc: if tries <= 2: time.sleep(10) + tries += 1 else: raise CommandExecutionError( "Error installing app({}): {}".format(app_id, exc) From cca011e0b02e4a63e2bd0b43fe10696feba9643f Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Sun, 17 Dec 2023 14:44:30 -0700 Subject: [PATCH 5/5] Make re-tries configurable --- salt/modules/mac_assistive.py | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/salt/modules/mac_assistive.py b/salt/modules/mac_assistive.py index 132cfe70ea4..90ab8814146 100644 --- a/salt/modules/mac_assistive.py +++ b/salt/modules/mac_assistive.py @@ -37,7 +37,7 @@ def __virtual__(): return __virtualname__ -def install(app_id, enable=True): +def install(app_id, enable=True, tries=3, wait=10): """ Install a bundle ID or command as being allowed to use assistive access. @@ -48,6 +48,12 @@ def install(app_id, enable=True): enabled Sets enabled or disabled status. Default is ``True``. + tries + How many times to try and write to a read-only tcc. Default is ``True``. + + wait + Number of seconds to wait between tries. Default is ``10``. + CLI Example: .. code-block:: bash @@ -55,15 +61,19 @@ def install(app_id, enable=True): salt '*' assistive.install /usr/bin/osascript salt '*' assistive.install com.smileonmymac.textexpander """ - tries = 1 + num_tries = 1 while True: with TccDB() as db: try: return db.install(app_id, enable=enable) except sqlite3.Error as exc: - if tries <= 2: - time.sleep(10) - tries += 1 + if "attempt to write a readonly database" not in str(exc): + raise CommandExecutionError( + "Error installing app({}): {}".format(app_id, exc) + ) + elif num_tries < tries: + time.sleep(wait) + num_tries += 1 else: raise CommandExecutionError( "Error installing app({}): {}".format(app_id, exc)