From cffc823f482128a38922cb0652bae3916d8bc289 Mon Sep 17 00:00:00 2001 From: Ludovico Magnocavallo Date: Sat, 21 Nov 2020 09:45:56 +0100 Subject: [PATCH] refactor access in bq module, add iam (#172) --- .../main.tf | 4 +- .../gcs-to-bq-with-dataflow/main.tf | 6 +-- modules/bigquery-dataset/README.md | 40 ++++++++++++---- modules/bigquery-dataset/main.tf | 48 ++++++++++--------- modules/bigquery-dataset/variables.tf | 42 +++++++++------- .../gcs_to_bq_with_dataflow/test_plan.py | 2 +- 6 files changed, 89 insertions(+), 53 deletions(-) diff --git a/cloud-operations/scheduled-asset-inventory-export-bq/main.tf b/cloud-operations/scheduled-asset-inventory-export-bq/main.tf index 9af8951b..4d88e429 100644 --- a/cloud-operations/scheduled-asset-inventory-export-bq/main.tf +++ b/cloud-operations/scheduled-asset-inventory-export-bq/main.tf @@ -125,8 +125,8 @@ module "bq" { source = "../../modules/bigquery-dataset" project_id = module.project.project_id id = var.cai_config.bq_dataset - access_roles = { - owner = { role = "OWNER", type = "user_by_email" } + access = { + owner = { role = "OWNER", type = "user" } } access_identities = { owner = module.service-account.email diff --git a/data-solutions/gcs-to-bq-with-dataflow/main.tf b/data-solutions/gcs-to-bq-with-dataflow/main.tf index f9fda8aa..1f85beaa 100644 --- a/data-solutions/gcs-to-bq-with-dataflow/main.tf +++ b/data-solutions/gcs-to-bq-with-dataflow/main.tf @@ -283,9 +283,9 @@ module "bigquery-dataset" { source = "../../modules/bigquery-dataset" project_id = module.project-service.project_id id = "bq_dataset" - access_roles = { - reader-group = { role = "READER", type = "service_account" } - owner = { role = "OWNER", type = "user_by_email" } + access = { + reader-group = { role = "READER", type = "user" } + owner = { role = "OWNER", type = "user" } } access_identities = { reader-group = module.service-account-bq.email diff --git a/modules/bigquery-dataset/README.md b/modules/bigquery-dataset/README.md index d227990b..0c83d608 100644 --- a/modules/bigquery-dataset/README.md +++ b/modules/bigquery-dataset/README.md @@ -22,18 +22,40 @@ module "bigquery-dataset" { source = "./modules/bigquery-dataset" project_id = "my-project" id = "my-dataset" - access_roles = { - reader-group = { role = "READER", type = "group_by_email" } - owner = { role = "OWNER", type = "user_by_email" } + access = { + reader-group = { role = "READER", type = "group" } + owner = { role = "OWNER", type = "user" } + project_owners = { role = "OWNER", type = "special_group" } + view_1 = { role = "READER", type = "view" } } access_identities = { - reader-group = "playground-test@ludomagno.net" - owner = "ludo@ludomagno.net" + reader-group = "playground-test@ludomagno.net" + owner = "ludo@ludomagno.net" + project_owners = "projectOwners" + view_1 = "my-project|my-dataset|my-table" } } -# tftest:modules=1:resources=3 +# tftest:modules=1:resources=5 ``` +### IAM roles + +Access configuration can also be specified via IAM instead of basic roles via the `iam` variable. When using IAM, basic roles cannot be used via the `access` family variables. + +```hcl +module "bigquery-dataset" { + source = "./modules/bigquery-dataset" + project_id = "my-project" + id = "my-dataset" + iam = { + "roles/bigquery.dataOwner" = ["user:user1@example.org"] + } +} +# tftest:modules=1:resources=2 +``` + +roles/bigquery.dataOwner + ### Dataset options Dataset options are set via the `options` variable. all options must be specified, but a `null` value can be set to options that need to use defaults. @@ -137,12 +159,12 @@ module "bigquery-dataset" { |---|---|:---: |:---:|:---:| | id | Dataset id. | string | ✓ | | | project_id | Id of the project where datasets will be created. | string | ✓ | | -| *access_identities* | Map of access identities used for access roles with type different from `view`. A separate variable is needed as identities can be set to dynamic values. | map(string) | | {} | -| *access_roles* | Map of access rules with role and identity type. Keys are arbitrary and only used to combine identities with each role. Valid types are `domain`, `group_by_email`, `special_group`, `user_by_email`, `view`. | map(object({...})) | | {} | -| *access_views* | Map of view data for access roles with identity type equal to `view`. A separate variable is needed as identities can be set to dynamic values. | map(object({...})) | | {} | +| *access* | Map of access rules with role and identity type. Keys are arbitrary and must match those in the `access_identities` variable, types are `domain`, `group`, `special_group`, `user`, `view`. | map(object({...})) | | ... | +| *access_identities* | Map of access identities used for basic access roles. View identities have the format 'project_id|dataset_id|table_id'. | map(string) | | {} | | *dataset_access* | Set access in the dataset resource instead of using separate resources. | bool | | false | | *encryption_key* | Self link of the KMS key that will be used to protect destination table. | string | | null | | *friendly_name* | Dataset friendly name. | string | | null | +| *iam* | IAM bindings in {ROLE => [MEMBERS]} format. Mutually exclusive with the access_* variables used for basic roles. | map(list(string)) | | {} | | *labels* | Dataset labels. | map(string) | | {} | | *location* | Dataset location. | string | | EU | | *options* | Dataset options. | object({...}) | | ... | diff --git a/modules/bigquery-dataset/main.tf b/modules/bigquery-dataset/main.tf index 89a5db7c..fa64a638 100644 --- a/modules/bigquery-dataset/main.tf +++ b/modules/bigquery-dataset/main.tf @@ -15,20 +15,19 @@ */ locals { - access_domain = { - for k, v in var.access_roles : k => v if v.type == "domain" - } - access_group = { - for k, v in var.access_roles : k => v if v.type == "group_by_email" - } - access_special = { - for k, v in var.access_roles : k => v if v.type == "special_group" - } - access_user = { - for k, v in var.access_roles : k => v if v.type == "user_by_email" - } - access_view = { - for k, v in var.access_roles : k => v if v.type == "view" + access_domain = { for k, v in var.access : k => v if v.type == "domain" } + access_group = { for k, v in var.access : k => v if v.type == "group" } + access_special = { for k, v in var.access : k => v if v.type == "special_group" } + access_user = { for k, v in var.access : k => v if v.type == "user" } + access_view = { for k, v in var.access : k => v if v.type == "view" } + identities_view = { + for k, v in local.access_view : k => try( + zipmap( + ["project", "dataset", "table"], + split("|", var.access_identities[k]) + ), + { project = null, dataset = null, table = null } + ) } } @@ -80,9 +79,9 @@ resource "google_bigquery_dataset" "default" { for_each = var.dataset_access ? local.access_view : {} content { view { - project_id = try(var.access.views[access.key].project_id, null) - dataset_id = try(var.access.views[access.key].dataset_id, null) - table_id = try(var.access.views[access.key].table_id, null) + project_id = local.identities_view[access.key].project + dataset_id = local.identities_view[access.key].dataset + table_id = local.identities_view[access.key].table } } } @@ -95,7 +94,6 @@ resource "google_bigquery_dataset" "default" { } } - resource "google_bigquery_dataset_access" "domain" { for_each = var.dataset_access ? {} : local.access_domain provider = google-beta @@ -138,12 +136,19 @@ resource "google_bigquery_dataset_access" "views" { project = var.project_id dataset_id = google_bigquery_dataset.default.dataset_id view { - project_id = try(var.access_views[each.key].project_id, null) - dataset_id = try(var.access_views[each.key].dataset_id, null) - table_id = try(var.access_views[each.key].table_id, null) + project_id = local.identities_view[each.key].project + dataset_id = local.identities_view[each.key].dataset + table_id = local.identities_view[each.key].table } } +resource "google_bigquery_dataset_iam_binding" "bindings" { + for_each = var.iam + dataset_id = google_bigquery_dataset.default.dataset_id + role = each.key + members = each.value +} + resource "google_bigquery_table" "default" { provider = google-beta for_each = var.tables @@ -187,7 +192,6 @@ resource "google_bigquery_table" "default" { } - resource "google_bigquery_table" "views" { depends_on = [google_bigquery_table.default] for_each = var.views diff --git a/modules/bigquery-dataset/variables.tf b/modules/bigquery-dataset/variables.tf index a0aab6c3..c27dc5ea 100644 --- a/modules/bigquery-dataset/variables.tf +++ b/modules/bigquery-dataset/variables.tf @@ -14,29 +14,33 @@ * limitations under the License. */ -variable "access_identities" { - description = "Map of access identities used for access roles with type different from `view`. A separate variable is needed as identities can be set to dynamic values." - type = map(string) - default = {} -} - -variable "access_roles" { - description = "Map of access rules with role and identity type. Keys are arbitrary and only used to combine identities with each role. Valid types are `domain`, `group_by_email`, `special_group`, `user_by_email`, `view`." +variable "access" { + description = "Map of access rules with role and identity type. Keys are arbitrary and must match those in the `access_identities` variable, types are `domain`, `group`, `special_group`, `user`, `view`." type = map(object({ role = string type = string })) default = {} + validation { + condition = can([ + for k, v in var.access : + index(["OWNER", "READER", "WRITER"], v.role) + ]) + error_message = "Access role must be one of 'OWNER', 'READER', 'WRITER'." + } + validation { + condition = can([ + for k, v in var.access : + index(["domain", "group", "special_group", "user", "view"], v.type) + ]) + error_message = "Access type must be one of 'domain', 'group', 'special_group', 'user', 'view'." + } } -variable "access_views" { - description = "Map of view data for access roles with identity type equal to `view`. A separate variable is needed as identities can be set to dynamic values." - type = map(object({ - project_id = string - dataset_id = string - table_id = string - })) - default = {} +variable "access_identities" { + description = "Map of access identities used for basic access roles. View identities have the format 'project_id|dataset_id|table_id'." + type = map(string) + default = {} } variable "dataset_access" { @@ -51,6 +55,12 @@ variable "encryption_key" { default = null } +variable "iam" { + description = "IAM bindings in {ROLE => [MEMBERS]} format. Mutually exclusive with the access_* variables used for basic roles." + type = map(list(string)) + default = {} +} + variable "labels" { description = "Dataset labels." type = map(string) diff --git a/tests/data_solutions/gcs_to_bq_with_dataflow/test_plan.py b/tests/data_solutions/gcs_to_bq_with_dataflow/test_plan.py index 03fc27da..0fd6d92d 100644 --- a/tests/data_solutions/gcs_to_bq_with_dataflow/test_plan.py +++ b/tests/data_solutions/gcs_to_bq_with_dataflow/test_plan.py @@ -24,4 +24,4 @@ def test_resources(e2e_plan_runner): "Test that plan works and the numbers of resources is as expected." modules, resources = e2e_plan_runner(FIXTURES_DIR) assert len(modules) == 14 - assert len(resources) == 60 + assert len(resources) == 61