Dapr azure hosting ext #371

FullStackChef · 2025-01-06T19:36:43Z

Closes #349

Created CommunityToolkit.Aspire.Hosting.Dapr.AzureExtensions
- Provides functionality for referencing azure resources for dapr components
Created CommunityToolkit.Aspire.Hosting.Dapr.AzureRedis
- Provides extensions for referencing azure redis for dapr state

PR Checklist

Other information

Create Dapr Azure Redis project Create Example AppHost + ApiService

Create Dapr resource for provisioning

Use AddAzureInfrastructure

Add AzureDaprComponentResource Start of unit tests

Fixes based on unit tests

davidfowl · 2025-01-08T14:41:23Z

src/CommunityToolkit.Aspire.Hosting.Dapr.AzureRedis/README.md

+2. **`ConfigureRedisStateComponent`**  
+   Internal helper that creates and configures a “state.redis” Dapr component based on the `AzureRedisCacheResource`.  
+   - Generates a valid hostname and port.  
+   - Supports Azure Entra ID (AadEnabled) for secure access.  
+   - Stores or references secrets in Azure Key Vault if password-based authentication is required.


Not sure internal details need to be in the readme

Have adjusted docs

aaronpowell

As I was doing the review, it made me wonder if it wouldn't make more sense to create a single package, CommunityToolkit.Aspire.Hosting.Dapr.Azure.

My understanding is that the AzureExtensions package doesn't really do anything on its own, you'd need to bring in a package such as the Dapr.AzureRedis one or you'd create your own using some other Azure service to act as the caching layer in Azure.

Is the risk that if you aren't wanting to use Azure Redis as the caching layer you're going to end up bringing in some dependencies that are unneeded to the application (and thus "bloat")?

I guess I don't know enough about Dapr and what's needed/how you go about hosting it in Azure to truly understand what alternatives there might need to be in the future.

aaronpowell · 2025-01-08T22:50:44Z

...unityToolkit.Aspire.Hosting.Java.AppHost/CommunityToolkit.Aspire.Hosting.Java.AppHost.csproj

@@ -22,7 +22,6 @@

  <Target Name="PublishRunMaven" AfterTargets="Build">
    <!-- As part of publishing, ensure the Java app is freshly built -->
-    <Exec WorkingDirectory="$(JavaAppRoot)" Command="./mvnw --quiet clean package" />


Can you revert this change, as we have a PR to resolve it (and it'll cause builds to fail if it's not included at the moment)

Done - I'm pretty sure I didn't intentionally remove this.

...tyToolkit.Aspire.Hosting.Dapr.AzureExtensions/ApplicationModel/AzureDaprComponentResource.cs

aaronpowell · 2025-01-08T22:52:58Z

src/CommunityToolkit.Aspire.Hosting.Dapr.AzureExtensions/AzureDaprHostingExtensions.cs

+    public static IResourceBuilder<AzureDaprComponentResource> AddAzureDaprResource(
+        this IResourceBuilder<IDaprComponentResource> builder,
+        [ResourceName] string name,
+        Action<AzureResourceInfrastructure> configureInfrastructure)


This parameter is non-null and thus required, is that intended?

Yes it is intended and required because it is necessary

