From 3fba32e54ecaa9b3b7b8797480519c2f336b8cfc Mon Sep 17 00:00:00 2001 From: roekatz Date: Wed, 26 Jun 2024 17:49:34 +0300 Subject: [PATCH 1/5] BasePolicyWatcherTask: Signal stop if broadcaster fails to connect --- .../opal-server/opal_server/policy/watcher/task.py | 11 ++++++----- packages/requires.txt | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/opal-server/opal_server/policy/watcher/task.py b/packages/opal-server/opal_server/policy/watcher/task.py index a2ba57558..e2b630eb5 100644 --- a/packages/opal-server/opal_server/policy/watcher/task.py +++ b/packages/opal-server/opal_server/policy/watcher/task.py @@ -50,11 +50,12 @@ async def _subscribe_internal(): ) if self._pubsub_endpoint.broadcaster is not None: - async with self._pubsub_endpoint.broadcaster.get_listening_context(): - await _subscribe_internal() - await self._pubsub_endpoint.broadcaster.get_reader_task() - - # Stop the watcher if broadcaster disconnects + try: + async with self._pubsub_endpoint.broadcaster.get_listening_context(): + await _subscribe_internal() + await self._pubsub_endpoint.broadcaster.get_reader_task() + finally: + # Stop the watcher if broadcaster disconnects / fails to connect self.signal_stop() else: # If no broadcaster is configured, just subscribe, no need to wait on anything diff --git a/packages/requires.txt b/packages/requires.txt index 5096c6000..e077c4bf3 100644 --- a/packages/requires.txt +++ b/packages/requires.txt @@ -1,7 +1,7 @@ idna>=3.3,<4 typer>=0.4.1,<1 fastapi>=0.109.1,<1 -fastapi_websocket_pubsub==0.3.7 +fastapi_websocket_pubsub==0.3.9 fastapi_websocket_rpc>=0.1.21,<1 gunicorn>=22.0.0,<23 pydantic[email]>=1.9.1,<2 From 73ea4d5f0b95e0c5af27c280824e9ec1ebc83341 Mon Sep 17 00:00:00 2001 From: roekatz Date: Wed, 26 Jun 2024 17:50:31 +0300 Subject: [PATCH 2/5] Random documentation fixes --- packages/opal-common/opal_common/async_utils.py | 9 ++++++--- packages/opal-server/opal_server/git_fetcher.py | 9 +-------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/packages/opal-common/opal_common/async_utils.py b/packages/opal-common/opal_common/async_utils.py index e7f6029fd..a2df90c69 100644 --- a/packages/opal-common/opal_common/async_utils.py +++ b/packages/opal-common/opal_common/async_utils.py @@ -22,9 +22,12 @@ async def run_sync( """Shorthand for running a sync function in an executor within an async context. - For example: def sync_function_that_takes_time_to_run(arg1, - arg2): time.sleep(5) async def async_function(): - await run_sync(sync_function_that_takes_time_to_run, 1, arg2=5) + For example: + def sync_function_that_takes_time_to_run(arg1, arg2): + time.sleep(5) + + async def async_function(): + await run_sync(sync_function_that_takes_time_to_run, 1, arg2=5) """ return await asyncio.get_event_loop().run_in_executor( None, partial(func, *args, **kwargs) diff --git a/packages/opal-server/opal_server/git_fetcher.py b/packages/opal-server/opal_server/git_fetcher.py index 0887c9160..5ea85c047 100644 --- a/packages/opal-server/opal_server/git_fetcher.py +++ b/packages/opal-server/opal_server/git_fetcher.py @@ -139,14 +139,7 @@ def __init__( ) async def _get_repo_lock(self): - # # This implementation works across multiple processes/threads, but is not fair (next acquiree is random) - # locks_dir = self._base_dir / ".locks" - # await aiofiles.os.makedirs(str(locks_dir), exist_ok=True) - - # return NamedLock( - # locks_dir / GitPolicyFetcher.source_id(self._source), attempt_interval=0.1 - # ) - + # Previous file based implementation worked across multiple processes/threads, but wasn't fair (next acquiree is random) # This implementation works only within the same process/thread, but is fair (next acquiree is the earliest to enter the lock) src_id = GitPolicyFetcher.source_id(self._source) lock = GitPolicyFetcher.repo_locks[src_id] = GitPolicyFetcher.repo_locks.get( From 36e6dc1a80c17460558e797c24250c37a327cbbe Mon Sep 17 00:00:00 2001 From: roekatz Date: Thu, 27 Jun 2024 16:20:40 +0300 Subject: [PATCH 3/5] Fix tests to explicitly choose 'master' as default branch The tests rely on that, and this value also depends on local git configuration --- packages/opal-common/opal_common/git/tests/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/opal-common/opal_common/git/tests/conftest.py b/packages/opal-common/opal_common/git/tests/conftest.py index c60099a5c..e3e5e0e14 100644 --- a/packages/opal-common/opal_common/git/tests/conftest.py +++ b/packages/opal-common/opal_common/git/tests/conftest.py @@ -98,7 +98,7 @@ def local_repo(tmp_path, helpers: Helpers) -> Repo: """ root: Path = tmp_path / "myrepo" root.mkdir() - repo = Repo.init(root) + repo = Repo.init(root, initial_branch="master") # create file to delete later helpers.create_new_file_commit(repo, root / "deleted.rego") From 03c011fabeae7e28f11965657e23f51cccbd1f0b Mon Sep 17 00:00:00 2001 From: roekatz Date: Thu, 27 Jun 2024 16:58:58 +0300 Subject: [PATCH 4/5] Tests: Change test server ports to avoid collisions --- .../opal_client/tests/server_to_client_intergation_test.py | 2 +- .../opal-server/opal_server/tests/policy_repo_webhook_test.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/opal-client/opal_client/tests/server_to_client_intergation_test.py b/packages/opal-client/opal_client/tests/server_to_client_intergation_test.py index 3e6522277..a3372c56f 100644 --- a/packages/opal-client/opal_client/tests/server_to_client_intergation_test.py +++ b/packages/opal-client/opal_client/tests/server_to_client_intergation_test.py @@ -31,7 +31,7 @@ from opal_server.server import OpalServer # Server settings -PORT = int(os.environ.get("PORT") or "9123") +PORT = int(os.environ.get("PORT") or "9124") UPDATES_URL = f"ws://localhost:{PORT}/ws" DATA_ROUTE = "/fetchable_data" DATA_URL = f"http://localhost:{PORT}{DATA_ROUTE}" diff --git a/packages/opal-server/opal_server/tests/policy_repo_webhook_test.py b/packages/opal-server/opal_server/tests/policy_repo_webhook_test.py index a6633f984..8c8fd0bb7 100644 --- a/packages/opal-server/opal_server/tests/policy_repo_webhook_test.py +++ b/packages/opal-server/opal_server/tests/policy_repo_webhook_test.py @@ -29,7 +29,7 @@ from opal_common.utils import get_authorization_header from opal_server.config import PolicySourceTypes, opal_server_config -PORT = int(os.environ.get("PORT") or "9123") +PORT = int(os.environ.get("PORT") or "9125") # Basic server route config WEBHOOK_ROUTE = "/webhook" From 6d8063053f8ac07195caf74693a2129906586bbb Mon Sep 17 00:00:00 2001 From: roekatz Date: Thu, 27 Jun 2024 18:05:26 +0300 Subject: [PATCH 5/5] Dokcer test: No need to build test image for client cedar since we don't test it --- .github/workflows/tests.yml | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index c044a2aa7..610cdd05d 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -79,19 +79,6 @@ jobs: tags: | permitio/opal-client:test - - name: Build client cedar - id: build_client_cedar - uses: docker/build-push-action@v2 - with: - file: docker/Dockerfile - push: false - target: client-cedar - cache-from: type=registry,ref=permitio/opal-client-cedar:latest - cache-to: type=inline - load: true - tags: | - permitio/opal-client-cedar:test - - name: Build server id: build_server uses: docker/build-push-action@v2