-
Notifications
You must be signed in to change notification settings - Fork 887
fix(cli): keep guest AppHost restore on the selected Aspire package source #17166
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 2 commits
cf82ee0
a49d604
7ef60d4
32d4b3d
0381a4e
845de43
d60807b
9b6d705
c3b0183
7518348
e68732a
b59e2a0
1f0c606
7e76ca4
6e8bc74
f55e41f
7295d17
5e069d7
057abfa
2b54a5c
968df31
d1564d2
ad0f434
0bd9fce
94512f5
9e82f97
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 |
|---|---|---|
|
|
@@ -26,6 +26,8 @@ namespace Aspire.Cli.Projects; | |
| /// </summary> | ||
| internal sealed class PrebuiltAppHostServer : IAppHostServerProject | ||
| { | ||
| private const string NuGetOrgSource = "https://api.nuget.org/v3/index.json"; | ||
|
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?
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. Why the constant? It is being used in a few places. |
||
|
|
||
| internal const string ClosureMetadataFileName = "closure-metadata.txt"; | ||
| internal const string ClosureSourcesFileName = "closure-sources.txt"; | ||
| internal const string ClosureTargetsFileName = "closure-targets.txt"; | ||
|
|
@@ -123,7 +125,8 @@ public string GetServerPath() | |
| public async Task<AppHostServerPrepareResult> PrepareAsync( | ||
| string sdkVersion, | ||
| IEnumerable<IntegrationReference> integrations, | ||
| CancellationToken cancellationToken = default) | ||
| CancellationToken cancellationToken = default, | ||
| string? packageSourceOverride = null) | ||
| { | ||
| var integrationList = integrations.ToList(); | ||
| var packageRefs = integrationList.Where(r => r.IsPackageReference).ToList(); | ||
|
|
@@ -160,6 +163,7 @@ public async Task<AppHostServerPrepareResult> PrepareAsync( | |
| packageRefs, | ||
| projectRefs, | ||
| requestedChannel, | ||
| packageSourceOverride, | ||
| cancellationToken).ConfigureAwait(false); | ||
|
|
||
| if (closureManifest.Entries.Any(static entry => entry.IsPackageBacked)) | ||
|
|
@@ -185,7 +189,7 @@ await IntegrationPackageProbeManifest.WriteAsync( | |
| { | ||
| // NuGet-only — use the bundled NuGet service (no SDK required) | ||
| _integrationProbeManifestPath = await RestoreNuGetPackagesAsync( | ||
| packageRefs, requestedChannel, cancellationToken); | ||
| packageRefs, requestedChannel, packageSourceOverride, cancellationToken); | ||
| } | ||
|
|
||
| var appSettingsContent = CreateAppSettingsContent(packageRefs, []); | ||
|
|
@@ -226,13 +230,17 @@ await IntegrationPackageProbeManifest.WriteAsync( | |
| private async Task<string> RestoreNuGetPackagesAsync( | ||
| List<IntegrationReference> packageRefs, | ||
| string? requestedChannel, | ||
| string? packageSourceOverride, | ||
| CancellationToken cancellationToken) | ||
| { | ||
| _logger.LogDebug("Restoring {Count} integration packages via bundled NuGet", packageRefs.Count); | ||
|
|
||
| var packages = packageRefs.Select(r => (r.Name, r.Version!)).ToList(); | ||
| using var temporaryNuGetConfig = await TryCreateTemporaryNuGetConfigAsync(requestedChannel, cancellationToken); | ||
| var sources = await GetNuGetSourcesAsync(requestedChannel, cancellationToken); | ||
| var useExactPackageVersions = !string.IsNullOrWhiteSpace(packageSourceOverride); | ||
| var packages = packageRefs | ||
| .Select(r => (r.Name, Version: GetRestoreVersion(r.Version!, useExactPackageVersions))) | ||
| .ToList(); | ||
| using var temporaryNuGetConfig = await TryCreateTemporaryNuGetConfigAsync(requestedChannel, packageSourceOverride, cancellationToken); | ||
|
Member
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. When
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. Thanks — looked into the blast radius before changing this. Today So the That makes the credentials / private-feeds / non-Aspire-PSM concern theoretical for the surface this PR introduces. Restore-failure context is a separate ask — that part is fair, and I've taken it on (see below). If I've missed a path that flows For the restore-failure-context piece of your comment, the next push enriches the
Member
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. Do we have any scenarios where our restore would interact with global nuget.config (or a nuget.config in a parent folder) after new? I'm mainly wondering if there's the potential for surprise if
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. new is supposed to write a nuget.config file based on the default or specified channel then add should use that nuget.config or channel.
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. Thanks, this is the behavior the branch now moves toward: One auth detail remains worth calling out: we reject embedded credentials, but sanitized URLs may not bind to user-level |
||
| var sources = await GetNuGetSourcesAsync(requestedChannel, packageSourceOverride, cancellationToken); | ||
|
|
||
| return await _nugetService.RestorePackagesAsync( | ||
| packages, | ||
|
|
@@ -253,13 +261,19 @@ private async Task<AppHostServerClosureManifest> BuildIntegrationClosureManifest | |
| List<IntegrationReference> packageRefs, | ||
| List<IntegrationReference> projectRefs, | ||
| string? requestedChannel, | ||
| string? packageSourceOverride, | ||
| CancellationToken cancellationToken) | ||
| { | ||
| var restoreDir = Path.Combine(_workingDirectory, "integration-restore"); | ||
| Directory.CreateDirectory(restoreDir); | ||
|
|
||
| var channelSources = await GetNuGetSourcesAsync(requestedChannel, cancellationToken); | ||
| var projectContent = GenerateIntegrationProjectFile(packageRefs, projectRefs, restoreDir, channelSources); | ||
| var channelSources = await GetNuGetSourcesAsync(requestedChannel, packageSourceOverride, cancellationToken); | ||
| var projectContent = GenerateIntegrationProjectFile( | ||
| packageRefs, | ||
| projectRefs, | ||
| restoreDir, | ||
| channelSources, | ||
| useExactPackageVersions: !string.IsNullOrWhiteSpace(packageSourceOverride)); | ||
| var projectFilePath = Path.Combine(restoreDir, IntegrationProjectFileName); | ||
| await File.WriteAllTextAsync(projectFilePath, projectContent, cancellationToken); | ||
|
|
||
|
|
@@ -354,7 +368,8 @@ internal static string GenerateIntegrationProjectFile( | |
| List<IntegrationReference> packageRefs, | ||
| List<IntegrationReference> projectRefs, | ||
| string restoreDir, | ||
| IEnumerable<string>? additionalSources = null) | ||
| IEnumerable<string>? additionalSources = null, | ||
| bool useExactPackageVersions = false) | ||
| { | ||
| var propertyGroup = new XElement("PropertyGroup", | ||
| new XElement("TargetFramework", DotNetBasedAppHostServerProject.TargetFramework), | ||
|
|
@@ -394,7 +409,7 @@ internal static string GenerateIntegrationProjectFile( | |
| } | ||
| return new XElement("PackageReference", | ||
| new XAttribute("Include", p.Name), | ||
| new XAttribute("Version", p.Version)); | ||
| new XAttribute("Version", GetRestoreVersion(p.Version, useExactPackageVersions))); | ||
| }))); | ||
| } | ||
|
|
||
|
|
@@ -461,26 +476,18 @@ internal static string GenerateIntegrationProjectFile( | |
| /// <summary> | ||
| /// Gets NuGet sources from the resolved channel for bundled restore. | ||
| /// </summary> | ||
| private async Task<IEnumerable<string>?> GetNuGetSourcesAsync(string? requestedChannel, CancellationToken cancellationToken) | ||
| private async Task<IEnumerable<string>?> GetNuGetSourcesAsync(string? requestedChannel, string? packageSourceOverride, CancellationToken cancellationToken) | ||
| { | ||
| var sources = new List<string>(); | ||
|
|
||
| try | ||
| if (!string.IsNullOrWhiteSpace(packageSourceOverride)) | ||
| { | ||
| var channels = await _packagingService.GetChannelsAsync(cancellationToken); | ||
|
|
||
| IEnumerable<PackageChannel> explicitChannels; | ||
| if (!string.IsNullOrEmpty(requestedChannel)) | ||
| { | ||
| var matchingChannel = channels.FirstOrDefault(c => string.Equals(c.Name, requestedChannel, StringComparison.OrdinalIgnoreCase)); | ||
| explicitChannels = matchingChannel is not null ? [matchingChannel] : channels.Where(c => c.Type == PackageChannelType.Explicit); | ||
| } | ||
| else | ||
| { | ||
| explicitChannels = channels.Where(c => c.Type == PackageChannelType.Explicit); | ||
| } | ||
| sources.Add(packageSourceOverride); | ||
| } | ||
|
|
||
| foreach (var channel in explicitChannels) | ||
| try | ||
| { | ||
| foreach (var channel in await GetExplicitRestoreChannelsAsync(requestedChannel, cancellationToken)) | ||
| { | ||
| if (channel.Mappings is null) | ||
| { | ||
|
|
@@ -501,11 +508,52 @@ internal static string GenerateIntegrationProjectFile( | |
| _logger.LogWarning(ex, "Failed to get package channels, relying on nuget.config and nuget.org fallback"); | ||
| } | ||
|
|
||
| if (!string.IsNullOrWhiteSpace(packageSourceOverride) && sources.Count == 1) | ||
| { | ||
| sources.Add(NuGetOrgSource); | ||
| } | ||
|
|
||
| return sources.Count > 0 ? sources : null; | ||
| } | ||
|
|
||
| private async Task<TemporaryNuGetConfig?> TryCreateTemporaryNuGetConfigAsync(string? requestedChannel, CancellationToken cancellationToken) | ||
| private async Task<TemporaryNuGetConfig?> TryCreateTemporaryNuGetConfigAsync(string? requestedChannel, string? packageSourceOverride, CancellationToken cancellationToken) | ||
| { | ||
| if (!string.IsNullOrWhiteSpace(packageSourceOverride)) | ||
|
radical marked this conversation as resolved.
|
||
| { | ||
| var mappings = new List<PackageMapping> | ||
| { | ||
| new("Aspire*", packageSourceOverride) | ||
| }; | ||
| var configureGlobalPackagesFolder = false; | ||
|
|
||
| try | ||
| { | ||
| foreach (var restoreChannel in await GetExplicitRestoreChannelsAsync(requestedChannel, cancellationToken)) | ||
| { | ||
| if (restoreChannel.Mappings is null) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| mappings.AddRange(restoreChannel.Mappings); | ||
|
radical marked this conversation as resolved.
Outdated
|
||
| configureGlobalPackagesFolder |= restoreChannel.ConfigureGlobalPackagesFolder; | ||
| } | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| _logger.LogWarning(ex, "Failed to get package channels while creating source override NuGet.config"); | ||
| } | ||
|
|
||
| if (!mappings.Any(static mapping => mapping.PackageFilter == PackageMapping.AllPackages)) | ||
| { | ||
| mappings.Add(new PackageMapping(PackageMapping.AllPackages, NuGetOrgSource)); | ||
| } | ||
|
|
||
| return await TemporaryNuGetConfig.CreateAsync( | ||
| [.. mappings.DistinctBy(static mapping => $"{mapping.PackageFilter}\0{mapping.Source}")], | ||
| configureGlobalPackagesFolder); | ||
| } | ||
|
|
||
| if (string.IsNullOrEmpty(requestedChannel)) | ||
| { | ||
| return null; | ||
|
|
@@ -541,6 +589,28 @@ internal static string GenerateIntegrationProjectFile( | |
| return await TemporaryNuGetConfig.CreateAsync(channel.Mappings, channel.ConfigureGlobalPackagesFolder); | ||
| } | ||
|
|
||
| private async Task<IEnumerable<PackageChannel>> GetExplicitRestoreChannelsAsync(string? requestedChannel, CancellationToken cancellationToken) | ||
| { | ||
| var channels = await _packagingService.GetChannelsAsync(cancellationToken); | ||
| if (!string.IsNullOrEmpty(requestedChannel)) | ||
| { | ||
| var matchingChannel = channels.FirstOrDefault(c => string.Equals(c.Name, requestedChannel, StringComparison.OrdinalIgnoreCase)); | ||
| return matchingChannel is not null ? [matchingChannel] : channels.Where(c => c.Type == PackageChannelType.Explicit).ToArray(); | ||
| } | ||
|
|
||
| return channels.Where(c => c.Type == PackageChannelType.Explicit).ToArray(); | ||
| } | ||
|
|
||
| private static string GetRestoreVersion(string version, bool useExactPackageVersions) | ||
| { | ||
| if (!useExactPackageVersions || version.Length == 0 || version[0] is '[' or '(') | ||
| { | ||
| return version; | ||
| } | ||
|
|
||
| return $"[{version}]"; | ||
| } | ||
|
|
||
| /// <inheritdoc /> | ||
| public (string SocketPath, Process Process, OutputCollector OutputCollector) Run( | ||
| int hostPid, | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.