Is someone always going to need to provide their own configuration callback? Could it not be nullable and we have a default (even if it's just a noop)?

Or is it more that these are methods that you wouldn't realistically use in isolation, you'd only use them through wrappers like the Dapr Azure Redis package?

I mean you could no-op it, but I don't see a use case where you would use it without configuration.

It is primarily designed to be used by the wrappers - and I'm sitting here arguing with myself whether you would realistically use it in isolation.

On one hand if a wrapped integration exists for the component you're wanting to use - it makes sense to use that, however if not this integration avoids the pain I experienced trying to figure it out.

I think provided that I (or someone) can contribute wrapped version in a timely manner then this library could in fact be a supporting library that wasn't published separately?

The more I think about this the more I think that this shouldn't be published on it's own and should only serve as the base for more specific integrations.

src/CommunityToolkit.Aspire.Hosting.Dapr.AzureExtensions/AzureDaprHostingExtensions.cs

tests/CommunityToolkit.Aspire.Hosting.Dapr.AzureRedis.Tests/ResourceCreationTests.cs

…cationModel/AzureDaprComponentResource.cs Co-authored-by: Aaron Powell <[email protected]>

Co-authored-by: Aaron Powell <[email protected]>

…sourceCreationTests.cs Co-authored-by: Aaron Powell <[email protected]>

FullStackChef · 2025-01-09T09:01:28Z

As I was doing the review, it made me wonder if it wouldn't make more sense to create a single package, CommunityToolkit.Aspire.Hosting.Dapr.Azure.

My understanding is that the AzureExtensions package doesn't really do anything on its own, you'd need to bring in a package such as the Dapr.AzureRedis one or you'd create your own using some other Azure service to act as the caching layer in Azure.

Is the risk that if you aren't wanting to use Azure Redis as the caching layer you're going to end up bringing in some dependencies that are unneeded to the application (and thus "bloat")?

I guess I don't know enough about Dapr and what's needed/how you go about hosting it in Azure to truly understand what alternatives there might need to be in the future.

@aaronpowell yeah it would be a pretty bloated integration. Dapr has multiple "building blocks" - state is just one of these building blocks can be used with redis, cosmosdb, postgres, table / blob storage or sql server (to name the azure ones). so you would end up referencing a rather large number of the aspire hosting packages. which felt like it is contra to the design of aspire.

aaronpowell · 2025-01-09T22:54:51Z

src/CommunityToolkit.Aspire.Hosting.Dapr.AzureExtensions/AzureDaprHostingExtensions.cs

+    public static IResourceBuilder<AzureDaprComponentResource> AddAzureDaprResource(
+        this IResourceBuilder<IDaprComponentResource> builder,
+        [ResourceName] string name,
+        Action<AzureResourceInfrastructure> configureInfrastructure)


Is someone always going to need to provide their own configuration callback? Could it not be nullable and we have a default (even if it's just a noop)?

Or is it more that these are methods that you wouldn't realistically use in isolation, you'd only use them through wrappers like the Dapr Azure Redis package?

src/CommunityToolkit.Aspire.Hosting.Dapr.AzureRedis/README.md

aaronpowell · 2025-01-13T01:37:49Z

I'm wondering if the Azure Extensions package should ship as a source-only package (see https://andrewlock.net/creating-source-only-nuget-packages/) so that there's one less binary people would end up with, as they'd compile in the stuff they need (also means things could be marked as internal for library authors).

aaronpowell

I think this looks good - anything else outstanding in your opinion @FullStackChef

remove Dapr Azure extensions project make dapr azure extensions internal include extensions in dapr redis package include extensions in dapr extensions tests make dapr redis internals visible to dapr redis tests

FullStackChef · 2025-01-15T19:37:09Z

@aaronpowell I've made the following changes per our conversation

updated the PR to target your dapr-migration branch
remove Dapr Azure extensions project
make dapr azure extensions internal
include extensions in dapr redis package
include extensions in dapr extensions tests
make dapr redis internals visible to dapr redis tests

I haven't yet removed the references to the existing dapr extension as it will mean my branch doesn't build and that feels wrong. Hoping we can do this as a cleanup task prior to merging dapr-migration?

Other than that - my src, samples and tests all build and the tests run successfully.

Let me know how you want to proceed.

aaronpowell · 2025-01-16T05:32:09Z

Yep, let's do it as cleanup once merged in

davidfowl · 2025-01-16T05:46:47Z

src/CommunityToolkit.Aspire.Hosting.Dapr.AzureRedis/AzureRedisCacheDaprHostingExtensions.cs

+                Value = BicepFunction.Interpolate($"{redisCacheResource.HostName}:{port}")
+            });
+
+            daprResourceBuilder.WithParameter("redisHost", source.GetOutput("daprConnectionString"));


Same comment as below, do this outside of the callback

davidfowl · 2025-01-16T05:50:51Z

src/CommunityToolkit.Aspire.Hosting.Dapr.AzureRedis/AzureRedisCacheDaprHostingExtensions.cs

