Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: use external kms key #131

Open
wants to merge 51 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
63d287e
feat: use external kms key
kierramarie Jun 24, 2024
f15491d
Merge branch 'main' into ksearle-ext-kms
kierramarie Jun 25, 2024
dd385ee
Merge branch 'main' into ksearle-ext-kms
kierramarie Jul 1, 2024
66e77da
refactor: update descriptions to be consistent with secrets manager
kierramarie Jul 2, 2024
f423d6f
fix: merge with main
kierramarie Jul 2, 2024
9b8a2ad
docs: update comment documentation
kierramarie Jul 3, 2024
3dea95d
Merge branch 'main' into ksearle-ext-kms
kierramarie Jul 3, 2024
8cdd9df
fix: some minor changes
kierramarie Jul 5, 2024
5ee35e3
Merge branch 'main' into ksearle-ext-kms
kierramarie Jul 15, 2024
9e8be14
fix: pull from remote
kierramarie Jul 15, 2024
d1d7dc2
Merge branch 'main' into ksearle-ext-kms
kierramarie Jul 24, 2024
4d16ae2
Merge branch 'main' into ksearle-ext-kms
kierramarie Jul 24, 2024
3d2e2b6
Merge branch 'main' into ksearle-ext-kms
kierramarie Aug 6, 2024
87b7fba
fix: upgrade test fail fix
kierramarie Aug 6, 2024
405bedc
docs: spelling fix
kierramarie Aug 6, 2024
a925492
Merge branch 'main' into ksearle-ext-kms
kierramarie Aug 8, 2024
f8451f6
fix: merge
kierramarie Aug 8, 2024
52cdeb2
refactor: move nonsensitive
kierramarie Aug 8, 2024
735cabb
fix: merge
kierramarie Aug 8, 2024
7fd6a27
Merge branch 'main' into ksearle-ext-kms
kierramarie Aug 13, 2024
028ca0d
Merge branch 'main' into ksearle-ext-kms
kierramarie Aug 26, 2024
7f426b9
chore: pre-commit fixes
kierramarie Aug 26, 2024
334b6f0
Merge branch 'main' into ksearle-ext-kms
kierramarie Aug 29, 2024
dba95e5
Merge branch 'main' into ksearle-ext-kms
kierramarie Aug 29, 2024
2ff05dc
Merge branch 'main' into ksearle-ext-kms
kierramarie Sep 4, 2024
1685537
Merge branch 'main' into ksearle-ext-kms
kierramarie Sep 4, 2024
a5aca1d
docs: precommit update
kierramarie Sep 5, 2024
55e62aa
Merge branch 'main' into ksearle-ext-kms
kierramarie Sep 5, 2024
a51d196
Merge branch 'main' into ksearle-ext-kms
kierramarie Sep 9, 2024
47ca80b
Merge branch 'main' into ksearle-ext-kms
ocofaigh Sep 11, 2024
b34526e
refactor: use crn parser
kierramarie Sep 23, 2024
95ede9d
Merge branch 'main' into ksearle-ext-kms
kierramarie Sep 23, 2024
127b4f1
docs: precommit
kierramarie Sep 23, 2024
536f305
Merge branch 'ksearle-ext-kms' of https://github.com/terraform-ibm-mo…
kierramarie Sep 23, 2024
b4ef48f
fix: precommit fixes
kierramarie Sep 23, 2024
5f3615b
refactor: bump version constraint
kierramarie Sep 23, 2024
fee6c76
Merge branch 'main' into ksearle-ext-kms
kierramarie Oct 2, 2024
36de7b1
fix: precommit issue
kierramarie Oct 2, 2024
a159a91
fix: scc version bump
kierramarie Oct 9, 2024
be6de2e
Merge branch 'main' into ksearle-ext-kms
kierramarie Oct 10, 2024
e1e5d60
docs: precommit update
kierramarie Oct 10, 2024
e9582bf
Merge branch 'ksearle-ext-kms' of https://github.com/terraform-ibm-mo…
kierramarie Oct 10, 2024
8222506
fix: auth policy cos cycle
kierramarie Oct 16, 2024
a40ad15
chore: merge with main
kierramarie Oct 16, 2024
9b7ccc5
fix: moved blocks
kierramarie Oct 16, 2024
db696cd
fix: only use buckets with xaccount kms
kierramarie Oct 18, 2024
b0fe94e
fix: add count to time_sleep
kierramarie Oct 18, 2024
8fb4f03
fix: cross_account bool condition
kierramarie Oct 21, 2024
5e39528
Merge branch 'main' into ksearle-ext-kms
kierramarie Oct 21, 2024
b1cf2aa
fix: precommit issue
kierramarie Oct 21, 2024
58d73ad
Merge branch 'main' into ksearle-ext-kms
kierramarie Oct 24, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions solutions/instances/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ This solution supports provisioning and configuring the following infrastructure
|------|---------|
| <a name="requirement_terraform"></a> [terraform](#requirement\_terraform) | >= 1.3.0 |
| <a name="requirement_ibm"></a> [ibm](#requirement\_ibm) | 1.66.0 |
| <a name="requirement_time"></a> [time](#requirement\_time) | 0.9.1 |

### Modules

Expand All @@ -37,6 +38,8 @@ This solution supports provisioning and configuring the following infrastructure
|------|------|
| [ibm_en_subscription_email.email_subscription](https://registry.terraform.io/providers/IBM-Cloud/ibm/1.66.0/docs/resources/en_subscription_email) | resource |
| [ibm_en_topic.en_topic](https://registry.terraform.io/providers/IBM-Cloud/ibm/1.66.0/docs/resources/en_topic) | resource |
| [ibm_iam_authorization_policy.cos_kms_policy](https://registry.terraform.io/providers/IBM-Cloud/ibm/1.66.0/docs/resources/iam_authorization_policy) | resource |
| [time_sleep.wait_for_authorization_policy](https://registry.terraform.io/providers/hashicorp/time/0.9.1/docs/resources/sleep) | resource |
| [ibm_en_destinations.en_destinations](https://registry.terraform.io/providers/IBM-Cloud/ibm/1.66.0/docs/data-sources/en_destinations) | data source |
| [ibm_iam_account_settings.iam_account_settings](https://registry.terraform.io/providers/IBM-Cloud/ibm/1.66.0/docs/data-sources/iam_account_settings) | data source |

Expand All @@ -52,11 +55,12 @@ This solution supports provisioning and configuring the following infrastructure
| <a name="input_existing_activity_tracker_crn"></a> [existing\_activity\_tracker\_crn](#input\_existing\_activity\_tracker\_crn) | The CRN of an existing Activity Tracker instance. Used to send Security and Compliance Center Object Storage bucket log data and all object write events to Activity Tracker. Only used if not supplying an existing Object Storage bucket. | `string` | `null` | no |
| <a name="input_existing_cos_instance_crn"></a> [existing\_cos\_instance\_crn](#input\_existing\_cos\_instance\_crn) | The CRN of an existing Object Storage instance. If not specified, an instance is created. | `string` | `null` | no |
| <a name="input_existing_en_crn"></a> [existing\_en\_crn](#input\_existing\_en\_crn) | The CRN of an Event Notification instance. Used to integrate with Security and Compliance Center. | `string` | `null` | no |
| <a name="input_existing_kms_instance_crn"></a> [existing\_kms\_instance\_crn](#input\_existing\_kms\_instance\_crn) | The CRN of the existing Hyper Protect Crypto Services or Key Protect instance. Applies only if not supplying an existing KMS root key and if `skip_cos_kms_auth_policy` is true. | `string` | `null` | no |
| <a name="input_existing_kms_instance_crn"></a> [existing\_kms\_instance\_crn](#input\_existing\_kms\_instance\_crn) | The CRN of the existing KMS instance (Hyper Protect Crypto Services or Key Protect). Required only if `existing_secrets_manager_crn` or `existing_secrets_manager_kms_key_crn` is not specified. If the KMS instance is in different account you must also provide a value for `ibmcloud_kms_api_key`. | `string` | `null` | no |
| <a name="input_existing_monitoring_crn"></a> [existing\_monitoring\_crn](#input\_existing\_monitoring\_crn) | The CRN of an existing IBM Cloud Monitoring instance. Used to send all Object Storage bucket request and usage metrics to, as well as Workload Protection data. Ignored if using existing Object Storage bucket and not provisioning Workload Protection. | `string` | `null` | no |
| <a name="input_existing_scc_cos_bucket_name"></a> [existing\_scc\_cos\_bucket\_name](#input\_existing\_scc\_cos\_bucket\_name) | The name of an existing bucket inside the existing Object Storage instance to use for Security and Compliance Center. If not specified, a bucket is created. | `string` | `null` | no |
| <a name="input_existing_scc_cos_kms_key_crn"></a> [existing\_scc\_cos\_kms\_key\_crn](#input\_existing\_scc\_cos\_kms\_key\_crn) | The CRN of an existing KMS key to use to encrypt the Security and Compliance Center Object Storage bucket. If no value is set for this variable, specify a value for either the `existing_kms_instance_crn` variable to create a key ring and key, or for the `existing_scc_cos_bucket_name` variable to use an existing bucket. | `string` | `null` | no |
| <a name="input_ibmcloud_api_key"></a> [ibmcloud\_api\_key](#input\_ibmcloud\_api\_key) | The IBM Cloud API key to deploy resources. | `string` | n/a | yes |
| <a name="input_ibmcloud_kms_api_key"></a> [ibmcloud\_kms\_api\_key](#input\_ibmcloud\_kms\_api\_key) | The IBM Cloud API key that can create a root key and key ring in the key management service (KMS) instance. If not specified, the 'ibmcloud\_api\_key' variable is used. Specify this key if the instance in `existing_kms_instance_crn` is in an account that's different from the Secrets Manager instance. Leave this input empty if the same account owns both instances. | `string` | `null` | no |
| <a name="input_kms_endpoint_type"></a> [kms\_endpoint\_type](#input\_kms\_endpoint\_type) | The endpoint for communicating with the KMS instance. Possible values: `public`, `private.` | `string` | `"private"` | no |
| <a name="input_management_endpoint_type_for_bucket"></a> [management\_endpoint\_type\_for\_bucket](#input\_management\_endpoint\_type\_for\_bucket) | The type of endpoint for the IBM Terraform provider to use to manage Object Storage buckets. Possible values: `public`, `private`m `direct`. If you specify `private`, enable virtual routing and forwarding in your account, and the Terraform runtime must have access to the the IBM Cloud private network. | `string` | `"private"` | no |
| <a name="input_prefix"></a> [prefix](#input\_prefix) | The prefix to add to all resources created by this solution. | `string` | `null` | no |
Expand All @@ -80,7 +84,7 @@ This solution supports provisioning and configuring the following infrastructure
| <a name="input_scc_workload_protection_instance_tags"></a> [scc\_workload\_protection\_instance\_tags](#input\_scc\_workload\_protection\_instance\_tags) | The list of tags to add to the Workload Protection instance. | `list(string)` | `[]` | no |
| <a name="input_scc_workload_protection_resource_key_tags"></a> [scc\_workload\_protection\_resource\_key\_tags](#input\_scc\_workload\_protection\_resource\_key\_tags) | The tags associated with the Workload Protection resource key. | `list(string)` | `[]` | no |
| <a name="input_scc_workload_protection_service_plan"></a> [scc\_workload\_protection\_service\_plan](#input\_scc\_workload\_protection\_service\_plan) | The pricing plan for the Workload Protection instance service. Possible values: `free-trial`, `graduated-tier`. | `string` | `"graduated-tier"` | no |
| <a name="input_skip_cos_kms_auth_policy"></a> [skip\_cos\_kms\_auth\_policy](#input\_skip\_cos\_kms\_auth\_policy) | Set to `true` to skip the creation of an IAM authorization policy that permits the Object Storage instance to read the encryption key from the KMS instance. An authorization policy must exist before an encrypted bucket can be created. | `bool` | `false` | no |
| <a name="input_skip_cos_kms_auth_policy"></a> [skip\_cos\_kms\_auth\_policy](#input\_skip\_cos\_kms\_auth\_policy) | Set to `true` to skip the creation of an IAM authorization policy that permits the Object Storage instance created 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. | `bool` | `false` | no |
| <a name="input_skip_scc_cos_auth_policy"></a> [skip\_scc\_cos\_auth\_policy](#input\_skip\_scc\_cos\_auth\_policy) | Set to `true` to skip creation of an IAM authorization policy that permits the Security and Compliance Center to write to the Object Storage instance created by this solution. Applies only if `provision_scc_instance` is true. | `bool` | `false` | no |
| <a name="input_skip_scc_workload_protection_auth_policy"></a> [skip\_scc\_workload\_protection\_auth\_policy](#input\_skip\_scc\_workload\_protection\_auth\_policy) | Set to `true` to skip creating an IAM authorization policy that permits the Security and Compliance Center instance to read from the Workload Protection instance. Applies only if `provision_scc_workload_protection` is true. | `bool` | `false` | no |
| <a name="input_use_existing_resource_group"></a> [use\_existing\_resource\_group](#input\_use\_existing\_resource\_group) | Whether to use an existing resource group. | `bool` | `false` | no |
Expand Down
30 changes: 29 additions & 1 deletion solutions/instances/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,33 @@ locals {
scc_workload_protection_instance_name = var.prefix != null ? "${var.prefix}-${var.scc_workload_protection_instance_name}" : var.scc_workload_protection_instance_name
scc_workload_protection_resource_key_name = var.prefix != null ? "${var.prefix}-${var.scc_workload_protection_instance_name}-key" : "${var.scc_workload_protection_instance_name}-key"
scc_cos_bucket_name = var.prefix != null ? "${var.prefix}-${var.scc_cos_bucket_name}" : var.scc_cos_bucket_name
create_cross_account_auth_policy = !var.skip_cos_kms_auth_policy && var.ibmcloud_kms_api_key != null

kms_service_name = var.existing_kms_instance_crn != null ? (
ocofaigh marked this conversation as resolved.
Show resolved Hide resolved
can(regex(".*kms.*", var.existing_kms_instance_crn)) ? "kms" : (
can(regex(".*hs-crypto.*", var.existing_kms_instance_crn)) ? "hs-crypto" : null
)
) : null
}

# Create IAM Authorization Policy to allow COS to access KMS for the encryption key
resource "ibm_iam_authorization_policy" "cos_kms_policy" {
kierramarie marked this conversation as resolved.
Show resolved Hide resolved
count = local.create_cross_account_auth_policy ? 1 : 0
# Conditionals with providers aren't possible, using ibm.kms as provider incase cross account is enabled
kierramarie marked this conversation as resolved.
Show resolved Hide resolved
provider = ibm.kms
source_service_account = data.ibm_iam_account_settings.iam_account_settings.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 COS instance in the account ${data.ibm_iam_account_settings.iam_account_settings.account_id} to read from the ${local.kms_service_name} instance GUID ${local.existing_kms_guid}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When adding an auth policy, you should add a time sleep before trying to deploy the service. For example, see https://github.com/terraform-ibm-modules/terraform-ibm-secrets-manager/blob/dbc64683a586dc0034ce61bc47d66d96cf0a9f3b/main.tf#L74-L79

Once the time sleep exists, you need to add an depends_on it for the call to cos module

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I understand the COS needs to already exist to create this auth policy so adding a sleep would create a cycle. Am I misunderstanding where the depends_on go?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kierramarie The auth policy has to exist before the COS bucket can be created. But the instance has to exist before the auth policy can be created. Unfortunately we are creating both instance and bucket in the COS module call, so I don't think we can apply the time sleep workaround here. If we used a different resource for creating the bucket (such as the buckets submodule), this would not be the case.

There might be a race condition here though.. what if the bucket creation started to happen before the auth policy was created. We have no way to control that.
We may need to split bucket creation into its own resource to fix this, but we would need to ensure not to break current consumers.

}

# 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.cos_kms_policy]
create_duration = "30s"
kierramarie marked this conversation as resolved.
Show resolved Hide resolved
}

# KMS root key for SCC COS bucket
Expand Down Expand Up @@ -79,6 +106,7 @@ locals {
scc_cos_kms_key_crn = var.existing_scc_cos_bucket_name != null ? null : var.existing_scc_cos_kms_key_crn != null ? var.existing_scc_cos_kms_key_crn : module.kms[0].keys[format("%s.%s", local.scc_cos_key_ring_name, local.scc_cos_key_name)].crn
cos_instance_crn = var.existing_cos_instance_crn != null ? var.existing_cos_instance_crn : module.cos[0].cos_instance_crn
cos_bucket_name = var.existing_scc_cos_bucket_name != null ? var.existing_scc_cos_bucket_name : module.cos[0].buckets[local.scc_cos_bucket_name].bucket_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) : module.cos[0].cos_instance_guid
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we found issues when using - 3 - do not use minus when parsing. Go forwards. There is now a CRN parser module at https://github.com/terraform-ibm-modules/terraform-ibm-common-utilities/tree/main/modules/crn-parser so can you use this instead?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Steve found a bug with CRN parser that prevents it being used here currently, so for now, proceed with this approach, but don't work backwards


activity_tracking = var.existing_activity_tracker_crn != null ? {
read_data_events = true
Expand Down Expand Up @@ -114,7 +142,7 @@ module "cos" {
kms_encryption_enabled = true
kms_guid = local.existing_kms_guid
kms_key_crn = local.scc_cos_kms_key_crn
skip_iam_authorization_policy = var.skip_cos_kms_auth_policy
skip_iam_authorization_policy = local.create_cross_account_auth_policy || var.skip_cos_kms_auth_policy
management_endpoint_type = var.management_endpoint_type_for_bucket
storage_class = var.scc_cos_bucket_class
resource_instance_id = local.cos_instance_crn
Expand Down
2 changes: 1 addition & 1 deletion solutions/instances/provider.tf
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ provider "ibm" {

provider "ibm" {
alias = "kms"
ibmcloud_api_key = var.ibmcloud_api_key
ibmcloud_api_key = var.ibmcloud_kms_api_key != null ? var.ibmcloud_kms_api_key : var.ibmcloud_api_key
region = local.kms_region
}

Expand Down
11 changes: 9 additions & 2 deletions solutions/instances/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ variable "prefix" {
variable "existing_kms_instance_crn" {
type = string
default = null
description = "The CRN of the existing Hyper Protect Crypto Services or Key Protect instance. Applies only if not supplying an existing KMS root key and if `skip_cos_kms_auth_policy` is true."
description = "The CRN of the existing KMS instance (Hyper Protect Crypto Services or Key Protect). Required only if `existing_secrets_manager_crn` or `existing_secrets_manager_kms_key_crn` is not specified. If the KMS instance is in different account you must also provide a value for `ibmcloud_kms_api_key`."
kierramarie marked this conversation as resolved.
Show resolved Hide resolved
}

variable "existing_scc_cos_kms_key_crn" {
Expand Down Expand Up @@ -70,6 +70,13 @@ variable "scc_cos_key_name" {
description = "The name for the key created for the Security and Compliance Center Object Storage bucket. Applies only if not specifying an existing key. If a prefix input variable is specified, the prefix is added to the name in the `<prefix>-<name>` format."
}

variable "ibmcloud_kms_api_key" {
type = string
description = "The IBM Cloud API key that can create a root key and key ring in the key management service (KMS) instance. If not specified, the 'ibmcloud_api_key' variable is used. Specify this key if the instance in `existing_kms_instance_crn` is in an account that's different from the Secrets Manager instance. Leave this input empty if the same account owns both instances."
kierramarie marked this conversation as resolved.
Show resolved Hide resolved
sensitive = true
default = null
}

########################################################################################################################
# COS variables
########################################################################################################################
Expand Down Expand Up @@ -142,7 +149,7 @@ variable "existing_scc_cos_bucket_name" {

variable "skip_cos_kms_auth_policy" {
type = bool
description = "Set to `true` to skip the creation of an IAM authorization policy that permits the Object Storage instance to read the encryption key from the KMS instance. An authorization policy must exist before an encrypted bucket can be created."
description = "Set to `true` to skip the creation of an IAM authorization policy that permits the Object Storage instance created 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."
default = false
}

Expand Down
4 changes: 4 additions & 0 deletions solutions/instances/version.tf
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,9 @@ terraform {
source = "IBM-Cloud/ibm"
version = "1.66.0"
}
time = {
source = "hashicorp/time"
version = "0.9.1"
}
}
}