Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 24 additions & 1 deletion src/Aspire.Hosting.JavaScript/JavaScriptHostingExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -677,7 +677,7 @@ private static void AddInstaller<TResource>(IResourceBuilder<TResource> resource
.WithParentRelationship(resource.Resource)
.ExcludeFromManifest();

resource.ApplicationBuilder.Eventing.Subscribe<BeforeStartEvent>((_, _) =>
resource.WithConfigurationFinalizer(_ =>
{
// set the installer's working directory to match the resource's working directory
// and set the install command and args based on the resource's annotations
Expand All @@ -692,6 +692,29 @@ private static void AddInstaller<TResource>(IResourceBuilder<TResource> resource
.WithWorkingDirectory(resource.Resource.WorkingDirectory)
.WithArgs(installCommand.Args);

if (resource.Resource.TryGetAnnotationsOfType<CertificateTrustConfigurationCallbackAnnotation>(out var trustConfigAnnotations))
{
// Use the same trust configuration as the parent resource
foreach (var trustConfigAnnotation in trustConfigAnnotations)
{
installerBuilder.WithAnnotation(trustConfigAnnotation, ResourceAnnotationMutationBehavior.Append);
}
}

if (resource.Resource.TryGetLastAnnotation<CertificateAuthorityCollectionAnnotation>(out var trustAnnotation))
{
if (installerBuilder.Resource.TryGetLastAnnotation<CertificateAuthorityCollectionAnnotation>(out var existingTrustAnnotation))
{
// Merge existing trust with parent's trust configuration
installerBuilder.WithAnnotation(CertificateAuthorityCollectionAnnotation.From(existingTrustAnnotation, trustAnnotation), ResourceAnnotationMutationBehavior.Replace);
}
else
{
// No existing trust, just copy from parent
installerBuilder.WithAnnotation(CertificateAuthorityCollectionAnnotation.From(trustAnnotation), ResourceAnnotationMutationBehavior.Replace);
}
}

return Task.CompletedTask;
});

Expand Down
39 changes: 35 additions & 4 deletions src/Aspire.Hosting.Python/PythonAppResourceBuilderExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1363,23 +1363,54 @@ private static void AddInstaller<T>(IResourceBuilder<T> builder, bool install) w
// For other package managers (pip, etc.), Python validation happens via PythonVenvCreatorResource
});

builder.ApplicationBuilder.Eventing.Subscribe<BeforeStartEvent>((_, _) =>
installerBuilder.WithConfigurationFinalizer(_ =>
{
// Set the installer's working directory to match the resource's working directory
// and set the install command and args based on the resource's annotations
if (!builder.Resource.TryGetLastAnnotation<PythonPackageManagerAnnotation>(out var packageManager) ||
!builder.Resource.TryGetLastAnnotation<PythonInstallCommandAnnotation>(out var installCommand))
{
// No package manager configured - don't fail, just don't run the installer
// This allows venv to be created without requiring a package manager
return Task.CompletedTask;
throw new InvalidOperationException("PythonPackageManagerAnnotation and PythonInstallCommandAnnotation are required when installing packages.");
}

installerBuilder
.WithCommand(packageManager.ExecutableName)
.WithWorkingDirectory(builder.Resource.WorkingDirectory)
.WithArgs(installCommand.Args);

if (builder.Resource.TryGetAnnotationsOfType<CertificateTrustConfigurationCallbackAnnotation>(out var trustConfigAnnotations))
{
// Use the same trust configuration as the parent resource
foreach (var trustConfigAnnotation in trustConfigAnnotations)
{
installerBuilder.WithAnnotation(trustConfigAnnotation, ResourceAnnotationMutationBehavior.Append);
}
}

installerBuilder.WithCertificateTrustConfiguration(ctx =>
{
if (ctx.Scope != CertificateTrustScope.None)
{
ctx.EnvironmentVariables["UV_NATIVE_TLS"] = "true";
}

return Task.CompletedTask;
});

if (builder.Resource.TryGetLastAnnotation<CertificateAuthorityCollectionAnnotation>(out var trustAnnotation))
{
if (installerBuilder.Resource.TryGetLastAnnotation<CertificateAuthorityCollectionAnnotation>(out var existingTrustAnnotation))
{
// Merge existing trust with parent's trust configuration
installerBuilder.WithAnnotation(CertificateAuthorityCollectionAnnotation.From(existingTrustAnnotation, trustAnnotation), ResourceAnnotationMutationBehavior.Replace);
}
else
{
// No existing trust, just copy from parent
installerBuilder.WithAnnotation(CertificateAuthorityCollectionAnnotation.From(trustAnnotation), ResourceAnnotationMutationBehavior.Replace);
}
}

return Task.CompletedTask;
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,34 @@ public enum CertificateTrustScope
/// </summary>
public sealed class CertificateAuthorityCollectionAnnotation : IResourceAnnotation
{
/// <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 values for <see cref="TrustDeveloperCertificates"/>
/// and <see cref="Scope"/> will be used, with null values being ignored (previous value if any will be retained).
/// </summary>
/// <param name="other">The other <see cref="CertificateAuthorityCollectionAnnotation"/>s that will be merged to create the new instance.</param>
/// <returns>A merged copy of the provided <see cref="CertificateAuthorityCollectionAnnotation"/> instances</returns>
public static CertificateAuthorityCollectionAnnotation From(params CertificateAuthorityCollectionAnnotation[] other)
{
ArgumentNullException.ThrowIfNull(other);

var annotation = new CertificateAuthorityCollectionAnnotation();
foreach (var item in other)
{
annotation.CertificateAuthorityCollections.AddRange(item.CertificateAuthorityCollections);
if (item.TrustDeveloperCertificates.HasValue)
{
annotation.TrustDeveloperCertificates = item.TrustDeveloperCertificates;
}
if (item.Scope.HasValue)
{
annotation.Scope = item.Scope;
}
}

return annotation;
}

/// <summary>
/// Gets the <see cref="CertificateAuthorityCollection"/> that is being referenced.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace Aspire.Hosting.ApplicationModel;

/// <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>
Comment on lines +6 to +9
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>

Copilot generated this review using guidance from repository custom instructions.
public sealed class FinalizeResourceConfigurationCallbackAnnotation : IResourceAnnotation
{
/// <summary>
/// The callback to be invoked during resource finalization.
/// </summary>
public required Func<FinalizeResourceConfigurationCallbackAnnotationContext, Task> Callback { get; init; }
}

/// <summary>
/// Context for a finalize resource configuration callback annotation.
/// </summary>
Comment on lines +18 to +20
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>

Copilot generated this review using guidance from repository custom instructions.
public sealed class FinalizeResourceConfigurationCallbackAnnotationContext
{
/// <summary>
/// The resource associated with the callback.
/// </summary>
public required IResource Resource { get; init; }

/// <summary>
/// The execution context for the callback.
/// </summary>
public required DistributedApplicationExecutionContext ExecutionContext { get; init; }

/// <summary>
/// The cancellation token for the callback.
/// </summary>
public required CancellationToken CancellationToken { get; init; }
}
20 changes: 20 additions & 0 deletions src/Aspire.Hosting/DistributedApplication.cs
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,26 @@ internal async Task ExecuteBeforeStartHooksAsync(CancellationToken cancellationT
{
await lifecycleHook.BeforeStartAsync(appModel, cancellationToken).ConfigureAwait(false);
}

foreach (var resource in appModel.Resources)
{
if (resource.TryGetAnnotationsOfType<FinalizeResourceConfigurationCallbackAnnotation>(out var finalizeAnnotations))
{
var context = new FinalizeResourceConfigurationCallbackAnnotationContext
{
Resource = resource,
ExecutionContext = execContext,
CancellationToken = cancellationToken,
};

// 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);
}
}
}
}
finally
{
Expand Down
21 changes: 21 additions & 0 deletions src/Aspire.Hosting/ResourceBuilderExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3027,6 +3027,27 @@ public static IResourceBuilder<T> ExcludeFromMcp<T>(this IResourceBuilder<T> bui
return builder.WithAnnotation(new ExcludeFromMcpAnnotation());
}

/// <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>
Comment on lines +3030 to +3038
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 to BeforeStartEvent
  • 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&lt;MyAnnotation&gt;(out _))
///         {
///             // Add default annotation if not present
///         }
///     });
/// </code>
/// </example>

Copilot generated this review using guidance from repository custom instructions.
public static IResourceBuilder<T> WithConfigurationFinalizer<T>(this IResourceBuilder<T> builder, Func<FinalizeResourceConfigurationCallbackAnnotationContext, Task> callback)
where T : IResource
{
ArgumentNullException.ThrowIfNull(builder);
ArgumentNullException.ThrowIfNull(callback);

return builder.WithAnnotation(new FinalizeResourceConfigurationCallbackAnnotation
{
Callback = callback,
}, ResourceAnnotationMutationBehavior.Append);
}

/// <summary>
/// Adds a callback to configure container image push options for the resource.
/// </summary>
Expand Down
30 changes: 25 additions & 5 deletions tests/Aspire.Hosting.Python.Tests/AddPythonAppTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ private static void AssertPythonCommandPath(string expectedVenvPath, string actu
var expectedCommand = OperatingSystem.IsWindows()
? Path.Join(expectedVenvPath, "Scripts", "python.exe")
: Path.Join(expectedVenvPath, "bin", "python");

Assert.Equal(expectedCommand, actualCommand);
}

Expand Down Expand Up @@ -552,7 +552,7 @@ public void WithVirtualEnvironment_UsesAppHostDirectoryWhenVenvOnlyExistsThere()
{
using var builder = TestDistributedApplicationBuilder.Create().WithTestAndResourceLogging(outputHelper);
using var tempAppDir = new TempDirectory();

// Create app directory as a subdirectory of AppHost (realistic scenario)
var appDirName = "python-app";
var appDirPath = Path.Combine(builder.AppHostDirectory, appDirName);
Expand Down Expand Up @@ -594,7 +594,7 @@ public void WithVirtualEnvironment_UsesAppHostDirectoryWhenVenvOnlyExistsThere()
public void WithVirtualEnvironment_PrefersAppDirectoryWhenVenvExistsInBoth()
{
using var builder = TestDistributedApplicationBuilder.Create().WithTestAndResourceLogging(outputHelper);

// Create app directory as a subdirectory of AppHost (realistic scenario)
var appDirName = "python-app";
var appDirPath = Path.Combine(builder.AppHostDirectory, appDirName);
Expand Down Expand Up @@ -663,7 +663,7 @@ public void WithVirtualEnvironment_DefaultsToAppDirectoryWhenVenvExistsInNeither
public void WithVirtualEnvironment_ExplicitPath_UsesVerbatim()
{
using var builder = TestDistributedApplicationBuilder.Create().WithTestAndResourceLogging(outputHelper);

// Create app directory as a subdirectory of AppHost
var appDirName = "python-app";
var appDirPath = Path.Combine(builder.AppHostDirectory, appDirName);
Expand All @@ -680,7 +680,7 @@ public void WithVirtualEnvironment_ExplicitPath_UsesVerbatim()
try
{
var scriptName = "main.py";

// Explicitly specify a custom venv path - should use it verbatim, not fall back to AppHost .venv
var resourceBuilder = builder.AddPythonApp("pythonProject", appDirName, scriptName)
.WithVirtualEnvironment("custom-venv");
Expand Down Expand Up @@ -2297,6 +2297,26 @@ private static async Task PublishBeforeStartEventAsync(DistributedApplication ap
var appModel = app.Services.GetRequiredService<DistributedApplicationModel>();
var eventing = app.Services.GetRequiredService<IDistributedApplicationEventing>();
await eventing.PublishAsync(new BeforeStartEvent(app.Services, appModel), CancellationToken.None);

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);
}
}
}
Comment on lines +2301 to +2319
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);
                }
            }
        }
    }
}
Suggested change
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);

Copilot uses AI. Check for mistakes.
}
}

Loading