Skip to content

Commit fa40383

Browse files
authored
Fixed Resource group ID is incorrect under subscription scope Azure#3198 (Azure#3199)
* Fixed Resource group ID is incorrect under subscription scope Azure#3198 * Add contribution
1 parent 375e4e5 commit fa40383

File tree

9 files changed

+351
-11
lines changed

9 files changed

+351
-11
lines changed

docs/CHANGELOG-v1.md

+5
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,17 @@ See [upgrade notes][1] for helpful information when upgrading from previous vers
3131

3232
What's changed since pre-release v1.40.0-B0147:
3333

34+
- General improvements:
35+
- Added first time contributor guide in docs by @that-ar-guy.
36+
[#3097](https://github.com/Azure/PSRule.Rules.Azure/issues/3097)
3437
- Engineering:
3538
- Quality updates to rule documentation by @BernieWhite.
3639
[#3102](https://github.com/Azure/PSRule.Rules.Azure/issues/3102)
3740
- Bug fixes:
3841
- Fixed evaluation of APIM policies when using embedded C# with quotes by @BernieWhite.
3942
[#3184](https://github.com/Azure/PSRule.Rules.Azure/issues/3184)
43+
- Fixed Resource group ID is incorrect under subscription scope by @BernieWhite.
44+
[#3198](https://github.com/Azure/PSRule.Rules.Azure/issues/3198)
4045

4146
## v1.40.0-B0147 (pre-release)
4247

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

+12-7
Original file line numberDiff line numberDiff line change
@@ -215,22 +215,27 @@ internal static string ResourceId(string? tenant, string? managementGroup, strin
215215
return ResourceId(tenant, managementGroup, subscriptionId, resourceGroup, typeParts, nameParts, scopeId, depth);
216216
}
217217

218-
internal static string ResourceId(string? tenant, string? managementGroup, string? subscriptionId, string? resourceGroup, string[]? resourceType, string[]? resourceName, string? scopeId, int depth = int.MaxValue)
218+
internal static string ResourceId(string? tenant, string? managementGroup, string? subscriptionId, string? resourceGroup, string[]? resourceTypeParts, string[]? resourceNameParts, string? scopeId, int depth = int.MaxValue)
219219
{
220-
var resourceTypeLength = resourceType?.Length ?? 0;
221-
var nameLength = resourceName?.Length ?? 0;
220+
var resourceTypeLength = resourceTypeParts?.Length ?? 0;
221+
var nameLength = resourceNameParts?.Length ?? 0;
222222
if (resourceTypeLength != nameLength)
223-
throw new TemplateFunctionException(nameof(resourceType), FunctionErrorType.MismatchingResourceSegments, PSRuleResources.MismatchingResourceSegments);
223+
throw new TemplateFunctionException(nameof(resourceTypeParts), FunctionErrorType.MismatchingResourceSegments, PSRuleResources.MismatchingResourceSegments);
224224

225225
if (!ResourceIdComponents(scopeId, out var scopeTenant, out var scopeManagementGroup, out var scopeSubscriptionId, out var scopeResourceGroup, out var scopeResourceType, out var scopeResourceName))
226226
{
227227
scopeResourceType = null;
228228
scopeResourceName = null;
229229
}
230230

231-
resourceType = MergeResourceNameOrType(scopeResourceType, resourceType);
232-
resourceName = MergeResourceNameOrType(scopeResourceName, resourceName);
233-
return ResourceId(scopeTenant ?? tenant, scopeManagementGroup ?? managementGroup, scopeSubscriptionId ?? subscriptionId, scopeResourceGroup ?? resourceGroup, resourceType, resourceName, depth);
231+
resourceTypeParts = MergeResourceNameOrType(scopeResourceType, resourceTypeParts);
232+
resourceNameParts = MergeResourceNameOrType(scopeResourceName, resourceNameParts);
233+
return ResourceId(scopeTenant ?? tenant, scopeManagementGroup ?? managementGroup, scopeSubscriptionId ?? subscriptionId, scopeResourceGroup ?? resourceGroup, resourceTypeParts, resourceNameParts, depth);
234+
}
235+
236+
internal static string ResourceGroupId(string subscriptionId, string resourceGroup)
237+
{
238+
return string.Format("/subscriptions/{0}/resourceGroups/{1}", subscriptionId, resourceGroup);
234239
}
235240

236241
private static string[]? MergeResourceNameOrType(string[]? parentNameOrType, string[]? nameOrType)

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

+17-4
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,8 @@ internal abstract class TemplateVisitor : ResourceManagerVisitor
7070
private const string PROPERTY_ROOTDEPLOYMENT = "rootDeployment";
7171
private const string PROPERTY_NULLABLE = "nullable";
7272

73+
private const string TYPE_RESOURCE_GROUPS = "Microsoft.Resources/resourceGroups";
74+
7375
internal sealed class TemplateContext : ResourceManagerVisitorContext, ITemplateContext
7476
{
7577
private const string CLOUD_PUBLIC = "AzureCloud";
@@ -1341,17 +1343,28 @@ private static ResourceValue ResourceInstance(TemplateContext context, JObject r
13411343
var scope = context.TryParentResourceId(resource, out var parentIds) && parentIds != null && parentIds.Length > 0 ? parentIds[0] : null;
13421344

13431345
string resourceId = null;
1344-
if (deploymentScope == DeploymentScope.ResourceGroup)
1345-
resourceId = ResourceHelper.ResourceId(tenant: null, managementGroup: null, subscriptionId: subscriptionId, resourceGroup: resourceGroupName, resourceType: type, resourceName: name, scopeId: scope);
13461346

1347+
// Handle special case when resource type is a resource group.
1348+
if (StringComparer.OrdinalIgnoreCase.Equals(type, TYPE_RESOURCE_GROUPS))
1349+
{
1350+
resourceId = ResourceHelper.ResourceGroupId(subscriptionId: subscriptionId, resourceGroup: name);
1351+
}
1352+
else if (deploymentScope == DeploymentScope.ResourceGroup)
1353+
{
1354+
resourceId = ResourceHelper.ResourceId(tenant: null, managementGroup: null, subscriptionId: subscriptionId, resourceGroup: resourceGroupName, resourceType: type, resourceName: name, scopeId: scope);
1355+
}
13471356
else if (deploymentScope == DeploymentScope.Subscription)
1357+
{
13481358
resourceId = ResourceHelper.ResourceId(tenant: null, managementGroup: null, subscriptionId: subscriptionId, resourceGroup: null, resourceType: type, resourceName: name, scopeId: null);
1349-
1359+
}
13501360
else if (deploymentScope == DeploymentScope.ManagementGroup)
1361+
{
13511362
resourceId = ResourceHelper.ResourceId(tenant: null, managementGroup: managementGroup, subscriptionId: null, resourceGroup: null, resourceType: type, resourceName: name, scopeId: scope);
1352-
1363+
}
13531364
else if (deploymentScope == DeploymentScope.Tenant)
1365+
{
13541366
resourceId = ResourceHelper.ResourceId(tenant: "/", managementGroup: null, subscriptionId: null, resourceGroup: null, resourceType: type, resourceName: name, scopeId: "/");
1367+
}
13551368

13561369
context.UpdateResourceScope(resource);
13571370
resource[PROPERTY_ID] = resourceId;

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

+24
Original file line numberDiff line numberDiff line change
@@ -33,4 +33,28 @@ public void ProcessTemplate_WhenManagementGroupAtTenant_ShouldReturnCompleteProp
3333
Assert.Equal("ffffffff-ffff-ffff-ffff-ffffffffffff", actual["properties"]["tenantId"].Value<string>());
3434
Assert.Equal("/providers/Microsoft.Management/managementGroups/mg-01", actual["properties"]["details"]["parent"]["id"].Value<string>());
3535
}
36+
37+
[Fact]
38+
public void ProcessTemplate_WhenResourceGroupFromSubscriptionScope_ShouldReturnCorrectIdentifiers()
39+
{
40+
var resources = ProcessTemplate(GetSourcePath("Bicep/ScopeTestCases/Tests.Bicep.2.json"), null, out _);
41+
42+
Assert.NotNull(resources);
43+
44+
var actual = resources.Where(r => r["name"].Value<string>() == "rg-1").FirstOrDefault();
45+
Assert.Equal("Microsoft.Resources/resourceGroups", actual["type"].Value<string>());
46+
Assert.Equal("rg-1", actual["name"].Value<string>());
47+
Assert.Equal("/subscriptions/ffffffff-ffff-ffff-ffff-ffffffffffff/resourceGroups/rg-1", actual["id"].Value<string>());
48+
49+
actual = resources.Where(r => r["name"].Value<string>() == "id-1").FirstOrDefault();
50+
Assert.Equal("id-1", actual["name"].Value<string>());
51+
Assert.Equal("/subscriptions/ffffffff-ffff-ffff-ffff-ffffffffffff/resourceGroups/rg-1/providers/Microsoft.ManagedIdentity/userAssignedIdentities/id-1", actual["id"].Value<string>());
52+
53+
actual = resources.Where(r => r["name"].Value<string>() == "registry-1").FirstOrDefault();
54+
Assert.Equal("registry-1", actual["name"].Value<string>());
55+
Assert.Equal("/subscriptions/ffffffff-ffff-ffff-ffff-ffffffffffff/resourceGroups/rg-1/providers/Microsoft.ContainerRegistry/registries/registry-1", actual["id"].Value<string>());
56+
57+
var property = actual["identity"]["userAssignedIdentities"].Value<JObject>().Properties().FirstOrDefault();
58+
Assert.Equal("/subscriptions/ffffffff-ffff-ffff-ffff-ffffffffffff/resourceGroups/rg-1/providers/Microsoft.ManagedIdentity/userAssignedIdentities/id-1", property.Name);
59+
}
3660
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT License.
3+
4+
targetScope = 'subscription'
5+
6+
param location string = deployment().location
7+
8+
@description('')
9+
param tags object = {}
10+
11+
resource rg 'Microsoft.Resources/resourceGroups@2024-07-01' = {
12+
name: 'rg-1'
13+
location: location
14+
tags: tags
15+
}
16+
17+
module id './Tests.Bicep.2.child2.bicep' = {
18+
scope: rg
19+
name: 'id-deploy'
20+
params: {
21+
name: 'id-1'
22+
location: location
23+
tags: tags
24+
}
25+
}
26+
27+
module registry './Tests.Bicep.2.child1.bicep' = {
28+
scope: rg
29+
name: 'registry-deploy'
30+
params: {
31+
location: location
32+
name: 'registry-1'
33+
tags: tags
34+
identityId: id.outputs.id
35+
}
36+
}
37+
38+
output userAssignedIdentityId string = id.outputs.id
39+
output registryId string = registry.outputs.id
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT License.
3+
4+
targetScope = 'resourceGroup'
5+
6+
param name string
7+
param location string
8+
param identityId string
9+
param tags object
10+
11+
resource containerRegistry 'Microsoft.ContainerRegistry/registries@2023-11-01-preview' = {
12+
name: name
13+
location: location
14+
identity: {
15+
type: 'UserAssigned'
16+
userAssignedIdentities: {
17+
'${identityId}': {}
18+
}
19+
}
20+
sku: {
21+
name: 'Premium'
22+
}
23+
tags: tags
24+
properties: {
25+
adminUserEnabled: false
26+
policies: {
27+
azureADAuthenticationAsArmPolicy: {
28+
status: 'enabled'
29+
}
30+
}
31+
}
32+
}
33+
34+
output id string = containerRegistry.id
35+
output url string = containerRegistry.properties.loginServer
36+
output identity object = containerRegistry.identity.userAssignedIdentities
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT License.
3+
4+
param name string
5+
param location string
6+
param tags object
7+
8+
resource id 'Microsoft.ManagedIdentity/userAssignedIdentities@2023-01-31' = {
9+
name: name
10+
location: location
11+
tags: tags
12+
}
13+
14+
output id string = id.id

0 commit comments

Comments
 (0)