feat: add optional REDIS_KEY_PREFIX for multi-app deployments, enable GCP Memorystore Valkey #19
Conversation
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
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
Add PrefixedRedis wrapper to support key prefixing in shared Redis instances (e.g., GCP Memorystore). When REDIS_KEY_PREFIX is set, all Redis keys are prefixed with the specified value. - Create api/src/redis.rs with PrefixedRedis wrapper - Update cluster-hashring to use start_with_prefix() - Update OAuth polling to use prefixed keys - Document REDIS_KEY_PREFIX in CLAUDE.md
samchristenoliphant
force-pushed
the
feat/redis-key-prefix
branch
from
2e76310 to
84addd2
Compare
- Add ValkeyConnectionFactory with GCP IAM token support - Tokens cached and auto-refreshed 5 minutes before expiry - Coordinator heartbeat monitors token TTL for connection refresh - PrefixedRedis supports factory-based connection refresh - New USE_VALKEY_IAM env var (default: false) - Backward compatible with standard Redis when IAM disabled
- Prevent token exposure in logs by not logging URLs with credentials - Add auto-refresh on auth errors in PrefixedRedis (handles token expiry) - Preserve URL scheme/path/query when injecting IAM credentials - Replace expect() panic with proper error handling - Add retry with exponential backoff for token fetch (3 attempts) - Deduplicate TOKEN_REFRESH_BUFFER_SECS constant
- Use redis ErrorKind::AuthenticationFailed instead of string matching - Add jitter to token fetch retry to prevent thundering herd
- Expand is_auth_error() to catch NOAUTH, WRONGPASS, and other auth patterns from expired IAM tokens (not just AuthenticationFailed) - Add warning log when token TTL conversion fails in valkey_auth - Add unit tests for auth error detection patterns - Add integration tests for PrefixedRedis (requires local Redis)
samchristenoliphant
changed the title
- Add Debug impl for ValkeyConnectionFactory (redacts sensitive fields) - Add Debug impl for PrefixedRedis (redacts connection) - Document magic values with named constants (retry params) - Improve doc comments: shorter first sentences, add # Errors sections - Add Send+Sync compile-time assertions for async types - Improve auth error detection using e.code() for NOAUTH/WRONGPASS - Keep string matching as fallback for robustness - Fix refresh_connection: return () instead of Result since errors are logged
dcadenas
approved these changes
Jan 27, 2026
Contributor
dcadenas
left a comment
dcadenas
left a comment
There was a problem hiding this comment.
looks good! I added some nit picks on rust guidelines stuff, merge whenever if it looks good to you
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
PrefixedRediswrapper to support key prefixing in shared Redis instances (e.g., GCP Memorystore)AUTH_MODE_IAMREDIS_KEY_PREFIXis set, all Redis keys are prefixed with the specified valueUSE_VALKEY_IAM=true, uses GCP Workload Identity for authenticationChanges
Redis Key Prefix (original)
api/src/redis.rs- NewPrefixedRediswrapper withsetex,get,delmethodskeycast/src/main.rs- ReadREDIS_KEY_PREFIXenv var and pass to both componentsValkey IAM Authentication (new)
cluster-hashring/src/valkey_auth.rs- NEW:ValkeyConnectionFactorywith GCP IAM token supportcluster-hashring/src/coordinator.rs- Addstart_with_config()with IAM param, token refresh in heartbeatcluster-hashring/src/registry.rs- Addregister_with_factory()andrefresh_connection()api/src/redis.rs- Addnew_with_factory()for factory-based connection refreshkeycast/src/main.rs- ReadUSE_VALKEY_IAMenv var, share factory between coordinator and APIEnvironment Variables
REDIS_URLREDIS_KEY_PREFIXUSE_VALKEY_IAMfalseToken Refresh Strategy
Test plan
cargo buildcargo testin cluster-hashring crateUSE_VALKEY_IAM=falseagainst local RedisUSE_VALKEY_IAM=true, verify: