feat: add crypto (KMS / Key Vault) backend and SQS exclude_env_creden…#16
Conversation
…tials; release v0.2.6 - New cloudrift.crypto module: get_crypto for AWS KMS and Azure Key Vault keys (encrypt/decrypt + base64 string helpers), with CryptoError hierarchy. - SQS from_iam_role gains exclude_env_credentials to drop the env-var credential provider so ambient process credentials can't shadow the instance/task role. - Includes pending storage and messaging (azure_bus / base) enhancements. - Bump version to 0.2.6, add azure-keyvault-keys extra, re-lock.
shreyas-lyzr
left a comment
shreyas-lyzr
left a comment
There was a problem hiding this comment.
Solid release. The crypto module is well-structured and consistent with the existing factory/ABC pattern. The messaging payload migration from JSON-dict to raw bytes+attributes is correctly propagated across both the SQS and Service Bus backends, and the test coverage is thorough. Three minor issues below — none blocking.
cloudrift/messaging/sqs.py, line 246 and 267 (body.decode() in send/send_batch)
The default .decode() will raise a bare UnicodeDecodeError if a caller passes non-UTF-8 bytes. SQS MessageBody is a Unicode field, so the constraint is real — it is just worth making explicit with .decode('utf-8') and wrapping any decode failure as MessageSendError so the caller gets an actionable error rather than a codec exception from inside a boto3 call.
cloudrift/messaging/sqs.py, dead_letter() (~line 371)
The emulated dead-letter sends the body to the DLQ with only a DeadLetterReason attribute — the original message's MessageAttributes (the user-defined flat map) are not forwarded. A DLQ consumer that routes on those attributes (e.g. content_type) has no way to access them. This is a pre-existing gap, but the new bytes+attributes contract makes it more visible. Storing attributes alongside the body in _pending and merging them into the DLQ send call would close it.
cloudrift/messaging/sqs.py and storage/s3.py, from_assume_role() (~line 153)
The sts.assume_role() call is synchronous and runs on the event loop thread. If this factory is ever called from inside a running async context (e.g. lazy backend init in an async handler), it will stall the loop. The docstring should note 'call this factory from synchronous context only', or the STS call should use run_in_executor. Not urgent for typical use at startup, but worth closing before the pattern spreads.
|
3 participants
…tials; release v0.2.6