-
Notifications
You must be signed in to change notification settings - Fork 689
Add support for Azure Redis enterprise #10726
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
This commit introduces a new Azure Redis Enterprise resource, enhancing the existing Redis functionality. Key changes include: - Introduced new Bicep modules for defining Azure Redis infrastructure. - Implemented extension methods in `AzureRedisEnterpriseExtensions.cs` for resource management. - Defined the Azure Redis Enterprise resource structure in `AzureRedisEnterpriseResource.cs`. Fix dotnet#6831
1a82dc3
to
d8f5be3
Compare
Add tests Revert Redis playground app
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 pull request adds support for Azure Redis Enterprise, providing an enterprise-grade alternative to the existing Azure Redis functionality. The implementation enables both Azure-hosted and container-based development scenarios with proper authentication and role assignment support.
Key changes include:
- New Azure Redis Enterprise resource type with Bicep template generation
- Extension methods for adding Redis Enterprise resources to the application model
- Container fallback support for local development
- Role assignment configuration for Azure identity-based access
Reviewed Changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
src/Aspire.Hosting.Azure.Redis/AzureRedisEnterpriseResource.cs |
Core resource implementation with connection string, hostname, and password handling |
src/Aspire.Hosting.Azure.Redis/AzureRedisEnterpriseExtensions.cs |
Extension methods for adding Redis Enterprise resources and container fallback |
tests/Aspire.Hosting.Azure.Tests/AzureRedisEnterpriseExtensionsTests.cs |
Comprehensive test coverage for the new functionality |
playground/ files |
Updated playground examples demonstrating Redis Enterprise usage |
Multiple snapshot files | Generated Bicep templates and test verification files |
Where's the equivalent of WithAccessKeyAuthentication? |
I'm wondering whether we need it or not. We don't support password/key auth for normal Azure resources, including SQL Server. The original 3 database ones (Redis, CosmosDB, and PostgreSQL) had access key / password because that's all we knew how to do at the time. It took us a few versions to figure out how to support Entra auth. So I think we leave WithAccessKeyAuthentication off until it is proven we need it. |
Redis and Postgres are special because they can work with compute that does not support azure natively (e.g keycloak). Sometimes client libs are updated to handle native azure RBAC but the ones that don't require username and password (or keys). |
🚀 Dogfood this PR with: curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 10726 Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 10726" |
1. Make Azure Redis Enterprise experimental for its initial release 2. Support WithAccessKeyAuthentication
Ok, I've responded to all existing feedback. Please take another look. |
{ | ||
infrastructure.Add(new ProvisioningOutput("connectionString", typeof(string)) | ||
{ | ||
Value = BicepFunction.Interpolate($"{redis.HostName}:10000,ssl=true") |
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.
10000 is the port number? Why?
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.
This was the suggestion from @CawaMS and team. And it is the value when I create one through the portal.
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.
Weird 😄
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.
Few questions but 👍🏾
See https://learn.microsoft.com/en-us/dotnet/aspire/caching/azure-cache-for-redis-integration?tabs=dotnet-cli#add-azure-cache-for-redis-authenticated-client for instructions how to do it until we have a client integration for it.
Looks like https://github.com/Azure/azure-dev/blob/a9a799fcecb24305bdf57298453574e3b0ac9622/cli/azd/pkg/azapi/azure_resource_types.go#L54-L96 needs to be updated. |
now assign copilot 😄 |
I did, but locally (VSCode + GHCP). I've been liking that one better than iterating in the PR. |
Description
This commit introduces a new Azure Redis Enterprise resource, enhancing the existing Redis functionality. Key changes include:
AzureRedisEnterpriseExtensions.cs
for resource management.AzureRedisEnterpriseResource.cs
.Fix #6831
Checklist
<remarks />
and<code />
elements on your triple slash comments?