FoundatioFx / Foundatio

Pluggable foundation blocks for building distributed apps.
Apache License 2.0
1.99k stars 244 forks source link

Upgrade Deprecated Azure SDK Clients #277

Open pinkfloydx33 opened 2 years ago

pinkfloydx33 commented 2 years ago

The current Azure-backed Foundatio libraries are using the older, deprecated Azure SDK clients. For example Queues are using CloudQueue instead of the much newer QueueClient.

More generally you are making using of the Microsoft.* prefixed packages, instead of the newer Azure.* prefixed ones.

Repository Package Replacement
Foundatio.AzureStorage Microsoft.Azure.Storage.Blob Azure.Storage.Blobs
Foundatio.AzureStorage Microsoft.Azure.Storage.Queue Azure.Storage.Queues
Foundatio.AzureServiceBus Microsoft.Azure.ServiceBus Azure.Messaging.ServiceBus
Foundatio.AzureServiceBus Microsoft.Azure.Management.ServiceBus Azure.ResourceManager.ServiceBus

This matters for many reasons. Here's some of them:

If our projects are already using the new (Azure.*) clients, then we have a few other problems. Use of any of the Azure-backed Foundatio projects now requires that we reference both sets of SDKs which of course increases the size of the published applications. But worse (IMO) we must configure the clients using totally different types of settings and manners of configuration.

For us, the latter case is actually the major driving force behind this issue. We are leveraging Azure Federated credentials (via Azure Workload Identity) to use Kubernetes Service Accounts to authenticate as managed identities. This is great, however it means we cannot use Foundatio's Azure libraries since they use the older identity packages and don't recognize new-style connection information. We'd have to maintain an extra connection string with credentials which defeats the purpose of using Azure Workload.

There is already a pull request in the Foundatio.AzureStorage repository from December 2020 that moved this library over to the Azure.* prefixed clients. It seems to be in permanent limbo.

There doesn't appear to be an official issue tracking that migration (just the PR) and as this actually impacts ServiceBus as well, I felt it made more sense to open the issue here (rather than in each repository). Apologies in advanced if I should have created multiple issues.

niemyjski commented 2 years ago

I haven't had time to finish that pr, if you could pick up where I left off that would be awesome. we'd want to support both identity options both passwordless and connection string. Would you please take over upgrading this for me?

pinkfloydx33 commented 2 years ago

I'd be more than happy to contribute but likely cannot do anything until the coming weekend. I'd also want to ensure that Servicebus was upgraded as well so that they match.

Password-less still use a connection string--it's just a different form of it. How much it supports under the hood is dependent upon which version of Azure.Identity gets used.

Originally I was going to suggest we take the dependency on Ms.E.Azure to support injecting queues/blobs/etc via the factory interfaces. But I think doing so would inadvertently restrict how the library is used.

That said, I think we need to consider how to ensure this library can play well with the extensions library. If we don't reference it ourselves then we should probably change the queue/blob/etc option builders to accept either

This way the user can inject pre-configured clients that adhere to things like AddAzureClients and ConfigureDefaults (for retries, logging, metrics, etc) without this library munging them. These would obviously take priority over the other options (if both were provided). It would give the user freedom to use either mechanism, continuing to allow Foundatio full control if they so choose...albeit at the cost of a little complexity here, which I personally think is worth it.

Then all we'd need to do is provide examples of how to use the azure factories + DI factory registrations to configure Foundatio for anyone leveraging such things.

What do you think?

niemyjski commented 2 years ago

If anything, I'd make it additive to what we already do, you could create a third constructor to take those concrete arguments as it doesn't really feel idiomatic to do it via options.

pinkfloydx33 commented 2 years ago

Sounds good to me

niemyjski commented 2 years ago

@pinkfloydx33 have you been able to make any progress on this?

pinkfloydx33 commented 2 years ago

Hi @niemyjski, I got busy and completely forgot about this. We've gone a different route with our application and no longer need the functionality, making it a much lower priority for me. I still believe it's worthwhile though and would contribute if only I had some free time :-(