Repository: salesforce/cloud-guardrails (language: python)

Issue:
Ensure Idempotence in Terraform output for optional params and required params 



Discussion / hints:

Fix patch:
diff --git a/azure_guardrails/command/generate_terraform.py b/azure_guardrails/command/generate_terraform.py
index 719b5fb..ace9f05 100644
--- a/azure_guardrails/command/generate_terraform.py
+++ b/azure_guardrails/command/generate_terraform.py
@@ -144,13 +144,13 @@ def generate_terraform(
     else:
         subscription = ""
 
-    summary_file_prefix = ""
+    parameter_requirement_str = ""
     if no_params:
-        summary_file_prefix = "no-params"
+        parameter_requirement_str = "no-params"
     elif params_required:
-        summary_file_prefix = "params-required"
+        parameter_requirement_str = "params-required"
     elif params_optional:
-        summary_file_prefix = "params-optional"
+        parameter_requirement_str = "params-optional"
 
     if service == "all":
         services = Services(config=config)
@@ -171,6 +171,7 @@ def generate_terraform(
         )
 
         terraform_template = TerraformTemplateWithParams(
+            parameter_requirement_str=parameter_requirement_str,
             parameters=display_names,
             subscription_name=subscription,
             management_group=management_group,
@@ -193,14 +194,14 @@ def markdown_summary(file_prefix: str) -> str:
                 f.write(markdown_table)
             return markdown_file_name
 
-        summary_file_prefix = f"{summary_file_prefix}-{service}-table"
+        parameter_requirement_str = f"{parameter_requirement_str}-{service}-table"
 
         # Write Markdown summary
-        markdown_file = markdown_summary(file_prefix=summary_file_prefix)
+        markdown_file = markdown_summary(file_prefix=parameter_requirement_str)
 
         if verbosity >= 1:
             utils.print_grey(f"Markdown file written to: {markdown_file}")
 
         # Write CSV summary
-        csv_file = f"{summary_file_prefix}.csv"
+        csv_file = f"{parameter_requirement_str}.csv"
         services.csv_summary(csv_file, verbosity=verbosity, no_params=no_params, params_optional=params_optional, params_required=params_required)
diff --git a/azure_guardrails/terraform/parameters/policy-set-with-parameters.tf b/azure_guardrails/terraform/parameters/policy-set-with-parameters.tf
index d55293c..ef311e2 100644
--- a/azure_guardrails/terraform/parameters/policy-set-with-parameters.tf
+++ b/azure_guardrails/terraform/parameters/policy-set-with-parameters.tf
@@ -1,14 +1,9 @@
-variable "name" { default = "{{ t.name }}" }
-variable "subscription_name" { default = "{{ t.subscription_name }}" }
-variable "management_group" { default = "{{ t.management_group }}" }
-variable "enforcement_mode" { default = {{ t.enforcement_mode }} }
-
-variable "category" {
-  type    = string
-  default = "Testing"
-}
-
 locals {
+  name_{{ t.name }} = "{{ t.name }}"
+  subscription_name_{{ t.name }} = "{{ t.subscription_name }}"
+  management_group_{{ t.name }} = "{{ t.management_group }}"
+  category_{{ t.name }} = "{{ t.category }}"
+  enforcement_mode_{{ t.name }} = {{ t.enforcement_mode }}
   policy_names = [{% for service_name, policies_with_params in t.policies_sorted_by_service.items() %}
     # -----------------------------------------------------------------------------------------------------------------
     # {{ service_name }}
@@ -25,18 +20,18 @@ locals {
 # Conditional data lookups: If the user supplies management group, look up the ID of the management group
 # ---------------------------------------------------------------------------------------------------------------------
 data "azurerm_management_group" "{{ t.name }}" {
-  count = var.management_group != "" ? 1 : 0
-  name  = var.management_group
+  count = local.management_group_{{ t.name }} != "" ? 1 : 0
+  display_name  = local.management_group_{{ t.name }}
 }
 
 ### If the user supplies subscription, look up the ID of the subscription
 data "azurerm_subscriptions" "{{ t.name }}" {
-  count                 = var.subscription_name != "" ? 1 : 0
-  display_name_contains = var.subscription_name
+  count                 = local.subscription_name_{{ t.name }} != "" ? 1 : 0
+  display_name_contains = local.subscription_name_{{ t.name }}
 }
 
 locals {
-  scope = var.management_group != "" ? data.azurerm_management_group.{{ t.name }}[0].id : element(data.azurerm_subscriptions.{{ t.name }}[0].subscriptions.*.id, 0)
+  scope = local.management_group_{{ t.name }} != "" ? data.azurerm_management_group.{{ t.name }}[0].id : element(data.azurerm_subscriptions.{{ t.name }}[0].subscriptions.*.id, 0)
 }
 
 # ---------------------------------------------------------------------------------------------------------------------
@@ -52,14 +47,14 @@ data "azurerm_policy_definition" "{{ t.name }}_definition_lookups" {
 # Azure Policy Initiative Definition
 # ---------------------------------------------------------------------------------------------------------------------
 
-resource "azurerm_policy_set_definition" "{{ t.name }}_guardrails" {
-  name                  = var.name
+resource "azurerm_policy_set_definition" "{{ t.name }}" {
+  name                  = local.name_{{ t.name }}
   policy_type           = "Custom"
-  display_name          = var.name
-  description           = var.name
-  management_group_name = var.management_group == "" ? null : var.management_group
+  display_name          = local.name_{{ t.name }}
+  description           = local.name_{{ t.name }}
+  management_group_name = local.management_group_{{ t.name }} == "" ? null : local.management_group_{{ t.name }}
   metadata = tostring(jsonencode({
-    category = var.category
+    category = local.category_{{ t.name }}
   }))
 
   {% for service_name, service_policy_details in t.policy_definition_reference_parameters.items() %}
@@ -82,11 +77,11 @@ PARAMETERS
 # Azure Policy Assignments
 # Apply the Policy Initiative to the specified scope
 # ---------------------------------------------------------------------------------------------------------------------
-resource "azurerm_policy_assignment" "{{ t.name }}_guardrails" {
-  name                 = var.name
-  policy_definition_id = azurerm_policy_set_definition.{{ t.name }}_guardrails.id
+resource "azurerm_policy_assignment" "{{ t.name }}" {
+  name                 = local.name_{{ t.name }}
+  policy_definition_id = azurerm_policy_set_definition.{{ t.name }}.id
   scope                = local.scope
-  enforcement_mode     = var.enforcement_mode
+  enforcement_mode     = local.enforcement_mode_{{ t.name }}
   parameters = jsonencode({
     {{ t.policy_assignment_parameters }}
 })
@@ -96,17 +91,17 @@ resource "azurerm_policy_assignment" "{{ t.name }}_guardrails" {
 # ---------------------------------------------------------------------------------------------------------------------
 # Outputs
 # ---------------------------------------------------------------------------------------------------------------------
-output "policy_assignment_ids" {
-  value       = azurerm_policy_assignment.{{ t.name }}_guardrails.*.id
+output "{{ t.name }}_policy_assignment_ids" {
+  value       = azurerm_policy_assignment.{{ t.name }}.*.id
   description = "The IDs of the Policy Assignments."
 }
 
-output "scope" {
+output "{{ t.name }}_scope" {
   value       = local.scope
   description = "The target scope - either the management group or subscription, depending on which parameters were supplied"
 }
 
-output "policy_set_definition_id" {
-  value       = azurerm_policy_set_definition.{{ t.name }}_guardrails.id
+output "{{ t.name }}_policy_set_definition_id" {
+  value       = azurerm_policy_set_definition.{{ t.name }}.id
   description = "The ID of the Policy Set Definition."
 }
diff --git a/azure_guardrails/terraform/terraform.py b/azure_guardrails/terraform/terraform.py
index d9361a1..aa049ca 100644
--- a/azure_guardrails/terraform/terraform.py
+++ b/azure_guardrails/terraform/terraform.py
@@ -29,6 +29,7 @@ def __init__(
             self.enforcement_string = "true"
         else:
             self.enforcement_string = "false"
+        self.category = "Testing"
 
     @staticmethod
     def _initiative_name(subscription_name: str, management_group: str) -> str:
@@ -57,6 +58,7 @@ def rendered(self) -> str:
             subscription_name=self.subscription_name,
             management_group=self.management_group,
             enforcement_mode=self.enforcement_string,
+            category=self.category
         )
         template_path = os.path.join(os.path.dirname(__file__), "no-parameters")
         env = Environment(loader=FileSystemLoader(template_path))  # nosec
@@ -136,37 +138,41 @@ class TerraformTemplateWithParams:
     def __init__(
         self,
         parameters: dict,
+        parameter_requirement_str: str,
         subscription_name: str = "",
         management_group: str = "",
         enforcement_mode: bool = False,
     ):
         self.name = self._initiative_name(
-            subscription_name=subscription_name, management_group=management_group
+            subscription_name=subscription_name, management_group=management_group,
+            parameter_requirement_str=parameter_requirement_str
         )
         self.service_parameters = self._parameters(parameters)
         self.subscription_name = subscription_name
         self.management_group = management_group
+        self.category = "Testing"
         if enforcement_mode:
             self.enforcement_string = "true"
         else:
             self.enforcement_string = "false"
 
     @staticmethod
-    def _initiative_name(subscription_name: str, management_group: str) -> str:
+    def _initiative_name(subscription_name: str, management_group: str, parameter_requirement_str: str) -> str:
         if subscription_name == "" and management_group == "":
             raise Exception(
                 "Please supply a value for the subscription name or the management group"
             )
-        # TODO: Differentiate between ParamsRequired and ParamsOptional
         if subscription_name:
             # shorten the name if it is over a certain length to avoid hitting limits
-            if len(subscription_name) > 55:
-                subscription_name = subscription_name[0:54]
-            initiative_name = f"{subscription_name}-params"
+            if len(subscription_name) > 50:
+                subscription_name = subscription_name[0:50]
+            initiative_name = f"{subscription_name}-{parameter_requirement_str}"
         else:
-            if len(management_group) > 55:
-                management_group = management_group[0:54]
-            initiative_name = f"{management_group}-params"
+            if len(management_group) > 50:
+                management_group = management_group[0:50]
+            initiative_name = f"{management_group}-{parameter_requirement_str}"
+        initiative_name = initiative_name.replace("-", "_")
+        initiative_name = initiative_name.lower()
         return initiative_name
 
     @staticmethod
@@ -241,7 +247,8 @@ def rendered(self) -> str:
             initiative_parameters=initiative_parameters,
             policies_sorted_by_service=self.policies_sorted_by_service,
             policy_definition_reference_parameters=self.service_parameters,
-            policy_assignment_parameters=self.policy_assignment_parameters
+            policy_assignment_parameters=self.policy_assignment_parameters,
+            category=self.category
         )
         template_path = os.path.join(os.path.dirname(__file__), "parameters")
         env = Environment(loader=FileSystemLoader(template_path))  # nosec
diff --git a/examples/terraform-demo-with-parameters/main.tf b/examples/terraform-demo-with-parameters/main.tf
index 7f51b6f..6515e47 100644
--- a/examples/terraform-demo-with-parameters/main.tf
+++ b/examples/terraform-demo-with-parameters/main.tf
@@ -1,26 +1,14 @@
-variable "name" { default = "example-params" }
-variable "subscription_name" { default = "example" }
-variable "management_group" { default = "" }
-variable "enforcement_mode" { default = false }
-
-variable "category" {
-  type    = string
-  default = "Testing"
-}
-provider "azurerm" {
-  features {}
-}
-
 locals {
+  name_example_params_optional = "example_params_optional"
+  subscription_name_example_params_optional = "example"
+  management_group_example_params_optional = ""
+  category_example_params_optional = "Testing"
+  enforcement_mode_example_params_optional = false
   policy_names = [
     # -----------------------------------------------------------------------------------------------------------------
     # API Management
     # -----------------------------------------------------------------------------------------------------------------
-    "API Management services should use a virtual network",
-    # -----------------------------------------------------------------------------------------------------------------
-    # App Platform
-    # -----------------------------------------------------------------------------------------------------------------
-    "Azure Spring Cloud should use network injection",
+    "API Management service should use a SKU that supports virtual networks",
     # -----------------------------------------------------------------------------------------------------------------
     # App Service
     # -----------------------------------------------------------------------------------------------------------------
@@ -95,36 +83,40 @@ locals {
     # Stream Analytics
     # -----------------------------------------------------------------------------------------------------------------
     "Resource logs in Azure Stream Analytics should be enabled",
+    # -----------------------------------------------------------------------------------------------------------------
+    # Synapse
+    # -----------------------------------------------------------------------------------------------------------------
+    "Auditing on Synapse workspace should be enabled",
   ]
   policy_definition_map = zipmap(
-    data.azurerm_policy_definition.example-params_definition_lookups.*.display_name,
-    data.azurerm_policy_definition.example-params_definition_lookups.*.id
+    data.azurerm_policy_definition.example_params_optional_definition_lookups.*.display_name,
+    data.azurerm_policy_definition.example_params_optional_definition_lookups.*.id
   )
 }
 
 # ---------------------------------------------------------------------------------------------------------------------
 # Conditional data lookups: If the user supplies management group, look up the ID of the management group
 # ---------------------------------------------------------------------------------------------------------------------
-data "azurerm_management_group" "example-params" {
-  count = var.management_group != "" ? 1 : 0
-  name  = var.management_group
+data "azurerm_management_group" "example_params_optional" {
+  count = local.management_group_example_params_optional != "" ? 1 : 0
+  display_name  = local.management_group_example_params_optional
 }
 
 ### If the user supplies subscription, look up the ID of the subscription
-data "azurerm_subscriptions" "example-params" {
-  count                 = var.subscription_name != "" ? 1 : 0
-  display_name_contains = var.subscription_name
+data "azurerm_subscriptions" "example_params_optional" {
+  count                 = local.subscription_name_example_params_optional != "" ? 1 : 0
+  display_name_contains = local.subscription_name_example_params_optional
 }
 
 locals {
-  scope = var.management_group != "" ? data.azurerm_management_group.example-params[0].id : element(data.azurerm_subscriptions.example-params[0].subscriptions.*.id, 0)
+  scope = local.management_group_example_params_optional != "" ? data.azurerm_management_group.example_params_optional[0].id : element(data.azurerm_subscriptions.example_params_optional[0].subscriptions.*.id, 0)
 }
 
 # ---------------------------------------------------------------------------------------------------------------------
 # Azure Policy Definition Lookups
 # ---------------------------------------------------------------------------------------------------------------------
 
-data "azurerm_policy_definition" "example-params_definition_lookups" {
+data "azurerm_policy_definition" "example_params_optional_definition_lookups" {
   count        = length(local.policy_names)
   display_name = local.policy_names[count.index]
 }
@@ -133,22 +125,22 @@ data "azurerm_policy_definition" "example-params_definition_lookups" {
 # Azure Policy Initiative Definition
 # ---------------------------------------------------------------------------------------------------------------------
 
-resource "azurerm_policy_set_definition" "example-params_guardrails" {
-  name                  = var.name
+resource "azurerm_policy_set_definition" "example_params_optional" {
+  name                  = local.name_example_params_optional
   policy_type           = "Custom"
-  display_name          = var.name
-  description           = var.name
-  management_group_name = var.management_group == "" ? null : var.management_group
+  display_name          = local.name_example_params_optional
+  description           = local.name_example_params_optional
+  management_group_name = local.management_group_example_params_optional == "" ? null : local.management_group_example_params_optional
   metadata = tostring(jsonencode({
-    category = var.category
+    category = local.category_example_params_optional
   }))
 
 
 
   policy_definition_reference {
-    policy_definition_id = lookup(local.policy_definition_map, "API Management services should use a virtual network")
+    policy_definition_id = lookup(local.policy_definition_map, "API Management service should use a SKU that supports virtual networks")
     parameter_values = jsonencode({
-      evaluatedSkuNames = { "value" : "[parameters('evaluatedSkuNames')]" }
+      listOfAllowedSKUs = { "value" : "[parameters('listOfAllowedSKUs')]" }
     })
     reference_id = null
   }
@@ -156,14 +148,6 @@ resource "azurerm_policy_set_definition" "example-params_guardrails" {
 
 
 
-  policy_definition_reference {
-    policy_definition_id = lookup(local.policy_definition_map, "Azure Spring Cloud should use network injection")
-    parameter_values = jsonencode({
-      evaluatedSkuNames = { "value" : "[parameters('evaluatedSkuNames')]" }
-    })
-    reference_id = null
-  }
-
 
   policy_definition_reference {
     policy_definition_id = lookup(local.policy_definition_map, "Ensure that 'Java version' is the latest, if used as a part of the API app")
@@ -263,6 +247,7 @@ resource "azurerm_policy_set_definition" "example-params_guardrails" {
 
 
 
+
   policy_definition_reference {
     policy_definition_id = lookup(local.policy_definition_map, "Azure Data Box jobs should enable double encryption for data at rest on the device")
     parameter_values = jsonencode({
@@ -466,21 +451,36 @@ resource "azurerm_policy_set_definition" "example-params_guardrails" {
   }
 
 
+  policy_definition_reference {
+    policy_definition_id = lookup(local.policy_definition_map, "Auditing on Synapse workspace should be enabled")
+    parameter_values = jsonencode({
+      setting = { "value" : "[parameters('setting')]" }
+    })
+    reference_id = null
+  }
+
 
 
 
   parameters = <<PARAMETERS
 {
-    "evaluatedSkuNames": {
-        "name": "evaluatedSkuNames",
+    "listOfAllowedSKUs": {
+        "name": "listOfAllowedSKUs",
         "type": "Array",
-        "description": "List of Azure Spring Cloud SKUs against which this policy will be evaluated.",
-        "display_name": "Azure Spring Cloud SKU Names",
+        "description": "The list of SKUs that can be specified for Azure API Management service.",
+        "display_name": "Allowed SKUs",
         "default_value": [
-            "Standard"
+            "Developer",
+            "Premium",
+            "Isolated"
         ],
         "allowed_values": [
-            "Standard"
+            "Developer",
+            "Basic",
+            "Standard",
+            "Premium",
+            "Isolated",
+            "Consumption"
         ]
     },
     "JavaLatestVersion": {
@@ -509,7 +509,7 @@ resource "azurerm_policy_set_definition" "example-params_guardrails" {
         "type": "String",
         "description": "Latest supported Python version for App Services",
         "display_name": "Linux Latest Python version",
-        "default_value": "3.8"
+        "default_value": "3.9"
     },
     "requiredRetentionDays": {
         "name": "requiredRetentionDays",
@@ -626,17 +626,17 @@ PARAMETERS
 # Azure Policy Assignments
 # Apply the Policy Initiative to the specified scope
 # ---------------------------------------------------------------------------------------------------------------------
-resource "azurerm_policy_assignment" "example-params_guardrails" {
-  name                 = var.name
-  policy_definition_id = azurerm_policy_set_definition.example-params_guardrails.id
+resource "azurerm_policy_assignment" "example_params_optional" {
+  name                 = local.name_example_params_optional
+  policy_definition_id = azurerm_policy_set_definition.example_params_optional.id
   scope                = local.scope
-  enforcement_mode     = var.enforcement_mode
+  enforcement_mode     = local.enforcement_mode_example_params_optional
   parameters = jsonencode({
-    evaluatedSkuNames = { "value" = ["Developer", "Premium"] }
+    listOfAllowedSKUs = { "value" = ["Developer", "Premium", "Isolated"] }
 	JavaLatestVersion = { "value" = "11" }
 	PHPLatestVersion = { "value" = "7.3" }
 	WindowsPythonLatestVersion = { "value" = "3.6" }
-	LinuxPythonLatestVersion = { "value" = "3.8" }
+	LinuxPythonLatestVersion = { "value" = "3.9" }
 	requiredRetentionDays = { "value" = "365" }
 	supportedSKUs = { "value" = ["DataBox", "DataBoxHeavy"] }
 	maxCores = { "value" = 32 }
@@ -653,17 +653,17 @@ resource "azurerm_policy_assignment" "example-params_guardrails" {
 # ---------------------------------------------------------------------------------------------------------------------
 # Outputs
 # ---------------------------------------------------------------------------------------------------------------------
-output "policy_assignment_ids" {
-  value       = azurerm_policy_assignment.example-params_guardrails.*.id
+output "example_params_optional_policy_assignment_ids" {
+  value       = azurerm_policy_assignment.example_params_optional.*.id
   description = "The IDs of the Policy Assignments."
 }
 
-output "scope" {
+output "example_params_optional_scope" {
   value       = local.scope
   description = "The target scope - either the management group or subscription, depending on which parameters were supplied"
 }
 
-output "policy_set_definition_id" {
-  value       = azurerm_policy_set_definition.example-params_guardrails.id
+output "example_params_optional_policy_set_definition_id" {
+  value       = azurerm_policy_set_definition.example_params_optional.id
   description = "The ID of the Policy Set Definition."
 }
diff --git a/examples/terraform-demo-with-parameters/provider.tf b/examples/terraform-demo-with-parameters/provider.tf
new file mode 100644
index 0000000..615cb43
--- /dev/null
+++ b/examples/terraform-demo-with-parameters/provider.tf
@@ -0,0 +1,3 @@
+provider "azurerm" {
+  features {}
+}
\ No newline at end of file
diff --git a/examples/terraform-demo-with-parameters/terraform.tf b/examples/terraform-demo-with-parameters/terraform.tf
new file mode 100644
index 0000000..f94b61f
--- /dev/null
+++ b/examples/terraform-demo-with-parameters/terraform.tf
@@ -0,0 +1,3 @@
+terraform {
+  required_version = ">= 0.12.0"
+}


Test patch:
diff --git a/test/terraform/test_terraform_v2.py b/test/terraform/test_terraform_v2.py
index b9f635c..79722c9 100644
--- a/test/terraform/test_terraform_v2.py
+++ b/test/terraform/test_terraform_v2.py
@@ -102,6 +102,7 @@ def setUp(self) -> None:
         management_group = ""
         enforcement_mode = False
         self.kubernetes_terraform_template = TerraformTemplateWithParams(
+            parameter_requirement_str="params",
             parameters=self.kubernetes_parameters,
             subscription_name=subscription_name,
             management_group=management_group,
@@ -132,6 +133,7 @@ def setUp(self) -> None:
             }
         }
         self.key_vault_terraform_template = TerraformTemplateWithParams(
+            parameter_requirement_str="params",
             parameters=self.key_vault_parameters,
             subscription_name=subscription_name,
             management_group=management_group,