Tanchwa commented 1 year ago

Issue Template

Prerequisites

[x ] I am running the latest version
[ x] I checked the documentation and found no answer
[x ] I checked to make sure that this issue has not already been filed

Context

Module Version: 2.8.2 Terraform v1.4.6 on darwin_amd64
provider registry.terraform.io/hashicorp/azuread v2.41.0
provider registry.terraform.io/hashicorp/azurerm v3.67.0
provider registry.terraform.io/hashicorp/local v2.4.0

{
    "description": "Customer-managed key based encryption should be configured for Databricks's managed services.",
    "displayName": "Audit - Databricks should use customer-managed key for encrypting managed services",
    "properties": {
        "metadata": {
            "category": "Azure Databricks",
            "version": "1.0.0"
        },
        "mode": "All",
        "policyRule": {
            "if": {
                "allOf": [
                    {
                        "equals": "Microsoft.Databricks/workspaces",
                        "field": "type"
                    },
                    {
                        "exists": false,
                        "field": "Microsoft.Databricks/workspaces/encryption.entities.managedServices.keySource"
                    }
                ]
            },
            "then": {
                "effect": "Audit"
            }
        }
    }
}

locals {
  # import the custom policy object from a library or specified file path
  policy_object = jsondecode(coalesce(try(
    file(var.file_path),
    file("${path.cwd}/Policiy Definitions/${title(var.policy_category)}/${var.policy_name}.json"),
    file("${path.root}/Policy Definitions/Custom Policies/${title(var.policy_category)}/${var.policy_name}.json"),
    file("${path.root}/../Policy Definitions/Custom Policies/${title(var.policy_category)}/${var.policy_name}.json"),
    file("${path.module}/../../Policy Definitions/Custom Policies/${title(var.policy_category)}/${var.policy_name}.json"),
    "{}" # return empty object if no policy is found
  )))

  # fallbacks
  title    = title(replace(local.policy_name, "/-|_|\\s/", " "))
  category = coalesce(var.policy_category, try((local.policy_object).properties.metadata.category, "General"))
  version  = coalesce(var.policy_version, try((local.policy_object).properties.metadata.version, "1.0.0"))
  mode     = coalesce(var.policy_mode, try((local.policy_object).properties.mode, "All"))

  # use local library attributes if runtime inputs are omitted
  policy_name  = coalesce(var.policy_name, try((local.policy_object).name, null))
  display_name = coalesce(var.display_name, try((local.policy_object).properties.displayName, local.title))
  description  = coalesce(var.policy_description, try((local.policy_object).properties.description, local.title))
  metadata     = coalesce(var.policy_metadata, try((local.policy_object).properties.metadata, merge({ category = local.category }, { version = local.version })))
  parameters   = coalesce(var.policy_parameters, try((local.policy_object).properties.parameters, null))
  policy_rule  = coalesce(var.policy_rule, try((local.policy_object).properties.policyRule, null))

  # manually generate the definition Id to prevent "Invalid for_each argument" on set_assignment plan/apply
  definition_id = var.management_group_id != null ? "${var.management_group_id}/providers/Microsoft.Authorization/policyDefinitions/${local.policy_name}" : azurerm_policy_definition.def.id
}

Expected Behavior

Module should be able to handle null values for certain fields in policy JSON Ex: not all policies have a parameters attribute, the module should not break if this is missing.

Current Behavior

policy definition module variables.tf currently does not have a way to properly handle null values (coalesce errors out on null) if you introduce a try block that results in null around the parameters locals value try(coalesce(var.policy_parameters, try((local.policy_object).properties.parameters, null)), null) , it results in a further error that APPEARS to attempt to allow null values, but is not functioning as expected ╷ │ Error: Invalid function argument │ │ on Modules/Policy/main.tf line 11, in resource "azurerm_policy_definition" "def": │ 11: parameters = length(local.parameters) > 0 ? jsonencode(local.parameters) : null │ ├──────────────── │ │ while calling length(value) │ │ local.parameters is null │ │ Invalid value for "value" parameter: argument must not be null.

Possible Solution

introduce the necessary null value handlers throughout the code to allow for null values, especially in the parameters field.

Failure Information (for bugs)

Steps to Reproduce

use policy definition module call on various policies with undefined attributes
terraform apply

Failure Logs

│ Error: Error in function call │ │ on Modules/Policy/variables.tf line 109, in locals: │ 109: parameters = coalesce(var.policy_parameters, try((local.policy_object).properties.parameters, null)) │ ├──────────────── │ │ while calling coalesce(vals...) │ │ local.policy_object is object with 3 attributes │ │ var.policy_parameters is null │ │ Call to function "coalesce" failed: no non-null, non-empty-string arguments.

│ Error: Invalid function argument │ │ on Modules/Policy/main.tf line 11, in resource "azurerm_policy_definition" "def": │ 11: parameters = length(local.parameters) > 0 ? jsonencode(local.parameters) : null │ ├──────────────── │ │ while calling length(value) │ │ local.parameters is null │ │ Invalid value for "value" parameter: argument must not be null. ╵

gettek commented 1 year ago

Hi @Tanchwa, please try updating the locals validation (variables.tf line 109) to the below and try again, let me know if this helps and I'll fix in the next release:

locals {
   parameters   = coalesce(var.policy_parameters, try((local.policy_object).properties.parameters, {}))
}