diff --git a/docs/CHANGELOG-v1.md b/docs/CHANGELOG-v1.md index 5ad20f67a04..7a5517d29ec 100644 --- a/docs/CHANGELOG-v1.md +++ b/docs/CHANGELOG-v1.md @@ -28,6 +28,12 @@ See [upgrade notes][1] for helpful information when upgrading from previous vers ## Unreleased +What's changed since v1.31.2: + +- Bug fixes: + - Fixed incorrect scope generated for subscription aliases by @BernieWhite. + [#2545](https://github.com/Azure/PSRule.Rules.Azure/issues/2545) + ## v1.31.2 What's changed since v1.31.1: diff --git a/src/PSRule.Rules.Azure/Data/Template/DeploymentScope.cs b/src/PSRule.Rules.Azure/Data/Template/DeploymentScope.cs new file mode 100644 index 00000000000..2e39263233b --- /dev/null +++ b/src/PSRule.Rules.Azure/Data/Template/DeploymentScope.cs @@ -0,0 +1,16 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +namespace PSRule.Rules.Azure.Data.Template +{ + internal enum DeploymentScope + { + ResourceGroup = 0, + + Subscription = 1, + + ManagementGroup = 2, + + Tenant = 3, + } +} diff --git a/src/PSRule.Rules.Azure/Data/Template/ResourceValue.cs b/src/PSRule.Rules.Azure/Data/Template/ResourceValue.cs index 9f729653cc8..787d42255bb 100644 --- a/src/PSRule.Rules.Azure/Data/Template/ResourceValue.cs +++ b/src/PSRule.Rules.Azure/Data/Template/ResourceValue.cs @@ -121,17 +121,18 @@ internal sealed class DeploymentValue : BaseResourceValue, IResourceValue, ILazy private readonly Lazy _Value; private readonly Dictionary _Outputs; - internal DeploymentValue(string id, string name, string symbolicName, JObject value, string[] dependencies, TemplateContext.CopyIndexState copy) - : this(id, name, symbolicName, () => value, dependencies, copy) { } + internal DeploymentValue(string id, string name, string symbolicName, string scope, DeploymentScope deploymentScope, JObject value, string[] dependencies, TemplateContext.CopyIndexState copy) + : this(id, name, symbolicName, scope, deploymentScope, () => value, dependencies, copy) { } - internal DeploymentValue(string id, string name, string symbolicName, Func value, string[] dependencies, TemplateContext.CopyIndexState copy) + internal DeploymentValue(string id, string name, string symbolicName, string scope, DeploymentScope deploymentScope, Func value, string[] dependencies, TemplateContext.CopyIndexState copy) : base(id, name, symbolicName, dependencies) { - _Value = new Lazy(value); Copy = copy; Properties = new DeploymentProperties(this); _Outputs = new Dictionary(StringComparer.OrdinalIgnoreCase); + Scope = scope; + DeploymentScope = deploymentScope; } private sealed class DeploymentProperties : ILazyObject @@ -187,6 +188,10 @@ public bool TryProperty(string propertyName, out object value) public ILazyObject Properties { get; } + public string Scope { get; } + + public DeploymentScope DeploymentScope { get; } + public void AddOutput(string name, ILazyValue output) { _Outputs.Add(name, output); diff --git a/src/PSRule.Rules.Azure/Data/Template/RuleDataExportVisitor.cs b/src/PSRule.Rules.Azure/Data/Template/RuleDataExportVisitor.cs index db4474daefa..ed390997ce5 100644 --- a/src/PSRule.Rules.Azure/Data/Template/RuleDataExportVisitor.cs +++ b/src/PSRule.Rules.Azure/Data/Template/RuleDataExportVisitor.cs @@ -28,6 +28,12 @@ internal sealed class RuleDataExportVisitor : TemplateVisitor private const string PROPERTY_SITECONFIG = "siteConfig"; private const string PROPERTY_SUBNETS = "subnets"; private const string PROPERTY_NETWORKINTERFACES = "networkInterfaces"; + private const string PROPERTY_SUBSCRIPTIONID = "subscriptionId"; + private const string PROPERTY_ACCEPTOWNERSHIPSTATE = "acceptOwnershipState"; + private const string PROPERTY_ACCEPTOWNERSHIPURL = "acceptOwnershipUrl"; + private const string PROPERTY_LOGINSERVER = "loginServer"; + private const string PROPERTY_RULES = "rules"; + private const string PROPERTY_RULEID = "ruleId"; private const string PLACEHOLDER_GUID = "ffffffff-ffff-ffff-ffff-ffffffffffff"; private const string IDENTITY_SYSTEMASSIGNED = "SystemAssigned"; @@ -42,6 +48,9 @@ internal sealed class RuleDataExportVisitor : TemplateVisitor private const string TYPE_VIRTUALNETWORK = "Microsoft.Network/virtualNetworks"; private const string TYPE_PRIVATEENDPOINT = "Microsoft.Network/privateEndpoints"; private const string TYPE_NETWORKINTERFACE = "Microsoft.Network/networkInterfaces"; + private const string TYPE_SUBSCRIPTIONALIAS = "Microsoft.Subscription/aliases"; + private const string TYPE_CONTAINERREGISTRY = "Microsoft.ContainerRegistry/registries"; + private const string TYPE_STORAGE_OBJECTREPLICATIONPOLICIES = "Microsoft.Storage/storageAccounts/objectReplicationPolicies"; private static readonly JsonMergeSettings _MergeSettings = new() { @@ -116,7 +125,10 @@ private static void ProjectRuntimeProperties(TemplateContext context, IResourceV { _ = ProjectManagedIdentity(context, resource) || ProjectVirtualNetwork(context, resource) || + ProjectContainerRegistry(context, resource) || ProjectPrivateEndpoints(context, resource) || + ProjectSubscriptionAlias(context, resource) || + StorageObjectReplicationPolicies(context, resource) || ProjectResource(context, resource); } @@ -195,6 +207,68 @@ private static bool ProjectPrivateEndpoints(TemplateContext context, IResourceVa return true; } + private static bool ProjectSubscriptionAlias(TemplateContext context, IResourceValue resource) + { + if (!resource.IsType(TYPE_SUBSCRIPTIONALIAS)) + return false; + + resource.Value.UseProperty(PROPERTY_PROPERTIES, out JObject properties); + + // Add subscriptionId + if (!properties.ContainsKeyInsensitive(PROPERTY_SUBSCRIPTIONID)) + { + properties[PROPERTY_SUBSCRIPTIONID] = Guid.NewGuid().ToString(); + } + + // Add acceptOwnershipState + if (!properties.ContainsKeyInsensitive(PROPERTY_ACCEPTOWNERSHIPSTATE)) + { + properties[PROPERTY_ACCEPTOWNERSHIPSTATE] = "Completed"; + } + + // Add acceptOwnershipUrl + if (!properties.ContainsKeyInsensitive(PROPERTY_ACCEPTOWNERSHIPURL)) + { + properties[PROPERTY_ACCEPTOWNERSHIPURL] = string.Empty; + } + return true; + } + + private static bool ProjectContainerRegistry(TemplateContext context, IResourceValue resource) + { + if (!resource.IsType(TYPE_CONTAINERREGISTRY)) + return false; + + resource.Value.UseProperty(PROPERTY_PROPERTIES, out JObject properties); + + // Add loginServer + if (!properties.ContainsKeyInsensitive(PROPERTY_LOGINSERVER)) + { + properties[PROPERTY_LOGINSERVER] = $"{resource.Name}.azurecr.io"; + } + return true; + } + + private static bool StorageObjectReplicationPolicies(TemplateContext context, IResourceValue resource) + { + if (!resource.IsType(TYPE_STORAGE_OBJECTREPLICATIONPOLICIES)) + return false; + + resource.Value.UseProperty(PROPERTY_PROPERTIES, out JObject properties); + + // Add rules.ruleId + if (properties.TryArrayProperty(PROPERTY_RULES, out var rules)) + { + foreach (var rule in rules.Values()) + { + if (!rule.ContainsKeyInsensitive(PROPERTY_RULEID)) + rule[PROPERTY_RULEID] = Guid.NewGuid().ToString(); + } + + } + return true; + } + private static bool ProjectSQLServer(IResourceValue resource) { if (!resource.IsType(TYPE_SQLSERVER)) diff --git a/src/PSRule.Rules.Azure/Data/Template/TemplateVisitor.cs b/src/PSRule.Rules.Azure/Data/Template/TemplateVisitor.cs index 6e1c397cc1e..3807d8aed23 100644 --- a/src/PSRule.Rules.Azure/Data/Template/TemplateVisitor.cs +++ b/src/PSRule.Rules.Azure/Data/Template/TemplateVisitor.cs @@ -25,6 +25,7 @@ namespace PSRule.Rules.Azure.Data.Template /// internal abstract class TemplateVisitor : ResourceManagerVisitor { + private const string TENANT_SCOPE = "/"; private const string RESOURCETYPE_DEPLOYMENT = "Microsoft.Resources/deployments"; private const string DEPLOYMENTSCOPE_INNER = "inner"; private const string PROPERTY_SCHEMA = "$schema"; @@ -174,7 +175,7 @@ internal TemplateContext(PipelineContext context) public ParameterDefaultsOption ParameterDefaults { get; private set; } /// - public DeploymentValue Deployment => _Deployment.Peek(); + public DeploymentValue Deployment => _Deployment.Count > 0 ? _Deployment.Peek() : null; public string TemplateFile { get; private set; } @@ -288,6 +289,15 @@ private bool TryResourceScope(JObject resource, out string scopeId) return false; var scope = ExpandProperty(this, resource, PROPERTY_SCOPE); + + // Check for tenant scope. + if (scope == TENANT_SCOPE) + { + //scopeId = Deployment.Scope; + scopeId = scope; + return true; + } + ResourceHelper.TryResourceIdComponents(scope, out var subscriptionId, out var resourceGroupName, out string[] resourceTypeComponents, out string[] nameComponents); subscriptionId ??= Subscription.SubscriptionId; resourceGroupName ??= ResourceGroup.Name; @@ -307,10 +317,17 @@ private bool TryParentScope(JObject resource, out string scopeId) return true; } + private bool TryDeploymentScope(out string scopeId) + { + scopeId = Deployment?.Scope; + return scopeId != null; + } + public void UpdateResourceScope(JObject resource) { if (!TryResourceScope(resource, out var scopeId) && - !TryParentScope(resource, out scopeId)) + !TryParentScope(resource, out scopeId) && + !TryDeploymentScope(out scopeId)) return; resource[PROPERTY_SCOPE] = scopeId; @@ -471,6 +488,11 @@ internal void EnterDeployment(string deploymentName, JObject template, bool isNe { var templateHash = template.GetHashCode().ToString(Thread.CurrentThread.CurrentCulture); 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/", RESOURCETYPE_DEPLOYMENT, "/", deploymentName); + var location = ResourceGroup.Location; + var templateLink = new JObject { { PROPERTY_ID, ResourceGroup.Id }, @@ -493,17 +515,20 @@ internal void EnterDeployment(string deploymentName, JObject template, bool isNe { PROPERTY_RESOURCENAME, isNested ? deploymentName : ParameterFile ?? TemplateFile }, { PROPERTY_NAME, deploymentName }, { PROPERTY_PROPERTIES, properties }, - { PROPERTY_LOCATION, ResourceGroup.Location }, + { PROPERTY_LOCATION, location }, { PROPERTY_TYPE, RESOURCETYPE_DEPLOYMENT }, { PROPERTY_METADATA, metadata }, + { PROPERTY_ID, id }, + { PROPERTY_SCOPE, scope }, + // Add a property to allow rules to detect root deployment. Related to: https://github.com/Azure/PSRule.Rules.Azure/issues/2109 { PROPERTY_ROOTDEPLOYMENT, !isNested } }; var path = template.GetResourcePath(parentLevel: 2); deployment.SetTargetInfo(TemplateFile, ParameterFile, path); - var deploymentValue = new DeploymentValue(string.Concat(ResourceGroup.Id, "/providers/", RESOURCETYPE_DEPLOYMENT, "/", deploymentName), deploymentName, null, deployment, null, null); + var deploymentValue = new DeploymentValue(id, deploymentName, null, scope, deploymentScope, deployment, null, null); AddResource(deploymentValue); _CurrentDeployment = deploymentValue; _Deployment.Push(deploymentValue); @@ -511,6 +536,39 @@ internal void EnterDeployment(string deploymentName, JObject template, bool isNe RootDeployment = _CurrentDeployment; } + private string GetDeploymentScope(string schema, out DeploymentScope deploymentScope) + { + if (!string.IsNullOrEmpty(schema)) + { + schema = schema.TrimEnd('#', ' '); + var parts = schema.Split('/'); + var template = parts[parts.Length - 1]; + + // Management Group + if (string.Equals(template, "managementGroupDeploymentTemplate.json", StringComparison.OrdinalIgnoreCase)) + { + deploymentScope = DeploymentScope.ManagementGroup; + return ManagementGroup.Id; + } + // Subscription + if (string.Equals(template, "subscriptionDeploymentTemplate.json", StringComparison.OrdinalIgnoreCase)) + { + deploymentScope = DeploymentScope.Subscription; + return Subscription.Id; + } + // Tenant + if (string.Equals(template, "tenantDeploymentTemplate.json", StringComparison.OrdinalIgnoreCase)) + { + deploymentScope = DeploymentScope.Tenant; + return Tenant.Id; + } + } + + // Resource Group + deploymentScope = DeploymentScope.ResourceGroup; + return ResourceGroup.Id; + } + internal void ExitDeployment() { _Deployment.Pop(); @@ -1200,7 +1258,16 @@ private static ResourceValue ResourceInstance(TemplateContext context, JObject r resourceGroupName = ResolveDeploymentScopeProperty(context, resource, PROPERTY_RESOURCEGROUP, resourceGroupName); } - var resourceId = ResourceHelper.CombineResourceId(subscriptionId, resourceGroupName, type, name); + string resourceId = null; + if (context.Deployment.DeploymentScope == DeploymentScope.ResourceGroup) + resourceId = ResourceHelper.CombineResourceId(subscriptionId, resourceGroupName, type, name); + + if (context.Deployment.DeploymentScope == DeploymentScope.Subscription) + resourceId = ResourceHelper.CombineResourceId(subscriptionId, null, type, name); + + if (context.Deployment.DeploymentScope == DeploymentScope.ManagementGroup || context.Deployment.DeploymentScope == DeploymentScope.Tenant) + resourceId = ResourceHelper.CombineResourceId(null, null, type, name); + context.UpdateResourceScope(resource); resource[PROPERTY_ID] = resourceId; return new ResourceValue(resourceId, name, type, symbolicName, resource, dependencies, copyIndex.Clone()); 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 3b402468941..fffb2bfdec2 100644 --- a/tests/PSRule.Rules.Azure.Tests/PSRule.Rules.Azure.Tests.csproj +++ b/tests/PSRule.Rules.Azure.Tests/PSRule.Rules.Azure.Tests.csproj @@ -209,6 +209,9 @@ PreserveNewest + + PreserveNewest + PreserveNewest diff --git a/tests/PSRule.Rules.Azure.Tests/TemplateVisitorTests.cs b/tests/PSRule.Rules.Azure.Tests/TemplateVisitorTests.cs index 10200110273..24f396e83ae 100644 --- a/tests/PSRule.Rules.Azure.Tests/TemplateVisitorTests.cs +++ b/tests/PSRule.Rules.Azure.Tests/TemplateVisitorTests.cs @@ -951,6 +951,23 @@ public void ContainsWithJValue() Assert.Equal("bc", actual["properties"]["stringToFind"].Value()); } + [Fact] + public void ManagementGroupScopedResource() + { + var resources = ProcessTemplate(GetSourcePath("Tests.Bicep.31.json"), null, out _); + Assert.Equal(4, resources.Length); + + var actual = resources[1]; + Assert.Equal("Microsoft.Subscription/aliases", actual["type"].Value()); + Assert.Equal("/", actual["scope"].Value()); + Assert.Equal("/providers/Microsoft.Subscription/aliases/sub1", actual["id"].Value()); + + actual = resources[3]; + Assert.Equal("Microsoft.Authorization/roleAssignments", actual["type"].Value()); + Assert.Equal("/subscriptions/00000000-0000-0000-0000-000000000000", actual["scope"].Value()); + Assert.Equal("/subscriptions/00000000-0000-0000-0000-000000000000/providers/Microsoft.Authorization/roleAssignments/8a869b90-6d6c-4307-9d0f-22dbc136ccd9", actual["id"].Value()); + } + #region Helper methods private static string GetSourcePath(string fileName) diff --git a/tests/PSRule.Rules.Azure.Tests/Tests.Bicep.31.bicep b/tests/PSRule.Rules.Azure.Tests/Tests.Bicep.31.bicep new file mode 100644 index 00000000000..4d53e28f976 --- /dev/null +++ b/tests/PSRule.Rules.Azure.Tests/Tests.Bicep.31.bicep @@ -0,0 +1,19 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +targetScope = 'managementGroup' + +resource subscriptionAlias 'Microsoft.Subscription/aliases@2021-10-01' = { + scope: tenant() + name: 'sub1' + properties: { + workload: 'DevTest' + displayName: 'sub1' + billingScope: ' /billingAccounts/nn/enrollmentAccounts/nn' + } +} + +module rbac './Tests.Bicep.31.child.bicep' = { + scope: subscription('00000000-0000-0000-0000-000000000000') + name: 'rbac' +} diff --git a/tests/PSRule.Rules.Azure.Tests/Tests.Bicep.31.child.bicep b/tests/PSRule.Rules.Azure.Tests/Tests.Bicep.31.child.bicep new file mode 100644 index 00000000000..e4956514688 --- /dev/null +++ b/tests/PSRule.Rules.Azure.Tests/Tests.Bicep.31.child.bicep @@ -0,0 +1,14 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +targetScope = 'subscription' + +resource assignment 'Microsoft.Authorization/roleAssignments@2022-04-01' = { + name: '8a869b90-6d6c-4307-9d0f-22dbc136ccd9' + properties: { + principalId: '8a869b90-6d6c-4307-9d0f-22dbc136ccd9' + roleDefinitionId: '8a869b90-6d6c-4307-9d0f-22dbc136ccd9' + principalType: 'ServicePrincipal' + description: 'Test role assignment for checking scope and ID.' + } +} diff --git a/tests/PSRule.Rules.Azure.Tests/Tests.Bicep.31.json b/tests/PSRule.Rules.Azure.Tests/Tests.Bicep.31.json new file mode 100644 index 00000000000..dd12a166a26 --- /dev/null +++ b/tests/PSRule.Rules.Azure.Tests/Tests.Bicep.31.json @@ -0,0 +1,61 @@ +{ + "$schema": "https://schema.management.azure.com/schemas/2019-08-01/managementGroupDeploymentTemplate.json#", + "contentVersion": "1.0.0.0", + "metadata": { + "_generator": { + "name": "bicep", + "version": "0.23.1.45101", + "templateHash": "1432163185374769317" + } + }, + "resources": [ + { + "type": "Microsoft.Subscription/aliases", + "apiVersion": "2021-10-01", + "scope": "/", + "name": "sub1", + "properties": { + "workload": "DevTest", + "displayName": "sub1", + "billingScope": " /billingAccounts/nn/enrollmentAccounts/nn" + } + }, + { + "type": "Microsoft.Resources/deployments", + "apiVersion": "2022-09-01", + "name": "rbac", + "subscriptionId": "00000000-0000-0000-0000-000000000000", + "location": "[deployment().location]", + "properties": { + "expressionEvaluationOptions": { + "scope": "inner" + }, + "mode": "Incremental", + "template": { + "$schema": "https://schema.management.azure.com/schemas/2018-05-01/subscriptionDeploymentTemplate.json#", + "contentVersion": "1.0.0.0", + "metadata": { + "_generator": { + "name": "bicep", + "version": "0.23.1.45101", + "templateHash": "17477755083280448345" + } + }, + "resources": [ + { + "type": "Microsoft.Authorization/roleAssignments", + "apiVersion": "2022-04-01", + "name": "8a869b90-6d6c-4307-9d0f-22dbc136ccd9", + "properties": { + "principalId": "8a869b90-6d6c-4307-9d0f-22dbc136ccd9", + "roleDefinitionId": "8a869b90-6d6c-4307-9d0f-22dbc136ccd9", + "principalType": "ServicePrincipal", + "description": "Test role assignment for checking scope and ID." + } + } + ] + } + } + } + ] +} \ No newline at end of file