Add new event for finalizing resource annotations and use to configure installer certificate trust #13200
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 13200Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 13200" |
There was a problem hiding this comment.
Pull request overview
This PR introduces a new WithConfigurationFinalizer API that enables resource configuration callbacks to be invoked after the BeforeStartEvent completes. This provides a reliable mechanism for resources to finalize their configuration based on all previously applied annotations. The implementation includes converting Python and JavaScript installer setup to use this new finalizer pattern, along with adding certificate trust configuration inheritance from parent resources to their installer resources.
Key changes:
- New public API
WithConfigurationFinalizerfor registering finalization callbacks that execute in LIFO order - Finalizers are invoked after
BeforeStartEventhandlers inDistributedApplication.ExecuteBeforeStartHooksAsync - Python and JavaScript installers now use finalizers instead of
BeforeStartEventsubscriptions for configuration - Certificate trust settings are inherited from parent resources to installer resources
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Aspire.Hosting/ResourceBuilderExtensions.cs | Adds the new public API WithConfigurationFinalizer extension method |
| src/Aspire.Hosting/ApplicationModel/FinalizeResourceConfigurationCallbackAnnotation.cs | New annotation types for the finalizer callback and its context |
| src/Aspire.Hosting/DistributedApplication.cs | Implements finalizer execution logic after BeforeStartEvent completes |
| src/Aspire.Hosting/ApplicationModel/CertificateAuthorityCollectionAnnotation.cs | Adds static From method to merge certificate authority annotations |
| src/Aspire.Hosting.Python/PythonAppResourceBuilderExtensions.cs | Converts installer configuration to use finalizer pattern and adds certificate trust inheritance |
| src/Aspire.Hosting.JavaScript/JavaScriptHostingExtensions.cs | Converts installer configuration to use finalizer pattern and adds certificate trust inheritance |
| tests/Aspire.Hosting.Python.Tests/AddPythonAppTests.cs | Updates test helper to invoke finalizers after BeforeStartEvent (includes whitespace cleanup) |
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
| foreach (var resource in appModel.Resources) | ||
| { | ||
| if (resource.TryGetAnnotationsOfType<FinalizeResourceConfigurationCallbackAnnotation>(out var finalizeAnnotations)) | ||
| { | ||
| var context = new FinalizeResourceConfigurationCallbackAnnotationContext | ||
| { | ||
| Resource = resource, | ||
| ExecutionContext = new DistributedApplicationExecutionContext(DistributedApplicationOperation.Run), | ||
| CancellationToken = CancellationToken.None, | ||
| }; | ||
|
|
||
| // Execute in reverse order; take as a list to avoid mutating the collection during enumeration | ||
| var callbacks = finalizeAnnotations.Reverse().ToList(); | ||
| foreach (var callback in callbacks) | ||
| { | ||
| await callback.Callback(context).ConfigureAwait(false); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This code duplicates the finalization logic from DistributedApplication.ExecuteBeforeStartHooksAsync (lines 502-520 in src/Aspire.Hosting/DistributedApplication.cs). The duplication includes identical logic for iterating resources, checking for annotations, creating the context, and executing callbacks in reverse order.
Consider extracting this common logic into a shared helper method or extension method that both the production code and tests can use. This would reduce maintenance burden and ensure the test helper stays in sync with production behavior.
For example:
// In a shared location
internal static class ResourceFinalizationHelper
{
internal static async Task ExecuteFinalizersAsync(
IEnumerable<IResource> resources,
DistributedApplicationExecutionContext execContext,
CancellationToken cancellationToken)
{
foreach (var resource in resources)
{
if (resource.TryGetAnnotationsOfType<FinalizeResourceConfigurationCallbackAnnotation>(out var finalizeAnnotations))
{
var context = new FinalizeResourceConfigurationCallbackAnnotationContext
{
Resource = resource,
ExecutionContext = execContext,
CancellationToken = cancellationToken,
};
var callbacks = finalizeAnnotations.Reverse().ToList();
foreach (var callback in callbacks)
{
await callback.Callback(context).ConfigureAwait(false);
}
}
}
}
}| foreach (var resource in appModel.Resources) | |
| { | |
| if (resource.TryGetAnnotationsOfType<FinalizeResourceConfigurationCallbackAnnotation>(out var finalizeAnnotations)) | |
| { | |
| var context = new FinalizeResourceConfigurationCallbackAnnotationContext | |
| { | |
| Resource = resource, | |
| ExecutionContext = new DistributedApplicationExecutionContext(DistributedApplicationOperation.Run), | |
| CancellationToken = CancellationToken.None, | |
| }; | |
| // Execute in reverse order; take as a list to avoid mutating the collection during enumeration | |
| var callbacks = finalizeAnnotations.Reverse().ToList(); | |
| foreach (var callback in callbacks) | |
| { | |
| await callback.Callback(context).ConfigureAwait(false); | |
| } | |
| } | |
| } | |
| await ResourceFinalizationHelper.ExecuteFinalizersAsync( | |
| appModel.Resources, | |
| new DistributedApplicationExecutionContext(DistributedApplicationOperation.Run), | |
| CancellationToken.None); |
| /// <summary> | ||
| /// Adds a resource configuration finalizer callback annotation to the resource. | ||
| /// This is the last safe opportunity to modify resource annotations before configuration processing begins and provides an | ||
| /// opportunity to apply default behaviors based on the final resource configuration. | ||
| /// </summary> | ||
| /// <typeparam name="T">The resource type.</typeparam> | ||
| /// <param name="builder">The resource builder.</param> | ||
| /// <param name="callback">The callback to be invoked during resource finalization. All resource configuration finalizers will be invoked in reverse order of their registration immediately after the BeforeStartEvent is complete.</param> | ||
| /// <returns>The <see cref="IResourceBuilder{T}"/>.</returns> |
There was a problem hiding this comment.
According to the custom XML documentation guidelines (CodingGuidelineID: 1000002), the new public API method WithConfigurationFinalizer is missing <remarks> and <example> tags. The PR description indicates these were not added ("Did you add <remarks /> and <code /> elements on your triple slash comments? - [x] No").
For public APIs, comprehensive documentation is required. Consider adding:
- A
<remarks>section explaining when and why to use this API, behavioral details, and how it relates toBeforeStartEvent - An
<example>section with a practical code example showing how to use this finalizer
Example:
/// <remarks>
/// Configuration finalizers are executed after all BeforeStartEvent handlers have completed,
/// providing a reliable point to apply default behaviors based on the final resource configuration.
/// Multiple finalizers can be registered on the same resource and will execute in LIFO order
/// (last registered executes first).
/// </remarks>
/// <example>
/// Add a configuration finalizer to set default values:
/// <code>
/// var resource = builder.AddResource("myresource")
/// .WithConfigurationFinalizer(async ctx =>
/// {
/// // Apply defaults based on final configuration
/// if (!ctx.Resource.TryGetLastAnnotation<MyAnnotation>(out _))
/// {
/// // Add default annotation if not present
/// }
/// });
/// </code>
/// </example>| /// <summary> | ||
| /// Annotation to register a callback to be invoked during resource finalization. Callbacks are executed in reverse order | ||
| /// of their registration immediately after the BeforeStartEvent is complete. | ||
| /// </summary> |
There was a problem hiding this comment.
According to the custom XML documentation guidelines (CodingGuidelineID: 1000002), public classes require comprehensive documentation including <remarks>. The <summary> should also be more detailed about the purpose and usage pattern.
Consider expanding the documentation:
/// <summary>
/// Annotation to register a callback that finalizes resource configuration after all BeforeStartEvent handlers complete.
/// </summary>
/// <remarks>
/// This annotation provides the last safe opportunity to modify resource annotations before configuration
/// processing begins. Callbacks are executed in LIFO order (last registered executes first), allowing
/// resources to reliably apply default behaviors based on the final resource configuration.
/// Multiple instances of this annotation can be added to a single resource.
/// </remarks>| /// <summary> | ||
| /// Context for a finalize resource configuration callback annotation. | ||
| /// </summary> |
There was a problem hiding this comment.
According to the custom XML documentation guidelines (CodingGuidelineID: 1000002), public classes require comprehensive documentation. Consider adding a <remarks> section to explain how this context is used:
/// <summary>
/// Provides context information for a finalize resource configuration callback.
/// </summary>
/// <remarks>
/// This context is passed to callbacks registered via <see cref="FinalizeResourceConfigurationCallbackAnnotation"/>
/// and provides access to the resource being finalized, the execution context, and a cancellation token.
/// </remarks>| { | ||
| /// <summary> | ||
| /// Creates a new <see cref="CertificateAuthorityCollectionAnnotation"/> instance from one or more merged <see cref="CertificateAuthorityCollectionAnnotation"/> instances. | ||
| /// Certificate authority collections from all provided instances will be combined into the new instance, while the last vlaues for <see cref="TrustDeveloperCertificates"/> |
There was a problem hiding this comment.
Typo in the word "values": "vlaues" should be "values".
| /// Certificate authority collections from all provided instances will be combined into the new instance, while the last vlaues for <see cref="TrustDeveloperCertificates"/> | |
| /// Certificate authority collections from all provided instances will be combined into the new instance, while the last values for <see cref="TrustDeveloperCertificates"/> |
1 participant
Description
Adds a new
WithConfigurationFinalizerAPI that takes a configuration callback to be invoked afterBeforeStartEvent. Callbacks are called in LIFO order, so the last registered callback will be called first, allowing resources to provide a reliable final callback that will run after all other annotation processing is complete.I've converted the Python And JS
AddInstallermethods to use this new callback to finalize the installer configuration and added certificate trust overrides based on the parent resource certificate trust configuration.Fixes #13195
Checklist
<remarks />and<code />elements on your triple slash comments?doc-ideatemplatebreaking-changetemplatediagnostictemplate