diff --git a/modules/organization-policy/README.md b/modules/organization-policy/README.md index 0b352699..f26357e4 100644 --- a/modules/organization-policy/README.md +++ b/modules/organization-policy/README.md @@ -20,27 +20,34 @@ module "org-policy-factory" { module "org-policy" { source = "./modules/organization-policy" - organization_policies = { + policies = { "folders/1234567890" = { - "constraints/iam.disableServiceAccountKeyUpload" = { - rules = [ - { - enforce = true - } - ] - } + # enforce boolean policy with no conditions + "iam.disableServiceAccountKeyUpload" = { + rules = [ + { + enforce = true + } + ] + }, + # Deny All for compute.vmCanIpForward policy + "compute.vmCanIpForward" = { + inherit_from_parent = false + rules = [ + deny = [] # stands for deny_all + ] + } }, "organizations/1234567890" = { + # allow only internal ingress when match condition env=prod "run.allowedIngress" = { rules = [ { + allow = ["internal"] condition = { description= "allow ingress" expression = "resource.matchTag('123456789/environment', 'prod')" title = "allow-for-prod-org" - }, - values = { - allowed_values = ["internal"] } } ] @@ -53,29 +60,25 @@ module "org-policy" { ## Org Policy definition format and structure -### Structure of `organization_policies` variable +### Structure of `policies` variable ```hcl -organization_policies = { +policies = { "parent_id" = { # parent id in format projects/project-id, folders/1234567890 or organizations/1234567890. "policy_name" = { # policy constraint id, for example compute.vmExternalIpAccess. inherit_from_parent = true|false # (Optional) Only for list constraints. Determines the inheritance behavior for this policy. reset = true|false # (Optional) Ignores policies set above this resource and restores the constraint_default enforcement behavior. rules = [ # Up to 10 PolicyRules are allowed. { - allow_all = true|false # (Optional) Only for list constraints. Setting this to true means that all values are allowed. - deny_all = true|false # (Optional) Only for list constraints. Setting this to true means that all values are denied. - enforce = true|false # (Optional) Only for boolean constraints. If true, then the Policy is enforced. - condition = { # (Optional) A condition which determines whether this rule is used in the evaluation of the policy. + allow = ["value1", "value2"] # (Optional) Only for list constraints. Stands for `allow_all` if set to empty list `[]` or to `values.allowed_values` if set to a list of values + denyl = ["value3", "value4"] # (Optional) Only for list constraints. Stands for `deny_all` if set to empty list `[]` or to `values.denied_values` if set to a list of values + enforce = true|false # (Optional) Only for boolean constraints. If true, then the Policy is enforced. + condition = { # (Optional) A condition which determines whether this rule is used in the evaluation of the policy. description = "Condition description" # (Optional) expression = "Condition expression" # (Optional) For example "resource.matchTag('123456789/environment', 'prod')". location = "policy-error.log" # (Optional) String indicating the location of the expression for error reporting. title = "condition-title" # (Optional) } - values = { # (Optional) Only for list constraints. List of values to be used for this PolicyRule. - allowed_values = ["value1", "value2"] # (Optional) List of values allowed at this resource. - denied_values = ["value3", "value4"] # (Optional) List of values denied at this resource. - } } ] } @@ -94,18 +97,14 @@ parent_id: # parent id in format projects/project-id, folders/1234567890 or orga inherit_from_parent: true|false # (Optional) Only for list constraints. Determines the inheritance behavior for this policy. reset: true|false # (Optional) Ignores policies set above this resource and restores the constraint_default enforcement behavior. rules: - - allow_all: true|false # (Optional) Only for list constraints. Setting this to true means that all values are allowed. - deny_all: true|false # (Optional) Only for list constraints. Setting this to true means that all values are denied. + - allow: ["value1", "value2"] # (Optional) Only for list constraints. Stands for `allow_all` if set to empty list `[]` or to `values.allowed_values` if set to a list of values + deny: ["value3", "value4"] # (Optional) Only for list constraints. Stands for `deny_all` if set to empty list `[]` or to `values.denied_values` if set to a list of values enforce: true|false # (Optional) Only for boolean constraints. If true, then the Policy is enforced. condition: # (Optional) A condition which determines whether this rule is used in the evaluation of the policy. description: Condition description # (Optional) expression: Condition expression # (Optional) For example resource.matchTag("123456789/environment", "prod") location: policy-error.log # (Optional) String indicating the location of the expression for error reporting. title: condition-title # (Optional) - values: # (Optional) Only for list constraints. List of values to be used for this PolicyRule. - allowed_values: ['value1', 'value2'] # (Optional) List of values allowed at this resource. - denied_values: ['value3', 'value4'] # (Optional) List of values denied at this resource. - ``` Module allows policies to be distributed into multiple yaml files for a better management and navigation. @@ -130,7 +129,7 @@ folders/1234567890: inherit_from_parent: false reset: false rules: - - allow_all: true + - allow: [] # Stands for allow_all = true projects/my-project-id: run.allowedIngress: inherit_from_parent: true @@ -144,7 +143,7 @@ projects/my-project-id: allowed_values: ['internal'] iam.allowServiceAccountCredentialLifetimeExtension: rules: - - allow_all: true + - deny: [] # Stands for deny_all = true compute.disableGlobalLoadBalancing: reset: true ``` @@ -155,12 +154,12 @@ projects/my-project-id: | name | description | type | required | default | |---|---|:---:|:---:|:---:| | [config_directory](variables.tf#L17) | Paths to a folder where organization policy configs are stored in yaml format. Files suffix must be `.yaml`. | string | | null | -| [organization_policies](variables.tf#L26) | Organization policies keyed by parent in format `projects/project-id`, `folders/1234567890` or `organizations/1234567890`. | any | | {} | +| [policies](variables.tf#L23) | Organization policies keyed by parent in format `projects/project-id`, `folders/1234567890` or `organizations/1234567890`. | map(map(object({…}))) | | {} | ## Outputs | name | description | sensitive | |---|---|:---:| -| [organization_policies](outputs.tf#L17) | Organization policies. | | +| [policies](outputs.tf#L17) | Organization policies. | | diff --git a/modules/organization-policy/main.tf b/modules/organization-policy/main.tf index 6c7c1537..960a8462 100644 --- a/modules/organization-policy/main.tf +++ b/modules/organization-policy/main.tf @@ -16,23 +16,23 @@ locals { - org_policy_files = var.config_directory == null ? [] : concat( + policy_files = var.config_directory == null ? [] : concat( [ for config_file in fileset("${path.root}/${var.config_directory}", "**/*.yaml") : "${path.root}/${var.config_directory}/${config_file}" ] ) - org_policies_raw = merge( + policies_raw = merge( merge( [ - for config_file in local.org_policy_files : + for config_file in local.policy_files : try(yamldecode(file(config_file)), {}) ]... - ), var.organization_policies) + ), var.policies) - org_policies_list = flatten([ - for parent, policies in local.org_policies_raw : [ + policies_list = flatten([ + for parent, policies in local.policies_raw : [ for policy_name, policy in policies : { parent = parent, policy_name = policy_name, @@ -40,12 +40,8 @@ locals { reset = try(policy["reset"], null), rules = [ for rule in try(policy["rules"], []) : { - allow_all = try( - rule["allow_all"], null) == true ? "TRUE" : try( - rule["allow_all"], null) == false ? "FALSE" : null, - deny_all = try( - rule["deny_all"], null) == true ? "TRUE" : try( - rule["deny_all"], null) == false ? "FALSE" : null, + allow_all = try(length(rule["allow"]), -1) == 0 ? "TRUE" : null + deny_all = try(length(rule["deny"]), -1) == 0 ? "TRUE" : null enforce = try(rule["enforce"], null) == true ? "TRUE" : try( rule["enforce"], null) == false ? "FALSE" : null, condition = try(rule["condition"], null) != null ? { @@ -54,9 +50,9 @@ locals { location = try(rule["condition"]["location"], null), title = try(rule["condition"]["title"], null) } : null, - values = try(rule["values"], null) != null ? { - allowed_values = try(rule["values"]["allowed_values"], null), - denied_values = try(rule["values"]["denied_values"], null) + values = try(length(rule["allow"]), 0) > 0 || try(length(rule["deny"]), 0) > 0 ? { + allowed_values = try(length(rule["allow"]), 0) > 0 ? rule["allow"] : null + denied_values = try(length(rule["deny"]), 0) > 0 ? rule["deny"] : null } : null } ] @@ -64,14 +60,14 @@ locals { ] ]) - org_policies_map = { - for item in local.org_policies_list : + policies_map = { + for item in local.policies_list : format("%s-%s", item["parent"], item["policy_name"]) => item } } resource "google_org_policy_policy" "primary" { - for_each = local.org_policies_map + for_each = local.policies_map name = format("%s/policies/%s", each.value.parent, each.value.policy_name) parent = each.value.parent diff --git a/modules/organization-policy/outputs.tf b/modules/organization-policy/outputs.tf index 5f545fd2..6134d876 100644 --- a/modules/organization-policy/outputs.tf +++ b/modules/organization-policy/outputs.tf @@ -14,7 +14,7 @@ * limitations under the License. */ -output "organization_policies" { +output "policies" { description = "Organization policies." value = google_org_policy_policy.primary } diff --git a/modules/organization-policy/variables.tf b/modules/organization-policy/variables.tf index 97284f9b..ff842dd9 100644 --- a/modules/organization-policy/variables.tf +++ b/modules/organization-policy/variables.tf @@ -20,11 +20,26 @@ variable "config_directory" { default = null } -# TODO: convert to a proper data structure map(map(object({...}))) once tf1.3 is released and optional object keys are avaliable, -# for now it will cause multiple keys to be set to null for every policy definition -# https://github.com/hashicorp/terraform/releases/tag/v1.3.0-alpha20220622 -variable "organization_policies" { +variable "policies" { description = "Organization policies keyed by parent in format `projects/project-id`, `folders/1234567890` or `organizations/1234567890`." - type = any - default = {} + type = map(map(object({ + inherit_from_parent = optional(bool) # List policy only. + reset = optional(bool) + rules = optional( + list(object({ + allow = optional(list(string)) # List policy only. Stands for `allow_all` if set to empty list `[]` or to `values.allowed_values` if set to a list of values + deny = optional(list(string)) # List policy only. Stands for `deny_all` if set to empty list `[]` or to `values.denied_values` if set to a list of values + enforce = optional(bool) # Boolean policy only. + condition = optional( + object({ + description = optional(string) + expression = optional(string) + location = optional(string) + title = optional(string) + }) + ) + })) + ) + }))) + default = {} } diff --git a/modules/organization-policy/versions.tf b/modules/organization-policy/versions.tf index c01e5da0..52c88cef 100644 --- a/modules/organization-policy/versions.tf +++ b/modules/organization-policy/versions.tf @@ -14,6 +14,10 @@ terraform { required_version = ">= 1.1.0" + + # TODO: Remove once Terraform 1.3 is released https://github.com/hashicorp/terraform/releases/tag/v1.3.0-alpha20220622 + experiments = [module_variable_optional_attrs] + required_providers { google = { source = "hashicorp/google" diff --git a/tests/modules/organization-policy/fixture/main.tf b/tests/modules/organization-policy/fixture/main.tf index addb3911..09a09267 100644 --- a/tests/modules/organization-policy/fixture/main.tf +++ b/tests/modules/organization-policy/fixture/main.tf @@ -17,6 +17,6 @@ module "org-policy" { source = "../../../../modules/organization-policy" - config_directory = var.config_directory - organization_policies = var.organization_policies + config_directory = var.config_directory + policies = var.policies } diff --git a/tests/modules/organization-policy/fixture/policies/test.yaml b/tests/modules/organization-policy/fixture/policies/test.yaml index 2630cd6b..4b81e524 100644 --- a/tests/modules/organization-policy/fixture/policies/test.yaml +++ b/tests/modules/organization-policy/fixture/policies/test.yaml @@ -22,20 +22,19 @@ folders/1234567890: inherit_from_parent: false reset: false rules: - - allow_all: true + - allow: [] projects/my-project-id: run.allowedIngress: inherit_from_parent: true rules: - - condition: + - allow: ['internal'] + condition: description: allow internal ingress expression: resource.matchTag("123456789/environment", "prod") location: test.log title: allow-for-prod - values: - allowed_values: ['internal'] iam.allowServiceAccountCredentialLifetimeExtension: rules: - - allow_all: true + - deny: [] compute.disableGlobalLoadBalancing: reset: true diff --git a/tests/modules/organization-policy/fixture/variables.tf b/tests/modules/organization-policy/fixture/variables.tf index 6e36bdec..709a9f98 100644 --- a/tests/modules/organization-policy/fixture/variables.tf +++ b/tests/modules/organization-policy/fixture/variables.tf @@ -24,7 +24,7 @@ variable "config_directory" { # TODO: convert to a proper data structure map(map(object({...}))) once tf1.3 is released and optional object keys are avaliable, # for now it will cause multiple keys to be set to null for every policy definition # https://github.com/hashicorp/terraform/releases/tag/v1.3.0-alpha20220622 -variable "organization_policies" { +variable "policies" { description = "Organization policies keyed by parent in format `projects/project-id`, `folders/1234567890` or `organizations/1234567890`." type = any default = {} diff --git a/tests/modules/organization-policy/test_plan.py b/tests/modules/organization-policy/test_plan.py index bb7879d7..fa7e5cd7 100644 --- a/tests/modules/organization-policy/test_plan.py +++ b/tests/modules/organization-policy/test_plan.py @@ -17,25 +17,23 @@ def test_org_policy_simple(plan_runner): org_policies = ( '{' '"folders/1234567890" = {' - ' "constraints/iam.disableServiceAccountKeyUpload" = {' - ' rules = [' - ' {' - ' enforce = true' - ' }' - ' ]' - ' }' + ' "constraints/iam.disableServiceAccountKeyUpload" = {' + ' rules = [' + ' {' + ' enforce = true,' + ' }' + ' ]' + ' }' ' },' ' "organizations/1234567890" = {' ' "run.allowedIngress" = {' ' rules = [' ' {' + ' allow = ["internal"],' ' condition = {' ' description= "allow ingress",' ' expression = "resource.matchTag(\'123456789/environment\', \'prod\')",' - ' title = "allow-for-prod-org",' - ' },' - ' values = {' - ' allowed_values = ["internal"]' + ' title = "allow-for-prod-org"' ' }' ' }' ' ]' @@ -44,7 +42,7 @@ def test_org_policy_simple(plan_runner): '}' ) _, resources = plan_runner( - organization_policies = org_policies + policies = org_policies ) assert len(resources) == 2 @@ -85,7 +83,7 @@ def test_combined_org_policy_config(plan_runner): ) _, resources = plan_runner( config_directory="./policies", - organization_policies = org_policies + policies = org_policies ) assert len(resources) == 6