Fixed Resource group ID is incorrect under subscription scope #3198

docs/CHANGELOG-v1.md

@@ -31,12 +31,17 @@ See [upgrade notes][1] for helpful information when upgrading from previous vers
 
 What's changed since pre-release v1.40.0-B0147:
 
+- General improvements:
+  - Added first time contributor guide in docs by @that-ar-guy.
+    [#3097](https://github.com/Azure/PSRule.Rules.Azure/issues/3097)
 - Engineering:
   - Quality updates to rule documentation by @BernieWhite.
     [#3102](https://github.com/Azure/PSRule.Rules.Azure/issues/3102)
 - Bug fixes:
   - Fixed evaluation of APIM policies when using embedded C# with quotes by @BernieWhite.
     [#3184](https://github.com/Azure/PSRule.Rules.Azure/issues/3184)
+  - Fixed Resource group ID is incorrect under subscription scope by @BernieWhite.
+    [#3198](https://github.com/Azure/PSRule.Rules.Azure/issues/3198)
 
 ## v1.40.0-B0147 (pre-release)
 

src/PSRule.Rules.Azure/Common/ResourceHelper.cs

@@ -71,7 +71,7 @@
     /// <returns>Returns the resource type if the Id is valid or <c>null</c> when an invalid resource Id is specified.</returns>
     internal static string? GetResourceType(string? resourceId)
     {
         return string.IsNullOrEmpty(resourceId) ? null : string.Join(SLASH, GetResourceIdTypeParts(resourceId));
     }
 
     /// <summary>
@@ -168,7 +168,7 @@
             while (i < result.Length && j <= depth)
             {
                 // If a resource provider is included prepend /providers.
                 if (resourceType[j].Contains(DOT))
                 {
                     result[i++] = SLASH;
                     result[i++] = PROVIDERS;
@@ -177,23 +177,23 @@
                 result[i++] = SLASH;
                 result[i++] = resourceType[j];
                 result[i++] = SLASH;
                 result[i++] = name[j++];
             }
         }
         return string.Concat(result);
     }
 
     internal static string CombineResourceId(string subscriptionId, string resourceGroup, string resourceType, string name, int depth = int.MaxValue, string scope = null)
     {
         TryResourceIdComponents(resourceType, name, out var typeComponents, out var nameComponents);
 
         // Handle scoped resource IDs.
         if (!string.IsNullOrEmpty(scope) && scope != SLASH && TryResourceIdComponents(scope, out subscriptionId, out resourceGroup, out var parentTypeComponents, nameComponents: out var parentNameComponents))
         {
             typeComponents = MergeResourceNameOrType(parentTypeComponents, typeComponents);
             nameComponents = MergeResourceNameOrType(parentNameComponents, nameComponents);
         }
         return CombineResourceId(subscriptionId, resourceGroup, typeComponents, nameComponents, depth);
     }
 
     internal static string ResourceId(string resourceType, string resourceName, string? scopeId, int depth = int.MaxValue)
@@ -215,22 +215,27 @@
         return ResourceId(tenant, managementGroup, subscriptionId, resourceGroup, typeParts, nameParts, scopeId, depth);
     }
 
-    internal static string ResourceId(string? tenant, string? managementGroup, string? subscriptionId, string? resourceGroup, string[]? resourceType, string[]? resourceName, string? scopeId, int depth = int.MaxValue)
+    internal static string ResourceId(string? tenant, string? managementGroup, string? subscriptionId, string? resourceGroup, string[]? resourceTypeParts, string[]? resourceNameParts, string? scopeId, int depth = int.MaxValue)
     {
-        var resourceTypeLength = resourceType?.Length ?? 0;
-        var nameLength = resourceName?.Length ?? 0;
+        var resourceTypeLength = resourceTypeParts?.Length ?? 0;
+        var nameLength = resourceNameParts?.Length ?? 0;
         if (resourceTypeLength != nameLength)
-            throw new TemplateFunctionException(nameof(resourceType), FunctionErrorType.MismatchingResourceSegments, PSRuleResources.MismatchingResourceSegments);
+            throw new TemplateFunctionException(nameof(resourceTypeParts), FunctionErrorType.MismatchingResourceSegments, PSRuleResources.MismatchingResourceSegments);
 
         if (!ResourceIdComponents(scopeId, out var scopeTenant, out var scopeManagementGroup, out var scopeSubscriptionId, out var scopeResourceGroup, out var scopeResourceType, out var scopeResourceName))
         {
             scopeResourceType = null;
             scopeResourceName = null;
         }
 
-        resourceType = MergeResourceNameOrType(scopeResourceType, resourceType);
-        resourceName = MergeResourceNameOrType(scopeResourceName, resourceName);
-        return ResourceId(scopeTenant ?? tenant, scopeManagementGroup ?? managementGroup, scopeSubscriptionId ?? subscriptionId, scopeResourceGroup ?? resourceGroup, resourceType, resourceName, depth);
+        resourceTypeParts = MergeResourceNameOrType(scopeResourceType, resourceTypeParts);
+        resourceNameParts = MergeResourceNameOrType(scopeResourceName, resourceNameParts);
+        return ResourceId(scopeTenant ?? tenant, scopeManagementGroup ?? managementGroup, scopeSubscriptionId ?? subscriptionId, scopeResourceGroup ?? resourceGroup, resourceTypeParts, resourceNameParts, depth);
+    }
+
+    internal static string ResourceGroupId(string subscriptionId, string resourceGroup)
+    {
+        return string.Format("/subscriptions/{0}/resourceGroups/{1}", subscriptionId, resourceGroup);
     }
 
     private static string[]? MergeResourceNameOrType(string[]? parentNameOrType, string[]? nameOrType)
@@ -479,9 +484,9 @@
             while (i < result.Length && j <= depth)
             {
                 result[i++] = SLASH;
                 result[i++] = resourceType[j];
                 result[i++] = SLASH;
                 result[i++] = name[j++];
             }
         }
         return string.Concat(result);

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

@@ -70,6 +70,8 @@ internal abstract class TemplateVisitor : ResourceManagerVisitor
         private const string PROPERTY_ROOTDEPLOYMENT = "rootDeployment";
         private const string PROPERTY_NULLABLE = "nullable";
 
+        private const string TYPE_RESOURCE_GROUPS = "Microsoft.Resources/resourceGroups";
+
         internal sealed class TemplateContext : ResourceManagerVisitorContext, ITemplateContext
         {
             private const string CLOUD_PUBLIC = "AzureCloud";
@@ -1341,17 +1343,28 @@ private static ResourceValue ResourceInstance(TemplateContext context, JObject r
             var scope = context.TryParentResourceId(resource, out var parentIds) && parentIds != null && parentIds.Length > 0 ? parentIds[0] : null;
 
             string resourceId = null;
-            if (deploymentScope == DeploymentScope.ResourceGroup)
-                resourceId = ResourceHelper.ResourceId(tenant: null, managementGroup: null, subscriptionId: subscriptionId, resourceGroup: resourceGroupName, resourceType: type, resourceName: name, scopeId: scope);
 
+            // Handle special case when resource type is a resource group.
+            if (StringComparer.OrdinalIgnoreCase.Equals(type, TYPE_RESOURCE_GROUPS))
+            {
+                resourceId = ResourceHelper.ResourceGroupId(subscriptionId: subscriptionId, resourceGroup: name);
+            }
+            else if (deploymentScope == DeploymentScope.ResourceGroup)
+            {
+                resourceId = ResourceHelper.ResourceId(tenant: null, managementGroup: null, subscriptionId: subscriptionId, resourceGroup: resourceGroupName, resourceType: type, resourceName: name, scopeId: scope);
+            }
             else if (deploymentScope == DeploymentScope.Subscription)
+            {
                 resourceId = ResourceHelper.ResourceId(tenant: null, managementGroup: null, subscriptionId: subscriptionId, resourceGroup: null, resourceType: type, resourceName: name, scopeId: null);
-
+            }
             else if (deploymentScope == DeploymentScope.ManagementGroup)
+            {
                 resourceId = ResourceHelper.ResourceId(tenant: null, managementGroup: managementGroup, subscriptionId: null, resourceGroup: null, resourceType: type, resourceName: name, scopeId: scope);
-
+            }
             else if (deploymentScope == DeploymentScope.Tenant)
+            {
                 resourceId = ResourceHelper.ResourceId(tenant: "/", managementGroup: null, subscriptionId: null, resourceGroup: null, resourceType: type, resourceName: name, scopeId: "/");
+            }
 
             context.UpdateResourceScope(resource);
             resource[PROPERTY_ID] = resourceId;

tests/PSRule.Rules.Azure.Tests/Bicep/ScopeTestCases/BicepScopeTests.cs

@@ -33,4 +33,28 @@ public void ProcessTemplate_WhenManagementGroupAtTenant_ShouldReturnCompleteProp
         Assert.Equal("ffffffff-ffff-ffff-ffff-ffffffffffff", actual["properties"]["tenantId"].Value<string>());
         Assert.Equal("/providers/Microsoft.Management/managementGroups/mg-01", actual["properties"]["details"]["parent"]["id"].Value<string>());
     }
+
+    [Fact]
+    public void ProcessTemplate_WhenResourceGroupFromSubscriptionScope_ShouldReturnCorrectIdentifiers()
+    {
+        var resources = ProcessTemplate(GetSourcePath("Bicep/ScopeTestCases/Tests.Bicep.2.json"), null, out _);
+
+        Assert.NotNull(resources);
+
+        var actual = resources.Where(r => r["name"].Value<string>() == "rg-1").FirstOrDefault();
+        Assert.Equal("Microsoft.Resources/resourceGroups", actual["type"].Value<string>());
+        Assert.Equal("rg-1", actual["name"].Value<string>());
+        Assert.Equal("/subscriptions/ffffffff-ffff-ffff-ffff-ffffffffffff/resourceGroups/rg-1", actual["id"].Value<string>());
+
+        actual = resources.Where(r => r["name"].Value<string>() == "id-1").FirstOrDefault();
+        Assert.Equal("id-1", actual["name"].Value<string>());
+        Assert.Equal("/subscriptions/ffffffff-ffff-ffff-ffff-ffffffffffff/resourceGroups/rg-1/providers/Microsoft.ManagedIdentity/userAssignedIdentities/id-1", actual["id"].Value<string>());
+
+        actual = resources.Where(r => r["name"].Value<string>() == "registry-1").FirstOrDefault();
+        Assert.Equal("registry-1", actual["name"].Value<string>());
+        Assert.Equal("/subscriptions/ffffffff-ffff-ffff-ffff-ffffffffffff/resourceGroups/rg-1/providers/Microsoft.ContainerRegistry/registries/registry-1", actual["id"].Value<string>());
+
+        var property = actual["identity"]["userAssignedIdentities"].Value<JObject>().Properties().FirstOrDefault();
+        Assert.Equal("/subscriptions/ffffffff-ffff-ffff-ffff-ffffffffffff/resourceGroups/rg-1/providers/Microsoft.ManagedIdentity/userAssignedIdentities/id-1", property.Name);
+    }
 }

tests/PSRule.Rules.Azure.Tests/Bicep/ScopeTestCases/Tests.Bicep.2.bicep

@@ -0,0 +1,39 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+targetScope = 'subscription'
+
+param location string = deployment().location
+
+@description('')
+param tags object = {}
+
+resource rg 'Microsoft.Resources/resourceGroups@2024-07-01' = {
+  name: 'rg-1'
+  location: location
+  tags: tags
+}
+
+module id './Tests.Bicep.2.child2.bicep' = {
+  scope: rg
+  name: 'id-deploy'
+  params: {
+    name: 'id-1'
+    location: location
+    tags: tags
+  }
+}
+
+module registry './Tests.Bicep.2.child1.bicep' = {
+  scope: rg
+  name: 'registry-deploy'
+  params: {
+    location: location
+    name: 'registry-1'
+    tags: tags
+    identityId: id.outputs.id
+  }
+}
+
+output userAssignedIdentityId string = id.outputs.id
+output registryId string = registry.outputs.id

tests/PSRule.Rules.Azure.Tests/Bicep/ScopeTestCases/Tests.Bicep.2.child1.bicep

@@ -0,0 +1,36 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+targetScope = 'resourceGroup'
+
+param name string
+param location string
+param identityId string
+param tags object
+
+resource containerRegistry 'Microsoft.ContainerRegistry/registries@2023-11-01-preview' = {
+  name: name
+  location: location
+  identity: {
+    type: 'UserAssigned'
+    userAssignedIdentities: {
+      '${identityId}': {}
+    }
+  }
+  sku: {
+    name: 'Premium'
+  }
+  tags: tags
+  properties: {
+    adminUserEnabled: false
+    policies: {
+      azureADAuthenticationAsArmPolicy: {
+        status: 'enabled'
+      }
+    }
+  }
+}
+
+output id string = containerRegistry.id
+output url string = containerRegistry.properties.loginServer
+output identity object = containerRegistry.identity.userAssignedIdentities

tests/PSRule.Rules.Azure.Tests/Bicep/ScopeTestCases/Tests.Bicep.2.child2.bicep

@@ -0,0 +1,14 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+param name string
+param location string
+param tags object
+
+resource id 'Microsoft.ManagedIdentity/userAssignedIdentities@2023-01-31' = {
+  name: name
+  location: location
+  tags: tags
+}
+
+output id string = id.id