Merge pull request #53 from aws-ia/usage-doc-updates

Improve usage and development documentation
aws-ia · Jun 7, 2022 · 26f0553 · 26f0553
2 parents b7396f0 + af5e78e
commit 26f0553
Show file tree

Hide file tree

Showing 7 changed files with 361 additions and 35 deletions.
diff --git a/.header.md b/.header.md
@@ -78,3 +78,63 @@ subnets = {
 ```
 
 The above example will cause only creating 2 new subnets in az `c` of the region being used.
+
+## Output manipulation
+
+The outputs in this module attempt to align to a methodology of outputting resource attributes in a reasonable collection. The benefit of this is that, most likely, attributes you want access to are already present without having to create new `output {}` for each possible attribute. The [potential] downside is that you will have to extract it yourself using HCL logic. Below are some common examples:
+
+### Extracting subnet IDs for private subnets
+
+Example Configuration:
+```terraform
+module "vpc" {
+  source  = "aws-ia/vpc/aws"
+  version = ">= 1.0.0"
+
+  name       = "multi-az-vpc"
+  cidr_block = "10.0.0.0/20"
+  az_count   = 3
+
+  subnets = {
+    private = { netmask = 24 }
+  }
+}
+```
+
+Extracting subnet_ids to a list (using `terraform console` for example output):
+```terraform
+> [ for _, value in module.vpc.private_subnet_attributes_by_az: value.id]
+[
+  "subnet-04a86315c4839b519",
+  "subnet-02a7249c8652a7136",
+  "subnet-09af79b5329b3681f",
+]
+```
+
+Alternatively, since these are maps, you can use key in another resource `for_each` loop. The benefit here is that your dependent resource will have keys that match the AZ the subnet is in:
+
+```terraform
+resource "aws_route53recoveryreadiness_cell" "cell_per_az" {
+  for_each = module.vpc.private_subnet_attributes_by_az
+
+  cell_name = "${each.key}-failover-cell-for-subnet-${each.value.id}"
+}
+...
+```
+
+Terraform Plan:
+
+```shell
+# aws_route53recoveryreadiness_cell.cell_per_az["us-east-1a"] will be created
++ resource "aws_route53recoveryreadiness_cell" "cell_per_az" {
+    + cell_name               = "us-east-1a-failover-cell-for-subnet-subnet-070696086c5864da1"
+    ...
+  }
+
+# aws_route53recoveryreadiness_cell.cell_per_az["us-east-1b"] will be created
+...
+```
+
+## Contributing
+
+Please see our [developer documentation](https://github.com/aws-ia/terraform-aws-vpc/contributing.md) for guidance on contributing to this module
diff --git a/README.md b/README.md
diff --git a/contributing.md b/contributing.md
@@ -0,0 +1,87 @@
+# Developer Documentation
+
+## Outputs Methodology
+
+This module organizes outputs by creating output collections of grouped entire resources. The benefit of this is that, most likely, attributes users want access to are already present without having to create new `output {}` for each possible attribute. The [potential] downside is that you will have to extract it yourself using HCL logic. See the [outputs.tf](https://github.com/aws-ia/terraform-aws-vpc/outputs.tf) for examples.
+
+Our naming convetion attempts to make the output content clear. `route_table_attributes_by_type_by_az` is a nested map of route table resource attributes grouped by their subnet type then by the az.  Example:
+```terraform
+route_table_attributes_by_type_by_az = {
+  "private" = {
+    "us-east-1a" = {
+      "id" = "rtb-0e77040c0598df003"
+      "route_table_id" = "rtb-0e77040c0598df003"
+      "tags" = tolist([
+        {
+          "key" = "Name"
+          "value" = "private-us-east-1a"
+        },
+      ])
+      "vpc_id" = "vpc-033e054f49409592a"
+    }
+    "us-east-1b" = {
+      ...
+    }
+  "public" = { ... }
+```
+
+## Adding new subnet types
+
+*Note: All subnet types **MUST** accept both `cidrs` and `netmask` arguments.*
+
+1. Updates to variables.tf
+
+    1. Add new to `subnets` key variable validation:
+
+        ```terraform
+        validation {
+          error_message = "Only valid key values \"public\", \"private\", or \"transit_gateway\"."
+          condition = length(setsubtract(keys(var.subnets), [
+            "public",
+            "private",
+            "transit_gateway",
+            "<new type here>"
+          ])) == 0
+        }
+        ```
+
+    1. Specify keys allowed in new variable type map. Copy an existing one and edit the keys to match what you expect users to input:
+
+        ```terraform
+        # All var.subnets.public valid keys
+        validation {
+          error_message = "Invalid key in public subnets. Valid options include: \"cidrs\", \"netmask\", \"name_prefix\", \"nat_gateway_configuration\", \"tags\"."
+          condition = length(setsubtract(keys(try(var.subnets.public, {})), [
+              "cidrs",
+              "netmask",
+              "name_prefix",
+              "nat_gateway_configuration",
+              "route_to_transit_gateway",
+              "tags"
+          ])) == 0
+        }
+        ```
+
+   1. Include in description:
+
+      ```terraform
+        **private subnet type options:**
+          - All shared keys above
+          - `route_to_nat`             = (Optional|bool) <>
+          - `route_to_transit_gateway` = (Optional|list(string)) <>
+      ```
+
+2. Write configuration code
+
+*Note: each for_each loop must account for if a user does not want to create the particular subnet type. Follow examples from other subnet types in main.tf*
+
+    * Create new `aws_subnet`
+    * Create new `awscc_ec2_route_table`
+    * Create new `awscc_ec2_subnet_route_table_association`
+    * Consider and create appropriate `aws_route`
+
+
+3. Create appropriate outputs
+
+    1. `output "<new subnet type>_subnet_attributes_by_az"`
+    1. add new type to `route_table_attributes_by_type_by_az`
diff --git a/examples/private_only/README.md b/examples/private_only/README.md
@@ -29,6 +29,6 @@ No inputs.
 
 | Name | Description |
 |------|-------------|
-| <a name="output_subnet_attributes"></a> [subnet\_attributes](#output\_subnet\_attributes) | n/a |
+| <a name="output_private_subnet_attributes"></a> [private\_subnet\_attributes](#output\_private\_subnet\_attributes) | n/a |
 | <a name="output_subnets"></a> [subnets](#output\_subnets) | Map of subnet types with key/value az = cidr. |
-<!-- END_TF_DOCS -->
+<!-- END_TF_DOCS -->
diff --git a/examples/private_only/outputs.tf b/examples/private_only/outputs.tf
@@ -3,6 +3,6 @@ output "subnets" {
   value       = module.vpc.subnets
 }
 
-output "subnet_attributes" {
+output "private_subnet_attributes" {
   value = module.vpc.private_subnet_attributes_by_az
 }
diff --git a/outputs.tf b/outputs.tf
@@ -3,23 +3,27 @@ output "vpc_attributes" {
   value       = local.vpc
 }
 
-output "subnets" {
-  description = "Map of subnets grouped by type with child map { az = cidr }."
+output "subnet_cidrs_by_type_by_az" {
   value       = module.calculate_subnets.subnets_by_type
-  /* Example:
+  description = <<-EOF
+  Map of subnets grouped by type with child map { az = cidr }.
+
+  Example:
+  ```
     subnets = {
-       private = {
-          us-east-1a = "10.0.0.0/24"
-          us-east-1b = "10.0.1.0/24"
-          us-east-1c = "10.0.2.0/24"
-        }
+      private = {
+        us-east-1a = "10.0.0.0/24"
+        us-east-1b = "10.0.1.0/24"
+        us-east-1c = "10.0.2.0/24"
+      }
       public  = {
-          us-east-1a = "10.0.3.0/24"
-          us-east-1b = "10.0.4.0/24"
-          us-east-1c = "10.0.5.0/24"
-        }
+        us-east-1a = "10.0.3.0/24"
+        us-east-1b = "10.0.4.0/24"
+        us-east-1c = "10.0.5.0/24"
+      }
     }
-  */
+  ```
+EOF
 }
 
 output "transit_gateway_attachment_id" {
@@ -28,22 +32,134 @@ output "transit_gateway_attachment_id" {
 }
 
 output "private_subnet_attributes_by_az" {
-  description = "Map of all private subnets containing their attributes."
   value       = try(aws_subnet.private, null)
+  description = <<-EOF
+  Map of all private subnets containing their attributes.
+
+  Example:
+  ```
+  private_subnet_attributes = {
+    "us-east-1a" = {
+      "arn" = "arn:aws:ec2:us-east-1:<>:subnet/subnet-04a86315c4839b519"
+      "assign_ipv6_address_on_creation" = false
+      ...
+      <all attributes of subnet: https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/subnet#attributes-reference>
+    }
+    "us-east-1b" = {...)
+  }
+  ```
+EOF
 }
 
 output "public_subnet_attributes_by_az" {
-  description = "Map of all public subnets containing their attributes."
   value       = try(aws_subnet.public, null)
+  description = <<-EOF
+  Map of all public subnets containing their attributes.
+
+  Example:
+  ```
+  public_subnet_attributes = {
+    "us-east-1a" = {
+      "arn" = "arn:aws:ec2:us-east-1:<>:subnet/subnet-04a86315c4839b519"
+      "assign_ipv6_address_on_creation" = false
+      ...
+      <all attributes of subnet: https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/subnet#attributes-reference>
+    }
+    "us-east-1b" = {...)
+  }
+  ```
+EOF
 }
 
 output "tgw_subnet_attributes_by_az" {
-  description = "Map of all transit gateway subnets containing their attributes."
   value       = try(aws_subnet.tgw, null)
+  description = <<-EOF
+  Map of all tgw subnets containing their attributes.
+
+  Example:
+  ```
+  tgw_subnet_attributes = {
+    "us-east-1a" = {
+      "arn" = "arn:aws:ec2:us-east-1:<>:subnet/subnet-04a86315c4839b519"
+      "assign_ipv6_address_on_creation" = false
+      ...
+      <all attributes of subnet: https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/subnet#attributes-reference>
+    }
+    "us-east-1b" = {...)
+  }
+  ```
+EOF
+}
+
+output "route_table_attributes_by_type_by_az" {
+  value = {
+    # TODO: omit keys if value is null
+    "private"         = awscc_ec2_route_table.private,
+    "public"          = awscc_ec2_route_table.public
+    "transit_gateway" = awscc_ec2_route_table.tgw
+  }
+  description = <<-EOF
+  Map of route tables by type => az => route table attributes. Example usage: module.vpc.route_table_by_subnet_type.private.id
+
+  Example:
+  ```
+  route_table_attributes_by_type_by_az = {
+    "private" = {
+      "us-east-1a" = {
+        "id" = "rtb-0e77040c0598df003"
+        "route_table_id" = "rtb-0e77040c0598df003"
+        "tags" = tolist([
+          {
+            "key" = "Name"
+            "value" = "private-us-east-1a"
+          },
+        ])
+        "vpc_id" = "vpc-033e054f49409592a"
+      }
+      "us-east-1b" = { ... }
+    "public" = { ... }
+  ```
+EOF
+}
+
+output "nat_gateway_attributes_by_az" {
+  value       = try(aws_nat_gateway.main, null)
+  description = <<-EOF
+  Map of nat gateway resource attributes by AZ.
+
+  Example:
+  ```
+  nat_gateway_attributes_by_az = {
+    "us-east-1a" = {
+      "allocation_id" = "eipalloc-0e8b20303eea88b13"
+      "connectivity_type" = "public"
+      "id" = "nat-0fde39f9550f4abb5"
+      "network_interface_id" = "eni-0d422727088bf9a86"
+      "private_ip" = "10.0.3.40"
+      "public_ip" = <>
+      "subnet_id" = "subnet-0f11c92e439c8ab4a"
+      "tags" = tomap({
+        "Name" = "nat-my-public-us-east-1a"
+      })
+      "tags_all" = tomap({
+        "Name" = "nat-my-public-us-east-1a"
+      })
+    }
+    "us-east-1b" = { ... }
+  }
+  ```
+EOF
+}
+
+## DEPRECATED OUTPUTS
+
+output "subnets" {
+  description = "DEPRECATED OUTPUT: this output has been renamed to `subnet_cidrs_by_type_by_az`. Please transition to that output and see it for a proper description."
+  value       = module.calculate_subnets.subnets_by_type
 }
 
 output "route_table_by_subnet_type" {
-  description = "Map of route tables by type => az => route table attributes. Example usage: module.vpc.route_table_by_subnet_type.private.id"
+  description = "DEPRECATED OUTPUT: this output has been renamed to `route_table_attributes_by_type_by_az`. Please transition to that output and see it for a proper description."
   value = {
     # TODO: omit keys if value is null
     "private"         = awscc_ec2_route_table.private,
@@ -53,6 +169,6 @@ output "route_table_by_subnet_type" {
 }
 
 output "nat_gateways_by_az" {
-  description = "Map of nat gateway resource attributes by AZ."
+  description = "DEPRECATED OUTPUT: this output has been renamed to `nat_gateway_attributes_by_az`. Please transition to that output and see it for a proper description."
   value       = try(aws_nat_gateway.main, null)
 }
diff --git a/variables.tf b/variables.tf
@@ -71,23 +71,23 @@ variable "subnets" {
   description = <<-EOF
   Configuration of subnets to build in VPC. 1 Subnet per AZ is created. Subnet types are defined as maps with the available keys: "private", "public", "transit_gateway". Each Subnet type offers its own set of available arguments detailed below.
 
-  Attributes shared across subnet types:
+  **Attributes shared across subnet types:**
   - `cidrs`       = (Optional|list(string)) **Cannot set if `netmask` is set.** List of CIDRs to set to subnets. Count of CIDRs defined must match quatity of azs in `az_count`.
   - `netmask`     = (Optional|Int) Netmask of the `var.cidr_block` to calculate for each subnet. **Cannot set if `cidrs` is set.**
   - `name_prefix` = (Optional|String) A string prefix to use for the name of your subnet and associated resources. Subnet type key name is used if omitted (aka private, public, transit_gateway). Example `name_prefix = "private"` for `var.subnets.private` is redundant.
   - `tags`        = (Optional|map(string)) Tags to set on the subnet and associated resources.
 
-  `private` subnet type options:
+  **private subnet type options:**
   - All shared keys above
   - `route_to_nat`             = (Optional|bool) Determines if routes to NAT Gateways should be created. Default = false. Must also set `var.subnets.public.nat_gateway_configuration`.
   - `route_to_transit_gateway` = (Optional|list(string)) Optionally create routes from private subnets to transit gateway subnets.
 
-  `public` subnet type options:
+  **public subnet type options:**
   - All shared keys above
   - `nat_gateway_configuration` = (Optional|string) Determines if NAT Gateways should be created and in how many AZs. Valid values = `"none"`, `"single_az"`, `"all_azs"`. Default = "none". Must also set `var.subnets.private.route_to_nat = true`.
   - `route_to_transit_gateway`  = (Optional|list(string)) Optionally create routes from private subnets to transit gateway subnets.
 
-  `transit_gateway` subnet type options:
+  **transit_gateway subnet type options:**
   - All shared keys above
   - `route_to_nat`                                    = (Optional|bool) Determines if routes to NAT Gateways should be created. Default = false. Must also set `var.subnets.public.nat_gateway_configuration`.
   - `transit_gateway_id`                              = (Required|string) Transit gateway to attach VPC to.