Skip to content

Commit

Permalink
Fixes scoping of subscription aliases #2545 (#2547)
Browse files Browse the repository at this point in the history
  • Loading branch information
BernieWhite authored Nov 22, 2023
1 parent 363b48e commit 1e9086e
Show file tree
Hide file tree
Showing 10 changed files with 291 additions and 9 deletions.
6 changes: 6 additions & 0 deletions docs/CHANGELOG-v1.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
16 changes: 16 additions & 0 deletions src/PSRule.Rules.Azure/Data/Template/DeploymentScope.cs
Original file line number Diff line number Diff line change
@@ -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,
}
}
13 changes: 9 additions & 4 deletions src/PSRule.Rules.Azure/Data/Template/ResourceValue.cs
Original file line number Diff line number Diff line change
Expand Up @@ -121,17 +121,18 @@ internal sealed class DeploymentValue : BaseResourceValue, IResourceValue, ILazy
private readonly Lazy<JObject> _Value;
private readonly Dictionary<string, ILazyValue> _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<JObject> value, string[] dependencies, TemplateContext.CopyIndexState copy)
internal DeploymentValue(string id, string name, string symbolicName, string scope, DeploymentScope deploymentScope, Func<JObject> value, string[] dependencies, TemplateContext.CopyIndexState copy)
: base(id, name, symbolicName, dependencies)
{

_Value = new Lazy<JObject>(value);
Copy = copy;
Properties = new DeploymentProperties(this);
_Outputs = new Dictionary<string, ILazyValue>(StringComparer.OrdinalIgnoreCase);
Scope = scope;
DeploymentScope = deploymentScope;
}

private sealed class DeploymentProperties : ILazyObject
Expand Down Expand Up @@ -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);
Expand Down
74 changes: 74 additions & 0 deletions src/PSRule.Rules.Azure/Data/Template/RuleDataExportVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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()
{
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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<JObject>())
{
if (!rule.ContainsKeyInsensitive(PROPERTY_RULEID))
rule[PROPERTY_RULEID] = Guid.NewGuid().ToString();
}

}
return true;
}

private static bool ProjectSQLServer(IResourceValue resource)
{
if (!resource.IsType(TYPE_SQLSERVER))
Expand Down
77 changes: 72 additions & 5 deletions src/PSRule.Rules.Azure/Data/Template/TemplateVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ namespace PSRule.Rules.Azure.Data.Template
/// </summary>
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";
Expand Down Expand Up @@ -174,7 +175,7 @@ internal TemplateContext(PipelineContext context)
public ParameterDefaultsOption ParameterDefaults { get; private set; }

/// <inheritdoc/>
public DeploymentValue Deployment => _Deployment.Peek();
public DeploymentValue Deployment => _Deployment.Count > 0 ? _Deployment.Peek() : null;

public string TemplateFile { get; private set; }

Expand Down Expand Up @@ -288,6 +289,15 @@ private bool TryResourceScope(JObject resource, out string scopeId)
return false;

var scope = ExpandProperty<string>(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;
Expand All @@ -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;
Expand Down Expand Up @@ -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 },
Expand All @@ -493,24 +515,60 @@ 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);
if (!isNested)
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();
Expand Down Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,9 @@
<None Update="Tests.Bicep.30.json">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</None>
<None Update="Tests.Bicep.31.json">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</None>
<None Update="Tests.Bicep.4.json">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</None>
Expand Down
17 changes: 17 additions & 0 deletions tests/PSRule.Rules.Azure.Tests/TemplateVisitorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -951,6 +951,23 @@ public void ContainsWithJValue()
Assert.Equal("bc", actual["properties"]["stringToFind"].Value<string>());
}

[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<string>());
Assert.Equal("/", actual["scope"].Value<string>());
Assert.Equal("/providers/Microsoft.Subscription/aliases/sub1", actual["id"].Value<string>());

actual = resources[3];
Assert.Equal("Microsoft.Authorization/roleAssignments", actual["type"].Value<string>());
Assert.Equal("/subscriptions/00000000-0000-0000-0000-000000000000", actual["scope"].Value<string>());
Assert.Equal("/subscriptions/00000000-0000-0000-0000-000000000000/providers/Microsoft.Authorization/roleAssignments/8a869b90-6d6c-4307-9d0f-22dbc136ccd9", actual["id"].Value<string>());
}

#region Helper methods

private static string GetSourcePath(string fileName)
Expand Down
19 changes: 19 additions & 0 deletions tests/PSRule.Rules.Azure.Tests/Tests.Bicep.31.bicep
Original file line number Diff line number Diff line change
@@ -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'
}
14 changes: 14 additions & 0 deletions tests/PSRule.Rules.Azure.Tests/Tests.Bicep.31.child.bicep
Original file line number Diff line number Diff line change
@@ -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.'
}
}
Loading

0 comments on commit 1e9086e

Please sign in to comment.