Fixes Bicep expand issues #2505 #2489 (#2516)

docs/CHANGELOG-v1.md

@@ -35,6 +35,11 @@ What's changed since pre-release v1.31.0-B0020:
     [#2508](https://github.com/Azure/PSRule.Rules.Azure/pull/2508)
   - Bump xunit to v2.6.1.
     [#2514](https://github.com/Azure/PSRule.Rules.Azure/pull/2514)
+- Bug fixes:
+  - Fixed dependency ordering with symbolic name by @BernieWhite.
+    [#2505](https://github.com/Azure/PSRule.Rules.Azure/issues/2505)
+  - Fixed nullable parameters for custom types by @BernieWhite.
+    [#2489](https://github.com/Azure/PSRule.Rules.Azure/issues/2489)
 
 ## v1.31.0-B0020 (pre-release)
 

src/PSRule.Rules.Azure/Data/Template/ResourceValue.cs

@@ -17,6 +17,8 @@ internal interface IResourceValue
 
         string Name { get; }
 
+        string SymbolicName { get; }
+
         string Type { get; }
 
         TemplateContext.CopyIndexState Copy { get; }
@@ -62,17 +64,20 @@ internal abstract class BaseResourceValue
     {
         private readonly HashSet<string> _Dependencies;
 
-        protected BaseResourceValue(string id, string name, string[] dependencies)
+        protected BaseResourceValue(string id, string name, string symbolicName, string[] dependencies)
         {
             Id = id;
             Name = name;
+            SymbolicName = symbolicName;
             _Dependencies = dependencies == null || dependencies.Length == 0 ? null : new HashSet<string>(dependencies, StringComparer.OrdinalIgnoreCase);
         }
 
         public string Id { get; }
 
         public string Name { get; }
 
+        public string SymbolicName { get; }
+
         public bool DependsOn(IResourceValue other, out int count)
         {
             count = 0;
@@ -82,15 +87,16 @@ public bool DependsOn(IResourceValue other, out int count)
             count = _Dependencies.Count;
             return _Dependencies.Contains(other.Id) ||
                 other.Copy != null && !string.IsNullOrEmpty(other.Copy.Name) && _Dependencies.Contains(other.Copy.Name) ||
-                !string.IsNullOrEmpty(other.Name) && _Dependencies.Contains(other.Name);
+                !string.IsNullOrEmpty(other.Name) && _Dependencies.Contains(other.Name) ||
+                !string.IsNullOrEmpty(other.SymbolicName) && _Dependencies.Contains(other.SymbolicName);
         }
     }
 
     [DebuggerDisplay("{Id}")]
     internal sealed class ResourceValue : BaseResourceValue, IResourceValue
     {
-        internal ResourceValue(string id, string name, string type, JObject value, string[] dependencies, TemplateContext.CopyIndexState copy)
-            : base(id, name, dependencies)
+        internal ResourceValue(string id, string name, string type, string symbolicName, JObject value, string[] dependencies, TemplateContext.CopyIndexState copy)
+            : base(id, name, symbolicName, dependencies)
         {
             Type = type;
             Value = value;
@@ -115,11 +121,11 @@ internal sealed class DeploymentValue : BaseResourceValue, IResourceValue, ILazy
         private readonly Lazy<JObject> _Value;
         private readonly Dictionary<string, ILazyValue> _Outputs;
 
-        internal DeploymentValue(string id, string name, JObject value, string[] dependencies, TemplateContext.CopyIndexState copy)
-            : this(id, name, () => value, dependencies, copy) { }
+        internal DeploymentValue(string id, string name, string symbolicName, JObject value, string[] dependencies, TemplateContext.CopyIndexState copy)
+            : this(id, name, symbolicName, () => value, dependencies, copy) { }
 
-        internal DeploymentValue(string id, string name, Func<JObject> value, string[] dependencies, TemplateContext.CopyIndexState copy)
-            : base(id, name, dependencies)
+        internal DeploymentValue(string id, string name, string symbolicName, Func<JObject> value, string[] dependencies, TemplateContext.CopyIndexState copy)
+            : base(id, name, symbolicName, dependencies)
         {
 
             _Value = new Lazy<JObject>(value);

src/PSRule.Rules.Azure/Data/Template/TemplateVisitor.cs

@@ -503,7 +503,7 @@ internal void EnterDeployment(string deploymentName, JObject template, bool isNe
 
                 var path = template.GetResourcePath(parentLevel: 2);
                 deployment.SetTargetInfo(TemplateFile, ParameterFile, path);
-                var deploymentValue = new DeploymentValue(string.Concat(ResourceGroup.Id, "/providers/", RESOURCETYPE_DEPLOYMENT, "/", deploymentName), deploymentName, deployment, null, null);
+                var deploymentValue = new DeploymentValue(string.Concat(ResourceGroup.Id, "/providers/", RESOURCETYPE_DEPLOYMENT, "/", deploymentName), deploymentName, null, deployment, null, null);
                 AddResource(deploymentValue);
                 _CurrentDeployment = deploymentValue;
                 _Deployment.Push(deploymentValue);
@@ -932,7 +932,9 @@ private static bool TryDefinition(JObject definition, out ITypeDefinition value)
             if (type == TypePrimitive.None)
                 return false;
 
-            value = new TypeDefinition(type, definition);
+            var isNullable = definition.TryBoolProperty(PROPERTY_NULLABLE, out var nullable) && nullable.HasValue;
+
+            value = new TypeDefinition(type, definition, isNullable);
             return true;
         }
 
@@ -1011,7 +1013,15 @@ private static bool TryParameterDefault(TemplateContext context, string paramete
         /// </summary>
         private static bool TryParameterNullable(TemplateContext context, string parameterName, JObject parameter)
         {
-            if (!parameter.TryBoolProperty(PROPERTY_NULLABLE, out var nullable) || !nullable.HasValue)
+            var isNullable = false;
+            if (parameter.TryGetProperty(PROPERTY_REF, out var typeRef) &&
+                context.TryDefinition(typeRef, out var definition) && definition.Nullable)
+                isNullable = true;
+
+            if (parameter.TryBoolProperty(PROPERTY_NULLABLE, out var nullable) && nullable.HasValue)
+                isNullable = true;
+
+            if (!isNullable)
                 return false;
 
             if (!TryParameterType(context, parameter, out var type))
@@ -1156,7 +1166,7 @@ private static IEnumerable<ResourceValue> ResourceExpand(TemplateContext context
                 if (!condition)
                     continue;
 
-                var r = ResourceInstance(context, instance, copyIndex);
+                var r = ResourceInstance(context, instance, copyIndex, symbolicName);
                 symbol?.Configure(r);
 
                 yield return r;
@@ -1168,7 +1178,7 @@ private static IEnumerable<ResourceValue> ResourceExpand(TemplateContext context
                 context.AddSymbol(symbol);
         }
 
-        private static ResourceValue ResourceInstance(TemplateContext context, JObject resource, TemplateContext.CopyIndexState copyIndex)
+        private static ResourceValue ResourceInstance(TemplateContext context, JObject resource, TemplateContext.CopyIndexState copyIndex, string symbolicName)
         {
             if (resource.TryGetProperty<JValue>(PROPERTY_NAME, out var nameValue))
                 resource[PROPERTY_NAME] = ResolveToken(context, nameValue);
@@ -1193,7 +1203,7 @@ private static ResourceValue ResourceInstance(TemplateContext context, JObject r
             var resourceId = ResourceHelper.CombineResourceId(subscriptionId, resourceGroupName, type, name);
             context.UpdateResourceScope(resource);
             resource[PROPERTY_ID] = resourceId;
-            return new ResourceValue(resourceId, name, type, resource, dependencies, copyIndex.Clone());
+            return new ResourceValue(resourceId, name, type, symbolicName, resource, dependencies, copyIndex.Clone());
         }
 
         private static string ResolveDeploymentScopeProperty(TemplateContext context, JObject resource, string propertyName, string defaultValue)

src/PSRule.Rules.Azure/Data/Template/TypeDefinition.cs

@@ -8,18 +8,23 @@ namespace PSRule.Rules.Azure.Data.Template
     internal interface ITypeDefinition
     {
         TypePrimitive Type { get; }
+
+        bool Nullable { get; }
     }
 
     internal sealed class TypeDefinition : ITypeDefinition
     {
-        public TypeDefinition(TypePrimitive type, JObject definition)
+        public TypeDefinition(TypePrimitive type, JObject definition, bool nullable)
         {
             Type = type;
             Definition = definition;
+            Nullable = nullable;
         }
 
         public TypePrimitive Type { get; }
 
         public JObject Definition { get; }
+
+        public bool Nullable { get; }
     }
 }

tests/PSRule.Rules.Azure.Tests/PSRule.Rules.Azure.Tests.csproj

@@ -191,6 +191,12 @@
     <None Update="Tests.Bicep.24.json">
       <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
     </None>
+    <None Update="Tests.Bicep.25.json">
+      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
+    </None>
+    <None Update="Tests.Bicep.26.json">
+      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
+    </None>
     <None Update="Tests.Bicep.27.json">
       <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
     </None>

tests/PSRule.Rules.Azure.Tests/ResourceDependencyComparerTests.cs

@@ -60,15 +60,42 @@ public void SortWithComparer()
             Assert.Equal("vnet-001/subnet-001", actual.Value["name"].Value<string>());
         }
 
+        [Fact]
+        public void SortSymbolicNameWithComparer()
+        {
+            var context = new TemplateContext();
+            var comparer = new ResourceDependencyComparer();
+            var resources = new IResourceValue[]
+            {
+                GetResourceValue(context, JObject.Parse("{ \"type\": \"Microsoft.Network/virtualNetworks\", \"name\": \"vnet-001\", \"dependsOn\": [ \"rt-002\" ] }"), symbolicName: "vnet-001"),
+                GetResourceValue(context, JObject.Parse("{ \"type\": \"Microsoft.Network/routeTables\", \"name\": \"rt-001\", \"dependsOn\": [ ] }"), symbolicName: "rt-001"),
+                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"),
+            };
+            Array.Sort(resources, comparer);
+
+            var actual = resources[0];
+            Assert.Equal("rt-001", actual.Value["name"].Value<string>());
+
+            actual = resources[1];
+            Assert.Equal("rt-002", actual.Value["name"].Value<string>());
+
+            actual = resources[2];
+            Assert.Equal("vnet-001", actual.Value["name"].Value<string>());
+
+            actual = resources[3];
+            Assert.Equal("vnet-001/subnet-001", actual.Value["name"].Value<string>());
+        }
+
         #region Helper methods
 
-        private static IResourceValue GetResourceValue(TemplateContext context, JObject resource)
+        private static IResourceValue GetResourceValue(TemplateContext context, JObject resource, string symbolicName = null)
         {
             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);
-            return new ResourceValue(resourceId, name, type, resource, dependencies, null);
+            return new ResourceValue(resourceId, name, type, symbolicName, resource, dependencies, null);
         }
 
         #endregion Helper methods

tests/PSRule.Rules.Azure.Tests/TemplateVisitorTests.cs

@@ -876,6 +876,32 @@ public void ArrayContains()
             Assert.Equal("Standard", resources[10]["properties"]["pricingTier"].Value<string>());
         }
 
+        [Fact]
+        public void Decriminators()
+        {
+            var resources = ProcessTemplate(GetSourcePath("Tests.Bicep.25.json"), null, out var templateContext);
+            Assert.Equal(2, resources.Length);
+
+            Assert.True(templateContext.RootDeployment.TryOutput("config", out JObject config));
+            Assert.Equal("bar", config["value"]["type"].Value<string>());
+            Assert.True(config["value"]["value"].Value<bool>());
+        }
+
+        [Fact]
+        public void SymbolicDependsOn()
+        {
+            var resources = ProcessTemplate(GetSourcePath("Tests.Bicep.26.json"), null, out var templateContext);
+            Assert.Equal(4, resources.Length);
+
+            var actual = resources[3].Value<JObject>();
+            Assert.Equal("Microsoft.Storage/storageAccounts", actual["type"].Value<string>());
+            Assert.Equal("example", actual["name"].Value<string>());
+            Assert.Equal(new string[] { "a", "b" }, actual["properties"]["fakeOptions"].Values<string>());
+
+            Assert.True(templateContext.RootDeployment.TryOutput("result", out JObject result));
+            Assert.Equal(new string[] { "a", "b" }, result["value"].Values<string>());
+        }
+
         [Fact]
         public void NullableParameters()
         {
@@ -885,6 +911,7 @@ public void NullableParameters()
             var actual = resources[2];
             Assert.Equal("Microsoft.Storage/storageAccounts", actual["type"].Value<string>());
             Assert.Equal("TLS1_2", actual["properties"]["minimumTlsVersion"].Value<string>());
+            Assert.Empty(actual["resources"][0]["properties"]["cors"]["corsRules"].Value<JArray>());
         }
 
         #region Helper methods

tests/PSRule.Rules.Azure.Tests/Tests.Bicep.14.bicep

@@ -3,11 +3,21 @@
 
 // Tests for custom types
 
+#disable-next-line no-unused-params
 param props custom = {
   name: 'abc'
   properties: {
     enabled: true
-    settings: []
+    settings: [
+      {
+        name: 'key1'
+        value: 1
+      }
+      {
+        name: 'key2'
+        value: 'value2'
+      }
+    ]
   }
 }
 
@@ -31,4 +41,3 @@ type custom = {
     settings: keyValue
   }
 }
-

tests/PSRule.Rules.Azure.Tests/Tests.Bicep.14.json

@@ -1,13 +1,12 @@
 {
   "$schema": "https://schema.management.azure.com/schemas/2019-04-01/deploymentTemplate.json#",
-  "languageVersion": "1.9-experimental",
+  "languageVersion": "2.0",
   "contentVersion": "1.0.0.0",
   "metadata": {
-    "_EXPERIMENTAL_WARNING": "Symbolic name support in ARM is experimental, and should be enabled for testing purposes only. Do not enable this setting for any production usage, or you may be unexpectedly broken at any time!",
     "_generator": {
       "name": "bicep",
-      "version": "0.14.46.61228",
-      "templateHash": "6078490264616788024"
+      "version": "0.23.1.45101",
+      "templateHash": "1423150262937573918"
     }
   },
   "definitions": {
@@ -16,10 +15,6 @@
       "prefixItems": [
         {
           "type": "object",
-          "required": [
-            "name",
-            "value"
-          ],
           "properties": {
             "name": {
               "type": "string"
@@ -31,10 +26,6 @@
         },
         {
           "type": "object",
-          "required": [
-            "name",
-            "value"
-          ],
           "properties": {
             "name": {
               "type": "string"
@@ -56,20 +47,12 @@
     },
     "custom": {
       "type": "object",
-      "required": [
-        "name",
-        "properties"
-      ],
       "properties": {
         "name": {
           "type": "string"
         },
         "properties": {
           "type": "object",
-          "required": [
-            "enabled",
-            "settings"
-          ],
           "properties": {
             "enabled": {
               "$ref": "#/definitions/enabled"
@@ -89,7 +72,16 @@
         "name": "abc",
         "properties": {
           "enabled": true,
-          "settings": []
+          "settings": [
+            {
+              "name": "key1",
+              "value": 1
+            },
+            {
+              "name": "key2",
+              "value": "value2"
+            }
+          ]
         }
       }
     }