aspnetcore [release/2.1] Turn on Component Governance

Mar 25 '22 22:03 wtgodbe

Hi @wtgodbe. If this is not a tell-mode PR, please make sure to follow the instructions laid out in the servicing process document. Otherwise, please add tell-mode label.

Mar 25 '22 22:03 ghost

Pushed update to run CompGov in every (internal) step, new build: https://dev.azure.com/dnceng/internal/_build/results?buildId=1685978&view=results

Mar 28 '22 16:03 wtgodbe

Just pushed an update to resolve non-template bugs & enable ProjectReferences globally, @dougbu PTAL

Build: https://dev.azure.com/dnceng/internal/_build/results?buildId=1689034&view=results

Mar 29 '22 21:03 wtgodbe

Changes appear incomplete

D:\a\_work\1\s\src\Servers\IIS\IISIntegration\src\Microsoft.AspNetCore.Server.IISIntegration.csproj : error NU1605: Detected package downgrade: System.Buffers from 4.5.1 to 4.5.0. Reference the package directly from the project to select a different version.  [C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\IDE\CommonExtensions\Microsoft\NuGet\NuGet.targets]
D:\a\_work\1\s\src\Servers\IIS\IISIntegration\src\Microsoft.AspNetCore.Server.IISIntegration.csproj : error NU1605:  Microsoft.AspNetCore.Server.IISIntegration -> Microsoft.AspNetCore.Http.Extensions -> System.Buffers (>= 4.5.1)  [C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\IDE\CommonExtensions\Microsoft\NuGet\NuGet.targets]
D:\a\_work\1\s\src\Servers\IIS\IISIntegration\src\Microsoft.AspNetCore.Server.IISIntegration.csproj : error NU1605:  Microsoft.AspNetCore.Server.IISIntegration -> System.Buffers (>= 4.5.0) [C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\IDE\CommonExtensions\Microsoft\NuGet\NuGet.targets]

Mar 29 '22 22:03 dougbu

Changes appear incomplete

I think we also need to turn UseLatestPackageReferences on globally - projects that aren't patching will use whatever's in the baseline (might not be latest), while projects that are patching (or test projects) will use latest, leading to conflicts

Mar 29 '22 22:03 wtgodbe

we also need to turn UseLatestPackageReferences on globally

This feels like a sledgehammer when a small hammer would have done the trick. First, the external versions from the baselines were fine before; why aren't they now❔ Second, the reported problem was specific to a single project.

Mar 29 '22 23:03 dougbu

This feels like a sledgehammer when a small hammer would have done the trick. First, the external versions from the baselines were fine before; why aren't they now❔ Second, the reported problem was specific to a single project.

In this case, it looks like the issue is because the BaselinePackageReference to System.Buffers in Microsoft.AspNetCore.Server.IISIntegration is 4.5.0, whereas in Microsoft.AspNetCore.Http.Extensions it's 4.5.1. Since the reference to Microsoft.AspNetCore.Http.Extensions is now a ProjectReference instead of a PackageReference, we pick up the newer 4.5.1 transitive reference. I think this would hit any project A that References external package X, and references repo project B that also references X, where B has patched since there's been an update to X, but A has not. This might just be the only such case currently.

Mar 29 '22 23:03 wtgodbe

This might just be the only such case currently.

For now, please limit the fix to the currently-failing case e.g. set $(UseLatestPackageReferences) to true just in Microsoft.AspNetCore.Server.IISIntegration.csproj. The settings in ResolveReferences.targets are all fallbacks if the project does nothing.

Mar 29 '22 23:03 dougbu

For now, please limit the fix to the currently-failing case e.g. set $(UseLatestPackageReferences) to true just in Microsoft.AspNetCore.Server.IISIntegration.csproj. The settings in ResolveReferences.targets are all fallbacks if the project does nothing.

That seems like a band-aid, if the scenario I described above is accurate then it seems like globally using latest package references would be the right thing - what's the downside of that? Also if the same failure class hit us again down the line, it would be extremely non-obvious to someone else why it's failing & how to fix it

Mar 29 '22 23:03 wtgodbe

That seems like a band-aid, if the scenario I described above is accurate then it seems like globally using latest package references would be the right thing

For release/2.1, latest package references are almost never the right thing to do. We're already making a huge change in enabling project references in servicing and this jumps all the way to "all bets are off".

Please don't layer another huge change on top of the existing one in this PR.

Mar 29 '22 23:03 dougbu

For release/2.1, latest package references are almost never the right thing to do

Could you explain why? The way I see it, enabling latest package references has become a necessary part of completing the major change we're making - doing just ProjectReferences and then applying the band-aid when necessary seems like a halfway solution & error prone

Mar 29 '22 23:03 wtgodbe

My concern here is about greatly expanding the scope of this change. Please add lots of comments pointing people to the targeted fix (much nicer term than Band-Aid 😄)

Mar 29 '22 23:03 dougbu

https://github.com/dotnet/aspnetcore-internal/pull/4034

Mar 30 '22 16:03 wtgodbe

Suspect a number of test sites are having trouble starting. The build logs aren't particularly clear unfortunately.

Mar 30 '22 18:03 dougbu

Now rebased on latest release/2.1. That meant no changes were required in src/SignalR/clients/ts/webdriver-tap-runner

Apr 06 '22 02:04 dougbu

I'm seeing lots of test failures like this in dotnet-watch:

Log Critical[0]: Unable to start Kestrel. System.InvalidOperationException: Dynamic port binding is not supported when binding to localhost. You must either bind to 127.0.0.1:0 or [::1]:0, or both.

Log Critical[0]: Unable to start Kestrel. System.IO.IOException: Failed to bind to address http://[::1]:51810/: address already in use. ---> Microsoft.AspNetCore.Connections.AddressInUseException: Address already in use ---> System.Net.Sockets.SocketException: Address already in use

@halter73 any idea what could be causing this one?

And almost every Identity test is failing on asserts like this one (empty collection when we expect a single): https://github.com/dotnet/aspnetcore/blob/7d4f0f0d276c72bf28d16f6919ab37298a17ed1b/src/Identity/test/Identity.FunctionalTests/LoginTests.cs#L231

@HaoK do you know why this might be happening?

Apr 07 '22 21:04 wtgodbe

Opened https://github.com/dotnet/aspnetcore/pull/41137 to try to see if CI failures are due to a reference resolution problem, or a new functional change the tests are now picking up

Apr 11 '22 17:04 wtgodbe

Ping @halter73 and @HaoK on https://github.com/dotnet/aspnetcore/pull/40885#issuecomment-1092243041 - this isn't manifesting like a reference resolution error, so I'm thinking that these tests are picking up a behavioral change that they weren't before since we're resolving newer references now, though I'm not 100% sure. It's only happening in tests that use test asset projects.

Apr 11 '22 20:04 wtgodbe

so I'm thinking that these tests are picking up a behavioral change that they weren't before since we're resolving newer references now

/fyi @halter73 @HaoK: In release/2.1, all non-implementation projects -- including test asset projects -- should pick up the latest package references and use project references already. See

https://github.com/dotnet/aspnetcore/blob/7d4f0f0d276c72bf28d16f6919ab37298a17ed1b/eng/targets/ResolveReferences.targets#L36
https://github.com/dotnet/aspnetcore/blob/7d4f0f0d276c72bf28d16f6919ab37298a17ed1b/eng/targets/ResolveReferences.targets#L47

That said, @(ProjectReference) items will now use the very latest of the entire transitive closure. Test asset projects may not have picked up newer versions of some indirect references before this change (or that in #41137).

Apr 11 '22 21:04 dougbu

I'm seeing lots of test failures like this in dotnet-watch:

Log Critical[0]: Unable to start Kestrel. System.InvalidOperationException: Dynamic port binding is not supported when binding to localhost. You must either bind to 127.0.0.1:0 or [::1]:0, or both.

Log Critical[0]: Unable to start Kestrel. System.IO.IOException: Failed to bind to address http://[::1]:51810/: address already in use. ---> Microsoft.AspNetCore.Connections.AddressInUseException: Address already in use ---> System.Net.Sockets.SocketException: Address already in use

@halter73 any idea what could be causing this one?

I'm really surprised to see the first exception. That implies dotnet-watch is trying to bind to http://localhost:0 which was never allowed. You can bind to http://127.0.0.1:0 or http://[::1]:0 but not http://localhost:0 you never could.

The http://[::1]:51810 AddressInUseException seems like your standard port conflict. I've seen code try to get around the first issue by first binding to the first available port 127.0.0.1 and then using the same port on http://[::1], but that's not guaranteed to work. That's why Kestrel doesn't allow http://localhost:0.

Are there any stack traces associated with these exceptions? They won't be in Kestrel's critical logs, but this should be thrown out to whatever is starting the host, so hopefully it's being logged there.

Apr 11 '22 21:04 halter73

Log Critical[0]: Unable to start Kestrel. System.InvalidOperationException: Dynamic port binding is not supported when binding to localhost. You must either bind to 127.0.0.1:0 or [::1]:0, or both.
     at Microsoft.AspNetCore.Server.Kestrel.Core.LocalhostListenOptions..ctor(Int32 port) in /_/src/Servers/Kestrel/Core/src/LocalhostListenOptions.cs:line 22
     at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.AddressBinder.ParseAddress(String address, Boolean& https) in /_/src/Servers/Kestrel/Core/src/Internal/AddressBinder.cs:line 141
     at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.AddressBinder.AddressesStrategy.BindAsync(AddressBindContext context) in /_/src/Servers/Kestrel/Core/src/Internal/AddressBinder.cs:line 252
     at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.AddressBinder.BindAsync(IServerAddressesFeature addresses, KestrelServerOptions serverOptions, ILogger logger, Func`2 createBinding) in /_/src/Servers/Kestrel/Core/src/Internal/AddressBinder.cs:line 46
     at Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServer.StartAsync[TContext](IHttpApplication`1 application, CancellationToken cancellationToken) in /_/src/Servers/Kestrel/Core/src/KestrelServer.cs:line 158

  Log Critical[0]: Unable to start Kestrel. System.IO.IOException: Failed to bind to address http://127.0.0.1:55464: address already in use. ---> Microsoft.AspNetCore.Connections.AddressInUseException: Error -4091 EADDRINUSE address already in use ---> Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal.Networking.UvException: Error -4091 EADDRINUSE address already in use
     at Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal.Networking.LibuvFunctions.ThrowError(Int32 statusCode) in /_/src/Servers/Kestrel/Transport.Libuv/src/Internal/Networking/LibuvFunctions.cs:line 83
     at Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal.Networking.UvTcpHandle.GetSockIPEndPoint() in /_/src/Servers/Kestrel/Transport.Libuv/src/Internal/Networking/UvTcpHandle.cs:line 72
     at Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal.Listener.ListenTcp(Boolean useFileHandle) in /_/src/Servers/Kestrel/Transport.Libuv/src/Internal/Listener.cs:line 68
     at Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal.Listener.<>c.<StartAsync>b__8_0(Listener listener) in /_/src/Servers/Kestrel/Transport.Libuv/src/Internal/Listener.cs:line 36
     at Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal.LibuvThread.DoPostWork() in /_/src/Servers/Kestrel/Transport.Libuv/src/Internal/LibuvThread.cs:line 374
  --- End of stack trace from previous location where exception was thrown ---
     at Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal.ListenerPrimary.StartAsync(String pipeName, Byte[] pipeMessage, IEndPointInformation endPointInformation, LibuvThread thread) in /_/src/Servers/Kestrel/Transport.Libuv/src/Internal/ListenerPrimary.cs:line 62
     at Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal.LibuvTransport.BindAsync() in /_/src/Servers/Kestrel/Transport.Libuv/src/Internal/LibuvTransport.cs:line 100
     --- End of inner exception stack trace ---
     at Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal.LibuvTransport.BindAsync() in /_/src/Servers/Kestrel/Transport.Libuv/src/Internal/LibuvTransport.cs:line 118
     at Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServer.<>c__DisplayClass22_0`1.<<StartAsync>g__OnBind|0>d.MoveNext() in /_/src/Servers/Kestrel/Core/src/KestrelServer.cs:line 155
  --- End of stack trace from previous location where exception was thrown ---
     at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.AddressBinder.BindEndpointAsync(ListenOptions endpoint, AddressBindContext context) in /_/src/Servers/Kestrel/Core/src/Internal/AddressBinder.cs:line 104
     --- End of inner exception stack trace ---
     at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.AddressBinder.BindEndpointAsync(ListenOptions endpoint, AddressBindContext context) in /_/src/Servers/Kestrel/Core/src/Internal/AddressBinder.cs:line 108
     at Microsoft.AspNetCore.Server.Kestrel.Core.LocalhostListenOptions.BindAsync(AddressBindContext context) in /_/src/Servers/Kestrel/Core/src/LocalhostListenOptions.cs:line 45
     at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.AddressBinder.AddressesStrategy.BindAsync(AddressBindContext context) in /_/src/Servers/Kestrel/Core/src/Internal/AddressBinder.cs:line 252
     at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.AddressBinder.BindAsync(IServerAddressesFeature addresses, KestrelServerOptions serverOptions, ILogger logger, Func`2 createBinding) in /_/src/Servers/Kestrel/Core/src/Internal/AddressBinder.cs:line 46
     at Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServer.StartAsync[TContext](IHttpApplication`1 application, CancellationToken cancellationToken) in /_/src/Servers/Kestrel/Core/src/KestrelServer.cs:line 158

Apr 11 '22 21:04 wtgodbe

I guess part of the stack trace is in Kestrel's logs, but not the part I'm interested in. I'm interested in where in dotnet-watch the host is being started and why it's binding to invalid and in-use addresses.

Apr 11 '22 21:04 halter73

The build output shows those kestrel failures are happening for (at least) Libuv.BindTests and Sockets.BindTests but I don't see those in the test tab - not sure why not. I might have been wrong about the failures being in the dotnet watch tests

Apr 11 '22 21:04 wtgodbe

Actually, I see the same Kestrel failures in a recent green CI run of 2.1, so that looks like a red herring. The Identity failures appear real though, and are the main test failure case in this PR (CC @HaoK)

https://dev.azure.com/dnceng/public/_build/results?buildId=1701788&view=logs&j=f31c9f97-4411-58e7-49ac-fc73f645e6b6&t=2bcaa12b-2f4b-5b1f-c519-10308f653190&l=3440

Apr 11 '22 21:04 wtgodbe

Looks like all the MVC style functional tests are broken, AzureAD + Identity functionals, when was the last time these tests were green? I don't see any changes in the code, so did we change any infrastructure since?

Apr 11 '22 23:04 HaoK

Assert.Single(emailSender.SentEmails);

Do you have a log or a trace that you can point me to for an example so I can see if anything jumps out?

Apr 11 '22 23:04 HaoK

I think the root issue is actually with 404's being returned from razor class libraries, so I see these 2 failures:

Microsoft.AspNetCore.Mvc.FunctionalTests.RazorPagesWithBasePathTest.PagesFromClassLibraries_CanBeServed

Both AzureAD And identity UI use razor pages with class libraries, so it would make sense that if those are broken on this build and always return 404, that's why all of these tests would fail

Apr 11 '22 23:04 HaoK

The failures started on this PR, likely since we're changing UseProjectReferences to be globally true to satisfy component governance - my theory is that's leading tests to pick up new behavior that they weren't before, causing the test failures. The other possibility is a reference resolution issue, though I haven't found anything in the logs that is a smoking gun for that.

Apr 11 '22 23:04 wtgodbe

I'm not sure how razor libraries work underneath the covers maybe @javiercn has some insight why use project references would cause them to break?

Apr 11 '22 23:04 HaoK

@TanayParikh could you take a look at the failures here since they appear to be related to Razor?

Apr 12 '22 16:04 wtgodbe