terraform-azurerm-vmseries-modules icon indicating copy to clipboard operation
terraform-azurerm-vmseries-modules copied to clipboard

refactor modules

Open FoSix opened this issue 1 year ago • 1 comments

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 1st 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 #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

  • [x] #325
  • [x] #326
  • [x] #327
  • [x] #328
  • [x] #329
  • [x] #330
  • [x] #331
  • [x] #332
  • [x] #333
  • [x] #334
  • [x] #335
  • [x] #337
  • [x] #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 1st, 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 1st, 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)
}

FoSix avatar Aug 29 '23 09:08 FoSix

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.

alperenkose avatar Jan 16 '24 08:01 alperenkose