diff --git a/docs/CHANGELOG-v1.md b/docs/CHANGELOG-v1.md
index 9fbb700e1a..2bf8a7e7f9 100644
--- a/docs/CHANGELOG-v1.md
+++ b/docs/CHANGELOG-v1.md
@@ -29,6 +29,12 @@ See [upgrade notes][1] for helpful information when upgrading from previous vers
## Unreleased
+What's changed since v1.41.2:
+
+- Bug fixes:
+ - Fixed ordering of symbolic copy loop dependencies by @BernieWhite.
+ [#3257](https://github.com/Azure/PSRule.Rules.Azure/issues/3257)
+
## v1.41.2
What's changed since v1.41.1:
diff --git a/src/PSRule.Rules.Azure/Data/Template/ResourceDependencyGraph.cs b/src/PSRule.Rules.Azure/Data/Template/ResourceDependencyGraph.cs
index bba94a1f0f..7cd694e047 100644
--- a/src/PSRule.Rules.Azure/Data/Template/ResourceDependencyGraph.cs
+++ b/src/PSRule.Rules.Azure/Data/Template/ResourceDependencyGraph.cs
@@ -5,116 +5,111 @@
using System.Collections.Generic;
using System.Diagnostics;
-namespace PSRule.Rules.Azure.Data.Template
+namespace PSRule.Rules.Azure.Data.Template;
+
+///
+/// A graph that tracks resource dependencies in scope of a deployment.
+///
+internal sealed class ResourceDependencyGraph
{
+ private readonly Dictionary _ById = new(StringComparer.OrdinalIgnoreCase);
+ private readonly Dictionary _ByName = new(StringComparer.OrdinalIgnoreCase);
+ private readonly Dictionary _BySymbolicName = new(StringComparer.OrdinalIgnoreCase);
+ private readonly Dictionary> _ByCopyName = new(StringComparer.OrdinalIgnoreCase);
+
+ [DebuggerDisplay("{Resource.Id}")]
+ private sealed class Node(IResourceValue resource, string[] dependencies)
+ {
+ internal readonly IResourceValue Resource = resource;
+ internal readonly string[] Dependencies = dependencies;
+ }
+
///
- /// A graph that tracks resource dependencies in scope of a deployment.
+ /// Sort the provided resources based on dependency graph.
///
- internal sealed class ResourceDependencyGraph
+ /// The resources to sort.
+ /// An ordered set of resources.
+ internal IResourceValue[] Sort(IResourceValue[] resources)
{
- private readonly Dictionary _ById = new(StringComparer.OrdinalIgnoreCase);
- private readonly Dictionary _ByName = new(StringComparer.OrdinalIgnoreCase);
- private readonly Dictionary _BySymbolicName = new(StringComparer.OrdinalIgnoreCase);
- private readonly Dictionary> _ByCopyName = new(StringComparer.OrdinalIgnoreCase);
+ if (resources == null || resources.Length <= 1)
+ return resources;
- [DebuggerDisplay("{Resource.Id}")]
- private sealed class Node
- {
- internal readonly IResourceValue Resource;
- internal readonly string[] Dependencies;
+ var stack = new List(resources.Length);
+ var visited = new HashSet(StringComparer.OrdinalIgnoreCase);
- public Node(IResourceValue resource, string[] dependencies)
+ foreach (var resource in resources)
+ {
+ if (TryGet(resource.Id, out var item))
{
- Resource = resource;
- Dependencies = dependencies;
+ Visit(item, visited, stack);
}
- }
-
- ///
- /// Sort the provided resources based on dependency graph.
- ///
- /// The resources to sort.
- /// An ordered set of resources.
- internal IResourceValue[] Sort(IResourceValue[] resources)
- {
- if (resources == null || resources.Length <= 1)
- return resources;
-
- var stack = new List(resources.Length);
- var visited = new HashSet(StringComparer.OrdinalIgnoreCase);
-
- foreach (var resource in resources)
+ else
{
- if (TryGet(resource.Id, out var item))
- {
- Visit(item, visited, stack);
- }
- else
- {
- stack.Add(resource);
- visited.Add(resource.Id);
- }
+ stack.Add(resource);
+ visited.Add(resource.Id);
}
- return stack.ToArray();
}
+ return [.. stack];
+ }
- ///
- /// Add a resource to the graph.
- ///
- /// The resource node to add to the graph.
- /// Any dependencies for the node.
- internal void Track(IResourceValue resource, string[] dependencies)
- {
- if (resource == null)
- return;
+ ///
+ /// Add a resource to the graph.
+ ///
+ /// The resource node to add to the graph.
+ /// Any dependencies for the node.
+ internal void Track(IResourceValue resource, string[] dependencies)
+ {
+ if (resource == null)
+ return;
- var item = new Node(resource, dependencies);
- _ById[resource.Id] = item;
- _ByName[resource.Name] = item;
+ var item = new Node(resource, dependencies);
+ _ById[resource.Id] = item;
+ _ByName[resource.Name] = item;
- if (!string.IsNullOrEmpty(resource.SymbolicName))
- _BySymbolicName[resource.SymbolicName] = item;
+ if (!string.IsNullOrEmpty(resource.SymbolicName))
+ _BySymbolicName[resource.SymbolicName] = item;
- if (resource.Copy != null && !string.IsNullOrEmpty(resource.Copy.Name))
+ if (resource.Copy != null && !string.IsNullOrEmpty(resource.Copy.Name))
+ {
+ if (!_ByCopyName.TryGetValue(resource.Copy.Name, out var copyItems))
{
- if (!_ByCopyName.TryGetValue(resource.Copy.Name, out var copyItems))
- {
- copyItems = new List();
- _ByCopyName[resource.Copy.Name] = copyItems;
- }
- copyItems.Add(item);
+ copyItems = [];
+ _ByCopyName[resource.Copy.Name] = copyItems;
}
+ copyItems.Add(item);
}
+ }
- private bool TryGet(string key, out Node item)
- {
- return _ById.TryGetValue(key, out item) ||
- _ByName.TryGetValue(key, out item) ||
- _BySymbolicName.TryGetValue(key, out item);
- }
+ private bool TryGet(string key, out Node item)
+ {
+ return _ById.TryGetValue(key, out item) ||
+ _ByName.TryGetValue(key, out item) ||
+ _BySymbolicName.TryGetValue(key, out item);
+ }
- ///
- /// Traverse a node and dependencies.
- ///
- private void Visit(Node source, HashSet visited, List stack)
- {
- if (visited.Contains(source.Resource.Id))
- return;
+ ///
+ /// Traverse a node and dependencies.
+ ///
+ private void Visit(Node source, HashSet visited, List stack)
+ {
+ if (visited.Contains(source.Resource.Id))
+ return;
- visited.Add(source.Resource.Id);
- for (var i = 0; source.Dependencies != null && i < source.Dependencies.Length; i++)
+ visited.Add(source.Resource.Id);
+ for (var i = 0; source.Dependencies != null && i < source.Dependencies.Length; i++)
+ {
+ if (_ByCopyName.TryGetValue(source.Dependencies[i], out var countItems))
{
- if (TryGet(source.Dependencies[i], out var item))
+ foreach (var countItem in countItems)
{
- Visit(item, visited, stack);
- }
- else if (_ByCopyName.TryGetValue(source.Dependencies[i], out var countItems))
- {
- foreach (var countItem in countItems)
- Visit(countItem, visited, stack);
+ Visit(countItem, visited, stack);
}
}
- stack.Add(source.Resource);
+ else if (TryGet(source.Dependencies[i], out var item))
+ {
+ Visit(item, visited, stack);
+ }
}
+ stack.Add(source.Resource);
}
}
diff --git a/tests/PSRule.Rules.Azure.Tests/DependencyMapTests.cs b/tests/PSRule.Rules.Azure.Tests/DependencyMapTests.cs
index 9a735dc842..d6623ac6d4 100644
--- a/tests/PSRule.Rules.Azure.Tests/DependencyMapTests.cs
+++ b/tests/PSRule.Rules.Azure.Tests/DependencyMapTests.cs
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.
+using System.Linq;
using Newtonsoft.Json.Linq;
using PSRule.Rules.Azure.Data.Template;
using static PSRule.Rules.Azure.Data.Template.TemplateVisitor;
@@ -10,7 +11,7 @@ namespace PSRule.Rules.Azure
public sealed class DependencyMapTests
{
[Fact]
- public void SortWithComparer()
+ public void SortDependencies_WithoutSymbolicName_ShouldReorderResources()
{
var context = new TemplateContext();
var resources = new IResourceValue[]
@@ -36,13 +37,13 @@ public void SortWithComparer()
// https://github.com/Azure/PSRule.Rules.Azure/issues/2255
context = new TemplateContext();
- resources = new IResourceValue[]
- {
+ resources =
+ [
GetResourceValue(context, JObject.Parse("{ \"type\": \"Microsoft.Network/virtualNetworks\", \"name\": \"vnet-001\", \"dependsOn\": [ \"/subscriptions/ffffffff-ffff-ffff-ffff-ffffffffffff/resourceGroups/ps-rule-test-rg/providers/Microsoft.Network/routeTables/rt-002\" ] }")),
GetResourceValue(context, JObject.Parse("{ \"type\": \"Microsoft.Network/routeTables\", \"name\": \"rt-001\", \"dependsOn\": [ ] }")),
GetResourceValue(context, JObject.Parse("{ \"type\": \"Microsoft.Network/routeTables\", \"name\": \"rt-002\" }")),
GetResourceValue(context, JObject.Parse("{ \"type\": \"Microsoft.Network/virtualNetworks/subnets\", \"name\": \"vnet-001/subnet-001\", \"dependsOn\": [ \"/subscriptions/ffffffff-ffff-ffff-ffff-ffffffffffff/resourceGroups/ps-rule-test-rg/providers/Microsoft.Network/virtualNetworks/vnet-001\" ] }")),
- };
+ ];
resources = context.SortDependencies(resources);
actual = resources[0];
@@ -59,7 +60,7 @@ public void SortWithComparer()
}
[Fact]
- public void SortSymbolicNameWithComparer()
+ public void SortDependencies_WithSymbolicName_ShouldReorderResources()
{
var context = new TemplateContext();
var resources = new IResourceValue[]
@@ -69,30 +70,65 @@ public void SortSymbolicNameWithComparer()
GetResourceValue(context, JObject.Parse("{ \"type\": \"Microsoft.Network/routeTables\", \"name\": \"rt-002\" }"), symbolicName: "rt-002"),
GetResourceValue(context, JObject.Parse("{ \"type\": \"Microsoft.Network/virtualNetworks/subnets\", \"name\": \"vnet-001/subnet-001\", \"dependsOn\": [ \"vnet-001\" ] }"), symbolicName: "vnet-001/subnet-001"),
};
- resources = context.SortDependencies(resources);
- var actual = resources[0];
- Assert.Equal("rt-002", actual.Value["name"].Value());
-
- actual = resources[1];
- Assert.Equal("vnet-001", actual.Value["name"].Value());
+ var actual = context.SortDependencies(resources).Select(r =>
+ {
+ return r.Value["name"].Value();
+ }).ToArray();
+
+ Assert.Equal(
+ [
+ "rt-002",
+ "vnet-001",
+ "rt-001",
+ "vnet-001/subnet-001"
+ ], actual);
+ }
- actual = resources[2];
- Assert.Equal("rt-001", actual.Value["name"].Value());
+ ///
+ /// If a dependency is a copy with a symbolic name all instances of the dependency should be reordered above the dependant resource.
+ ///
+ [Fact]
+ public void SortDependencies_WithMultipleResourceCopies_ShouldReorderAllResources()
+ {
+ var context = new TemplateContext();
+ var resources = new IResourceValue[]
+ {
+ GetResourceValue(context, JObject.Parse("{ \"type\": \"Microsoft.Network/virtualNetworks\", \"name\": \"vnet-001\", \"dependsOn\": [ \"routeTables\" ] }"), symbolicName: "vnet-001"),
+ GetResourceValue(context, JObject.Parse("{ \"type\": \"Microsoft.Network/routeTables\", \"name\": \"rt-001\", \"dependsOn\": [ ] }"), symbolicName: "routeTables", copyInstance: 0),
+ GetResourceValue(context, JObject.Parse("{ \"type\": \"Microsoft.Network/routeTables\", \"name\": \"rt-002\" }"), symbolicName: "routeTables", copyInstance: 1),
+ GetResourceValue(context, JObject.Parse("{ \"type\": \"Microsoft.Network/virtualNetworks/subnets\", \"name\": \"vnet-001/subnet-001\", \"dependsOn\": [ \"vnet-001\" ] }"), symbolicName: "vnet-001/subnet-001"),
+ };
- actual = resources[3];
- Assert.Equal("vnet-001/subnet-001", actual.Value["name"].Value());
+ var actual = context.SortDependencies(resources).Select(r =>
+ {
+ return r.Value["name"].Value();
+ }).ToArray();
+
+ Assert.Equal(
+ [
+ "rt-001",
+ "rt-002",
+ "vnet-001",
+ "vnet-001/subnet-001"
+ ], actual);
}
#region Helper methods
- private static IResourceValue GetResourceValue(TemplateContext context, JObject resource, string symbolicName = null)
+ private static IResourceValue GetResourceValue(TemplateContext context, JObject resource, string symbolicName = null, int? copyInstance = null)
{
+ var copy = copyInstance == null || symbolicName == null ? null : new TemplateContext.CopyIndexState
+ {
+ Name = symbolicName,
+ Index = copyInstance.Value
+ };
+
resource.TryGetProperty("name", out var name);
resource.TryGetProperty("type", out var type);
resource.TryGetDependencies(out var dependencies);
var resourceId = ResourceHelper.CombineResourceId(context.Subscription.SubscriptionId, context.ResourceGroup.Name, type, name);
- var result = new ResourceValue(resourceId, name, type, symbolicName, resource, null);
+ var result = new ResourceValue(resourceId, name, type, symbolicName, resource, copy);
context.TrackDependencies(result, dependencies);
return result;
}