Introduce an IServiceCollection extension method for enabling HTTPS configuration

[amcasey]

Background and Motivation

#48956 raised the point that ListenOptions.UseHttps will be unusable in 8.0 for servers constructed without an IWebHostBuilder, since the extension method for registering IHttpsConfigurationService only applies to that type. If we'd prefer to avoid breaking such users, we could introduce a comparable extension method on IServiceCollection - that's all the IWebHostBuilder extension method does anyway.

Proposed API

namespace Microsoft.AspNetCore.Obscure;

public static class HttpsConfigurationServiceCollectionExtensions
{
+    public IServiceCollection UseKestrelHttpsConfiguration(this IServiceCollection serviceCollection);
}

Usage Examples

serviceCollection.UseKestrelHttpsConfiguration();

Alternative Designs

I suppose, hypothetically, we could expose more of the internal HTTPS configuration types directly and let consumers register them as they see fit, but that seems a lot more complicated and harder to maintain.

Risks

Users might be confused about the difference between IWebHostBuilder.UseKestrelHttpsConfiguration and IServiceCollection.UseKestrelHttpsConfiguration.

[ghost]

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

• The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
• The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
• Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

[amcasey]

Obviously, the namespace should not actually be called "Obscure" - that's just there to make the point that it should probably require a special import.

[halter73]

API Review Notes

• Is it possible to have the UseKestrelHttpsConfiguration behavior be the default if you use the legacy KestrelServer ctor (instead of KestrelServerImpl like UseKestrel adds)?
  • Maybe. It would be better if we could avoid the break without forcing people to call a new API. As long as it doesn't affect trimming in the mainline scenarios, this might be okay.
• If we do add this, it's more conventional for IServiceCollection extension methods to start with Add instead of Use.
• We generally don't use namespaces to hide extension methods. If we want to make it less discoverable, we can use a normal static method.

[Tratcher]

Creating kestrel manually from the KestrelServer constructor is the only way to do this without IWebHostBuilder, right? I agree with @halter73, just make that constructor always work, no new API.

[halter73]

Part of the problem is that KestrelServerOptions.Listen(endpoint, listenOptions => listenOptions.UseHttps()) runs inline and needs Kestrel's HTTPS services to be initialized as the KestrelServerOptions is resolved which is before the KestrelServer constructor is called to potentially connect the HTTPS services to the options.

But we probably should still use HttpsConfigurationService rather than SimpleHttpsConfigurationService when constructing a KestrelServerImpl in KestrelServer so that HTTPS still works if the endpoints are loaded from config, or the app developer successfully copies the service descriptors for Kestrel's internal services before building a service provider to resolve the KestrelServerOptions and then manually calls the KestrelServer constructor anyway rather than just resolve the IServer.

aspnetcore/src/Servers/Kestrel/Core/src/KestrelServer.cs

Lines 33 to 39 in ca08b82

	_innerKestrelServer = new KestrelServerImpl(
	options,
	new[] { transportFactory ?? throw new ArgumentNullException(nameof(transportFactory)) },
	Array.Empty<IMultiplexedConnectionListenerFactory>(),
	new SimpleHttpsConfigurationService(loggerFactory),
	loggerFactory,
	new KestrelMetrics(new DummyMeterFactory()));

I think our best option is to obsolete KestrelServer​ and its constructor and point people to CreateSlimBuilder() instead. You also cannot use HTTP/3 or metrics if you use this constructor, so this is even more reason to obsolete it. @JamesNK

Normally, I'd say go to whatever lengths are necessary to avoid breaking changes (which is why may never actually delete the deprecated KestrelServer constructor), but new API doesn't stop the break. The existing code to make UseHttps work with the public KestrelServer constructor already involves hacks like a custom SerpKestrelServerOptionsSetup : IConfigureOptions<KestrelServerOptions>, so I'm not too concerned about how clean the workaround looks.

If customers absolutely do not want to use CreateSlimBuilder(), they also have the option copy the service descriptors from UseKestrel using a custom IWebHostBuilder, and then resolve the IServer. It's still ugly and fragile (which is why we should deprecate KestrelServer), but less so than the custom IConfigureOptions​, and it would have the upside of giving you things like HTTP/3 support if you want it.

If we must add a new API to make adding Kestrel HTTPS services to an IServiceCollection more convenient without an IWebHostBuilder, hiding it in a non-extension static method might help prevent it from causing confusion since it's so rarely needed. I don't think we should add something specifically for just Kestrel's HTTPS-related services like UseKestrelHttpsConfiguration. It should just be AddKestrel (Required services + HTTPS + HTTP/3) and AddKestrelCore (Required services) to align with the "Use"-prefixed IWebHostBuilder counterparts.

[amcasey]

Part of the problem is that KestrelServerOptions.Listen(endpoint, listenOptions => listenOptions.UseHttps()) runs inline and needs Kestrel's HTTPS services to be initialized as the KestrelServerOptions is resolved which is before the KestrelServer constructor is called to potentially connect the HTTPS services to the options.

Yep. KestrelServer has an IHttpsConfigurationService, but it's too late to add it to the service collection and the problem isn't KestrelServer but ListenOptions.UseHttps.

But we probably should still use HttpsConfigurationService rather than SimpleHttpsConfigurationService when constructing a KestrelServerImpl in KestrelServer so that HTTPS still works if the endpoints are loaded from config, or the app developer successfully copies the service descriptors for Kestrel's internal services before building a service provider to resolve the KestrelServerOptions and then manually calls the KestrelServer constructor anyway rather than just resolve the IServer.

Sorry, I didn't follow this. SimpleHttpsConfigurationService is a subset of the real implementation tailored to the ways it can actually be consumed by KestrelServerImpl. I'm not sure what difference providing a fuller implementation would make.

I think our best option is to obsolete KestrelServer​ and its constructor and point people to CreateSlimBuilder() instead. You also cannot use HTTP/3 or metrics if you use this constructor, so this is even more reason to obsolete it. @JamesNK

We discussed this and I believe we could quite a bit of pushback from the community.

Normally, I'd say go to whatever lengths are necessary to avoid breaking changes (which is why may never actually delete the deprecated KestrelServer constructor), but new API doesn't stop the break. The existing code to make UseHttps work with the public KestrelServer constructor already involves hacks like a custom SerpKestrelServerOptionsSetup : IConfigureOptions<KestrelServerOptions>, so I'm not too concerned about how clean the workaround looks.

What's the workaround? Copy-paste the internal implementation type and register that?

If customers absolutely do not want to use CreateSlimBuilder(), they also have the option copy the service descriptors from UseKestrel using a custom IWebHostBuilder, and then resolve the IServer. It's still ugly and fragile (which is why we should deprecate KestrelServer), but less so than the custom IConfigureOptions​, and it would have the upside of giving you things like HTTP/3 support if you want it.

I don't have an immediate sense of how feasible this is, but a bad workaround is qualitatively different from no workaround.

If we must add a new API to make adding Kestrel HTTPS services to an IServiceCollection more convenient without an IWebHostBuilder, hiding it in a non-extension static method might help prevent it from causing confusion since it's so rarely needed. I don't think we should add something specifically for just Kestrel's HTTPS-related services like UseKestrelHttpsConfiguration. It should just be AddKestrel (Required services + HTTPS + HTTP/3) and AddKestrelCore (Required services) to align with the "Use"-prefixed IWebHostBuilder counterparts.

AddKestrelHttpsConfiguration would be a one-liner it would be easy to be confident about - it's less obvious to me that AddKestrel would be.

[amcasey]

I've closed the underlying issue with an explanatory comment and multiple proposed workarounds.