-
Notifications
You must be signed in to change notification settings - Fork 1.9k
feat(infra): Add IAM support for Redis #5267
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Summary
This review covers only the changes made since the last review (commit 96aacb9), not the entire PR.
The most recent changes address the code duplication issue identified in the previous review by refactoring the SSL context creation logic. The developer has extracted the SSL context setup code into a dedicated create_redis_ssl_context_if_iam()
function and modified configure_redis_iam_auth()
to call this function instead of duplicating the SSL context creation logic.
The refactored code now follows the DRY (Don't Repeat Yourself) principle more effectively. The create_redis_ssl_context_if_iam()
function centralizes the SSL context creation with system CA certificates, hostname verification, and certificate requirements. The configure_redis_iam_auth()
function now simply calls this helper function to obtain the SSL context, eliminating the code duplication that existed in the previous version.
This change maintains the same functionality for Redis IAM authentication while improving code organization and maintainability. The SSL context configuration remains consistent across both functions, ensuring that IAM authentication continues to work properly with AWS ElastiCache using system CA certificates and proper SSL verification.
Confidence score: 5/5
- This PR is safe to merge with minimal risk
- Score reflects successful resolution of the previous code duplication concern and simple refactoring with no functional changes
- No files require special attention
1 file reviewed, no comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
5 issues found across 4 files
React with 👍 or 👎 to teach cubic. You can also tag @cubic-dev-ai
to give feedback, ask questions, or re-run the review.
* jira jql support * jira jql fixes
9c4ddfb
to
5748be6
Compare
* feat: JIRA support for custom JQL filter (#5164) * jira jql support * jira jql fixes * Address comment --------- Co-authored-by: sktbcpraha <131408565+sktbcpraha@users.noreply.github.com>
* feat: JIRA support for custom JQL filter (#5164) * jira jql support * jira jql fixes * Address comment --------- Co-authored-by: sktbcpraha <131408565+sktbcpraha@users.noreply.github.com>
* feat: JIRA support for custom JQL filter (onyx-dot-app#5164) * jira jql support * jira jql fixes * Address comment --------- Co-authored-by: sktbcpraha <131408565+sktbcpraha@users.noreply.github.com>
Description
[Provide a brief description of the changes in this PR]
Adding support for Redis IAM
How Has This Been Tested?
[Describe the tests you ran to verify your changes]
Backporting (check the box to trigger backport action)
Note: You have to check that the action passes, otherwise resolve the conflicts manually and tag the patches.
Summary by cubic
Adds Redis IAM authentication for AWS ElastiCache behind a USE_REDIS_IAM_AUTH flag. Enables passwordless, TLS-secured connections for both sync and async Redis usage, and updates Celery config to work with IAM.