[RFC,02/31] terraform/OCI: One default value to rule them

Commit Message

Chuck Lever April 1, 2025, 12:59 a.m. UTC

From: Chuck Lever <[e-mail]>

There are 3 sets of default values for each terraform setting.

1. The default value set in terraform/oci/Kconfig
2. The default value in playbooks/roles/gen_tfvars/defaults/main.yml
3. The default value specified in terraform/oci/vars.tf

Often these defaults are different values. It's confusing for
developers, but worse is that it can result in hard-to-diagnose
terraform provider error messages (eg, "Where did that 'invalid'
come from?").

IMO a better plan is to let Kconfig provide the only default value,
and remove the other two. We then have a single authoritative
default value for each setting.

If a required variable is missing a value during "terraform plan",
that will always be a local bug, not something exposed to the
provider to report via an obscure error message.

There are two exceptions:

- For  Kconfig booleans, often the variable is defined with a "y"
  value in the "true" case, but not defined at all in the "false"
  case. So for booleans, the gen_tfvars template needs a default
  value in gen_tfvars/defaults/main.yml of "false".

- Where a terraform resource expects an argument to be not present
  in some configurations, then vars.tf needs to explicitly define a
  default value of "null". Only in those cases it is OK for no
  value to be provided by Kconfig.

This is a refactoring change; no behavior change is intended.

Signed-off-by: Chuck Lever <[e-mail]>
---
 playbooks/roles/gen_tfvars/defaults/main.yml | 18 +--------
 terraform/oci/vars.tf                        | 42 ++++++++++----------
 2 files changed, 21 insertions(+), 39 deletions(-)

Patch

diff --git a/playbooks/roles/gen_tfvars/defaults/main.yml b/playbooks/roles/gen_tfvars/defaults/main.yml
index e892d85572a8..f64102b2fbf5 100644
--- a/playbooks/roles/gen_tfvars/defaults/main.yml
+++ b/playbooks/roles/gen_tfvars/defaults/main.yml
@@ -48,23 +48,7 @@  terraform_gce_scatch_disk_type: "invalid"
 terraform_gce_image_name: "invalid"
 terraform_gce_credentials: "invalid"
 
-terraform_oci_region: "invalid"
-terraform_oci_tenancy_ocid: "invalid"
-terraform_oci_user_ocid: "invalid"
-terraform_oci_user_private_key_path: "invalid"
-terraform_oci_user_fingerprint: "invalid"
-terraform_oci_availablity_domain_ocid: "invalid"
-terraform_oci_compartment_ocid: "invalid"
-terraform_oci_shape: "invalid"
-terraform_oci_instance_flex_ocpus: "invalid"
-terraform_oci_instance_flex_memory_in_gbs: "invalid"
-terraform_oci_os_image_ocid: "invalid"
-terraform_oci_assign_public_ip: "invalid"
-terraform_oci_subnet_ocid: "invalid"
-terraform_oci_data_volume_display_name: "invalid"
-terraform_oci_data_volume_device_file_name: "invalid"
-terraform_oci_sparse_volume_display_name: "invalid"
-terraform_oci_sparse_volume_device_file_name: "invalid"
+terraform_oci_assign_public_ip: "false"
 
 terraform_openstack_cloud_name: "invalid"
 terraform_openstack_instance_prefix: "invalid"
diff --git a/terraform/oci/vars.tf b/terraform/oci/vars.tf
index f43e1ee281fc..4c6383d9b231 100644
--- a/terraform/oci/vars.tf
+++ b/terraform/oci/vars.tf
@@ -1,103 +1,101 @@ 
 variable "oci_region" {
   description = "An OCI region"
-  default = ""
+  type        = string
 }
 
 variable "oci_tenancy_ocid" {
   description = "OCID of your tenancy"
-  default = ""
+  type        = string
 }
 
 variable "oci_user_ocid" {
   description = "OCID of the user calling the API"
-  default = ""
+  type        = string
 }
 
 variable "oci_user_private_key_path" {
   description = "The path of the private key stored on your computer"
-  default = ""
+  type        = string
 }
 
 variable "oci_user_fingerprint" {
   description = "Fingerprint for the key pair being used"
-  default = ""
+  type        = string
 }
 
 variable "oci_availablity_domain" {
   description = "Name of availability domain"
-  default = ""
+  type        = string
 }
 
 variable "oci_compartment_ocid" {
   description = "OCID of compartment"
-  default = ""
+  type        = string
 }
 
 variable "oci_shape" {
   description = "Shape name"
-  default = ""
+  type        = string
 }
 
 variable "oci_instance_flex_ocpus" {
+  default     = null
   description = "The total number of OCPUs available to the instance."
-  type = number
-  default = null
+  type        = number
 }
 
 variable "oci_instance_flex_memory_in_gbs" {
+  default     = null
   description = "The total amount of memory available to the instance, in gigabytes."
-  type = number
-  default = null
+  type        = number
 }
 
 variable "oci_os_image_ocid" {
   description = "OCID of OS image"
-  default = ""
+  type        = string
 }
 
 variable "oci_assign_public_ip" {
   description = "Assign public IP to the instance"
-  default = false
+  type        = bool
 }
 
 variable "oci_subnet_ocid" {
   description = "Subnet OCID"
-  default = ""
+  type        = string
 }
 
 variable "oci_volumes_enable_extra" {
   description = "Create additional block volumes per instance"
-  default     = false
+  type        = bool
 }
 
 variable "oci_volumes_per_instance" {
   description = "The count of additional block volumes per instance"
   type        = number
-  default     = 0
 }
 
 variable "oci_volumes_size" {
   description = "The size of additional block volumes, in gibibytes"
   type        = number
-  default     = 0
 }
 
 variable "oci_data_volume_display_name" {
   description = "Display name to use for the data volume"
-  default = "data"
+  type        = string
 }
 
 variable oci_data_volume_device_file_name {
   description = "Data volume's device file name"
-  default = "/dev/oracleoci/oraclevdb"
+  type        = string
 }
 
 variable "oci_sparse_volume_display_name" {
   description = "Display name to use for the sparse volume"
-  default = "sparse"
+  type        = string
 }
 
 variable oci_sparse_volume_device_file_name {
   description = "Sparse volume's device file name"
-  default = "/dev/oracleoci/oraclevdc"
+  type        = string
 }