Test

.github/workflows/pre-commit.yml

@@ -13,4 +13,13 @@ jobs:
     - uses: actions/setup-python@v4
       with:
         python-version: 3.x
-    - uses: pre-commit/[email protected]
+    - name: install pre-commit
+      run: python -m pip install 'pre-commit<4'
+    - name: show environment
+      run: python -m pip freeze --local
+    - uses: actions/cache@v4
+      with:
+        path: ~/.cache/pre-commit
+        key: pre-commit-${{ hashFiles('.pre-commit-config.yaml') }}
+    - name: run pre-commit
+      run: pre-commit run --show-diff-on-failure --color=always --all-files

.github/workflows/tests.yml

@@ -116,13 +116,43 @@ jobs:
           OPAL_IMAGE_TAG: test
           OPAL_TESTS_POLICY_REPO_DEPLOY_KEY: ${{ secrets.OPAL_TESTS_POLICY_REPO_DEPLOY_KEY }}
         run: |
-          # Prepare git for using tests policy repo
+          # Validate secret is available
+          if [ -z "$OPAL_TESTS_POLICY_REPO_DEPLOY_KEY" ]; then
+            echo "Error: OPAL_TESTS_POLICY_REPO_DEPLOY_KEY secret is not set"
+            exit 1
+          fi
+
+          # Print secret length (safely)
+          echo "Deploy key length: $(echo "$OPAL_TESTS_POLICY_REPO_DEPLOY_KEY" | wc -c) bytes"
+
+          # Check if secret looks like a valid SSH key
+          if ! echo "$OPAL_TESTS_POLICY_REPO_DEPLOY_KEY" | grep -q "BEGIN.*PRIVATE KEY"; then
+            echo "Error: Deploy key does not look like a valid SSH private key"
+            exit 1
+          fi
+
+          # Clear any existing SSH setup
+          rm -rf ~/.ssh
+          mkdir -p ~/.ssh
+          chmod 700 ~/.ssh
+
+          # Write deploy key
           export OPAL_POLICY_REPO_SSH_KEY_PATH=$(realpath ./opal-tests-policy-repo-key)
-          echo "$OPAL_TESTS_POLICY_REPO_DEPLOY_KEY" > $OPAL_POLICY_REPO_SSH_KEY_PATH
-          chmod 400 $OPAL_POLICY_REPO_SSH_KEY_PATH
+          # Use printf to ensure newlines are preserved
+          printf "%s\n" "$OPAL_TESTS_POLICY_REPO_DEPLOY_KEY" > $OPAL_POLICY_REPO_SSH_KEY_PATH
+          chmod 600 $OPAL_POLICY_REPO_SSH_KEY_PATH
 
-          git config --global core.sshCommand "ssh -i $OPAL_POLICY_REPO_SSH_KEY_PATH -o IdentitiesOnly=yes"
-          git config --global user.name "$GITHUB_ACTOR"
-          git config --global user.email "<>"
+          # Verify key content
+          if ! ssh-keygen -l -f $OPAL_POLICY_REPO_SSH_KEY_PATH &>/dev/null; then
+            echo "Error: Invalid SSH key format"
+            exit 1
+          fi
 
-          ./run.sh
+          # Setup known hosts
+          ssh-keyscan github.com >> ~/.ssh/known_hosts
+          chmod 644 ~/.ssh/known_hosts
+
+          # Configure SSH
+          ssh-agent bash -c "ssh-add $OPAL_POLICY_REPO_SSH_KEY_PATH && git clone [email protected]:permitio/opal-tests-policy-repo.git"
+
+          ./run.sh

.pre-commit-config.yaml

@@ -18,8 +18,8 @@ repos:
     hooks:
       - id: codespell
         args: [--skip, "*.json"]
-  - repo: https://github.com/myint/docformatter
-    rev: v1.5.1
+  - repo: https://github.com/PyCQA/docformatter
+    rev: v1.7.5
     hooks:
       - id: docformatter
         args: [--in-place]

packages/__packaging__.py

