feat: add tls and api key authentication support#1
Merged
vieiralucas merged 7 commits intomainfrom Mar 21, 2026
Merged
Conversation
add ca_cert, client_cert, client_key, and api_key parameters to both Client and AsyncClient. when ca_cert is provided, uses grpc secure channel with ssl credentials. when api_key is set, injects authorization bearer metadata via grpc interceptors. update admin.proto with api key and acl management messages. add integration tests for tls and api key auth. fully backward compatible — no auth/tls by default.
There was a problem hiding this comment.
3 issues found across 9 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="fila/client.py">
<violation number="1" location="fila/client.py:112">
P1: `client_cert`/`client_key` are silently ignored when `ca_cert` is not provided, falling through to an insecure channel. A user who forgets `ca_cert` gets a plaintext connection with no indication that their mTLS credentials were discarded. Raise a `ValueError` when client credentials are supplied without a CA certificate.</violation>
</file>
<file name="tests/conftest.py">
<violation number="1" location="tests/conftest.py:257">
P2: Close the gRPC channel in the retry loop even when `ListQueues` fails; currently the error path leaks channels during readiness polling.</violation>
</file>
<file name="fila/async_client.py">
<violation number="1" location="fila/async_client.py:44">
P2: Missing `compression` field when reconstructing `ClientCallDetails`. The sync interceptor in `client.py` correctly forwards `client_call_details.compression`, but the async interceptor drops it. This silently resets per-RPC compression settings to `None` for any call passing through the interceptor.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
- fix ClientCallDetails instantiation by using concrete subclass (grpc.ClientCallDetails is abstract and cannot be constructed directly) - fix mypy errors: use type: ignore[misc] for grpc class subclassing - add compression field to async interceptor ClientCallDetails (was dropped) - add ValueError when client_cert/client_key provided without ca_cert - close gRPC channels on error path in fixture retry loops
- use getattr for compression field on async ClientCallDetails (not all grpc versions expose it) - remove unused type: ignore[override] comments flagged by mypy - make test_missing_api_key_rejected resilient to server binaries that predate the bootstrap_apikey feature
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="tests/test_client.py">
<violation number="1" location="tests/test_client.py:217">
P2: Do not skip the test for unexpected RPC errors; this masks real failures. Only skip for the specific non-enforcement case and fail otherwise.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
grpc.aio.ClientCallDetails is a namedtuple (method, timeout, metadata, credentials, wait_for_ready) — no compression field. Override __new__ to pass exactly 5 args to the namedtuple constructor.
Replace unused type: ignore[call-arg] with type: ignore[no-any-return] on _AsyncClientCallDetails.__new__ to fix mypy lint failure in CI.
Cubic P2: non-UNAUTHENTICATED RPC errors during the auth probe should pytest.fail() rather than pytest.skip(), so real failures are not masked.
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.
Summary
Add TLS and API key authentication support to the Python SDK, achieving feature parity with the Rust SDK (Epic 15).
ca_cert,client_cert,client_keyparams for TLS/mTLS on both sync and async clientsapi_keyparam for Bearer token auth on every RPCgrpc.intercept_channel) and async (grpc.aiointerceptors)Test plan
🤖 Generated with Claude Code
Summary by cubic
Adds TLS/mTLS and API key auth to the Python SDK (sync and async), including a
tls=Trueoption to use the system trust store. Extends the admin API with API key and ACL management; behavior is unchanged when no TLS/auth options are set.New Features
ClientandAsyncClientsupporttls=True(system trust store) andca_cert,client_cert,client_keyfor TLS/mTLS (ca_certimplies TLS).api_keyaddsauthorization: Bearer <key>on all RPCs via interceptors (sync and async).CreateApiKey,RevokeApiKey,ListApiKeys,SetAcl,GetAcl; cluster fields added toGetStatsResponse,QueueInfo,ListQueuesResponse. README and tests updated for TLS/mTLS, system trust store, and API key (sync + async).Bug Fixes
ClientCallDetailsfor sync; async uses a concrete_AsyncClientCallDetails.__new__matching the 5-field namedtuple.ValueErrorifclient_cert/client_keyprovided without TLS.Written for commit 8360d5f. Summary will update on new commits.