feat(redis): add redix_opts for SSL/TLS support - #59
Conversation
WalkthroughThe PR adds per-pool Redix options to Anubis.Server.Session.Store.Redis via a new Sequence Diagram(s)sequenceDiagram
participant App as Application
participant Store as Redis.Store (init)
participant Config as Config (:redix_opts)
participant PoolSupervisor as Pool Supervisor
participant Redix as Redix.start_link
App->>Store: start_link(opts)
Store->>Config: read :redix_opts (may be nil)
Store->>Store: validate_redix_opts(:redix_opts)
rect rgb(230,245,230)
note over Store: Merge Phase\n(drop external :name, keep internal naming)
Store->>Store: take internal defaults\n(name, sync_connect, exit_on_disconnection)
Store->>Store: drop user :name
Store->>Store: merge defaults <- custom_opts (custom wins)
end
Store->>PoolSupervisor: build pool children spec (per-member redix_opts)
PoolSupervisor->>Redix: start_link(redis_url + per-member redix_opts)
Redix-->>PoolSupervisor: started / error
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Areas to verify (priority P0–P3):
Review with confidence, clarity & light humor 😎. Ensure MCP spec compliance & good Elixir/OTP patterns. Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
a95cc5b to
cbd83ed
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
lib/anubis/server/session/store/redis.ex(2 hunks)
🔇 Additional comments (3)
lib/anubis/server/session/store/redis.ex (3)
16-33: Solid SSL/TLS documentation! 👍The example configuration for Upstash-style TLS connections is clear and will save users significant time. The
rediss://URL andcustomize_hostname_checkpattern is exactly what folks need for cloud Redis providers.The earlier P3 suggestion about explicitly documenting merge precedence (custom options override defaults except
:name) still applies if you'd like to make this even more self-explanatory for MCP integrators.
110-114: P1 concern addressed::namestripping works correctly! ✅This implementation properly prevents users from accidentally breaking the connection pool by overriding the internally-managed
:namekey. TheKeyword.drop([:name])ensures pool integrity while still allowing customization of SSL options,:sync_connect,:exit_on_disconnection, etc.Exactly the fix the earlier P1 review requested. Well done!
121-129: Merge logic is spot-on! 🎯The default options establish internal requirements (
:name,:sync_connect: false,:exit_on_disconnection: false), thenKeyword.merge(custom_redix_opts)allows user options to override everything except:name(which was stripped earlier). Custom SSL/TLS settings, socket options, and even tweaks to:sync_connectwill work as expected without breaking the pool's naming convention.
cbd83ed to
237c50c
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
lib/anubis/server/session/store/redis.ex (1)
16-18: Clarify how:redix_optsare merged & reserved keys (P3)The SSL/TLS example is helpful and the basic config snippet is clear. To fully set expectations for MCP users, consider adding a short note in the moduledoc that:
:redix_optsare merged into a per-connection default option set (name,sync_connect,exit_on_disconnection),- user-provided keys override those defaults, and
:nameis treated as reserved and ignored from:redix_opts(pool naming is always internal).This matches the behaviour implemented later in
init/1and saves people from wondering why a customname:is being dropped.Also applies to: 19-33
Add :redix_opts configuration option to Redis session store that allows
passing custom Redix connection options. This enables connecting to
cloud Redis providers requiring SSL/TLS with custom hostname verification
(like Upstash, Redis Enterprise, etc.).
Example configuration for Upstash:
config :anubis_mcp, :session_store,
adapter: Anubis.Server.Session.Store.Redis,
redis_url: "rediss://...",
redix_opts: [
ssl: true,
socket_opts: [
customize_hostname_check: [
match_fun: :public_key.pkix_verify_hostname_match_fun(:https)
]
]
]
Custom options are merged with defaults (custom takes precedence).
237c50c to
cf1cb9e
Compare
|
thanks for the contribution! |
|
could you only ensure the lint on ci pass? basically it's missing formatting |
Fixes CI formatting check failures: - Replace Keyword.drop([:name]) with Keyword.delete(:name) - Collapse multi-line keyword list to single line per formatter rules Changes: - Line 115: Use Keyword.delete/2 instead of Keyword.drop/1 for single key - Lines 124-130: Inline keyword list merge for better readability Quality verification: - Follows Elixir formatter conventions - Functionally identical (Keyword.delete(:name) is semantically correct for single key) 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
lib/anubis/server/session/store/redis.ex(3 hunks)
🔇 Additional comments (4)
lib/anubis/server/session/store/redis.ex (4)
110-115: Extraction and validation logic is solid! ✓The pipeline correctly:
- Fetches
:redix_optswith a safe default- Validates it's a proper keyword list (or
nil)- Strips the reserved
:namekey before mergingThis addresses the P1 concern from earlier reviews—users can no longer accidentally break the pool's internal naming scheme while still being able to customize SSL, socket options, and other Redix settings.
122-124: Merge logic is correct! 🎯
Keyword.merge/2with defaults first andcustom_redix_optssecond gives custom options the intended precedence. Since:namewas already stripped fromcustom_redix_opts(line 115), the internally computedchild_idalways controls the connection name, while users can still override:sync_connect,:exit_on_disconnection, or add SSL options.The comments clearly document the behavior. Nice work!
128-128: Clean integration with Redix! 🔌Passing
redis_urland the computedredix_optstoRedix.start_link/2is exactly the right call pattern. The defaults match the old hardcoded behavior (backward compatible), and custom SSL/TLS options now flow through correctly.
301-309: Validation helper is tight and user-friendly! ✓The function correctly:
- Accepts
niland treats it as "no extra options" (forgiving)- Uses
Keyword.keyword?/1to enforce proper keyword list structure (precise)- Raises a clear
ArgumentErrorfor invalid inputThis catches configuration mistakes early (e.g.,
[:ssl, true]instead of[ssl: true]) with a helpful error message, rather than letting Redix orKeyword.delete/2fail cryptically later.
🚀 Want to release this? --- ## [0.17.0](v0.16.0...v0.17.0) (2025-12-09) ### Features * **redis:** add redix_opts for SSL/TLS support ([#59](#59)) ([33658ab](33658ab)) ### Bug Fixes * added server component description/0 callback ([#58](#58)) ([a094473](a094473)) * redix should be loaded ([#71](#71)) ([09b872f](09b872f)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added Redis SSL/TLS support via redix options. * **Bug Fixes** * Fixed server component description callback issue. * Fixed redix loading issue. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
🚀 Want to release this? --- ## [0.17.0](v0.16.0...v0.17.0) (2025-12-09) ### Features * **redis:** add redix_opts for SSL/TLS support ([#59](#59)) ([3fd674a](3fd674a)) ### Bug Fixes * added server component description/0 callback ([#58](#58)) ([a094473](a094473)) * redix should be loaded ([#71](#71)) ([c352414](c352414)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added Redis SSL/TLS support via redix options. * **Bug Fixes** * Fixed server component description callback issue. * Fixed redix loading issue. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 participants
Summary
Add
:redix_optsconfiguration option to Redis session store that allows passing custom Redix connection options. This enables connecting to cloud Redis providers requiring SSL/TLS with custom hostname verification (like Upstash, Redis Enterprise, etc.).Problem
The Redis session store currently hardcodes Redix connection options, making it impossible to connect to cloud Redis providers that require SSL/TLS with custom hostname verification.
Upstash and similar providers use wildcard SSL certificates (e.g.,
*.upstash.io) which require custom hostname verification. Currently, users must create a full custom adapter (~400 lines) just to add SSL options.Solution
Add a
:redix_optsconfiguration option that merges with the default Redix options:Changes
custom_redix_optsfrom config options (defaults to[])Test plan
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.