Skip to content

Revert peer matching logic to fix database resource resolution regression #10456

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jul 17, 2025
Merged

Revert peer matching logic to fix database resource resolution regression #10456

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f8d4de9
Initial plan
Copilot Jul 16, 2025
5f7f778
Revert TryMatchAgainstResources to first-one-wins logic and remove fa…
Copilot Jul 16, 2025
45a1e14
Update src/Aspire.Dashboard/Model/ResourceOutgoingPeerResolver.cs
davidfowl Jul 16, 2025
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
28 changes: 4 additions & 24 deletions src/Aspire.Dashboard/Model/ResourceOutgoingPeerResolver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,43 +180,23 @@ internal static bool TryResolvePeerCore(IDictionary<string, ResourceViewModel> r
/// <summary>
/// Checks if a transformed peer address matches any of the resource addresses using their cached addresses.
/// Applies the same transformations to resource addresses for consistent matching.
/// Returns true only if exactly one resource matches; false if no matches or multiple matches are found.
/// Returns true and outputs the first matching resource if a match is found; otherwise, returns false.
/// </summary>
private static bool TryMatchAgainstResources(string peerAddress, IDictionary<string, ResourceViewModel> resources, [NotNullWhen(true)] out string? name, [NotNullWhen(true)] out ResourceViewModel? resourceMatch)
{
ResourceViewModel? foundResource = null;

foreach (var (_, resource) in resources)
{
foreach (var resourceAddress in resource.CachedAddresses)
{
if (DoesAddressMatch(resourceAddress, peerAddress))
{
if (foundResource is null)
{
foundResource = resource;
}
else if (!string.Equals(foundResource.Name, resource.Name, StringComparisons.ResourceName))
{
// Multiple different resources match - return false immediately
name = null;
resourceMatch = null;
return false;
}
break; // No need to check other addresses for this resource once we found a match
name = ResourceViewModel.GetResourceName(resource, resources);
resourceMatch = resource;
return true;
}
}
}

// Return true only if exactly one resource matched
if (foundResource is not null)
{
name = ResourceViewModel.GetResourceName(foundResource, resources);
resourceMatch = foundResource;
return true;
}

// Return false if no matches found
name = null;
resourceMatch = null;
return false;
Expand Down
96 changes: 16 additions & 80 deletions tests/Aspire.Dashboard.Tests/ResourceOutgoingPeerResolverTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,99 +358,35 @@ private static ResourceViewModel CreateResourceWithParameterValue(string name, s
}

[Fact]
public void MultipleResourcesMatch_SqlServerAddresses_ReturnsFalse()
{
// Arrange - Multiple SQL Server resources with same address
var resources = new Dictionary<string, ResourceViewModel>
{
["sqlserver1"] = CreateResource("sqlserver1", "localhost", 1433),
["sqlserver2"] = CreateResource("sqlserver2", "localhost", 1433)
};

// Act & Assert - Both resources would match "localhost:1433"
// so this should return false (ambiguous match)
Assert.False(TryResolvePeerName(resources, [KeyValuePair.Create("peer.service", "localhost:1433")], out var name));
Assert.Null(name);
}

[Fact]
public void MultipleResourcesMatch_RedisAddresses_ReturnsFalse()
{
// Arrange - Multiple Redis resources with equivalent addresses
var resources = new Dictionary<string, ResourceViewModel>
{
["redis-cache"] = CreateResource("redis-cache", "localhost", 6379),
["redis-session"] = CreateResource("redis-session", "localhost", 6379)
};

// Act & Assert - Both resources would match "localhost:6379"
// so this should return false (ambiguous match)
Assert.False(TryResolvePeerName(resources, [KeyValuePair.Create("peer.service", "localhost:6379")], out var name));
Assert.Null(name);
}

[Fact]
public void MultipleResourcesMatch_SqlServerCommaFormat_ReturnsFalse()
{
// Arrange - Multiple SQL Server resources where comma format would match both
var resources = new Dictionary<string, ResourceViewModel>
{
["sqldb1"] = CreateResource("sqldb1", "localhost", 1433),
["sqldb2"] = CreateResource("sqldb2", "localhost", 1433)
};

// Act & Assert - SQL Server comma format "localhost,1433" should match both resources
// so this should return false (ambiguous match)
Assert.False(TryResolvePeerName(resources, [KeyValuePair.Create("peer.service", "localhost,1433")], out var name));
Assert.Null(name);
}

[Fact]
public void MultipleResourcesMatch_MixedPortFormats_ReturnsFalse()
{
// Arrange - Resources with same logical address but different port formats
var resources = new Dictionary<string, ResourceViewModel>
{
["db-primary"] = CreateResource("db-primary", "dbserver", 5432),
["db-replica"] = CreateResource("db-replica", "dbserver", 5432)
};

// Act & Assert - Should be ambiguous since both resources have same address
Assert.False(TryResolvePeerName(resources, [KeyValuePair.Create("server.address", "dbserver"), KeyValuePair.Create("server.port", "5432")], out var name));
Assert.Null(name);
}

[Fact]
public void MultipleResourcesMatch_AddressTransformation_ReturnsFalse()
public void MultipleResourcesMatch_ViaTransformation_ReturnsFirstMatch()
{
// Arrange - Multiple resources with exact same address (not just after transformation)
// Arrange - Resources that match via address transformation
var resources = new Dictionary<string, ResourceViewModel>
{
["web-frontend"] = CreateResource("web-frontend", "localhost", 8080),
["web-backend"] = CreateResource("web-backend", "localhost", 8080)
["sql-primary"] = CreateResource("sql-primary", "localhost", 1433),
["sql-replica"] = CreateResource("sql-replica", "127.0.0.1", 1433)
};

// Act & Assert - Both resources have identical cached address "localhost:8080"
// so this should return false (ambiguous match)
Assert.False(TryResolvePeerName(resources, [KeyValuePair.Create("peer.service", "localhost:8080")], out var name));
Assert.Null(name);
// Act & Assert - Should return the first match found
Assert.True(TryResolvePeerName(resources, [KeyValuePair.Create("peer.service", "127.0.0.1:1433")], out var name));
Assert.Equal("sql-replica", name);
}

[Fact]
public void MultipleResourcesMatch_ViaTransformation_ReturnsFirstMatch()
public void MultipleResourcesSameAddress_ReturnsFirstMatch()
{
// Arrange - Resources that become ambiguous after address transformation
// Note: This test documents current behavior where transformation order matters
// Test to verify that "first one wins" logic is restored
var resources = new Dictionary<string, ResourceViewModel>
{
["sql-primary"] = CreateResource("sql-primary", "localhost", 1433),
["sql-replica"] = CreateResource("sql-replica", "127.0.0.1", 1433)
["database1"] = CreateResource("database1", "localhost", 5432),
["database2"] = CreateResource("database2", "localhost", 5432)
};

// Act & Assert - Due to transformation order, this currently finds sql-replica first
// before the transformation that would make sql-primary match as well
Assert.True(TryResolvePeerName(resources, [KeyValuePair.Create("peer.service", "127.0.0.1:1433")], out var name));
Assert.Equal("sql-replica", name);
// Should return the first match found (verifies regression fix)
Assert.True(TryResolvePeerName(resources, [KeyValuePair.Create("peer.service", "localhost:5432")], out var name));
Assert.NotNull(name);
// Should match one of the databases (first one found)
Assert.Contains(name, new[] { "database1", "database2" });
}

[Fact]
Expand Down
Loading