Merge patch v1.41.1 (#3249)

Azure · Feb 8, 2025 · c67c6f4 · c67c6f4
1 parent 8a51acf
commit c67c6f4
Show file tree

Hide file tree

Showing 14 changed files with 351 additions and 24 deletions.
diff --git a/docs/CHANGELOG-v1.md b/docs/CHANGELOG-v1.md
@@ -29,12 +29,22 @@ See [upgrade notes][1] for helpful information when upgrading from previous vers
 
 ## Unreleased
 
-What's changed since v1.41.0:
+What's changed since v1.41.1:
 
 - General improvements:
   - Added a new quickstart guide for using Azure Pipelines with PSRule by @that-ar-guy.  
     [#3220](https://github.com/Azure/PSRule.Rules.Azure/pull/3220)
 
+## v1.41.1
+
+What's changed since v1.41.0:
+
+- Bug fixes:
+  - Fixed incorrect generation of resource ID for tenant scoped deployments by @BernieWhite.
+    [#3237](https://github.com/Azure/PSRule.Rules.Azure/issues/3237)
+  - Fixed in-flight export of subscription resource type `Microsoft.Subscription` by @BernieWhite.
+    [#3231](https://github.com/Azure/PSRule.Rules.Azure/issues/3231)
+
 ## v1.41.0
 
 What's changed since v1.40.0:

diff --git a/src/PSRule.Rules.Azure/Common/ResourceHelper.cs b/src/PSRule.Rules.Azure/Common/ResourceHelper.cs
@@ -120,6 +120,20 @@ internal static bool TryResourceGroup(string resourceId, out string? subscriptio
             TryConsumeResourceGroupPart(idParts, ref i, out resourceGroupName);
     }
 
+    /// <summary>
+    /// Get the name of the management group from the specified resource Id.
+    /// </summary>
+    internal static bool TryManagementGroup(string resourceId, out string? managementGroup)
+    {
+        managementGroup = null;
+        if (string.IsNullOrEmpty(resourceId))
+            return false;
+
+        var idParts = resourceId.Split(SLASH_C);
+        var i = 0;
+        return TryConsumeManagementGroupPart(idParts, ref i, out managementGroup);
+    }
+
     /// <summary>
     /// Combines Id fragments to form a resource Id.
     /// </summary>

diff --git a/src/PSRule.Rules.Azure/Data/Template/TemplateVisitor.cs b/src/PSRule.Rules.Azure/Data/Template/TemplateVisitor.cs
@@ -39,7 +39,7 @@ internal abstract class TemplateVisitor : ResourceManagerVisitor
         private const string PROPERTY_TYPE = "type";
         private const string PROPERTY_PROPERTIES = "properties";
         private const string PROPERTY_TEMPLATE = "template";
-        private const string PROPERTY_TEMPLATELINK = "templateLink";
+        private const string PROPERTY_TEMPLATE_LINK = "templateLink";
         private const string PROPERTY_LOCATION = "location";
         private const string PROPERTY_COPY = "copy";
         private const string PROPERTY_NAME = "name";
@@ -49,15 +49,15 @@ internal abstract class TemplateVisitor : ResourceManagerVisitor
         private const string PROPERTY_MODE = "mode";
         private const string PROPERTY_DEFAULTVALUE = "defaultValue";
         private const string PROPERTY_SECRETNAME = "secretName";
-        private const string PROPERTY_PROVISIONINGSTATE = "provisioningState";
+        private const string PROPERTY_PROVISIONING_STATE = "provisioningState";
         private const string PROPERTY_ID = "id";
         private const string PROPERTY_URI = "uri";
         private const string PROPERTY_TEMPLATEHASH = "templateHash";
         private const string PROPERTY_EXPRESSIONEVALUATIONOPTIONS = "expressionEvaluationOptions";
         private const string PROPERTY_SCOPE = "scope";
-        private const string PROPERTY_RESOURCEGROUP = "resourceGroup";
-        private const string PROPERTY_SUBSCRIPTIONID = "subscriptionId";
-        private const string PROPERTY_MANAGEMENTGROUP = "managementGroup";
+        private const string PROPERTY_RESOURCE_GROUP = "resourceGroup";
+        private const string PROPERTY_SUBSCRIPTION_ID = "subscriptionId";
+        private const string PROPERTY_MANAGEMENT_GROUP = "managementGroup";
         private const string PROPERTY_NAMESPACE = "namespace";
         private const string PROPERTY_MEMBERS = "members";
         private const string PROPERTY_OUTPUT = "output";
@@ -481,7 +481,7 @@ internal void EnterDeployment(string deploymentName, JObject template, bool isNe
                 TryObjectProperty(template, PROPERTY_METADATA, out var metadata);
                 TryStringProperty(template, PROPERTY_SCHEMA, out var schema);
                 var scope = GetDeploymentScope(schema, out var deploymentScope);
-                var id = string.Concat(scope, "/providers/", RESOURCE_TYPE_DEPLOYMENT, "/", deploymentName);
+                var id = string.Concat(scope == "/" ? string.Empty : scope, "/providers/", RESOURCE_TYPE_DEPLOYMENT, "/", deploymentName);
                 var location = ResourceGroup.Location;
 
                 var templateLink = new JObject
@@ -493,10 +493,10 @@ internal void EnterDeployment(string deploymentName, JObject template, bool isNe
                 var properties = new JObject
                 {
                     { PROPERTY_TEMPLATE, template.CloneTemplateJToken() },
-                    { PROPERTY_TEMPLATELINK, templateLink },
+                    { PROPERTY_TEMPLATE_LINK, templateLink },
                     { PROPERTY_PARAMETERS, _Parameters },
                     { PROPERTY_MODE, "Incremental" },
-                    { PROPERTY_PROVISIONINGSTATE, "Accepted" },
+                    { PROPERTY_PROVISIONING_STATE, "Accepted" },
                     { PROPERTY_TEMPLATEHASH, templateHash },
                     { PROPERTY_OUTPUTS, new JObject() }
                 };
@@ -673,7 +673,7 @@ internal bool TryParameterDefault(string parameterName, ParameterType type, out
                             return true;
                         }
                         break;
-                };
+                }
                 return false;
             }
 
@@ -1406,14 +1406,14 @@ private static DeploymentScope GetDeploymentScope(TemplateContext context, JObje
             }
 
             // Handle special case for cross-scope deployments which may have an alternative subscription or resource group set.
-            subscriptionId = ResolveDeploymentScopeProperty(context, resource, PROPERTY_SUBSCRIPTIONID, contextValue:
+            subscriptionId = ResolveDeploymentScopeProperty(context, resource, PROPERTY_SUBSCRIPTION_ID, contextValue:
                 context.Deployment.DeploymentScope == DeploymentScope.Subscription ||
                 context.Deployment.DeploymentScope == DeploymentScope.ResourceGroup ? context.Subscription.SubscriptionId : null);
 
-            resourceGroupName = ResolveDeploymentScopeProperty(context, resource, PROPERTY_RESOURCEGROUP, contextValue:
+            resourceGroupName = ResolveDeploymentScopeProperty(context, resource, PROPERTY_RESOURCE_GROUP, contextValue:
                 context.Deployment.DeploymentScope == DeploymentScope.ResourceGroup ? context.ResourceGroup.Name : null);
 
-            managementGroup = ResolveDeploymentScopeProperty(context, resource, PROPERTY_MANAGEMENTGROUP, contextValue:
+            managementGroup = ResolveDeploymentScopeProperty(context, resource, PROPERTY_MANAGEMENT_GROUP, contextValue:
                 context.Deployment.DeploymentScope == DeploymentScope.ManagementGroup ? context.ManagementGroup.Name : null);
 
             // Update the deployment scope.
@@ -1541,20 +1541,32 @@ private TemplateContext GetDeploymentContext(TemplateContext context, string dep
             var resourceGroup = new ResourceGroupOption(context.ResourceGroup);
             var tenant = new TenantOption(context.Tenant);
             var managementGroup = new ManagementGroupOption(context.ManagementGroup);
-            if (TryStringProperty(resource, PROPERTY_SUBSCRIPTIONID, out var subscriptionId))
+            if (TryStringProperty(resource, PROPERTY_SUBSCRIPTION_ID, out var subscriptionId))
             {
                 var targetSubscriptionId = ExpandString(context, subscriptionId);
                 if (!string.IsNullOrEmpty(subscriptionId))
                     subscription.SubscriptionId = targetSubscriptionId;
             }
 
-            if (TryStringProperty(resource, PROPERTY_RESOURCEGROUP, out var resourceGroupName))
+            if (TryStringProperty(resource, PROPERTY_RESOURCE_GROUP, out var resourceGroupName))
             {
                 var targetResourceGroup = ExpandString(context, resourceGroupName);
                 if (!string.IsNullOrEmpty(targetResourceGroup))
                     resourceGroup.Name = targetResourceGroup;
             }
 
+            if (TryStringProperty(resource, PROPERTY_SCOPE, out var scopeId) && ResourceHelper.ResourceIdComponents(scopeId, out _, out var managementGroupName, out subscriptionId, out resourceGroupName, out _, out _))
+            {
+                if (!string.IsNullOrEmpty(managementGroupName))
+                    managementGroup.Name = managementGroupName;
+
+                if (!string.IsNullOrEmpty(subscriptionId))
+                    subscription.SubscriptionId = subscriptionId;
+
+                if (!string.IsNullOrEmpty(resourceGroupName))
+                    resourceGroup.Name = resourceGroupName;
+            }
+
             resourceGroup.SubscriptionId = subscription.SubscriptionId;
             TryObjectProperty(template, PROPERTY_PARAMETERS, out var templateParameters);
 

diff --git a/src/PSRule.Rules.Azure/Pipeline/Export/ResourceExportVisitor.cs b/src/PSRule.Rules.Azure/Pipeline/Export/ResourceExportVisitor.cs
@@ -19,8 +19,8 @@ internal sealed class ResourceExportVisitor
     private const string PROPERTY_PROPERTIES = "properties";
     private const string PROPERTY_ZONES = "zones";
     private const string PROPERTY_RESOURCES = "resources";
-    private const string PROPERTY_SUBSCRIPTIONID = "subscriptionId";
-    private const string PROPERTY_RESOURCEGROUPNAME = "resourceGroupName";
+    private const string PROPERTY_SUBSCRIPTION_ID = "subscriptionId";
+    private const string PROPERTY_RESOURCE_GROUP_NAME = "resourceGroupName";
     private const string PROPERTY_KIND = "kind";
     private const string PROPERTY_SHAREDKEY = "sharedKey";
     private const string PROPERTY_NETWORKPROFILE = "networkProfile";
@@ -235,11 +235,12 @@ private async Task GetProperties(ResourceContext context, JObject resource, stri
     private static void SetResourceIdentifiers(JObject resource, string resourceType, string resourceId)
     {
         if (ResourceHelper.TryResourceGroup(resourceId, out var subscriptionId, out var resourceGroupName) &&
-            !string.Equals(resourceType, TYPE_RESOURCES_RESOURCEGROUP, StringComparison.OrdinalIgnoreCase))
-            resource.Add(PROPERTY_RESOURCEGROUPNAME, resourceGroupName);
+            !string.Equals(resourceType, TYPE_RESOURCES_RESOURCEGROUP, StringComparison.OrdinalIgnoreCase) &&
+            !resource.ContainsKeyInsensitive(PROPERTY_RESOURCE_GROUP_NAME))
+            resource.Add(PROPERTY_RESOURCE_GROUP_NAME, resourceGroupName);
 
-        if (!string.IsNullOrEmpty(subscriptionId))
-            resource.Add(PROPERTY_SUBSCRIPTIONID, subscriptionId);
+        if (!string.IsNullOrEmpty(subscriptionId) && !resource.ContainsKeyInsensitive(PROPERTY_SUBSCRIPTION_ID))
+            resource.Add(PROPERTY_SUBSCRIPTION_ID, subscriptionId);
     }
 
     private bool TryGetLatestAPIVersion(string resourceType, out string apiVersion)

diff --git a/src/PSRule.Rules.Azure/Pipeline/Export/SubscriptionExportContext.cs b/src/PSRule.Rules.Azure/Pipeline/Export/SubscriptionExportContext.cs
@@ -12,7 +12,7 @@ namespace PSRule.Rules.Azure.Pipeline.Export;
 /// <summary>
 /// A context to export an Azure subscription.
 /// </summary>
-internal class SubscriptionExportContext : ExportDataContext, ISubscriptionExportContext
+internal sealed class SubscriptionExportContext : ExportDataContext, ISubscriptionExportContext
 {
     private readonly string _ResourceEndpoint;
     private readonly string _ResourceGroupEndpoint;
@@ -27,7 +27,7 @@ public SubscriptionExportContext(PipelineContext context, ExportSubscriptionScop
     {
         _ResourceEndpoint = $"{RESOURCE_MANAGER_URL}/subscriptions/{scope.SubscriptionId}/resources?api-version=2021-04-01{GetFilter(tag)}";
         _ResourceGroupEndpoint = $"{RESOURCE_MANAGER_URL}/subscriptions/{scope.SubscriptionId}/resourcegroups?api-version=2021-04-01{GetFilter(tag)}";
-        _SubscriptionEndpoint = $"{RESOURCE_MANAGER_URL}/subscriptions/{scope.SubscriptionId}";
+        _SubscriptionEndpoint = $"{RESOURCE_MANAGER_URL}/subscriptions/{scope.SubscriptionId}?api-version=2022-12-01";
         SubscriptionId = scope.SubscriptionId;
         TenantId = scope.TenantId;
         RefreshToken(scope.TenantId);

diff --git a/src/PSRule.Rules.Azure/Pipeline/ResourceDataPipeline.cs b/src/PSRule.Rules.Azure/Pipeline/ResourceDataPipeline.cs
@@ -28,6 +28,8 @@ internal sealed class ResourceDataPipeline : ExportDataPipeline
     private const string PROPERTY_DISPLAYNAME = "displayName";
     private const string PROPERTY_TENANTID = "tenantId";
 
+    private const string PLACEHOLDER_SUBSCRIPTION_TYPE = "Microsoft.Subscription";
+
     private const string ERRORID_RESOURCEDATAEXPAND = "PSRule.Rules.Azure.ResourceDataExpand";
 
     private readonly ConcurrentQueue<JObject> _Resources;
@@ -170,7 +172,7 @@ private async Task<int> GetSubscriptionAsync(ISubscriptionExportContext context)
         var r = await context.GetSubscriptionAsync();
         if (r != null)
         {
-            r.Add(PROPERTY_TYPE, "Microsoft.Subscription");
+            r.Add(PROPERTY_TYPE, PLACEHOLDER_SUBSCRIPTION_TYPE);
             if (r.TryGetProperty(PROPERTY_DISPLAYNAME, out var displayName))
                 r.Add(PROPERTY_NAME, displayName);
 

diff --git a/tests/PSRule.Rules.Azure.Tests/Bicep/ScopeTestCases/BicepScopeTests.cs b/tests/PSRule.Rules.Azure.Tests/Bicep/ScopeTestCases/BicepScopeTests.cs
@@ -57,4 +57,20 @@ public void ProcessTemplate_WhenResourceGroupFromSubscriptionScope_ShouldReturnC
         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);
     }
+
+    [Fact]
+    public void ProcessTemplate_WhenTenantScope_ShouldReturnCorrectIdentifiers()
+    {
+        var resources = ProcessTemplate(GetSourcePath("Bicep/ScopeTestCases/Tests.Bicep.3.json"), null, out _);
+
+        Assert.NotNull(resources);
+
+        var actual = resources.Where(r => r["name"].Value<string>() == "mg-test").FirstOrDefault();
+        Assert.Equal("Microsoft.Management/managementGroups", actual["type"].Value<string>());
+        Assert.Equal("/providers/Microsoft.Management/managementGroups/mg-test", actual["id"].Value<string>());
+
+        actual = resources.Where(r => r["name"].Value<string>() == "mg-test/00000000-0000-0000-0000-000000000000").FirstOrDefault();
+        Assert.Equal("Microsoft.Management/managementGroups/subscriptions", actual["type"].Value<string>());
+        Assert.Equal("/providers/Microsoft.Management/managementGroups/mg-test/subscriptions/00000000-0000-0000-0000-000000000000", actual["id"].Value<string>());
+    }
 }
diff --git a/tests/PSRule.Rules.Azure.Tests/Bicep/ScopeTestCases/Tests.Bicep.3.bicep b/tests/PSRule.Rules.Azure.Tests/Bicep/ScopeTestCases/Tests.Bicep.3.bicep
@@ -0,0 +1,25 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+// Test case for https://github.com/Azure/PSRule.Rules.Azure/issues/3237
+
+// Based on AVM subscription placement module.
+
+targetScope = 'tenant'
+
+module mg './Tests.Bicep.3.child1.bicep' = {
+  name: 'mg'
+  scope: tenant()
+}
+
+module child './Tests.Bicep.3.child2.bicep' = {
+  name: 'child'
+  params: {
+    managementGroups: [
+      {
+        id: mg.outputs.managementGroupId
+        subscriptionId: mg.outputs.subscriptionId
+      }
+    ]
+  }
+}
diff --git a/tests/PSRule.Rules.Azure.Tests/Bicep/ScopeTestCases/Tests.Bicep.3.child1.bicep b/tests/PSRule.Rules.Azure.Tests/Bicep/ScopeTestCases/Tests.Bicep.3.child1.bicep
@@ -0,0 +1,19 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+targetScope = 'tenant'
+
+resource managementGroup 'Microsoft.Management/managementGroups@2023-04-01' = {
+  name: 'mg-test'
+  properties: {
+    displayName: 'Test Management Group'
+    details: {
+      parent: {
+        id: ''
+      }
+    }
+  }
+}
+
+output managementGroupId string = managementGroup.id
+output subscriptionId string = '00000000-0000-0000-0000-000000000000'
diff --git a/tests/PSRule.Rules.Azure.Tests/Bicep/ScopeTestCases/Tests.Bicep.3.child2.bicep b/tests/PSRule.Rules.Azure.Tests/Bicep/ScopeTestCases/Tests.Bicep.3.child2.bicep
@@ -0,0 +1,21 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+targetScope = 'tenant'
+
+param managementGroups managementGroup[]
+
+type managementGroup = {
+  id: string
+  subscriptionId: string
+}
+
+module customSubscriptionPlacement './Tests.Bicep.3.child3.bicep' = [
+  for (place, index) in managementGroups: {
+    name: 'placement-${uniqueString(place.id)}${index}'
+    params: {
+      managementGroupId: place.id
+      subscriptionId: place.subscriptionId
+    }
+  }
+]
diff --git a/tests/PSRule.Rules.Azure.Tests/Bicep/ScopeTestCases/Tests.Bicep.3.child3.bicep b/tests/PSRule.Rules.Azure.Tests/Bicep/ScopeTestCases/Tests.Bicep.3.child3.bicep
@@ -0,0 +1,11 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+targetScope = 'tenant'
+
+param managementGroupId string
+param subscriptionId string
+
+resource customSubscriptionPlacement 'Microsoft.Management/managementGroups/subscriptions@2023-04-01' = {
+  name: '${last(split(managementGroupId, '/'))}/${subscriptionId}'
+}