dotnet / aspire

Tools, templates, and packages to accelerate building observable, production-ready apps
https://learn.microsoft.com/dotnet/aspire
MIT License
3.86k stars 462 forks source link

Dashboard should use X509CertificateLoader instead of obsolete X509Certificate2 constructor #5461

Open eerhardt opened 2 months ago

eerhardt commented 2 months ago

Is there an existing issue for this?

Describe the bug

https://github.com/dotnet/aspire/blob/f5d9f80c3c667f5c84debfd427e7b0ae83493f7d/src/Aspire.Dashboard/ResourceService/DashboardClient.cs#L162-L172

new X509Certificate2 is obsolete in net9.0. Trying to target the dashboard to net9.0 gives a warning here.

See:

Expected Behavior

We should be able to target net9.0 without obsolete warnings.

Steps To Reproduce

No response

Exceptions (if any)

No response

.NET Version info

No response

Anything else?

cc @drewnoakes @JamesNK

drewnoakes commented 2 months ago

X509CertificateLoader is unavailable in .NET 8 so this work will have to wait until the upgrade commences.

We currently use this constructor to load a user-specified certificate. The new APIs make assumptions about the format of those certificates, so we'll need to use GetCertContentType to determine the format and call the appropriate non-obsolete API to load a cert.

There's another usage in test code that should be simpler to address.

drewnoakes commented 2 months ago

The new APIs make assumptions about the format of those certificates, so we'll need to use GetCertContentType to determine the format and call the appropriate non-obsolete API to load a cert.

Looking at https://github.com/dotnet/runtime/pull/104165, this statement may not be correct. It may be enough to just call X509CertificateLoader.LoadCertificate.

Either way this requires upgrading projects to net9.0.

eerhardt commented 2 months ago

X509CertificateLoader is unavailable in .NET 8 so this work will have to wait until the upgrade commences.

Either way this requires upgrading projects to net9.0.

This type is being shipped for downlevel in https://www.nuget.org/packages/Microsoft.Bcl.Cryptography. So we should just be able to reference that package, and stay targeting net8.0.

(A quick "how do you figure this out" note: if you search for the api on https://apisof.net/, it shows a table of which TFMs the API is available. If the API is "in box" the icon is dark purple. If the API is supported by a nuget package, it is shaded. Clicking on the icon will tell you which NuGet package the API ships in.)

drewnoakes commented 1 month ago

Conclusion from feedback on #5688 by @bartonjs is that we don't need to migrate until the repo moves to targeting net9.0, which isn't happening any time soon. There's some good discussion on that PR that'll be useful as and when that move occurs.