Skip to content

Commit

Permalink
fix: issue preventing from creating a COS instance only with authoriz…
Browse files Browse the repository at this point in the history
…ation policy (#410)
  • Loading branch information
shemau authored Jun 12, 2023
1 parent 33792ab commit 5c7567b
Show file tree
Hide file tree
Showing 8 changed files with 31 additions and 69 deletions.
30 changes: 15 additions & 15 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
```

Expand Down
2 changes: 2 additions & 0 deletions examples/complete/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
16 changes: 3 additions & 13 deletions examples/existing-resources/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
}

##############################################################################
Expand All @@ -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
}
7 changes: 4 additions & 3 deletions main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
)
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions module-metadata.json
Original file line number Diff line number Diff line change
Expand Up @@ -801,7 +801,7 @@
},
"pos": {
"filename": "main.tf",
"line": 260
"line": 261
}
},
"instance_cbr_rule": {
Expand Down Expand Up @@ -878,7 +878,7 @@
},
"pos": {
"filename": "main.tf",
"line": 294
"line": 295
}
}
}
Expand Down
3 changes: 0 additions & 3 deletions profiles/fscloud/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -67,7 +65,6 @@ This rule is ignored because the module achieves the same resiliency as cross-re
| <a name="input_secondary_existing_hpcs_instance_guid"></a> [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 |
| <a name="input_secondary_hpcs_key_crn"></a> [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 |
| <a name="input_secondary_region"></a> [secondary\_region](#input\_secondary\_region) | region for the secondary bucket | `string` | `"us-east"` | no |
| <a name="input_skip_iam_authorization_policy"></a> [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 |
| <a name="input_sysdig_crn"></a> [sysdig\_crn](#input\_sysdig\_crn) | Sysdig Monitoring crn for COS bucket. Only required if 'create\_cos\_bucket' is true. | `string` | `null` | no |

## Outputs
Expand Down
32 changes: 5 additions & 27 deletions profiles/fscloud/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
6 changes: 0 additions & 6 deletions profiles/fscloud/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down

0 comments on commit 5c7567b

Please sign in to comment.