Fixed parent not exists effect #2608 (#2685)

Azure · Feb 13, 2024 · cf5b5b9 · cf5b5b9
1 parent dfbad68
commit cf5b5b9
Show file tree

Hide file tree

Showing 5 changed files with 171 additions and 15 deletions.
diff --git a/docs/CHANGELOG-v1.md b/docs/CHANGELOG-v1.md
@@ -37,6 +37,8 @@ What's changed since v1.33.0:
 - Bug fixes:
   - Fixed `Azure.AKS.AuthorizedIPs` is not valid for a private cluster by @BernieWhite.
     [#2677](https://github.com/Azure/PSRule.Rules.Azure/issues/2677)
+  - Fixed generating rule for VM extensions from policy is incorrect by @BernieWhite.
+    [#2608](https://github.com/Azure/PSRule.Rules.Azure/issues/2608)
 
 ## v1.33.0
 

diff --git a/src/PSRule.Rules.Azure/Data/Policy/PolicyAssignmentVisitor.cs b/src/PSRule.Rules.Azure/Data/Policy/PolicyAssignmentVisitor.cs
@@ -15,7 +15,6 @@
 using PSRule.Rules.Azure.Data.Template;
 using PSRule.Rules.Azure.Pipeline;
 using PSRule.Rules.Azure.Resources;
-using YamlDotNet.Core.Tokens;
 
 namespace PSRule.Rules.Azure.Data.Policy
 {
@@ -1471,9 +1470,22 @@ private static void EffectConditions(PolicyAssignmentContext context, PolicyDefi
                 !then.TryObjectProperty(PROPERTY_DETAILS, out var details))
                 return;
 
-            if (IsIfNotExistsEffect(context, then))
+            // Handle not exist effect for a parent type.
+            if (IsIfNotExistsEffect(context, then) && details.TryStringProperty(PROPERTY_TYPE, out var type) && ChildOf(policyDefinition, type))
+            {
+                // Replace with parent type.
+                policyDefinition.Types.Clear();
+                policyDefinition.Types.Add(type);
+                context.SetDefaultResourceType(type);
+
+                // Clean up condition that applies to child.
+                policyDefinition.Where = null;
+
+                policyDefinition.Condition = AndExistanceExpression(context, details, null, applyToChildren: false);
+            }
+            // Handle not exist effect.
+            else if (IsIfNotExistsEffect(context, then))
             {
-                //policyDefinition.Where = policyDefinition.Condition;
                 policyDefinition.Condition = AndExistanceExpression(context, details, DefaultEffectConditions(context, details));
             }
             else
@@ -1482,6 +1494,15 @@ private static void EffectConditions(PolicyAssignmentContext context, PolicyDefi
             }
         }
 
+        private static bool ChildOf(PolicyDefinition policyDefinition, string type)
+        {
+            if (policyDefinition == null || policyDefinition.Types == null || policyDefinition.Types.Count != 1 || string.IsNullOrEmpty(type))
+                return false;
+
+            var currentType = policyDefinition.Types[0];
+            return type.Length < currentType.Length && currentType.StartsWith($"{type}/", StringComparison.OrdinalIgnoreCase);
+        }
+
         private static void CompleteCondition(PolicyAssignmentContext context, PolicyDefinition policyDefinition, string effect)
         {
             if (policyDefinition.Condition != null)
@@ -1555,7 +1576,7 @@ private static JObject TypeExpression(PolicyAssignmentContext context, JObject d
             };
         }
 
-        private static JObject AndExistanceExpression(PolicyAssignmentContext context, JObject details, JObject subselector)
+        private static JObject AndExistanceExpression(PolicyAssignmentContext context, JObject details, JObject subselector, bool applyToChildren = true)
         {
             if (details == null || !details.TryObjectProperty(PROPERTY_EXISTENCECONDITION, out var existenceCondition))
             {
@@ -1568,19 +1589,24 @@ private static JObject AndExistanceExpression(PolicyAssignmentContext context, J
 
             VisitCondition(context, null, existenceCondition);
 
-            var allOf = new JArray
-            {
-                existenceCondition
-            };
-            var existenceExpression = new JObject
+            if (applyToChildren)
             {
-                { PROPERTY_FIELD, PROPERTY_RESOURCES },
-                { PROPERTY_ALLOF, allOf }
-            };
-            if (subselector != null && subselector.Count > 0)
-                existenceExpression[PROPERTY_WHERE] = subselector;
+                var allOf = new JArray
+                {
+                    existenceCondition
+                };
+                var existenceExpression = new JObject
+                {
+                    { PROPERTY_FIELD, PROPERTY_RESOURCES },
+                    { PROPERTY_ALLOF, allOf }
+                };
 
-            return existenceExpression;
+                if (subselector != null && subselector.Count > 0)
+                    existenceExpression[PROPERTY_WHERE] = subselector;
+
+                existenceCondition = existenceExpression;
+            }
+            return existenceCondition;
         }
 
         private static JObject AndNameCondition(JObject details, JObject condition)

diff --git a/tests/PSRule.Rules.Azure.Tests/PSRule.Rules.Azure.Tests.csproj b/tests/PSRule.Rules.Azure.Tests/PSRule.Rules.Azure.Tests.csproj
@@ -38,6 +38,9 @@
     <None Update="Policy.assignment.3.json">
       <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
     </None>
+    <None Update="Policy.assignment.4.json">
+      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
+    </None>
     <None Update="Policy.assignment.json">
       <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
     </None>

diff --git a/tests/PSRule.Rules.Azure.Tests/Policy.assignment.4.json b/tests/PSRule.Rules.Azure.Tests/Policy.assignment.4.json
@@ -0,0 +1,101 @@
+[
+  {
+    "Identity": null,
+    "Location": null,
+    "Name": "assignment.4",
+    "ResourceId": "/subscriptions/00000000-0000-0000-0000-000000000000/providers/Microsoft.Authorization/policyAssignments/assignment.4",
+    "ResourceName": "assignment.4",
+    "ResourceGroupName": null,
+    "ResourceType": "Microsoft.Authorization/policyAssignments",
+    "SubscriptionId": "00000000-0000-0000-0000-000000000000",
+    "Sku": null,
+    "PolicyAssignmentId": "/subscriptions/00000000-0000-0000-0000-000000000000/providers/Microsoft.Authorization/policyAssignments/assignment.4",
+    "Properties": {
+      "Scope": "/subscriptions/00000000-0000-0000-0000-000000000000",
+      "NotScopes": [],
+      "DisplayName": "Virtual machines' Guest Configuration extension should be deployed with system-assigned managed identity",
+      "Description": null,
+      "Metadata": {
+        "assignedBy": "",
+        "parameterScopes": {},
+        "createdBy": "",
+        "createdOn": "",
+        "updatedBy": null,
+        "updatedOn": null
+      },
+      "EnforcementMode": 1,
+      "PolicyDefinitionId": "/providers/Microsoft.Authorization/policyDefinitions/d26f7642-7545-4e18-9b75-8c9bbdee3a9a",
+      "Parameters": {
+        "tagName": {
+          "value": "env"
+        }
+      },
+      "NonComplianceMessages": [
+        {
+          "Message": "Virtual machines' Guest Configuration extension should be deployed with system-assigned managed identity",
+          "PolicyDefinitionReferenceId": null
+        }
+      ]
+    },
+    "PolicyDefinitions": [
+      {
+        "Name": "d26f7642-7545-4e18-9b75-8c9bbdee3a9a",
+        "ResourceId": "/providers/Microsoft.Authorization/policyDefinitions/d26f7642-7545-4e18-9b75-8c9bbdee3a9a",
+        "ResourceName": "d26f7642-7545-4e18-9b75-8c9bbdee3a9a",
+        "ResourceType": "Microsoft.Authorization/policyDefinitions",
+        "SubscriptionId": null,
+        "Properties": {
+          "Description": "The Guest Configuration extension requires a system assigned managed identity. Azure virtual machines in the scope of this policy will be non-compliant when they have the Guest Configuration extension installed but do not have a system assigned managed identity. Learn more at https://aka.ms/gcpol",
+          "DisplayName": "Virtual machines' Guest Configuration extension should be deployed with system-assigned managed identity",
+          "Metadata": {
+            "version": "1.0.1",
+            "category": "Security Center"
+          },
+          "Mode": "Indexed",
+          "Parameters": {
+            "effect": {
+              "type": "String",
+              "metadata": {
+                "displayName": "Effect",
+                "description": "Enable or disable the execution of the policy"
+              },
+              "allowedValues": [
+                "AuditIfNotExists",
+                "Disabled"
+              ],
+              "defaultValue": "AuditIfNotExists"
+            }
+          },
+          "PolicyRule": {
+            "if": {
+              "allOf": [
+                {
+                  "field": "type",
+                  "equals": "Microsoft.Compute/virtualMachines/extensions"
+                },
+                {
+                  "field": "Microsoft.Compute/virtualMachines/extensions/publisher",
+                  "equals": "Microsoft.GuestConfiguration"
+                }
+              ]
+            },
+            "then": {
+              "effect": "[parameters('effect')]",
+              "details": {
+                "type": "Microsoft.Compute/virtualMachines",
+                "name": "[first(split(field('fullName'), '/'))]",
+                "existenceCondition": {
+                  "field": "identity.type",
+                  "contains": "SystemAssigned"
+                }
+              }
+            }
+          },
+          "PolicyType": 2
+        },
+        "PolicyDefinitionId": "/providers/Microsoft.Authorization/policyDefinitions/d26f7642-7545-4e18-9b75-8c9bbdee3a9a"
+      }
+    ],
+    "exemptions": []
+  }
+]
diff --git a/tests/PSRule.Rules.Azure.Tests/PolicyAssignmentVisitorTests.cs b/tests/PSRule.Rules.Azure.Tests/PolicyAssignmentVisitorTests.cs
@@ -204,6 +204,30 @@ public void GetPolicyBaseline()
             Assert.Equal(new string[] { "Azure.Policy.b8a4e2d03e09", "PSRule.Rules.Azure\\Azure.KeyVault.SoftDelete" }, baseline.Include);
         }
 
+        /// <summary>
+        /// This tests for a reverse cases where the child extension is audit if a setting on the parent resource is set.
+        /// </summary>
+        [Fact]
+        public void GetParentAudit()
+        {
+            var context = new PolicyAssignmentContext(GetContext());
+            var visitor = new PolicyAssignmentDataExportVisitor();
+            foreach (var assignment in GetAssignmentData("Policy.assignment.4.json").Where(a => a["Name"].Value<string>() == "assignment.4"))
+                visitor.Visit(context, assignment);
+
+            var definitions = context.GetDefinitions();
+            Assert.NotNull(definitions);
+            Assert.Single(definitions);
+
+            var actual = definitions.FirstOrDefault(definition => definition.DefinitionId == "/providers/Microsoft.Authorization/policyDefinitions/d26f7642-7545-4e18-9b75-8c9bbdee3a9a");
+            Assert.NotNull(actual);
+            Assert.Single(actual.Types);
+            Assert.Equal("Microsoft.Compute/virtualMachines", actual.Types[0]);
+            Assert.Null(actual.Where);
+            Assert.Equal("{\"field\":\"identity.type\",\"contains\":\"SystemAssigned\"}", actual.Condition.ToString(Formatting.None));
+            Assert.Equal(new string[] { "PSRule.Rules.Azure\\Azure.Policy.Indexed" }, actual.With);
+        }
+
         #region Helper methods
 
         private static PipelineContext GetContext(PSRuleOption option = null)