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

Convert-PolicyResourcesToDetails in parallel does not throw if it fails #777

Open
o-l-a-v opened this issue Oct 15, 2024 · 9 comments
Open
Assignees

Comments

@o-l-a-v
Copy link
Contributor

o-l-a-v commented Oct 15, 2024

Describe the bug

Convert-PolicyResourcesToDetails does not throw if it fails when running in parallel.

Snippet from our CI/CD run:

Image

To Reproduce

Have a faulty policyDefinition matching the fail that was thrown in the screenshot?

Expected behavior

If ANY of those parallel jobs have ANY failure the function should throw.

Screenshots

Already added to "Describe the bug".

EPAC Version

10.6.2

@o-l-a-v o-l-a-v added the bug Something isn't working label Oct 15, 2024
@o-l-a-v
Copy link
Contributor Author

o-l-a-v commented Oct 15, 2024

An obeservation: The starter kit does not set $ErrorActionPreference = Stop when doing Build-DeploymentPlans, and neither do we.

Default error action in PowerShell is to continue on error.

This might be related? And shouldn't we expect the plan to fail if Convert-PolicyResourcesToDetails has >= 0 failures?

@anwather
Copy link
Collaborator

Possibly it should fail - however that code block shouldn't be possible to reach so I'm curious what did the faulty policy look like? I don't want to go down the path of running tests on all the policy objects before they are deployed at this stage.,

@anwather anwather self-assigned this Oct 15, 2024
@o-l-a-v
Copy link
Contributor Author

o-l-a-v commented Oct 16, 2024

Here is the faulty policyDefinition:

{
  "$schema": "https://raw.githubusercontent.com/Azure/enterprise-azure-policy-as-code/main/Schemas/policy-definition-schema.json",
  "name": "3b98831c-f754-4ce7-bb79-b88f99ed9f2c",
  "properties": {
    "displayName": "Inherit a tag from the subscription",
    "mode": "All",
    "description": "Adds or replaces the specified tag and value from the containing subscription when any resource is created or updated. Existing resources can be remediated by triggering a remediation task.",
    "metadata": {
      "category": "Tags",
      "version": "1.0.0"
    },
    "version": "1.0.0",
    "parameters": {
      "tagName": {
        "type": "String",
        "metadata": {
          "displayName": "Tag Name",
          "description": "Name of the tag, such as 'environment'"
        }
      }
    },
    "policyRule": {
      "if": {
        "allOf": [
          {
            "field": "[concat('tags[', parameters('tagName'), ']')]",
            "notEquals": "[subscription().tags[parameters('tagName')]]"
          },
          {
            "value": "[subscription().tags[parameters('tagName')]]",
            "notEquals": ""
          }
        ]
      },
      "then": {
        "effect": "[parameters('effect')]",
        "details": {
          "roleDefinitionIds": [
            "/providers/microsoft.authorization/roleDefinitions/b24988ac-6180-42a0-ab88-20f7382dd24c"
          ],
          "operations": [
            {
              "operation": "addOrReplace",
              "field": "[concat('tags[', parameters('tagName'), ']')]",
              "value": "[subscription().tags[parameters('tagName')]]"
            }
          ]
        }
      }
    },
    "versions": [
      "1.0.0"
    ]
  },
  "id": "/providers/Microsoft.Authorization/policyDefinitions/b27a0cbd-a167-4dfa-ae64-4337be671140",
  "type": "Microsoft.Authorization/policyDefinitions"
}

Parameter "effect" was wrongfully removed. And id and type at the bottom was not removed.

Seems EPAC does not correctly validate schema either? There is no "additionalProperties": false in the schema:

@anwather
Copy link
Collaborator

It may be worth investigating the AzPolicyTest powershell module to perform this testing. The repo can be forked and updated to include tests such as check that parameters specified in the policy rules are also specified in the parameters section. The purpose of the schema in EPAC was simply to provide Intellisense to assist with authoring in VS Code - not to provide strict validation.

Accompanying blog for AzPolicyTest -> https://blog.tyang.org/2024/03/08/azpolicytest-v2-release

@o-l-a-v
Copy link
Contributor Author

o-l-a-v commented Oct 17, 2024

Is there a way to get EPAC to "compile" / build EPAC policyDefinitions and policySetDefinitions to "full fat" ARM templates, which then can be linted / tested by AzPolicyTest?

From initial testing, AzPolicyTest would require a lot of changes. But making EPAC output testable JSON files might be less work?

@anwather
Copy link
Collaborator

So I tested using the on a policy definition - it didn't need to be in the full ARM template version for it to work. If it was me looking to this as I said I would fork that repo and expand on the tests - or help contribute to the original project (which I've see you've already done :) )

@anwather
Copy link
Collaborator

My testing:

{
  "$schema": "https://raw.githubusercontent.com/Azure/enterprise-azure-policy-as-code/main/Schemas/policy-definition-schema.json",
  "name": "rg-require-tag",
  "properties": {
    "displayName": "Custom - Require a tag on resource groups",
    "description": "Enforces existence of a tag on resource groups.",
    "mode": "All",
    "metadata": {
      "category": "Tags",
      "version": "1.0.0"
    },
    "parameters": {
      "effect": {
        "metadata": {
          "displayName": "Effect",
          "description": "Set to Audit or Deny"
        },
        "allowedValues": [
          "Audit",
          "Deny",
          "Disabled"
        ],
        "type": "String",
        "defaultValue": "Deny"
      },
      "tagName": {
        "metadata": {
          "displayName": "Tag Name",
          "description": "Name of the tag, such as 'environment'"
        },
        "type": "String"
      }
    },
    "policyRule": {
      "if": {
        "allOf": [
          {
            "field": "type",
            "equals": "Microsoft.Resources/subscriptions/resourceGroups"
          },
          {
            "field": "[concat('tags[', parameters('tagName'), ']')]",
            "exists": "false"
          }
        ]
      },
      "then": {
        "effect": "[parameters('effect')]"
      }
    }
  }
}

Output:
Image

I didn't adjust the file at all (except rename to .json from .jsonc

@o-l-a-v
Copy link
Contributor Author

o-l-a-v commented Oct 18, 2024

Nice, you're right @anwather. 😊 I made a PR to add JSONC support.

@anwather
Copy link
Collaborator

anwather commented Oct 22, 2024

As discussed, this is caused by invalid policy definitions which can be discovered using AzPolicyTest. I'm going to mark this as an enhancement to be completed at a later date as these errors tend to be surfaced during a deploy stage anyway.

@anwather anwather added enhancement - future and removed bug Something isn't working labels Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants