Skip to content

Commit 252130c

Browse files
CopilotdavidfowlJamesNK
authored
Fix TryMatchAgainstResources to return false for multiple resource matches (#10411)
* Initial plan * Update TryMatchAgainstResources to return false for multiple matches and add comprehensive tests Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com> * Simplify TryMatchAgainstResources to use foundResource instead of matchCount Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com> * Remove duplicated TryResolvePeerNameCore method and use TryResolvePeerCore instead Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com> * Use Name property comparison instead of reference equality for resource matching Co-authored-by: JamesNK <303201+JamesNK@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com> Co-authored-by: JamesNK <303201+JamesNK@users.noreply.github.com>
1 parent f611600 commit 252130c

File tree

2 files changed

+138
-31
lines changed

2 files changed

+138
-31
lines changed

src/Aspire.Dashboard/Model/ResourceOutgoingPeerResolver.cs

Lines changed: 26 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -144,35 +144,10 @@ private static bool ArePropertyValuesEquivalent(ResourceViewModel resource1, Res
144144

145145
public bool TryResolvePeer(KeyValuePair<string, string>[] attributes, out string? name, out ResourceViewModel? matchedResource)
146146
{
147-
var address = OtlpHelpers.GetPeerAddress(attributes);
148-
if (address != null)
149-
{
150-
// Apply transformers to the peer address cumulatively
151-
var transformedAddress = address;
152-
153-
// First check exact match
154-
if (TryMatchAgainstResources(transformedAddress, _resourceByName, out name, out matchedResource))
155-
{
156-
return true;
157-
}
158-
159-
// Then apply each transformer cumulatively and check
160-
foreach (var transformer in s_addressTransformers)
161-
{
162-
transformedAddress = transformer(transformedAddress);
163-
if (TryMatchAgainstResources(transformedAddress, _resourceByName, out name, out matchedResource))
164-
{
165-
return true;
166-
}
167-
}
168-
}
169-
170-
name = null;
171-
matchedResource = null;
172-
return false;
147+
return TryResolvePeerCore(_resourceByName, attributes, out name, out matchedResource);
173148
}
174149

175-
internal static bool TryResolvePeerNameCore(IDictionary<string, ResourceViewModel> resources, KeyValuePair<string, string>[] attributes, [NotNullWhen(true)] out string? name, [NotNullWhen(true)] out ResourceViewModel? resourceMatch)
150+
internal static bool TryResolvePeerCore(IDictionary<string, ResourceViewModel> resources, KeyValuePair<string, string>[] attributes, [NotNullWhen(true)] out string? name, [NotNullWhen(true)] out ResourceViewModel? resourceMatch)
176151
{
177152
var address = OtlpHelpers.GetPeerAddress(attributes);
178153
if (address != null)
@@ -205,22 +180,43 @@ internal static bool TryResolvePeerNameCore(IDictionary<string, ResourceViewMode
205180
/// <summary>
206181
/// Checks if a transformed peer address matches any of the resource addresses using their cached addresses.
207182
/// Applies the same transformations to resource addresses for consistent matching.
183+
/// Returns true only if exactly one resource matches; false if no matches or multiple matches are found.
208184
/// </summary>
209185
private static bool TryMatchAgainstResources(string peerAddress, IDictionary<string, ResourceViewModel> resources, [NotNullWhen(true)] out string? name, [NotNullWhen(true)] out ResourceViewModel? resourceMatch)
210186
{
187+
ResourceViewModel? foundResource = null;
188+
211189
foreach (var (_, resource) in resources)
212190
{
213191
foreach (var resourceAddress in resource.CachedAddresses)
214192
{
215193
if (DoesAddressMatch(resourceAddress, peerAddress))
216194
{
217-
name = ResourceViewModel.GetResourceName(resource, resources);
218-
resourceMatch = resource;
219-
return true;
195+
if (foundResource is null)
196+
{
197+
foundResource = resource;
198+
}
199+
else if (!string.Equals(foundResource.Name, resource.Name, StringComparisons.ResourceName))
200+
{
201+
// Multiple different resources match - return false immediately
202+
name = null;
203+
resourceMatch = null;
204+
return false;
205+
}
206+
break; // No need to check other addresses for this resource once we found a match
220207
}
221208
}
222209
}
223210

211+
// Return true only if exactly one resource matched
212+
if (foundResource is not null)
213+
{
214+
name = ResourceViewModel.GetResourceName(foundResource, resources);
215+
resourceMatch = foundResource;
216+
return true;
217+
}
218+
219+
// Return false if no matches found
224220
name = null;
225221
resourceMatch = null;
226222
return false;

tests/Aspire.Dashboard.Tests/ResourceOutgoingPeerResolverTests.cs

Lines changed: 112 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ public void NameAndDisplayNameDifferent_MultipleInstances_ReturnName()
217217

218218
private static bool TryResolvePeerName(IDictionary<string, ResourceViewModel> resources, KeyValuePair<string, string>[] attributes, out string? peerName)
219219
{
220-
return ResourceOutgoingPeerResolver.TryResolvePeerNameCore(resources, attributes, out peerName, out _);
220+
return ResourceOutgoingPeerResolver.TryResolvePeerCore(resources, attributes, out peerName, out _);
221221
}
222222

223223
[Fact]
@@ -357,6 +357,117 @@ private static ResourceViewModel CreateResourceWithParameterValue(string name, s
357357
properties: properties);
358358
}
359359

360+
[Fact]
361+
public void MultipleResourcesMatch_SqlServerAddresses_ReturnsFalse()
362+
{
363+
// Arrange - Multiple SQL Server resources with same address
364+
var resources = new Dictionary<string, ResourceViewModel>
365+
{
366+
["sqlserver1"] = CreateResource("sqlserver1", "localhost", 1433),
367+
["sqlserver2"] = CreateResource("sqlserver2", "localhost", 1433)
368+
};
369+
370+
// Act & Assert - Both resources would match "localhost:1433"
371+
// so this should return false (ambiguous match)
372+
Assert.False(TryResolvePeerName(resources, [KeyValuePair.Create("peer.service", "localhost:1433")], out var name));
373+
Assert.Null(name);
374+
}
375+
376+
[Fact]
377+
public void MultipleResourcesMatch_RedisAddresses_ReturnsFalse()
378+
{
379+
// Arrange - Multiple Redis resources with equivalent addresses
380+
var resources = new Dictionary<string, ResourceViewModel>
381+
{
382+
["redis-cache"] = CreateResource("redis-cache", "localhost", 6379),
383+
["redis-session"] = CreateResource("redis-session", "localhost", 6379)
384+
};
385+
386+
// Act & Assert - Both resources would match "localhost:6379"
387+
// so this should return false (ambiguous match)
388+
Assert.False(TryResolvePeerName(resources, [KeyValuePair.Create("peer.service", "localhost:6379")], out var name));
389+
Assert.Null(name);
390+
}
391+
392+
[Fact]
393+
public void MultipleResourcesMatch_SqlServerCommaFormat_ReturnsFalse()
394+
{
395+
// Arrange - Multiple SQL Server resources where comma format would match both
396+
var resources = new Dictionary<string, ResourceViewModel>
397+
{
398+
["sqldb1"] = CreateResource("sqldb1", "localhost", 1433),
399+
["sqldb2"] = CreateResource("sqldb2", "localhost", 1433)
400+
};
401+
402+
// Act & Assert - SQL Server comma format "localhost,1433" should match both resources
403+
// so this should return false (ambiguous match)
404+
Assert.False(TryResolvePeerName(resources, [KeyValuePair.Create("peer.service", "localhost,1433")], out var name));
405+
Assert.Null(name);
406+
}
407+
408+
[Fact]
409+
public void MultipleResourcesMatch_MixedPortFormats_ReturnsFalse()
410+
{
411+
// Arrange - Resources with same logical address but different port formats
412+
var resources = new Dictionary<string, ResourceViewModel>
413+
{
414+
["db-primary"] = CreateResource("db-primary", "dbserver", 5432),
415+
["db-replica"] = CreateResource("db-replica", "dbserver", 5432)
416+
};
417+
418+
// Act & Assert - Should be ambiguous since both resources have same address
419+
Assert.False(TryResolvePeerName(resources, [KeyValuePair.Create("server.address", "dbserver"), KeyValuePair.Create("server.port", "5432")], out var name));
420+
Assert.Null(name);
421+
}
422+
423+
[Fact]
424+
public void MultipleResourcesMatch_AddressTransformation_ReturnsFalse()
425+
{
426+
// Arrange - Multiple resources with exact same address (not just after transformation)
427+
var resources = new Dictionary<string, ResourceViewModel>
428+
{
429+
["web-frontend"] = CreateResource("web-frontend", "localhost", 8080),
430+
["web-backend"] = CreateResource("web-backend", "localhost", 8080)
431+
};
432+
433+
// Act & Assert - Both resources have identical cached address "localhost:8080"
434+
// so this should return false (ambiguous match)
435+
Assert.False(TryResolvePeerName(resources, [KeyValuePair.Create("peer.service", "localhost:8080")], out var name));
436+
Assert.Null(name);
437+
}
438+
439+
[Fact]
440+
public void MultipleResourcesMatch_ViaTransformation_ReturnsFirstMatch()
441+
{
442+
// Arrange - Resources that become ambiguous after address transformation
443+
// Note: This test documents current behavior where transformation order matters
444+
var resources = new Dictionary<string, ResourceViewModel>
445+
{
446+
["sql-primary"] = CreateResource("sql-primary", "localhost", 1433),
447+
["sql-replica"] = CreateResource("sql-replica", "127.0.0.1", 1433)
448+
};
449+
450+
// Act & Assert - Due to transformation order, this currently finds sql-replica first
451+
// before the transformation that would make sql-primary match as well
452+
Assert.True(TryResolvePeerName(resources, [KeyValuePair.Create("peer.service", "127.0.0.1:1433")], out var name));
453+
Assert.Equal("sql-replica", name);
454+
}
455+
456+
[Fact]
457+
public void SingleResourceAfterTransformation_ReturnsTrue()
458+
{
459+
// Arrange - Only one resource that matches after address transformation
460+
var resources = new Dictionary<string, ResourceViewModel>
461+
{
462+
["unique-service"] = CreateResource("unique-service", "localhost", 8080),
463+
["other-service"] = CreateResource("other-service", "remotehost", 9090)
464+
};
465+
466+
// Act & Assert - Only the first resource should match "127.0.0.1:8080" after transformation
467+
Assert.True(TryResolvePeerName(resources, [KeyValuePair.Create("peer.service", "127.0.0.1:8080")], out var name));
468+
Assert.Equal("unique-service", name);
469+
}
470+
360471
private sealed class MockDashboardClient(Task<ResourceViewModelSubscription> subscribeResult) : IDashboardClient
361472
{
362473
public bool IsEnabled => true;

0 commit comments

Comments
 (0)