Add support for ASP.NET Core 3.0 to Amazon.Lambda.AspNetCoreServer #508

normj · 2019-08-19T07:11:28Z

Issue #, if available:
#481

Description of changes:
Add support for ASP.NET Core 3.0 to Amazon.Lambda.AspNetCoreServer. .NET Core 3.0 will not be an LTS and so will not be supported in Lambda as a built in runtime. It is possible to use .NET Core 3.0 with the Amazon.Lambda.RuntimeSupport NuGet package and Lambda custom runtimes. Check out the following blog post on how to use it. https://aws.amazon.com/blogs/developer/announcing-amazon-lambda-runtimesupport/

For example on how to use Amazon.Lambda.RuntimeSupport with ASP.NET Core 3.0 check out this example. https://github.com/normj/LambdaNETCoreSamples The sample contains a prebuilt NuGet package of Amazon.Lambda.AspNetCoreServer with this PR in the CustomRuntimeAspNetCore30/nuget-cache directory.

Breaking Changes

This PR will be a major version bump as there will be breaking changes to that will affect .NET Core 2.1 ASP.NET Core Lambda function's as well. The breaking change only affect users that override any of the Post methods to customize serialization.

The cause of the breaking changes is Microsoft.AspNetCore.Hosting.Internal.HostingApplication.Context was exposed in some of the virtual methods to customize the serialization. Even though the object was public the ASP.NET team did put it in an Internal namespace and so this library should never have directly used it let alone expose it in some of the virtual methods. In ASP.NET Core 3.0 HostingApplication.Context was moved to Microsoft.AspNetCore.Hosting namespace and marked as internal.

Breaking Change List:

protected method CreateContext now returns back object
protected method ProcessRequest now passes in context as an object
protected virtual method PostCreateContext was removed
- To customize HttpContext's User property override the PostMarshallHttpAuthenticationFeature method.
- To customize the items in HttpContext's Items collection override PostMarshallItemsFeatureFeature method.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

… old pubternal classes that are now internal.

…and netcoreapp3.0. The 3.0 tests are currently not passing.

…ne with it

…shallHttpAuthenticationFeature

davidfowl · 2019-08-19T13:12:33Z

@Tratcher can you review and give feedback please

normj · 2019-08-19T23:22:03Z

@davidfowl @Tratcher That would be greatly appreciated.

Tratcher · 2019-08-22T20:54:41Z

Libraries/src/Amazon.Lambda.AspNetCoreServer/APIGatewayProxyFunction.cs

@@ -135,6 +102,40 @@ private protected override void InternalCustomResponseExceptionHandling(HostingA
        /// <param name="lambdaContext"></param>
        protected override void MarshallRequest(InvokeFeatures features, APIGatewayProxyRequest apiGatewayRequest, ILambdaContext lambdaContext)
        {
+            {
+                var authFeatures = (IHttpAuthenticationFeature) features;


This is a weird style to work with the concrete InvokeFeatures type but keep downcasting to the individual features. It'd be cleaner if you removed the explicit implementation prefix. E.g. IHttpAuthenticationFeature.User { get; set; } -> User { get; set; }.

Alternatively, change the APIs to take the abstract IFeatureCollection and call Get<IHttpAuthenticationFeature>() when you want to work with a specific feature. This might not quite match your usage here as you are the one populating the initial values and you know exactly which features are implemented.

I agree it is weird but at this point I'm not ready to change the design. I actually think I got this from the ASP.NET Core repo back in the 1.0 days because I wouldn't have thought to do this way.

Tratcher · 2019-08-22T21:00:08Z

Libraries/src/Amazon.Lambda.AspNetCoreServer/AbstractAspNetCoreFunction.cs

@@ -224,7 +228,7 @@ protected void Start()
        /// Creates a <see cref="HostingApplication.Context"/> object using the <see cref="LambdaServer"/> field in the class.


The IServer check above could be done more effectively between Build and Start. If they didn't call UseApiGateway then Start is likely to fail for a variety of false positives (e.g. can't start the default Kestrel server).

I moved the call to UseLambdaServer to after the point the end user could customize the IWebHostBuilder. This also pointed out that the comment should say UseLambdaServer since UseApiGateway is obsolete

Tratcher · 2019-08-22T21:05:11Z

Libraries/src/Amazon.Lambda.AspNetCoreServer/ApplicationLoadBalancerFunction.cs

@@ -45,6 +45,14 @@ protected ApplicationLoadBalancerFunction(StartupMode startupMode)
        /// <inheritdoc/>
        protected override void MarshallRequest(InvokeFeatures features, ApplicationLoadBalancerRequest lambdaRequest, ILambdaContext lambdaContext)
        {
+            {
+                var requestFeatures = (IHttpAuthenticationFeature) features;


Is the downcast even required here? That should be implicit.

Tratcher · 2019-08-28T20:38:04Z

Libraries/src/Amazon.Lambda.AspNetCoreServer/AbstractAspNetCoreFunction.cs

+
+            {
+                var itemFeatures = (IItemsFeature) features;
+                itemFeatures.Items = new Dictionary<object, object>();


We use a customized dictionary that doesn't throw KeyNotFoundException for any missing elements. Much easier for developers to consume.
https://github.com/aspnet/AspNetCore/blob/ab02951b37ac0cb09f8f6c3ed0280b46d89b06e0/src/Http/Http/src/Internal/ItemsDictionary.cs#L23-L24

Since ItemsDictionary is an internal I have copied the code into this repo to give the same experience.

Tratcher · 2019-08-28T20:42:19Z

Libraries/src/Amazon.Lambda.AspNetCoreServer/Internal/InvokeFeatures.cs

+            long? count,
+            CancellationToken cancellationToken = default(CancellationToken))
+        {
+            throw new NotImplementedException("SendFileAsync is not implemented for Serverless ASP.NET Core functions");


Several middleware depend on this. A default implementation is available here: https://github.com/aspnet/AspNetCore/blob/ab02951b37ac0cb09f8f6c3ed0280b46d89b06e0/src/Http/Http/src/SendFileFallback.cs#L22

Thanks, I have copied this default implementation.

Tratcher · 2019-08-28T20:42:56Z

Libraries/src/Amazon.Lambda.AspNetCoreServer/Internal/InvokeFeatures.cs

+            throw new NotImplementedException("SendFileAsync is not implemented for Serverless ASP.NET Core functions");
+        }
+
+        Task IHttpResponseBodyFeature.StartAsync(CancellationToken cancellationToken = default(CancellationToken))


This should at least trigger OnStarting events.

OnStarting events are triggered in the AbstractAspNetCoreFunction.ProcessRequest method. The starting and complete events are a little weird in this context because they are only triggered before and after the response from ASP.NET Core is marshaled into the Lambda request. Nothing is sent to the client until the function is complete and the Lambda response is returned to either API Gateway or Application Load Balancer.

…s://github.com/CasperSkydt/aws-lambda-dotnet into aspnetcore30-support # Conflicts: # Libraries/src/Amazon.Lambda.AspNetCoreServer/APIGatewayProxyFunction.cs

…da.Tools

…ct output stream

normj · 2019-10-24T04:12:30Z

Manually merged into dev

normj and others added 7 commits June 24, 2019 17:19

Add ASP.NET 3.0 support to Amazon.Lambda.AspNetCoreServer

08f4da2

Changed LambdaContextLogger to use logger instead

b0461b6

Use IItemsFeature and IHttpAuthenticationFeature instead of using the…

de553fc

… old pubternal classes that are now internal.

Setup Amazon.Lambda.AspNetCoreServer tests to run both netcoreapp2.1 …

f87adb7

…and netcoreapp3.0. The 3.0 tests are currently not passing.

Implement IHttpResponseBodyFeature for .NET Core 3.0

7f51269

Fixed bug of incorrectly disposing HttpContext object before being do…

e3b04f1

…ne with it

Fixed up method names for PostMarshallItemsFeatureFeature and PostMar…

8ccabce

…shallHttpAuthenticationFeature

normj mentioned this pull request Aug 19, 2019

Support ASP.NET Core 3.0 with Amazon.Lambda.AspNetCoreServer #481

Closed

Tratcher reviewed Aug 28, 2019

View reviewed changes

normj added 5 commits September 17, 2019 23:03

Merge branch 'feature-use-logger-instead-lambdacontextlogger' of http…

f84cf51

…s://github.com/CasperSkydt/aws-lambda-dotnet into aspnetcore30-support # Conflicts: # Libraries/src/Amazon.Lambda.AspNetCoreServer/APIGatewayProxyFunction.cs

Responding to CR comments for ASP.NET Core 3.0 support in Amazon.Lamb…

f532103

…da.Tools

Use floating version number for FSharp.Core in test project

dd4cdde

Fix issue with IHttpResponseBodyFeature.SendFileAsync not using corre…

08be0bb

…ct output stream

Bump version number of Amazon.Lambda.AspNetCoreServer to 4.0.0-preview2

f647d3a

normj closed this Oct 24, 2019

normj deleted the aspnetcore30-support branch December 19, 2019 07:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for ASP.NET Core 3.0 to Amazon.Lambda.AspNetCoreServer #508

Add support for ASP.NET Core 3.0 to Amazon.Lambda.AspNetCoreServer #508

normj commented Aug 19, 2019

davidfowl commented Aug 19, 2019

normj commented Aug 19, 2019

Tratcher Aug 22, 2019

normj Sep 18, 2019

Tratcher Aug 22, 2019

normj Sep 18, 2019

Tratcher Aug 22, 2019

normj Sep 18, 2019

Tratcher Aug 28, 2019

normj Sep 18, 2019

Tratcher Aug 28, 2019

normj Sep 18, 2019

Tratcher Aug 28, 2019

normj Sep 18, 2019

normj commented Oct 24, 2019

		@@ -224,7 +228,7 @@ protected void Start()
		/// Creates a <see cref="HostingApplication.Context"/> object using the <see cref="LambdaServer"/> field in the class.

Add support for ASP.NET Core 3.0 to Amazon.Lambda.AspNetCoreServer #508

Add support for ASP.NET Core 3.0 to Amazon.Lambda.AspNetCoreServer #508

Conversation

normj commented Aug 19, 2019

Breaking Changes

davidfowl commented Aug 19, 2019

normj commented Aug 19, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

normj commented Oct 24, 2019