fix: Improve security group creation logic #375
Conversation
Update conditional logic for creating security groups and subnet data sources to handle cases where vpc_id is null and subnet_ids are provided. Use the Terraform `one()` function for safer vpc_id selection. This ensures correct resource creation and avoids potential errors in edge cases.
The conditional for the aws_subnet data resource count no longer checks if var.vpc_id is null. This simplifies the logic and ensures the subnet data resource is created whenever a security group is needed and subnet_ids are provided.
The conditional for the aws_subnet data resource count no longer checks if var.vpc_id is null. This simplifies the logic and ensures the subnet data resource is created whenever a security group is needed and subnet_ids are provided.
RoseSecurity changed the title 
| | ||
| data "aws_subnet" "this" { | ||
| count = local.create_security_group ? 1 : 0 | ||
| count = local.create_security_group && length(var.subnet_ids) > 0 ? 1 : 0 |
There was a problem hiding this comment.
this won't work when passing computed values
There was a problem hiding this comment.
Very true, should we update the default variable declaration to require the subnet_ids to be provided then?
There was a problem hiding this comment.
thats now how we manage variables - we provide default values so that we can disable all resource creation by simply setting create = true. Terraform doesn't allow us to simply disable resource creation and ignore required values (they aren't required if the resource is not going to be created/executed), which results in us providing a value, any value, to ensure we can disable without forcing users to provide values
There was a problem hiding this comment.
This is fair and sounds more like the user didn't properly enter the parameters they needed with the configuration they were attempting to construct. Closing this PR now
RoseSecurity changed the title RoseSecurity changed the title 2 participants
Description
subnet_idswas declared as optional but caused an error when not provided. The problem occurred whencreate_security_groupwas enabled (the default) withnetwork_mode = "awsvpc"butsubnet_idswas empty. The data source attempted to queryelement(var.subnet_ids, 0)on an empty list, causing a runtime error. The fix adds conditional logic to only create the subnet data source whensubnet_idsis non-empty, and only creates the security group when subnet information is available to derive or use the VPC ID. The VPC ID reference now uses theone()function to safely handle the optional data source, preferring an explicitvpc_idwhen provided. The count expressions avoid null checks to prevent Terraform planning errors whenvpc_idis a computed value from another resource. Users can now omitsubnet_idswhen they don't need the module to create a security group, or provide onlysubnet_idsto have the VPC ID automatically derived, making the module's behavior consistent with the variable's optional declaration.Motivation and Context
How Has This Been Tested?
examples/*projectspre-commit run -aon my pull request