From 239b269b86978e2e5e1013e08a4bec1aacc78782 Mon Sep 17 00:00:00 2001 From: Bernie White Date: Sun, 8 Dec 2024 03:14:34 +1000 Subject: [PATCH 1/2] Fixed Resource group ID is incorrect under subscription scope #3198 --- docs/CHANGELOG-v1.md | 2 + .../Common/ResourceHelper.cs | 19 +- .../Data/Template/TemplateVisitor.cs | 21 +- .../Bicep/ScopeTestCases/BicepScopeTests.cs | 24 +++ .../Bicep/ScopeTestCases/Tests.Bicep.2.bicep | 39 ++++ .../ScopeTestCases/Tests.Bicep.2.child1.bicep | 36 ++++ .../ScopeTestCases/Tests.Bicep.2.child2.bicep | 14 ++ .../Bicep/ScopeTestCases/Tests.Bicep.2.json | 201 ++++++++++++++++++ .../PSRule.Rules.Azure.Tests.csproj | 3 + 9 files changed, 348 insertions(+), 11 deletions(-) create mode 100644 tests/PSRule.Rules.Azure.Tests/Bicep/ScopeTestCases/Tests.Bicep.2.bicep create mode 100644 tests/PSRule.Rules.Azure.Tests/Bicep/ScopeTestCases/Tests.Bicep.2.child1.bicep create mode 100644 tests/PSRule.Rules.Azure.Tests/Bicep/ScopeTestCases/Tests.Bicep.2.child2.bicep create mode 100644 tests/PSRule.Rules.Azure.Tests/Bicep/ScopeTestCases/Tests.Bicep.2.json diff --git a/docs/CHANGELOG-v1.md b/docs/CHANGELOG-v1.md index 156ee452737..4059350d1e0 100644 --- a/docs/CHANGELOG-v1.md +++ b/docs/CHANGELOG-v1.md @@ -37,6 +37,8 @@ What's changed since pre-release v1.40.0-B0147: - 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) diff --git a/src/PSRule.Rules.Azure/Common/ResourceHelper.cs b/src/PSRule.Rules.Azure/Common/ResourceHelper.cs index 945a2659251..2868096a4bb 100644 --- a/src/PSRule.Rules.Azure/Common/ResourceHelper.cs +++ b/src/PSRule.Rules.Azure/Common/ResourceHelper.cs @@ -215,12 +215,12 @@ internal static string ResourceId(string? tenant, string? managementGroup, strin 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)) { @@ -228,9 +228,14 @@ internal static string ResourceId(string? tenant, string? managementGroup, strin 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) diff --git a/src/PSRule.Rules.Azure/Data/Template/TemplateVisitor.cs b/src/PSRule.Rules.Azure/Data/Template/TemplateVisitor.cs index 60bf4fdfce4..b53e761e166 100644 --- a/src/PSRule.Rules.Azure/Data/Template/TemplateVisitor.cs +++ b/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; diff --git a/tests/PSRule.Rules.Azure.Tests/Bicep/ScopeTestCases/BicepScopeTests.cs b/tests/PSRule.Rules.Azure.Tests/Bicep/ScopeTestCases/BicepScopeTests.cs index 6f5f8784d3f..a30c944bb19 100644 --- a/tests/PSRule.Rules.Azure.Tests/Bicep/ScopeTestCases/BicepScopeTests.cs +++ b/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()); Assert.Equal("/providers/Microsoft.Management/managementGroups/mg-01", actual["properties"]["details"]["parent"]["id"].Value()); } + + [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() == "rg-1").FirstOrDefault(); + Assert.Equal("Microsoft.Resources/resourceGroups", actual["type"].Value()); + Assert.Equal("rg-1", actual["name"].Value()); + Assert.Equal("/subscriptions/ffffffff-ffff-ffff-ffff-ffffffffffff/resourceGroups/rg-1", actual["id"].Value()); + + actual = resources.Where(r => r["name"].Value() == "id-1").FirstOrDefault(); + Assert.Equal("id-1", actual["name"].Value()); + Assert.Equal("/subscriptions/ffffffff-ffff-ffff-ffff-ffffffffffff/resourceGroups/rg-1/providers/Microsoft.ManagedIdentity/userAssignedIdentities/id-1", actual["id"].Value()); + + actual = resources.Where(r => r["name"].Value() == "registry-1").FirstOrDefault(); + Assert.Equal("registry-1", actual["name"].Value()); + Assert.Equal("/subscriptions/ffffffff-ffff-ffff-ffff-ffffffffffff/resourceGroups/rg-1/providers/Microsoft.ContainerRegistry/registries/registry-1", actual["id"].Value()); + + var property = actual["identity"]["userAssignedIdentities"].Value().Properties().FirstOrDefault(); + Assert.Equal("/subscriptions/ffffffff-ffff-ffff-ffff-ffffffffffff/resourceGroups/rg-1/providers/Microsoft.ManagedIdentity/userAssignedIdentities/id-1", property.Name); + } } diff --git a/tests/PSRule.Rules.Azure.Tests/Bicep/ScopeTestCases/Tests.Bicep.2.bicep b/tests/PSRule.Rules.Azure.Tests/Bicep/ScopeTestCases/Tests.Bicep.2.bicep new file mode 100644 index 00000000000..faf2e1f232d --- /dev/null +++ b/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 diff --git a/tests/PSRule.Rules.Azure.Tests/Bicep/ScopeTestCases/Tests.Bicep.2.child1.bicep b/tests/PSRule.Rules.Azure.Tests/Bicep/ScopeTestCases/Tests.Bicep.2.child1.bicep new file mode 100644 index 00000000000..e68d748b8bf --- /dev/null +++ b/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 diff --git a/tests/PSRule.Rules.Azure.Tests/Bicep/ScopeTestCases/Tests.Bicep.2.child2.bicep b/tests/PSRule.Rules.Azure.Tests/Bicep/ScopeTestCases/Tests.Bicep.2.child2.bicep new file mode 100644 index 00000000000..5022a342ae4 --- /dev/null +++ b/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 diff --git a/tests/PSRule.Rules.Azure.Tests/Bicep/ScopeTestCases/Tests.Bicep.2.json b/tests/PSRule.Rules.Azure.Tests/Bicep/ScopeTestCases/Tests.Bicep.2.json new file mode 100644 index 00000000000..bb4b3785ac3 --- /dev/null +++ b/tests/PSRule.Rules.Azure.Tests/Bicep/ScopeTestCases/Tests.Bicep.2.json @@ -0,0 +1,201 @@ +{ + "$schema": "https://schema.management.azure.com/schemas/2018-05-01/subscriptionDeploymentTemplate.json#", + "contentVersion": "1.0.0.0", + "metadata": { + "_generator": { + "name": "bicep", + "version": "0.31.92.45157", + "templateHash": "3324470663519838888" + } + }, + "parameters": { + "location": { + "type": "string", + "defaultValue": "[deployment().location]" + }, + "tags": { + "type": "object", + "defaultValue": {}, + "metadata": { + "description": "" + } + } + }, + "resources": [ + { + "type": "Microsoft.Resources/resourceGroups", + "apiVersion": "2024-07-01", + "name": "rg-1", + "location": "[parameters('location')]", + "tags": "[parameters('tags')]" + }, + { + "type": "Microsoft.Resources/deployments", + "apiVersion": "2022-09-01", + "name": "id-deploy", + "resourceGroup": "rg-1", + "properties": { + "expressionEvaluationOptions": { + "scope": "inner" + }, + "mode": "Incremental", + "parameters": { + "name": { + "value": "id-1" + }, + "location": { + "value": "[parameters('location')]" + }, + "tags": { + "value": "[parameters('tags')]" + } + }, + "template": { + "$schema": "https://schema.management.azure.com/schemas/2019-04-01/deploymentTemplate.json#", + "contentVersion": "1.0.0.0", + "metadata": { + "_generator": { + "name": "bicep", + "version": "0.31.92.45157", + "templateHash": "10617217360526734802" + } + }, + "parameters": { + "name": { + "type": "string" + }, + "location": { + "type": "string" + }, + "tags": { + "type": "object" + } + }, + "resources": [ + { + "type": "Microsoft.ManagedIdentity/userAssignedIdentities", + "apiVersion": "2023-01-31", + "name": "[parameters('name')]", + "location": "[parameters('location')]", + "tags": "[parameters('tags')]" + } + ], + "outputs": { + "id": { + "type": "string", + "value": "[resourceId('Microsoft.ManagedIdentity/userAssignedIdentities', parameters('name'))]" + } + } + } + }, + "dependsOn": [ + "[subscriptionResourceId('Microsoft.Resources/resourceGroups', 'rg-1')]" + ] + }, + { + "type": "Microsoft.Resources/deployments", + "apiVersion": "2022-09-01", + "name": "registry-deploy", + "resourceGroup": "rg-1", + "properties": { + "expressionEvaluationOptions": { + "scope": "inner" + }, + "mode": "Incremental", + "parameters": { + "location": { + "value": "[parameters('location')]" + }, + "name": { + "value": "registry-1" + }, + "tags": { + "value": "[parameters('tags')]" + }, + "identityId": { + "value": "[reference(extensionResourceId(format('/subscriptions/{0}/resourceGroups/{1}', subscription().subscriptionId, 'rg-1'), 'Microsoft.Resources/deployments', 'id-deploy'), '2022-09-01').outputs.id.value]" + } + }, + "template": { + "$schema": "https://schema.management.azure.com/schemas/2019-04-01/deploymentTemplate.json#", + "contentVersion": "1.0.0.0", + "metadata": { + "_generator": { + "name": "bicep", + "version": "0.31.92.45157", + "templateHash": "6162665765515449124" + } + }, + "parameters": { + "name": { + "type": "string" + }, + "location": { + "type": "string" + }, + "identityId": { + "type": "string" + }, + "tags": { + "type": "object" + } + }, + "resources": [ + { + "type": "Microsoft.ContainerRegistry/registries", + "apiVersion": "2023-11-01-preview", + "name": "[parameters('name')]", + "location": "[parameters('location')]", + "identity": { + "type": "UserAssigned", + "userAssignedIdentities": { + "[format('{0}', parameters('identityId'))]": {} + } + }, + "sku": { + "name": "Premium" + }, + "tags": "[parameters('tags')]", + "properties": { + "adminUserEnabled": false, + "policies": { + "azureADAuthenticationAsArmPolicy": { + "status": "enabled" + } + } + } + } + ], + "outputs": { + "id": { + "type": "string", + "value": "[resourceId('Microsoft.ContainerRegistry/registries', parameters('name'))]" + }, + "url": { + "type": "string", + "value": "[reference(resourceId('Microsoft.ContainerRegistry/registries', parameters('name')), '2023-11-01-preview').loginServer]" + }, + "identity": { + "type": "object", + "value": "[reference(resourceId('Microsoft.ContainerRegistry/registries', parameters('name')), '2023-11-01-preview', 'full').identity.userAssignedIdentities]" + } + } + } + }, + "dependsOn": [ + "[extensionResourceId(format('/subscriptions/{0}/resourceGroups/{1}', subscription().subscriptionId, 'rg-1'), 'Microsoft.Resources/deployments', 'id-deploy')]", + "[subscriptionResourceId('Microsoft.Resources/resourceGroups', 'rg-1')]" + ] + } + ], + "outputs": { + "userAssignedIdentityId": { + "type": "string", + "value": "[reference(extensionResourceId(format('/subscriptions/{0}/resourceGroups/{1}', subscription().subscriptionId, 'rg-1'), 'Microsoft.Resources/deployments', 'id-deploy'), '2022-09-01').outputs.id.value]" + }, + "registryId": { + "type": "string", + "value": "[reference(extensionResourceId(format('/subscriptions/{0}/resourceGroups/{1}', subscription().subscriptionId, 'rg-1'), 'Microsoft.Resources/deployments', 'registry-deploy'), '2022-09-01').outputs.id.value]" + } + } +} \ No newline at end of file diff --git a/tests/PSRule.Rules.Azure.Tests/PSRule.Rules.Azure.Tests.csproj b/tests/PSRule.Rules.Azure.Tests/PSRule.Rules.Azure.Tests.csproj index 59bb55e6a5d..032832ec8b4 100644 --- a/tests/PSRule.Rules.Azure.Tests/PSRule.Rules.Azure.Tests.csproj +++ b/tests/PSRule.Rules.Azure.Tests/PSRule.Rules.Azure.Tests.csproj @@ -317,6 +317,9 @@ PreserveNewest + + PreserveNewest + PreserveNewest From 5565d335c6a26fd330ea12de8052b2d71a522b9c Mon Sep 17 00:00:00 2001 From: Bernie White Date: Sun, 8 Dec 2024 03:18:20 +1000 Subject: [PATCH 2/2] Add contribution --- docs/CHANGELOG-v1.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/CHANGELOG-v1.md b/docs/CHANGELOG-v1.md index 4059350d1e0..f1a1c986439 100644 --- a/docs/CHANGELOG-v1.md +++ b/docs/CHANGELOG-v1.md @@ -31,6 +31,9 @@ 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)