From 8428254db9a7a6129cc80f6e900234fc2197e3ff Mon Sep 17 00:00:00 2001 From: Khuzaima-Shakeel Date: Tue, 8 Apr 2025 13:39:00 +0530 Subject: [PATCH 1/3] feat: updated code to use cross-object referencing for validations --- main.tf | 40 ------------------------------- variables.tf | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 40 deletions(-) diff --git a/main.tf b/main.tf index 05fbfba1..d908f2a0 100644 --- a/main.tf +++ b/main.tf @@ -1,46 +1,6 @@ ############################################################################## # terraform-ibm-landing-zone-vpc -############################################################################## -locals { - # input variable validation - # tflint-ignore: terraform_unused_declarations - validate_default_secgroup_rules = var.clean_default_sg_acl && (var.security_group_rules != null && length(var.security_group_rules) > 0) ? tobool("var.clean_default_sg_acl is true and var.security_group_rules are not empty, which are in direct conflict of each other. If you would like the default VPC Security Group to be empty, you must remove default rules from var.security_group_rules.") : true - - # tflint-ignore: terraform_unused_declarations - validate_existing_vpc_id = !var.create_vpc && var.existing_vpc_id == null ? tobool("If var.create_vpc is false, then provide a value for var.existing_vpc_id to create vpc.") : true - - # tflint-ignore: terraform_unused_declarations - validate_existing_subnet_id = !var.create_subnets && length(var.existing_subnets) == 0 ? tobool("If var.create_subnet is false, then provide a value for var.existing_subnets to create subnets.") : true - # tflint-ignore: terraform_unused_declarations - validate_existing_vpc_and_subnet = var.create_vpc == true && var.create_subnets == false ? tobool("If user is not providing a vpc then they should also not be providing a subnet") : true - - # tflint-ignore: terraform_unused_declarations - validate_hub_vpc_input = (var.hub_vpc_id != null && var.hub_vpc_crn != null) ? tobool("var.hub_vpc_id and var.hub_vpc_crn are mutually exclusive. Hence cannot have values at the same time.") : true - - # tflint-ignore: terraform_unused_declarations - validate_hub_vpc_id_input = (var.enable_hub_vpc_id && var.hub_vpc_id == null) ? tobool("var.hub_vpc_id must be passed when var.enable_hub_vpc_id is True.") : true - - # tflint-ignore: terraform_unused_declarations - validate_enable_hub_vpc_id_input = (!var.enable_hub_vpc_id && var.hub_vpc_id != null) ? tobool("var.enable_hub_vpc_id must be true when var.hub_vpc_id is not null.") : true - # tflint-ignore: terraform_unused_declarations - validate_hub_vpc_crn_input = (var.enable_hub_vpc_crn && var.hub_vpc_crn == null) ? tobool("var.hub_vpc_crn must be passed when var.enable_hub_vpc_crn is True.") : true - - # tflint-ignore: terraform_unused_declarations - validate_enable_hub_vpc_crn_input = (!var.enable_hub_vpc_crn && var.hub_vpc_crn != null) ? tobool("var.enable_hub_vpc_crn must be true when var.hub_vpc_crn is not null.") : true - - # tflint-ignore: terraform_unused_declarations - validate_manual_servers_input = (var.resolver_type == "manual" && length(var.manual_servers) == 0) ? tobool("var.manual_servers must be set when var.resolver_type is manual") : true - - # tflint-ignore: terraform_unused_declarations - validate_resolver_type_input = (var.resolver_type != null && var.update_delegated_resolver == true) ? tobool("var.resolver_type cannot be set if var.update_delegated_resolver is set to true. Only one type of resolver can be created by VPC.") : true - - # tflint-ignore: terraform_unused_declarations - validate_vpc_flow_logs_inputs = (var.enable_vpc_flow_logs) ? ((var.create_authorization_policy_vpc_to_cos) ? ((var.existing_cos_instance_guid != null && var.existing_storage_bucket_name != null) ? true : tobool("Please provide COS instance & bucket name to create flow logs collector.")) : ((var.existing_storage_bucket_name != null) ? true : tobool("Please provide COS bucket name to create flow logs collector"))) : false - - # tflint-ignore: terraform_unused_declarations - validate_skip_spoke_auth_policy_input = (var.hub_account_id == null && !var.skip_spoke_auth_policy && !var.enable_hub && (var.enable_hub_vpc_id || var.enable_hub_vpc_crn)) ? tobool("var.hub_account_id must be set when var.skip_spoke_auth_policy is False and either var.enable_hub_vpc_id or var.enable_hub_vpc_crn is true.") : true -} ############################################################################## # Check if existing vpc id is passed diff --git a/variables.tf b/variables.tf index f0508036..8fd8e8d8 100644 --- a/variables.tf +++ b/variables.tf @@ -6,6 +6,16 @@ variable "create_vpc" { description = "Indicates whether user wants to use an existing vpc or create a new one. Set it to true to create a new vpc" type = bool default = true + + validation { + condition = var.create_vpc || var.existing_vpc_id != null + error_message = "If 'create_vpc' is false, then you must provide a value for 'existing_vpc_id'." + } + + validation { + condition = !(var.create_vpc == false && var.create_subnets == true) + error_message = "If 'create_vpc' is false, then 'create_subnets' must also be false. You cannot create subnets without providing a VPC." + } } variable "existing_vpc_id" { @@ -383,6 +393,11 @@ variable "create_subnets" { description = "Indicates whether user wants to use existing subnets or create new. Set it to true to create new subnets." type = bool default = true + + validation { + condition = var.create_subnets || length(var.existing_subnets) > 0 + error_message = "If 'create_subnets' is false, then you must provide a non-empty list for 'existing_subnets'." + } } variable "existing_subnets" { @@ -454,6 +469,11 @@ variable "security_group_rules" { ]) )) == 0 } + + validation { + error_message = "var.clean_default_sg_acl is true and var.security_group_rules are not empty, which are in direct conflict. If you want to clean the default SG, you must not pass security_group_rules." + condition = !(var.clean_default_sg_acl && length(var.security_group_rules) > 0) + } } variable "clean_default_sg_acl" { @@ -501,6 +521,18 @@ variable "enable_vpc_flow_logs" { description = "Flag to enable vpc flow logs. If true, flow log collector will be created" type = bool default = false + + validation { + condition = ( + !var.enable_vpc_flow_logs || + ( + var.create_authorization_policy_vpc_to_cos + ? (var.existing_cos_instance_guid != null && var.existing_storage_bucket_name != null) + : (var.existing_storage_bucket_name != null) + ) + ) + error_message = "To enable VPC flow logs, provide COS bucket name. If authorization policy creation is enabled, also provide COS instance GUID." + } } variable "create_authorization_policy_vpc_to_cos" { @@ -543,6 +575,16 @@ variable "skip_spoke_auth_policy" { description = "Set to true to skip the creation of an authorization policy between the DNS resolution spoke and hub, only enable this if a policy already exists between these two VPCs. See https://cloud.ibm.com/docs/vpc?topic=vpc-vpe-dns-sharing-s2s-auth&interface=ui for more details." type = bool default = false + + validation { + condition = ( + var.hub_account_id != null || + var.skip_spoke_auth_policy || + var.enable_hub || + !(var.enable_hub_vpc_id || var.enable_hub_vpc_crn) + ) + error_message = "var.hub_account_id must be set when var.skip_spoke_auth_policy is false and either var.enable_hub_vpc_id or var.enable_hub_vpc_crn is true and enable_hub is false." + } } variable "hub_account_id" { @@ -561,6 +603,16 @@ variable "hub_vpc_id" { description = "Indicates the id of the hub VPC for DNS resolution. See https://cloud.ibm.com/docs/vpc?topic=vpc-hub-spoke-model. Mutually exclusive with hub_vpc_crn." type = string default = null + + validation { + condition = !(var.hub_vpc_id != null && var.hub_vpc_crn != null) + error_message = "The inputs 'hub_vpc_id' and 'hub_vpc_crn' are mutually exclusive. Only one of them can be set at a time." + } + + validation { + condition = !(var.enable_hub_vpc_id && var.hub_vpc_id == null) + error_message = "The input 'hub_vpc_id' must be provided when 'enable_hub_vpc_id' is set to true." + } } variable "enable_hub_vpc_crn" { @@ -573,12 +625,22 @@ variable "hub_vpc_crn" { description = "Indicates the crn of the hub VPC for DNS resolution. See https://cloud.ibm.com/docs/vpc?topic=vpc-hub-spoke-model. Mutually exclusive with hub_vpc_id." type = string default = null + + validation { + condition = !(var.enable_hub_vpc_crn && var.hub_vpc_crn == null) + error_message = "The input 'hub_vpc_crn' must be provided when 'enable_hub_vpc_crn' is set to true." + } } variable "update_delegated_resolver" { description = "If set to true, and if the vpc is configured to be a spoke for DNS resolution (enable_hub_vpc_crn or enable_hub_vpc_id set), then the spoke VPC resolver will be updated to a delegated resolver." type = bool default = false + + validation { + condition = !(var.update_delegated_resolver == true && var.resolver_type != null) + error_message = "var.resolver_type cannot be set if var.update_delegated_resolver is true. Only one type of resolver can be created by VPC." + } } variable "skip_custom_resolver_hub_creation" { @@ -620,6 +682,11 @@ variable "manual_servers" { zone_affinity = optional(string) })) default = [] + + validation { + condition = !(var.resolver_type == "manual" && length(var.manual_servers) == 0) + error_message = "The input 'manual_servers' must be set when 'resolver_type' is 'manual'." + } } variable "dns_location" { From f7bb667f66a98d92a635b46e51e4ce6eb811eba9 Mon Sep 17 00:00:00 2001 From: Khuzaima-Shakeel Date: Mon, 14 Apr 2025 17:00:50 +0530 Subject: [PATCH 2/3] resolve review comments --- variables.tf | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/variables.tf b/variables.tf index 8fd8e8d8..683fa7d8 100644 --- a/variables.tf +++ b/variables.tf @@ -8,13 +8,13 @@ variable "create_vpc" { default = true validation { - condition = var.create_vpc || var.existing_vpc_id != null - error_message = "If 'create_vpc' is false, then you must provide a value for 'existing_vpc_id'." + condition = !(var.create_vpc == false && var.existing_vpc_id == null) + error_message = "You must either enable 'create_vpc' or provide 'existing_vpc_id', but not both or neither." } validation { condition = !(var.create_vpc == false && var.create_subnets == true) - error_message = "If 'create_vpc' is false, then 'create_subnets' must also be false. You cannot create subnets without providing a VPC." + error_message = "If create_vpc is false, then create_subnets must also be false. You cannot create subnets without first creating a VPC." } } @@ -393,11 +393,6 @@ variable "create_subnets" { description = "Indicates whether user wants to use existing subnets or create new. Set it to true to create new subnets." type = bool default = true - - validation { - condition = var.create_subnets || length(var.existing_subnets) > 0 - error_message = "If 'create_subnets' is false, then you must provide a non-empty list for 'existing_subnets'." - } } variable "existing_subnets" { @@ -408,6 +403,14 @@ variable "existing_subnets" { })) default = [] nullable = false + + validation { + condition = ( + (var.create_subnets && length(var.existing_subnets) == 0) || + (!var.create_subnets && length(var.existing_subnets) > 0) + ) + error_message = "You must either set 'create_subnets' to true and not provide 'existing_subnets', or set it to false and provide a non-empty list of 'existing_subnets'." + } } ############################################################################## @@ -471,8 +474,8 @@ variable "security_group_rules" { } validation { - error_message = "var.clean_default_sg_acl is true and var.security_group_rules are not empty, which are in direct conflict. If you want to clean the default SG, you must not pass security_group_rules." - condition = !(var.clean_default_sg_acl && length(var.security_group_rules) > 0) + condition = !(var.clean_default_sg_acl && var.security_group_rules != null && length(var.security_group_rules) > 0) + error_message = "var.clean_default_sg_acl is true and var.security_group_rules are not empty, which are in direct conflict. If you want to clean the default VPC Security Group, you must not pass security_group_rules." } } @@ -531,7 +534,7 @@ variable "enable_vpc_flow_logs" { : (var.existing_storage_bucket_name != null) ) ) - error_message = "To enable VPC flow logs, provide COS bucket name. If authorization policy creation is enabled, also provide COS instance GUID." + error_message = "To enable VPC Flow Logs, provide COS Bucket name. If you're creating an authorization policy then also provide COS instance GUID." } } From e06b3e65858baf66e3a28a8e2c84e1c1bf742995 Mon Sep 17 00:00:00 2001 From: Khuzaima-Shakeel Date: Thu, 17 Apr 2025 12:51:16 +0530 Subject: [PATCH 3/3] resolve review comments --- variables.tf | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/variables.tf b/variables.tf index 683fa7d8..71d5eed9 100644 --- a/variables.tf +++ b/variables.tf @@ -14,7 +14,7 @@ variable "create_vpc" { validation { condition = !(var.create_vpc == false && var.create_subnets == true) - error_message = "If create_vpc is false, then create_subnets must also be false. You cannot create subnets without first creating a VPC." + error_message = "You cannot create subnets without creating a VPC. Hence if 'create_vpc' is false, then 'create_subnets' can not be true." } } @@ -409,7 +409,7 @@ variable "existing_subnets" { (var.create_subnets && length(var.existing_subnets) == 0) || (!var.create_subnets && length(var.existing_subnets) > 0) ) - error_message = "You must either set 'create_subnets' to true and not provide 'existing_subnets', or set it to false and provide a non-empty list of 'existing_subnets'." + error_message = "You must either set 'create_subnets' to true and leave 'existing_subnets' empty, or set 'create_subnets' to false and provide a non-empty list for 'existing_subnets'." } }