Skip to content

Commit

Permalink
Fixed dependency ordering for cross scope deployments #2850
Browse files Browse the repository at this point in the history
  • Loading branch information
BernieWhite committed May 6, 2024
1 parent 7639775 commit cbe5b9b
Show file tree
Hide file tree
Showing 8 changed files with 295 additions and 17 deletions.
3 changes: 3 additions & 0 deletions docs/CHANGELOG-v1.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ What's changed since v1.36.0:
- General improvements:
- Quality updates to documentation by @BernieWhite.
[#2570](https://github.com/Azure/PSRule.Rules.Azure/issues/2570)
- Bug fixes:
- Fixed dependency ordering for cross scope deployments by @BernieWhite.
[#2850](https://github.com/Azure/PSRule.Rules.Azure/issues/2850)

## v1.36.0

Expand Down
85 changes: 68 additions & 17 deletions src/PSRule.Rules.Azure/Data/Template/TemplateVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,12 @@ private static bool GetResourceNameType(JObject resource, out string name, out s
return resource != null && resource.TryGetProperty(PROPERTY_NAME, out name) && resource.TryGetProperty(PROPERTY_TYPE, out type);
}

/// <summary>
/// Read the scope from a specified <c>scope</c> property.
/// </summary>
/// <param name="resource">The resource object.</param>
/// <param name="scopeId">The scope if set.</param>
/// <returns>Returns <c>true</c> if the scope property was set on the resource.</returns>
private bool TryResourceScope(JObject resource, out string scopeId)
{
scopeId = null;
Expand All @@ -306,6 +312,13 @@ private bool TryResourceScope(JObject resource, out string scopeId)
return true;
}

/// <summary>
/// Read the scope from the name and type properties if this is a sub-resource.
/// For example: A sub-resource may use name segments such as <c>vnet-1/subnet-1</c>.
/// </summary>
/// <param name="resource">The resource object.</param>
/// <param name="scopeId">The calculated scope.</param>
/// <returns>Returns <c>true</c> if the scope could be calculated from name segments.</returns>
private bool TryParentScope(JObject resource, out string scopeId)
{
scopeId = null;
Expand All @@ -318,12 +331,21 @@ private bool TryParentScope(JObject resource, out string scopeId)
return true;
}

/// <summary>
/// Get the scope of the resource based on the scope of the deployment.
/// </summary>
/// <param name="scopeId">The scope of the deployment.</param>
/// <returns>Returns <c>true</c> if a deployment scope was found.</returns>
private bool TryDeploymentScope(out string scopeId)
{
scopeId = Deployment?.Scope;
return scopeId != null;
}

/// <summary>
/// Updated the <c>scope</c> property with the scope that the resource will be deployed into.
/// </summary>
/// <param name="resource">The resource object.</param>
public void UpdateResourceScope(JObject resource)
{
if (!TryResourceScope(resource, out var scopeId) &&
Expand Down Expand Up @@ -1281,27 +1303,19 @@ private static ResourceValue ResourceInstance(TemplateContext context, JObject r
resource.TryGetProperty(PROPERTY_NAME, out var name);
resource.TryGetProperty(PROPERTY_TYPE, out var type);

var subscriptionId = context.Subscription.SubscriptionId;
var resourceGroupName = context.ResourceGroup.Name;

// Handle special case for cross-scope deployments which may have an alternative subscription or resource group set.
if (IsDeploymentResource(type))
{
subscriptionId = ResolveDeploymentScopeProperty(context, resource, PROPERTY_SUBSCRIPTIONID, subscriptionId);
resourceGroupName = ResolveDeploymentScopeProperty(context, resource, PROPERTY_RESOURCEGROUP, resourceGroupName);
}
var deploymentScope = GetDeploymentScope(context, resource, type, out var subscriptionId, out var resourceGroupName);

// Get scope if specified.
var scope = context.TryParentResourceId(resource, out var parentIds) && parentIds != null && parentIds.Length > 0 ? parentIds[0] : null;

string resourceId = null;
if (context.Deployment.DeploymentScope == DeploymentScope.ResourceGroup)
if (deploymentScope == DeploymentScope.ResourceGroup)
resourceId = ResourceHelper.CombineResourceId(subscriptionId, resourceGroupName, type, name, scope: scope);

if (context.Deployment.DeploymentScope == DeploymentScope.Subscription)
else if (deploymentScope == DeploymentScope.Subscription)
resourceId = ResourceHelper.CombineResourceId(subscriptionId, null, type, name);

if (context.Deployment.DeploymentScope == DeploymentScope.ManagementGroup || context.Deployment.DeploymentScope == DeploymentScope.Tenant)
else if (deploymentScope == DeploymentScope.ManagementGroup || deploymentScope == DeploymentScope.Tenant)
resourceId = ResourceHelper.CombineResourceId(null, null, type, name);

context.UpdateResourceScope(resource);
Expand All @@ -1315,14 +1329,51 @@ private static ResourceValue ResourceInstance(TemplateContext context, JObject r
return result;
}

private static string ResolveDeploymentScopeProperty(TemplateContext context, JObject resource, string propertyName, string defaultValue)
/// <summary>
/// Get the deployment scope for the resource.
/// </summary>
/// <param name="context">The current context.</param>
/// <param name="resource">The resource.</param>
/// <param name="type">The resource type.</param>
/// <param name="subscriptionId">Returns the subscription ID if the scope is within a subscription.</param>
/// <param name="resourceGroupName">Returns the resource group name if the scope is with a resource group.</param>
/// <returns>The deployment scope.</returns>
private static DeploymentScope GetDeploymentScope(TemplateContext context, JObject resource, string type, out string subscriptionId, out string resourceGroupName)
{
if (resource.TryGetProperty<JValue>(propertyName, out var value))
if (!IsDeploymentResource(type))
{
resourceGroupName = context.ResourceGroup.Name;
subscriptionId = context.Subscription.SubscriptionId;
return context.Deployment.DeploymentScope;
}

// Handle special case for cross-scope deployments which may have an alternative subscription or resource group set.
subscriptionId = ResolveDeploymentScopeProperty(context, resource, PROPERTY_SUBSCRIPTIONID, contextValue:
context.Deployment.DeploymentScope == DeploymentScope.Subscription ||
context.Deployment.DeploymentScope == DeploymentScope.ResourceGroup ? context.Subscription.SubscriptionId : null);
resourceGroupName = ResolveDeploymentScopeProperty(context, resource, PROPERTY_RESOURCEGROUP, contextValue:
context.Deployment.DeploymentScope == DeploymentScope.ResourceGroup ? context.ResourceGroup.Name : null);

// Update the deployment scope.
if (context.Deployment.DeploymentScope == DeploymentScope.ResourceGroup || resourceGroupName != null)
{
defaultValue = ResolveToken(context, value).Value<string>();
resource[propertyName] = defaultValue;
return DeploymentScope.ResourceGroup;
}
return defaultValue;
if (context.Deployment.DeploymentScope == DeploymentScope.Subscription || subscriptionId != null)
{
return DeploymentScope.Subscription;
}
return context.Deployment.DeploymentScope;
}

private static string ResolveDeploymentScopeProperty(TemplateContext context, JObject resource, string propertyName, string contextValue)
{
var resolvedValue = contextValue;
if (resource.TryGetProperty<JValue>(propertyName, out var value) && value.Type != JTokenType.Null)
resolvedValue = ResolveToken(context, value).Value<string>();

resource[propertyName] = resolvedValue;
return resolvedValue;
}

private void ResourceOuter(TemplateContext context, IResourceValue resource)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,9 @@
<None Update="Tests.Bicep.37.json">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</None>
<None Update="Tests.Bicep.38.json">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</None>
<None Update="Tests.Bicep.4.json">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</None>
Expand Down
26 changes: 26 additions & 0 deletions tests/PSRule.Rules.Azure.Tests/TemplateVisitorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1151,6 +1151,32 @@ public void ProcessTemplate_WhenListFromKnownSecretType_ShouldReturnSecretObject
Assert.Equal("{{Secret}}", secretValue);
}

/// <summary>
/// Test case for https://github.com/Azure/PSRule.Rules.Azure/issues/2850.
/// </summary>
[Fact]
public void ProcessTemplate_WhenResourceReferenced_AlwaysPopulatedId()
{
var resources = ProcessTemplate(GetSourcePath("Tests.Bicep.38.json"), null, out _);

// Check managed identity deployment.
var actual = resources.FirstOrDefault(r => r["type"].Value<string>() == "Microsoft.Resources/deployments" && r["name"].Value<string>() == "mi");
Assert.Equal("/subscriptions/ffffffff-ffff-ffff-ffff-ffffffffffff/resourceGroups/rg1", actual["scope"].Value<string>());

// Check the app service deployment.
actual = resources.FirstOrDefault(r => r["type"].Value<string>() == "Microsoft.Resources/deployments" && r["name"].Value<string>() == "site");
Assert.Equal("/subscriptions/ffffffff-ffff-ffff-ffff-ffffffffffff/resourceGroups/rg2", actual["scope"].Value<string>());

// Check the managed identity resource
actual = resources.FirstOrDefault(r => r["type"].Value<string>() == "Microsoft.ManagedIdentity/userAssignedIdentities" && r["name"].Value<string>() == "mi");
Assert.Equal("/subscriptions/ffffffff-ffff-ffff-ffff-ffffffffffff/resourceGroups/rg1", actual["scope"].Value<string>());

// Check app service resource.
actual = resources.FirstOrDefault(r => r["type"].Value<string>() == "Microsoft.Web/sites" && r["name"].Value<string>() == "app");
Assert.Equal("/subscriptions/ffffffff-ffff-ffff-ffff-ffffffffffff/resourceGroups/rg2", actual["scope"].Value<string>());
Assert.True(actual["identity"]["userAssignedIdentities"].Value<JObject>().TryObjectProperty("/subscriptions/ffffffff-ffff-ffff-ffff-ffffffffffff/resourceGroups/rg1/providers/Microsoft.ManagedIdentity/userAssignedIdentities/mi", out var o));
}

#region Helper methods

private static string GetSourcePath(string fileName)
Expand Down
28 changes: 28 additions & 0 deletions tests/PSRule.Rules.Azure.Tests/Tests.Bicep.38.bicep
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

// Test case for https://github.com/Azure/PSRule.Rules.Azure/issues/2850

// Handle from subscription scope.
targetScope = 'subscription'

param location string = deployment().location

module site './Tests.Bicep.38.site.bicep' = {
name: 'site'
scope: resourceGroup('rg2')
params: {
name: 'app'
location: location
identityId: identity.outputs.id
}
}

module identity './Tests.Bicep.38.child.bicep' = {
name: 'mi'
scope: resourceGroup('rg1')
params: {
name: 'mi'
location: location
}
}
12 changes: 12 additions & 0 deletions tests/PSRule.Rules.Azure.Tests/Tests.Bicep.38.child.bicep
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

param name string
param location string = resourceGroup().location

resource identity 'Microsoft.ManagedIdentity/userAssignedIdentities@2023-01-31' = {
name: name
location: location
}

output id string = identity.id
137 changes: 137 additions & 0 deletions tests/PSRule.Rules.Azure.Tests/Tests.Bicep.38.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
{
"$schema": "https://schema.management.azure.com/schemas/2018-05-01/subscriptionDeploymentTemplate.json#",
"contentVersion": "1.0.0.0",
"metadata": {
"_generator": {
"name": "bicep",
"version": "0.26.170.59819",
"templateHash": "17875932582636530478"
}
},
"parameters": {
"location": {
"type": "string",
"defaultValue": "[deployment().location]"
}
},
"resources": [
{
"type": "Microsoft.Resources/deployments",
"apiVersion": "2022-09-01",
"name": "site",
"resourceGroup": "rg2",
"properties": {
"expressionEvaluationOptions": {
"scope": "inner"
},
"mode": "Incremental",
"parameters": {
"name": {
"value": "app"
},
"location": {
"value": "[parameters('location')]"
},
"identityId": {
"value": "[reference(extensionResourceId(format('/subscriptions/{0}/resourceGroups/{1}', subscription().subscriptionId, 'rg1'), 'Microsoft.Resources/deployments', 'mi'), '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.26.170.59819",
"templateHash": "9869420960933296536"
}
},
"parameters": {
"location": {
"type": "string",
"defaultValue": "[resourceGroup().location]"
},
"identityId": {
"type": "string"
},
"name": {
"type": "string"
}
},
"resources": [
{
"type": "Microsoft.Web/sites",
"apiVersion": "2023-01-01",
"name": "[parameters('name')]",
"location": "[parameters('location')]",
"identity": {
"type": "UserAssigned",
"userAssignedIdentities": {
"[format('{0}', parameters('identityId'))]": {}
}
},
"properties": {}
}
]
}
},
"dependsOn": [
"[extensionResourceId(format('/subscriptions/{0}/resourceGroups/{1}', subscription().subscriptionId, 'rg1'), 'Microsoft.Resources/deployments', 'mi')]"
]
},
{
"type": "Microsoft.Resources/deployments",
"apiVersion": "2022-09-01",
"name": "mi",
"resourceGroup": "rg1",
"properties": {
"expressionEvaluationOptions": {
"scope": "inner"
},
"mode": "Incremental",
"parameters": {
"name": {
"value": "mi"
},
"location": {
"value": "[parameters('location')]"
}
},
"template": {
"$schema": "https://schema.management.azure.com/schemas/2019-04-01/deploymentTemplate.json#",
"contentVersion": "1.0.0.0",
"metadata": {
"_generator": {
"name": "bicep",
"version": "0.26.170.59819",
"templateHash": "15990157618564203975"
}
},
"parameters": {
"name": {
"type": "string"
},
"location": {
"type": "string",
"defaultValue": "[resourceGroup().location]"
}
},
"resources": [
{
"type": "Microsoft.ManagedIdentity/userAssignedIdentities",
"apiVersion": "2023-01-31",
"name": "[parameters('name')]",
"location": "[parameters('location')]"
}
],
"outputs": {
"id": {
"type": "string",
"value": "[resourceId('Microsoft.ManagedIdentity/userAssignedIdentities', parameters('name'))]"
}
}
}
}
}
]
}
18 changes: 18 additions & 0 deletions tests/PSRule.Rules.Azure.Tests/Tests.Bicep.38.site.bicep
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

param location string = resourceGroup().location
param identityId string
param name string

resource app 'Microsoft.Web/sites@2023-01-01' = {
name: name
location: location
identity: {
type: 'UserAssigned'
userAssignedIdentities: {
'${identityId}': {}
}
}
properties: {}
}

0 comments on commit cbe5b9b

Please sign in to comment.