Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 5, 2025

Ports PR #12692 to this branch. Replaces static SecretsStore methods with a dependency injection-based factory pattern that provides thread-safe synchronization for user secrets file access.

Architecture Changes

Factory & Interfaces:

  • IUserSecretsManager - interface for user secrets operations
  • UserSecretsManagerFactory - singleton factory with lock-based instance caching
  • NoopUserSecretsManager - null object pattern when user secrets not configured
  • JsonFlattener - utility for JSON flattening/unflattening (extracted from DeploymentStateManagerBase)

Thread Safety:

  • Each UserSecretsManager instance has its own SemaphoreSlim
  • Factory ensures same instance per normalized file path
  • Prevents race conditions when multiple resources generate passwords concurrently

Core Changes

  • DistributedApplicationBuilder - registers IUserSecretsManager singleton in DI container
  • UserSecretsParameterDefault - factory-based constructor with optional custom factory for test isolation
  • UserSecretsDeploymentStateManager - injects IUserSecretsManager via constructor
  • VersionCheckService - injects IUserSecretsManager instead of using static methods
  • Removed src/Shared/SecretsStore.cs and static method calls

Test Updates

  • Added 6 concurrency tests validating thread-safe behavior
  • Moved JsonFlattenerTests from Azure.Tests to Aspire.Hosting.Tests
  • Updated all test helpers to use new factory pattern
// Before: static method with race conditions
SecretsStore.TrySetUserSecret(assembly, "Parameters:password", value);

// After: DI-injected manager with per-instance locking
_userSecretsManager.TrySetSecret("Parameters:password", value);
Original prompt

Port #12692 to this branch


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI changed the title [WIP] Port changes from pull request #12692 Refactor user secrets to DI-based factory with thread-safe synchronization Nov 5, 2025
Copilot AI requested a review from davidfowl November 5, 2025 04:51
Copilot finished work on behalf of davidfowl November 5, 2025 04:51
@github-actions
Copy link
Contributor

github-actions bot commented Nov 5, 2025

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

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

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 12695"

@davidfowl
Copy link
Member

This is important but we should wait bake it more in main before the backport.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants