-
Notifications
You must be signed in to change notification settings - Fork 952
Allow using server authentication cert config in runtime WithContainerFiles callbacks, fix MacOS keychain access spam with dev cert #13151
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
d06fc0f
ef6b320
64a4219
e453337
64bffd2
540e423
51053cd
e97c4f5
9ecbc62
820d7b8
42f6377
1f256d3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ | |
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using System.Diagnostics; | ||
| using System.Diagnostics.CodeAnalysis; | ||
|
|
||
| namespace Aspire.Hosting.ApplicationModel; | ||
|
|
||
|
|
@@ -276,4 +277,40 @@ public sealed class ContainerFileSystemCallbackContext | |
| /// The app model resource the callback is associated with. | ||
| /// </summary> | ||
| public required IResource Model { get; init; } | ||
|
|
||
| /// <summary> | ||
| /// The path to the server authentication certificate file inside the container. | ||
| /// </summary> | ||
| [Experimental("ASPIRECERTIFICATES001", UrlFormat = "https://aka.ms/aspire/diagnostics/{0}")] | ||
| public ContainerFileSystemCallbackServerAuthenticationCertificateContext? ServerAuthenticationCertificateContext { get; set; } | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Represents the context for server authentication certificate files in a <see cref="ContainerFileSystemCallbackContext"/>. | ||
| /// </summary> | ||
| [Experimental("ASPIRECERTIFICATES001", UrlFormat = "https://aka.ms/aspire/diagnostics/{0}")] | ||
| public sealed class ContainerFileSystemCallbackServerAuthenticationCertificateContext | ||
| { | ||
| /// <summary> | ||
| /// A reference expression that resolves to the path to the server authentication certificate file inside the container. | ||
| /// Use GetValueAsync to resolve the path. | ||
| /// </summary> | ||
|
danegsta marked this conversation as resolved.
|
||
| public ReferenceExpression CertificatePath { get; init; } = null!; | ||
|
|
||
| /// <summary> | ||
| /// A reference expression that resolves to the path to the server authentication key file inside the container. | ||
| /// Use GetValueAsync to resolve the path. | ||
| /// </summary> | ||
|
danegsta marked this conversation as resolved.
|
||
| public ReferenceExpression KeyPath { get; init; } = null!; | ||
|
|
||
| /// <summary> | ||
| /// A reference expression that resolves to the path to the server authentication PFX file inside the container. | ||
| /// Use GetValueAsync to resolve the path. | ||
| /// </summary> | ||
|
Comment on lines
+306
to
+309
|
||
| public ReferenceExpression PfxPath { get; init; } = null!; | ||
|
|
||
| /// <summary> | ||
| /// The password for the server authentication key inside the container or null if no password is required. | ||
| /// </summary> | ||
|
danegsta marked this conversation as resolved.
|
||
| public string? Password { get; init; } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -63,13 +63,20 @@ private ReferenceExpression(string format, IValueProvider[] valueProviders, stri | |
| public string ValueExpression => | ||
| string.Format(CultureInfo.InvariantCulture, Format, _manifestExpressions); | ||
|
|
||
| /// <summary> | ||
| /// Indicates whether this expression was ever referenced to get its value. | ||
| /// </summary> | ||
| internal bool WasResolved { get; set; } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need this?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I’m only generating key material that’s actually reference; if no resource actually references the pfx or pem key, I’m not exporting them. I’m using this to check the usage.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I dont like it, can we put it outside of the reference expression.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Other option (without making the API weird) would be a custom IValueProvider |
||
|
|
||
| /// <summary> | ||
| /// Gets the value of the expression. The final string value after evaluating the format string and its parameters. | ||
| /// </summary> | ||
| /// <param name="context">A context for resolving the value.</param> | ||
| /// <param name="cancellationToken">A <see cref="CancellationToken"/>.</param> | ||
| public async ValueTask<string?> GetValueAsync(ValueProviderContext context, CancellationToken cancellationToken) | ||
| { | ||
| WasResolved = true; | ||
|
|
||
| // NOTE: any logical changes to this method should also be made to ExpressionResolver.EvalExpressionAsync | ||
| if (Format.Length == 0) | ||
| { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The documentation says "The path to the server authentication certificate file inside the container" but this property actually contains a
ContainerFileSystemCallbackServerAuthenticationCertificateContextobject, not a path. The documentation should describe what the context contains, e.g., "The server authentication certificate context containing paths to certificate files and associated configuration."