Skip to content

DA rally feedback #336

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
May 21, 2025
Merged

DA rally feedback #336

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4be4056
DA rally feedback
alex-reiff May 15, 2025
6c1f56e
Merge branch 'main' into da-rally
alex-reiff May 19, 2025
c8cfb32
updated feedback
alex-reiff May 19, 2025
5a8c882
more feedbacl
alex-reiff May 20, 2025
188410d
more feedback
alex-reiff May 20, 2025
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
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ You need the following permissions to run this module.

| Name | Description | Type | Default | Required |
|------|-------------|------|---------|:--------:|
| <a name="input_allowed_network"></a> [allowed\_network](#input\_allowed\_network) | The types of service endpoints to set on the Secrets Manager instance. Possible values are `private-only` or `public-and-private`. | `string` | `"public-and-private"` | no |
| <a name="input_allowed_network"></a> [allowed\_network](#input\_allowed\_network) | The types of service endpoints to set on the Secrets Manager instance. Possible values are `private-only` or `public-and-private`. For more details, see https://cloud.ibm.com/docs/secrets-manager?topic=secrets-manager-endpoints#service-endpoints | `string` | `"public-and-private"` | no |
| <a name="input_cbr_rules"></a> [cbr\_rules](#input\_cbr\_rules) | (Optional, list) List of CBR rules to create | <pre>list(object({<br/> description = string<br/> account_id = string<br/> rule_contexts = list(object({<br/> attributes = optional(list(object({<br/> name = string<br/> value = string<br/> }))) }))<br/> enforcement_mode = string<br/> operations = optional(list(object({<br/> api_types = list(object({<br/> api_type_id = string<br/> }))<br/> })))<br/> }))</pre> | `[]` | no |
| <a name="input_enable_event_notification"></a> [enable\_event\_notification](#input\_enable\_event\_notification) | Set this to true to enable lifecycle notifications for your Secrets Manager instance by connecting an Event Notifications service. When setting this to true, a value must be passed for `existing_en_instance_crn` and `existing_sm_instance_crn` must be null. | `bool` | `false` | no |
| <a name="input_endpoint_type"></a> [endpoint\_type](#input\_endpoint\_type) | The type of endpoint (public or private) to connect to the Secrets Manager API. The Terraform provider uses this endpoint type to interact with the Secrets Manager API and configure Event Notifications. | `string` | `"public"` | no |
Expand Down
31 changes: 10 additions & 21 deletions ibm_catalog.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@
}
},
{
"key": "skip_sm_ce_iam_authorization_policy"
"key": "skip_secrets_manager_certificate_engine_iam_authorization_policy"
},
{
"key": "allowed_network",
Expand All @@ -217,32 +217,20 @@
"key": "existing_secrets_manager_kms_key_crn"
},
{
"key": "skip_sm_kms_iam_authorization_policy"
"key": "skip_secrets_manager_kms_iam_authorization_policy"
},
{
"key": "ibmcloud_kms_api_key"
},
{
"key": "kms_endpoint_type",
"options": [
{
"displayname": "Public",
"value": "public"
},
{
"displayname": "Private",
"value": "private"
}
]
},
{
"key": "kms_key_ring_name"
},
{
"key": "kms_key_name"
},
{
"key": "event_notifications_email_list"
"key": "event_notifications_email_list",
"type": "array"
Copy link
Contributor

@ocofaigh ocofaigh May 19, 2025

Choose a reason for hiding this comment

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

we need to add the custom array type for strings:

{
  "key": "event_notifications_email_list",
  "type": "array",
  "custom_config": {
    "grouping": "deployment",
    "original_grouping": "deployment",
    "config_constraints": {
      "type": "string"
    }
  }
},

},
{
"key": "event_notifications_from_email"
Expand All @@ -254,7 +242,7 @@
"key": "existing_event_notifications_instance_crn"
},
{
"key": "skip_event_notifications_iam_authorization_policy"
"key": "skip_secrets_manager_event_notifications_iam_authorization_policy"
},
{
"key": "secrets_manager_cbr_rules"
Expand Down Expand Up @@ -425,7 +413,7 @@
}
},
{
"key": "skip_sm_ce_iam_authorization_policy"
"key": "skip_secrets_manager_certificate_engine_iam_authorization_policy"
},
{
"key": "existing_resource_group_name",
Expand All @@ -443,7 +431,7 @@
"key": "existing_secrets_manager_kms_key_crn"
},
{
"key": "skip_sm_kms_iam_authorization_policy"
"key": "skip_secrets_manager_kms_iam_authorization_policy"
},
{
"key": "ibmcloud_kms_api_key"
Expand All @@ -455,7 +443,8 @@
"key": "kms_key_name"
},
{
"key": "event_notifications_email_list"
"key": "event_notifications_email_list",
"type": "array"
Copy link
Contributor

@ocofaigh ocofaigh May 19, 2025

Choose a reason for hiding this comment

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

we need to add the custom array type for strings:

{
  "key": "event_notifications_email_list",
  "type": "array",
  "custom_config": {
    "grouping": "deployment",
    "original_grouping": "deployment",
    "config_constraints": {
      "type": "string"
    }
  }
},

},
{
"key": "event_notifications_from_email"
Expand All @@ -467,7 +456,7 @@
"key": "existing_event_notifications_instance_crn"
},
{
"key": "skip_event_notifications_iam_authorization_policy"
"key": "skip_secrets_manager_event_notifications_iam_authorization_policy"
},
{
"key": "secrets_manager_cbr_rules"
Expand Down
7 changes: 3 additions & 4 deletions solutions/fully-configurable/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ This solution supports the following:
| <a name="input_ibmcloud_api_key"></a> [ibmcloud\_api\_key](#input\_ibmcloud\_api\_key) | The IBM Cloud API key used to provision 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_encryption_enabled"></a> [kms\_encryption\_enabled](#input\_kms\_encryption\_enabled) | Set to true to enable Secrets Manager Secrets Encryption using customer managed keys. When set to true, a value must be passed for either `existing_kms_instance_crn` or `existing_secrets_manager_kms_key_crn`. Cannot be set to true if passing a value for `existing_secrets_manager_crn`. | `bool` | `false` | no |
| <a name="input_kms_endpoint_type"></a> [kms\_endpoint\_type](#input\_kms\_endpoint\_type) | The endpoint for communicating with the Key Protect or Hyper Protect Crypto Services instance. Possible values: `public`, `private`. Applies only if `existing_secrets_manager_kms_key_crn` is not specified. | `string` | `"private"` | no |
| <a name="input_kms_key_name"></a> [kms\_key\_name](#input\_kms\_key\_name) | The name for the new root key. Applies only if `existing_secrets_manager_kms_key_crn` is not specified. If a prefix input variable is passed, it is added to the value in the `<prefix>-value` format. | `string` | `"secrets-manager-key"` | no |
| <a name="input_kms_key_ring_name"></a> [kms\_key\_ring\_name](#input\_kms\_key\_ring\_name) | The name for the new key ring to store the key. Applies only if `existing_secrets_manager_kms_key_crn` is not specified. If a prefix input variable is passed, it is added to the value in the `<prefix>-value` format. . | `string` | `"secrets-manager-key-ring"` | no |
| <a name="input_prefix"></a> [prefix](#input\_prefix) | The prefix to add to all resources created by this solution. To not use any prefix value, you can set this value to `null` or an empty string. | `string` | n/a | yes |
Expand All @@ -73,9 +72,9 @@ This solution supports the following:
| <a name="input_secrets_manager_instance_name"></a> [secrets\_manager\_instance\_name](#input\_secrets\_manager\_instance\_name) | The name to give the Secrets Manager instance provisioned by this solution. If a prefix input variable is specified, it is added to the value in the `<prefix>-value` format. Applies only if `existing_secrets_manager_crn` is not provided. | `string` | `"secrets-manager"` | no |
| <a name="input_secrets_manager_resource_tags"></a> [secrets\_manager\_resource\_tags](#input\_secrets\_manager\_resource\_tags) | The list of resource tags you want to associate with your Secrets Manager instance. Applies only if `existing_secrets_manager_crn` is not provided. | `list(any)` | `[]` | no |
| <a name="input_service_plan"></a> [service\_plan](#input\_service\_plan) | The pricing plan to use when provisioning a Secrets Manager instance. Possible values: `standard`, `trial`. You can create only one Trial instance of Secrets Manager per account. Before you can create a new Trial instance, you must delete the existing Trial instance and its reclamation. [Learn more](https://cloud.ibm.com/docs/secrets-manager?topic=secrets-manager-create-instance&interface=ui#upgrade-instance-standard). | `string` | n/a | yes |
| <a name="input_skip_event_notifications_iam_authorization_policy"></a> [skip\_event\_notifications\_iam\_authorization\_policy](#input\_skip\_event\_notifications\_iam\_authorization\_policy) | If set to true, this skips the creation of a service to service authorization from Secrets Manager to Event Notifications. If false, the service to service authorization is created. | `bool` | `false` | no |
| <a name="input_skip_sm_ce_iam_authorization_policy"></a> [skip\_sm\_ce\_iam\_authorization\_policy](#input\_skip\_sm\_ce\_iam\_authorization\_policy) | Whether to skip the creation of the IAM authorization policies required to enable the IAM credentials engine. If set to false, policies will be created that grants the Secrets Manager instance 'Operator' access to the IAM identity service, and 'Groups Service Member Manage' access to the IAM groups service. | `bool` | `false` | no |
| <a name="input_skip_sm_kms_iam_authorization_policy"></a> [skip\_sm\_kms\_iam\_authorization\_policy](#input\_skip\_sm\_kms\_iam\_authorization\_policy) | Set to true to skip the creation of an IAM authorization policy that permits all Secrets Manager instances in the resource group 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_secrets_manager_certificate_engine_iam_authorization_policy"></a> [skip\_secrets\_manager\_certificate\_engine\_iam\_authorization\_policy](#input\_skip\_secrets\_manager\_certificate\_engine\_iam\_authorization\_policy) | Whether to skip the creation of the IAM authorization policies required to enable the IAM credentials engine (if you are using an existing Secrets Manager isntance, attempting to re-create can cause conflicts if the policies already exist). If set to false, policies will be created that grants the Secrets Manager instance 'Operator' access to the IAM identity service, and 'Groups Service Member Manage' access to the IAM groups service. | `bool` | `false` | no |
| <a name="input_skip_secrets_manager_event_notifications_iam_authorization_policy"></a> [skip\_secrets\_manager\_event\_notifications\_iam\_authorization\_policy](#input\_skip\_secrets\_manager\_event\_notifications\_iam\_authorization\_policy) | If set to true, this skips the creation of a service to service authorization from Secrets Manager to Event Notifications. If false, the service to service authorization is created. | `bool` | `false` | no |
| <a name="input_skip_secrets_manager_kms_iam_authorization_policy"></a> [skip\_secrets\_manager\_kms\_iam\_authorization\_policy](#input\_skip\_secrets\_manager\_kms\_iam\_authorization\_policy) | Set to true to skip the creation of an IAM authorization policy that permits all Secrets Manager instances in the resource group 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 |

### Outputs

Expand Down
12 changes: 6 additions & 6 deletions solutions/fully-configurable/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ locals {
parsed_service_name = var.existing_kms_instance_crn != null ? module.kms_instance_crn_parser[0].service_name : (var.existing_secrets_manager_kms_key_crn != null ? module.kms_key_crn_parser[0].service_name : null)
is_hpcs_key = local.parsed_service_name == "hs-crypto" ? true : false

create_cross_account_auth_policy = var.existing_secrets_manager_crn == null && !var.skip_sm_kms_iam_authorization_policy && var.ibmcloud_kms_api_key != null
create_cross_account_auth_policy = var.existing_secrets_manager_crn == null && !var.skip_secrets_manager_kms_iam_authorization_policy && var.ibmcloud_kms_api_key != null
create_cross_account_hpcs_auth_policy = local.create_cross_account_auth_policy == true && local.is_hpcs_key ? 1 : 0

kms_service_name = var.existing_secrets_manager_kms_key_crn != null ? module.kms_key_crn_parser[0].service_name : (var.existing_kms_instance_crn != null ? module.kms_instance_crn_parser[0].service_name : null)
Expand Down Expand Up @@ -134,8 +134,8 @@ module "kms" {
create_key_protect_instance = false
region = local.kms_region
existing_kms_instance_crn = var.existing_kms_instance_crn
key_ring_endpoint_type = var.kms_endpoint_type
key_endpoint_type = var.kms_endpoint_type
key_ring_endpoint_type = "private"
key_endpoint_type = "private"
keys = [
{
key_ring_name = local.kms_key_ring_name
Expand Down Expand Up @@ -174,16 +174,16 @@ module "secrets_manager" {
secrets_manager_name = "${local.prefix}${var.secrets_manager_instance_name}"
sm_service_plan = var.service_plan
sm_tags = var.secrets_manager_resource_tags
skip_iam_authorization_policy = var.skip_sm_ce_iam_authorization_policy
skip_iam_authorization_policy = var.skip_secrets_manager_certificate_engine_iam_authorization_policy
# kms dependency
is_hpcs_key = local.is_hpcs_key
kms_encryption_enabled = var.kms_encryption_enabled
kms_key_crn = local.kms_key_crn
skip_kms_iam_authorization_policy = var.skip_sm_kms_iam_authorization_policy || local.create_cross_account_auth_policy
skip_kms_iam_authorization_policy = var.skip_secrets_manager_kms_iam_authorization_policy || local.create_cross_account_auth_policy
# event notifications dependency
enable_event_notification = local.enable_event_notifications
existing_en_instance_crn = var.existing_event_notifications_instance_crn
skip_en_iam_authorization_policy = var.skip_event_notifications_iam_authorization_policy
skip_en_iam_authorization_policy = var.skip_secrets_manager_event_notifications_iam_authorization_policy
cbr_rules = var.secrets_manager_cbr_rules
endpoint_type = var.secrets_manager_endpoint_type
allowed_network = var.allowed_network
Expand Down
18 changes: 4 additions & 14 deletions solutions/fully-configurable/variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,9 @@ variable "service_plan" {
}
}

variable "skip_sm_ce_iam_authorization_policy" {
variable "skip_secrets_manager_certificate_engine_iam_authorization_policy" {
type = bool
description = "Whether to skip the creation of the IAM authorization policies required to enable the IAM credentials engine. If set to false, policies will be created that grants the Secrets Manager instance 'Operator' access to the IAM identity service, and 'Groups Service Member Manage' access to the IAM groups service."
description = "Whether to skip the creation of the IAM authorization policies required to enable the IAM credentials engine (if you are using an existing Secrets Manager isntance, attempting to re-create can cause conflicts if the policies already exist). If set to false, policies will be created that grants the Secrets Manager instance 'Operator' access to the IAM identity service, and 'Groups Service Member Manage' access to the IAM groups service."
default = false
}

Expand Down Expand Up @@ -149,7 +149,7 @@ variable "secret_groups" {
# Key Protect
########################################################################################################################

variable "skip_sm_kms_iam_authorization_policy" {
variable "skip_secrets_manager_kms_iam_authorization_policy" {
type = bool
description = "Set to true to skip the creation of an IAM authorization policy that permits all Secrets Manager instances in the resource group 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 Expand Up @@ -210,16 +210,6 @@ variable "existing_kms_instance_crn" {
}
}

variable "kms_endpoint_type" {
Copy link
Contributor

@ocofaigh ocofaigh May 19, 2025

Choose a reason for hiding this comment

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

I think we can keep the variable but hide it in the ibm_catalog.json

type = string
description = "The endpoint for communicating with the Key Protect or Hyper Protect Crypto Services instance. Possible values: `public`, `private`. Applies only if `existing_secrets_manager_kms_key_crn` is not specified."
default = "private"
validation {
condition = can(regex("public|private", var.kms_endpoint_type))
error_message = "The kms_endpoint_type value must be 'public' or 'private'."
}
}

variable "kms_key_ring_name" {
type = string
default = "secrets-manager-key-ring"
Expand Down Expand Up @@ -249,7 +239,7 @@ variable "existing_event_notifications_instance_crn" {
default = null
}

variable "skip_event_notifications_iam_authorization_policy" {
variable "skip_secrets_manager_event_notifications_iam_authorization_policy" {
type = bool
description = "If set to true, this skips the creation of a service to service authorization from Secrets Manager to Event Notifications. If false, the service to service authorization is created."
default = false
Expand Down
Loading