fix: fixed timing bug with auth policy creation<br>* updated existing_kms_guid to now be existing_kms_crn (#10)

iamar7 · web-flow · commit 0de113ff11d3 · 2024-04-03T16:26:18.000+01:00
diff --git a/cra-config.yaml b/cra-config.yaml
@@ -6,4 +6,4 @@ CRA_TARGETS:
     PROFILE_ID: "bfacb71d-4b84-41ac-9825-e8a3a3eb7405" # SCC profile ID (currently set to IBM Cloud Framework for Financial Services 1.6.0 profile).
     CRA_ENVIRONMENT_VARIABLES:
         TF_VAR_resource_group_name: "test"
-        TF_VAR_existing_kms_guid: "XXXXXXXXXXXXXXXX"
+        TF_VAR_existing_kms_crn: "XXXX:hs-crypto:XXXXXXX:XXXXX:XXXXX"
diff --git a/solutions/instances/catalogValidationValues.json.template b/solutions/instances/catalogValidationValues.json.template
@@ -1,6 +1,6 @@
 {
   "ibmcloud_api_key": $VALIDATION_APIKEY,
   "resource_group_name": $PREFIX,
-  "existing_kms_guid": $HPCS_US_SOUTH_GUID,
+  "existing_kms_crn": $HPCS_US_SOUTH_CRN,
   "kms_region": "us-south"
 }
diff --git a/solutions/instances/main.tf b/solutions/instances/main.tf
@@ -5,26 +5,26 @@
 locals {
   archive_api_key = var.log_archive_api_key == null ? var.ibmcloud_api_key : var.log_archive_api_key
 
-  cos_instance_crn            = var.existing_cos_instance_crn != null ? var.existing_cos_instance_crn : module.cos[0].cos_instance_crn
-  archive_cos_bucket_name     = var.existing_log_archive_cos_bucket_name != null ? var.existing_log_archive_cos_bucket_name : module.cos[0].buckets[var.log_archive_cos_bucket_name].bucket_name
-  archive_cos_bucket_endpoint = var.existing_log_archive_cos_bucket_endpoint != null ? var.existing_log_archive_cos_bucket_endpoint : module.cos[0].buckets[var.log_archive_cos_bucket_name].s3_endpoint_private
+  cos_instance_crn            = var.existing_cos_instance_crn != null ? var.existing_cos_instance_crn : module.cos_instance[0].cos_instance_crn
+  existing_kms_guid           = var.existing_kms_crn != null ? element(split(":", var.existing_kms_crn), length(split(":", var.existing_kms_crn)) - 3) : length(local.bucket_config_map) == 2 ? null : tobool("The CRN of the existing KMS is not provided.")
+  cos_instance_guid           = var.existing_cos_instance_crn == null ? module.cos_instance[0].cos_instance_guid : element(split(":", var.existing_cos_instance_crn), length(split(":", var.existing_cos_instance_crn)) - 3)
+  archive_cos_bucket_name     = var.existing_log_archive_cos_bucket_name != null ? var.existing_log_archive_cos_bucket_name : module.cos_bucket[0].buckets[var.log_archive_cos_bucket_name].bucket_name
+  archive_cos_bucket_endpoint = var.existing_log_archive_cos_bucket_endpoint != null ? var.existing_log_archive_cos_bucket_endpoint : module.cos_bucket[0].buckets[var.log_archive_cos_bucket_name].s3_endpoint_private
   cos_kms_key_crn             = (var.existing_log_archive_cos_bucket_name != null && var.existing_at_cos_target_bucket_name != null) ? null : var.existing_cos_kms_key_crn != null ? var.existing_cos_kms_key_crn : module.kms[0].keys[format("%s.%s", var.cos_key_ring_name, var.cos_key_name)].crn
 
-  cos_target_bucket_name     = var.existing_at_cos_target_bucket_name != null ? var.existing_at_cos_target_bucket_name : module.cos[0].buckets[var.at_cos_target_bucket_name].bucket_name
-  cos_target_bucket_endpoint = var.existing_at_cos_target_bucket_endpoint != null ? var.existing_at_cos_target_bucket_endpoint : module.cos[0].buckets[var.at_cos_target_bucket_name].s3_endpoint_private
+  cos_target_bucket_name     = var.existing_at_cos_target_bucket_name != null ? var.existing_at_cos_target_bucket_name : module.cos_bucket[0].buckets[var.at_cos_target_bucket_name].bucket_name
+  cos_target_bucket_endpoint = var.existing_at_cos_target_bucket_endpoint != null ? var.existing_at_cos_target_bucket_endpoint : module.cos_bucket[0].buckets[var.at_cos_target_bucket_name].s3_endpoint_private
 
   bucket_config_1 = var.existing_log_archive_cos_bucket_name == null ? {
-    class  = var.log_archive_cos_bucket_class
-    name   = var.log_archive_cos_bucket_name
-    tag    = var.archive_bucket_access_tags
-    policy = var.skip_cos_kms_auth_policy
+    class = var.log_archive_cos_bucket_class
+    name  = var.log_archive_cos_bucket_name
+    tag   = var.archive_bucket_access_tags
   } : null
 
   bucket_config_2 = var.existing_at_cos_target_bucket_name == null ? {
-    class  = var.at_cos_target_bucket_class
-    name   = var.at_cos_target_bucket_name
-    tag    = var.at_cos_bucket_access_tags
-    policy = var.skip_cos_kms_auth_policy == true ? false : true
+    class = var.at_cos_target_bucket_class
+    name  = var.at_cos_target_bucket_name
+    tag   = var.at_cos_bucket_access_tags
   } : null
 
   bucket_config_map = var.existing_log_archive_cos_bucket_name == null ? (
@@ -43,6 +43,12 @@ locals {
     enable = true
     days   = 366
   } : null
+
+  kms_service = var.existing_kms_crn != null ? (
+    can(regex(".*kms.*", var.existing_kms_crn)) ? "kms" : (
+      can(regex(".*hs-crypto.*", var.existing_kms_crn)) ? "hs-crypto" : null
+    )
+  ) : null
 }
 
 #######################################################################################################################
@@ -51,7 +57,7 @@ locals {
 
 module "resource_group" {
   source                       = "terraform-ibm-modules/resource-group/ibm"
-  version                      = "1.1.4"
+  version                      = "1.1.5"
   resource_group_name          = var.existing_resource_group == false ? var.resource_group_name : null
   existing_resource_group_name = var.existing_resource_group == true ? var.resource_group_name : null
 }
@@ -62,7 +68,7 @@ module "resource_group" {
 
 module "observability_instance" {
   source  = "terraform-ibm-modules/observability-instances/ibm"
-  version = "2.11.0"
+  version = "2.12.2"
   providers = {
     logdna.at = logdna.at
     logdna.ld = logdna.ld
@@ -121,12 +127,12 @@ module "kms" {
   providers = {
     ibm = ibm.kms
   }
-  count                       = (var.existing_cos_kms_key_crn != null || (var.existing_log_archive_cos_bucket_name != null && var.existing_at_cos_target_bucket_name != null)) ? 0 : 1 # no need to create any KMS resources if passing an existing key, or bucket
+  count                       = (var.existing_cos_kms_key_crn != null || (length(local.bucket_config_map) == 0)) ? 0 : 1 # no need to create any KMS resources if passing an existing key, or bucket
   source                      = "terraform-ibm-modules/kms-all-inclusive/ibm"
-  version                     = "4.8.3"
+  version                     = "4.8.5"
   create_key_protect_instance = false
   region                      = var.kms_region
-  existing_kms_instance_guid  = var.existing_kms_guid
+  existing_kms_instance_guid  = local.existing_kms_guid
   key_ring_endpoint_type      = var.kms_endpoint_type
   key_endpoint_type           = var.kms_endpoint_type
   keys = [
@@ -151,31 +157,61 @@ module "kms" {
 # COS
 #######################################################################################################################
 
-module "cos" {
+# workaround for https://github.com/IBM-Cloud/terraform-provider-ibm/issues/4478
+resource "time_sleep" "wait_for_authorization_policy" {
+  depends_on      = [ibm_iam_authorization_policy.policy]
+  count           = var.skip_cos_kms_auth_policy ? 0 : 1
+  create_duration = "30s"
+}
+
+# The auth policy is being created here instead of in COS module because of this limitation: https://github.com/terraform-ibm-modules/terraform-ibm-observability-da/issues/8
+
+# Create IAM Authorization Policy to allow COS to access KMS for the encryption key
+resource "ibm_iam_authorization_policy" "policy" {
+  count                       = (var.skip_cos_kms_auth_policy || (length(local.bucket_config_map) == 0)) ? 0 : 1
+  source_service_name         = "cloud-object-storage"
+  source_resource_instance_id = local.cos_instance_guid
+  target_service_name         = local.kms_service
+  target_resource_instance_id = local.existing_kms_guid
+  roles                       = ["Reader"]
+  description                 = "Allow the COS instance with GUID ${local.cos_instance_guid} reader access to the kms_service instance GUID ${local.existing_kms_guid}"
+}
+
+module "cos_instance" {
   providers = {
     ibm = ibm.cos
   }
-  count                    = (var.existing_log_archive_cos_bucket_name == null || var.existing_at_cos_target_bucket_name == null) ? 1 : 0 # no need to call COS module if consumer is passing existing COS bucket
+  count                    = (var.existing_cos_instance_crn == null) ? 1 : 0 # no need to call COS module if consumer is using existing COS instance
   source                   = "terraform-ibm-modules/cos/ibm//modules/fscloud"
   version                  = "7.5.3"
   resource_group_id        = module.resource_group.resource_group_id
-  create_cos_instance      = var.existing_cos_instance_crn == null ? true : false # don't create instance if existing one passed in
+  create_cos_instance      = true
   create_resource_key      = false
   cos_instance_name        = var.cos_instance_name
   cos_tags                 = var.cos_instance_tags
   existing_cos_instance_id = var.existing_cos_instance_crn
   access_tags              = var.cos_instance_access_tags
   cos_plan                 = "standard"
+}
+
+module "cos_bucket" {
+  depends_on = [time_sleep.wait_for_authorization_policy]
+  providers = {
+    ibm = ibm.cos
+  }
+  count   = (length(local.bucket_config_map) != 0) ? 1 : 0 # no need to call COS module if consumer is using existing COS bucket
+  source  = "terraform-ibm-modules/cos/ibm//modules/buckets"
+  version = "7.5.3"
   bucket_configs = [
     for value in local.bucket_config_map :
     {
       access_tags                   = value.tag
       bucket_name                   = value.name
       add_bucket_name_suffix        = var.add_bucket_name_suffix
       kms_encryption_enabled        = true
-      kms_guid                      = var.existing_kms_guid
+      kms_guid                      = local.existing_kms_guid
       kms_key_crn                   = local.cos_kms_key_crn
-      skip_iam_authorization_policy = value.policy
+      skip_iam_authorization_policy = true
       management_endpoint_type      = var.management_endpoint_type_for_bucket
       storage_class                 = value.class
       resource_instance_id          = local.cos_instance_crn
diff --git a/solutions/instances/outputs.tf b/solutions/instances/outputs.tf
@@ -2,6 +2,7 @@
 # Outputs
 ##############################################################################
 
+## Log analysis
 output "log_analysis_name" {
   value       = module.observability_instance.log_analysis_name
   description = "The name of the provisioned Log Analysis instance."
@@ -23,6 +24,7 @@ output "log_analysis_ingestion_key" {
   sensitive   = true
 }
 
+## Cloud Monitoring
 output "cloud_monitoring_name" {
   value       = module.observability_instance.cloud_monitoring_name
   description = "The name of the provisioned IBM cloud monitoring instance."
@@ -44,22 +46,56 @@ output "cloud_monitoring_access_key" {
   sensitive   = true
 }
 
+## COS Instance
+output "cos_instance_id" {
+  description = "COS instance id"
+  value       = var.existing_cos_instance_crn == null ? module.cos_instance[0].cos_instance_id : null
+}
+
+output "cos_instance_guid" {
+  description = "COS instance guid"
+  value       = var.existing_cos_instance_crn == null ? module.cos_instance[0].cos_instance_guid : null
+}
+
+output "cos_instance_name" {
+  description = "COS instance name"
+  value       = var.existing_cos_instance_crn == null ? module.cos_instance[0].cos_instance_name : null
+}
+
+output "cos_instance_crn" {
+  description = "COS instance crn"
+  value       = var.existing_cos_instance_crn == null ? module.cos_instance[0].cos_instance_crn : null
+}
+
+## COS Buckets
 output "log_archive_cos_bucket_name" {
-  value       = var.existing_log_archive_cos_bucket_name == null ? module.cos[0].buckets[var.log_archive_cos_bucket_name].bucket_name : var.existing_log_archive_cos_bucket_name
+  value       = var.existing_log_archive_cos_bucket_name == null ? module.cos_bucket[0].buckets[var.log_archive_cos_bucket_name].bucket_name : var.existing_log_archive_cos_bucket_name
   description = "The name of log archive COS bucket"
 }
 
 output "at_cos_target_bucket_name" {
-  value       = var.existing_at_cos_target_bucket_name == null ? module.cos[0].buckets[var.at_cos_target_bucket_name].bucket_name : var.existing_at_cos_target_bucket_name
-  description = "The name of the at cos target bucket"
+  value       = var.existing_at_cos_target_bucket_name == null ? module.cos_bucket[0].buckets[var.at_cos_target_bucket_name].bucket_name : var.existing_at_cos_target_bucket_name
+  description = "The name of the AT target COS bucket"
 }
 
+## Activity Tracker
 output "at_cos_targets" {
   value       = module.observability_instance.activity_tracker_targets
-  description = "The map of created targets"
+  description = "The map of created activity_tracker targets"
 }
 
 output "at_routes" {
   value       = module.observability_instance.activity_tracker_routes
-  description = "The map of created routes"
+  description = "The map of created activity_tracker routes"
+}
+
+## KMS
+output "kms_key_rings" {
+  description = "IDs of new KMS Key Rings created"
+  value       = length(module.kms) > 0 ? module.kms[0].key_rings : null
+}
+
+output "kms_keys" {
+  description = "IDs of new KMS Keys created"
+  value       = length(module.kms) > 0 ? module.kms[0].keys : null
 }
diff --git a/solutions/instances/variables.tf b/solutions/instances/variables.tf
@@ -251,10 +251,10 @@ variable "kms_region" {
   description = "The region in which KMS instance exists."
 }
 
-variable "existing_kms_guid" {
+variable "existing_kms_crn" {
   type        = string
   default     = null
-  description = "The GUID of of the KMS instance used for the COS bucket root Key. Only required if not supplying an existing KMS root key and if 'skip_cos_kms_auth_policy' is true."
+  description = "The CRN of the KMS instance used for the COS bucket root Key. Only required if not supplying an existing KMS root key and if 'skip_cos_kms_auth_policy' is true."
 }
 
 variable "existing_cos_kms_key_crn" {
diff --git a/solutions/instances/version.tf b/solutions/instances/version.tf
@@ -10,5 +10,9 @@ terraform {
       source  = "logdna/logdna"
       version = "1.16.0"
     }
+    time = {
+      source  = "hashicorp/time"
+      version = "0.11.1"
+    }
   }
 }
diff --git a/tests/pr_test.go b/tests/pr_test.go
@@ -76,7 +76,7 @@ func TestInstancesInSchematics(t *testing.T) {
 	options.TerraformVars = []testschematic.TestSchematicTerraformVar{
 		{Name: "ibmcloud_api_key", Value: options.RequiredEnvironmentVars["TF_VAR_ibmcloud_api_key"], DataType: "string", Secure: true},
 		{Name: "resource_group_name", Value: options.Prefix, DataType: "string"},
-		{Name: "existing_kms_guid", Value: permanentResources["hpcs_south"], DataType: "string"},
+		{Name: "existing_kms_crn", Value: permanentResources["hpcs_south_crn"], DataType: "string"},
 		{Name: "kms_region", Value: "us-south", DataType: "string"}, // KMS instance is in us-south
 		{Name: "cos_region", Value: region, DataType: "string"},
 		{Name: "cos_instance_tags", Value: options.Tags, DataType: "list(string)"},
@@ -104,7 +104,7 @@ func TestRunUpgradeSolutionInstances(t *testing.T) {
 	options.TerraformVars = map[string]interface{}{
 		"resource_group_name":                 options.Prefix,
 		"cos_instance_access_tags":            permanentResources["accessTags"],
-		"existing_kms_guid":                   permanentResources["hpcs_south"],
+		"existing_kms_crn":                    permanentResources["hpcs_south_crn"],
 		"kms_endpoint_type":                   "public",
 		"kms_region":                          "us-south",
 		"management_endpoint_type_for_bucket": "public",

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"ibmcloud_api_key": $VALIDATION_APIKEY,`
`3`	`3`	`"resource_group_name": $PREFIX,`
`4`		`- "existing_kms_guid": $HPCS_US_SOUTH_GUID,`
	`4`	`+ "existing_kms_crn": $HPCS_US_SOUTH_CRN,`
`5`	`5`	`"kms_region": "us-south"`
`6`	`6`	`}`
Original file line number	Diff line number	Diff line change
`@@ -251,10 +251,10 @@ variable "kms_region" {`
`251`	`251`	`description = "The region in which KMS instance exists."`
`252`	`252`	`}`
`253`	`253`
`254`		`-variable "existing_kms_guid" {`
	`254`	`+variable "existing_kms_crn" {`
`255`	`255`	`type = string`
`256`	`256`	`default = null`
`257`		`- description = "The GUID of of the KMS instance used for the COS bucket root Key. Only required if not supplying an existing KMS root key and if 'skip_cos_kms_auth_policy' is true."`
	`257`	`+ description = "The CRN of the KMS instance used for the COS bucket root Key. Only required if not supplying an existing KMS root key and if 'skip_cos_kms_auth_policy' is true."`
`258`	`258`	`}`
`259`	`259`
`260`	`260`	`variable "existing_cos_kms_key_crn" {`
Original file line number	Diff line number	Diff line change
`@@ -10,5 +10,9 @@ terraform {`
`10`	`10`	`source = "logdna/logdna"`
`11`	`11`	`version = "1.16.0"`
`12`	`12`	`}`
	`13`	`+ time = {`
	`14`	`+ source = "hashicorp/time"`
	`15`	`+ version = "0.11.1"`
	`16`	`+ }`
`13`	`17`	`}`
`14`	`18`	`}`