From 5c7567b08ed4bde09a03016ecd77d6b5cc9dfb46 Mon Sep 17 00:00:00 2001 From: shemau Date: Mon, 12 Jun 2023 14:11:08 +0100 Subject: [PATCH] fix: issue preventing from creating a COS instance only with authorization policy (#410) --- README.md | 30 +++++++++++++-------------- examples/complete/main.tf | 2 ++ examples/existing-resources/main.tf | 16 +++------------ main.tf | 7 ++++--- module-metadata.json | 4 ++-- profiles/fscloud/README.md | 3 --- profiles/fscloud/main.tf | 32 +++++------------------------ profiles/fscloud/variables.tf | 6 ------ 8 files changed, 31 insertions(+), 69 deletions(-) diff --git a/README.md b/README.md index 45a6706f..7ab0be7f 100644 --- a/README.md +++ b/README.md @@ -29,28 +29,28 @@ provider "ibm" { # - COS bucket with retention, encryption, monitoring and activity tracking module "cos_module" { # Replace "main" with a GIT release version to lock into a specific release - source = "git::https://github.com/terraform-ibm-modules/terraform-ibm-cos?ref=main" - resource_group_id = "xxXXxxXXxXxXXXXxxXxxxXXXXxXXXXX" - region = "us-south" - cos_instance_name = "my-cos-instance" - bucket_name = "my-cos-bucket" + source = "git::https://github.com/terraform-ibm-modules/terraform-ibm-cos?ref=main" + resource_group_id = "xxXXxxXXxXxXXXXxxXxxxXXXXxXXXXX" + region = "us-south" + cos_instance_name = "my-cos-instance" + bucket_name = "my-cos-bucket" existing_kms_instance_guid = "xxxxxxxx-XXXX-XXXX-XXXX-xxxxxxxx" kms_key_crn = "crn:v1:bluemix:public:kms:us-south:a/xxXXxxXXxXxXXXXxxXxxxXXXXxXXXXX:xxxxxx-XXXX-XXXX-XXXX-xxxxxx:key:xxxxxx-XXXX-XXXX-XXXX-xxxxxx" - sysdig_crn = "crn:v1:bluemix:public:sysdig-monitor:us-south:a/xxXXxxXXxXxXXXXxxXxxxXXXXxXXXXX:xxXXxxXXxXxXXXXxxXxxxXXXXxXXXXX::" - activity_tracker_crn = "crn:v1:bluemix:public:logdnaat:us-south:a/xxXXxxXXxXxXXXXxxXxxxXXXXxXXXXX:xxXXxxXXxXxXXXXxxXxxxXXXXxXXXXX::" + sysdig_crn = "crn:v1:bluemix:public:sysdig-monitor:us-south:a/xxXXxxXXxXxXXXXxxXxxxXXXXxXXXXX:xxXXxxXXxXxXXXXxxXxxxXXXXxXXXXX::" + activity_tracker_crn = "crn:v1:bluemix:public:logdnaat:us-south:a/xxXXxxXXxXxXXXXxxXxxxXXXXxXXXXX:xxXXxxXXxXxXXXXxxXxxxXXXXxXXXXX::" } # Creates additional bucket in instance created above: module "additional_cos_bucket" { # Replace "main" with a GIT release version to lock into a specific release - source = "git::https://github.com/terraform-ibm-modules/terraform-ibm-cos?ref=main" - bucket_name = "additional-bucket" - resource_group_id = "xxXXxxXXxXxXXXXxxXxxxXXXXxXXXXX" - region = "us-south" - sysdig_crn = "crn:v1:bluemix:public:sysdig-monitor:us-south:a/xxXXxxXXxXxXXXXxxXxxxXXXXxXXXXX:xxXXxxXXxXxXXXXxxXxxxXXXXxXXXXX::" - activity_tracker_crn = "crn:v1:bluemix:public:logdnaat:us-south:a/xxXXxxXXxXxXXXXxxXxxxXXXXxXXXXX:xxXXxxXXxXxXXXXxxXxxxXXXXxXXXXX::" - existing_cos_instance_id = module.cos_module.cos_instance_id - kms_key_crn = "crn:v1:bluemix:public:kms:us-south:a/xxXXxxXXxXxXXXXxxXxxxXXXXxXXXXX:xxxxxx-XXXX-XXXX-XXXX-xxxxxx:key:xxxxxx-XXXX-XXXX-XXXX-xxxxxx" + source = "git::https://github.com/terraform-ibm-modules/terraform-ibm-cos?ref=main" + bucket_name = "additional-bucket" + resource_group_id = "xxXXxxXXxXxXXXXxxXxxxXXXXxXXXXX" + region = "us-south" + sysdig_crn = "crn:v1:bluemix:public:sysdig-monitor:us-south:a/xxXXxxXXxXxXXXXxxXxxxXXXXxXXXXX:xxXXxxXXxXxXXXXxxXxxxXXXXxXXXXX::" + activity_tracker_crn = "crn:v1:bluemix:public:logdnaat:us-south:a/xxXXxxXXxXxXXXXxxXxxxXXXXxXXXXX:xxXXxxXXxXxXXXXxxXxxxXXXXxXXXXX::" + existing_cos_instance_id = module.cos_module.cos_instance_id + kms_key_crn = "crn:v1:bluemix:public:kms:us-south:a/xxXXxxXXxXxXXXXxxXxxxXXXXxXXXXX:xxxxxx-XXXX-XXXX-XXXX-xxxxxx:key:xxxxxx-XXXX-XXXX-XXXX-xxxxxx" } ``` diff --git a/examples/complete/main.tf b/examples/complete/main.tf index 83eb57f9..369a9527 100644 --- a/examples/complete/main.tf +++ b/examples/complete/main.tf @@ -173,6 +173,7 @@ module "cos_bucket1" { # - Activity Tracking module "cos_bucket2" { source = "../../" + depends_on = [module.cos_bucket1] # Required since bucket1 creates the IAM authorization policy bucket_name = "${var.prefix}-bucket-2" add_bucket_name_suffix = true management_endpoint_type_for_bucket = var.management_endpoint_type_for_bucket @@ -184,6 +185,7 @@ module "cos_bucket2" { activity_tracker_crn = local.at_crn create_cos_instance = false existing_cos_instance_id = module.cos_bucket1.cos_instance_id + skip_iam_authorization_policy = true # Required since bucket1 creates the IAM authorization policy # disable retention for test environments - enable for stage/prod retention_enabled = false kms_key_crn = module.key_protect_all_inclusive.keys["${local.key_ring_name}.${local.key_name}"].crn diff --git a/examples/existing-resources/main.tf b/examples/existing-resources/main.tf index f683b242..ab742c2b 100644 --- a/examples/existing-resources/main.tf +++ b/examples/existing-resources/main.tf @@ -48,22 +48,11 @@ module "cos_instance" { cos_instance_name = "${var.prefix}-cos" create_cos_bucket = false resource_group_id = module.resource_group.resource_group_id - existing_kms_instance_guid = module.key_protect_all_inclusive.key_protect_guid region = var.region cross_region_location = null activity_tracker_crn = null access_tags = var.access_tags resource_key_existing_serviceid_crn = ibm_iam_service_id.resource_key_existing_serviceid.crn - skip_iam_authorization_policy = true -} - -# Create IAM Authorization Policy to allow COS to access key protect for the encryption key -resource "ibm_iam_authorization_policy" "policy" { - source_service_name = "cloud-object-storage" - source_resource_instance_id = module.cos_instance.cos_instance_guid - target_service_name = "kms" - target_resource_instance_id = module.key_protect_all_inclusive.key_protect_guid - roles = ["Reader"] } ############################################################################## @@ -87,6 +76,7 @@ module "cos" { cross_region_location = null kms_encryption_enabled = true # disable retention for test environments - enable for stage/prod - retention_enabled = false - activity_tracker_crn = null + retention_enabled = false + activity_tracker_crn = null + existing_kms_instance_guid = module.key_protect_all_inclusive.key_protect_guid } diff --git a/main.tf b/main.tf index c83cf567..13d69b94 100644 --- a/main.tf +++ b/main.tf @@ -26,7 +26,7 @@ locals { # tflint-ignore: terraform_unused_declarations validate_cross_region_and_plan_input = var.cross_region_location != null && var.cos_plan == "cos-one-rate-plan" ? tobool("var.cos_plan is 'cos-one-rate-plan', then var.cross_region_location cannot be set as the one rate plan does not support cross region.") : true # tflint-ignore: terraform_unused_declarations - validate_kp_guid_input = var.kms_encryption_enabled && var.create_cos_instance && var.skip_iam_authorization_policy == false && var.existing_kms_instance_guid == null ? tobool("A value must be passed for var.existing_kms_instance_guid when creating an instance, var.kms_encryption_enabled is true and var.skip_iam_authorization_policy is false.") : true + validate_kp_guid_input = var.kms_encryption_enabled && var.create_cos_bucket && var.skip_iam_authorization_policy == false && var.existing_kms_instance_guid == null ? tobool("A value must be passed for var.existing_kms_instance_guid when creating a bucket when var.kms_encryption_enabled is true and var.skip_iam_authorization_policy is false.") : true # tflint-ignore: terraform_unused_declarations validate_cross_region_location_inputs = var.create_cos_bucket && ((var.cross_region_location == null && var.region == null) || (var.cross_region_location != null && var.region != null)) ? tobool("If var.create_cos_bucket is true, then value needs to be provided for var.cross_region_location or var.region, but not both") : true # tflint-ignore: terraform_unused_declarations @@ -65,8 +65,8 @@ resource "ibm_resource_key" "resource_key" { locals { cos_instance_id = var.create_cos_instance ? ibm_resource_instance.cos_instance[0].id : var.existing_cos_instance_id cos_instance_guid = var.create_cos_instance ? ibm_resource_instance.cos_instance[0].guid : element(split(":", var.existing_cos_instance_id), length(split(":", var.existing_cos_instance_id)) - 3) - create_access_policy_kms = var.kms_encryption_enabled && var.create_cos_instance && !var.skip_iam_authorization_policy - kms_service = local.create_access_policy_kms && var.kms_key_crn != null ? ( + create_access_policy_kms = var.kms_encryption_enabled && var.create_cos_bucket && !var.skip_iam_authorization_policy + kms_service = local.create_access_policy_kms ? ( can(regex(".*kms.*", var.kms_key_crn)) ? "kms" : ( can(regex(".*hs-crypto.*", var.kms_key_crn)) ? "hs-crypto" : null ) @@ -178,6 +178,7 @@ resource "ibm_cos_bucket" "cos_bucket" { resource "ibm_cos_bucket" "cos_bucket1" { count = (!var.kms_encryption_enabled && var.create_cos_bucket) ? 1 : 0 bucket_name = var.add_bucket_name_suffix ? "${var.bucket_name}-${random_string.bucket_name_suffix[0].result}" : var.bucket_name + depends_on = [ibm_iam_authorization_policy.policy] resource_instance_id = local.cos_instance_id region_location = var.region cross_region_location = var.cross_region_location diff --git a/module-metadata.json b/module-metadata.json index 09176149..f39601ac 100644 --- a/module-metadata.json +++ b/module-metadata.json @@ -801,7 +801,7 @@ }, "pos": { "filename": "main.tf", - "line": 260 + "line": 261 } }, "instance_cbr_rule": { @@ -878,7 +878,7 @@ }, "pos": { "filename": "main.tf", - "line": 294 + "line": 295 } } } diff --git a/profiles/fscloud/README.md b/profiles/fscloud/README.md index be2dd479..7b3a1d12 100644 --- a/profiles/fscloud/README.md +++ b/profiles/fscloud/README.md @@ -34,8 +34,6 @@ This rule is ignored because the module achieves the same resiliency as cross-re |------|------| | [ibm_cos_bucket_replication_rule.cos_replication_rule](https://registry.terraform.io/providers/ibm-cloud/ibm/latest/docs/resources/cos_bucket_replication_rule) | resource | | [ibm_iam_authorization_policy.policy](https://registry.terraform.io/providers/ibm-cloud/ibm/latest/docs/resources/iam_authorization_policy) | resource | -| [ibm_iam_authorization_policy.primary_kms_policy](https://registry.terraform.io/providers/ibm-cloud/ibm/latest/docs/resources/iam_authorization_policy) | resource | -| [ibm_iam_authorization_policy.secondary_kms_policy](https://registry.terraform.io/providers/ibm-cloud/ibm/latest/docs/resources/iam_authorization_policy) | resource | | [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 @@ -67,7 +65,6 @@ This rule is ignored because the module achieves the same resiliency as cross-re | [secondary\_existing\_hpcs\_instance\_guid](#input\_secondary\_existing\_hpcs\_instance\_guid) | The GUID of the Hyper Protect Crypto service in which the key specified in var.hpcs\_key\_crn is coming from. Required if var.create\_cos\_instance is true in order to create an IAM Access Policy to allow Key protect to access the newly created COS instance. Only required if 'create\_cos\_bucket' is true. | `string` | `null` | no | | [secondary\_hpcs\_key\_crn](#input\_secondary\_hpcs\_key\_crn) | CRN of the Hyper Protect Crypto service to use to encrypt the data in the COS Bucket. Only required if 'create\_cos\_bucket' is true. | `string` | `null` | no | | [secondary\_region](#input\_secondary\_region) | region for the secondary bucket | `string` | `"us-east"` | no | -| [skip\_iam\_authorization\_policy](#input\_skip\_iam\_authorization\_policy) | Set to true to skip the creation of an IAM authorization policy that permits the COS instance created to read the encryption key from the KMS instance in `primary_existing_hpcs_instance_guid` and `secondary_existing_hpcs_instance_guid`. WARNING: An authorization policy must exist before an encrypted bucket can be created | `bool` | `false` | no | | [sysdig\_crn](#input\_sysdig\_crn) | Sysdig Monitoring crn for COS bucket. Only required if 'create\_cos\_bucket' is true. | `string` | `null` | no | ## Outputs diff --git a/profiles/fscloud/main.tf b/profiles/fscloud/main.tf index d2b4ebfd..3e6f5a50 100644 --- a/profiles/fscloud/main.tf +++ b/profiles/fscloud/main.tf @@ -21,12 +21,11 @@ locals { } module "cos_instance" { - source = "../../" - resource_group_id = var.resource_group_id - create_cos_instance = var.create_cos_instance - existing_cos_instance_id = var.existing_cos_instance_id - create_cos_bucket = false - # Since two policies are needed we disable here and define them manually below + source = "../../" + resource_group_id = var.resource_group_id + create_cos_instance = var.create_cos_instance + existing_cos_instance_id = var.existing_cos_instance_id + create_cos_bucket = false skip_iam_authorization_policy = true cos_instance_name = var.cos_instance_name create_hmac_key = var.create_hmac_key @@ -40,27 +39,7 @@ module "cos_instance" { access_tags = var.access_tags } -# Create IAM Authorization Policies to allow COS to access kms for the encryption key -resource "ibm_iam_authorization_policy" "primary_kms_policy" { - count = var.skip_iam_authorization_policy ? 0 : 1 - source_service_name = "cloud-object-storage" - source_resource_instance_id = module.cos_instance.cos_instance_guid - target_service_name = "hs-crypto" - target_resource_instance_id = var.primary_existing_hpcs_instance_guid - roles = ["Reader"] -} - -resource "ibm_iam_authorization_policy" "secondary_kms_policy" { - count = var.skip_iam_authorization_policy ? 0 : 1 - source_service_name = "cloud-object-storage" - source_resource_instance_id = module.cos_instance.cos_instance_guid - target_service_name = "hs-crypto" - target_resource_instance_id = var.secondary_existing_hpcs_instance_guid - roles = ["Reader"] -} - module "cos_primary_bucket" { - depends_on = [ibm_iam_authorization_policy.primary_kms_policy] source = "../../" resource_group_id = var.resource_group_id region = var.primary_region @@ -84,7 +63,6 @@ module "cos_primary_bucket" { } module "cos_secondary_bucket" { - depends_on = [ibm_iam_authorization_policy.secondary_kms_policy] source = "../../" resource_group_id = var.resource_group_id region = var.secondary_region diff --git a/profiles/fscloud/variables.tf b/profiles/fscloud/variables.tf index 42c7d795..8016d7ef 100644 --- a/profiles/fscloud/variables.tf +++ b/profiles/fscloud/variables.tf @@ -218,12 +218,6 @@ variable "instance_cbr_rules" { # Validation happens in the rule module } -variable "skip_iam_authorization_policy" { - type = bool - description = "Set to true to skip the creation of an IAM authorization policy that permits the COS instance created to read the encryption key from the KMS instance in `primary_existing_hpcs_instance_guid` and `secondary_existing_hpcs_instance_guid`. WARNING: An authorization policy must exist before an encrypted bucket can be created" - default = false -} - variable "access_tags" { type = list(string) description = "Optional list of access tags to be added to the created resources"