addressed review comments

Author: Vipin Kumar

README.md

@@ -305,7 +305,6 @@ Optionally, you need the following permissions to attach Access Management tags
 | <a name="input_existing_secrets_manager_instance_crn"></a> [existing\_secrets\_manager\_instance\_crn](#input\_existing\_secrets\_manager\_instance\_crn) | CRN of secrets manager instance where ingress certificate secrets will be stored. | `string` | `null` | no |
 | <a name="input_force_delete_storage"></a> [force\_delete\_storage](#input\_force\_delete\_storage) | Flag indicating whether or not to delete attached storage when destroying the cluster - Default: false | `bool` | `false` | no |
 | <a name="input_ignore_worker_pool_size_changes"></a> [ignore\_worker\_pool\_size\_changes](#input\_ignore\_worker\_pool\_size\_changes) | Enable if using worker autoscaling. Stops Terraform managing worker count | `bool` | `false` | no |
-| <a name="input_is_default_secrets_manager_instance"></a> [is\_default\_secrets\_manager\_instance](#input\_is\_default\_secrets\_manager\_instance) | Whether the secrets manager instance provided will be default for storing ingress certificates. | `bool` | `true` | no |
 | <a name="input_kms_config"></a> [kms\_config](#input\_kms\_config) | Use to attach a KMS instance to the cluster. If account\_id is not provided, defaults to the account in use. | <pre>object({<br/>    crk_id           = string<br/>    instance_id      = string<br/>    private_endpoint = optional(bool, true) # defaults to true<br/>    account_id       = optional(string)     # To attach KMS instance from another account<br/>    wait_for_apply   = optional(bool, true) # defaults to true so terraform will wait until the KMS is applied to the master, ready and deployed<br/>  })</pre> | `null` | no |
 | <a name="input_manage_all_addons"></a> [manage\_all\_addons](#input\_manage\_all\_addons) | Instructs Terraform to manage all cluster addons, even if addons were installed outside of the module. If set to 'true' this module will destroy any addons that were installed by other sources. | `bool` | `false` | no |
 | <a name="input_number_of_lbs"></a> [number\_of\_lbs](#input\_number\_of\_lbs) | The number of LBs to associated the `additional_lb_security_group_names` security group with. | `number` | `1` | no |
@@ -314,7 +313,7 @@ Optionally, you need the following permissions to attach Access Management tags
 | <a name="input_pod_subnet_cidr"></a> [pod\_subnet\_cidr](#input\_pod\_subnet\_cidr) | Specify a custom subnet CIDR to provide private IP addresses for pods. The subnet must have a CIDR of at least `/23` or larger. Default value is `172.30.0.0/16` when the variable is set to `null`. | `string` | `null` | no |
 | <a name="input_region"></a> [region](#input\_region) | The IBM Cloud region where the cluster will be provisioned. | `string` | n/a | yes |
 | <a name="input_resource_group_id"></a> [resource\_group\_id](#input\_resource\_group\_id) | The Id of an existing IBM Cloud resource group where the cluster will be grouped. | `string` | n/a | yes |
-| <a name="input_secrets_manager_secret_group_id"></a> [secrets\_manager\_secret\_group\_id](#input\_secrets\_manager\_secret\_group\_id) | Secret group id where ingress secrets will be kept in the secrets manager instance. If not specified, default group will be used. | `string` | `""` | no |
+| <a name="input_secrets_manager_secret_group_id"></a> [secrets\_manager\_secret\_group\_id](#input\_secrets\_manager\_secret\_group\_id) | Secret group id where ingress secrets will be kept in the secrets manager instance. If not specified, default group will be used. | `string` | `null` | no |
 | <a name="input_service_subnet_cidr"></a> [service\_subnet\_cidr](#input\_service\_subnet\_cidr) | Specify a custom subnet CIDR to provide private IP addresses for services. The subnet must be at least `/24` or larger. Default value is `172.21.0.0/16` when the variable is set to `null`. | `string` | `null` | no |
 | <a name="input_tags"></a> [tags](#input\_tags) | Metadata labels describing this cluster deployment, i.e. test | `list(string)` | `[]` | no |
 | <a name="input_use_existing_cos"></a> [use\_existing\_cos](#input\_use\_existing\_cos) | Flag indicating whether or not to use an existing COS instance for OpenShift internal registry storage. Only applicable if 'enable\_registry\_storage' is true | `bool` | `false` | no |

ibm_catalog.json

@@ -391,9 +391,6 @@
             },
             {
               "key": "secrets_manager_secret_group_id"
-            },
-            {
-              "key": "is_default_secrets_manager_instance"
             }
           ],
           "dependencies": [

main.tf

@@ -753,6 +753,6 @@ resource "ibm_container_ingress_instance" "instance" {
   depends_on      = [ibm_container_vpc_cluster.cluster, ibm_container_vpc_cluster.autoscaling_cluster, ibm_container_vpc_worker_pool.pool, ibm_container_vpc_worker_pool.autoscaling_pool]
   cluster         = var.cluster_name
   instance_crn    = var.existing_secrets_manager_instance_crn
-  is_default      = var.is_default_secrets_manager_instance
+  is_default      = true
   secret_group_id = var.secrets_manager_secret_group_id
 }

solutions/fully-configurable/README.md

@@ -83,7 +83,6 @@ The following resources are provisioned by this example:
 | <a name="input_ibmcloud_api_key"></a> [ibmcloud\_api\_key](#input\_ibmcloud\_api\_key) | The IBM Cloud api key. | `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 for the cluster. If not specified, the 'ibmcloud\_api\_key' variable is used. Specify this key if the KMS instance in `existing_kms_instance_crn` is in an account that is different from the cluster's account. Leave this input empty if both the cluster and the KMS instance are in the same account. | `string` | `null` | no |
 | <a name="input_ignore_worker_pool_size_changes"></a> [ignore\_worker\_pool\_size\_changes](#input\_ignore\_worker\_pool\_size\_changes) | Enable if using worker autoscaling. Stops Terraform managing worker count. | `bool` | `false` | no |
-| <a name="input_is_default_secrets_manager_instance"></a> [is\_default\_secrets\_manager\_instance](#input\_is\_default\_secrets\_manager\_instance) | Whether the secrets manager instance provided will be default for storing ingress certificates. | `bool` | `true` | no |
 | <a name="input_kms_encryption_enabled_boot_volume"></a> [kms\_encryption\_enabled\_boot\_volume](#input\_kms\_encryption\_enabled\_boot\_volume) | Set this to true to control the encryption keys used to encrypt the data that for the block storage volumes for VPC. If set to false, the data is encrypted by using randomly generated keys. For more info on encrypting block storage volumes, see https://cloud.ibm.com/docs/vpc?topic=vpc-creating-instances-byok | `bool` | `false` | no |
 | <a name="input_kms_encryption_enabled_cluster"></a> [kms\_encryption\_enabled\_cluster](#input\_kms\_encryption\_enabled\_cluster) | Set to true to enable KMS encryption for the cluster's Object Storage bucket. When set to true, a value must be passed for either `existing_cluster_kms_key_crn` or `existing_kms_instance_crn`. | `bool` | `false` | 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`. Applies only if `kms_encryption_enabled_cluster` is true | `string` | `"private"` | no |
@@ -94,7 +93,7 @@ The following resources are provisioned by this example:
 | <a name="input_pod_subnet_cidr"></a> [pod\_subnet\_cidr](#input\_pod\_subnet\_cidr) | Specify a custom subnet CIDR to provide private IP addresses for pods. The subnet must have a CIDR of at least `/23` or larger. Default value is `172.30.0.0/16` when the variable is set to `null`. | `string` | `null` | no |
 | <a name="input_prefix"></a> [prefix](#input\_prefix) | The prefix to add to all resources that this solution creates (e.g `prod`, `test`, `dev`). To not use any prefix value, you can set this value to `null` or an empty string. | `string` | n/a | yes |
 | <a name="input_provider_visibility"></a> [provider\_visibility](#input\_provider\_visibility) | Set the visibility value for the IBM terraform provider. Supported values are `public`, `private`, `public-and-private`. [Learn more](https://registry.terraform.io/providers/IBM-Cloud/ibm/latest/docs/guides/custom-service-endpoints). | `string` | `"private"` | no |
-| <a name="input_secrets_manager_secret_group_id"></a> [secrets\_manager\_secret\_group\_id](#input\_secrets\_manager\_secret\_group\_id) | Secret group id where ingress secrets will be kept in the secrets manager instance. If not specified, default group will be used. | `string` | `""` | no |
+| <a name="input_secrets_manager_secret_group_id"></a> [secrets\_manager\_secret\_group\_id](#input\_secrets\_manager\_secret\_group\_id) | Secret group id where ingress secrets will be kept in the secrets manager instance. If not specified, default group will be used. | `string` | `null` | no |
 | <a name="input_service_subnet_cidr"></a> [service\_subnet\_cidr](#input\_service\_subnet\_cidr) | Specify a custom subnet CIDR to provide private IP addresses for services. The subnet must be at least `/24` or larger. Default value is `172.21.0.0/16` when the variable is set to `null`. | `string` | `null` | no |
 | <a name="input_use_private_endpoint"></a> [use\_private\_endpoint](#input\_use\_private\_endpoint) | Set this to true to force all api calls to use the IBM Cloud private endpoints. | `bool` | `true` | no |
 | <a name="input_verify_worker_network_readiness"></a> [verify\_worker\_network\_readiness](#input\_verify\_worker\_network\_readiness) | By setting this to true, a script will run kubectl commands to verify that all worker nodes can communicate successfully with the master. If the runtime does not have access to the kube cluster to run kubectl commands, this should be set to false. | `bool` | `true` | no |

solutions/fully-configurable/main.tf

@@ -233,5 +233,4 @@ module "ocp_base" {
   enable_secrets_manager_for_ingress    = var.enable_secrets_manager_for_ingress
   existing_secrets_manager_instance_crn = var.existing_secrets_manager_instance_crn
   secrets_manager_secret_group_id       = var.secrets_manager_secret_group_id
-  is_default_secrets_manager_instance   = var.is_default_secrets_manager_instance
 }

solutions/fully-configurable/variables.tf

@@ -486,7 +486,10 @@ variable "enable_secrets_manager_for_ingress" {
   description = "Whether to enable secrets manager for storing ingress certificate."
   default     = false
   validation {
-    condition     = !var.enable_secrets_manager_for_ingress || var.existing_secrets_manager_instance_crn != null
+    condition = anytrue([
+      !var.enable_secrets_manager_for_ingress,
+      var.existing_secrets_manager_instance_crn != null
+    ])
     error_message = "'existing_secrets_manager_instance_crn' should be provided if setting 'enable_secrets_manager_for_ingress' to true."
   }
 }
@@ -500,15 +503,5 @@ variable "existing_secrets_manager_instance_crn" {
 variable "secrets_manager_secret_group_id" {
   type        = string
   description = "Secret group id where ingress secrets will be kept in the secrets manager instance. If not specified, default group will be used."
-  default     = ""
-}
-
-variable "is_default_secrets_manager_instance" {
-  type        = bool
-  description = "Whether the secrets manager instance provided will be default for storing ingress certificates."
-  default     = true
-  validation {
-    condition     = var.is_default_secrets_manager_instance || var.secrets_manager_secret_group_id == null
-    error_message = "Secret groups are only supported for default Secrets Manager instances. Either set 'is_default_secrets_manager_instance' to true or do not provide a value for 'secrets_manager_secret_group_id'."
-  }
+  default     = null
 }

variables.tf

@@ -410,7 +410,10 @@ variable "enable_secrets_manager_for_ingress" {
   description = "Whether to enable secrets manager for storing ingress certificate."
   default     = false
   validation {
-    condition     = !var.enable_secrets_manager_for_ingress || var.existing_secrets_manager_instance_crn != null
+    condition = anytrue([
+      !var.enable_secrets_manager_for_ingress,
+      var.existing_secrets_manager_instance_crn != null
+    ])
     error_message = "'existing_secrets_manager_instance_crn' should be provided if setting 'enable_secrets_manager_for_ingress' to true."
   }
 }
@@ -424,15 +427,5 @@ variable "existing_secrets_manager_instance_crn" {
 variable "secrets_manager_secret_group_id" {
   type        = string
   description = "Secret group id where ingress secrets will be kept in the secrets manager instance. If not specified, default group will be used."
-  default     = ""
-}
-
-variable "is_default_secrets_manager_instance" {
-  type        = bool
-  description = "Whether the secrets manager instance provided will be default for storing ingress certificates."
-  default     = true
-  validation {
-    condition     = var.is_default_secrets_manager_instance || var.secrets_manager_secret_group_id == null
-    error_message = "Secret groups are only supported for default Secrets Manager instances. Either set 'is_default_secrets_manager_instance' to true or do not provide a value for 'secrets_manager_secret_group_id'."
-  }
+  default     = null
 }