Skip to content

Commit

Permalink
Fixes for policy function handling Azure#1654 Azure#1323
Browse files Browse the repository at this point in the history
  • Loading branch information
BernieWhite committed Nov 23, 2022
1 parent b5f56e1 commit 9841dc0
Show file tree
Hide file tree
Showing 10 changed files with 320 additions and 21 deletions.
6 changes: 6 additions & 0 deletions docs/CHANGELOG-v1.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ See [upgrade notes][1] for helpful information when upgrading from previous vers

## Unreleased

What's changed since pre-release v1.22.0-B0062:

- New rules:
- API Management:
- Check API management instances uses multi-region deployment by @BenjaminEngeset.
Expand All @@ -41,6 +43,10 @@ See [upgrade notes][1] for helpful information when upgrading from previous vers
[#1876](https://github.com/Azure/PSRule.Rules.Azure/issues/1876)
- Fixed an item with the same key for parameters by @BernieWhite
[#1871](https://github.com/Azure/PSRule.Rules.Azure/issues/1871)
- Fixed policy parse of `requestContext` function by @BernieWhite.
[#1654](https://github.com/Azure/PSRule.Rules.Azure/issues/1654)
- Fixed handling of policy type field by @BernieWhite.
[#1323](https://github.com/Azure/PSRule.Rules.Azure/issues/1323)

## v1.22.0-B0062 (pre-release)

Expand Down
4 changes: 4 additions & 0 deletions src/PSRule.Rules.Azure/Common/StringExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ internal static string SplitLastSegment(this string str, char c)
return str.Substring(lastSubstringIndex + 1);
}

/// <summary>
/// Check if the string is an Azure Resource Manager expression.
/// Expression use the <c>[function()]</c> syntax.
/// </summary>
internal static bool IsExpressionString(this string str)
{
return str != null &&
Expand Down
83 changes: 73 additions & 10 deletions src/PSRule.Rules.Azure/Data/Policy/PolicyAssignmentVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ namespace PSRule.Rules.Azure.Data.Policy
/// <summary>
/// This visitor processes each assignment to convert the assignment in to one or mny rules.
/// </summary>
internal abstract class PolicyAssignmentVisitor
internal abstract class PolicyAssignmentVisitor : ResourceManagerVisitor
{
private const string PROPERTY_POLICYASSIGNMENTID = "policyAssignmentId";
private const string PROPERTY_PARAMETERS = "parameters";
Expand All @@ -39,8 +39,8 @@ internal abstract class PolicyAssignmentVisitor
private const string PROPERTY_TYPE = "type";
private const string PROPERTY_NAME = "name";
private const string PROPERTY_DEFAULTVALUE = "defaultValue";
private const string PROPERTY_ALL_OF = "allOf";
private const string PROPERTY_ANY_OF = "anyOf";
private const string PROPERTY_ALLOF = "allOf";
private const string PROPERTY_ANYOF = "anyOf";
private const string PROPERTY_EQUALS = "equals";
private const string FIELD_NOTEQUALS = "notEquals";
private const string FIELD_GREATER = "greater";
Expand Down Expand Up @@ -102,7 +102,7 @@ public sealed class PolicyAssignmentContext : ITemplateContext

internal PolicyAssignmentContext(PipelineContext context)
{
_ExpressionFactory = new ExpressionFactory();
_ExpressionFactory = new ExpressionFactory(policy: true);
_ExpressionBuilder = new ExpressionBuilder(_ExpressionFactory);
_PolicyAliasProviderHelper = new PolicyAliasProviderHelper();
_Definitions = new List<PolicyDefinition>();
Expand Down Expand Up @@ -322,15 +322,15 @@ private void ExpressionToObjectPathArrayFilter(JArray expression, string clause,
clauseSeparator = $" {clause} ";
}

else if (obj.TryArrayProperty(PROPERTY_ALL_OF, out var allOfExpression))
else if (obj.TryArrayProperty(PROPERTY_ALLOF, out var allOfExpression))
{
objectPath.Append($" {clause} ");
objectPath.Append(GROUP_OPEN);
ExpressionToObjectPathArrayFilter(allOfExpression, AND_CLAUSE, objectPath);
objectPath.Append(GROUP_CLOSE);
}

else if (obj.TryArrayProperty(PROPERTY_ANY_OF, out var anyOfExpression))
else if (obj.TryArrayProperty(PROPERTY_ANYOF, out var anyOfExpression))
{
objectPath.Append($" {clause} ");
objectPath.Append(GROUP_OPEN);
Expand Down Expand Up @@ -450,7 +450,7 @@ internal void ExpandPolicyRule(JToken policyRule, IList<string> types)
}

// nested allOf in where expression
else if (whereExpression.TryArrayProperty(PROPERTY_ALL_OF, out var allofExpression))
else if (whereExpression.TryArrayProperty(PROPERTY_ALLOF, out var allofExpression))
{
var splitAliasPath = outerFieldAliasPath.SplitByLastSubstring(COLLECTION_ALIAS);
var filter = new StringBuilder();
Expand All @@ -459,7 +459,7 @@ internal void ExpandPolicyRule(JToken policyRule, IList<string> types)
}

// nested anyOf in where expression
else if (whereExpression.TryArrayProperty(PROPERTY_ANY_OF, out var anyOfExpression))
else if (whereExpression.TryArrayProperty(PROPERTY_ANYOF, out var anyOfExpression))
{
var splitAliasPath = outerFieldAliasPath.SplitByLastSubstring(COLLECTION_ALIAS);
var filter = new StringBuilder();
Expand Down Expand Up @@ -812,6 +812,8 @@ protected virtual bool TryPolicyDefinition(PolicyAssignmentContext context, JObj

// Modify policy rule
TrimPolicyRule(policyRule);
VisitPolicyRule(context, policyRule);

context.ExpandPolicyRule(policyRule, result.Types);
if (!TryPolicyRuleEffect(then, out var effect) ||
ShouldFilterRule(then, effect))
Expand All @@ -833,6 +835,67 @@ protected virtual bool TryPolicyDefinition(PolicyAssignmentContext context, JObj
return true;
}

/// <summary>
/// Visit the policyRule node.
/// </summary>
private static void VisitPolicyRule(PolicyAssignmentContext context, JObject policyRule)
{
if (policyRule.TryObjectProperty(PROPERTY_IF, out var condition))
VisitCondition(context, condition);
}

/// <summary>
/// Visit a policy condition node.
/// </summary>
private static void VisitCondition(PolicyAssignmentContext context, JObject condition)
{
if (condition.TryArrayProperty(PROPERTY_ALLOF, out var allOf))
{
foreach (var item in allOf.Values<JObject>())
{
VisitCondition(context, item);
}
}
else if (condition.TryArrayProperty(PROPERTY_ANYOF, out var anyOf))
{
foreach (var item in anyOf.Values<JObject>())
{
VisitCondition(context, item);
}
}
else
{
if (condition.TryStringProperty(PROPERTY_VALUE, out var s) && s.IsExpressionString())
{
VisitValueExpression(context, condition, s);
}
}
}

private static void VisitValueExpression(PolicyAssignmentContext context, JObject condition, string s)
{
var tokens = ExpressionParser.Parse(s);

// Handle [requestContext().apiVersion]
if (tokens.ConsumeFunction("requestContext") &&
tokens.ConsumeGroup() &&
tokens.ConsumePropertyName("apiVersion"))
{
condition.Remove(PROPERTY_VALUE);
condition.Add(PROPERTY_FIELD, "apiVersion");
}

// Handle "[field('type')]
else if (tokens.ConsumeFunction("field") &&
tokens.TryTokenType(ExpressionTokenType.GroupStart, out _) &&
tokens.ConsumeString("type") &&
tokens.TryTokenType(ExpressionTokenType.GroupEnd, out _))
{
condition.Remove(PROPERTY_VALUE);
condition.Add(PROPERTY_TYPE, DOT);
}
}

/// <summary>
/// Get hash of definitionID + condition + pre-condition
/// </summary>
Expand Down Expand Up @@ -934,7 +997,7 @@ private static JObject AndExistanceExpression(JObject details, JObject subselect
var existanceExpression = new JObject
{
{ PROPERTY_FIELD, PROPERTY_RESOURCES },
{ PROPERTY_ALL_OF, allOf }
{ PROPERTY_ALLOF, allOf }
};
if (subselector != null && subselector.Count > 0)
existanceExpression[PROPERTY_WHERE] = subselector;
Expand Down Expand Up @@ -965,7 +1028,7 @@ private static JObject AndCondition(JObject left, JObject right)
};
return new JObject
{
{ PROPERTY_ALL_OF, allOf }
{ PROPERTY_ALLOF, allOf }
};
}
return left == null || left.Count == 0 ? right : left;
Expand Down
12 changes: 12 additions & 0 deletions src/PSRule.Rules.Azure/Data/ResourceManagerVisitor.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

namespace PSRule.Rules.Azure.Data
{
/// <summary>
/// A base class for all Azure Resource Manager visitors that implement common methods.
/// </summary>
internal abstract class ResourceManagerVisitor
{
}
}
11 changes: 9 additions & 2 deletions src/PSRule.Rules.Azure/Data/Template/ExpressionBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -222,11 +222,18 @@ internal sealed class ExpressionFactory
{
private readonly Dictionary<string, IFunctionDescriptor> _Descriptors;

public ExpressionFactory()
public ExpressionFactory(bool policy = false)
{
_Descriptors = new Dictionary<string, IFunctionDescriptor>(StringComparer.OrdinalIgnoreCase);
foreach (var d in Functions.Builtin)
foreach (var d in Functions.Common)
With(d);

// Azure policy specific functions should be added
if (policy)
{
foreach (var d in Functions.Policy)
With(d);
}
}

public bool TryDescriptor(string name, out IFunctionDescriptor descriptor)
Expand Down
14 changes: 13 additions & 1 deletion src/PSRule.Rules.Azure/Data/Template/Functions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ internal static class Functions
private const char SINGLE_QUOTE = '\'';
private const char DOUBLE_QUOTE = '"';

internal readonly static IFunctionDescriptor[] Builtin = new IFunctionDescriptor[]
internal readonly static IFunctionDescriptor[] Common = new IFunctionDescriptor[]
{
// Array and object
new FunctionDescriptor("array", Array),
Expand Down Expand Up @@ -154,6 +154,18 @@ internal static class Functions
new FunctionDescriptor("uriComponentToString", UriComponentToString),
};

/// <summary>
/// Functions specific to Azure Policy.
/// See <seealso href="https://learn.microsoft.com/azure/governance/policy/concepts/definition-structure#policy-functions">Policy Functions</seealso>.
/// </summary>
internal readonly static IFunctionDescriptor[] Policy = new IFunctionDescriptor[]
{
//new FunctionDescriptor("addDays", AddDays),
//new FunctionDescriptor("field", Field),
//new FunctionDescriptor("requestContext", RequestContext),
//new FunctionDescriptor("policy", Policy),
};

private static readonly CultureInfo AzureCulture = new("en-US");

#region Array and object
Expand Down
5 changes: 4 additions & 1 deletion src/PSRule.Rules.Azure/Data/Template/TemplateVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@

namespace PSRule.Rules.Azure.Data.Template
{
/// <summary>
/// A string expression.
/// </summary>
public delegate T StringExpression<T>();

internal interface IValidationContext
Expand Down Expand Up @@ -60,7 +63,7 @@ internal interface ITemplateContext : IValidationContext
/// <summary>
/// The base class for a template visitor.
/// </summary>
internal abstract class TemplateVisitor
internal abstract class TemplateVisitor : ResourceManagerVisitor
{
private const string RESOURCETYPE_DEPLOYMENT = "Microsoft.Resources/deployments";
private const string DEPLOYMENTSCOPE_INNER = "inner";
Expand Down
64 changes: 59 additions & 5 deletions src/PSRule.Rules.Azure/Data/Template/TokenStream.cs
Original file line number Diff line number Diff line change
@@ -1,15 +1,22 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Text;

namespace PSRule.Rules.Azure.Data.Template
{
/// <summary>
/// The available token types used for Azure Resource Manager expressions.
/// </summary>
[System.Diagnostics.CodeAnalysis.SuppressMessage("Naming", "CA1720:Identifier contains type name", Justification = "Represents standard type.")]
public enum ExpressionTokenType : byte
{
/// <summary>
/// Null token.
/// </summary>
None,

/// <summary>
Expand All @@ -18,7 +25,7 @@ public enum ExpressionTokenType : byte
Element,

/// <summary>
/// A property '.property_name'.
/// A property <c>.property_name</c>.
/// </summary>
Property,

Expand All @@ -33,22 +40,22 @@ public enum ExpressionTokenType : byte
Numeric,

/// <summary>
/// Start a grouping '('.
/// Start a grouping <c>'('</c>.
/// </summary>
GroupStart,

/// <summary>
/// End a grouping ')'.
/// End a grouping <c>')'</c>.
/// </summary>
GroupEnd,

/// <summary>
/// Start an index '['.
/// Start an index <c>'['</c>.
/// </summary>
IndexStart,

/// <summary>
/// End an index ']'.
/// End an index <c>']'</c>.
/// </summary>
IndexEnd
}
Expand Down Expand Up @@ -120,6 +127,53 @@ internal static void IndexEnd(this TokenStream stream)
{
stream.Add(new ExpressionToken(ExpressionTokenType.IndexEnd, null));
}

internal static bool ConsumeFunction(this TokenStream stream, string name)
{
if (stream == null ||
stream.Count == 0 ||
stream.Current.Type != ExpressionTokenType.Element ||
!string.Equals(stream.Current.Content, name, StringComparison.OrdinalIgnoreCase))
return false;

stream.Pop();
return true;
}

internal static bool ConsumePropertyName(this TokenStream stream, string propertyName)
{
if (stream == null ||
stream.Count == 0 ||
stream.Current.Type != ExpressionTokenType.Property ||
!string.Equals(stream.Current.Content, propertyName, StringComparison.OrdinalIgnoreCase))
return false;

stream.Pop();
return true;
}

internal static bool ConsumeString(this TokenStream stream, string s)
{
if (stream == null ||
stream.Count == 0 ||
stream.Current.Type != ExpressionTokenType.Property ||
!string.Equals(stream.Current.Content, s, StringComparison.OrdinalIgnoreCase))
return false;

stream.Pop();
return true;
}

internal static bool ConsumeGroup(this TokenStream stream)
{
if (stream == null ||
stream.Count == 0 ||
stream.Current.Type != ExpressionTokenType.GroupStart)
return false;

stream.SkipGroup();
return true;
}
}

/// <summary>
Expand Down
Loading

0 comments on commit 9841dc0

Please sign in to comment.