From b0ec148337b87101d293fdf653901bd903bf9a7e Mon Sep 17 00:00:00 2001 From: Adam Bovill Date: Fri, 6 Oct 2023 19:21:07 +0000 Subject: [PATCH] feat(standard): adds kebab-case-only filename standard This adds a new standard: tf files will use kebaba-case-only. It also adds a new pre-commit check for this. --- .pre-commit-config.yaml | 7 ++ docs/changes.md | 5 + docs/style/README.md | 6 +- docs/style/file-names.md | 3 + docs/style/naming.md | 224 ++++++++++++++++++++++++++++++++++++++- 5 files changed, 239 insertions(+), 6 deletions(-) create mode 100644 docs/style/file-names.md diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index a599a8d..3b19c15 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -29,3 +29,10 @@ repos: hooks: - id: shellcheck - id: shfmt + - repo: local + hooks: + - id: kebab-case-only + name: kebab case only + entry: tf filenames must be lower-case only + language: fail + files: "[^a-z0-9./-]+\\.tf" diff --git a/docs/changes.md b/docs/changes.md index 72866d3..4ad7cfc 100644 --- a/docs/changes.md +++ b/docs/changes.md @@ -46,3 +46,8 @@ to ensure a new version of the standards is released but not that the actual sta | style | no | Changes due to code style, whitespace, or documentation linting | | build | no | Do not use - build is unused with terraform workspaces | | perf | no | Do not use | + +## Folder Structure + +Folders should be used to create clean groupings. Each folder should contain a `README.md` that summarizes the folder's +contents and provides links to any relevant documentation. diff --git a/docs/style/README.md b/docs/style/README.md index 0744c3e..cda2591 100644 --- a/docs/style/README.md +++ b/docs/style/README.md @@ -1,4 +1,6 @@ # Style -- [conditional-resource-creation](conditional-resource-creation.md) - Creating resources based on variables -- [control-blocks](control-blocks.md) - How to use advanced control in Terraform +We have strong style guides to ensure that our code is consistent and easy to read. We want to take discussing style +_out_ of our review process. + +This section seeks to define the style and structure that everyone should follow. diff --git a/docs/style/file-names.md b/docs/style/file-names.md new file mode 100644 index 0000000..a0dc6c7 --- /dev/null +++ b/docs/style/file-names.md @@ -0,0 +1,3 @@ +# File names + +File and directory names should be `lower-case-kebab-case`. diff --git a/docs/style/naming.md b/docs/style/naming.md index 29b49d0..02c1487 100644 --- a/docs/style/naming.md +++ b/docs/style/naming.md @@ -1,7 +1,223 @@ -# Naming +# Naming conventions -First - follow all the guidance in [https://www.terraform-best-practices.com/naming](https://www.terraform-best-practices.com/naming) +Direct copy: https://www.terraform-best-practices.com/naming -## File names +## General conventions -File and directory names should be `lower-case-kebab-case`. +{% hint style="info" %} +There should be no reason to not follow at least these conventions :) +{% endhint %} + +{% hint style="info" %} +Beware that actual cloud resources often have restrictions in allowed names. Some resources, for example, can't contain dashes, some must be camel-cased. The conventions in this book refer to Terraform names themselves. +{% endhint %} + +1. Use `_` (underscore) instead of `-` (dash) everywhere (in resource names, data source names, variable names, outputs, etc). +2. Prefer to use lowercase letters and numbers (even though UTF-8 is supported). + +## Resource and data source arguments + +1. Do not repeat resource type in resource name (not partially, nor completely): + + {% hint style="success" %} + `resource "aws_route_table" "public" {}` + {% endhint %} + + {% hint style="danger" %} + `resource "aws_route_table" "public_route_table" {}` + {% endhint %} + + {% hint style="danger" %} + `resource "aws_route_table" "public_aws_route_table" {}` + {% endhint %} + +2. Resource name should be named `this` if there is no more descriptive and general name available, or if the resource module creates a single resource of this type (eg, in [AWS VPC module](https://github.com/terraform-aws-modules/terraform-aws-vpc) there is a single resource of type `aws_nat_gateway` and multiple resources of type`aws_route_table`, so `aws_nat_gateway` should be named `this` and `aws_route_table` should have more descriptive names - like `private`, `public`, `database`). +3. Always use singular nouns for names. +4. Use `-` inside arguments values and in places where value will be exposed to a human (eg, inside DNS name of RDS instance). +5. Include argument `count` / `for_each` inside resource or data source block as the first argument at the top and separate by newline after it. +6. Include argument `tags,` if supported by resource, as the last real argument, following by `depends_on` and `lifecycle`, if necessary. All of these should be separated by a single empty line. +7. When using conditions in an argument`count` / `for_each` prefer boolean values instead of using `length` or other expressions. + +## Code examples of `resource` + +### Usage of `count` / `for_each` + +{% hint style="success" %} +{% code title="main.tf" %} + +```hcl +resource "aws_route_table" "public" { + count = 2 + + vpc_id = "vpc-12345678" + # ... remaining arguments omitted +} + +resource "aws_route_table" "private" { + for_each = toset(["one", "two"]) + + vpc_id = "vpc-12345678" + # ... remaining arguments omitted +} +``` + +{% endcode %} +{% endhint %} + +{% hint style="danger" %} +{% code title="main.tf" %} + +```hcl +resource "aws_route_table" "public" { + vpc_id = "vpc-12345678" + count = 2 + + # ... remaining arguments omitted +} +``` + +{% endcode %} +{% endhint %} + +### Placement of `tags` + +{% hint style="success" %} +{% code title="main.tf" %} + +```hcl +resource "aws_nat_gateway" "this" { + count = 2 + + allocation_id = "..." + subnet_id = "..." + + tags = { + Name = "..." + } + + depends_on = [aws_internet_gateway.this] + + lifecycle { + create_before_destroy = true + } +} +``` + +{% endcode %} +{% endhint %} + +{% hint style="danger" %} +{% code title="main.tf" %} + +```hcl +resource "aws_nat_gateway" "this" { + count = 2 + + tags = "..." + + depends_on = [aws_internet_gateway.this] + + lifecycle { + create_before_destroy = true + } + + allocation_id = "..." + subnet_id = "..." +} +``` + +{% endcode %} +{% endhint %} + +### Conditions in `count` + +{% hint style="success" %} +{% code title="outputs.tf" %} + +```hcl +resource "aws_nat_gateway" "that" { # Best + count = var.create_public_subnets ? 1 : 0 +} + +resource "aws_nat_gateway" "this" { # Good + count = length(var.public_subnets) > 0 ? 1 : 0 +} +``` + +{% endcode %} +{% endhint %} + +## Variables + +1. Don't reinvent the wheel in resource modules: use `name`, `description`, and `default` value for variables as defined in the "Argument Reference" section for the resource you are working with. +2. Support for validation in variables is rather limited (e.g. can't access other variables or do lookups). Plan accordingly because in many cases this feature is useless. +3. Use the plural form in a variable name when type is `list(...)` or `map(...)`. +4. Order keys in a variable block like this: `description` , `type`, `default`, `validation`. +5. Always include `description` on all variables even if you think it is obvious (you will need it in the future). +6. Prefer using simple types (`number`, `string`, `list(...)`, `map(...)`, `any`) over specific type like `object()` unless you need to have strict constraints on each key. +7. Use specific types like `map(map(string))` if all elements of the map have the same type (e.g. `string`) or can be converted to it (e.g. `number` type can be converted to `string`). +8. Use type `any` to disable type validation starting from a certain depth or when multiple types should be supported. +9. Value `{}` is sometimes a map but sometimes an object. Use `tomap(...)` to make a map because there is no way to make an object. + +## Outputs + +Make outputs consistent and understandable outside of its scope (when a user is using a module it should be obvious what type and attribute of the value it returns). + +1. The name of output should describe the property it contains and be less free-form than you would normally want. +2. Good structure for the name of output looks like `{name}_{type}_{attribute}` , where: + 1. `{name}` is a resource or data source name without a provider prefix. `{name}` for `aws_subnet` is `subnet`, for`aws_vpc` it is `vpc`. + 2. `{type}` is a type of a resource sources + 3. `{attribute}` is an attribute returned by the output + 4. [See examples](naming.md#code-examples-of-output). +3. If the output is returning a value with interpolation functions and multiple resources, `{name}` and `{type}` there should be as generic as possible (`this` as prefix should be omitted). [See example](naming.md#code-examples-of-output). +4. If the returned value is a list it should have a plural name. [See example](naming.md#use-plural-name-if-the-returning-value-is-a-list). +5. Always include `description` for all outputs even if you think it is obvious. +6. Avoid setting `sensitive` argument unless you fully control usage of this output in all places in all modules. +7. Prefer `try()` (available since Terraform 0.13) over `element(concat(...))` (legacy approach for the version before 0.13) + +### Code examples of `output` + +Return at most one ID of security group: + +{% hint style="success" %} +{% code title="outputs.tf" %} + +```hcl +output "security_group_id" { + description = "The ID of the security group" + value = try(aws_security_group.this[0].id, aws_security_group.name_prefix[0].id, "") +} +``` + +{% endcode %} +{% endhint %} + +When having multiple resources of the same type, `this` should be omitted in the name of output: + +{% hint style="danger" %} +{% code title="outputs.tf" %} + +```hcl +output "this_security_group_id" { + description = "The ID of the security group" + value = element(concat(coalescelist(aws_security_group.this.*.id, aws_security_group.web.*.id), [""]), 0) +} +``` + +{% endcode %} +{% endhint %} + +### Use plural name if the returning value is a list + +{% hint style="success" %} +{% code title="outputs.tf" %} + +```hcl +output "rds_cluster_instance_endpoints" { + description = "A list of all cluster instance endpoints" + value = aws_rds_cluster_instance.this.*.endpoint +} +``` + +{% endcode %} +{% endhint %}