Add Azure PostgreSQL managed identity #5930

eerhardt · 2024-09-25T21:40:53Z

Description

Add a new API to replace AsAzure and PublishAsAzure. This creates an Azure resource to start, so it can be configured like other azure resources. Since this is a new API, we can use managed identity by default.
Add an API to convert managed identity to password based auth, so users can still go that route if they need to.
Add RunAsContainer following the RunAsEmulator model. This allows local development against a PostreSQL container and publish to go to a manged Azure PostgreSQL flexible server.

Checklist

Is this feature complete?
- No. Follow-up changes expected. Still need Update Azure Sql to use new API pattern #5998
Are you including unit tests for the changes and scenario tests if relevant?
- Yes
Did you add public API?
- Yes
  - If yes, did you have an API Review for it?
    - Yes
  - Did you add <remarks /> and <code /> elements on your triple slash comments?
    - Yes
Does the change make any security assumptions or guarantees?
- Yes
  - If yes, have you done a threat model and had a security review?
    - Yes
Does the change require an update in our Aspire docs?
- Yes
  - Link to aspire-docs issue: Document Entra ID and new Azure API pattern for Redis, PostgreSQL, and SQLServer docs-aspire#1704

Microsoft Reviewers: Open in CodeFlow

Similar to dotnet#5930, we add a new API pattern for working with Azure resources that may also be local containers. - Add a new API to replace AsAzure and PublishAsAzure. This creates an Azure resource to start, so it can be configured like other azure resources. Since this is a new API, we can use managed identity by default. - Add an API to convert managed identity to password based auth, so users can still go that route if they need to. - Add RunAsContainer following the RunAsEmulator model. This allows local development against a Redis container and publish to go to a manged Azure Cache for Redis. Fix dotnet#5794

* Support Managed Identity in Azure Cache for Redis Similar to #5930, we add a new API pattern for working with Azure resources that may also be local containers. - Add a new API to replace AsAzure and PublishAsAzure. This creates an Azure resource to start, so it can be configured like other azure resources. Since this is a new API, we can use managed identity by default. - Add an API to convert managed identity to password based auth, so users can still go that route if they need to. - Add RunAsContainer following the RunAsEmulator model. This allows local development against a Redis container and publish to go to a manged Azure Cache for Redis. Fix #5794

davidfowl · 2024-09-28T19:28:21Z

@eerhardt Can you do this as well #3299? I think postgres is the only database that we need to fix.

See #3277. It should be a simple change to the bicep.

mitchdenny · 2024-09-30T03:10:18Z

@eerhardt is this something you want to get in for 9.0 or 9.x?

mitchdenny · 2024-09-30T03:12:32Z

The downside of this approach is composability of resources. Let's say I wanted to have a resource that depended on a Postgres database instance. With this change the implementor of that would need to have extensions for each cloud provider (unless they just allow any IResourceWithConnectionString).

davidfowl · 2024-09-30T04:44:57Z

The downside of this approach is composability of resources. Let's say I wanted to have a resource that depended on a Postgres database instance. With this change the implementor of that would need to have extensions for each cloud provider (unless they just allow any IResourceWithConnectionString).

That can be solved with an IPostgresResource interface right?

eerhardt · 2024-09-30T16:29:06Z

@eerhardt is this something you want to get in for 9.0 or 9.x?

9.0. Azure PostgreSQL should support managed identity (dotnet/aspire#5793) is in the 9.0 milestone.

eerhardt · 2024-09-30T16:32:24Z

The downside of this approach is composability of resources. Let's say I wanted to have a resource that depended on a Postgres database instance. With this change the implementor of that would need to have extensions for each cloud provider (unless they just allow any IResourceWithConnectionString).

We already have a "composability of resources" problem with the current API. You can't call .ConfigureConstruct on the Azure resource for Postgres. Even worse - there is no Azure resource for Postgres in the model. So if you wanted to add an event to inspect all the resources (and possibly modify them), you won't see the AzurePostgresResource resource in the model.

That can be solved with an IPostgresResource interface right?

Yes, we can add that, if necessary.

- Add a new API to replace AsAzure and PublishAsAzure. This creates an Azure resource to start, so it can be configured like other azure resources. Since this is a new API, we can use managed identity by default. - Add an API to convert managed identity to password based auth, so users can still go that route if they need to. - Add RunAsContainer following the RunAsEmulator model. This allows local development against a PostreSQL container and publish to go to a manged Azure PostgreSQL flexible server. Fix dotnet#5793

Add Obsolete to old APIs

Fix up playground apps.

eerhardt · 2024-09-30T23:02:16Z

This PR is ready for review. I believe I have everything ready for this to be merged.

davidfowl · 2024-09-30T23:27:50Z

tests/Aspire.Hosting.Azure.Tests/AzureContainerAppsTests.cs

@@ -98,7 +99,7 @@ param outputs_azure_container_apps_environment_id string
          }
        }
        """;
-
+        output.WriteLine(bicep);


You want this for when the bicep changes and you need to update it. You can copy and paste the new output from the test log

mitchdenny · 2024-10-01T00:17:47Z

I believe it would be good to add an IPostgresResource interface (or add a follow-up for this).

mitchdenny · 2024-10-01T00:18:45Z

playground/bicep/BicepSample.AppHost/Program.cs

-var pg = builder.AddPostgres("postgres2", administratorLogin, administratorLoginPassword)
-                .AsAzurePostgresFlexibleServer()
+var pg = builder.AddAzurePostgresFlexibleServer("postgres2")
+                .WithPasswordAuth(administratorLogin, administratorLoginPassword)


Should this be WithPasswordAuthentication?

The reason I chose these names (WithPasswordAuth here and WithAccessKeyAuth in Reds) is because they match the APIs from Azure.Provisioning (which I assume just match the names in the rest API).

aspire/src/Aspire.Hosting.Azure.PostgreSQL/AzurePostgresExtensions.cs

Lines 192 to 196 in e3e903a

postgres.AuthConfig = new PostgreSqlFlexibleServerAuthConfig()

{

ActiveDirectoryAuth = PostgreSqlFlexibleServerActiveDirectoryAuthEnum.Enabled,

PasswordAuth = PostgreSqlFlexibleServerPasswordAuthEnum.Disabled

};

But looking again at Redis, the name is DisableAccessKeyAuthentication, which is inconsistent with the above Postgres APIs. So it is probably better we just completely follow .NET Design Guidelines with our APIs here and use full words.

FYI - @tg-msft. I'm assuming this isn't something we can change/fix in Azure.Provisioning since it is just generated from the service APIs / typespec.

We should do an API review, WithAccessKey is not intuitive. We should take the liberty here to add some opinions to this layer of the stack. We can do this in a follow up PR.

WithAccessKey is not intuitive

Where is that? For Redis it is:

var redis = builder.AddAzureRedis("cache") .WithAccessKeyAuthentication();

now that I renamed Auth to be Authentication.

@philon-msft - is there a reason that the portal / ARM use the term "access key"?

Azure Redis doesn't let you choose your own (like a "password") - instead it generates one for you (more like an "access key").
So in general Redis terminology it's a password, but for Azure Redis it's an access key.

Correct, it's passwordAuth on the wire and we're not looking to deviate from our automated translation of the existing ARM experience. There's no way Azure.Provisioning would scale to all of Azure otherwise.

Azure Redis doesn't let you choose your own (like a "password") - instead it generates one for you (more like an "access key"). So in general Redis terminology it's a password, but for Azure Redis it's an access key.

Does Azure Redis let me add users with their own passwords and ACLs?

Not standard Redis users+passwords, but you can add Entra users/apps/ManagedIdentities and assign them to roles for ACL functionality. See doc for the Entra experience

mitchdenny · 2024-10-01T01:01:46Z

Overall I think this looks good. I do think that there is another approach we could take here (just putting it out there for sake of completeness.

We could do this:

var builder = DistributedApplication.Create(args);
var db = builder.AddPostgres("pgsql").AsAzure(out var azpgsql).AddDatabase("db");
azpgsql.DoAzureyApiStuff();

Folks have historically avoided out vars but we see them a lot more these days. All that said I think your approach here is fine.

davidfowl

lgtm

eerhardt · 2024-10-01T19:18:12Z

I believe it would be good to add an IPostgresResource interface (or add a follow-up for this).

Opened Add interfaces to PostgreSQL, Redis, and SqlServer Hosting Resources (dotnet/aspire#6055)

eerhardt requested review from davidfowl, mitchdenny and sebastienros September 25, 2024 21:40

This was referenced Sep 26, 2024

Support Managed Identity in Azure Cache for Redis #5966

Merged

Update Azure Sql to use new API pattern #5998

Closed

Document Entra ID and new Azure API pattern for Redis, PostgreSQL, and SQLServer dotnet/docs-aspire#1704

Closed

eerhardt force-pushed the PostgresManagedIdentity branch from 1c73ff2 to 38f0274 Compare September 27, 2024 23:22

eerhardt force-pushed the PostgresManagedIdentity branch from a5d6f5d to c272c30 Compare September 30, 2024 22:56

eerhardt added 4 commits September 30, 2024 17:59

Revert unnecessary changes.

7118027

Add Obsolete to old APIs

Re-run generate manifest

ab896ff

Add tests and XML docs.

e3e903a

Fix up playground apps.

eerhardt force-pushed the PostgresManagedIdentity branch from c272c30 to e3e903a Compare September 30, 2024 23:00

eerhardt marked this pull request as ready for review September 30, 2024 23:01

davidfowl reviewed Sep 30, 2024

View reviewed changes

mitchdenny reviewed Oct 1, 2024

View reviewed changes

mitchdenny approved these changes Oct 1, 2024

View reviewed changes

davidfowl approved these changes Oct 1, 2024

View reviewed changes

Rename Auth methods to spell out Authentication

2c11449

eerhardt enabled auto-merge (squash) October 1, 2024 15:19

eerhardt merged commit 00ae9c6 into dotnet:main Oct 1, 2024
9 checks passed

eerhardt mentioned this pull request Oct 1, 2024

Add interfaces to PostgreSQL, Redis, and SqlServer Hosting Resources #6055

Open

eerhardt deleted the PostgresManagedIdentity branch October 1, 2024 19:18

github-actions bot locked and limited conversation to collaborators Nov 1, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Azure PostgreSQL managed identity #5930

Add Azure PostgreSQL managed identity #5930

eerhardt commented Sep 25, 2024 •

edited

Loading

davidfowl commented Sep 28, 2024 •

edited

Loading

mitchdenny commented Sep 30, 2024

mitchdenny commented Sep 30, 2024

davidfowl commented Sep 30, 2024

eerhardt commented Sep 30, 2024

eerhardt commented Sep 30, 2024

eerhardt commented Sep 30, 2024

davidfowl Sep 30, 2024

eerhardt Oct 1, 2024

mitchdenny commented Oct 1, 2024

mitchdenny Oct 1, 2024

eerhardt Oct 1, 2024

eerhardt Oct 1, 2024

davidfowl Oct 1, 2024

eerhardt Oct 1, 2024

eerhardt Oct 1, 2024

philon-msft Oct 1, 2024

tg-msft Oct 1, 2024

eerhardt Oct 1, 2024

philon-msft Oct 1, 2024

mitchdenny commented Oct 1, 2024

davidfowl left a comment

eerhardt commented Oct 1, 2024

	postgres.AuthConfig = new PostgreSqlFlexibleServerAuthConfig()
	{
	ActiveDirectoryAuth = PostgreSqlFlexibleServerActiveDirectoryAuthEnum.Enabled,
	PasswordAuth = PostgreSqlFlexibleServerPasswordAuthEnum.Disabled
	};

Add Azure PostgreSQL managed identity #5930

Add Azure PostgreSQL managed identity #5930

Conversation

eerhardt commented Sep 25, 2024 • edited Loading

Description

Checklist

Microsoft Reviewers: Open in CodeFlow

davidfowl commented Sep 28, 2024 • edited Loading

mitchdenny commented Sep 30, 2024

mitchdenny commented Sep 30, 2024

davidfowl commented Sep 30, 2024

eerhardt commented Sep 30, 2024

eerhardt commented Sep 30, 2024

eerhardt commented Sep 30, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mitchdenny commented Oct 1, 2024

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

mitchdenny commented Oct 1, 2024

davidfowl left a comment

Choose a reason for hiding this comment

eerhardt commented Oct 1, 2024

eerhardt commented Sep 25, 2024 •

edited

Loading

davidfowl commented Sep 28, 2024 •

edited

Loading