From de23a029afd4c0c6ed33a307bad23eaedebedef3 Mon Sep 17 00:00:00 2001 From: ocofaigh Date: Mon, 14 Oct 2024 19:25:52 +0100 Subject: [PATCH 1/8] cleanup --- README.md | 6 +- main.tf | 50 ++++-- outputs.tf | 6 + solutions/standard/main.tf | 289 ++++++++++++++++++++++---------- solutions/standard/outputs.tf | 4 +- solutions/standard/variables.tf | 10 +- variables.tf | 4 +- 7 files changed, 257 insertions(+), 112 deletions(-) diff --git a/README.md b/README.md index 30abdb59..c76597a7 100644 --- a/README.md +++ b/README.md @@ -91,7 +91,6 @@ To create service credentials, access the Event Notifications service, and acces | [time_sleep.wait_for_cos_authorization_policy](https://registry.terraform.io/providers/hashicorp/time/latest/docs/resources/sleep) | resource | | [time_sleep.wait_for_kms_authorization_policy](https://registry.terraform.io/providers/hashicorp/time/latest/docs/resources/sleep) | resource | | [ibm_en_integrations.en_integrations](https://registry.terraform.io/providers/IBM-Cloud/ibm/latest/docs/data-sources/en_integrations) | data source | -| [ibm_iam_account_settings.iam_account_settings](https://registry.terraform.io/providers/IBM-Cloud/ibm/latest/docs/data-sources/iam_account_settings) | data source | ### Inputs @@ -112,14 +111,15 @@ To create service credentials, access the Event Notifications service, and acces | [root\_key\_id](#input\_root\_key\_id) | The key ID of a root key, existing in the KMS instance passed in `var.existing_kms_instance_crn`, which will be used to encrypt the data encryption keys which are then used to encrypt the data. Required only if `var.kms_encryption_enabled` is set to `true`. | `string` | `null` | no | | [service\_credential\_names](#input\_service\_credential\_names) | The mapping of names and roles for service credentials that you want to create for the Event Notifications instance. | `map(string)` | `{}` | no | | [service\_endpoints](#input\_service\_endpoints) | Specify whether you want to enable public, or both public and private service endpoints. Possible values: `public`, `public-and-private` | `string` | `"public-and-private"` | no | -| [skip\_en\_cos\_auth\_policy](#input\_skip\_en\_cos\_auth\_policy) | Whether an IAM authorization policy is created for your Event Notifications instance to interact with your Object Storage bucket. Set to `true` to use an existing policy. Ignored if `cos_integration_enabled` is set to `false`. | `bool` | `false` | no | -| [skip\_en\_kms\_auth\_policy](#input\_skip\_en\_kms\_auth\_policy) | Set to `true` to skip the creation of an IAM authorization policy that permits all Event Notifications instances in the resource group to read the encryption key from the KMS instance. If set to `false`, specify a value for the KMS instance in the `existing_kms_instance_guid` variable. In addition, no policy is created if `kms_encryption_enabled` is set to `false`. | `bool` | `false` | no | +| [skip\_en\_cos\_auth\_policy](#input\_skip\_en\_cos\_auth\_policy) | Set to `true` to skip the creation of an IAM authorization policy that permits the Event Notifications instance `Object Writer` and `Reader` access to the given Object Storage bucket. Ignored if `cos_integration_enabled` is set to `false`. | `bool` | `false` | no | +| [skip\_en\_kms\_auth\_policy](#input\_skip\_en\_kms\_auth\_policy) | Set to `true` to skip the creation of an IAM authorization policy that permits the Event Notifications instance to read the encryption key from the KMS instance. If set to `false`, a value must be passed for the KMS instance and key using inputs `existing_kms_instance_crn` and `root_key_id`. In addition, no policy is created if `kms_encryption_enabled` is set to `false`. | `bool` | `false` | no | | [tags](#input\_tags) | The list of tags to add to the Event Notifications instance. | `list(string)` | `[]` | no | ### Outputs | Name | Description | |------|-------------| +| [account\_id](#output\_account\_id) | The Event Notifications account ID. | | [crn](#output\_crn) | The Event Notifications instance CRN. | | [event\_notification\_instance\_name](#output\_event\_notification\_instance\_name) | The name of the Event Notifications instance. | | [guid](#output\_guid) | The globally unique identifier of the Event Notifications instance. | diff --git a/main.tf b/main.tf index 2f776098..56f66b16 100644 --- a/main.tf +++ b/main.tf @@ -19,6 +19,9 @@ locals { can(regex(".*hs-crypto.*", var.existing_kms_instance_crn)) ? "hs-crypto" : null ) ) : null + + # Get account ID + account_id = ibm_resource_instance.en_instance.account_id } resource "ibm_resource_instance" "en_instance" { @@ -79,13 +82,6 @@ resource "ibm_en_integration" "en_kms_integration" { } } -############################################################################## -# Get Cloud Account ID -############################################################################## - -data "ibm_iam_account_settings" "iam_account_settings" { -} - ############################################################################## # IAM Authorization Policy ############################################################################## @@ -102,30 +98,26 @@ resource "ibm_iam_authorization_policy" "cos_policy" { source_resource_instance_id = ibm_resource_instance.en_instance.guid roles = ["Object Writer", "Reader"] description = "Allow EN instance with GUID ${ibm_resource_instance.en_instance.guid} `Object Writer` and `Reader` access to the COS instance with GUID ${local.existing_cos_instance_guid}." - resource_attributes { name = "serviceName" operator = "stringEquals" value = "cloud-object-storage" } - resource_attributes { name = "accountId" operator = "stringEquals" - value = data.ibm_iam_account_settings.iam_account_settings.account_id + value = local.account_id } resource_attributes { name = "serviceInstance" operator = "stringEquals" value = local.existing_cos_instance_guid } - resource_attributes { name = "resourceType" operator = "stringEquals" value = "bucket" } - resource_attributes { name = "resource" operator = "stringEquals" @@ -145,10 +137,38 @@ resource "ibm_iam_authorization_policy" "kms_policy" { count = var.kms_encryption_enabled == false || var.skip_en_kms_auth_policy ? 0 : 1 source_service_name = "event-notifications" source_resource_instance_id = ibm_resource_instance.en_instance.guid - target_service_name = local.kms_service - target_resource_instance_id = local.existing_kms_instance_guid roles = ["Reader"] - description = "Allow Event Notification instance ${ibm_resource_instance.en_instance.guid} to read from the ${local.kms_service} instance ${local.existing_kms_instance_guid}" + description = "Allow Event Notifications instance ${ibm_resource_instance.en_instance.guid} to read the ${local.kms_service} key ${var.root_key_id} from instance ${local.existing_kms_instance_guid}" + resource_attributes { + name = "serviceName" + operator = "stringEquals" + value = local.kms_service + } + resource_attributes { + name = "accountId" + operator = "stringEquals" + value = local.account_id + } + resource_attributes { + name = "serviceInstance" + operator = "stringEquals" + value = local.existing_kms_instance_guid + } + resource_attributes { + name = "resourceType" + operator = "stringEquals" + value = "key" + } + resource_attributes { + name = "resource" + operator = "stringEquals" + value = var.root_key_id + } + # Scope of policy now includes the key, so ensure to create new policy before + # destroying old one to prevent any disruption to every day services. + lifecycle { + create_before_destroy = true + } } # workaround for https://github.com/IBM-Cloud/terraform-provider-ibm/issues/4478 diff --git a/outputs.tf b/outputs.tf index 3fb25ebf..d4891066 100644 --- a/outputs.tf +++ b/outputs.tf @@ -6,11 +6,17 @@ output "event_notification_instance_name" { description = "The name of the Event Notifications instance." value = ibm_resource_instance.en_instance.name } + output "crn" { description = "The Event Notifications instance CRN." value = ibm_resource_instance.en_instance.crn } +output "account_id" { + description = "The Event Notifications account ID." + value = local.account_id +} + output "guid" { description = "The globally unique identifier of the Event Notifications instance." value = ibm_resource_instance.en_instance.guid diff --git a/solutions/standard/main.tf b/solutions/standard/main.tf index face1a89..2fe49bc3 100644 --- a/solutions/standard/main.tf +++ b/solutions/standard/main.tf @@ -2,6 +2,7 @@ # Resource Group ######################################################################################################################## +# Create new resource group, or take in existing group module "resource_group" { count = var.existing_en_instance_crn == null ? 1 : 0 source = "terraform-ibm-modules/resource-group/ibm" @@ -14,78 +15,154 @@ module "resource_group" { # KMS keys ####################################################################################################################### +# Input variable validation locals { - # If a KMS key is passed, or an existing EN CRN is passed; do not create keys - create_kms_keys = var.existing_kms_root_key_crn != null || var.existing_en_instance_crn != null ? false : true - parsed_existing_kms_root_key_crn = var.existing_kms_root_key_crn != null ? split(":", var.existing_kms_root_key_crn) : [] - existing_kms_root_key_id = length(local.parsed_existing_kms_root_key_crn) > 0 ? local.parsed_existing_kms_root_key_crn[length(local.parsed_existing_kms_root_key_crn) - 1] : null - parsed_existing_kms_instance_crn = var.existing_kms_instance_crn != null ? split(":", var.existing_kms_instance_crn) : [] - kms_region = length(local.parsed_existing_kms_instance_crn) > 0 ? local.parsed_existing_kms_instance_crn[5] : null - kms_instance_guid = var.existing_kms_instance_crn != null ? element(split(":", var.existing_kms_instance_crn), length(split(":", var.existing_kms_instance_crn)) - 3) : module.kms[0].kms_instance_guid - create_cross_account_auth_policy = (!var.skip_en_kms_auth_policy || !var.skip_cos_kms_auth_policy) && var.ibmcloud_kms_api_key != null && var.existing_cos_instance_crn != null - existing_kms_guid = var.existing_kms_instance_crn != null ? element(split(":", var.existing_kms_instance_crn), length(split(":", var.existing_kms_instance_crn)) - 3) : tobool("The CRN of the existing KMS is not provided.") - en_key_name = var.prefix != null ? "${var.prefix}-${var.en_key_name}" : var.en_key_name - en_key_ring_name = var.prefix != null ? "${var.prefix}-${var.en_key_ring_name}" : var.en_key_ring_name - en_kms_key_id = local.existing_kms_root_key_id != null ? local.existing_kms_root_key_id : var.existing_en_instance_crn == null ? module.kms[0].keys[format("%s.%s", local.en_key_ring_name, local.en_key_name)].key_id : null - cos_key_name = var.prefix != null ? "${var.prefix}-${var.cos_key_name}" : var.cos_key_name - cos_key_ring_name = var.prefix != null ? "${var.prefix}-${var.cos_key_ring_name}" : var.cos_key_ring_name - cos_instance_guid = var.existing_cos_instance_crn != null ? element(split(":", var.existing_cos_instance_crn), length(split(":", var.existing_cos_instance_crn)) - 3) : null - cos_kms_key_crn = var.existing_cos_bucket_name != null ? null : var.existing_kms_root_key_crn != null ? var.existing_kms_root_key_crn : var.existing_en_instance_crn == null ? module.kms[0].keys[format("%s.%s", local.cos_key_ring_name, local.cos_key_name)].crn : null - - existing_en_instance_guid = var.existing_en_instance_crn != null ? element(split(":", var.existing_en_instance_crn), length(split(":", var.existing_en_instance_crn)) - 3) : null - use_existing_en_instance = var.existing_en_instance_crn != null - - eventnotification_guid = local.use_existing_en_instance ? data.ibm_resource_instance.existing_en_instance[0].guid : module.event_notifications[0].guid - - kms_service_name = var.existing_kms_instance_crn != null ? ( - can(regex(".*kms.*", var.existing_kms_instance_crn)) ? "kms" : ( - can(regex(".*hs-crypto.*", var.existing_kms_instance_crn)) ? "hs-crypto" : null - ) - ) : null - + # Validate that a value has been passed for 'existing_kms_instance_crn' if not using existing EN instance # tflint-ignore: terraform_unused_declarations - validate_sm_crn = length(local.service_credential_secrets) > 0 && var.existing_secrets_manager_instance_crn == null ? tobool("`existing_secrets_manager_instance_crn` is required when adding service credentials to a secrets manager secret.") : false + validate_kms_input = var.existing_kms_instance_crn == null && var.existing_en_instance_crn == null ? tobool("A value for 'existing_kms_instance_crn' must be passed when no value is passed for 'existing_en_instance_crn'.") : true +} +# If existing KMS root key CRN passed, parse details from it +module "kms_root_key_crn_parser" { + count = var.existing_kms_root_key_crn != null ? 1 : 0 + source = "terraform-ibm-modules/common-utilities/ibm//modules/crn-parser" + version = "1.0.0" + crn = var.existing_kms_root_key_crn +} - existing_secrets_manager_instance_crn_split = var.existing_secrets_manager_instance_crn != null ? split(":", var.existing_secrets_manager_instance_crn) : null - existing_secrets_manager_instance_guid = var.existing_secrets_manager_instance_crn != null ? element(local.existing_secrets_manager_instance_crn_split, length(local.existing_secrets_manager_instance_crn_split) - 3) : null - existing_secrets_manager_instance_region = var.existing_secrets_manager_instance_crn != null ? element(local.existing_secrets_manager_instance_crn_split, length(local.existing_secrets_manager_instance_crn_split) - 5) : null +# If existing KMS intance CRN passed, parse details from it +module "kms_instance_crn_parser" { + count = var.existing_kms_instance_crn != null ? 1 : 0 + source = "terraform-ibm-modules/common-utilities/ibm//modules/crn-parser" + version = "1.0.0" + crn = var.existing_kms_instance_crn +} +# If not using an existing COS bucket, or an existing EN instance, parse details from the KMS key CRN used for COS +module "cos_kms_key_crn_parser" { + count = var.existing_cos_bucket_name == null || var.existing_en_instance_crn == null ? 1 : 0 + source = "terraform-ibm-modules/common-utilities/ibm//modules/crn-parser" + version = "1.0.0" + crn = local.cos_kms_key_crn } -# Data source to account settings for retrieving cross account id -data "ibm_iam_account_settings" "iam_account_settings" { - count = local.create_cross_account_auth_policy ? 1 : 0 +locals { + # If an existing KMS root key, or an existing EN instance is passed, do not create a new KMS root key + create_kms_keys = var.existing_kms_root_key_crn != null || var.existing_en_instance_crn != null ? false : true + # If existing KMS root key CRN passed, parse the ID from it + existing_en_kms_root_key_id = var.existing_kms_root_key_crn != null ? module.kms_root_key_crn_parser[0].resource : null + # Determine the KMS root key ID value (new key or existing key) + en_kms_key_id = local.existing_en_kms_root_key_id != null ? local.existing_en_kms_root_key_id : var.existing_en_instance_crn == null ? module.kms[0].keys[format("%s.%s", local.en_key_ring_name, local.en_key_name)].key_id : null + # If existing KMS instance CRN passed, parse the region from it + kms_region = var.existing_kms_instance_crn != null ? module.kms_instance_crn_parser[0].region : null + # If existing KMS instance CRN passed, parse the GUID from it + kms_instance_guid = var.existing_kms_instance_crn != null ? module.kms_instance_crn_parser[0].service_instance : null + # If existing KMS instance CRN passed, parse the service name from it + kms_service_name = var.existing_kms_instance_crn != null ? module.kms_instance_crn_parser[0].service_name : null + # If existing KMS instance CRN passed, parse the account ID from it + # TODO: update logic once CRN parser supports outputting account id (tracked in https://github.com/terraform-ibm-modules/terraform-ibm-common-utilities/issues/17) + kms_account_id = var.existing_kms_instance_crn != null ? split("/", module.kms_instance_crn_parser[0].scope)[1] : null + # Create cross account EN / KMS auth policy if not using existing EN instance, if 'skip_en_kms_auth_policy' is false, and a value is passed for 'ibmcloud_kms_api_key' + create_cross_account_en_kms_auth_policy = var.existing_en_instance_crn == null && !var.skip_en_kms_auth_policy && var.ibmcloud_kms_api_key != null + # Create cross account COS / KMS auth policy if not using existing EN instance, if not using existing bucket, if 'skip_cos_kms_auth_policy' is false, and if a value is passed for 'ibmcloud_kms_api_key' + create_cross_account_cos_kms_auth_policy = var.existing_en_instance_crn == null && var.existing_cos_bucket_name == null && !var.skip_cos_kms_auth_policy && var.ibmcloud_kms_api_key != null + # If a prefix value is passed, add it to the EN key name + en_key_name = var.prefix != null ? "${var.prefix}-${var.en_key_name}" : var.en_key_name + # If a prefix value is passed, add it to the EN key ring name + en_key_ring_name = var.prefix != null ? "${var.prefix}-${var.en_key_ring_name}" : var.en_key_ring_name + # If a prefix value is passed, add it to the COS key name + cos_key_name = var.prefix != null ? "${var.prefix}-${var.cos_key_name}" : var.cos_key_name + # If a prefix value is passed, add it to the COS key ring name + cos_key_ring_name = var.prefix != null ? "${var.prefix}-${var.cos_key_ring_name}" : var.cos_key_ring_name + # Determine the COS KMS key CRN (new key or existing key). It will only have a value if not using an existing bucket or existing EN instance + cos_kms_key_crn = var.existing_cos_bucket_name != null ? null : var.existing_kms_root_key_crn != null ? var.existing_kms_root_key_crn : var.existing_en_instance_crn == null ? module.kms[0].keys[format("%s.%s", local.cos_key_ring_name, local.cos_key_name)].crn : null + # If existing KMS instance CRN passed, parse the account ID from it + cos_kms_key_id = local.cos_kms_key_crn != null ? module.cos_kms_key_crn_parser[0].service_instance : null } -# Create IAM Authorization Policy to allow COS to access KMS for the encryption key +# Create cross account IAM Authorization Policy to allow COS to read the KMS encryption key resource "ibm_iam_authorization_policy" "cos_kms_policy" { - count = local.create_cross_account_auth_policy ? 1 : 0 + count = local.create_cross_account_cos_kms_auth_policy ? 1 : 0 provider = ibm.kms - source_service_account = data.ibm_iam_account_settings.iam_account_settings[0].account_id + source_service_account = local.cos_account_id source_service_name = "cloud-object-storage" source_resource_instance_id = local.cos_instance_guid - target_service_name = local.kms_service_name - target_resource_instance_id = local.existing_kms_guid roles = ["Reader"] - description = "Allow the COS instance with GUID ${local.cos_instance_guid} to read from the ${local.kms_service_name} instance GUID ${local.existing_kms_guid}" + description = "Allow the COS instance ${local.cos_instance_guid} to read the ${local.kms_service_name} key ${local.cos_kms_key_id} from the instance ${local.kms_instance_guid}" + resource_attributes { + name = "serviceName" + operator = "stringEquals" + value = local.kms_service_name + } + resource_attributes { + name = "accountId" + operator = "stringEquals" + value = local.kms_account_id + } + resource_attributes { + name = "serviceInstance" + operator = "stringEquals" + value = local.kms_instance_guid + } + resource_attributes { + name = "resourceType" + operator = "stringEquals" + value = "key" + } + resource_attributes { + name = "resource" + operator = "stringEquals" + value = local.cos_kms_key_id + } + # Scope of policy now includes the key, so ensure to create new policy before + # destroying old one to prevent any disruption to every day services. + lifecycle { + create_before_destroy = true + } } -# Create IAM Authorization Policy to allow EN to access KMS for the encryption key +# Create cross account IAM Authorization Policy to allow EN to read the KMS encryption key resource "ibm_iam_authorization_policy" "en_kms_policy" { - count = local.create_cross_account_auth_policy ? 1 : 0 + count = local.create_cross_account_en_kms_auth_policy ? 1 : 0 provider = ibm.kms - source_service_account = data.ibm_iam_account_settings.iam_account_settings[0].account_id + source_service_account = module.event_notifications[0].account_id source_service_name = "event-notifications" source_resource_instance_id = module.event_notifications[0].guid - target_service_name = local.kms_service_name - target_resource_instance_id = local.existing_kms_guid roles = ["Reader"] - description = "Allow the EN instance with GUID ${module.event_notifications[0].guid} reader access to the ${local.kms_service_name} instance GUID ${local.existing_kms_guid}" - + description = "Allow the EN instance with GUID ${module.event_notifications[0].guid} to read the ${local.kms_service_name} key ${local.cos_kms_key_id} from the instance ${local.kms_instance_guid}}" + resource_attributes { + name = "serviceName" + operator = "stringEquals" + value = local.kms_service_name + } + resource_attributes { + name = "accountId" + operator = "stringEquals" + value = local.kms_account_id + } + resource_attributes { + name = "serviceInstance" + operator = "stringEquals" + value = local.kms_instance_guid + } + resource_attributes { + name = "resourceType" + operator = "stringEquals" + value = "key" + } + resource_attributes { + name = "resource" + operator = "stringEquals" + value = local.cos_kms_key_id + } + # Scope of policy now includes the key, so ensure to create new policy before + # destroying old one to prevent any disruption to every day services. + lifecycle { + create_before_destroy = true + } } -# KMS root key for Event Notifications +# Create KMS root keys module "kms" { providers = { ibm = ibm.kms @@ -134,26 +211,41 @@ module "kms" { # COS ####################################################################################################################### +# If existing COS intance CRN passed, parse details from it +module "cos_instance_crn_parser" { + count = var.existing_cos_instance_crn != null ? 1 : 0 + source = "terraform-ibm-modules/common-utilities/ibm//modules/crn-parser" + version = "1.0.0" + crn = var.existing_cos_instance_crn +} + locals { - # If a bucket name is passed, or an existing EN CRN is passed; do not create bucket (or instance) - create_cos_bucket = var.existing_cos_bucket_name != null || var.existing_en_instance_crn != null ? false : true + # Validate mutually exclusive inputs # tflint-ignore: terraform_unused_declarations - validate_cos_regions = var.cos_bucket_region != null && var.cross_region_location != null ? tobool("Cannot provide values for var.cos_bucket_region and var.cross_region_location") : true - + validate_cos_regions = var.cos_bucket_region != null && var.cross_region_location != null ? tobool("Cannot provide values for 'cos_bucket_region' and 'cross_region_location'. Pick one or the other, or alternatively pass no values for either and allow it to default to the 'region' input.") : true + # If a bucket name is passed, or an existing EN CRN is passed; do not create COS resources + create_cos_bucket = var.existing_cos_bucket_name != null || var.existing_en_instance_crn != null ? false : true + # determine COS details cos_bucket_name = var.existing_cos_bucket_name != null ? var.existing_cos_bucket_name : local.create_cos_bucket ? (var.prefix != null ? "${var.prefix}-${var.cos_bucket_name}" : var.cos_bucket_name) : null cos_bucket_name_with_suffix = var.existing_cos_bucket_name != null ? var.existing_cos_bucket_name : local.create_cos_bucket ? module.cos[0].bucket_name : null cos_bucket_region = var.cos_bucket_region != null ? var.cos_bucket_region : var.cross_region_location != null ? null : var.region cos_instance_name = var.prefix != null ? "${var.prefix}-${var.cos_instance_name}" : var.cos_instance_name + cos_endpoint = var.existing_cos_bucket_name == null ? (local.create_cos_bucket ? "https://${module.cos[0].s3_endpoint_direct}" : null) : var.existing_cos_endpoint + # If existing COS instance CRN passed, parse the GUID from it, otherwise get GUID from COS module output + cos_instance_guid = var.existing_cos_instance_crn == null ? module.cos[0].cos_instance_guid : module.cos_instance_crn_parser[0].service_instance + # Parse the COS account ID from the CRN + # TODO: update logic once CRN parser supports outputting account id (tracked in https://github.com/terraform-ibm-modules/terraform-ibm-common-utilities/issues/17) + cos_account_id = var.existing_cos_instance_crn != null ? split("/", module.cos_instance_crn_parser[0].scope)[1] : module.cos[0].cos_account_id } module "cos" { count = local.create_cos_bucket ? 1 : 0 source = "terraform-ibm-modules/cos/ibm" - version = "8.11.15" + version = "8.12.0" create_cos_instance = var.existing_cos_instance_crn == null ? true : false create_cos_bucket = local.create_cos_bucket existing_cos_instance_id = var.existing_cos_instance_crn - skip_iam_authorization_policy = local.create_cross_account_auth_policy || var.skip_cos_kms_auth_policy + skip_iam_authorization_policy = local.create_cross_account_en_kms_auth_policy || local.create_cross_account_cos_kms_auth_policy || var.skip_cos_kms_auth_policy add_bucket_name_suffix = var.add_bucket_name_suffix resource_group_id = module.resource_group[0].resource_group_id region = local.cos_bucket_region @@ -177,17 +269,29 @@ module "cos" { # Event Notifications ######################################################################################################################## +# If existing EN intance CRN passed, parse details from it +module "existing_en_crn_parser" { + count = var.existing_en_instance_crn != null ? 1 : 0 + source = "terraform-ibm-modules/common-utilities/ibm//modules/crn-parser" + version = "1.0.0" + crn = var.existing_en_instance_crn +} + locals { - # COS Related - cos_endpoint = var.existing_cos_bucket_name == null ? (local.create_cos_bucket ? "https://${module.cos[0].s3_endpoint_public}" : null) : var.existing_cos_endpoint - # Event Notification Related - parsed_existing_en_instance_crn = var.existing_en_instance_crn != null ? split(":", var.existing_en_instance_crn) : [] - existing_en_guid = length(local.parsed_existing_en_instance_crn) > 0 ? local.parsed_existing_en_instance_crn[7] : null + # determine if existing EN instance being used + use_existing_en_instance = var.existing_en_instance_crn != null + # if using existing EN instance, parse the GUID from it + existing_en_instance_guid = local.use_existing_en_instance ? module.existing_en_crn_parser[0].service_instance : null + # determine the EN GUID + eventnotification_guid = local.use_existing_en_instance ? local.existing_en_instance_guid : module.event_notifications[0].guid + # determine the EN CRN + eventnotification_crn = local.use_existing_en_instance ? var.existing_en_instance_crn : module.event_notifications[0].crn } -data "ibm_resource_instance" "existing_en" { - count = var.existing_en_instance_crn == null ? 0 : 1 - identifier = var.existing_en_instance_crn +# Lookup instance if using an existing one +data "ibm_resource_instance" "existing_en_instance" { + count = local.use_existing_en_instance ? 1 : 0 + identifier = local.existing_en_instance_guid } module "event_notifications" { @@ -205,7 +309,7 @@ module "event_notifications" { kms_endpoint_url = var.kms_endpoint_url existing_kms_instance_crn = var.existing_kms_instance_crn root_key_id = local.en_kms_key_id - skip_en_kms_auth_policy = local.create_cross_account_auth_policy || var.skip_en_kms_auth_policy + skip_en_kms_auth_policy = local.create_cross_account_en_kms_auth_policy || local.create_cross_account_cos_kms_auth_policy || var.skip_en_kms_auth_policy # COS Related cos_integration_enabled = true cos_bucket_name = local.cos_bucket_name_with_suffix @@ -214,24 +318,27 @@ module "event_notifications" { cos_endpoint = local.cos_endpoint } -# create a service authorization between Secrets Manager and the target service (Event Notification) -resource "ibm_iam_authorization_policy" "secrets_manager_key_manager" { - count = var.skip_en_sm_auth_policy || var.existing_secrets_manager_instance_crn == null ? 0 : 1 - source_service_name = "secrets-manager" - source_resource_instance_id = local.existing_secrets_manager_instance_guid - target_service_name = "event-notifications" - target_resource_instance_id = local.eventnotification_guid - roles = ["Key Manager"] - description = "Allow Secrets Manager instance to manage key for the event-notification instance" -} +######################################################################################################################## +# Service Credentials +######################################################################################################################## -# workaround for https://github.com/IBM-Cloud/terraform-provider-ibm/issues/4478 -resource "time_sleep" "wait_for_en_authorization_policy" { - depends_on = [ibm_iam_authorization_policy.secrets_manager_key_manager] - create_duration = "30s" +# If existing EN intance CRN passed, parse details from it +module "existing_sm_crn_parser" { + count = var.existing_secrets_manager_instance_crn != null ? 1 : 0 + source = "terraform-ibm-modules/common-utilities/ibm//modules/crn-parser" + version = "1.0.0" + crn = var.existing_secrets_manager_instance_crn } locals { + # Validate that a value has been passed for 'existing_secrets_manager_instance_crn' if creating credentials using the 'service_credential_secrets' input + # tflint-ignore: terraform_unused_declarations + validate_sm_crn = length(var.service_credential_secrets) > 0 && var.existing_secrets_manager_instance_crn == null ? tobool("'existing_secrets_manager_instance_crn' is required when adding service credentials with the 'service_credential_secrets' input.") : false + # parse SM GUID from CRN + existing_secrets_manager_instance_guid = var.existing_secrets_manager_instance_crn != null ? module.existing_sm_crn_parser[0].service_instance : null + # parse SM regiob from CRN + existing_secrets_manager_instance_region = var.existing_secrets_manager_instance_crn != null ? module.existing_sm_crn_parser[0].region : null + # generate list of service credential secrets to create service_credential_secrets = [ for service_credentials in var.service_credential_secrets : { secret_group_name = service_credentials.secret_group_name @@ -247,7 +354,7 @@ locals { service_credentials_ttl = secret.service_credentials_ttl service_credential_secret_description = secret.service_credential_secret_description service_credentials_source_service_role = secret.service_credentials_source_service_role - service_credentials_source_service_crn = local.use_existing_en_instance ? data.ibm_resource_instance.existing_en_instance[0].id : module.event_notifications[0].crn + service_credentials_source_service_crn = local.eventnotification_crn secret_type = "service_credentials" #checkov:skip=CKV_SECRET_6 } ] @@ -255,6 +362,23 @@ locals { ] } +# create a service authorization between Secrets Manager and the target service (Event Notification) +resource "ibm_iam_authorization_policy" "secrets_manager_key_manager" { + count = var.skip_en_sm_auth_policy || var.existing_secrets_manager_instance_crn == null ? 0 : 1 + source_service_name = "secrets-manager" + source_resource_instance_id = local.existing_secrets_manager_instance_guid + target_service_name = "event-notifications" + target_resource_instance_id = local.eventnotification_guid + roles = ["Key Manager"] + description = "Allow Secrets Manager instance to manage key for the event-notification instance" +} + +# workaround for https://github.com/IBM-Cloud/terraform-provider-ibm/issues/4478 +resource "time_sleep" "wait_for_en_authorization_policy" { + depends_on = [ibm_iam_authorization_policy.secrets_manager_key_manager] + create_duration = "30s" +} + module "secrets_manager_service_credentials" { count = length(local.service_credential_secrets) > 0 ? 1 : 0 depends_on = [time_sleep.wait_for_en_authorization_policy] @@ -265,10 +389,3 @@ module "secrets_manager_service_credentials" { endpoint_type = var.existing_secrets_manager_endpoint_type secrets = local.service_credential_secrets } - -# this extra block is needed when passing in an existing EN instance - the resource data block -# requires an id to retrieve the data -data "ibm_resource_instance" "existing_en_instance" { - count = local.use_existing_en_instance ? 1 : 0 - identifier = local.existing_en_instance_guid -} diff --git a/solutions/standard/outputs.tf b/solutions/standard/outputs.tf index c4e1cf56..79a54881 100644 --- a/solutions/standard/outputs.tf +++ b/solutions/standard/outputs.tf @@ -4,7 +4,7 @@ output "event_notification_instance_name" { description = "Event Notification name" - value = var.existing_en_instance_crn == null ? module.event_notifications[0].event_notification_instance_name : data.ibm_resource_instance.existing_en[0].name + value = var.existing_en_instance_crn == null ? module.event_notifications[0].event_notification_instance_name : data.ibm_resource_instance.existing_en_instance[0].name } output "crn" { @@ -14,7 +14,7 @@ output "crn" { output "guid" { description = "Event Notification guid" - value = var.existing_en_instance_crn == null ? module.event_notifications[0].guid : local.existing_en_guid + value = local.eventnotification_guid } output "service_credentials_json" { diff --git a/solutions/standard/variables.tf b/solutions/standard/variables.tf index 5bba2dc9..13cedf85 100644 --- a/solutions/standard/variables.tf +++ b/solutions/standard/variables.tf @@ -98,7 +98,8 @@ variable "existing_en_instance_crn" { variable "existing_kms_instance_crn" { type = string - description = "The CRN of the KMS instance (Hyper Protect Crypto Services or Key Protect instance). If the KMS instance is in different account you must also provide a value for `ibmcloud_kms_api_key`." + description = "The CRN of the KMS instance (Hyper Protect Crypto Services or Key Protect instance). If the KMS instance is in different account you must also provide a value for `ibmcloud_kms_api_key`. Not required if passing an existing instance using the `existing_en_instance_crn` input." + default = null } variable "existing_kms_root_key_crn" { @@ -109,7 +110,8 @@ variable "existing_kms_root_key_crn" { variable "kms_endpoint_url" { type = string - description = "The KMS endpoint URL to use when you configure KMS encryption. The Hyper Protect Crypto Services endpoint URL format is `https://api.private..hs-crypto.cloud.ibm.com:` and the Key Protect endpoint URL format is `https://.kms.cloud.ibm.com`. Only required if not passing existing key." + description = "The KMS endpoint URL to use when you configure KMS encryption. The Hyper Protect Crypto Services endpoint URL format is `https://api.private..hs-crypto.cloud.ibm.com:` and the Key Protect endpoint URL format is `https://.kms.cloud.ibm.com`. Not required if passing an existing instance using the `existing_en_instance_crn` input, or an existing key using the `existing_kms_root_key_crn` input." + default = null } variable "kms_endpoint_type" { @@ -148,7 +150,7 @@ variable "cos_key_name" { variable "skip_en_kms_auth_policy" { type = bool - description = "Set to true to skip the creation of an IAM authorization policy that permits the Event Notification instance to read the encryption key from the KMS instance. If set to false, pass in a value for the KMS instance in the `existing_kms_instance_crn` variable. If a value is specified for `ibmcloud_kms_api_key`, the policy is created in the KMS account." + description = "Set to true to skip the creation of an IAM authorization policy that permits the Event Notifications instance to read the encryption key from the KMS instance. If a value is specified for `ibmcloud_kms_api_key`, the policy is created in the KMS account." default = false } @@ -185,7 +187,7 @@ variable "cos_bucket_name" { variable "skip_en_cos_auth_policy" { type = bool - description = "Whether an IAM authorization policy is created for your Event Notifications instance to interact with your Object Storage bucket. Set to `true` to use an existing policy. Ignored if `cos_integration_enabled` is set to `false`." + description = "Set to `true` to skip the creation of an IAM authorization policy that permits the Event Notifications instance `Object Writer` and `Reader` access to the given Object Storage bucket. Set to `true` to use an existing policy." default = false } diff --git a/variables.tf b/variables.tf index 0587bf79..9d5d0a75 100644 --- a/variables.tf +++ b/variables.tf @@ -88,7 +88,7 @@ variable "cbr_rules" { variable "skip_en_kms_auth_policy" { type = bool - description = "Set to `true` to skip the creation of an IAM authorization policy that permits all Event Notifications instances in the resource group to read the encryption key from the KMS instance. If set to `false`, specify a value for the KMS instance in the `existing_kms_instance_guid` variable. In addition, no policy is created if `kms_encryption_enabled` is set to `false`." + description = "Set to `true` to skip the creation of an IAM authorization policy that permits the Event Notifications instance to read the encryption key from the KMS instance. If set to `false`, a value must be passed for the KMS instance and key using inputs `existing_kms_instance_crn` and `root_key_id`. In addition, no policy is created if `kms_encryption_enabled` is set to `false`." default = false } @@ -100,7 +100,7 @@ variable "kms_encryption_enabled" { variable "skip_en_cos_auth_policy" { type = bool - description = "Whether an IAM authorization policy is created for your Event Notifications instance to interact with your Object Storage bucket. Set to `true` to use an existing policy. Ignored if `cos_integration_enabled` is set to `false`." + description = "Set to `true` to skip the creation of an IAM authorization policy that permits the Event Notifications instance `Object Writer` and `Reader` access to the given Object Storage bucket. Ignored if `cos_integration_enabled` is set to `false`." default = false } From 60ee5c7563c9966b4914876800b426c0adbfae92 Mon Sep 17 00:00:00 2001 From: ocofaigh Date: Mon, 14 Oct 2024 19:35:02 +0100 Subject: [PATCH 2/8] update desc --- solutions/standard/variables.tf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/solutions/standard/variables.tf b/solutions/standard/variables.tf index 13cedf85..8cb09d4d 100644 --- a/solutions/standard/variables.tf +++ b/solutions/standard/variables.tf @@ -110,7 +110,7 @@ variable "existing_kms_root_key_crn" { variable "kms_endpoint_url" { type = string - description = "The KMS endpoint URL to use when you configure KMS encryption. The Hyper Protect Crypto Services endpoint URL format is `https://api.private..hs-crypto.cloud.ibm.com:` and the Key Protect endpoint URL format is `https://.kms.cloud.ibm.com`. Not required if passing an existing instance using the `existing_en_instance_crn` input, or an existing key using the `existing_kms_root_key_crn` input." + description = "The KMS endpoint URL to use when you configure KMS encryption. The Hyper Protect Crypto Services endpoint URL format is `https://api.private..hs-crypto.cloud.ibm.com:` and the Key Protect endpoint URL format is `https://.kms.cloud.ibm.com`. Not required if passing an existing instance using the `existing_en_instance_crn` input." default = null } From 46c9d9b0964f89f9827a881a34f4f3052bb1a750 Mon Sep 17 00:00:00 2001 From: ocofaigh Date: Mon, 14 Oct 2024 21:13:29 +0100 Subject: [PATCH 3/8] SKIP UPGRADE TEST - see pr comments --- .secrets.baseline | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.secrets.baseline b/.secrets.baseline index c7ee6368..675a23b8 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -3,7 +3,7 @@ "files": "go.sum|^.secrets.baseline$", "lines": null }, - "generated_at": "2023-12-12T05:17:42Z", + "generated_at": "2023-12-13T05:17:42Z", "plugins_used": [ { "name": "AWSKeyDetector" From 48d7cf04b8dd3397624a8fbcab5db35c3e7a6aa0 Mon Sep 17 00:00:00 2001 From: ocofaigh Date: Tue, 15 Oct 2024 11:01:21 +0100 Subject: [PATCH 4/8] update tests --- solutions/standard/main.tf | 8 ++--- tests/existing-resources/main.tf | 10 ++++++ tests/existing-resources/outputs.tf | 10 ++++++ tests/pr_test.go | 51 ++++++++++++++++++++++++----- 4 files changed, 67 insertions(+), 12 deletions(-) diff --git a/solutions/standard/main.tf b/solutions/standard/main.tf index 2fe49bc3..4e3630c3 100644 --- a/solutions/standard/main.tf +++ b/solutions/standard/main.tf @@ -231,11 +231,11 @@ locals { cos_bucket_region = var.cos_bucket_region != null ? var.cos_bucket_region : var.cross_region_location != null ? null : var.region cos_instance_name = var.prefix != null ? "${var.prefix}-${var.cos_instance_name}" : var.cos_instance_name cos_endpoint = var.existing_cos_bucket_name == null ? (local.create_cos_bucket ? "https://${module.cos[0].s3_endpoint_direct}" : null) : var.existing_cos_endpoint - # If existing COS instance CRN passed, parse the GUID from it, otherwise get GUID from COS module output - cos_instance_guid = var.existing_cos_instance_crn == null ? module.cos[0].cos_instance_guid : module.cos_instance_crn_parser[0].service_instance - # Parse the COS account ID from the CRN + # If not using existing EN instance, and if existing COS instance CRN passed, parse the GUID from it, otherwise get GUID from COS module output + cos_instance_guid = var.existing_en_instance_crn == null ? var.existing_cos_instance_crn == null ? module.cos[0].cos_instance_guid : module.cos_instance_crn_parser[0].service_instance : null + # If not using existing EN instance, parse the COS account ID from the CRN # TODO: update logic once CRN parser supports outputting account id (tracked in https://github.com/terraform-ibm-modules/terraform-ibm-common-utilities/issues/17) - cos_account_id = var.existing_cos_instance_crn != null ? split("/", module.cos_instance_crn_parser[0].scope)[1] : module.cos[0].cos_account_id + cos_account_id = var.existing_en_instance_crn == null ? var.existing_cos_instance_crn != null ? split("/", module.cos_instance_crn_parser[0].scope)[1] : module.cos[0].cos_account_id : null } module "cos" { diff --git a/tests/existing-resources/main.tf b/tests/existing-resources/main.tf index f57c16b5..03e20d3a 100644 --- a/tests/existing-resources/main.tf +++ b/tests/existing-resources/main.tf @@ -15,3 +15,13 @@ module "event_notification" { region = var.region cos_integration_enabled = false } + +module "cos" { + source = "terraform-ibm-modules/cos/ibm" + version = "8.11.11" + resource_group_id = module.resource_group.resource_group_id + cos_instance_name = "${var.prefix}-cos" + cos_tags = var.resource_tags + create_cos_bucket = false + kms_encryption_enabled = false +} diff --git a/tests/existing-resources/outputs.tf b/tests/existing-resources/outputs.tf index 738779c3..37f3b14f 100644 --- a/tests/existing-resources/outputs.tf +++ b/tests/existing-resources/outputs.tf @@ -26,3 +26,13 @@ output "event_notification_instance_guid" { description = "Event Notification guid" value = module.event_notification.guid } + +output "cos_crn" { + description = "COS CRN" + value = module.cos.cos_instance_crn +} + +output "cos_crn" { + description = "COS CRN" + value = module.cos.cos_instance_crn +} diff --git a/tests/pr_test.go b/tests/pr_test.go index 2f70c8d6..2d432cf0 100644 --- a/tests/pr_test.go +++ b/tests/pr_test.go @@ -210,7 +210,6 @@ func TestRunUpgradeDASolution(t *testing.T) { "resource_group_name": options.Prefix, "region": region, "existing_kms_instance_crn": permanentResources["hpcs_south_crn"], - "existing_kms_root_key_crn": permanentResources["hpcs_south_root_key_crn"], "kms_endpoint_url": permanentResources["hpcs_south_private_endpoint"], "management_endpoint_type_for_bucket": "public", } @@ -259,6 +258,10 @@ func TestRunExistingResourcesInstances(t *testing.T) { var region = validRegions[rand.Intn(len(validRegions))] + // ------------------------------------------------------------------------------------ + // Deploy EN DA passing in existing RG and EN + // ------------------------------------------------------------------------------------ + options := testschematic.TestSchematicOptionsDefault(&testschematic.TestSchematicOptions{ Testing: t, Prefix: "en-exs-res", @@ -271,24 +274,56 @@ func TestRunExistingResourcesInstances(t *testing.T) { DeleteWorkspaceOnFail: false, WaitJobCompleteMinutes: 60, }) - options.TerraformVars = []testschematic.TestSchematicTerraformVar{ {Name: "ibmcloud_api_key", Value: options.RequiredEnvironmentVars["TF_VAR_ibmcloud_api_key"], DataType: "string", Secure: true}, - {Name: "ibmcloud_kms_api_key", Value: options.RequiredEnvironmentVars["TF_VAR_ibmcloud_api_key"], DataType: "string", Secure: true}, {Name: "region", Value: region, DataType: "string"}, {Name: "resource_group_name", Value: terraform.Output(t, existingTerraformOptions, "resource_group_name"), DataType: "string"}, - {Name: "existing_kms_instance_crn", Value: permanentResources["hpcs_south_crn"], DataType: "string"}, + {Name: "use_existing_resource_group", Value: true, DataType: "bool"}, {Name: "existing_en_instance_crn", Value: terraform.Output(t, existingTerraformOptions, "event_notification_instance_crn"), DataType: "string"}, - {Name: "kms_endpoint_url", Value: permanentResources["hpcs_south_private_endpoint"], DataType: "string"}, } err := options.RunSchematicTest() - if assert.NoError(t, err) { - t.Log("TestRunExistingResourcesInstances Passed") + t.Log("TestRunExistingResourcesInstances using existing RG and EN Passed") } else { - t.Error("TestRunExistingResourcesInstances Failed") + t.Error("TestRunExistingResourcesInstances using existing RG and EN Failed") } + + // ------------------------------------------------------------------------------------ + // Deploy EN DA passing in existing RG, COS instance (not bucket), and KMS key + // ------------------------------------------------------------------------------------ + + options2 := testschematic.TestSchematicOptionsDefault(&testschematic.TestSchematicOptions{ + Testing: t, + Prefix: "en-exs-res2", + TarIncludePatterns: []string{ + "*.tf", + solutionDADir + "/*.tf", + }, + TemplateFolder: solutionDADir, + Tags: []string{"test-schematic"}, + DeleteWorkspaceOnFail: false, + WaitJobCompleteMinutes: 60, + }) + options2.TerraformVars = []testschematic.TestSchematicTerraformVar{ + {Name: "ibmcloud_api_key", Value: options.RequiredEnvironmentVars["TF_VAR_ibmcloud_api_key"], DataType: "string", Secure: true}, + {Name: "ibmcloud_kms_api_key", Value: options.RequiredEnvironmentVars["TF_VAR_ibmcloud_api_key"], DataType: "string", Secure: true}, + {Name: "region", Value: region, DataType: "string"}, + {Name: "resource_group_name", Value: terraform.Output(t, existingTerraformOptions, "resource_group_name"), DataType: "string"}, + {Name: "use_existing_resource_group", Value: true, DataType: "bool"}, + {Name: "existing_kms_instance_crn", Value: permanentResources["hpcs_south_crn"], DataType: "string"}, + {Name: "existing_kms_root_key_crn", Value: permanentResources["hpcs_south_root_key_crn"], DataType: "string"}, + {Name: "kms_endpoint_url", Value: permanentResources["hpcs_south_private_endpoint"], DataType: "string"}, + {Name: "existing_cos_instance_crn", Value: terraform.Output(t, existingTerraformOptions, "cos_crn"), DataType: "string"}, + } + + err2 := options.RunSchematicTest() + if assert.NoError(t, err2) { + t.Log("TestRunExistingResourcesInstances using existing RG, COS instance, and KMS key Passed") + } else { + t.Error("TestRunExistingResourcesInstances using existing RG, COS instance, and KMS key Failed") + } + } // Check if "DO_NOT_DESTROY_ON_FAILURE" is set From b69600c23725fa39bdd704d87163db8c1ae05358 Mon Sep 17 00:00:00 2001 From: ocofaigh Date: Tue, 15 Oct 2024 19:06:07 +0100 Subject: [PATCH 5/8] add extra tests --- solutions/standard/main.tf | 75 ++++++++++++++++------------- tests/existing-resources/main.tf | 4 +- tests/existing-resources/outputs.tf | 11 +++-- tests/pr_test.go | 43 ++++++++++++----- 4 files changed, 84 insertions(+), 49 deletions(-) diff --git a/solutions/standard/main.tf b/solutions/standard/main.tf index 4e3630c3..60686671 100644 --- a/solutions/standard/main.tf +++ b/solutions/standard/main.tf @@ -40,7 +40,7 @@ module "kms_instance_crn_parser" { # If not using an existing COS bucket, or an existing EN instance, parse details from the KMS key CRN used for COS module "cos_kms_key_crn_parser" { - count = var.existing_cos_bucket_name == null || var.existing_en_instance_crn == null ? 1 : 0 + count = var.existing_cos_bucket_name == null && var.existing_en_instance_crn == null ? 1 : 0 source = "terraform-ibm-modules/common-utilities/ibm//modules/crn-parser" version = "1.0.0" crn = local.cos_kms_key_crn @@ -75,9 +75,42 @@ locals { # If a prefix value is passed, add it to the COS key ring name cos_key_ring_name = var.prefix != null ? "${var.prefix}-${var.cos_key_ring_name}" : var.cos_key_ring_name # Determine the COS KMS key CRN (new key or existing key). It will only have a value if not using an existing bucket or existing EN instance - cos_kms_key_crn = var.existing_cos_bucket_name != null ? null : var.existing_kms_root_key_crn != null ? var.existing_kms_root_key_crn : var.existing_en_instance_crn == null ? module.kms[0].keys[format("%s.%s", local.cos_key_ring_name, local.cos_key_name)].crn : null + # cos_kms_key_crn = var.existing_cos_bucket_name != null ? null : var.existing_kms_root_key_crn != null ? var.existing_kms_root_key_crn : var.existing_en_instance_crn == null ? module.kms[0].keys[format("%s.%s", local.cos_key_ring_name, local.cos_key_name)].crn : null + cos_kms_key_crn = var.existing_en_instance_crn != null || var.existing_cos_bucket_name != null ? null : var.existing_kms_root_key_crn != null ? var.existing_kms_root_key_crn : module.kms[0].keys[format("%s.%s", local.cos_key_ring_name, local.cos_key_name)].crn # If existing KMS instance CRN passed, parse the account ID from it - cos_kms_key_id = local.cos_kms_key_crn != null ? module.cos_kms_key_crn_parser[0].service_instance : null + cos_kms_key_id = local.cos_kms_key_crn != null ? module.cos_kms_key_crn_parser[0].resource : null + # Event Notifications KMS Key ring config + en_kms_key = { + key_ring_name = local.en_key_ring_name + existing_key_ring = false + force_delete_key_ring = true + keys = [ + { + key_name = local.en_key_name + standard_key = false + rotation_interval_month = 3 + dual_auth_delete_enabled = false + force_delete = true + } + ] + } + # Event Notifications COS bucket KMS Key ring config + en_cos_kms_key = { + key_ring_name = local.cos_key_ring_name + existing_key_ring = false + force_delete_key_ring = true + keys = [ + { + key_name = local.cos_key_name + standard_key = false + rotation_interval_month = 3 + dual_auth_delete_enabled = false + force_delete = true + } + ] + } + # If not using existing EN instance or KMS key, create Key. Don't create a COS KMS key if using existing COS bucket. + all_keys = local.create_kms_keys ? var.existing_cos_bucket_name != null ? [local.en_kms_key] : concat([local.en_kms_key], [local.en_cos_kms_key]) : [] } # Create cross account IAM Authorization Policy to allow COS to read the KMS encryption key @@ -175,36 +208,7 @@ module "kms" { existing_kms_instance_crn = var.existing_kms_instance_crn key_ring_endpoint_type = var.kms_endpoint_type key_endpoint_type = var.kms_endpoint_type - keys = [ - { - key_ring_name = local.en_key_ring_name - existing_key_ring = false - force_delete_key_ring = true - keys = [ - { - key_name = local.en_key_name - standard_key = false - rotation_interval_month = 3 - dual_auth_delete_enabled = false - force_delete = true - } - ] - }, - { - key_ring_name = local.cos_key_ring_name - existing_key_ring = false - force_delete_key_ring = true - keys = [ - { - key_name = local.cos_key_name - standard_key = false - rotation_interval_month = 3 - dual_auth_delete_enabled = false - force_delete = true - } - ] - } - ] + keys = local.all_keys } ####################################################################################################################### @@ -223,6 +227,11 @@ locals { # Validate mutually exclusive inputs # tflint-ignore: terraform_unused_declarations validate_cos_regions = var.cos_bucket_region != null && var.cross_region_location != null ? tobool("Cannot provide values for 'cos_bucket_region' and 'cross_region_location'. Pick one or the other, or alternatively pass no values for either and allow it to default to the 'region' input.") : true + + # Validate cos inputs when using existing bucket + # tflint-ignore: terraform_unused_declarations + validate_cos_bucket = var.existing_cos_bucket_name != null && (var.existing_cos_instance_crn == null || var.existing_cos_endpoint == null) ? tobool("When passing a value for 'existing_cos_bucket_name', you must also pass values for 'existing_cos_instance_crn' and 'existing_cos_endpoint'.") : true + # If a bucket name is passed, or an existing EN CRN is passed; do not create COS resources create_cos_bucket = var.existing_cos_bucket_name != null || var.existing_en_instance_crn != null ? false : true # determine COS details diff --git a/tests/existing-resources/main.tf b/tests/existing-resources/main.tf index 03e20d3a..93d3dcac 100644 --- a/tests/existing-resources/main.tf +++ b/tests/existing-resources/main.tf @@ -20,8 +20,10 @@ module "cos" { source = "terraform-ibm-modules/cos/ibm" version = "8.11.11" resource_group_id = module.resource_group.resource_group_id + region = var.region cos_instance_name = "${var.prefix}-cos" cos_tags = var.resource_tags - create_cos_bucket = false + bucket_name = "${var.prefix}-bucket" + retention_enabled = false kms_encryption_enabled = false } diff --git a/tests/existing-resources/outputs.tf b/tests/existing-resources/outputs.tf index 37f3b14f..fdcffde8 100644 --- a/tests/existing-resources/outputs.tf +++ b/tests/existing-resources/outputs.tf @@ -32,7 +32,12 @@ output "cos_crn" { value = module.cos.cos_instance_crn } -output "cos_crn" { - description = "COS CRN" - value = module.cos.cos_instance_crn +output "bucket_name" { + description = "COS bucket name" + value = module.cos.bucket_name +} + +output "s3_endpoint_direct" { + description = "COS bucket name" + value = module.cos.s3_endpoint_direct } diff --git a/tests/pr_test.go b/tests/pr_test.go index 2d432cf0..3e8ef30c 100644 --- a/tests/pr_test.go +++ b/tests/pr_test.go @@ -226,7 +226,7 @@ func TestRunExistingResourcesInstances(t *testing.T) { t.Parallel() // ------------------------------------------------------------------------------------ - // Provision RG, EN + // Provision existing resources first // ------------------------------------------------------------------------------------ prefix := fmt.Sprintf("en-existing-%s", strings.ToLower(random.UniqueId())) @@ -281,13 +281,8 @@ func TestRunExistingResourcesInstances(t *testing.T) { {Name: "use_existing_resource_group", Value: true, DataType: "bool"}, {Name: "existing_en_instance_crn", Value: terraform.Output(t, existingTerraformOptions, "event_notification_instance_crn"), DataType: "string"}, } - err := options.RunSchematicTest() - if assert.NoError(t, err) { - t.Log("TestRunExistingResourcesInstances using existing RG and EN Passed") - } else { - t.Error("TestRunExistingResourcesInstances using existing RG and EN Failed") - } + assert.NoError(t, err, "TestRunExistingResourcesInstances using existing RG and EN Failed") // ------------------------------------------------------------------------------------ // Deploy EN DA passing in existing RG, COS instance (not bucket), and KMS key @@ -316,13 +311,37 @@ func TestRunExistingResourcesInstances(t *testing.T) { {Name: "kms_endpoint_url", Value: permanentResources["hpcs_south_private_endpoint"], DataType: "string"}, {Name: "existing_cos_instance_crn", Value: terraform.Output(t, existingTerraformOptions, "cos_crn"), DataType: "string"}, } + err2 := options2.RunSchematicTest() + assert.NoError(t, err2, "TestRunExistingResourcesInstances using existing RG, COS instance, and KMS key Failed") + + // ------------------------------------------------------------------------------------ + // Deploy EN DA passing in existing RG, COS instance and bucket + // ------------------------------------------------------------------------------------ - err2 := options.RunSchematicTest() - if assert.NoError(t, err2) { - t.Log("TestRunExistingResourcesInstances using existing RG, COS instance, and KMS key Passed") - } else { - t.Error("TestRunExistingResourcesInstances using existing RG, COS instance, and KMS key Failed") + options3 := testschematic.TestSchematicOptionsDefault(&testschematic.TestSchematicOptions{ + Testing: t, + Prefix: "en-exs-res2", + TarIncludePatterns: []string{ + "*.tf", + solutionDADir + "/*.tf", + }, + TemplateFolder: solutionDADir, + Tags: []string{"test-schematic"}, + DeleteWorkspaceOnFail: false, + WaitJobCompleteMinutes: 60, + }) + options3.TerraformVars = []testschematic.TestSchematicTerraformVar{ + {Name: "ibmcloud_api_key", Value: options.RequiredEnvironmentVars["TF_VAR_ibmcloud_api_key"], DataType: "string", Secure: true}, + {Name: "region", Value: region, DataType: "string"}, + {Name: "resource_group_name", Value: terraform.Output(t, existingTerraformOptions, "resource_group_name"), DataType: "string"}, + {Name: "use_existing_resource_group", Value: true, DataType: "bool"}, + {Name: "existing_kms_instance_crn", Value: permanentResources["hpcs_south_crn"], DataType: "string"}, + {Name: "existing_cos_instance_crn", Value: terraform.Output(t, existingTerraformOptions, "cos_crn"), DataType: "string"}, + {Name: "existing_cos_bucket_name", Value: terraform.Output(t, existingTerraformOptions, "bucket_name"), DataType: "string"}, + {Name: "existing_cos_endpoint", Value: terraform.Output(t, existingTerraformOptions, "s3_endpoint_direct"), DataType: "string"}, } + err3 := options3.RunSchematicTest() + assert.NoError(t, err3, "TestRunExistingResourcesInstances using existing RG, COS instance and bucket Failed") } From 3c18ed625b06ab3f58c0e82a25fa0bf7db2df766 Mon Sep 17 00:00:00 2001 From: ocofaigh Date: Tue, 15 Oct 2024 19:32:49 +0100 Subject: [PATCH 6/8] fix test --- tests/existing-resources/outputs.tf | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/existing-resources/outputs.tf b/tests/existing-resources/outputs.tf index fdcffde8..3c3b05ca 100644 --- a/tests/existing-resources/outputs.tf +++ b/tests/existing-resources/outputs.tf @@ -37,7 +37,7 @@ output "bucket_name" { value = module.cos.bucket_name } -output "s3_endpoint_direct" { +output "s3_endpoint_direct_url" { description = "COS bucket name" - value = module.cos.s3_endpoint_direct + value = "https://${module.cos.s3_endpoint_direct}" } From 6dc6e2ba890c51687912ce7856b849ba25fb0415 Mon Sep 17 00:00:00 2001 From: ocofaigh Date: Tue, 15 Oct 2024 20:03:24 +0100 Subject: [PATCH 7/8] update validation --- solutions/standard/main.tf | 4 ++-- tests/pr_test.go | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/solutions/standard/main.tf b/solutions/standard/main.tf index 60686671..40c359dd 100644 --- a/solutions/standard/main.tf +++ b/solutions/standard/main.tf @@ -17,9 +17,9 @@ module "resource_group" { # Input variable validation locals { - # Validate that a value has been passed for 'existing_kms_instance_crn' if not using existing EN instance + # Validate that a value has been passed for 'existing_kms_instance_crn' and 'kms_endpoint_url' if not using existing EN instance # tflint-ignore: terraform_unused_declarations - validate_kms_input = var.existing_kms_instance_crn == null && var.existing_en_instance_crn == null ? tobool("A value for 'existing_kms_instance_crn' must be passed when no value is passed for 'existing_en_instance_crn'.") : true + validate_kms_input = (var.existing_kms_instance_crn == null || var.kms_endpoint_url == null) && var.existing_en_instance_crn == null ? tobool("A value for 'existing_kms_instance_crn' and 'kms_endpoint_url' must be passed when no value is passed for 'existing_en_instance_crn'.") : true } # If existing KMS root key CRN passed, parse details from it diff --git a/tests/pr_test.go b/tests/pr_test.go index 3e8ef30c..2b706b7a 100644 --- a/tests/pr_test.go +++ b/tests/pr_test.go @@ -336,9 +336,10 @@ func TestRunExistingResourcesInstances(t *testing.T) { {Name: "resource_group_name", Value: terraform.Output(t, existingTerraformOptions, "resource_group_name"), DataType: "string"}, {Name: "use_existing_resource_group", Value: true, DataType: "bool"}, {Name: "existing_kms_instance_crn", Value: permanentResources["hpcs_south_crn"], DataType: "string"}, + {Name: "kms_endpoint_url", Value: permanentResources["hpcs_south_private_endpoint"], DataType: "string"}, {Name: "existing_cos_instance_crn", Value: terraform.Output(t, existingTerraformOptions, "cos_crn"), DataType: "string"}, {Name: "existing_cos_bucket_name", Value: terraform.Output(t, existingTerraformOptions, "bucket_name"), DataType: "string"}, - {Name: "existing_cos_endpoint", Value: terraform.Output(t, existingTerraformOptions, "s3_endpoint_direct"), DataType: "string"}, + {Name: "existing_cos_endpoint", Value: terraform.Output(t, existingTerraformOptions, "s3_endpoint_direct_url"), DataType: "string"}, } err3 := options3.RunSchematicTest() assert.NoError(t, err3, "TestRunExistingResourcesInstances using existing RG, COS instance and bucket Failed") From 0f7a719ef0e7dafe6b26972be331a35b0680aba9 Mon Sep 17 00:00:00 2001 From: ocofaigh Date: Wed, 16 Oct 2024 11:14:24 +0100 Subject: [PATCH 8/8] pr comments --- solutions/standard/main.tf | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/solutions/standard/main.tf b/solutions/standard/main.tf index 01c95dfc..6567d650 100644 --- a/solutions/standard/main.tf +++ b/solutions/standard/main.tf @@ -75,9 +75,8 @@ locals { # If a prefix value is passed, add it to the COS key ring name cos_key_ring_name = var.prefix != null ? "${var.prefix}-${var.cos_key_ring_name}" : var.cos_key_ring_name # Determine the COS KMS key CRN (new key or existing key). It will only have a value if not using an existing bucket or existing EN instance - # cos_kms_key_crn = var.existing_cos_bucket_name != null ? null : var.existing_kms_root_key_crn != null ? var.existing_kms_root_key_crn : var.existing_en_instance_crn == null ? module.kms[0].keys[format("%s.%s", local.cos_key_ring_name, local.cos_key_name)].crn : null cos_kms_key_crn = var.existing_en_instance_crn != null || var.existing_cos_bucket_name != null ? null : var.existing_kms_root_key_crn != null ? var.existing_kms_root_key_crn : module.kms[0].keys[format("%s.%s", local.cos_key_ring_name, local.cos_key_name)].crn - # If existing KMS instance CRN passed, parse the account ID from it + # If existing KMS instance CRN passed, parse the key ID from it cos_kms_key_id = local.cos_kms_key_crn != null ? module.cos_kms_key_crn_parser[0].resource : null # Event Notifications KMS Key ring config en_kms_key = { @@ -344,7 +343,7 @@ locals { validate_sm_crn = length(var.service_credential_secrets) > 0 && var.existing_secrets_manager_instance_crn == null ? tobool("'existing_secrets_manager_instance_crn' is required when adding service credentials with the 'service_credential_secrets' input.") : false # parse SM GUID from CRN existing_secrets_manager_instance_guid = var.existing_secrets_manager_instance_crn != null ? module.existing_sm_crn_parser[0].service_instance : null - # parse SM regiob from CRN + # parse SM region from CRN existing_secrets_manager_instance_region = var.existing_secrets_manager_instance_crn != null ? module.existing_sm_crn_parser[0].region : null # generate list of service credential secrets to create service_credential_secrets = [