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

Initiative fails for built-in policies without parameters #104

Closed
3 tasks done
ashkurenpxl opened this issue Mar 6, 2024 · 5 comments · Fixed by #105
Closed
3 tasks done

Initiative fails for built-in policies without parameters #104

ashkurenpxl opened this issue Mar 6, 2024 · 5 comments · Fixed by #105

Comments

@ashkurenpxl
Copy link

Issue Template

Prerequisites

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

Context

Initiative creation is failing when some built-in policies are present in member definition list.

  • Module Version: 2.9.1
  • Terraform Version: 1.7.4
  • AzureRM Provider Version: 3.94.0
locals {
  policies = [
    "Audit VMs that do not use managed disks",
    "Monitor missing Endpoint Protection in Azure Security Center",
  ]
}

data "azurerm_policy_definition" "main" {
  for_each     = toset(local.policies)
  display_name = each.key
}

data "azurerm_management_group" "root" {
  name = "xxx"
}

module "initiative" {
  source                  = "gettek/policy-as-code/azurerm//modules/initiative"
  version                 = "2.9.1"
  initiative_name         = "test_initiative"
  initiative_display_name = "Test Initiative"
  initiative_description  = "Description"
  initiative_category     = "Security"
  initiative_version      = "1.0.0"
  management_group_id     = data.azurerm_management_group.root.id
  merge_effects           = false

  member_definitions = data.azurerm_policy_definition.main
}

Expected Behavior

Initiative is deployed without errors.

Current Behavior

Planning failed. Terraform encountered an error while generating this plan.

╷
│ Error: Error in function call
│
│   on .terraform/modules/initiative/modules/initiative/variables.tf line 89, in locals:
│   89:       parameters             = coalesce(null, jsondecode(d.parameters), null)
│     ├────────────────
│     │ d.parameters is ""
│
│ Call to function "jsondecode" failed: EOF.
╵

Possible Solution

Test d.parameters on empty string.

Failure Information (for bugs)

A policy has parameters value set to {}. For some reason, it gets translated to empty string in terraform.

Steps to Reproduce

  1. Create azurerm provider
  2. Copy the code above
  3. Fill in management group details
  4. Perform terraform plan
@judiethel
Copy link

judiethel commented Mar 6, 2024

Hi,
we discovered the same issues within our code.
In some built-in policies there are parameters = "" this leads to the error.

I have just checked the code and think that this line could solve our issues:
parameters = coalesce(null, try(jsondecode(d.parameters), {}), null)

Hoping for a quick fix. Thank you.

@ashkurenpxl
Copy link
Author

I would also argue the need of using coalesce with first and third parameters set to null.

@gettek
Copy link
Owner

gettek commented Mar 7, 2024

I would also argue the need of using coalesce with first and third parameters set to null.

hashicorp/terraform#25014 (comment)

@ashkurenpxl
Copy link
Author

I would also argue the need of using coalesce with first and third parameters set to null.

hashicorp/terraform#25014 (comment)

Thanks for the link. The described workaround is needed when one has multiple variables that need to be coalesced and can all be null. In the current case, we have only one variable jsondecode(d.parameters), what's the goal of using coalesce ?

@gettek
Copy link
Owner

gettek commented Mar 7, 2024

I would also argue the need of using coalesce with first and third parameters set to null.

hashicorp/terraform#25014 (comment)

Thanks for the link. The described workaround is needed when one has multiple variables that need to be coalesced and can all be null. In the current case, we have only one variable jsondecode(d.parameters), what's the goal of using coalesce ?

I've removed the coalesce as the formatting fixes released in 2.9.0 addressed previous issues around null attributes.

NOTE: if you want to mix both built-ins and customs in a set, you will need to concat them as in the example below. Unless you can share a better way;

data "azurerm_policy_definition" "tst" {
  for_each     = toset([
    "Audit VMs that do not use managed disks",
    "Monitor missing Endpoint Protection in Azure Security Center",
  ])
  display_name = each.key
}

module "initiative" {
  source                  = "..//modules/initiative"
  initiative_name         = "test_initiative"
  initiative_display_name = "Test Initiative"
  initiative_description  = "Description"
  initiative_category     = "Security"
  initiative_version      = "1.0.0"
  management_group_id     = data.azurerm_management_group.org.id
  merge_effects           = false

  member_definitions = concat(
    [for d in data.azurerm_policy_definition.tst: d],
    [
      module.configure_asc["auto_enroll_subscriptions"].definition,
      module.configure_asc["auto_provision_log_analytics_agent_custom_workspace"].definition,
    ]
  )
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants