Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
2 changes: 1 addition & 1 deletion src/Aspire.Cli/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ internal static async Task<IHost> BuildApplicationAsync(string[] args, CliStartu
var globalSettingsFilePath = GetGlobalSettingsPath(startupContext.Logger);
var globalSettingsFile = new FileInfo(globalSettingsFilePath);
var workingDirectory = new DirectoryInfo(Environment.CurrentDirectory);
ConfigurationHelper.RegisterSettingsFiles(builder.Configuration, workingDirectory, globalSettingsFile);
ConfigurationHelper.RegisterSettingsFiles(builder.Configuration, workingDirectory, globalSettingsFile, startupContext.Logger);

TrySetLocaleOverride(LocaleHelpers.GetLocaleOverride(builder.Configuration), startupContext.Logger, startupContext.ErrorWriter);
WarnIfGlobalSettingsContainAppHostPath(globalSettingsFile, startupContext.ErrorWriter);
Expand Down
80 changes: 79 additions & 1 deletion src/Aspire.Cli/Utils/ConfigurationHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using Aspire.Cli.Configuration;
using Aspire.Cli.Resources;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.Logging;

namespace Aspire.Cli.Utils;

Expand All @@ -23,7 +24,7 @@ internal static class ConfigurationHelper
AllowTrailingCommas = true
};

internal static void RegisterSettingsFiles(IConfigurationBuilder configuration, DirectoryInfo workingDirectory, FileInfo globalSettingsFile)
internal static void RegisterSettingsFiles(IConfigurationBuilder configuration, DirectoryInfo workingDirectory, FileInfo globalSettingsFile, ILogger? logger = null)
Comment thread
mitchdenny marked this conversation as resolved.
Outdated
{
var currentDirectory = workingDirectory;

Expand All @@ -46,6 +47,42 @@ internal static void RegisterSettingsFiles(IConfigurationBuilder configuration,
var legacySettingsPath = BuildPathToSettingsJsonFile(currentDirectory.FullName);
if (File.Exists(legacySettingsPath))
{
// Eagerly migrate the legacy layout to aspire.config.json on startup so that
// even read-only commands (aspire doctor, aspire ls, aspire --version, etc.)
// move the workspace forward on the user's first run of a newer CLI. Without
// this, migration only happens when a command actively writes settings
// (aspire run/add/update/pipeline), leaving workspaces stuck on the legacy
// layout indefinitely for users who never invoke those commands.
// See https://github.com/microsoft/aspire/issues/15488.
if (LegacySettingsFileHasMigratableData(legacySettingsPath))
{
try
{
_ = AspireConfigFile.LoadOrCreate(currentDirectory.FullName);
var migratedPath = Path.Combine(currentDirectory.FullName, AspireConfigFile.FileName);
if (File.Exists(migratedPath))
{
logger?.LogInformation(
"Migrated legacy {LegacyPath} to {MigratedPath} on CLI startup.",
legacySettingsPath,
migratedPath);
localSettingsFile = new FileInfo(migratedPath);
break;
}
}
catch (Exception ex)
{
// Migration is best-effort during startup. If it fails (read-only
// directory, IO error, malformed legacy JSON), fall back to using the
// legacy file directly so the CLI still works. The next command that
// writes settings will retry the migration through its normal path.
logger?.LogWarning(
ex,
"Failed to migrate legacy {LegacyPath} to aspire.config.json on startup. Falling back to the legacy file.",
legacySettingsPath);
}
}

localSettingsFile = new FileInfo(legacySettingsPath);
break;
}
Expand All @@ -71,6 +108,47 @@ internal static string BuildPathToSettingsJsonFile(string workingDirectory)
return Path.Combine(workingDirectory, ".aspire", "settings.json");
}

/// <summary>
/// Returns <c>true</c> when a legacy <c>.aspire/settings.json</c> file contains an
/// <c>appHostPath</c> entry — the signal that this is a real AppHost workspace worth
/// migrating to <c>aspire.config.json</c>. Files that exist purely as directory-walking
/// stop markers (empty JSON object, whitespace, comment-only, or files that only carry
/// flat colon-keys awaiting in-place normalization) are not migrated because doing so
/// would needlessly materialize an <c>aspire.config.json</c> alongside them with no
/// meaningful content.
/// </summary>
private static bool LegacySettingsFileHasMigratableData(string legacySettingsPath)
{
try
{
var content = File.ReadAllText(legacySettingsPath);
if (string.IsNullOrWhiteSpace(content))
{
return false;
}

// Read appHostPath directly with JsonDocument to avoid coupling this check to the
// strict-typed AspireJsonConfiguration deserializer, which would fail for files
// that contain loosely-typed values (e.g. string "false" in a bool dictionary).
using var doc = JsonDocument.Parse(content, ParseOptions);
if (doc.RootElement.ValueKind != JsonValueKind.Object)
{
return false;
}

return doc.RootElement.TryGetProperty("appHostPath", out var appHostPathElement)
&& appHostPathElement.ValueKind == JsonValueKind.String
&& !string.IsNullOrEmpty(appHostPathElement.GetString());
}
catch
{
// Treat unreadable or malformed legacy files as not-migratable so startup does
// not throw. The user will see the same JSON parse error from the regular
// configuration registration path below if the file is genuinely broken.
return false;
}
}

internal static DirectoryInfo? GetLegacySettingsRootDirectory(FileInfo settingsFile)
{
if (!string.Equals(settingsFile.Name, AspireJsonConfiguration.FileName, StringComparison.OrdinalIgnoreCase))
Expand Down
101 changes: 101 additions & 0 deletions tests/Aspire.Cli.Tests/Configuration/ConfigurationHelperTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -171,4 +171,105 @@ public void GetWorkspaceAspireDirectory_UsesLegacySettingsParentDirectory()
Path.Combine(workspace.WorkspaceRoot.FullName, AspireJsonConfiguration.SettingsFolder),
aspireDirectory.FullName);
}

