Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge patch v1.41.1 #3249

Merged
merged 4 commits into from
Feb 8, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion docs/CHANGELOG-v1.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
14 changes: 14 additions & 0 deletions src/PSRule.Rules.Azure/Common/ResourceHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Check warning on line 74 in src/PSRule.Rules.Azure/Common/ResourceHelper.cs

View workflow job for this annotation

GitHub Actions / Build

Possible null reference argument for parameter 'resourceId' in 'string[] ResourceHelper.GetResourceIdTypeParts(string resourceId)'.
}

/// <summary>
Expand Down Expand Up @@ -120,6 +120,20 @@
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>
Expand Down Expand Up @@ -168,7 +182,7 @@
while (i < result.Length && j <= depth)
{
// If a resource provider is included prepend /providers.
if (resourceType[j].Contains(DOT))

Check warning on line 185 in src/PSRule.Rules.Azure/Common/ResourceHelper.cs

View workflow job for this annotation

GitHub Actions / Build

Dereference of a possibly null reference.
{
result[i++] = SLASH;
result[i++] = PROVIDERS;
Expand All @@ -177,23 +191,23 @@
result[i++] = SLASH;
result[i++] = resourceType[j];
result[i++] = SLASH;
result[i++] = name[j++];

Check warning on line 194 in src/PSRule.Rules.Azure/Common/ResourceHelper.cs

View workflow job for this annotation

GitHub Actions / Build

Dereference of a possibly null reference.
}
}
return string.Concat(result);
}

internal static string CombineResourceId(string subscriptionId, string resourceGroup, string resourceType, string name, int depth = int.MaxValue, string scope = null)

Check warning on line 200 in src/PSRule.Rules.Azure/Common/ResourceHelper.cs

View workflow job for this annotation

GitHub Actions / Build

Cannot convert null literal to non-nullable reference type.
{
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))

Check warning on line 205 in src/PSRule.Rules.Azure/Common/ResourceHelper.cs

View workflow job for this annotation

GitHub Actions / Build

Converting null literal or possible null value to non-nullable type.
{
typeComponents = MergeResourceNameOrType(parentTypeComponents, typeComponents);
nameComponents = MergeResourceNameOrType(parentNameComponents, nameComponents);
}
return CombineResourceId(subscriptionId, resourceGroup, typeComponents, nameComponents, depth);

Check warning on line 210 in src/PSRule.Rules.Azure/Common/ResourceHelper.cs

View workflow job for this annotation

GitHub Actions / Build

Possible null reference argument for parameter 'resourceGroup' in 'string ResourceHelper.CombineResourceId(string subscriptionId, string resourceGroup, string[] resourceType, string[] name, int depth = 2147483647)'.

Check warning on line 210 in src/PSRule.Rules.Azure/Common/ResourceHelper.cs

View workflow job for this annotation

GitHub Actions / Build

Possible null reference argument for parameter 'resourceType' in 'string ResourceHelper.CombineResourceId(string subscriptionId, string resourceGroup, string[] resourceType, string[] name, int depth = 2147483647)'.

Check warning on line 210 in src/PSRule.Rules.Azure/Common/ResourceHelper.cs

View workflow job for this annotation

GitHub Actions / Build

Possible null reference argument for parameter 'name' in 'string ResourceHelper.CombineResourceId(string subscriptionId, string resourceGroup, string[] resourceType, string[] name, int depth = 2147483647)'.
}

internal static string ResourceId(string resourceType, string resourceName, string? scopeId, int depth = int.MaxValue)
Expand Down Expand Up @@ -484,9 +498,9 @@
while (i < result.Length && j <= depth)
{
result[i++] = SLASH;
result[i++] = resourceType[j];

Check warning on line 501 in src/PSRule.Rules.Azure/Common/ResourceHelper.cs

View workflow job for this annotation

GitHub Actions / Build

Dereference of a possibly null reference.
result[i++] = SLASH;
result[i++] = name[j++];

Check warning on line 503 in src/PSRule.Rules.Azure/Common/ResourceHelper.cs

View workflow job for this annotation

GitHub Actions / Build

Dereference of a possibly null reference.
}
}
return string.Concat(result);
Expand Down
40 changes: 26 additions & 14 deletions src/PSRule.Rules.Azure/Data/Template/TemplateVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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";
Expand Down Expand Up @@ -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
Expand All @@ -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() }
};
Expand Down Expand Up @@ -673,7 +673,7 @@ internal bool TryParameterDefault(string parameterName, ParameterType type, out
return true;
}
break;
};
}
return false;
}

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down
4 changes: 3 additions & 1 deletion src/PSRule.Rules.Azure/Pipeline/ResourceDataPipeline.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>());
}
}
Original file line number Diff line number Diff line change
@@ -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
}
]
}
}
Original file line number Diff line number Diff line change
@@ -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'
Original file line number Diff line number Diff line change
@@ -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
}
}
]
Original file line number Diff line number Diff line change
@@ -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}'
}
Loading
Loading