Skip to content

Commit

Permalink
Handle missing settings.json for Codespaces/Devcontainers (#6880)
Browse files Browse the repository at this point in the history
  • Loading branch information
mitchdenny authored Dec 7, 2024
1 parent a4b55cf commit 1be73ac
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 19 deletions.
3 changes: 2 additions & 1 deletion .devcontainer/contributing/devcontainer.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@
"extensions": [
"ms-dotnettools.csdevkit",
"ms-azuretools.vscode-bicep",
"ms-azuretools.azure-dev"
"ms-azuretools.azure-dev",
"GitHub.copilot"
],
"settings": {
"remote.autoForwardPorts": true,
Expand Down
6 changes: 4 additions & 2 deletions src/Aspire.Hosting/Dashboard/DashboardLifecycleHook.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@ internal sealed class DashboardLifecycleHook(IConfiguration configuration,
IHostApplicationLifetime hostApplicationLifetime,
CodespacesUrlRewriter codespaceUrlRewriter,
IOptions<CodespacesOptions> codespacesOptions,
IOptions<DevcontainersOptions> devcontainersOptions) : IDistributedApplicationLifecycleHook, IAsyncDisposable
IOptions<DevcontainersOptions> devcontainersOptions,
DevcontainerSettingsWriter settingsWriter
) : IDistributedApplicationLifecycleHook, IAsyncDisposable
{
private Task? _dashboardLogsTask;
private CancellationTokenSource? _dashboardLogsCts;
Expand Down Expand Up @@ -249,7 +251,7 @@ private void ConfigureAspireDashboardResource(IResource dashboardResource)

if (codespacesOptions.Value.IsCodespace || devcontainersOptions.Value.IsDevcontainer)
{
await DevcontainerPortForwardingHelper.SetPortAttributesAsync(
await settingsWriter.SetPortAttributesAsync(
firstDashboardUrl.Port,
firstDashboardUrl.Scheme,
"aspire-dashboard",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,14 @@ internal sealed class DevcontainerPortForwardingLifecycleHook : IDistributedAppl
private readonly ILogger _hostingLogger;
private readonly IOptions<CodespacesOptions> _codespacesOptions;
private readonly IOptions<DevcontainersOptions> _devcontainersOptions;
private readonly DevcontainerSettingsWriter _settingsWriter;

public DevcontainerPortForwardingLifecycleHook(ILoggerFactory loggerFactory, IOptions<CodespacesOptions> codespacesOptions, IOptions<DevcontainersOptions> devcontainersOptions)
public DevcontainerPortForwardingLifecycleHook(ILoggerFactory loggerFactory, IOptions<CodespacesOptions> codespacesOptions, IOptions<DevcontainersOptions> devcontainersOptions, DevcontainerSettingsWriter settingsWriter)
{
_hostingLogger = loggerFactory.CreateLogger("Aspire.Hosting");
_codespacesOptions = codespacesOptions;
_devcontainersOptions = devcontainersOptions;
_settingsWriter = settingsWriter;
}

public async Task AfterEndpointsAllocatedAsync(DistributedApplicationModel appModel, CancellationToken cancellationToken)
Expand Down Expand Up @@ -60,7 +62,7 @@ public async Task AfterEndpointsAllocatedAsync(DistributedApplicationModel appMo
// and writing it each time. Its like this for now beause I need to use the logic
// in a few places (here and when we print out the Dashboard URL) - but will need
// to come back and optimize this to support some kind of batching.
await DevcontainerPortForwardingHelper.SetPortAttributesAsync(
await _settingsWriter.SetPortAttributesAsync(
endpoint.AllocatedEndpoint!.Port,
endpoint.UriScheme,
$"{resource.Name}-{endpoint.Name}",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,31 +3,36 @@

using System.Globalization;
using System.Text.Json.Nodes;
using Aspire.Hosting.Devcontainers.Codespaces;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;

namespace Aspire.Hosting.Devcontainers;

internal class DevcontainerPortForwardingHelper
internal class DevcontainerSettingsWriter(ILogger<DevcontainerSettingsWriter> logger, IOptions<CodespacesOptions> codespaceOptions, IOptions<DevcontainersOptions> devcontainerOptions)
{
private const string CodespaceSettingsPath = "/home/vscode/.vscode-remote/data/Machine/settings.json";
private const string LocalDevcontainerSettingsPath = "/home/vscode/.vscode-server/data/Machine/settings.json";
private const string PortAttributesFieldName = "remote.portsAttributes";
private const int WriteLockTimeoutMs = 2000;
private static readonly SemaphoreSlim s_writeLock = new SemaphoreSlim(1);
private const int WriteLockTimeoutMs = 2000;
private readonly SemaphoreSlim _writeLock = new SemaphoreSlim(1);

public static async Task SetPortAttributesAsync(int port, string protocol, string label, CancellationToken cancellationToken = default)
public async Task SetPortAttributesAsync(int port, string protocol, string label, CancellationToken cancellationToken = default)
{
ArgumentNullException.ThrowIfNullOrEmpty(protocol);
ArgumentNullException.ThrowIfNullOrEmpty(label);

var settingsPath = GetSettingsPath();

var acquired = await s_writeLock.WaitAsync(WriteLockTimeoutMs, cancellationToken).ConfigureAwait(false);
var acquired = await _writeLock.WaitAsync(WriteLockTimeoutMs, cancellationToken).ConfigureAwait(false);

if (!acquired)
{
throw new DistributedApplicationException($"Failed to acquire semaphore for settings file: {settingsPath}");
}

await EnsureSettingsFileExists(settingsPath, cancellationToken).ConfigureAwait(false);

var settingsContent = await File.ReadAllTextAsync(settingsPath, cancellationToken).ConfigureAwait(false);
var settings = (JsonObject)JsonObject.Parse(settingsContent)!;

Expand Down Expand Up @@ -62,24 +67,46 @@ public static async Task SetPortAttributesAsync(int port, string protocol, strin
settingsContent = settings.ToString();
await File.WriteAllTextAsync(settingsPath, settingsContent, cancellationToken).ConfigureAwait(false);

s_writeLock.Release();
_writeLock.Release();

static string GetSettingsPath()
string GetSettingsPath()
{
// For some reason the machine settings path is different between Codespaces and local Devcontainers
// so we probe for it here. We could use options to figure out which one to look for here but that
// would require taking a dependency on the options system here which seems like overkill.
if (File.Exists(CodespaceSettingsPath))
// so we decide which one to use here based on the options.
if (codespaceOptions.Value.IsCodespace)
{
return CodespaceSettingsPath;
}
else if (File.Exists(LocalDevcontainerSettingsPath))
else if (devcontainerOptions.Value.IsDevcontainer)
{
return LocalDevcontainerSettingsPath;
}
else
{
throw new DistributedApplicationException("Could not find a devcontainer settings file.");
throw new DistributedApplicationException("Codespaces or Devcontainer not detected.");
}
}

async Task EnsureSettingsFileExists(string path, CancellationToken cancellationToken)
{
try
{
if (!File.Exists(path))
{
// The extra ceremony here is to avoid accidentally overwriting the file if it was
// created after we checked for its existence. If the file exists when we go to write
// it then we will throw and log a warning, but otherwise continue executing.
using var stream = File.Open(path, FileMode.CreateNew);
using var writer = new StreamWriter(stream);
await writer.WriteAsync("{}".AsMemory(), cancellationToken).ConfigureAwait(false);
}
}
catch (IOException ex) when (ex.Message == $"The file '{path}' already exists.")
{
// This is OK, but it should be rare enough that if it starts happening we probably
// want to know about it in logs that end users submit so we know to take a closer
// look at what is going on.
logger.LogWarning("Race condition detected when creating Devcontainer settings file: {Path}", path);
}
}
}
Expand Down
1 change: 1 addition & 0 deletions src/Aspire.Hosting/DistributedApplicationBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,7 @@ public DistributedApplicationBuilder(DistributedApplicationOptions options)
_innerBuilder.Services.AddSingleton<CodespacesUrlRewriter>();
_innerBuilder.Services.AddHostedService<CodespacesResourceUrlRewriterService>();
_innerBuilder.Services.TryAddEnumerable(ServiceDescriptor.Singleton<IConfigureOptions<DevcontainersOptions>, ConfigureDevcontainersOptions>());
_innerBuilder.Services.AddSingleton<DevcontainerSettingsWriter>();
_innerBuilder.Services.TryAddLifecycleHook<DevcontainerPortForwardingLifecycleHook>();

Eventing.Subscribe<BeforeStartEvent>(BuiltInDistributedApplicationEventSubscriptionHandlers.InitializeDcpAnnotations);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ private static DashboardLifecycleHook CreateHook(
{
codespacesOptions ??= Options.Create(new CodespacesOptions());
devcontainersOptions ??= Options.Create(new DevcontainersOptions());

var settingsWriter = new DevcontainerSettingsWriter(NullLogger<DevcontainerSettingsWriter>.Instance, codespacesOptions, devcontainersOptions);
var rewriter = new CodespacesUrlRewriter(codespacesOptions);

return new DashboardLifecycleHook(
Expand All @@ -124,7 +124,8 @@ private static DashboardLifecycleHook CreateHook(
new TestHostApplicationLifetime(),
rewriter,
codespacesOptions,
devcontainersOptions
devcontainersOptions,
settingsWriter
);
}

Expand Down

0 comments on commit 1be73ac

Please sign in to comment.