+
+        var daprResourceBuilder = builder.AddAzureDaprResource(redisDaprState, configureInfrastructure);
+
+        source.ConfigureInfrastructure(redisCache =>


Conventionally this parameter is "infra" as it refers to the bicep module

makes sense - will change

davidfowl · 2025-01-16T05:51:04Z

src/CommunityToolkit.Aspire.Hosting.Dapr.AzureRedis/AzureRedisCacheDaprHostingExtensions.cs

+            else
+            {
+                redisCache.ConfigureSecretAccess(daprComponent, redisCacheResource);
+                daprResourceBuilder.WithParameter("redisPasswordSecretUri", source.GetOutput("redisPasswordSecretUri"));


I wouldn't have this inside of the ConfigureInfrastructure callback (don't cross the streams).

@davidfowl I appreciate the ghostbusters ref, but I'm not sure how else to do this and I'm not really clear why?

This is being added conditionally as a result of something that happens inside ConfigureInfrastructure (checking if the redis has entra Id set) I could use a variable, but I don't know that that actually changes anything other than abstracting it away from view.

You want to think about where these callbacks run, what state is captured in them and what values you are referencing. It's not just C# code running that can all comingle, the lifetime and objects you use matter in these callbacks (were compiling to bicep) 😄 .

Treat that callback as producing the bicep module. It runs super late just before we are about to write the bicep to disk. You don't want to set parameters that late right?

I'm stuck sorry, In theory I understand what you're saying,

I just don't see how I can conditionally add this variable outside of configure infrastructure when AD authentication is set inside of configure infrastructure and if you call WithAccessKeyAuthentication then both the removal of AD and the addition of keyvault happens inside of ConfigureInfrastructure

I feel like this would be achievable if more of the conditional attributes made use of annotations

Why is this line hard to move out of the callback:

daprResourceBuilder.WithParameter("redisPasswordSecretUri", source.GetOutput("redisPasswordSecretUri"));

because that line is conditional. It is only called if someone has explicitly disabled AD authentication.
From aspire

var redisState = builder.AddAzureRedis("redisState") .WithAccessKeyAuthentication() // This calls ConfigureInfrastructure to disable AD auth .RunAsContainer();

WithAccessKeyAuthentication() calls

redis.RedisConfiguration.IsAadEnabled.ClearValue(); redis.IsAccessKeyAuthenticationDisabled.ClearValue();

and sets up key vault infra for the redis

If I move that line outside of the call back, the parameter is added unconditionally

Okay @davidfowl - I think I've worked out the issue. It looks like there are significant code changes under the hood in the main branch of the Aspire.Hosting.Azure.Redis package

v9.0.0
https://github.com/dotnet/aspire/blob/v9.0.0/src/Aspire.Hosting.Azure.Redis/AzureRedisExtensions.cs#L118
https://github.com/dotnet/aspire/blob/v9.0.0/src/Aspire.Hosting.Azure.Redis/AzureRedisExtensions.cs#L218

vs
main
https://github.com/dotnet/aspire/blob/main/src/Aspire.Hosting.Azure.Redis/AzureRedisExtensions.cs#L116
https://github.com/dotnet/aspire/blob/main/src/Aspire.Hosting.Azure.Redis/AzureRedisExtensions.cs#L188
https://github.com/dotnet/aspire/blob/main/src/Aspire.Hosting.Azure.Redis/AzureRedisExtensions.cs#L218

I've created a draft PR
dotnet/aspire#7135

the above would allow me to make the changes you're suggesting here, but not until the next release

Feel free to leave in the hack for now (consider using private reflection)

aaronpowell · 2025-01-17T01:56:04Z

As long as @davidfowl is happy with the changes, I reckon we're good to merge this in now

* Dapr migration initial import (#378) * Dropping files from Aspire repo * Moving to the right namespace/folder naming * Adding to solution and getting it to compile * Adding in the Dapr tests from the Aspire repo Had to remove the DaprSchemaTests file as we can't do schema tests (missing a lot of infrastructure from the Aspire repo). Had to edit the DaprTests to not use EnvironmentVariableEvaluator, which we can't leverage as it uses some internal types from Aspire.Hosting. This means that our testing of the environment variables is slightly different, and the values we assert against are not the docker internal host endpoints, but the public endpoints * Dapr azure hosting ext (#371) * Create Dap Azure extensions project Create Dapr Azure Redis project Create Example AppHost + ApiService * Work in progress Create Dapr resource for provisioning * Remove specific resource as not generating properly Use AddAzureInfrastructure * Updated to use secret refs * Fix - Remove code used for testing Add AzureDaprComponentResource Start of unit tests * Unit tests + Fixes based on unit tests * remove bicep file * Tests for AzureRedis * Add Readme and perform small cleanup tasks * Update src/CommunityToolkit.Aspire.Hosting.Dapr.AzureExtensions/ApplicationModel/AzureDaprComponentResource.cs Co-authored-by: Aaron Powell <[email protected]> * Apply suggestions from code review Co-authored-by: Aaron Powell <[email protected]> * Revert unintentional change to Java.AppHost * Update tests/CommunityToolkit.Aspire.Hosting.Dapr.AzureRedis.Tests/ResourceCreationTests.cs Co-authored-by: Aaron Powell <[email protected]> * Updated azure redis documentation also added missing xmldocs for api * correct unit test approach * remove unnecessary comment * null checking * More null checking * Update Azure redis readme * Update AzureExtension readme * move extensions to shared files remove Dapr Azure extensions project make dapr azure extensions internal include extensions in dapr redis package include extensions in dapr extensions tests make dapr redis internals visible to dapr redis tests * change redisCache to infra rename source to redisBuilder --------- Co-authored-by: Aaron Powell <[email protected]> * Fixing broken sln * Dapr example and tests (#394) * Migrating sample from dotnet/aspire repo * Adding dapr tests * Adding dapr setup to CI workflow * Following the setup-dapr instructions * Debugging CI * Falling back to looking at PATH * Fixing line endings replacement * Added PATH lookup to Windows * Adding some diagnostic info * Changing log level * init was only running on linux, which I think is wrong * Turning up logging * Bypass logging * Interpolated strings * Adding resource logger service * Java app build extension (#348) * Adding the ability to do a maven build in the app host This uses an event to run the mvnw build (or it can be customised via options), so it is only used when the app host is running the resource Fixes #339 * Adding a 'build with maven' command Allows you to rebuild the java app without having to restart the whole app host. * Expanding test coverage * Some more tests * Windows exe needs a file extension * Rolling back some changes * Renaming step * Tidying up the tests * Requiring docker for dapr tests * Adding codeowners * Moving dapr extensions to use our dapr package * Forgot to update the tests project * Adding some assembly filters * Adding readme updates * Slight break in the markdown --------- Co-authored-by: Brett Smith <[email protected]>

FullStackChef added 9 commits January 6, 2025 09:29

Create Dap Azure extensions project

cb357fe

Create Dapr Azure Redis project Create Example AppHost + ApiService

Work in progress

21e200a

Create Dapr resource for provisioning

Remove specific resource as not generating properly

fbe984f

Use AddAzureInfrastructure

Updated to use secret refs

b3ffeca

Fix - Remove code used for testing

035207c

Add AzureDaprComponentResource Start of unit tests

Unit tests +

b499fc4

Fixes based on unit tests

remove bicep file

f95eb10

Tests for AzureRedis

5b8ea45

Add Readme and perform small cleanup tasks

2176a04

WhitWaldo mentioned this pull request Jan 7, 2025

Improve integration between Dapr and Aspire dapr/dotnet-sdk#1428

Open

davidfowl reviewed Jan 8, 2025

View reviewed changes

aaronpowell requested changes Jan 8, 2025

View reviewed changes

FullStackChef and others added 4 commits January 9, 2025 15:19

Update src/CommunityToolkit.Aspire.Hosting.Dapr.AzureExtensions/Appli…

6a2c466

…cationModel/AzureDaprComponentResource.cs Co-authored-by: Aaron Powell <[email protected]>

Apply suggestions from code review

aae735b

Co-authored-by: Aaron Powell <[email protected]>

Revert unintentional change to Java.AppHost

dfa31a0

Update tests/CommunityToolkit.Aspire.Hosting.Dapr.AzureRedis.Tests/Re…

9c8f846

…sourceCreationTests.cs Co-authored-by: Aaron Powell <[email protected]>

FullStackChef added 5 commits January 9, 2025 23:02

Updated azure redis documentation also added missing xmldocs for api

a49de7b

correct unit test approach

57b20fa

remove unnecessary comment

fb2fc3d

null checking

f00affb

More null checking

b74d541

aaronpowell reviewed Jan 9, 2025

View reviewed changes

FullStackChef added 2 commits January 11, 2025 11:23

Update Azure redis readme

8b232a4

Update AzureExtension readme

60b56d0

FullStackChef requested review from aaronpowell and davidfowl January 10, 2025 22:38

Merge branch 'main' into dapr-azure-hosting-ext

2a1d9f0

aaronpowell reviewed Jan 15, 2025

View reviewed changes

FullStackChef changed the base branch from main to dapr-migration January 15, 2025 18:52

move extensions to shared files

33b26bc

remove Dapr Azure extensions project make dapr azure extensions internal include extensions in dapr redis package include extensions in dapr extensions tests make dapr redis internals visible to dapr redis tests

FullStackChef marked this pull request as ready for review January 15, 2025 19:32

aaronpowell approved these changes Jan 16, 2025

View reviewed changes

davidfowl reviewed Jan 16, 2025

View reviewed changes

change redisCache to infra rename source to redisBuilder

70d797a

FullStackChef mentioned this pull request Jan 16, 2025

Make UseAccessKeyAuthentication public so that it is accessible for p… dotnet/aspire#7135

Closed

17 tasks

aaronpowell merged commit 3c8d42f into CommunityToolkit:dapr-migration Jan 20, 2025
2 checks passed

FullStackChef mentioned this pull request Jan 25, 2025

Mark resource auth type public dotnet/aspire#7250

Merged

17 tasks

FullStackChef deleted the dapr-azure-hosting-ext branch February 1, 2025 20:50

Alirexaa added this to the 9.2 milestone Feb 6, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dapr azure hosting ext #371

Dapr azure hosting ext #371

FullStackChef commented Jan 6, 2025 •

edited

Loading

davidfowl Jan 8, 2025

FullStackChef Jan 9, 2025

aaronpowell left a comment

aaronpowell Jan 8, 2025

FullStackChef Jan 9, 2025

aaronpowell Jan 8, 2025

FullStackChef Jan 9, 2025

aaronpowell Jan 9, 2025

FullStackChef Jan 10, 2025

FullStackChef Jan 10, 2025

FullStackChef commented Jan 9, 2025

aaronpowell Jan 9, 2025

aaronpowell commented Jan 13, 2025

aaronpowell left a comment

FullStackChef commented Jan 15, 2025

aaronpowell commented Jan 16, 2025

davidfowl Jan 16, 2025 •

edited

Loading

davidfowl Jan 16, 2025

FullStackChef Jan 16, 2025

davidfowl Jan 16, 2025

FullStackChef Jan 16, 2025

davidfowl Jan 16, 2025

FullStackChef Jan 16, 2025

davidfowl Jan 16, 2025

FullStackChef Jan 16, 2025

FullStackChef Jan 16, 2025

davidfowl Jan 17, 2025

aaronpowell commented Jan 17, 2025


		var daprResourceBuilder = builder.AddAzureDaprResource(redisDaprState, configureInfrastructure);

		source.ConfigureInfrastructure(redisCache =>

Dapr azure hosting ext #371

Dapr azure hosting ext #371

Conversation

FullStackChef commented Jan 6, 2025 • edited Loading

PR Checklist

Other information

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aaronpowell left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FullStackChef commented Jan 9, 2025

Choose a reason for hiding this comment

aaronpowell commented Jan 13, 2025

aaronpowell left a comment

Choose a reason for hiding this comment

FullStackChef commented Jan 15, 2025

aaronpowell commented Jan 16, 2025

davidfowl Jan 16, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aaronpowell commented Jan 17, 2025

FullStackChef commented Jan 6, 2025 •

edited

Loading

davidfowl Jan 16, 2025 •

edited

Loading