@@ -22,11 +22,13 @@
 def get_install_requires(here):
     """Gets the contents of install_requires from text file.
 
-    Getting the minimum requirements from a text file allows us to pre-install
-    them in docker, speeding up our docker builds and better utilizing the docker layer cache.
+    Getting the minimum requirements from a text file allows us to pre-
+    install them in docker, speeding up our docker builds and better
+    utilizing the docker layer cache.
 
-    The requirements in requires.txt are in fact the minimum set of packages
-    you need to run OPAL (and are thus different from a "requirements.txt" file).
+    The requirements in requires.txt are in fact the minimum set of
+    packages you need to run OPAL (and are thus different from a
+    "requirements.txt" file).
     """
     with open(os.path.join(here, "requires.txt")) as fp:
         return [

packages/opal-client/opal_client/callbacks/api.py

@@ -30,12 +30,12 @@ async def require_listener_token(claims: JWTClaims = Depends(authenticator)):
 
     @router.get("", response_model=List[CallbackEntry])
     async def list_callbacks():
-        """list all the callbacks currently registered by OPAL client."""
+        """List all the callbacks currently registered by OPAL client."""
         return list(register.all())
 
     @router.get("/{key}", response_model=CallbackEntry)
     async def get_callback_by_key(key: str):
-        """get a callback by its key (if such callback is indeed
+        """Get a callback by its key (if such callback is indeed
         registered)."""
         callback = register.get(key)
         if callback is None:
@@ -47,7 +47,7 @@ async def get_callback_by_key(key: str):
 
     @router.post("", response_model=CallbackEntry)
     async def register_callback(entry: CallbackEntry):
-        """register a new callback by OPAL client, to be called on OPA state
+        """Register a new callback by OPAL client, to be called on OPA state
         updates."""
         saved_key = register.put(url=entry.url, config=entry.config, key=entry.key)
         saved_entry = register.get(saved_key)
@@ -60,7 +60,7 @@ async def register_callback(entry: CallbackEntry):
 
     @router.delete("/{key}", status_code=status.HTTP_204_NO_CONTENT)
     async def get_callback_by_key(key: str):
-        """unregisters a callback identified by its key (if such callback is
+        """Unregisters a callback identified by its key (if such callback is
         indeed registered)."""
         callback = register.get(key)
         if callback is None:

packages/opal-client/opal_client/callbacks/register.py

@@ -58,14 +58,14 @@ def _register(self, key: str, url: str, config: HttpFetcherConfig):
         self._callbacks[key] = (url, config)
 
     def calc_hash(self, url: str, config: HttpFetcherConfig) -> str:
-        """gets a unique hash key from a callback url and config."""
+        """Gets a unique hash key from a callback url and config."""
         m = hashlib.sha256()
         m.update(url.encode())
         m.update(config.json().encode())
         return m.hexdigest()
 
     def get(self, key: str) -> Optional[CallbackEntry]:
-        """gets a registered callback by its key, or None if no such key found
+        """Gets a registered callback by its key, or None if no such key found
         in register."""
         callback = self._callbacks.get(key, None)
         if callback is None:
@@ -79,7 +79,7 @@ def put(
         config: Optional[HttpFetcherConfig] = None,
         key: Optional[str] = None,
     ) -> str:
-        """puts a callback in the register.
+        """Puts a callback in the register.
 
         if no config is provided, the default callback config will be
         used. if no key is provided, the key will be calculated by
@@ -100,12 +100,12 @@ def put(
         return callback_key
 
     def remove(self, key: str):
-        """removes a callback from the register, if exists."""
+        """Removes a callback from the register, if exists."""
         if key in self._callbacks:
             del self._callbacks[key]
 
     def all(self) -> Generator[CallbackEntry, None, None]:
-        """a generator yielding all the callback configs currently registered.
+        """A generator yielding all the callback configs currently registered.
 
         Yields:
             the next callback config found

packages/opal-client/opal_client/callbacks/reporter.py

@@ -13,7 +13,7 @@
 
 
 class CallbacksReporter:
-    """can send a report to callbacks registered on the callback register."""
+    """Can send a report to callbacks registered on the callback register."""
 
     def __init__(
         self, register: CallbacksRegister, data_fetcher: DataFetcher = None

packages/opal-client/opal_client/client.py

@@ -233,7 +233,7 @@ def _init_engine_runner(
         return False
 
     def _init_fast_api_app(self):
-        """inits the fastapi app object."""
+        """Inits the fastapi app object."""
         app = FastAPI(
             title="OPAL Client",
             description="OPAL is an administration layer for Open Policy Agent (OPA), detecting changes"
@@ -248,7 +248,7 @@ def _init_fast_api_app(self):
         return app
 
     def _configure_api_routes(self, app: FastAPI):
-        """mounts the api routes on the app object."""
+        """Mounts the api routes on the app object."""
 
         authenticator = JWTAuthenticator(self.verifier)
 
@@ -269,7 +269,7 @@ def _configure_api_routes(self, app: FastAPI):
         @app.get("/", include_in_schema=False)
         @app.get("/healthy", include_in_schema=False)
         async def healthy():
-            """returns 200 if updates keep being successfully fetched from the
+            """Returns 200 if updates keep being successfully fetched from the
             server and applied to the policy store."""
             healthy = await self.policy_store.is_healthy()
 
@@ -285,7 +285,7 @@ async def healthy():
 
         @app.get("/ready", include_in_schema=False)
         async def ready():
-            """returns 200 if the policy store is ready to serve requests."""
+            """Returns 200 if the policy store is ready to serve requests."""
             ready = self._backup_loaded or await self.policy_store.is_ready()
 
             if ready:
@@ -301,7 +301,7 @@ async def ready():
         return app
 
     def _configure_lifecycle_callbacks(self, app: FastAPI):
-        """registers callbacks on app startup and shutdown.
+        """Registers callbacks on app startup and shutdown.
 
         on app startup we launch our long running processes (async
         tasks) on the event loop. on app shutdown we stop these long
@@ -352,7 +352,7 @@ async def start_client_background_tasks(self):
         )
 
     async def stop_client_background_tasks(self):
-        """stops all background tasks (called on shutdown event)"""
+        """Stops all background tasks (called on shutdown event)"""
         logger.info("stopping background tasks...")
 
         # stopping opa runner
@@ -483,7 +483,7 @@ async def maybe_init_healthcheck_policy(self):
             raise
 
     def _trigger_shutdown(self):
-        """this will send SIGTERM (Keyboard interrupt) to the worker, making
+        """This will send SIGTERM (Keyboard interrupt) to the worker, making
         uvicorn send "lifespan.shutdown" event to Starlette via the ASGI
         lifespan interface.
 

packages/opal-client/opal_client/data/fetcher.py

@@ -12,7 +12,7 @@
 
 
 class DataFetcher:
-    """fetches policy data from backend."""
+    """Fetches policy data from backend."""
 
     def __init__(self, default_data_url: str = None, token: str = None):
         """

packages/opal-client/opal_client/data/rpc.py

@@ -3,7 +3,7 @@
 
 
 class TenantAwareRpcEventClientMethods(RpcEventClientMethods):
-    """use this methods class when the server uses
+    """Use this methods class when the server uses
     `TenantAwareRpcEventServerMethods`."""
 
     TOPIC_SEPARATOR = "::"

packages/opal-client/opal_client/data/updater.py

@@ -360,7 +360,7 @@ async def _update_policy_data(
         update: DataUpdate,
         store_queue_number: TakeANumberQueue.Number,
     ):
-        """fetches policy data (policy configuration) from backend and updates
+        """Fetches policy data (policy configuration) from backend and updates
         it into policy-store (i.e. OPA)"""
 
         if update is None:

packages/opal-client/opal_client/engine/options.py

@@ -69,16 +69,16 @@ class Config:
 
         @classmethod
         def alias_generator(cls, string: str) -> str:
-            """converts field named tls_private_key_file to --tls-private-key-
+            """Converts field named tls_private_key_file to --tls-private-key-
             file (to be used by opa cli)"""
             return "--{}".format(string.replace("_", "-"))
 
     def get_cli_options_dict(self):
-        """returns a dict that can be passed to the OPA cli."""
+        """Returns a dict that can be passed to the OPA cli."""
         return self.dict(exclude_none=True, by_alias=True, exclude={"files"})
 
     def get_opa_startup_files(self) -> str:
-        """returns a list of startup policies and data."""
+        """Returns a list of startup policies and data."""
         files = self.files if self.files is not None else []
         return " ".join(files)
 
@@ -110,7 +110,7 @@ class Config:
 
         @classmethod
         def alias_generator(cls, string: str) -> str:
-            """converts field named tls_private_key_file to --tls-private-key-
+            """Converts field named tls_private_key_file to --tls-private-key-
             file (to be used by opa cli)"""
             return "--{}".format(string.replace("_", "-"))
 

packages/opal-client/opal_client/engine/runner.py

@@ -20,7 +20,7 @@ async def wait_until_process_is_up(
     wait_interval: float = 0.1,
     timeout: Optional[float] = None,
 ):
-    """waits until the pid of the process exists, then optionally runs a
+    """Waits until the pid of the process exists, then optionally runs a
     callback.
 
     optionally receives a timeout to give up.
@@ -68,12 +68,12 @@ async def __aexit__(self, exc_type, exc, tb):
         await self.stop()
 
     def start(self):
-        """starts the runner task, and launches the OPA subprocess."""
+        """Starts the runner task, and launches the OPA subprocess."""
         logger.info("Launching engine runner")
         self._run_task = asyncio.create_task(self._run())
 
     async def stop(self):
-        """stops the runner task (and terminates OPA)"""
+        """Stops the runner task (and terminates OPA)"""
         self._init_events()
         if not self._should_stop.is_set():
             logger.info("Stopping policy engine runner")
@@ -86,7 +86,7 @@ async def stop(self):
         self._run_task = None
 
     async def wait_until_done(self):
-        """waits until the engine runner task is complete.
+        """Waits until the engine runner task is complete.
 
         this is great when using engine runner as a context manager.
         """
@@ -108,7 +108,7 @@ async def _run(self):
                 break
 
     async def pipe_logs(self):
-        """gets a stream of logs from the opa process, and logs it into the
+        """Gets a stream of logs from the opa process, and logs it into the
         main opal log."""
         self._engine_panicked = False
 
@@ -146,7 +146,7 @@ async def _pipe_logs_stream(stream: asyncio.StreamReader):
             logger.error("restart policy engine due to a detected panic")
 
     async def handle_log_line(self, line: bytes) -> bool:
-        """handles a single line of log from the engine process.
+        """Handles a single line of log from the engine process.
 
         returns True if the engine panicked.
         """
@@ -194,11 +194,11 @@ async def _run_process_until_terminated(self) -> int:
         return return_code
 
     def register_process_initial_start_callbacks(self, callbacks: List[AsyncCallback]):
-        """register a callback to run when OPA is started the first time."""
+        """Register a callback to run when OPA is started the first time."""
         self._on_process_initial_start_callbacks.extend(callbacks)
 
     def register_process_restart_callbacks(self, callbacks: List[AsyncCallback]):
-        """register a callback to run when OPA is restarted (i.e: OPA was
+        """Register a callback to run when OPA is restarted (i.e: OPA was
         already up, then got terminated, and now is up again).
 
         this is most often used to keep OPA's cache (policy and data)
@@ -209,7 +209,7 @@ def register_process_restart_callbacks(self, callbacks: List[AsyncCallback]):
         self._on_process_restart_callbacks.extend(callbacks)
 
     async def _run_start_callbacks(self):
-        """runs callbacks after OPA process starts."""
+        """Runs callbacks after OPA process starts."""
         # TODO: make policy store expose the /health api of OPA
         await asyncio.sleep(1)
 
@@ -261,7 +261,7 @@ def setup_opa_runner(
         initial_start_callbacks: Optional[List[AsyncCallback]] = None,
         rehydration_callbacks: Optional[List[AsyncCallback]] = None,
     ):
-        """factory for OpaRunner, accept optional callbacks to run in certain
+        """Factory for OpaRunner, accept optional callbacks to run in certain
         lifecycle events.
 
         Initial Start Callbacks:

packages/opal-client/opal_client/policy/fetcher.py

@@ -26,7 +26,7 @@ def force_valid_bundle(bundle) -> PolicyBundle:
 
 
 class PolicyFetcher:
-    """fetches policy from backend."""
+    """Fetches policy from backend."""
 
     def __init__(self, backend_url=None, token=None):
         """

packages/opal-client/opal_client/policy/topics.py

@@ -6,7 +6,7 @@
 
 
 def default_subscribed_policy_directories() -> List[str]:
-    """wraps the configured value of POLICY_SUBSCRIPTION_DIRS, but dedups
+    """Wraps the configured value of POLICY_SUBSCRIPTION_DIRS, but dedups
     intersecting dirs."""
     subscription_directories = [
         Path(d) for d in opal_client_config.POLICY_SUBSCRIPTION_DIRS