Support prompting for unresolved parameters during deploy #11160

captainsafia · 2025-08-28T20:05:52Z

Contributes to #10448.

Copilot

Pull Request Overview

This PR adds support for prompting users for unresolved parameter values during Azure deployment, addressing the need to collect missing parameter values interactively rather than failing the deployment.

Adds parameter resolution step before Azure Bicep provisioning that prompts for unresolved parameters
Exposes the WaitForValueTcs property on ParameterResource for testing purposes
Updates Azure deployment context to include interaction service for parameter prompting

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
tests/Aspire.Hosting.Azure.Tests/AzureDeployerTests.cs	Adds tests for parameter prompting functionality during deployment
src/Aspire.Hosting/ApplicationModel/ParameterResource.cs	Changes WaitForValueTcs visibility from internal to public
src/Aspire.Hosting.Azure/AzureEnvironmentResource.cs	Injects interaction service into Azure deploying context
src/Aspire.Hosting.Azure/AzureDeployingContext.cs	Implements parameter resolution logic with user prompting
playground/deployers/Deployers.AppHost/AppHost.cs	Adds example parameters for testing deployment scenarios

src/Aspire.Hosting.Azure/AzureDeployingContext.cs

src/Aspire.Hosting/ApplicationModel/ParameterResource.cs

src/Aspire.Hosting.Azure/AzureDeployingContext.cs

eerhardt · 2025-08-28T22:19:53Z

src/Aspire.Hosting/ApplicationModel/ParameterResource.cs

@@ -75,7 +75,7 @@ internal string ConfigurationKey
    /// <summary>
    /// A task completion source that can be used to wait for the value of the parameter to be set.
    /// </summary>
-    internal TaskCompletionSource<string>? WaitForValueTcs { get; set; }
+    public TaskCompletionSource<string>? WaitForValueTcs { get; set; }


We shouldn't be exposing the TaskCompletionSource publicly. That would mean anyone would be able to complete the Task. Instead, you just want the Task to be exposed, right?

Maybe we should flip this instead. ParameterResource has the ability for callers to give it a Task<string> that it can use to pull the value from?

We shouldn't be exposing the TaskCompletionSource publicly. That would mean anyone would be able to complete the Task.

In this case, we do want to complete the TCS from outside the core API

Maybe we should flip this instead. ParameterResource has the ability for callers to give it a Task that it can use to pull the value from?

Is this similar to SetValue from #11160 (comment). Something like public void SetValue(Task<string> valueProvider) then we'd resolve the valueProvider that is passed in GetValueAsync.

Another option is to do something similar to what we did with ProvisioningContextProvider and have a common ParameterProcessor implementation that both run-mode and publish-mode scenarios use.

I don't think a TCS should be exposed publically. Instead I would expected there to be a Task property on the type and, if publically required, a method to complete it, e.g.

public Task ValueAvailableTask { get; } public void SetValue(string s); // completes the task

