Skip to content

Conversation

@pharring
Copy link

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

@JerryNixon JerryNixon marked this pull request as ready for review January 21, 2026 21:47
Copilot AI review requested due to automatic review settings January 21, 2026 21:47
Copy link
Contributor

Copilot AI left a 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 CreateConnectionMultiplexerAsync method 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.


// If no password is provided, and the endpoint (or at least one of them) is non-localhost,
// attempt to use Entra authentication.
if (string.IsNullOrEmpty(options.Password) && !options.EndPoints.Any(IsLocalhostEndpoint))
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic for determining whether to use Entra authentication checks if "at least one endpoint is non-localhost". However, the condition !options.EndPoints.Any(IsLocalhostEndpoint) returns true only when NONE of the endpoints are localhost. This means that if you have a mix of localhost and non-localhost endpoints, it will still attempt Entra authentication.

The comment on line 492 says "or at least one of them" but the code checks "NONE of them are localhost". This is a logical inconsistency. If the intent is to use Entra when ANY endpoint is non-localhost, the condition should be options.EndPoints.Any(ep => !IsLocalhostEndpoint(ep)).

Suggested change
if (string.IsNullOrEmpty(options.Password) && !options.EndPoints.Any(IsLocalhostEndpoint))
if (string.IsNullOrEmpty(options.Password) && options.EndPoints.Any(ep => !IsLocalhostEndpoint(ep)))

Copilot uses AI. Check for mistakes.
Comment on lines +496 to +499
options = await options.ConfigureForAzureWithTokenCredentialAsync(new DefaultAzureCredential());
}

return await ConnectionMultiplexer.ConnectAsync(options);
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CreateConnectionMultiplexerAsync method lacks error handling for Azure credential failures or Redis connection failures. When DefaultAzureCredential fails to authenticate (e.g., no managed identity is available, or credentials are misconfigured), or when ConfigureForAzureWithTokenCredentialAsync fails, the method will throw an unhandled exception. Consider adding try-catch blocks with more descriptive error messages to help users troubleshoot authentication or connection issues.

Suggested change
options = await options.ConfigureForAzureWithTokenCredentialAsync(new DefaultAzureCredential());
}
return await ConnectionMultiplexer.ConnectAsync(options);
try
{
options = await options.ConfigureForAzureWithTokenCredentialAsync(new DefaultAzureCredential());
}
catch (CredentialUnavailableException ex)
{
throw new InvalidOperationException(
"Failed to acquire Azure credentials for Redis. " +
"Ensure that a managed identity or other Azure credentials are correctly configured " +
"for this environment before enabling Entra authentication for Redis.",
ex);
}
catch (AuthenticationFailedException ex)
{
throw new InvalidOperationException(
"Azure authentication for Redis failed while using DefaultAzureCredential. " +
"Verify that the configured identity has permission to access the Redis resource " +
"and that all required environment settings are correct.",
ex);
}
}
try
{
return await ConnectionMultiplexer.ConnectAsync(options);
}
catch (RedisConnectionException ex)
{
throw new InvalidOperationException(
"Failed to connect to Redis using the provided configuration. " +
"Verify that the Redis connection string, network connectivity, and Redis instance " +
"configuration are correct.",
ex);
}

Copilot uses AI. Check for mistakes.
Comment on lines +480 to +500
private static async Task<IConnectionMultiplexer> CreateConnectionMultiplexerAsync(string connectionString)
{
ConfigurationOptions options = ConfigurationOptions.Parse(connectionString);

// Determine if an endpoint is localhost/loopback
static bool IsLocalhostEndpoint(EndPoint ep) => ep switch
{
DnsEndPoint dns => string.Equals(dns.Host, "localhost", StringComparison.OrdinalIgnoreCase),
IPEndPoint ip => IPAddress.IsLoopback(ip.Address),
_ => false,
};

// If no password is provided, and the endpoint (or at least one of them) is non-localhost,
// attempt to use Entra authentication.
if (string.IsNullOrEmpty(options.Password) && !options.EndPoints.Any(IsLocalhostEndpoint))
{
options = await options.ConfigureForAzureWithTokenCredentialAsync(new DefaultAzureCredential());
}

return await ConnectionMultiplexer.ConnectAsync(options);
}
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new CreateConnectionMultiplexerAsync method and its Entra authentication logic lack test coverage. Consider adding unit tests to verify:

  1. Correct behavior when password is provided (should not use Entra)
  2. Correct behavior when connecting to localhost without password (should not use Entra)
  3. Correct behavior when connecting to a non-localhost endpoint without password (should use Entra)
  4. Behavior with mixed localhost and non-localhost endpoints

Copilot uses AI. Check for mistakes.
Comment on lines +485 to +491
static bool IsLocalhostEndpoint(EndPoint ep) => ep switch
{
DnsEndPoint dns => string.Equals(dns.Host, "localhost", StringComparison.OrdinalIgnoreCase),
IPEndPoint ip => IPAddress.IsLoopback(ip.Address),
_ => false,
};

Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The IsLocalhostEndpoint function checks for "localhost" in DnsEndPoint but doesn't handle common variations like "127.0.0.1" or "::1" when provided as DNS names. While IPEndPoint handles IP loopback addresses correctly, if someone provides "127.0.0.1" as a DNS hostname in the connection string (parsed as DnsEndPoint), it won't be detected as localhost. Consider adding additional checks for common localhost IP addresses as DNS names.

Suggested change
static bool IsLocalhostEndpoint(EndPoint ep) => ep switch
{
DnsEndPoint dns => string.Equals(dns.Host, "localhost", StringComparison.OrdinalIgnoreCase),
IPEndPoint ip => IPAddress.IsLoopback(ip.Address),
_ => false,
};
static bool IsLocalhostEndpoint(EndPoint ep)
{
return ep switch
{
DnsEndPoint dns => IsLocalhostHostName(dns.Host),
IPEndPoint ip => IPAddress.IsLoopback(ip.Address),
_ => false,
};
static bool IsLocalhostHostName(string host)
{
if (string.Equals(host, "localhost", StringComparison.OrdinalIgnoreCase))
{
return true;
}
if (IPAddress.TryParse(host, out IPAddress? parsedIp))
{
return IPAddress.IsLoopback(parsedIp);
}
return false;
}
}

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants