ENG-3194 - Ensure Redis connection URL is correct when configured to use a cluster + SSL

[johnewart]

Ticket ENG-3194

Description Of Changes

Ensures that when SSL and cluster mode are enabled that the query parameters in the generated URL are correct.

Code Changes

• Update URL generation in Redis config
• Add a test to ensure the URL contains the correct parameters

Steps to Confirm

N/A - no manual tests required

Pre-Merge Checklist

• Issue requirements met
• All CI pipelines succeeded
• CHANGELOG.md updated
  • Add a db-migration This indicates that a change includes a database migration label to the entry if your change includes a DB migration
  • Add a high-risk This issue suggests changes that have a high-probability of breaking existing code label to the entry if your change includes a high-risk change (i.e. potential for performance impact or unexpected regression) that should be flagged
  • Updates unreleased work already in Changelog, no new entry necessary
• UX feedback:
  • All UX related changes have been reviewed by a designer
  • No UX review needed
• Followup issues:
  • Followup issues created
  • No followup issues
• Database migrations:
  • Ensure that your downrev is up to date with the latest revision on main
  • Ensure that your downgrade() migration is correct and works
    • If a downgrade migration is not possible for this change, please call this out in the PR description!
  • No migrations
• Documentation:
  • Documentation complete, PR opened in fidesdocs
  • Documentation issue created in fidesdocs
  • If there are any new client scopes created as part of the pull request, remember to update public-facing documentation that references our scope registry
  • No documentation updates required

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
fides-plus-nightly	Ready	Preview, Comment	Apr 2, 2026 9:24pm

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
fides-privacy-center	Ignored		Apr 2, 2026 9:24pm

[claude]

Code Review

The fix is correct and well-scoped. get_cluster_connection_url() was returning a bare redis+cluster:// or rediss+cluster:// URL with no query parameters even when SSL was configured, which would silently drop certificate validation settings. The new code mirrors the existing SSL param-building logic from the non-cluster URL builder, which is the right approach.

No blockers. Two minor observations are left as inline comments:

1. ssl_cert_reqs (and ssl_ca_certs) are pre-encoded before being passed to urlencode, which technically double-encodes them — though in practice this is invisible since the expected values contain no special characters. This is a pre-existing pattern, not introduced here, and is worth a follow-up cleanup.

2. Missing test coverage for (a) cluster mode without SSL, and (b) cluster + SSL + auth credentials together. Neither is a correctness concern for the current change, but they'd improve regression safety.

The changelog is noted as unchecked in the PR template — please add an entry before merge.

[claude]

Minor: the value for ssl_cert_reqs is pre-encoded with quote_plus before being handed to urlencode(..., quote_via=quote), which means it gets encoded twice. For the expected values ("none", "required", "optional") this has no visible effect since they contain no special characters — but it's an inconsistency worth noting. The ssl_ca_certs value below is pre-encoded with quote similarly. Since this mirrors the existing pattern in the non-cluster URL builder (lines 310–314), I'd suggest tracking this as a follow-up cleanup rather than a blocker. Ideally, raw values would be passed and urlencode would handle all encoding uniformly.

[claude]

The three new tests cover the SSL-on path well. Two small gaps worth considering:

1. No test for the non-SSL cluster path. get_cluster_connection_url() with ssl=False (the original behavior) is not tested. Adding a simple case like RedisSettings(cluster_enabled=True) → "redis+cluster://:testpassword@redis:6379/0" would confirm the non-params branch still works and prevent regressions.

2. No test combining cluster + SSL + auth credentials. The non-cluster TLS tests don't cover auth either, but since the cluster implementation explicitly handles self.password/self.user, a test like RedisSettings(cluster_enabled=True, ssl=True, password="secret", user="admin") would give more confidence the auth prefix and SSL params are composed correctly in the same URL.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.03%. Comparing base (b3235db) to head (4eb5c66).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7811      +/-   ##
==========================================
+ Coverage   85.01%   85.03%   +0.01%     
==========================================
  Files         614      614              
  Lines       40094    40101       +7     
  Branches     4671     4673       +2     
==========================================
+ Hits        34087    34098      +11     
+ Misses       4965     4959       -6     
- Partials     1042     1044       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[JadeCara]

🚢