Unless you want to cancel or set an exception (now we're recreating a TCS with a new name).

I changed the implementation in 5d805e1 so that ParameterProcessor is public. This let's the Azure deployer and any other deployers that are implemented add support for parameter resolution.

Some notes on this:

Users now get prompted about saving values to user secrets, which isn't something we want to support for deployment. We may need to opt this out, I can see it being valuable for super local deployment scenarios but not necessarily deployments from CI.

We need to add a WaitForParameterResolutionAsync method to the processor so that we can support the dashboard's fire and forget model and the

davidfowl · 2025-08-30T14:57:11Z

Does every deployment system need to implement their own parameter processing and prompting? Why wouldn't this be as generic as what we have at runtime with the ParameterProcessor?

captainsafia · 2025-09-02T14:57:20Z

Does every deployment system need to implement their own parameter processing and prompting? Why wouldn't this be as generic as what we have at runtime with the ParameterProcessor?

Yep, this is a scenario where this proposal:

Another option is to do something similar to what we did with ProvisioningContextProvider and have a common ParameterProcessor implementation that both run-mode and publish-mode scenarios use.

would come in handy. If we centralize parameter resolution as a core public API, then it wouldn't need to be rewritten in multiple places.

github-actions · 2025-09-02T21:51:16Z

🚀 Dogfood this PR with:

curl -fsSL https://github.com/raw/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 11160

Or

Run remotely in PowerShell:

iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 11160"

src/Aspire.Hosting/ApplicationModel/ParameterResource.cs

src/Aspire.Hosting.Azure/AzureDeployingContext.cs

playground/deployers/Deployers.AppHost/AppHost.cs

davidfowl · 2025-09-03T19:08:20Z

src/Aspire.Hosting/Orchestrator/ParameterProcessor.cs

    ResourceNotificationService notificationService,
    ResourceLoggerService loggerService,
    IInteractionService interactionService,
    ILogger<ParameterProcessor> logger,
    DistributedApplicationOptions options)
 {
    private readonly List<ParameterResource> _unresolvedParameters = [];
-
-    public async Task InitializeParametersAsync(IEnumerable<ParameterResource> parameterResources)
+    private Task? _parameterResolutionTask;


Delete this task.

src/Aspire.Hosting/Orchestrator/ParameterProcessor.cs

eerhardt · 2025-09-03T19:10:34Z

src/Aspire.Hosting.Azure/AzureEnvironmentResource.cs

@@ -76,14 +77,16 @@ private Task DeployAsync(DeployingContext context)
        var activityPublisher = context.Services.GetRequiredService<IPublishingActivityReporter>();
        var containerImageBuilder = context.Services.GetRequiredService<IResourceContainerImageBuilder>();
        var processRunner = context.Services.GetRequiredService<IProcessRunner>();
+        var parameterProcessor = context.Services.GetRequiredService<ParameterProcessor>();


Do we want this concrete type as the DI service? Or do we want an interface?

Interface when we figure out what to expose. This API is sketchy as is.

eerhardt · 2025-09-03T19:12:08Z

src/Aspire.Hosting/Orchestrator/ParameterProcessor.cs

+    /// Initializes parameter resources and handles unresolved parameters if interaction service is available.
+    /// </summary>
+    /// <param name="parameterResources">The parameter resources to initialize.</param>
+    /// <param name="waitForResolution">Whether to wait for all parameters to be resolved before returning.</param>


Suggested change

/// <param name="waitForResolution">Whether to wait for all parameters to be resolved before returning.</param>

/// <param name="waitForResolution">Whether to wait for all parameters to be resolved before completing the returned Task.</param>

eerhardt · 2025-09-03T19:13:12Z

src/Aspire.Hosting/Orchestrator/ParameterProcessor.cs

@@ -14,16 +15,24 @@ namespace Aspire.Hosting.Orchestrator;
 /// <summary>
 /// Handles processing of parameter resources during application orchestration.
 /// </summary>
-internal sealed class ParameterProcessor(
+[Experimental("ASPIREINTERACTION001", UrlFormat = "https://aka.ms/aspire/diagnostics/{0}")]
+public sealed class ParameterProcessor(


I think this is the first public type in the Aspire.Hosting.Orchestrator namespace. I'm wondering if we should make an interface in a separate namespace that is already exposed.

Good callout -- I think we should move this to the Aspire.Hosting namespace to match what we do for InteractionService.

Support prompting for unresolved parameters during deploy

71c8353

Copilot AI review requested due to automatic review settings August 28, 2025 20:05

captainsafia requested a review from eerhardt as a code owner August 28, 2025 20:05

github-actions bot added the area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication label Aug 28, 2025

Copilot AI reviewed Aug 28, 2025

View reviewed changes

src/Aspire.Hosting.Azure/AzureDeployingContext.cs Outdated Show resolved Hide resolved

src/Aspire.Hosting.Azure/AzureDeployingContext.cs Outdated Show resolved Hide resolved

captainsafia commented Aug 28, 2025

View reviewed changes

src/Aspire.Hosting/ApplicationModel/ParameterResource.cs Outdated Show resolved Hide resolved

davidfowl reviewed Aug 28, 2025

View reviewed changes

src/Aspire.Hosting.Azure/AzureDeployingContext.cs Outdated Show resolved Hide resolved

Fix input generation

d1c5c7c

eerhardt reviewed Aug 28, 2025

View reviewed changes

Make ParameterProcessor public

5d805e1

captainsafia requested a review from mitchdenny as a code owner September 2, 2025 21:51

captainsafia mentioned this pull request Sep 2, 2025

[Failing test]: Aspire.Cli.Tests.Commands.NewCommandTests. NewCommandDoesNotPromptForOutputPathIfSpecifiedOnCommandLine #11222

Open

1 task

Quarantine test

dfa1d71

davidfowl reviewed Sep 3, 2025

View reviewed changes

src/Aspire.Hosting/ApplicationModel/ParameterResource.cs Outdated Show resolved Hide resolved

davidfowl reviewed Sep 3, 2025

View reviewed changes

src/Aspire.Hosting.Azure/AzureDeployingContext.cs Outdated Show resolved Hide resolved

davidfowl reviewed Sep 3, 2025

View reviewed changes

playground/deployers/Deployers.AppHost/AppHost.cs Show resolved Hide resolved

Feedback

eb8c89d

davidfowl reviewed Sep 3, 2025

View reviewed changes

src/Aspire.Hosting/Orchestrator/ParameterProcessor.cs Outdated Show resolved Hide resolved

eerhardt reviewed Sep 3, 2025

View reviewed changes

davidfowl approved these changes Sep 3, 2025

View reviewed changes

eerhardt reviewed Sep 3, 2025

View reviewed changes

captainsafia added 2 commits September 3, 2025 13:30

Feedback

a1f56fc

Fix waitForResolution default

a3e42e4

captainsafia merged commit b024fa4 into main Sep 3, 2025
305 checks passed

captainsafia deleted the safia/deploy-params branch September 3, 2025 21:37

dotnet-policy-service bot added this to the 9.5 milestone Sep 3, 2025

	/// <param name="waitForResolution">Whether to wait for all parameters to be resolved before returning.</param>
	/// <param name="waitForResolution">Whether to wait for all parameters to be resolved before completing the returned Task.</param>

Support prompting for unresolved parameters during deploy #11160

Support prompting for unresolved parameters during deploy #11160

Uh oh!

Conversation

captainsafia commented Aug 28, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davidfowl Aug 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davidfowl commented Aug 30, 2025

Uh oh!

captainsafia commented Sep 2, 2025

Uh oh!

github-actions bot commented Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

davidfowl Aug 29, 2025 •

edited

Loading

github-actions bot commented Sep 2, 2025 •

edited

Loading