Merge pull request #24 from nick4fake/fix/12

#12: Invalid syntax, when enforce variable is default (empty string)

README.md

@@ -35,11 +35,11 @@ To control module's behavior, change variables' values regarding the following:
 - `exclude_folders`: a list of folder IDs to be excluded from this policy. These folders must be lower in the hierarchy than the policy root.
 - `exclude_projects`: a list of project IDs to be excluded from this policy. They must be lower in the hierarchy than the policy root.
 - Boolean policies (with `policy_type: "boolean"`) can set the following variables:
-  - `enforce`: if "true" the policy is enforced at the root, if "false" the policy is not enforced at the root. (default `true`)
+  - `enforce`: if `true` or `null` then the policy is enforced at the root; if `false` then policy is not enforced at the root. (default `null`)
 - List policies (with `policy_type: "list"`) can set **one of** the following variables. Only one may be set.
-  - `enforce`: if "true" policy will deny all, if "false" policy will allow all (default `true`)
-  - `allow`: list of values to include in the policy with ALLOW behavior
-  - `deny`: list of values to include in the policy with DENY behavior
+  - `enforce`: if `true` or `null` then policy will deny all; if `false` then policy will allow all (default `null`)
+  - `allow`: list of values to include in the policy with ALLOW behavior. Set `enforce` to `null` to use it.
+  - `deny`: list of values to include in the policy with DENY behavior. Set `enforce` to `null` to use it.
 - List policies with allow or deny values require the length to be set (a workaround for [this terraform issue](https://github.com/hashicorp/terraform/issues/10857))
   - `allow_list_length`
   - `deny_list_length`
@@ -53,7 +53,7 @@ To control module's behavior, change variables' values regarding the following:
 | allow\_list\_length | The number of elements in the allow list | number | `"0"` | no |
 | constraint | The constraint to be applied | string | n/a | yes |
 | deny | (Only for list constraints) List of values which should be denied | list(string) | `<list>` | no |
-| deny\_list\_length | The number of elements in the allow list | number | `"0"` | no |
+| deny\_list\_length | The number of elements in the deny list | number | `"0"` | no |
 | enforce | If boolean constraint, whether the policy is enforced at the root; if list constraint, whether to deny all (true) or allow all | bool | `"null"` | no |
 | exclude\_folders | List of folders to exclude from the policy | list(string) | `<list>` | no |
 | exclude\_projects | List of projects to exclude from the policy | list(string) | `<list>` | no |

boolean_constraints.tf

@@ -24,7 +24,7 @@ resource "google_organization_policy" "org_policy_boolean" {
   constraint = var.constraint
 
   boolean_policy {
-    enforced = var.enforce
+    enforced = var.enforce != false
   }
 }
 
@@ -38,7 +38,7 @@ resource "google_folder_organization_policy" "folder_policy_boolean" {
   constraint = var.constraint
 
   boolean_policy {
-    enforced = var.enforce
+    enforced = var.enforce != false
   }
 }
 
@@ -52,7 +52,7 @@ resource "google_project_organization_policy" "project_policy_boolean" {
   constraint = var.constraint
 
   boolean_policy {
-    enforced = var.enforce
+    enforced = var.enforce != false
   }
 }
 
@@ -66,7 +66,7 @@ resource "google_folder_organization_policy" "policy_boolean_exclude_folders" {
   constraint = var.constraint
 
   boolean_policy {
-    enforced = var.enforce != true
+    enforced = var.enforce == false
   }
 }
 
@@ -80,6 +80,6 @@ resource "google_project_organization_policy" "policy_boolean_exclude_projects"
   constraint = var.constraint
 
   boolean_policy {
-    enforced = var.enforce != true
+    enforced = var.enforce == false
   }
 }

list_constraints.tf

@@ -18,7 +18,7 @@
   Organization policy, allow all (list constraint)
  *****************************************/
 resource "google_organization_policy" "org_policy_list_allow_all" {
-  count = local.organization && local.list_policy && local.enforce == false && var.deny_list_length == 0 ? 1 : 0
+  count = local.organization && local.list_policy && local.enforce == false ? 1 : 0
 
   org_id     = var.organization_id
   constraint = var.constraint
@@ -34,7 +34,7 @@ resource "google_organization_policy" "org_policy_list_allow_all" {
   Folder policy, allow all (list constraint)
  *****************************************/
 resource "google_folder_organization_policy" "folder_policy_list_allow_all" {
-  count = local.folder && local.list_policy && local.enforce == false && var.deny_list_length == 0 ? 1 : 0
+  count = local.folder && local.list_policy && local.enforce == false ? 1 : 0
 
   folder     = var.folder_id
   constraint = var.constraint
@@ -50,7 +50,7 @@ resource "google_folder_organization_policy" "folder_policy_list_allow_all" {
   Project policy, allow all (list constraint)
  *****************************************/
 resource "google_project_organization_policy" "project_policy_list_allow_all" {
-  count = local.project && local.list_policy && local.enforce == false && var.deny_list_length == 0 ? 1 : 0
+  count = local.project && local.list_policy && local.enforce == false ? 1 : 0
 
   project    = var.project_id
   constraint = var.constraint
@@ -66,7 +66,7 @@ resource "google_project_organization_policy" "project_policy_list_allow_all" {
   Organization policy, deny all (list constraint)
  *****************************************/
 resource "google_organization_policy" "org_policy_list_deny_all" {
-  count = local.organization && local.list_policy && local.enforce == true && var.deny_list_length == 0 ? 1 : 0
+  count = local.organization && local.list_policy && local.enforce == true ? 1 : 0
 
   org_id     = var.organization_id
   constraint = var.constraint
@@ -82,7 +82,7 @@ resource "google_organization_policy" "org_policy_list_deny_all" {
   Folder policy, deny all (list constraint)
  *****************************************/
 resource "google_folder_organization_policy" "folder_policy_list_deny_all" {
-  count = local.folder && local.list_policy && local.enforce == true && var.deny_list_length == 0 ? 1 : 0
+  count = local.folder && local.list_policy && local.enforce == true ? 1 : 0
 
   folder     = var.folder_id
   constraint = var.constraint
@@ -98,7 +98,7 @@ resource "google_folder_organization_policy" "folder_policy_list_deny_all" {
   Project policy, deny all (list constraint)
  *****************************************/
 resource "google_project_organization_policy" "project_policy_list_deny_all" {
-  count = local.project && local.list_policy && local.enforce == true && var.deny_list_length == 0 ? 1 : 0
+  count = local.project && local.list_policy && local.enforce == true ? 1 : 0
 
   project    = var.project_id
   constraint = var.constraint

main.tf

@@ -18,17 +18,21 @@
   Locals configuration for module logic
  *****************************************/
 locals {
-  organization                 = var.policy_for == "organization"
-  folder                       = var.policy_for == "folder"
-  project                      = var.policy_for == "project"
-  boolean_policy               = var.policy_type == "boolean"
-  list_policy                  = var.policy_type == "list" && ! local.invalid_config
-  enforce                      = var.allow_list_length > 0 || var.deny_list_length > 0 ? null : var.enforce
+  organization   = var.policy_for == "organization"
+  folder         = var.policy_for == "folder"
+  project        = var.policy_for == "project"
+  boolean_policy = var.policy_type == "boolean"
+  list_policy    = var.policy_type == "list" && ! local.invalid_config
+
+  // If allow/deny list empty and enforce is not set, enforce is set to true
+  enforce                      = var.allow_list_length > 0 || var.deny_list_length > 0 ? null : var.enforce != false
   exclude_folders_list_length  = length(compact(var.exclude_folders))
   exclude_projects_list_length = length(compact(var.exclude_projects))
   invalid_config_case_1        = var.deny_list_length > 0 && var.allow_list_length > 0
-  invalid_config_case_2        = var.allow_list_length + var.deny_list_length > 0 && var.enforce != null
-  invalid_config               = var.policy_type == "list" && local.invalid_config_case_1 || local.invalid_config_case_2
+
+  // We use var.enforce here because allow/deny lists can not be used together with enforce flag
+  invalid_config_case_2 = var.allow_list_length + var.deny_list_length > 0 && var.enforce != null
+  invalid_config        = var.policy_type == "list" && local.invalid_config_case_1 || local.invalid_config_case_2
 }
 
 /******************************************

test/integration/boolean_constraints/launch.sh

@@ -58,7 +58,6 @@ module "org-policy-boolean-project" {
   policy_for  = "project"
   constraint  = "$PROJECT_CONSTRAINT"
   project_id  = "$PROJECT_ID"
-  enforce     = "true"
   policy_type = "boolean"
 }
 module "org-policy-boolean-folder" {

test/integration/list_constraints/launch.sh

@@ -62,7 +62,6 @@ module "org-policy-list-project" {
   policy_for  = "project"
   constraint  = "$PROJECT_CONSTRAINT_DENY_ALL"
   project_id  = "$PROJECT_ID"
-  enforce     = "true"
   policy_type = "list"
 }
 module "org-policy-list-folder" {

test/task_helper_functions.sh

@@ -17,16 +17,20 @@
 test_list_constraints() {
   source_test_env
   init_credentials
+  # shellcheck disable=SC2164
   cd test/integration/list_constraints
   bash launch.sh
+  # shellcheck disable=SC2164
   cd -
 }
 
 test_boolean_constraints() {
   source_test_env
   init_credentials
+  # shellcheck disable=SC2164
   cd test/integration/boolean_constraints
   bash launch.sh
+  # shellcheck disable=SC2164
   cd -
 }
 

variables.tf

@@ -84,7 +84,7 @@ variable "allow_list_length" {
 }
 
 variable "deny_list_length" {
-  description = "The number of elements in the allow list"
+  description = "The number of elements in the deny list"
   type        = number
   default     = 0
 }