Skip to content

Commit

Permalink
chore: copying existing standards
Browse files Browse the repository at this point in the history
  • Loading branch information
greenkiwi committed Apr 21, 2023
1 parent d9ad0f9 commit 046be71
Show file tree
Hide file tree
Showing 13 changed files with 312 additions and 3 deletions.
27 changes: 27 additions & 0 deletions docs/changes.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Changes & Updates to the Standards

Our development standards and best practices are always evolving. This repository is the source of truth for the
standards and best practices we use at Turo. This document is a changelog of the changes and updates to the standards as
well as the toll that we use the make these changes.

If you have a suggestion for a change or update to the standards, please open an issue in this repository.

Changes are managed through pull requests and merged using
the [Rebase & Merge](https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/about-pull-request-merges#rebase-and-merge-your-pull-request-commits)
workflow.

## Guidelines for changes

At Turo we believe in convention & prior art over configuration and bespoke creations. To that end, we try to base our
standards and practices off of industry standards and best practices. When making changes to the standards or
substantial existing prior art.

We use this document to allow us:

- To clarify how an existing standard might be applied
- To extend an existing pattern where we see a need -- for example: allowing for splitting `variables.tf` into
multiple `variables.<function>.tf` files.
- To define best practices where we have found no prior examples or subtantial prior art

In general, if you are proposing a new change we hope that the change is based on prior art and that you can provide
examples of how the change is being used in the industry.
19 changes: 19 additions & 0 deletions docs/index.md
Original file line number Diff line number Diff line change
@@ -1 +1,20 @@
# Turo Terraform Development Standards

## Baseline standards

We use the following standards as a baseline for all Terraform code at Turo:

- [conventional commits](https://www.conventionalcommits.org/en/v1.0.0/)
- [Rebase & Merge](https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/about-pull-request-merges#rebase-and-merge-your-pull-request-commits)
workflow
- [pre-commit](https://pre-commit.com/) for linting
- [mkdocs](https://www.mkdocs.org/) for documentation -- For full documentation
visit [mkdocs.org](https://www.mkdocs.org).
- Formatting -- all files are formatted using appropriate language specific tools during the pre-commit phase to ensure
baseline style is consistent.
- Terraform Documentation -- all Terraform code is documented using the [terraform_docs](https://terraform-docs.io/)
- Standards are enforced with code whereever possible. For example, we use [pre-commit](https://pre-commit.com/) to
enforce
formatting and linting standards.
- Scripts to rule them all -- we use [scripts to rule them all](https://github.com/github/scripts-to-rule-them-all) to
provide consistent usage and functionality across all repositories.
10 changes: 10 additions & 0 deletions docs/modules/input-variables.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# Module: Input Variables

All module input variables **MUST** be in files named either `variables.tf` or `variables.<function>.tf`.

Developers **MAY** group variables into files by function. For example, if a module has variables that are used for logging,
the developer **MAY** choose to group those variables in a file named `variables.logging.tf`.

When module is to be sourced from outside the current GitHub repo, its variables **MUST** have a description.

When variables are expected to be used as inputs to other modules, developers **SHOULD** attempt to structure them in a
12 changes: 12 additions & 0 deletions docs/modules/outputs.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# Module: Outputs

All module outputs **MUST** be in files named either `outputs.tf` or `outputs.<function>.tf`.

Developer **MAY** choose to group outputs by function. For example, if a module has outputs that are used for logging,
the developer **MAY** choose to group those outputs in a file named `outputs.logging.tf`.

When module is to be sourced from outside the current GitHub repo, its outputs **MUST** have a description.

When outputs are expected to be used as inputs to other modules, developers **SHOULD** attempt to structure them in a
way that makes it easy to understand what the output is and how it should be used and can easily be used as an input to
another module.
4 changes: 4 additions & 0 deletions docs/sources.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# Sources of Inspiration

- https://developer.hashicorp.com/terraform/language
- https://www.terraform-best-practices.com/
96 changes: 96 additions & 0 deletions docs/style/conditional-resource-creation.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
# Conditional Resource Creation

When you have to create a resource conditionally based on a boolean or some other binary value, opt for the count
control block for most cases. The most cases here mean, a simple conditional creation of an isolated resource which may
have just one or two references as inputs for other resources for example

```hcl
# example for using count block for conditionally creating a resource
data "aws_iam_policy_document" "this" {
count = local.iam_role_enabled ? 1 : 0
statement {
configurations = "..."
}
}
resource "aws_iam_policy" "this" {
name = var.name
path = "/"
description = "IAM policy for..."
policy = data.aws_iam_policy_document.this[0].json
tags = var.tags
}
```

Avoid count block if you have multiple resources created using the same binary condition and in such cases favor
for_each block (or even better consider modularizing the portion of resources that are conditionally created using the
same binary value)

```hcl
# example for_each use case where multiple resources are dependant on one single
# binary condition and chain referenced using the same key consistently across. If
# we had used count block, the references then would have to have used indexes which
# are hard to understand.
locals {
iam_resources = toset(var.iam_role_enabled ? ["resource_name"] : [])
}
data "aws_kms_key" "this" {
for_each = iam_resources
key_id = var.kms_key_id
}
data "aws_iam_policy_document" "this" {
for_each iam_resources
statement {
sid = "statement identifier..."
effect = "Allow"
actions = ["kms:Decrypt"]
resources = [data.aws_kms_key.this[each.key].arn]
}
}
resource "aws_iam_policy" "this" {
for_each = iam_resources
name = var.name
path = "/"
description = "IAM policy for..."
policy = data.aws_iam_policy_document.this[
each.key
].json
tags = var.tags
}
```

One more thing to consider is before going with one of the above solution is to check whether we can group the
conditionally created resources together as an internal module. Are there enough resources that are related to each
other and created using the same binary condition/check? If we go with the option to create an internal module, the
whole of it is instantiated behind the one binary condition and each individual resource in it don’t have to be guarded
with the checks. If the module is enabled, the whole group of resources under it are created. If the module is not
enabled none are created. In this scenario, go with count block for enabling or disabling the module. for_each is not
preferred for the module since we are dealing with one single place to enable/disable the creation and count is easier
to comprehend.

```hcl
# assuming all of the iam related resources from the example above is changed into
# an internal module named "iam_role" (with a directory named "iam_role"), here is
# an example of enabling the module. The resources that make up this module don't
# specify the conditional check individually.
module "iam_role" {
source = "./iam-role"
count = local.iam_role_enabled ? 1 : 0
configuration = "..."
tags = local.tags
}
```
13 changes: 13 additions & 0 deletions docs/style/control-blocks.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# Control blocks: `for_each` and `count`

Control blocks **MUST** have an empty line after them to make it easy to know that they are a control block rather than
an attribute.

```hcl
resource "aws_sns_topic_policy" "allow" {
for_each = {for k, v in var.sns_notifications : k => v if var.create_sns_policy}
arn = each.value.topic_arn
policy = data.aws_iam_policy_document.sns[each.key].json
}
```
27 changes: 27 additions & 0 deletions docs/style/locals.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
## Locals & Data

## Hardcoded values

Please store hardcoded values as local variables with appropriate names

### Do this

```hcl
locals {
segment_aws_identifier = "arn:aws:iam::107630771604:user/s3-copy" # Segment AWS account
}
principals {
type = "AWS"
identifiers = [local.segment_aws_identifier]
}
```

### Not this

```hcl
principals {
type = "AWS"
identifiers = ["arn:aws:iam::107630771604:user/s3-copy"]
}
```
19 changes: 19 additions & 0 deletions docs/style/maps.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# Maps

We should prefer using `=` over `:` in maps

## Good

```hcl
common_tags = {
Environment = "production"
}
```

## Bad

```hcl
common_tags = {
Environment : "production"
}
```
16 changes: 16 additions & 0 deletions docs/style/module-sourcing.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# Module source

Module source **MUST** have an empty line after them to make it easy to know that they are not attributes.

If the module has a `for_each` or `count` the source **MUST** be the first attribute and there should be a newline
between the source and the `for_each`/`count` and that control statement **MUST** have a newline after it.

```hcl
module "wrapper" {
source = "../"
for_each = var.items
create_bucket = try(each.value.create_bucket, var.defaults.create_bucket, true)
}
```
Empty file added docs/style/naming.md
Empty file.
3 changes: 0 additions & 3 deletions docs/test.md

This file was deleted.

69 changes: 69 additions & 0 deletions docs/versioning.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
# Versioning

We have the following guidance for versioning within our repositories.

## Lock files

We check in lock files for all our projects. This is to ensure that we are all using the same versions of
dependencies.This is especially important for our libraries, as we want to ensure that the version of a library that we
are using is the same version that our consumers are using.

We lock two platform:

1. `linux_amd64` for our CI & Terraform Cloud agent platform
2. `darwin_amd64` for our local development platform -- note that `arm` support is not consistent enough for us to use
that for local development. That may change in the future.

Locking can be performed with the following command:

```hcl
terraform -chdir = "$TERRAFORM_DIR" providers lock -platform = darwin_amd64 -platform = linux_amd64
```

Where `$TERRAFORM_DIR` is the directory containing the Terraform code.

## Provider Version Constraints

Here are the conventions we use when specifying the version of a given Terraform provider. The thought is that by having
only minimum versions specified in our Terraform modules, we can update the provider version in a Terraform workspace
without having to release new versions of our Terraform modules.

In general, when using version constraint pinning syntax, pin to the major version. e.g. `version = "~> 3.5"`.

`version = ">= 5.0.0, <6.0.0"`

See here for details for how Terraform interprets version constraints.

## Terraform Workspaces

Use major version pinning here. Use the ~> syntax to allow the rightmost version number to vary as new provider releases
become available. e.g. `version = "~> 3.5"` which indicates any version is acceptable from the `3.x` series, as long as the
version is `3.5` or higher.

## Terraform Modules

Use the `>=` syntax to allow the version specified to be a minimum. e.g. `version = ">= 3.5.0"` which indicates version
`3.5.0` or later, including `3.6`, `4.0`, etc.

This is true unless we know that the module will not be compatible with a future version of the provider.

## Tests within Terraform Modules

Use major version pinning here. Use the `~>` syntax to allow the rightmost version number to vary as new provider releases
become available. e.g. `version = "~> 3.5"` which indicates any version is acceptable from the `3.x` series, as long as the
version is `3.5` or higher.

## CI Validation

We expect that repos will perform the following linting steps during the pre-commit and CI process.

```hcl
- repo: https://github.com/antonbabenko/pre-commit-terraform
rev: v1.77.1
hooks:
- id: terraform_providers_lock
stages: [commit]
args:
- --args=-platform=darwin_amd64
- --args=-platform=linux_amd64
```

0 comments on commit 046be71

Please sign in to comment.