[Fact]
public void RegisterSettingsFiles_MigratesLegacySettingsToAspireConfigJsonOnStartup()
{
// Reproduces https://github.com/microsoft/aspire/issues/15488: a user upgrades the
// CLI and runs it against an existing AppHost workspace that only has the legacy
// .aspire/settings.json. The CLI must eagerly migrate the workspace to
// aspire.config.json on startup, regardless of which command the user runs (even
// read-only commands that never pass createSettingsFile: true to ProjectLocator).
using var workspace = TemporaryWorkspace.Create(outputHelper);

var legacyDir = workspace.CreateDirectory(AspireJsonConfiguration.SettingsFolder);
var legacySettingsPath = Path.Combine(legacyDir.FullName, AspireJsonConfiguration.FileName);
File.WriteAllText(legacySettingsPath, """
{
"appHostPath": "MyApp.csproj",
"channel": "stable"
}
""");

var aspireConfigPath = Path.Combine(workspace.WorkspaceRoot.FullName, AspireConfigFile.FileName);
Assert.False(File.Exists(aspireConfigPath), "Precondition: aspire.config.json should not yet exist.");

var globalDir = workspace.CreateDirectory("global-aspire");
var globalSettingsFile = new FileInfo(Path.Combine(globalDir.FullName, AspireConfigFile.FileName));

var builder = new ConfigurationBuilder();
ConfigurationHelper.RegisterSettingsFiles(builder, workspace.WorkspaceRoot, globalSettingsFile);

Assert.True(
File.Exists(aspireConfigPath),
"aspire.config.json should have been created by eager migration during RegisterSettingsFiles.");
}

[Fact]
public void RegisterSettingsFiles_DoesNotOverwriteExistingAspireConfigJson()
{
using var workspace = TemporaryWorkspace.Create(outputHelper);

// Both files present: the workspace was already migrated but the legacy file was
// retained (this is the documented transition state — see AspireConfigFile.LoadOrCreate
// and https://github.com/microsoft/aspire/issues/15239). Startup migration must not
// clobber the existing aspire.config.json or re-migrate from stale legacy data.
var aspireConfigPath = Path.Combine(workspace.WorkspaceRoot.FullName, AspireConfigFile.FileName);
var existingContent = """
{
"appHost": { "path": "Current.csproj" },
"channel": "daily"
}
""";
File.WriteAllText(aspireConfigPath, existingContent);

var legacyDir = workspace.CreateDirectory(AspireJsonConfiguration.SettingsFolder);
var legacySettingsPath = Path.Combine(legacyDir.FullName, AspireJsonConfiguration.FileName);
File.WriteAllText(legacySettingsPath, """
{ "appHostPath": "Stale.csproj", "channel": "stable" }
""");

var globalDir = workspace.CreateDirectory("global-aspire");
var globalSettingsFile = new FileInfo(Path.Combine(globalDir.FullName, AspireConfigFile.FileName));

var builder = new ConfigurationBuilder();
ConfigurationHelper.RegisterSettingsFiles(builder, workspace.WorkspaceRoot, globalSettingsFile);

Assert.Equal(existingContent, File.ReadAllText(aspireConfigPath));

var config = builder.Build();
Assert.Equal("Current.csproj", config["appHost:path"]);
Assert.Equal("daily", config["channel"]);
}

[Fact]
public void RegisterSettingsFiles_FallsBackToLegacyWhenMigrationFails()
{
using var workspace = TemporaryWorkspace.Create(outputHelper);

var legacyDir = workspace.CreateDirectory(AspireJsonConfiguration.SettingsFolder);
var legacySettingsPath = Path.Combine(legacyDir.FullName, AspireJsonConfiguration.FileName);
// Malformed JSON causes AspireConfigFile.LoadOrCreate → AspireJsonConfiguration.Load
// to throw. Startup must catch and continue registering the legacy file so the CLI
// doesn't crash before any command runs.
File.WriteAllText(legacySettingsPath, "{ this is not valid json");

Comment thread
mitchdenny marked this conversation as resolved.
Outdated
var globalDir = workspace.CreateDirectory("global-aspire");
var globalSettingsFile = new FileInfo(Path.Combine(globalDir.FullName, AspireConfigFile.FileName));

var builder = new ConfigurationBuilder();
var ex = Record.Exception(() =>
ConfigurationHelper.RegisterSettingsFiles(builder, workspace.WorkspaceRoot, globalSettingsFile));

// RegisterSettingsFiles itself shouldn't crash on a malformed legacy file. The
// downstream JSON registration may still throw via AddSettingsFile — that's pre-existing
// behavior and is the right signal for "your settings.json is broken". The migration
// step specifically must not introduce a new crash path.
var migratedPath = Path.Combine(workspace.WorkspaceRoot.FullName, AspireConfigFile.FileName);
Assert.False(File.Exists(migratedPath), "Malformed legacy file should not produce a partial aspire.config.json.");
// Either no exception (graceful), or the same InvalidOperationException previously
// thrown by AddSettingsFile for malformed JSON. Both are acceptable; what we're
// proving is that we didn't crash inside the new migration step itself.
Assert.True(ex is null or InvalidOperationException, $"Unexpected exception type: {ex?.GetType().FullName}");
}
}
Loading