[OND211-2418] Merge feature branch to master (2FA, Python 3.12-slim, portable Makefiles)#24
Merged
Merged
Conversation
…e gate on the Keycloak auth surface Bundles the hand-written D18 client-side Keycloak/offline-token auth helper (utils/keycloak.py + client_config Keycloak fields) with comprehensive hermetic unit tests (mocked token endpoint, no network) and a CI workflow (.github/workflows/tests.yml, python 3.12) enforcing >=90% line coverage on that hand-written surface (generated *_pb2 stubs excluded). Locally verified in a clean python-3.12 venv: tests pass, hand-written coverage >=98%.
Close remaining gaps on the Keycloak/offline-token auth surface (targeted tests; pragma genuinely-unreachable defensive guards) and bump --cov-fail-under to 100. Verified in a clean python-3.12 venv: 100% coverage.
…s unit tests Complete typing was already in place (mypy disallow_untyped_defs); this adds Args/Returns/Raises docstrings to the remaining private helpers in keycloak.py and to every test function, fixture helper and fake-transport class in the hand-written D18 auth-surface test suite. Docs-only: no logic changes, the 100% coverage gate and mypy stay green.
… provider Re-sync the headless Keycloak offline-token provider to the nlu reference: add a background daemon thread that proactively refreshes the access token leeway- before-expiry (mirroring the nodejs/js/ts/angular SDKs), with a weakref-to-self loop target so the provider can still be garbage-collected, stop()/close()/__del__ and context-manager teardown, and the lazy read-time refresh kept as a fallback. Re-port the matching hermetic unit tests. Behavior and tests stay byte-for-byte identical to nlu modulo the family import path.
The helper functions the main demo %run-includes (s2t_ex_1/2, live_speech_helper, say/synthesis_request, find_pipeline_by_language_s2t/t2s) called the high-level s2t/t2s wrappers, which do not forward the bearer. Route each through the low-level stub with s2t_provider/t2s_provider.bearer_metadata() when Keycloak is on, falling back to the wrapper otherwise. NLU calls (auto-attach) are unchanged.
grpc-python rejects a capital 'Authorization' metadata key at call time (ValueError: metadata was invalid / Illegal header key), so the bearer never reached a real server. Use the lowercase wire key, matching the nlu client and the HTTP/2 requirement. The capital form was introduced by the earlier 'canonical Authorization header key' change and is only correct for the browser/gRPC-web clients (Envoy transcodes there).
… the bearer The s2t/t2s service wrappers now thread the Keycloak bearer on every call, so the demo and its helper no longer need the low-level stub + provider.bearer_metadata() guards. Drop the shared token-provider setup and call the high-level client.services.<svc>.<rpc>() wrappers directly (NLU already auto-attached).
- requirements.txt: ondewo-logging -> loguru (unpinned) - examples and tutorial notebook: import loguru logger as logger_console - mypy.ini: point ignore_missing_imports at loguru instead of ondewo.logging Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… token TLS verification
…ed refresh test does not hang
…c service wrappers (make-build regen regression)
- Pin ONDEWO_PROTO_COMPILER_GIT_BRANCH to tags/5.10.0 in Makefile - Bump ondewo-proto-compiler submodule to tag 5.10.0 - pre-commit autoupdate: mypy v1.15.0->v2.1.0, black 25.1.0->26.5.1, pre-commit-hooks v5.0.0->v6.0.0, flake8 7.1.2->7.3.0, giticket v1.4->1.92 Verified python stub generation with the 5.10.0 compiler image and ran the unit test suite (46 passed).
…r production - Merge origin/master into feature branch (clean, ort strategy) - Keep ONDEWO_PROTO_COMPILER_GIT_BRANCH=tags/5.10.0 - Bump ondewo-csi-api submodule to feature-branch tip (5.2.0-15); csi stubs regenerated with 5.10.0 python proto-compiler are byte-identical - Tests: 54 passed, 100% coverage on hand-written auth/config surface
…ux+macOS) portability
There was a problem hiding this comment.
Pull request overview
This PR merges the OND211-2418 feature work into master, primarily standardizing authentication across the CSI client and examples on Keycloak bearer (offline-token) login, updating build/dev tooling for portability, and adding hermetic unit tests + CI gating for the new auth/config surface.
Changes:
- Introduces a Keycloak headless offline-token (
offline_access) bearer-token provider with refresh logic, and wires services to attachAuthorization: Bearer ...metadata. - Updates examples/tutorial notebooks and README to reflect bearer-only auth and environment-driven configuration.
- Adds a unit-test suite (plus GitHub Actions workflow) that hermetically validates Keycloak token acquisition/refresh and verifies metadata propagation, with 100% coverage gating on hand-written modules.
Reviewed changes
Copilot reviewed 38 out of 43 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tutorials/Jupyter_demo.ipynb | Updates tutorial to describe Keycloak bearer auth and new config fields. |
| tutorials/Jupyter_demo_helper.ipynb | Switches logging to loguru + minor cleanup. |
| tests/unit/examples/test_examples.py | Adds hermetic tests ensuring examples compile and keycloak_auth_example behavior is correct. |
| tests/unit/examples/init.py | Adds package marker for unit example tests. |
| tests/unit/client/test_keycloak.py | Adds comprehensive hermetic unit tests for Keycloak token provider + refresh loop. |
| tests/unit/client/test_conversations_metadata.py | Verifies bearer metadata is attached on outgoing stub calls (sync + async). |
| tests/unit/client/test_client_config.py | Adds tests for new ClientConfig Keycloak fields + legacy compatibility expectations. |
| tests/unit/client/init.py | Adds package marker for unit client tests. |
| tests/unit/init.py | Adds package marker for unit tests. |
| tests/init.py | Adds package marker for tests. |
| requirements.txt | Updates runtime dependencies (loguru, requests, ondewo client libs, etc.). |
| requirements-dev.txt | Adds dev dependency python-dotenv. |
| README.md | Updates examples/auth documentation to reflect bearer-only Keycloak flow and config fields. |
| ondewo/csi/conversation_pb2.pyi | Docstring formatting/escaping updates in generated typing stubs. |
| ondewo/csi/conversation_pb2_grpc.py | Docstring formatting updates in generated gRPC code. |
| ondewo/csi/client/utils/keycloak.py | Adds Keycloak offline-token provider + shared-provider factory. |
| ondewo/csi/client/utils/init.py | Adds utils package marker. |
| ondewo/csi/client/services/conversations.py | Wires metadata into every Conversations RPC (sync). |
| ondewo/csi/client/services/async_conversations.py | Wires metadata into every Conversations RPC (async). |
| ondewo/csi/client/core/services_interface.py | Introduces sync ServicesInterface with Keycloak metadata property. |
| ondewo/csi/client/core/async_services_interface.py | Introduces async ServicesInterface with Keycloak metadata property. |
| ondewo/csi/client/core/init.py | Adds core package marker. |
| ondewo/csi/client/client.py | Fixes client docstring text for CSI + removes legacy “login” wording. |
| ondewo/csi/client/client_config.py | Extends ClientConfig with Keycloak fields, validation, and derived properties. |
| ondewo/csi/client/async_client.py | Fixes async client docstring text for CSI + removes legacy “login” wording. |
| mypy.ini | Updates mypy ignore section to match loguru imports. |
| Makefile | Replaces sed usage with portable perl; updates submodule branch/tag refs. |
| examples/streamer.py | Switches example logging to loguru. |
| examples/speech2speech_with_hangup_example.py | Moves example config to dotenv/env vars; adds error handling and logging. |
| examples/speech2speech_example.py | Moves example config to dotenv/env vars; adds error handling and logging. |
| examples/s2s_streaming_example.py | Moves config to dotenv/env vars; improves paths and error handling. |
| examples/s2s_pipelines_example.py | Moves config to dotenv/env vars; adds logging/error handling and retry options. |
| examples/multi_client_example.py | Moves config to dotenv/env vars; adds error handling and logging. |
| examples/keycloak_auth_example.py | Adds a minimal Keycloak bearer-auth example driven by environment variables. |
| examples/healthcheck_example.py | Moves config to dotenv/env vars; adds error handling and logging. |
| examples/environment.env | Adds a non-secret template env file for running examples consistently. |
| examples/convert_files.py | Moves config to dotenv/env vars; wraps execution in __main__ with logging. |
| examples/configs/keycloak_grpc_placeholder.json | Adds a placeholder JSON config for Keycloak bearer usage. |
| Dockerfile.utils | Updates utils image base to python:3.12-slim. |
| CLAUDE.md | Adds repository guidance doc (tooling/logging/docstring conventions). |
| .pre-commit-config.yaml | Updates hook revisions; adjusts mypy/black/pre-commit hooks configuration. |
| .github/workflows/tests.yml | Adds CI job running unit tests with coverage gating for hand-written code. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+552
to
+555
| key: int = id(config) | ||
| with _PROVIDER_REGISTRY_LOCK: | ||
| provider: Optional[KeycloakTokenProvider] = _PROVIDER_REGISTRY.get(key) | ||
| if provider is None: |
Comment on lines
+494
to
+499
| if response.status_code < 200 or response.status_code >= 300: | ||
| raise KeycloakAuthenticationError( | ||
| f'Keycloak token {action} failed with status {response.status_code}: {response.text}' | ||
| ) | ||
| body: Dict[str, Any] = response.json() | ||
| return body |
Comment on lines
120
to
+126
| pipelines: List[S2sPipeline] = list(list_response.pipelines) | ||
| log.info(f"Received {len(pipelines)} S2S pipeline(s).") | ||
| print(pipelines) | ||
|
|
||
| # get the S2S pipeline | ||
| print(conversations_service.get_s2s_pipeline(S2sPipelineId(id=pipelines[0].id))) | ||
| log.info("DONE: s2s_pipelines_example: main") |
Comment on lines
+154
to
+168
| def _build_provider( | ||
| transport: FakeTransport, | ||
| token_expiration_in_s: int | None = None, | ||
| ) -> KeycloakTokenProvider: | ||
| """Construct a `KeycloakTokenProvider` wired to the fake transport and shared test fixtures. | ||
|
|
||
| The proactive background-refresh thread is disabled (`start_background_refresh=False`) so | ||
| the lazy read-path assertions on exact `transport.calls` counts stay deterministic; the | ||
| background loop is covered explicitly in `TestBackgroundRefresh` by driving `_refresh_loop`. | ||
|
|
||
| Args: | ||
| transport (FakeTransport): | ||
| The fake token endpoint backing the provider. | ||
| token_expiration_in_s (int | None): | ||
| Optional upper bound on auto-refresh; `None` keeps refreshing unbounded. |
Comment on lines
+1043
to
+1062
| def _build_background_provider( | ||
| transport: FakeTransport, | ||
| clock: Dict[str, float], | ||
| stop_event: ScriptedEvent, | ||
| token_expiration_in_s: int | None = None, | ||
| ) -> KeycloakTokenProvider: | ||
| """Construct a provider wired to an injected clock + scripted event, background thread off. | ||
|
|
||
| The background thread is not started here; tests call `_refresh_loop` directly with the | ||
| scripted event so the loop body runs deterministically without a real thread or sleeps. | ||
|
|
||
| Args: | ||
| transport (FakeTransport): | ||
| The fake token endpoint backing the provider. | ||
| clock (Dict[str, float]): | ||
| The shared mutable monotonic clock (`{'now': <seconds>}`). | ||
| stop_event (ScriptedEvent): | ||
| The scripted event driving `wait()`/`is_set()` in the loop. | ||
| token_expiration_in_s (int | None): | ||
| Optional upper bound on auto-refresh; `None` keeps refreshing unbounded. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Merges OND211-2418 to master. Includes Keycloak 2FA work, Dockerfile.utils on python:3.12-slim, and all Makefile sed replaced with portable perl (Linux GNU + macOS BSD identical; reviewed: escaping via make -n, byte-identical differential tests, make parses, no BSD-hostile constructs).