Address reviewer comments

Author: Nuru

examples/complete/main.tf

@@ -17,7 +17,7 @@ module "new_security_group" {
   source = "../.."
 
   vpc_id              = module.vpc.vpc_id
-  open_egress_enabled = true
+  allow_all_egress = true
   rule_matrix = {
     # Allow ingress on ports 22 and 80 from created security grup, existing security group, and CIDR "10.0.0.0/8"
     source_security_group_ids = [aws_security_group.existing.id]

examples/complete/outputs.tf

@@ -1,25 +1,25 @@
-output "new_sg_id" {
+output "created_sg_id" {
   description = "The new one Security Group ID"
   value       = module.new_security_group.id
 }
 
-output "new_sg_arn" {
+output "created_sg_arn" {
   description = "The new one Security Group ARN"
   value       = module.new_security_group.arn
 }
 
-output "new_sg_name" {
+output "created_sg_name" {
   description = "The new one Security Group Name"
   value       = module.new_security_group.name
 }
 
-output "new_sg_details" {
+output "created_sg_details" {
   description = "Details about the security group created"
   value       = module.new_security_group.security_group_details
 }
 
-output "new_sg_rules" {
-  description = "Details about all the security group rules created for the existing security group"
+output "created_sg_rules" {
+  description = "Details about all the security group rules created for the created security group"
   value       = module.new_security_group.rules
 }
 

main.tf

@@ -9,7 +9,7 @@ locals {
   security_group_id = local.enabled ? (
     # Use coalesce() here to hack an error message into the output
     var.create_security_group ? local.created_security_group.id : coalesce(var.existing_security_group_id,
-    "use_existing_security_group is true, but no ID supplied ")
+    "`create_security_group` is false, but no ID was supplied ")
   ) : null
 
   rules = local.enabled && var.rules != null ? {
@@ -118,7 +118,7 @@ resource "aws_security_group_rule" "cidr" {
 
 
 resource "aws_security_group_rule" "egress" {
-  count = local.enabled && var.open_egress_enabled ? 1 : 0
+  count = local.enabled && var.allow_all_egress ? 1 : 0
 
   security_group_id = local.security_group_id
   type              = "egress"

test/src/examples_complete_test.go

@@ -40,9 +40,9 @@ func TestExamplesComplete(t *testing.T) {
 	// Run `terraform output` to get the value of an output variable
 
 	// Verify that outputs are valid when `security_group_enabled=true`
-	newSgID := terraform.Output(t, terraformOptions, "new_sg_id")
-	newSgARN := terraform.Output(t, terraformOptions, "new_sg_arn")
-	newSgName := terraform.Output(t, terraformOptions, "new_sg_name")
+	newSgID := terraform.Output(t, terraformOptions, "created_sg_id")
+	newSgARN := terraform.Output(t, terraformOptions, "created_sg_arn")
+	newSgName := terraform.Output(t, terraformOptions, "created_sg_name")
 
 	assert.Contains(t, newSgID, "sg-", "SG ID should contains substring 'sg-'")
 	assert.Contains(t, newSgARN, "arn:aws:ec2", "SG ID should contains substring 'arn:aws:ec2'")

variables.tf

@@ -55,38 +55,40 @@ variable "rules" {
   EOT
 }
 
-variable "open_egress_enabled" {
+variable "allow_all_egress" {
   type        = bool
   default     = false
   description = <<-EOT
-    A convenience. Add to the rules in `var.rules` a rule that allows all egress.
+    A convenience that adds to the rules in `var.rules` a rule that allows all egress.
     If this is false and `var.rules` does not specify any egress rules, then
     no egress will be allowed.
     EOT
 }
 
 variable "rule_matrix" {
+    # rule_matrix is independent of the `rules` input.
+    # Only the rules specified in the `rule_matrix` object are applied to the subjects.
+    #  Schema:
+    #  {
+    #    # these top level lists define all the subjects to which rule_matrix rules will be applied
+    #    source_security_group_ids = list of source security group IDs to apply all rules to
+    #    cidr_blocks = list of ipv4 CIDR blocks to apply all rules to
+    #    ipv6_cidr_blocks= list of ipv6 CIDR blocks to apply all rules to
+    #    prefix_list_ids = list of prefix list IDs to apply all rules to
+    #    self = # set "true" to apply the rules to the created or existing security group
+    #
+    #    # each rule in the rules list will be applied to every subject defined above
+    #    rules = [{
+    #      type = "egress"
+    #      from_port = 0
+    #      to_port = 65535
+    #      protocol = "all"
+    #      description = "Allow full egress"
+    #    }]
+
   type        = any
   default     = { rules = [] }
   description = <<-EOT
-    A convenience. Apply the same list of rules to all the provided security groups and CIDRs and self.
-    Type is object as specified in the default, but keys are optional except for `rules`.
-    The `rules` list is a list of maps that are fully compatible with the `aws_security_group_rule` resource,
-    but any keys already at the top level will be ignored.  Rules keys listed in the default are required, except for `description`.
-    All elements of the list must have the same set of keys and each key must have a consistent value type.
-    Example:
-    {
-      source_security_group_ids = []
-      cidr_blocks= []
-      ipv6_cidr_blocks= []
-      prefix_list_ids = []
-      self = true
-      rules = [{
-        type = "egress"
-        from_port = 0
-        to_port = 65535
-        protocol = "all"
-        description = "Allow full egress"
-    }]
+    A convenient way to apply the same set of rules to a set of subjects. See README for details.
     EOT
 }