refactor modules

[FoSix]

An issue tracker for modules (and example) refactor.

Introduction

The key points to address when refactoring the modules:

• promote resource names to the variables level - this should affect resources(vnets, route tables, NSGs), not sub-resources (subnets, UDRs, NSG rules)
• define types for all variables, even complex ones, use optional keyword
• bump min supported TF version to 1.3
• migrate to new documentation format:
  • README in document layout
  • introduction, purpose, usage should be moved to .header.md
  • every variable should have a description, where the 1^st sentence (ending with a .) should contain a summary
  • when documenting complex types, follow the format described below
  • keep the variables in variables.tf in order that makes sense (see below)
  • keep the description of variables that repeat between example the same, like name or tags (see below for a copy&paste pattern to follow)
• get rid of lookup/try statements where possible -> relay on default values (for complex variables as well)
• get rid of boolean variables that do not add any value, like enable_zones - use smart defaults instead
• be bold, if you see some old code, or you think something can be done better, DO IT (this will be a breaking change anyway 😄 )
• when you work on a module, do fix all examples using that module. Make sure the code is the same across all examples
• add tests to you module (follow vnet refactor #337) TBD
• inside module, add a comment with link to terraform registry before a resource block, like this:

  # https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/linux_virtual_machine_scale_set
  resource "azurerm_linux_virtual_machine_scale_set" "example" {
  name                = "example-vmss"
  ...

Issues per module

• appgw refactor #325
• application_insights refactor #326
• bootstrap & vmseries refactor #327
• gwlb refactor #328
• loadbalancer refactor #329
• natgw refactor #330
• panorama refactor #331
• virtual_machine refactor #332
• virtual_network_gateway refactor #333
• vmseries refactor #334
• vmss refactor #335
• vnet refactor #337
• vnet_peering refactor #336

Variables ordering

Some basic principals:

• unless a module is constructed in a different way, variables.tf should start with these vars in this particular order:
  • name
  • resource_group_name
  • locations
  • tags
• keep vars related to the same type of resource next to each other, i.e. create_subnets should be next to subnets
• order the variables, either:
  • based on decision steps when designing infrastructure, i.e. create_subnets 1^st, then subnets definition (you have to decide if you create subnets, or do you source them, then you provide the specs, which will differ based on your decision)
  • based on dependencies between resources, i.e. you define NSGs and RTs 1^st, then Subnets, as in subnets variable we point to already defined NSGs and RTs.
  • if no other criteria matches, based on importance or usage frequency.

Keep in mind that his order will be retained in a README.md.

Description format

Follow the example below:

description = <<-EOF
A short, one sentence description of the variable.

Some more detailed description, can be multiple lines.

List of either required or important properties:

- `name`                   - (`string`, required) name of the Network Security Group.
- `some_optional_value`    - (`string`, optional, defaults to `null`) some description
- `some_complex_property`  - (`map`, optional) A list of objects representing something.
  - `name`                    - (`string`, required) name of the something
  - `some_number`             - (`number`, optional, defaults to `5`) numeric value
  - `some_value_1`            - (`string`, required, mutually exclusive with `some_value_2`) some description
  - `some_value_2`            - (`string`, required, mutually exclusive with `some_value_1`) some description
  - `some_optional_value`     - (`bool`, optional, defaults to `false`)

List of other, optional properties:

- `less_important_1`    - (`string`, optional, defaults to `null`) some description
- `less_important_2`    - (`string`, optional, defaults Azure defaults) some description
- `less_important_3`    - (`string`, optional, defaults to `""`) some description
- `less_important_4`    - (`list(string)`, optional, defaults to `[]`) some description

Example:
```hcl
{
  "object_1" = {
    name = "name of object 1"
    .....
  }
}
```
EOF

Common variables

Replace the following variables with this definitions:

variable "name" {
  description = "The name of the Azure Virtual Network."
  type        = string
}

variable "resource_group_name" {
  description = "The name of the Resource Group to use."
  type        = string
}

variable "location" {
  description = "The name of the Azure region to deploy the resources in."
  type        = string
}

variable "tags" {
  description = "The map of tags to assign to all created resources."
  default     = {}
  type        = map(string)
}

[alperenkose]

A NOTE: I think we should add these guidelines to somewhere like CONTRIBUTING before closing this issue; in order to follow the same conventions in the future.