-
Notifications
You must be signed in to change notification settings - Fork 304
Allow Entra authentication when connecting to Redis #3047
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
Allow Entra authentication when connecting to Redis #3047
Conversation
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.
Pull request overview
This PR adds support for Azure Entra authentication when connecting to Azure Managed Redis or Azure Cache for Redis instances. The implementation automatically detects when Entra authentication should be used based on whether a password is present in the connection string and whether the endpoint is localhost.
Changes:
- Added new
CreateConnectionMultiplexerAsyncmethod that handles both traditional password-based and Entra-based Redis authentication - Added Microsoft.Azure.StackExchangeRedis package to enable Azure-specific Redis features
- Updated Azure.Identity and Microsoft.Extensions.Configuration.Binder package versions
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/Service/Startup.cs | Added CreateConnectionMultiplexerAsync method with Entra authentication support and localhost detection logic |
| src/Service/Azure.DataApiBuilder.Service.csproj | Added Microsoft.Azure.StackExchangeRedis package reference |
| src/Directory.Packages.props | Updated Azure.Identity to 1.17.1, Microsoft.Extensions.Configuration.Binder to 8.0.2, and added Microsoft.Azure.StackExchangeRedis 3.3.1 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
/azp run |
|
Azure Pipelines successfully started running 6 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 6 pipeline(s). |
souvikghosh04
left a comment
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.
Added some review comments. also, it would be good to mention how this was tested as I don't see any supporting tests for the change. having test coverage with different scenarios would be great.
…/github.com/pharring/data-api-builder into dev/pharring/AddEntraAuthForRedisConnection
|
/azp run |
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.
LGTM, thank you for these contributions :)
|
Azure Pipelines successfully started running 6 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 6 pipeline(s). |
|
I'm investigating the unit test failure(s). The response message says the audience is "null" instead of BAD_AUDIENCE. Possibly caused by the Azure.Identity version bump (perhaps the latest MSAL obscures the audience name). |
…/github.com/pharring/data-api-builder into dev/pharring/AddEntraAuthForRedisConnection
Head branch was pushed to by a user without write access
|
/azp run |
|
Azure Pipelines successfully started running 6 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 6 pipeline(s). |
|
@copilot: can you fix the following formatting issues:
|
|
/azp run |
|
Azure Pipelines successfully started running 6 pipeline(s). |
Why make this change?
So you can use an Azure Managed Redis or Azure Cache for Redis instance with Entra authentication as a level 2 cache.
What is this change?
If the Redis connection string does not include a password and you are NOT connecting to localhost, then we assume you are connecting to an instance that requires Entra authentication. In that case, we use the DefaultAzureCredential (managed identity) to connect.
How was this tested?
Tested manually and ran unit tests.
Sample Request(s)
N/A