.NET 8 Upgrade (Multi Framework targetting) #2181

seantleonard · 2024-04-22T18:45:55Z

Why make this change?

Closes .NET 8 upgrade #2140

What is this change?

Updates all .csproj files to target both .NET8 and .NET6. (source: How to multi target

    <TargetFrameworks>net8.0;net6.0</TargetFrameworks>

Update Directory.Packages.Props to include framework specific sections targeting dependency versions compatible with .NET6
- Ref: https://josef.codes/multi-targeting-your-nuget-packages/

<ItemGroup Condition="'$(TargetFramework)' == 'net8.0'">
  <PackageVersion Include="Microsoft.AspNetCore.Authorization" Version="8.0.4" />
</ItemGroup>
<ItemGroup Condition="'$(TargetFramework)' == 'net6.0'">
  <PackageVersion Include="Microsoft.AspNetCore.TestHost" Version="6.0.29" />
</ItemGroup>

Added framework specific constructors for EasyAuth/Simulator AuthenticationHandlers because ISystemClock was deprecated in .NET8 and is a required constructor parameter when overriding AuthenticationHandler dotnet class used for custom authentication handlers.
- https://learn.microsoft.com/en-us/dotnet/core/compatibility/aspnet-core/8.0/isystemclock-obsolete
- Multi target dotnet project && code
Updated instances of pulling client role header value (GraphQLFilterParser, SqlMutationEngine) due to stricter nullability checks (now errors) in .NET 8. The following original code "possibly returns null" and needs to be handled.

GetHttpContext().Request.Headers[AuthorizationResolver.CLIENT_ROLE_HEADER];

Changed to be a conditional check:

if (!GetHttpContext().Request.Headers.TryGetValue(AuthorizationResolver.CLIENT_ROLE_HEADER, out StringValues headerValues) && headerValues.Count != 1)

Due to stricter .NET 8 nullability checks, DabCacheService specification of return type now specifies nullable jsonElement? to be consistent with method signature.

async (FusionCacheFactoryExecutionContext<JsonElement?> ctx, CancellationToken ct) =>

Startup class also gets a fix for stricter nullability when processing config file name. Previously a null value was possible according to the compiler:
OLD:

string configFileName = Configuration.GetValue<string>("ConfigFileName", FileSystemRuntimeConfigLoader.DEFAULT_CONFIG_FILE_NAME);

NEW

string configFileName = Configuration.GetValue<string>("ConfigFileName") ?? FileSystemRuntimeConfigLoader.DEFAULT_CONFIG_FILE_NAME;

How was this tested?

This isn't a feature addition, but ensure that all existing tests pass.

Integration Tests
Unit Tests

…ues that weren't causing issues in .net6. Also removed "ISystemClock" from authenticationhandlers as it is deprecated in .net8. Internally .net8 provides replacement so only change on our part was removing the param. all references to headerdictionary use trygetvalue and check that result count is 1 to make sure user didn't supply >1 role. when that is fulfilled , the stringvalues object is converted to string and the nullable error goes away.

Add framework dependent constructors for EasyAuth/SimulatorAuthenticationHandlers. Directory.Packages.Props now has framework dependent section to define separate package versions.

JerryNixon · 2024-04-24T15:17:07Z

We know there are performance improvements from this even if we do not measure for them. This includes improvements to JIT compilation and reduced memory consumption. The .NET team has bragged about this too much for us not to see those benefits coming for free. But the best part is the improved developer experience with the latest C# language features—that's the win!

By the way, .NET 8 is set to be EOL in 2026 (compared to .NET 7 in 2024). The team alternates LTS with only even versions (similar to DAB), which means our next upgrade will be .NET 10, not .NET 9 - which is considered a maintenance version. I bring this up because .NET 9 is right around the corner, and I don't want the team to feel discouraged to see .NET 9 coming out at the same time we upgrade to .NET 8. We're upgrading to the right version, LTS version of .NET, and it's perfect.

…s from main

seantleonard · 2024-05-02T21:27:34Z

Issue: dotnet/aspnetcore#53332

System.InvalidOperationException: Unable to resolve service for type 'Microsoft.AspNetCore.Routing.EndpointDataSource' while attempting to activate 'Microsoft.AspNetCore.Authorization.Policy.AuthorizationPolicyCache'.

Present in ClientRoleHeaderAuthorizationMiddlewareTests

Aniruddh25

Waiting for usage of nuget.org

.pipelines/templates/build-pipelines.yml

src/Directory.Packages.props

src/Core/Models/GraphQLFilterParsers.cs

Nuget.config

src/Core/AuthenticationHelpers/AppServiceAuthentication.cs

src/Core/AuthenticationHelpers/ClientRoleHeaderAuthenticationMiddleware.cs

src/Core/AuthenticationHelpers/EasyAuthAuthenticationHandler.cs

Aniruddh25 · 2024-05-02T22:01:54Z

Also, check if we get different binaries for each of the target framework, and if not, would the same binary work for both .net 8/.net6

seantleonard · 2024-05-02T22:39:12Z

Issue: dotnet/aspnetcore#53332

System.InvalidOperationException: Unable to resolve service for type 'Microsoft.AspNetCore.Routing.EndpointDataSource' while attempting to activate 'Microsoft.AspNetCore.Authorization.Policy.AuthorizationPolicyCache'.

Present in ClientRoleHeaderAuthorizationMiddlewareTests

Fixed this via: dotnet/aspnetcore#53332 (comment)

…aimtypes aren't used. (dotnet8 change, but this test modification is backwards compatible per successful "net6 tests".) update usings update notice file generation with latest sqlclient version behind scenes updated nuget dependency versions in our upstream feed. updating build pipeline comments. added mitigating helper function that is backwards compatible with net6 so that we don't get unhandled json exception that halts startup. revert accidental nuget feed change.

seantleonard · 2024-05-03T20:54:53Z

/azp run

Ignore seralize/deserialize unit test due to attempting to serialize when latest .net guidance says not to. Attached github discussion link from dotnet/runtime.

…ta-api-builder into dev/sean/dotnet8upgrade

seantleonard · 2024-05-03T21:10:56Z

/azp run

…nstall to dwsql/mssql windows pipelines. Not just linux.

…nd net8 config files in test project.

seantleonard · 2024-05-03T21:34:35Z

/azp run

…ction string in configs in both net6 and net8 folders.

seantleonard · 2024-05-03T22:06:43Z

/azp run

Dockerfile

src/Service.Tests/Authentication/Helpers/WebHostBuilderHelper.cs

…and net8.0 target frameworks explicitly and then optionally zip all the output. Updated create-manifest-file to accommodate the net8 and net6 multitarget builds.

seantleonard · 2024-05-06T20:07:01Z

/azp run

Aniruddh25

LGTM

src/Service.Tests/Unittests/SerializationDeserializationTests.cs

src/Service.Tests/Authentication/JwtTokenAuthenticationUnitTests.cs

src/Cli.Tests/Cli.Tests.csproj

ayush3797

LGTM!

…t to preserve running on .net6 and apply fix to running on .net8 by removing object converter. After testing , this still works but a better solution may exist. moved Microsoft.Extensions.Configuration.Json/Binder to targetframework specific directory.packages.props section to align dependency versions with dotnetruntime versions. this helped ensure .net6 version of serializationdeserialization test to pass on both runtimes.

seantleonard · 2024-05-09T19:25:37Z

/azp run

seantleonard added 2 commits March 28, 2024 13:13

Update targerframeworks for all projects to include net8 and net6.

f145905

Add framework dependent constructors for EasyAuth/SimulatorAuthenticationHandlers. Directory.Packages.Props now has framework dependent section to define separate package versions.

seantleonard added this to the 1.1rc milestone Apr 22, 2024

seantleonard requested review from mbhaskar, Aniruddh25, Mathos1432, gledis69, aaronburtle, tarazou9, ayush3797, abhishekkumams, aaronpowell, severussundar, ravishetye, rohkhann, neeraj-sharma2592 and sourabh1007 as code owners April 22, 2024 18:45

seantleonard added 3 commits April 22, 2024 11:48

Merge branch 'main' into dev/sean/dotnet8upgrade

1063902

updated formatting and merge conflict errors.

4518169

update build pipeline to use .net8 and add comment.

1ab7ee5

merge main added using in mutation engine and used Dockerfile content…

dea4d4d

…s from main

Aniruddh25 requested changes May 2, 2024

View reviewed changes

seantleonard added 2 commits May 3, 2024 13:50

Merge branch 'main' into dev/sean/dotnet8upgrade

036c7a4

add both net6 and net8 sdks to all pipelines.

903c4da

Ignore seralize/deserialize unit test due to attempting to serialize when latest .net guidance says not to. Attached github discussion link from dotnet/runtime.

Merge branch 'dev/sean/dotnet8upgrade' of https://github.com/Azure/da…

402ad76

…ta-api-builder into dev/sean/dotnet8upgrade

seantleonard added 2 commits May 3, 2024 14:21

ignore correct serializationdeserializationtest and add .net8/6 sdk i…

021d1c3

…nstall to dwsql/mssql windows pipelines. Not just linux.

update pipelines to json substitute connection strings for all net6 a…

e339832

…nd net8 config files in test project.

another attempt to get filetransform task to do json replace of conne…

c6dc14e

…ction string in configs in both net6 and net8 folders.

Aniruddh25 reviewed May 3, 2024

View reviewed changes

Dockerfile Outdated Show resolved Hide resolved

Aniruddh25 reviewed May 3, 2024

View reviewed changes

src/Service.Tests/Authentication/Helpers/WebHostBuilderHelper.cs Show resolved Hide resolved

seantleonard added 2 commits May 6, 2024 12:51

updates the publish.ps1 script to run dotnet publish for both net6.0 …

0e9ad76

…and net8.0 target frameworks explicitly and then optionally zip all the output. Updated create-manifest-file to accommodate the net8 and net6 multitarget builds.

Merge branch 'main' into dev/sean/dotnet8upgrade

2cdc94d

Aniruddh25 approved these changes May 6, 2024

View reviewed changes

src/Service.Tests/Unittests/SerializationDeserializationTests.cs Outdated Show resolved Hide resolved

Aniruddh25 reviewed May 6, 2024

View reviewed changes

src/Service.Tests/Authentication/JwtTokenAuthenticationUnitTests.cs Show resolved Hide resolved

ayush3797 reviewed May 7, 2024

View reviewed changes

src/Cli.Tests/Cli.Tests.csproj Show resolved Hide resolved

ayush3797 approved these changes May 7, 2024

View reviewed changes

seantleonard added 2 commits May 9, 2024 12:11

Merge branch 'main' into dev/sean/dotnet8upgrade

0dffcbf

Aniruddh25 approved these changes May 9, 2024

View reviewed changes

seantleonard merged commit 85323c7 into main May 9, 2024
7 checks passed

seantleonard deleted the dev/sean/dotnet8upgrade branch May 9, 2024 22:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

.NET 8 Upgrade (Multi Framework targetting) #2181

.NET 8 Upgrade (Multi Framework targetting) #2181

seantleonard commented Apr 22, 2024 •

edited

JerryNixon commented Apr 24, 2024

seantleonard commented May 2, 2024

Aniruddh25 left a comment

Aniruddh25 commented May 2, 2024

seantleonard commented May 2, 2024

seantleonard commented May 3, 2024

seantleonard commented May 3, 2024

seantleonard commented May 3, 2024

seantleonard commented May 3, 2024

seantleonard commented May 6, 2024

Aniruddh25 left a comment

ayush3797 left a comment

seantleonard commented May 9, 2024

.NET 8 Upgrade (Multi Framework targetting) #2181

.NET 8 Upgrade (Multi Framework targetting) #2181

Conversation

seantleonard commented Apr 22, 2024 • edited

Why make this change?

What is this change?

How was this tested?

JerryNixon commented Apr 24, 2024

seantleonard commented May 2, 2024

Aniruddh25 left a comment

Choose a reason for hiding this comment

Aniruddh25 commented May 2, 2024

seantleonard commented May 2, 2024

seantleonard commented May 3, 2024

seantleonard commented May 3, 2024

seantleonard commented May 3, 2024

seantleonard commented May 3, 2024

seantleonard commented May 6, 2024

Aniruddh25 left a comment

Choose a reason for hiding this comment

ayush3797 left a comment

Choose a reason for hiding this comment

seantleonard commented May 9, 2024

seantleonard commented Apr 22, 2024 •

edited