Improvements to RunWithHttpsDevCert extensions #667
Conversation
|
|
||
| private static readonly char[] _invalidPathChars = Path.GetInvalidPathChars(); | ||
|
|
||
| private static string Sanitize(string name) |
There was a problem hiding this comment.
assuming this is not hot and you're already using Linq here how about making this a one liner
return new string(name.Select(c => Path.GetInvalidPathChars().Contains(c) ? '_' : c).ToArray());There was a problem hiding this comment.
It coulde be LINQ, but this is derived from the existing method we have in Aspire.Hosting for volume name sanitization, so I'm inclined to just keep it this way for now.
DamianEdwards
changed the title
| @@ -0,0 +1,10 @@ | |||
| <Project> | |||
There was a problem hiding this comment.
It wouldn't hurt to stick a comment in here in case someone stumbles in and asks what this is for.
| @@ -17,96 +18,162 @@ public static class DevCertHostingExtensions | |||
| /// Use <see cref="ResourceBuilderExtensions.WithHttpsEndpoint{TResource}"/> to configure an HTTPS endpoint. | |||
| /// </remarks> | |||
| public static IResourceBuilder<TResource> RunWithHttpsDevCertificate<TResource>( | |||
| this IResourceBuilder<TResource> builder, string certFileEnv, string certKeyFileEnv, Action<string, string>? onSuccessfulExport = null) | |||
| this IResourceBuilder<TResource> builder, CertificateFileFormat certificateFileFormat, string certFileEnv, string? certPasswordOrKeyEnv, Func<IServiceProvider, string, string?, Task>? onSuccessfulExport = null) | |||
There was a problem hiding this comment.
The name certPasswordOrKeyEnv is a bit confusing. is there a better API or documentation or something we can do here?
There was a problem hiding this comment.
Maybe an options class (one per type?) that has all the information for each cert type?
|
|
||
| if (File.Exists(certExportPath)) | ||
| { | ||
| logger.LogTrace("Previously exported key file is present but dev cert file '{CertPath}' is missing. Deleting dev cert file and re-exporting.", certExportPath); |
There was a problem hiding this comment.
Why do we warn above in PfxWithPassword, but only trace here?
| string[] args = ["dev-certs", "https", "--export-path", $"\"{certExportPath}\"", "--format", formatArg, passwordArgs]; | ||
| var argsString = string.Join(' ', args); |
There was a problem hiding this comment.
(nit) why make a string[] and then join it? Why not just make a single string?
var argsString = $"dev-certs https --export-path \"{certExportPath}\" --format {formatArg} {passwordArgs}";
| var objDir = GetMetadataValue(assemblyMetadata, "AppHostProjectBaseIntermediateOutputPath"); | ||
| var dirPath = projectDir is not null && objDir is not null | ||
| ? Path.Join(projectDir, objDir, "aspire") | ||
| : Directory.CreateTempSubdirectory(GetAppHostSpecificTempDirPrefix(builder)).FullName; |
There was a problem hiding this comment.
Should we even mess with the temp directory stuff if the OutputPath isn't found in the assembly? Can we just throw and make the code simpler?
| var projectDir = GetMetadataValue(assemblyMetadata, "AppHostProjectPath"); | ||
| var objDir = GetMetadataValue(assemblyMetadata, "AppHostProjectBaseIntermediateOutputPath"); |
There was a problem hiding this comment.
I half wonder if we should just stamp the full path into the file during the build and then we just need to read the 1 value out and use it.
DamianEdwards
force-pushed
the
damianedwards/CreateTempSubdirectory
branch
from
fed7983 to
33fa711
Compare
Changes the dev cert extensions to put the exported cert files in
/obj/aspireand falls back to user temp when/objisn't known.Also added support for using the dev cert in PFX file format with a password and updated the Node sample to use that. Also can now separately export the dev cert for scenarios where you need the PEM file for trusting (Node.js can listen on TLS using a PFX file but adding extra trusted certs requires a PEM file). The password is generated via the generated parameters helpers so it's stored in user secrets.
Updated the Metrics sample to account for changes.
Fixes #666