Skip to content
Merged
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
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)
{
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
147 changes: 146 additions & 1 deletion tests/Aspire.Cli.Tests/Configuration/ConfigurationHelperTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using Aspire.Cli.Tests.Utils;
using Aspire.Cli.Utils;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.Logging.Abstractions;

namespace Aspire.Cli.Tests.Configuration;

Expand All @@ -22,7 +23,7 @@ private static IConfiguration BuildConfigurationFromSettingsFile(
var globalSettingsFile = new FileInfo(Path.Combine(globalDir.FullName, AspireConfigFile.FileName));

var builder = new ConfigurationBuilder();
ConfigurationHelper.RegisterSettingsFiles(builder, workspace.WorkspaceRoot, globalSettingsFile);
ConfigurationHelper.RegisterSettingsFiles(builder, workspace.WorkspaceRoot, globalSettingsFile, NullLogger.Instance);
return builder.Build();
}

Expand Down Expand Up @@ -171,4 +172,148 @@ 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, NullLogger.Instance);

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, NullLogger.Instance);

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_GuardRejectsUnparseableLegacyFile()
{
using var workspace = TemporaryWorkspace.Create(outputHelper);

var legacyDir = workspace.CreateDirectory(AspireJsonConfiguration.SettingsFolder);
var legacySettingsPath = Path.Combine(legacyDir.FullName, AspireJsonConfiguration.FileName);
// Unparseable JSON fails JsonDocument.Parse inside LegacySettingsFileHasMigratableData,
// so the guard short-circuits and returns false before migration is attempted. The
// downstream JSON registration via AddSettingsFile is what surfaces the parse error
// to the user, which is the pre-existing "your settings.json is broken" signal. The
// migration step itself must not introduce a new crash path on top of that.
File.WriteAllText(legacySettingsPath, "{ this is not valid json");

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, NullLogger.Instance));

// The guard rejected the file before migration ran, so aspire.config.json must not have
// been materialized at the workspace root.
var migratedPath = Path.Combine(workspace.WorkspaceRoot.FullName, AspireConfigFile.FileName);
Assert.False(File.Exists(migratedPath), "Unparseable 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 the guard prevented us from crashing inside the new migration step.
Assert.True(ex is null or InvalidOperationException, $"Unexpected exception type: {ex?.GetType().FullName}");
}

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

var legacyDir = workspace.CreateDirectory(AspireJsonConfiguration.SettingsFolder);
var legacySettingsPath = Path.Combine(legacyDir.FullName, AspireJsonConfiguration.FileName);
// This file is *parseable* JSON with a valid string appHostPath, so
// LegacySettingsFileHasMigratableData returns true and we enter the migration try
// block. However, "features" is typed Dictionary<string, bool> with a strict
// FlexibleBooleanDictionaryConverter, so passing a string for it causes
// AspireJsonConfiguration.Load (invoked by AspireConfigFile.LoadOrCreate) to throw a
// JsonException. That exception must be caught and we must fall back to registering
// the legacy file directly.
File.WriteAllText(legacySettingsPath, """
{
"appHostPath": "MyApp.csproj",
"features": "not-an-object"
}
""");

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, NullLogger.Instance));

Assert.Null(ex);

// Migration failed inside LoadOrCreate, so aspire.config.json must not exist at the
// workspace root. Its absence proves we hit the catch block and continued past the
// failed migration rather than half-writing a new config file.
var migratedPath = Path.Combine(workspace.WorkspaceRoot.FullName, AspireConfigFile.FileName);
Assert.False(File.Exists(migratedPath), "Failed migration should not produce a partial aspire.config.json.");

// The fallback registered the legacy file directly, so appHostPath remains readable
// from configuration via its flat key (the JSON source flattens nested objects with ':',
// but appHostPath is a root-level scalar so its key is unchanged).
var config = builder.Build();
Assert.Equal("MyApp.csproj", config["appHostPath"]);
}
}
2 changes: 1 addition & 1 deletion tests/Aspire.Cli.Tests/Utils/CliTestHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public static IServiceCollection CreateServiceCollection(TemporaryWorkspace work

var globalSettingsFilePath = Path.Combine(options.WorkingDirectory.FullName, ".aspire", "settings.global.json");
var globalSettingsFile = new FileInfo(globalSettingsFilePath);
ConfigurationHelper.RegisterSettingsFiles(configBuilder, options.WorkingDirectory, globalSettingsFile);
ConfigurationHelper.RegisterSettingsFiles(configBuilder, options.WorkingDirectory, globalSettingsFile, NullLogger.Instance);

var configuration = configBuilder.Build();
services.AddSingleton<IConfiguration>(configuration);
Expand Down
Loading