Skip to content
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

Version 6.5.1 adds an unexpected dependency on the Microsoft.WindowsDesktop.App Runtime. #516

Closed
rarowston opened this issue Jan 31, 2024 · 13 comments
Labels

Comments

@rarowston
Copy link

Please clearly describe what the SQL Sink is doing incorrectly:

Updating from version 6.5.0 to version 6.5.1 adds an unexpected dependency on the Microsoft.WindowsDesktop.App Runtime.

Deploying the application to the server after upgrading the version causes the application to fail to start. Some digging revealed that the failure is due to a missing runtime. My testing seems to indicate that it is this specific version bump that added this dependency.

Please clearly describe the expected behaviour:

Version 6.5.1 can run successfully using only the Microsoft.AspNetCore.App and Microsoft.NETCore.App runtimes.
Failing that, if this runtime dependency is required, a note in the release notes/readme to warn others that this is now a dependency that they will need to manually install on their hosting environment if they have not already.

List the names and versions of all Serilog packages used in the project:

  • Serilog.Sinks.MSSqlServer
  • Serilog.Settings.Configuration
  • Serilog.AspNetCore

Target framework and operating system:

.NET 8
Windows Server 2012

Author's note:
This is not a critical issue as I can work around it by installing the runtime or not upgrading the version (though there are transitive vulnerabilities with this option). I am mostly posting this to help others more quickly diagnose this issue if they run into the same situation.
Reducing dependencies is a nice-to-have as well.

@ckadluba
Copy link
Member

ckadluba commented Jan 31, 2024

Hi @rarowston,

thank you for reporting this and for your investigation.

We did not change any target framework definitions in the sink library. Also there was only one direct dependency which was upgraded from release 6.5.0 to 6.5.1 which was to fix a security vulnerability. This dependency is Microsoft.Data.SqlClient and it was upgraded from 5.0.1 to 5.1.4 (dotnet/SqlClient@v5.0.1...v5.1.4#diff-2d39be8d043bee88a5034519b11cd2b45b0117b3baa8528f11def532bcdfe611).

This needs furhter investigation.

@ckadluba
Copy link
Member

@rarowston what error message do you get when the application fails to start?

@jonorossi
Copy link
Member

The bug (AzureAD/microsoft-authentication-library-for-dotnet#4468) is in Microsoft.Identity.Client used by SqlClient. The library assumes *-windows TFM means desktop Windows.

@Kolthor
Copy link

Kolthor commented Jan 31, 2024

We recently had the same issue after upgrading some NuGet packages. We didn't investigate it so far as to find out which package introduced the problem, but it was when this package was 6.5.0, and it is very possible that we used a more recent SqlClient than 6.5.0 required.

What we did to solve it, was changing the target framework in our projects from net6.0-windows to net6.0.

Even updating the dotnet hosting bundle on the server, didn't solve the problem.

@ckadluba
Copy link
Member

ckadluba commented Jan 31, 2024

@jonorossi thanks for the hint. SqlClient even had their own issue regarding this (dotnet/SqlClient#2298) which is now closed because it has to be fixed in MSAL (with reference to the bug you posted).

So i thing we have to wait for that if it is not a critical issue for the MSSSQL sink.

@ckadluba ckadluba added the bug label Jan 31, 2024
@rarowston
Copy link
Author

Well found @jonorossi
The MSAL issue does indeed seem to be the root cause of this problem and I have been able to "fix" it with the workaround of removing the specific windows target. This adds a lot of warnings as there are some windows only functions in use, but this does work.
It looks like they've added it to a milestone for Feb 8th (https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/milestone/124) so hopefully they add a fix soon and this can cascade back up.

@SimonCropp
Copy link

for context here is the related PR AzureAD/microsoft-authentication-library-for-dotnet#4567

@ckadluba
Copy link
Member

ckadluba commented Feb 5, 2024

Very good. Thank you!

@rarowston
Copy link
Author

rarowston commented Jun 13, 2024

The PR that fixes the root problem of this has been merged in and released, though It will likely take a while for this to cascade up through all affected packages. Eventually I suspect this will be fixed with a package update.

In the meantime, for the benefit of anyone else in the same position, I can now fix this issue by directly including a newer version of Azure.Identity.

@ckadluba
Copy link
Member

According to this https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/releases/tag/4.61.0 this should be fixed in Microsoft.Client.Identity 4.61.0 which we just upgraded by using SqlClient 5.1.6 in our latest dev release:

https://www.nuget.org/packages/Serilog.Sinks.MSSqlServer/6.7.1-dev-00084

If anyone who experienced this issue could try it out and give us some feedback, we would highly appreciate this.

@ckadluba
Copy link
Member

Fixed in release 6.7.1 #555

@rarowston
Copy link
Author

I have updated to 6.7.1 and removed specific version references to the Azure Identity package (which was the workaround while waiting for this fix to cascade all the way up).

I can confirm that I no longer have the issue after this update, so this indeed appears to be fully resolved.

Thanks for all your help

@ckadluba
Copy link
Member

@rarowston thank you very much for your help and the nice words. I'm glad we could solve this issue. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants