From 6d5bac107ed2da39a672d6fdcdfd72707c7e59bc Mon Sep 17 00:00:00 2001 From: Robert Crandall Date: Thu, 12 Mar 2026 14:48:49 -0700 Subject: [PATCH 01/10] CCR --- .../terraform-provider-pr-review/SKILL.md | 88 ++ .../references/review-checklist.md | 74 ++ requirements/develop_modules/composition.mdx | 380 ++++++++ requirements/develop_modules/index.mdx | 89 ++ requirements/develop_modules/providers.mdx | 367 +++++++ requirements/develop_modules/publish.mdx | 45 + requirements/develop_modules/refactoring.mdx | 448 +++++++++ requirements/develop_modules/structure.mdx | 133 +++ requirements/module-best-practices.md | 908 ++++++++++++++++++ requirements/terraform-best-practices.md | 752 +++++++++++++++ 10 files changed, 3284 insertions(+) create mode 100644 .github/skills/terraform-provider-pr-review/SKILL.md create mode 100644 .github/skills/terraform-provider-pr-review/references/review-checklist.md create mode 100644 requirements/develop_modules/composition.mdx create mode 100644 requirements/develop_modules/index.mdx create mode 100644 requirements/develop_modules/providers.mdx create mode 100644 requirements/develop_modules/publish.mdx create mode 100644 requirements/develop_modules/refactoring.mdx create mode 100644 requirements/develop_modules/structure.mdx create mode 100644 requirements/module-best-practices.md create mode 100644 requirements/terraform-best-practices.md diff --git a/.github/skills/terraform-provider-pr-review/SKILL.md b/.github/skills/terraform-provider-pr-review/SKILL.md new file mode 100644 index 0000000000..15a0ec63b9 --- /dev/null +++ b/.github/skills/terraform-provider-pr-review/SKILL.md @@ -0,0 +1,88 @@ +--- +name: terraform-provider-pr-review +description: "Review pull requests for the Terraform GitHub provider. Use when asked to review a PR, audit Terraform provider changes, check schema or state behavior, verify acceptance tests, or catch regressions in resources/data sources/docs/examples." +argument-hint: "PR number/URL or 'active/open PR' plus focus areas (schema, tests, docs, migration, security)" +user-invocable: true +--- + +# Terraform Provider PR Review + +Use this skill to perform a high-signal code review for this repository (`integrations/terraform-provider-github`). + +## Review Goals + +- Find correctness bugs, regressions, and provider behavior changes. +- Validate schema/state compatibility for Terraform users. +- Check test coverage (unit and acceptance), docs, and examples. +- Identify risk around GitHub API usage, permissions, and error handling. + +## Terraform Background for Reviewers + +This section summarizes the Terraform module and provider concepts most relevant to PR review. Refer back here when evaluating schema, examples, or state changes. + +### Key Concepts + +- **Resources and Data Sources**: A *resource* (`resource` block) manages a lifecycle object (create, read, update, delete). A *data source* (`data` block) only reads existing objects. Both declare a *schema* of typed attributes. +- **Schema Attributes**: Each attribute has a `Type` (string, int, bool, list, set, map), and flags like `Required`, `Optional`, `Computed`, `ForceNew`, `Default`, `Sensitive`, and `Description`. Changing any flag alters user-visible behavior. +- **State**: Terraform stores the last-known attribute values of every managed resource in *state*. The *read* function must produce output consistent with state or Terraform will detect drift and propose changes. +- **Plan and Apply**: `terraform plan` computes a diff between desired config and current state. `terraform apply` executes that diff. Bugs in schema or read logic cause perpetual diffs, surprise replacements, or silent data loss. +- **Imports**: Users can adopt existing infrastructure with `terraform import`. The resource's read function must be able to populate full state from just the resource ID. + +### Module Structure (for reviewing `examples/`) + +- A module is a directory of `.tf` files. The recommended layout is `main.tf`, `variables.tf`, `outputs.tf`, and a `README.md`. +- Variables and outputs should always include `description` and `type`. +- Child modules must **not** contain `provider` blocks — provider configuration belongs exclusively in root modules. +- Each module should declare `required_providers` with a minimum version constraint (`>=`). + +### Backward Compatibility Rules + +- **Safe (minor/patch)**: Adding new optional attributes with defaults, adding new resources or data sources. +- **Breaking (major)**: Removing or renaming attributes, changing `Optional` to `Required`, changing `Type`, adding `ForceNew` to an existing attribute. +- When renaming resources or restructuring modules, Terraform's `moved` block lets users migrate state without destroying infrastructure. If a PR restructures resources, check that migration guidance is provided. +- Removing a `moved` block is itself a breaking change. + +## Inputs + +- PR identifier: URL, number, or `active/open PR`. +- Optional focus areas: `schema`, `state`, `tests`, `docs`, `examples`, `security`, `performance`. + +## Procedure + +1. Gather PR context. +2. Inspect changed files and classify them: + - Provider/resource/data-source code under `github/` + - Tests (`*_test.go`, especially acceptance tests) + - Docs/site content under `website/` + - Example configurations under `examples/` +3. For schema changes, verify backward compatibility using the rules in *Terraform Background*: flag attribute removal/renames, type changes, new `ForceNew`, or `Optional`→`Required` transitions. +4. For example/configuration changes, confirm they follow standard module structure (`main.tf`, `variables.tf`, `outputs.tf`, `README.md`), do not embed `provider` blocks in child modules, and include `description` on variables/outputs. +5. Review against the checklist in [Terraform Provider Review Checklist](./references/review-checklist.md). +6. Prioritize findings by severity and provide actionable fixes. +7. Report residual risk and testing gaps when uncertain. + +## Output Format + +Return findings first, ordered by severity. + +1. `HIGH`/`MEDIUM`/`LOW` title - short impact statement +2. File reference: `path/to/file.go:line` +3. Why this is a problem (runtime behavior, Terraform UX, upgrade risk) +4. Suggested fix (concise) + +Then include: + +- `Open Questions / Assumptions` +- `Residual Risk` +- `Change Summary` (brief) + +If no issues are found, explicitly state `No blocking findings found` and list remaining risk areas. + +## Repository Notes + +- Acceptance and manual validation are important in this provider. See contribution guidance in `CONTRIBUTING.md`. +- Prefer matching resource/data source tests when implementation behavior changes. +- When schema or state semantics change, treat as high-risk unless compatibility is clearly preserved. +- Breaking changes must follow semantic versioning: attribute removal/rename or new required fields warrant a major version bump. +- Example configurations under `examples/` should be self-contained, follow standard module structure, and not include `provider` blocks inside nested modules. +- Sensitive attributes (tokens, secrets, private keys) must be marked `Sensitive: true` in the schema and must not appear in log output. diff --git a/.github/skills/terraform-provider-pr-review/references/review-checklist.md b/.github/skills/terraform-provider-pr-review/references/review-checklist.md new file mode 100644 index 0000000000..75e241ebe2 --- /dev/null +++ b/.github/skills/terraform-provider-pr-review/references/review-checklist.md @@ -0,0 +1,74 @@ +# Terraform Provider Review Checklist + +Use this checklist when reviewing PRs in `terraform-provider-github`. + +## 1. Correctness and Behavior + +- Verify CRUD/read logic correctly maps GitHub API responses to Terraform schema/state. +- Check for nil handling, default-value drift, and state flattening/expansion mismatches. +- Confirm update paths do not accidentally force replacement or wipe optional fields. +- Validate retry/backoff and error classification for API failures. + +## 2. Schema and State Compatibility + +- Any `schema.Schema` changes (`Type`, `Optional`, `Required`, `Computed`, `ForceNew`, `Default`) can change user behavior. +- Confirm imports still work and read functions produce stable state. +- Flag any behavior that may break existing state without migration notes/tests. +- Watch for attribute rename/removal without deprecation path. +- Adding an optional attribute with a default is backward-compatible; removing or renaming an attribute is a breaking change that needs a deprecation cycle. +- New or changed attributes should include `ValidateFunc`/`ValidateDiagFunc` to catch invalid values at plan time rather than at apply time. +- All attributes should have a `Description` string for documentation generation. +- When resources are renamed or restructured, check for `moved` block guidance or state migration documentation so existing users don't face resource destruction on upgrade. +- Mark attributes that hold secrets with `Sensitive: true` in the schema to prevent leaking values in plan output and state. + +## 3. Terraform UX and Drift + +- Ensure diff suppression, normalization, and API canonicalization avoid perpetual diffs. +- Check that empty vs null handling is intentional. +- Verify list/set ordering behavior and deterministic state output. + +## 4. Testing Expectations + +- For behavior changes, check matching tests under `github/*_test.go`. +- Prefer acceptance tests for API-facing changes (`TF_ACC=1` scenarios). +- Ensure tests assert the bugfix/regression target, not only happy path. +- Flag missing tests when logic changed but test coverage did not. + +## 5. Docs and Examples + +- If resource/data source behavior changed, review website docs updates under `website/`. +- If user workflow changed, review corresponding example updates under `examples/`. +- Confirm examples still reflect current schema and argument names. +- Example directories should follow standard module structure (`main.tf`, `variables.tf`, `outputs.tf`) with a `README.md` explaining purpose and usage. +- Example configurations should not embed `provider` blocks inside child modules; provider configuration belongs in root modules only. +- Variables and outputs in examples should include `description` and `type`. +- If a PR adds or changes a resource, verify there is at least one example showing typical usage. + +## 6. Security and Permissions + +- Verify sensitive values are not logged or accidentally exposed in state. +- Check token/credential handling and least-privilege assumptions. +- Ensure permission scope requirements are documented for new API calls. +- Confirm no secrets or credentials are hardcoded in example configurations. +- Verify that debug/trace logging does not print sensitive attribute values. +- Sensitive outputs should be marked `sensitive = true` so Terraform redacts them in CLI output. + +## 7. Performance and API Safety + +- Flag new N+1 patterns, excessive API calls, or missing pagination handling. +- Check for rate-limit-sensitive loops and absent caching where needed. +- Confirm context cancellation/timeouts are respected in long operations. + +## 8. Versioning and Changelog + +- Breaking changes (attribute removal/rename, forced replacement, new required fields) must be called out for a MAJOR version bump under semantic versioning. +- Backward-compatible additions (new optional attributes with defaults, new resources/data sources) correspond to MINOR version bumps. +- Bug fixes with no schema change correspond to PATCH version bumps. +- Verify the PR description or CHANGELOG includes a clear summary of what changed and the user impact. + +## 9. Review Report Rules + +- Report findings first, ordered by severity. +- Include precise file references like `github/resource_x.go:123`. +- Explain impact in Terraform terms: plan/apply behavior, drift, state compatibility. +- Mention residual risks if tests or docs are incomplete. diff --git a/requirements/develop_modules/composition.mdx b/requirements/develop_modules/composition.mdx new file mode 100644 index 0000000000..e14a6bad6b --- /dev/null +++ b/requirements/develop_modules/composition.mdx @@ -0,0 +1,380 @@ +--- +page_title: Module Composition +description: |- + Module composition allows infrastructure to be described from modular + building blocks. +# START AUTO GENERATED METADATA, DO NOT EDIT +created_at: 2025-11-19T13:27:46Z +last_modified: 2025-11-19T13:27:46Z +# END AUTO GENERATED METADATA +--- + +# Module Composition + +In a simple Terraform configuration with only one root module, we create a +flat set of resources and use Terraform's expression syntax to describe the +relationships between these resources: + +```hcl +resource "aws_vpc" "example" { + cidr_block = "10.1.0.0/16" +} + +resource "aws_subnet" "example" { + vpc_id = aws_vpc.example.id + + availability_zone = "us-west-2b" + cidr_block = cidrsubnet(aws_vpc.example.cidr_block, 4, 1) +} +``` + +When we introduce `module` blocks, our configuration becomes hierarchical +rather than flat: each module contains its own set of resources, and possibly +its own child modules, which can potentially create a deep, complex tree of +resource configurations. + +However, in most cases we strongly recommend keeping the module tree flat, +with only one level of child modules, and use a technique similar to the +above of using expressions to describe the relationships between the modules: + +```hcl +module "network" { + source = "./modules/aws-network" + + base_cidr_block = "10.0.0.0/8" +} + +module "consul_cluster" { + source = "./modules/aws-consul-cluster" + + vpc_id = module.network.vpc_id + subnet_ids = module.network.subnet_ids +} +``` + +We call this flat style of module usage _module composition_, because it +takes multiple [composable](https://en.wikipedia.org/wiki/Composability) +building-block modules and assembles them together to produce a larger system. +Instead of a module _embedding_ its dependencies, creating and managing its +own copy, the module _receives_ its dependencies from the root module, which +can therefore connect the same modules in different ways to produce different +results. + +The rest of this page discusses some more specific composition patterns that +may be useful when describing larger systems with Terraform. + +## Dependency Inversion + +In the example above, we saw a `consul_cluster` module that presumably describes +a cluster of [HashiCorp Consul](https://www.consul.io/) servers running in +an AWS VPC network, and thus it requires as arguments the identifiers of both +the VPC itself and of the subnets within that VPC. + +An alternative design would be to have the `consul_cluster` module describe +its _own_ network resources, but if we did that then it would be hard for +the Consul cluster to coexist with other infrastructure in the same network, +and so where possible we prefer to keep modules relatively small and pass in +their dependencies. + +This [dependency inversion](https://en.wikipedia.org/wiki/Dependency_inversion_principle) +approach also improves flexibility for future +refactoring, because the `consul_cluster` module doesn't know or care how +those identifiers are obtained by the calling module. A future refactor may +separate the network creation into its own configuration, and thus we may +pass those values into the module from data sources instead: + +```hcl +data "aws_vpc" "main" { + tags = { + Environment = "production" + } +} + +data "aws_subnet_ids" "main" { + vpc_id = data.aws_vpc.main.id +} + +module "consul_cluster" { + source = "./modules/aws-consul-cluster" + + vpc_id = data.aws_vpc.main.id + subnet_ids = data.aws_subnet_ids.main.ids +} +``` + +### Conditional Creation of Objects + +In situations where the same module is used across multiple environments, +it's common to see that some necessary object already exists in some +environments but needs to be created in other environments. + +For example, this can arise in development environment scenarios: for cost +reasons, certain infrastructure may be shared across multiple development +environments, while in production the infrastructure is unique and managed +directly by the production configuration. + +Rather than trying to write a module that itself tries to detect whether something +exists and create it if not, we recommend applying the dependency inversion +approach: making the module accept the object it needs as an argument, via +an input variable. + +For example, consider a situation where a Terraform module deploys compute +instances based on a disk image, and in some environments there is a +specialized disk image available while other environments share a common +base disk image. Rather than having the module itself handle both of these +scenarios, we can instead declare an input variable for an object representing +the disk image. Using AWS EC2 as an example, we might declare a common subtype +of the `aws_ami` resource type and data source schemas: + +```hcl +variable "ami" { + type = object({ + # Declare an object using only the subset of attributes the module + # needs. Terraform will allow any object that has at least these + # attributes. + id = string + architecture = string + }) +} +``` + +The caller of this module can now itself directly represent whether this is +an AMI to be created inline or an AMI to be retrieved from elsewhere: + +```hcl +# In situations where the AMI will be directly managed: + +resource "aws_ami_copy" "example" { + name = "local-copy-of-ami" + source_ami_id = "ami-abc123" + source_ami_region = "eu-west-1" +} + +module "example" { + source = "./modules/example" + + ami = aws_ami_copy.example +} +``` + +```hcl +# Or, in situations where the AMI already exists: + +data "aws_ami" "example" { + owner = "9999933333" + + tags = { + application = "example-app" + environment = "dev" + } +} + +module "example" { + source = "./modules/example" + + ami = data.aws_ami.example +} +``` + +This is consistent with Terraform's declarative style: rather than creating +modules with complex conditional branches, we directly describe what +should already exist and what we want Terraform to manage itself. + +By following this pattern, we can be explicit about in which situations we +expect the AMI to already be present and which we don't. A future reader +of the configuration can then directly understand what it is intending to do +without first needing to inspect the state of the remote system. + +In the above example, the object to be created or read is simple enough to +be given inline as a single resource, but we can also compose together multiple +modules as described elsewhere on this page in situations where the +dependencies themselves are complicated enough to benefit from abstractions. + +## Assumptions and Guarantees + +Every module has implicit assumptions and guarantees that define what data it expects and what data it produces for consumers. + +- **Assumption:** A condition that must be true in order for the configuration of a particular resource to be usable. For example, an `aws_instance` configuration can have the assumption that the given AMI will always be configured for the `x86_64` CPU architecture. +- **Guarantee:** A characteristic or behavior of an object that the rest of the configuration should be able to rely on. For example, an `aws_instance` configuration can have the guarantee that an EC2 instance will be running in a network that assigns it a private DNS record. + +We recommend [validating your configuration](/terraform/language/validate) to help capture and test for assumptions and guarantees. This helps future maintainers understand the configuration design and intent. Configuration validation returns useful information about errors earlier and in context, helping consumers more easily diagnose issues in their configurations. + +The following examples creates a precondition that checks whether the EC2 instance has an encrypted root volume. + +```hcl +output "api_base_url" { + value = "https://${aws_instance.example.private_dns}:8433/" + + # The EC2 instance must have an encrypted root volume. + precondition { + condition = data.aws_ebs_volume.example.encrypted + error_message = "The server's root volume is not encrypted." + } +} +``` + +## Multi-cloud Abstractions + +Terraform itself intentionally does not attempt to abstract over similar +services offered by different vendors, because we want to expose the full +functionality in each offering and yet unifying multiple offerings behind a +single interface will tend to require a "lowest common denominator" approach. + +However, through composition of Terraform modules it is possible to create +your own lightweight multi-cloud abstractions by making your own tradeoffs +about which platform features are important to you. + +Opportunities for such abstractions arise in any situation where multiple +vendors implement the same concept, protocol, or open standard. For example, +the basic capabilities of the domain name system are common across all vendors, +and although some vendors differentiate themselves with unique features such +as geolocation and smart load balancing, you may conclude that in your use-case +you are willing to eschew those features in return for creating modules that +abstract the common DNS concepts across multiple vendors: + +```hcl +module "webserver" { + source = "./modules/webserver" +} + +locals { + fixed_recordsets = [ + { + name = "www" + type = "CNAME" + ttl = 3600 + records = [ + "webserver01", + "webserver02", + "webserver03", + ] + }, + ] + server_recordsets = [ + for i, addr in module.webserver.public_ip_addrs : { + name = format("webserver%02d", i) + type = "A" + records = [addr] + } + ] +} + +module "dns_records" { + source = "./modules/route53-dns-records" + + route53_zone_id = var.route53_zone_id + recordsets = concat(local.fixed_recordsets, local.server_recordsets) +} +``` + +In the above example, we've created a lightweight abstraction in the form of +a "recordset" object. This contains the attributes that describe the general +idea of a DNS recordset that should be mappable onto any DNS provider. + +We then instantiate one specific _implementation_ of that abstraction as a +module, in this case deploying our recordsets to Amazon Route53. + +If we later wanted to switch to a different DNS provider, we'd need only to +replace the `dns_records` module with a new implementation targeting that +provider, and all of the configuration that _produces_ the recordset +definitions can remain unchanged. + +We can create lightweight abstractions like these by defining Terraform object +types representing the concepts involved and then using these object types +for module input variables. In this case, all of our "DNS records" +implementations would have the following variable declared: + +```hcl +variable "recordsets" { + type = list(object({ + name = string + type = string + ttl = number + records = list(string) + })) +} +``` + +While DNS serves as a simple example, there are many more opportunities to +exploit common elements across vendors. A more complex example is Kubernetes, +where there are now many different vendors offering hosted Kubernetes clusters +and even more ways to run Kubernetes yourself. + +If the common functionality across all of these implementations is sufficient +for your needs, you may choose to implement a set of different modules that +describe a particular Kubernetes cluster implementation and all have the common +trait of exporting the hostname of the cluster as an output value: + +```hcl +output "hostname" { + value = azurerm_kubernetes_cluster.main.fqdn +} +``` + +You can then write _other_ modules that expect only a Kubernetes cluster +hostname as input and use them interchangeably with any of your Kubernetes +cluster modules: + +```hcl +module "k8s_cluster" { + source = "modules/azurerm-k8s-cluster" + + # (Azure-specific configuration arguments) +} + +module "monitoring_tools" { + source = "modules/monitoring_tools" + + cluster_hostname = module.k8s_cluster.hostname +} +``` + +## Data-only Modules + +Most modules contain `resource` blocks and thus describe infrastructure to be +created and managed. It may sometimes be useful to write modules that do not +describe any new infrastructure at all, but merely retrieve information about +existing infrastructure that was created elsewhere using +[data sources](/terraform/language/data-sources). + +As with conventional modules, we suggest using this technique only when the +module raises the level of abstraction in some way, in this case by +encapsulating exactly how the data is retrieved. + +A common use of this technique is when a system has been decomposed into several +subsystem configurations but there is certain infrastructure that is shared +across all of the subsystems, such as a common IP network. In this situation, +we might write a shared module called `join-network-aws` which can be called +by any configuration that needs information about the shared network when +deployed in AWS: + +```hcl +module "network" { + source = "./modules/join-network-aws" + + environment = "production" +} + +module "k8s_cluster" { + source = "./modules/aws-k8s-cluster" + + subnet_ids = module.network.aws_subnet_ids +} +``` + +The `network` module itself could retrieve this data in a number of different +ways: it could query the AWS API directly using +[`aws_vpc`](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/vpc) +and +[`aws_subnet_ids`](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/subnet_ids) +data sources, or it could read saved information from a Consul cluster using +[`consul_keys`](https://registry.terraform.io/providers/hashicorp/consul/latest/docs/data-sources/keys), +or it might read the outputs directly from the state of the configuration that +manages the network using +[`terraform_remote_state`](/terraform/language/state/remote-state-data). + +The key benefit of this approach is that the source of this information can +change over time without updating every configuration that depends on it. +Furthermore, if you design your data-only module with a similar set of outputs +as a corresponding management module, you can swap between the two relatively +easily when refactoring. diff --git a/requirements/develop_modules/index.mdx b/requirements/develop_modules/index.mdx new file mode 100644 index 0000000000..5e1eab23c3 --- /dev/null +++ b/requirements/develop_modules/index.mdx @@ -0,0 +1,89 @@ +--- +page_title: Creating Modules +description: >- + Modules are containers for multiple resources that are used together in a + configuration. Learn when to create modules and about module structure. +# START AUTO GENERATED METADATA, DO NOT EDIT +created_at: 2025-11-19T13:27:46Z +last_modified: 2025-11-19T13:27:46Z +# END AUTO GENERATED METADATA +--- + +# Creating Modules + +> **Hands-on:** Try the [Reuse Configuration with Modules](/terraform/tutorials/modules?utm_source=WEBSITE&utm_medium=WEB_IO&utm_offer=ARTICLE_PAGE&utm_content=DOCS) tutorials. + +A _module_ is a container for multiple resources that are used together. +You can use modules to create lightweight abstractions, so that you can +describe your infrastructure in terms of its architecture, rather than +directly in terms of physical objects. + +The `.tf` files in your working directory when you run [`terraform plan`](/terraform/cli/commands/plan) +or [`terraform apply`](/terraform/cli/commands/apply) together form the _root_ +module. That module may [call other modules](/terraform/language/modules/syntax#calling-a-child-module) +and connect them together by passing output values from one to input values +of another. + +To learn how to _use_ modules, see [the Modules configuration section](/terraform/language/modules). +This section is about _creating_ re-usable modules that other configurations +can include using `module` blocks. + +## Module structure + +Re-usable modules are defined using all of the same +[configuration language](/terraform/language) concepts we use in root modules. +Most commonly, modules use: + +* [Input variables](/terraform/language/block/variable) to accept values from + the calling module. +* [Output values](/terraform/language/block/output) to return results to the + calling module, which it can then use to populate arguments elsewhere. +* [Resources](/terraform/language/resources) to define one or more + infrastructure objects that the module will manage. + +To define a module, create a new directory for it and place one or more `.tf` +files inside just as you would do for a root module. Terraform can load modules +either from local relative paths or from remote repositories; if a module will +be re-used by lots of configurations you may wish to place it in its own +version control repository. + +Modules can also call other modules using a `module` block, but we recommend +keeping the module tree relatively flat and using [module composition](/terraform/language/modules/develop/composition) +as an alternative to a deeply-nested tree of modules, because this makes +the individual modules easier to re-use in different combinations. + +## When to write a module + +In principle any combination of resources and other constructs can be factored +out into a module, but over-using modules can make your overall Terraform +configuration harder to understand and maintain, so we recommend moderation. + +A good module should raise the level of abstraction by describing a new concept +in your architecture that is constructed from resource types offered by +providers. + +For example, `aws_instance` and `aws_elb` are both resource types belonging to +the AWS provider. You might use a module to represent the higher-level concept +"[HashiCorp Consul](https://www.consul.io/) cluster running in AWS" which +happens to be constructed from these and other AWS provider resources. + +We _do not_ recommend writing modules that are just thin wrappers around single +other resource types. If you have trouble finding a name for your module that +isn't the same as the main resource type inside it, that may be a sign that +your module is not creating any new abstraction and so the module is +adding unnecessary complexity. Just use the resource type directly in the +calling module instead. + +### No-Code Provisioning in HCP Terraform + +You can also create no-code ready modules to enable the no-code provisioning workflow in HCP Terraform. No-code provisioning lets users deploy a module's resources in HCP Terraform without writing any Terraform configuration. + +No-code ready modules have additional requirements and considerations. Refer to [Designing No-Code Ready Modules](/terraform/cloud-docs/no-code-provisioning/module-design) in the HCP Terraform documentation for details. + +## Refactoring module resources + +You can include [refactoring blocks](/terraform/language/modules/develop/refactoring) to record how resource +names and module structure have changed from previous module versions. +Terraform uses that information during planning to reinterpret existing objects +as if they had been created at the corresponding new addresses, eliminating a +separate workflow step to replace or migrate existing objects. diff --git a/requirements/develop_modules/providers.mdx b/requirements/develop_modules/providers.mdx new file mode 100644 index 0000000000..5bd39d2c21 --- /dev/null +++ b/requirements/develop_modules/providers.mdx @@ -0,0 +1,367 @@ +--- +page_title: Providers Within Modules - Configuration Language +description: >- + Use providers within Terraform modules. Learn about version constraints, aliases, implicit inheritance, and passing providers to Terraform modules. +# START AUTO GENERATED METADATA, DO NOT EDIT +created_at: 2025-11-19T13:27:46Z +last_modified: 2025-11-19T13:27:46Z +# END AUTO GENERATED METADATA +--- + +# Providers Within Modules + +[inpage-providers]: #providers-within-modules + +In a configuration with multiple modules, there are some special considerations +for how resources are associated with provider configurations. + +Each resource in the configuration must be associated with one provider +configuration. Provider configurations, unlike most other concepts in +Terraform, are global to an entire Terraform configuration and can be shared +across module boundaries. Provider configurations can be defined only in a +root Terraform module. + +Providers can be passed down to descendant modules in two ways: either +_implicitly_ through inheritance, or _explicitly_ via the `providers` argument +within a `module` block. These two options are discussed in more detail in the +following sections. + +A module intended to be called by one or more other modules must not contain +any `provider` blocks. A module containing its own provider configurations is +not compatible with the `for_each`, `count`, and `depends_on` arguments that +were introduced in Terraform v0.13. For more information, see +[Legacy Shared Modules with Provider Configurations](#legacy-shared-modules-with-provider-configurations). + +Provider configurations are used for all operations on associated resources, +including destroying remote objects and refreshing state. Terraform retains, as +part of its state, a reference to the provider configuration that was most +recently used to apply changes to each resource. When a `resource` block is +removed from the configuration, this record in the state will be used to locate +the appropriate configuration because the resource's `provider` argument +(if any) will no longer be present in the configuration. + +As a consequence, you must ensure that all resources that belong to a +particular provider configuration are destroyed before you can remove that +provider configuration's block from your configuration. If Terraform finds +a resource instance tracked in the state whose provider configuration block is +no longer available then it will return an error during planning, prompting you +to reintroduce the provider configuration. + +## Provider Version Constraints in Modules + +Although provider _configurations_ are shared between modules, each module must +declare its own [provider requirements](/terraform/language/providers/requirements), so that +Terraform can ensure that there is a single version of the provider that is +compatible with all modules in the configuration and to specify the +[source address](/terraform/language/providers/requirements#source-addresses) that serves as +the global (module-agnostic) identifier for a provider. + +To declare that a module requires particular versions of a specific provider, +use a `required_providers` block inside a `terraform` block: + +```hcl +terraform { + required_providers { + aws = { + source = "hashicorp/aws" + version = ">= 2.7.0" + } + } +} +``` + +A provider requirement says, for example, "This module requires version v2.7.0 +of the provider `hashicorp/aws` and will refer to it as `aws`." It doesn't, +however, specify any of the configuration settings that determine what remote +endpoints the provider will access, such as an AWS region; configuration +settings come from provider _configurations_, and a particular overall Terraform +configuration can potentially have +[several different configurations for the same provider](/terraform/language/block/provider#select-an-alternate-provider-configuration). + +## Provider Aliases Within Modules + +To declare multiple configuration names for a provider within a module, add the +`configuration_aliases` argument: + +```hcl +terraform { + required_providers { + aws = { + source = "hashicorp/aws" + version = ">= 2.7.0" + configuration_aliases = [ aws.alternate ] + } + } +} +``` + +The above requirements are identical to the previous, with the addition of the +alias provider configuration name `aws.alternate`, which can be referenced by +resources using the `provider` argument. + +If you are writing a shared Terraform module, constrain only the minimum +required provider version using a `>=` constraint. This should specify the +minimum version containing the features your module relies on, and thus allow a +user of your module to potentially select a newer provider version if other +features are needed by other parts of their overall configuration. + +## Implicit Provider Inheritance + +For convenience in simple configurations, a child module automatically inherits +[default provider configurations](/terraform/language/block/provider#alias) from its parent. This means that +explicit `provider` blocks appear only in the root module, and downstream +modules can simply declare resources for that provider and have them +automatically associated with the root provider configurations. + +For example, the root module might contain only a `provider` block and a +`module` block to instantiate a child module: + +```hcl +provider "aws" { + region = "us-west-1" +} + +module "child" { + source = "./child" +} +``` + +The child module can then use any resource from this provider with no further +provider configuration required: + +```hcl +resource "aws_s3_bucket" "example" { + bucket = "provider-inherit-example" +} +``` + +We recommend using this approach when a single configuration for each provider +is sufficient for an entire configuration. + +~> **Note:** Only provider configurations are inherited by child modules, not provider source or version requirements. Each module must [declare its own provider requirements](/terraform/language/providers/requirements). This is especially important for non-HashiCorp providers. + +In more complex situations there may be +[multiple provider configurations](/terraform/language/block/provider#using-an-alternate-provider-configuration), +or a child module may need to use different provider settings than +its parent. For such situations, you must pass providers explicitly. + +## Passing Providers Explicitly + +When child modules each need a different configuration of a particular +provider, or where the child module requires a different provider configuration +than its parent, you can use the `providers` argument within a `module` block +to explicitly define which provider configurations are available to the +child module. For example: + +```hcl +# The default "aws" configuration is used for AWS resources in the root +# module where no explicit provider instance is selected. +provider "aws" { + region = "us-west-1" +} + +# An alternate configuration is also defined for a different +# region, using the alias "usw2". +provider "aws" { + alias = "usw2" + region = "us-west-2" +} + +# An example child module is instantiated with the alternate configuration, +# so any AWS resources it defines will use the us-west-2 region. +module "example" { + source = "./example" + providers = { + aws = aws.usw2 + } +} +``` + +The `providers` argument within a `module` block is similar to +[the `provider` argument](/terraform/language/block/resource#provider) +within a resource, but is a map rather than a single string because a module may +contain resources from many different providers. + +The keys of the `providers` map are provider configuration names as expected by +the child module, and the values are the names of corresponding configurations +in the _current_ module. + +Modules implicitly inherit default providers. Setting a `providers` argument within a `module` block overrides the default inheritance behavior for that provider. + +Additional provider configurations (those with the `alias` argument set) are +_never_ inherited automatically by child modules, and so must always be passed +explicitly using the `providers` map. For example, a module +that configures connectivity between networks in two AWS regions is likely +to need both a source and a destination region. In that case, the root module +may look something like this: + +```hcl +provider "aws" { + alias = "usw1" + region = "us-west-1" +} + +provider "aws" { + alias = "usw2" + region = "us-west-2" +} + +module "tunnel" { + source = "./tunnel" + providers = { + aws.src = aws.usw1 + aws.dst = aws.usw2 + } +} +``` + +The subdirectory `./tunnel` must then declare the configuration aliases for the +provider so the calling module can pass configurations with these names in its `providers` argument: + +```hcl +terraform { + required_providers { + aws = { + source = "hashicorp/aws" + version = ">= 2.7.0" + configuration_aliases = [ aws.src, aws.dst ] + } + } +} +``` + +Each resource should then have its own `provider` attribute set to either +`aws.src` or `aws.dst` to choose which of the two provider configurations to +use. + +## Legacy Shared Modules with Provider Configurations + +In Terraform v0.10 and earlier there was no explicit way to use different +configurations of a provider in different modules in the same configuration, +and so module authors commonly worked around this by writing `provider` blocks +directly inside their modules, making the module have its own separate +provider configurations separate from those declared in the root module. + +However, that pattern had a significant drawback: because a provider +configuration is required to destroy the remote object associated with a +resource instance as well as to create or update it, a provider configuration +must always stay present in the overall Terraform configuration for longer +than all of the resources it manages. If a particular module includes +both resources and the provider configurations for those resources then +removing the module from its caller would violate that constraint: both the +resources and their associated providers would, in effect, be removed +simultaneously. + +Terraform v0.11 introduced the mechanisms described in earlier sections to +allow passing provider configurations between modules in a structured way, and +thus we explicitly recommended against writing a child module with its own +provider configuration blocks. However, that legacy pattern continued to work +for compatibility purposes -- though with the same drawback -- until Terraform +v0.13. + +Terraform v0.13 introduced the possibility for a module itself to use the +`for_each`, `count`, and `depends_on` arguments, but the implementation of +those unfortunately conflicted with the support for the legacy pattern. + +To retain the backward compatibility as much as possible, Terraform v0.13 +continues to support the legacy pattern for module blocks that do not use these +new features, but a module with its own provider configurations is not +compatible with `for_each`, `count`, or `depends_on`. Terraform will produce an +error if you attempt to combine these features. For example: + +``` +Error: Module does not support count + + on main.tf line 15, in module "child": + 15: count = 2 + +Module "child" cannot be used with count because it contains a nested provider +configuration for "aws", at child/main.tf:2,10-15. + +This module can be made compatible with count by changing it to receive all of +its provider configurations from the calling module, by using the "providers" +argument in the calling module block. +``` + +To make a module compatible with the new features, you must remove all of the +`provider` blocks from its definition. + +If the new version of the module declares `configuration_aliases`, or if the +calling module needs the child module to use different provider configurations +than its own default provider configurations, the calling module must then +include an explicit `providers` argument to describe which provider +configurations the child module will use: + +```hcl +provider "aws" { + region = "us-west-1" +} + +provider "aws" { + region = "us-east-1" + alias = "east" +} + +module "child" { + count = 2 + providers = { + # By default, the child module would use the + # default (unaliased) AWS provider configuration + # using us-west-1, but this will override it + # to use the additional "east" configuration + # for its resources instead. + aws = aws.east + } +} +``` + +Since the association between resources and provider configurations is +static, module calls using `for_each` or `count` cannot pass different +provider configurations to different instances. If you need different +instances of your module to use different provider configurations then you +must use a separate `module` block for each distinct set of provider +configurations: + +```hcl +provider "aws" { + alias = "usw1" + region = "us-west-1" +} + +provider "aws" { + alias = "usw2" + region = "us-west-2" +} + +provider "google" { + alias = "usw1" + credentials = file("account.json") + project = "my-project-id" + region = "us-west1" + zone = "us-west1-a" +} + +provider "google" { + alias = "usw2" + credentials = file("account.json") + project = "my-project-id" + region = "us-west2" + zone = "us-west2-a" +} + +module "bucket_w1" { + source = "./publish_bucket" + providers = { + aws.src = aws.usw1 + google.src = google.usw1 + } +} + +module "bucket_w2" { + source = "./publish_bucket" + providers = { + aws.src = aws.usw2 + google.src = google.usw2 + } +} +``` diff --git a/requirements/develop_modules/publish.mdx b/requirements/develop_modules/publish.mdx new file mode 100644 index 0000000000..5d83b6665d --- /dev/null +++ b/requirements/develop_modules/publish.mdx @@ -0,0 +1,45 @@ +--- +page_title: Publishing Modules +description: A module is a container for multiple resources that are used together. +# START AUTO GENERATED METADATA, DO NOT EDIT +created_at: 2025-11-19T13:27:46Z +last_modified: 2025-11-19T13:27:46Z +# END AUTO GENERATED METADATA +--- + +# Publishing Modules + +If you've built a module that you intend to be reused, we recommend +[publishing the module](/terraform/registry/modules/publish) on the +[Terraform Registry](https://registry.terraform.io). This will version +your module, generate documentation, and more. + +Published modules can be easily consumed by Terraform, and users can +[constrain module versions](/terraform/language/modules/syntax#version) +for safe and predictable updates. The following example shows how a caller +might use a module from the Terraform Registry: + +```hcl +module "consul" { + source = "hashicorp/consul/aws" +} +``` + +If you do not wish to publish your modules in the public registry, you can +instead use a [private registry](/terraform/registry/private) to get +the same benefits. + +We welcome contributions of Terraform modules from our community members, partners, and customers. Our ecosystem is made richer by each new module created or an existing one updated, as they reflect the wide range of experience and technical requirements of the community that uses them. Our cloud provider partners often seek to develop specific modules for popular or challenging use cases on their platform and utilize them as valuable learning experiences to empathize with their users. Similarly, our community module developers incorporate a variety of opinions and use cases from the broader Terraform community. Both types of modules have their place in the Terraform registry, accessible to practitioners who can decide which modules best fit their requirements. + +## Distribution via other sources + +Although the registry is the native mechanism for distributing re-usable +modules, Terraform can also install modules from +[various other sources](/terraform/language/modules/sources). The alternative sources +do not support the first-class versioning mechanism, but some sources have +their own mechanisms for selecting particular VCS commits, etc. + +We recommend that modules distributed via other protocols still use the +[standard module structure](/terraform/language/modules/develop/structure) so that they can +be used in a similar way as a registry module or be published on the registry +at a later time. diff --git a/requirements/develop_modules/refactoring.mdx b/requirements/develop_modules/refactoring.mdx new file mode 100644 index 0000000000..b1f0782cac --- /dev/null +++ b/requirements/develop_modules/refactoring.mdx @@ -0,0 +1,448 @@ +--- +page_title: Refactor modules +description: How to make backward-compatible changes to modules already in use. +# START AUTO GENERATED METADATA, DO NOT EDIT +created_at: 2025-11-19T13:27:46Z +last_modified: 2025-12-02T06:26:00-08:00 +# END AUTO GENERATED METADATA +--- + +# Refactor modules + +By default, Terraform interprets a change as an instruction to destroy the existing resource and create a new resource at the new address. You can use a `moved` block to update a resource address without destroying it. + + +> **Hands On:** Try the [Use Configuration to Move Resources](/terraform/tutorials/configuration-language/move-config) tutorial. + +## Overview + +Add a `moved` block to your configuration to move or rename an object. Before creating a new plan for the resource specified in the `to` argument, Terraform checks the state for an existing resource at the `from` address. Terraform includes the update to the resource address in the next execution plan. The address change does not destroy the resource. + +You can use `moved` blocks for the following use cases: + +- [Move a resource or module](#move-a-resource-or-module) +- [Rename a resource](#rename-a-resource) +- [Create multiple instances](#create-multiple-instances) +- [Rename a module call](#rename-a-module-call) +- [Split a module](#split-a-module) + +When the module or resource is no longer necessary, you can [remove `moved` blocks](#remove-a-moved-block) from the configuration. Note that removing a `moved` block is a breaking change. + +## Requirements + +Terraform v1.1 and later is required to use `moved` blocks to explicitly refactor module addresses. Instead, use the [`terraform state mv` CLI command](/terraform/cli/commands/state/mv). + +## Move a resource or module + +Add a [`moved` block](/terraform/language/block/moved) to your configuration and specify the following arguments: + +- `from`: The previous address of the module or resource. +- `to`: The new address of the module or resource. + +In the following example, the resource named `aws_instance.a` has been moved to `aws_instance.b`: + +```hcl +moved { + from = aws_instance.a + to = aws_instance.b +} +``` + +Before creating a new plan for `aws_instance.b`, Terraform first checks +whether there is an existing object for `aws_instance.a` recorded in the state. +If there is an existing object, Terraform renames that object to +`aws_instance.b` and then proceeds with creating a plan. The resulting plan is +as if the object had originally been created at `aws_instance.b`, avoiding any +need to destroy it during apply. + +The `from` and `to` addresses both use a special addressing syntax that allows +selecting modules, resources, and resources inside child modules. + +## Rename a resource + +Consider this example module with a resource configuration: + +```hcl +resource "aws_instance" "a" { + count = 2 + + # (resource-type-specific configuration) +} +``` + +Applying this configuration for the first time would cause Terraform to +create `aws_instance.a[0]` and `aws_instance.a[1]`. + +If you later choose a different name for this resource, then you can change the +name label in the `resource` block and record the old name inside a `moved` block: + +```hcl +resource "aws_instance" "b" { + count = 2 + + # (resource-type-specific configuration) +} + +moved { + from = aws_instance.a + to = aws_instance.b +} +``` + +When creating the next plan for each configuration using this module, Terraform +treats any existing objects belonging to `aws_instance.a` as if they had +been created for `aws_instance.b`: `aws_instance.a[0]` will be treated as +`aws_instance.b[0]`, and `aws_instance.a[1]` as `aws_instance.b[1]`. + +New instances of the module, which _never_ had an +`aws_instance.a`, will ignore the `moved` block and propose to create +`aws_instance.b[0]` and `aws_instance.b[1]` as normal. + +Both of the addresses in this example referred to a resource as a whole, and +so Terraform recognizes the move for all instances of the resource. That is, +it covers both `aws_instance.a[0]` and `aws_instance.a[1]` without the need +to identify each one separately. + +Each resource type has a separate schema so objects of different types +are not typically compatible. You can always use the `moved` block to change +the name of a resource, but some providers also let you change an object from +one resource type to another. Refer to the provider documentation for details +on which resources can move between types. You _cannot_ use the `moved` +block to change a managed resource (a `resource` block) into a data +resource (a `data` block). + +## Create multiple instances + +You can use a `moved` block when creating multiple instances of a resource or module call while preserving the original instance. + +### Enable `count` or `for_each` for a resource + +The following example module contains a single-instance resource bound to the address `aws_instance.a`: + +```hcl +resource "aws_instance" "a" { + # (resource-type-specific configuration) +} +``` + +Later, you use [`for_each`](/terraform/language/meta-arguments#for_each) with this +resource to systematically declare multiple instances. To preserve an object +that was previously associated with `aws_instance.a`, you must add a +`moved` block to specify which instance key the object will take in the new +configuration. + +In the following example, Terraform does not plan to destroy any existing object at +`aws_instance.a`. Instead, Terraform treats that object as if it were originally +created as `aws_instance.a["small"]`: + +```hcl +locals { + instances = tomap({ + big = { + instance_type = "m3.large" + } + small = { + instance_type = "t2.medium" + } + }) +} + +resource "aws_instance" "a" { + for_each = local.instances + + instance_type = each.value.instance_type + # (other resource-type-specific configuration) +} + +moved { + from = aws_instance.a + to = aws_instance.a["small"] +} +``` + + +When at least one of the two addresses includes an instance key, such as +`["small"]` in the previous example, Terraform understands both addresses as +referring to specific instances of a resource rather than the resource as a +whole. That means you can use `moved` to switch between keys and to add and +remove keys as you switch between `count`, `for_each`, or neither. + +The following examples are also valid `moved` blocks that record +changes to resource instance keys in a similar way: + +```hcl +# Both old and new configuration used "for_each", but the +# "small" element was renamed to "tiny". +moved { + from = aws_instance.b["small"] + to = aws_instance.b["tiny"] +} + +# The old configuration used "count" and the new configuration +# uses "for_each", with the following mappings from +# index to key: +moved { + from = aws_instance.c[0] + to = aws_instance.c["small"] +} +moved { + from = aws_instance.c[1] + to = aws_instance.c["tiny"] +} + +# The old configuration used "count", and the new configuration +# uses neither "count" nor "for_each", and you want to keep +# only the object at index 2. +moved { + from = aws_instance.d[2] + to = aws_instance.d +} +``` + +When you add `count` to an existing resource that didn't previously have the argument, +Terraform automatically proposes moving the original object to instance `0` +unless you write a `moved` block that explicitly mentions that resource. +However, we recommend writing out the corresponding `moved` block +explicitly to make the change clearer to future readers of the module. + +### Enable `count` or `for_each` for a module call + +The following example is a single-instance module that creates objects whose +addresses begin with `module.a`: + + +```hcl +module "a" { + source = "../modules/example" + + # (module arguments) +} +``` + +In later module versions, you may need to use +[`count`](/terraform/language/meta-arguments#count) with this resource to systematically +declare multiple instances. To preserve an object that was previously associated +with `aws_instance.a` alone, you can add a `moved` block to specify which +instance key that object will take in the new configuration. + +The following configuration above directs Terraform to treat all objects in `module.a` as +if they were originally created in `module.a[2]`. As a result, Terraform plans +to create new objects only for `module.a[0]` and `module.a[1]`: + +```hcl +module "a" { + source = "../modules/example" + count = 3 + + # (module arguments) +} + +moved { + from = module.a + to = module.a[2] +} +``` + +When at least one of the two addresses includes an instance key, such as +`[2]` in the previous example, Terraform understands both addresses as +referring to specific instances of a module call rather than the module +call as a whole. That means you can use `moved` to switch between keys and to +add and remove keys as you switch between `count`, `for_each`, or neither. + +## Rename a module call + +You can rename a call to a module in a similar way as renaming a resource. +Consider the following original module version: + +```hcl +module "a" { + source = "../modules/example" + + # (module arguments) +} +``` + +When applying this configuration, Terraform would prefix the addresses for +any resources declared in this module with the module path `module.a`. +For example, a resource `aws_instance.example` would have the full address +`module.a.aws_instance.example`. + +If you later choose a better name for this module call, then you can change the +name label in the `module` block and record the old name inside a `moved` block: + +```hcl +module "b" { + source = "../modules/example" + + # (module arguments) +} + +moved { + from = module.a + to = module.b +} +``` + +When creating the next plan for each configuration using this module, Terraform +will treat any existing object addresses beginning with `module.a` as if +they had instead been created in `module.b`. `module.a.aws_instance.example` +would be treated as `module.b.aws_instance.example`. + +Both of the addresses in this example referred to a module call as a whole, and +so Terraform recognizes the move for all instances of the call. If this +module call used `count` or `for_each` then it would apply to all of the +instances, without the need to specify each one separately. + + + +## Split a module + +As a module grows to support new requirements, it might eventually grow big +enough to warrant splitting into two separate modules. + +Consider this example module: + +```hcl +resource "aws_instance" "a" { + # (other resource-type-specific configuration) +} + +resource "aws_instance" "b" { + # (other resource-type-specific configuration) +} + +resource "aws_instance" "c" { + # (other resource-type-specific configuration) +} +``` + +You can split this into two modules as follows: + +* `aws_instance.a` now belongs to module "x". +* `aws_instance.b` also belongs to module "x". +* `aws_instance.c` belongs module "y". + +To achieve this refactoring without replacing existing objects bound to the +old resource addresses, you must: + +1. Write module "x", copying over the two resources it should contain. +1. Write module "y", copying over the one resource it should contain. +1. Edit the original module to no longer include any of these resources, and + instead to contain only shim configuration to migrate existing users. + +The new modules "x" and "y" should contain only `resource` blocks: + +```hcl +# module "x" + +resource "aws_instance" "a" { + # (other resource-type-specific configuration) +} + +resource "aws_instance" "b" { + # (other resource-type-specific configuration) +} +``` + +```hcl +# module "y" + +resource "aws_instance" "c" { + # (other resource-type-specific configuration) +} +``` + +The original module, now only a shim for backward-compatibility, calls the +two new modules and indicates that the resources moved into them: + +```hcl +module "x" { + source = "../modules/x" + + # ... +} + +module "y" { + source = "../modules/y" + + # ... +} + +moved { + from = aws_instance.a + to = module.x.aws_instance.a +} + +moved { + from = aws_instance.b + to = module.x.aws_instance.b +} + +moved { + from = aws_instance.c + to = module.y.aws_instance.c +} +``` + +When an existing user of the original module upgrades to the new "shim" +version, Terraform notices these three `moved` blocks and behaves +as if the objects associated with the three old resource addresses were +originally created inside the two new modules. + +New users of this family of modules may use either the combined shim module +_or_ the two new modules separately. You may wish to communicate to your +existing users that the old module is now deprecated and so they should use +the two separate modules for any new needs. + +The multi-module refactoring situation is unusual in that it violates the +typical rule that a parent module sees its child module as a "closed box", +unaware of exactly which resources are declared inside it. This compromise +assumes that all three of these modules are maintained by the same people +and distributed together in a single +[module package](/terraform/language/modules/sources#modules-in-package-sub-directories). + +Terraform resolves module references in `moved` blocks relative to the module +instance they are defined in. For example, if the original module above were +already a child module named `module.original`, the reference to +`module.x.aws_instance.a` would resolve as +`module.original.module.x.aws_instance.a`. A module may only make `moved` +statements about its own objects and objects of its child modules. + +If you need to refer to resources within a module that was called using +`count` or `for_each` meta-arguments, you must specify a specific instance +key to use in order to match with the new location of the resource +configuration: + +```hcl +moved { + from = aws_instance.example + to = module.new[2].aws_instance.example +} +``` + +## Remove a `moved` block + +Over time, a long-lasting module may accumulate many `moved` blocks. + +Removing a `moved` block is a breaking change because any configurations that refer to the old address will plan to delete the existing object instead of move it. We strongly recommend that you retain all historical `moved` blocks from earlier versions of your modules to preserve the upgrade path for users of any previous version. + +If you do decide to remove `moved` blocks, proceed with caution. It can be safe to remove `moved` blocks when you are maintaining private modules within an organization and you are certain that all users have successfully run `terraform apply` with your new module version. + +If you need to rename or move the same object twice, we recommend chaining `moved` blocks to document the full change history: + +```hcl +moved { + from = aws_instance.a + to = aws_instance.b +} + +moved { + from = aws_instance.b + to = aws_instance.c +} +``` + +Recording a sequence of moves in this way allows for successful upgrades for +both configurations with objects at `aws_instance.a` and configurations with +objects at `aws_instance.b`. In both cases, Terraform treats the existing +object as if it had been originally created as `aws_instance.c`. diff --git a/requirements/develop_modules/structure.mdx b/requirements/develop_modules/structure.mdx new file mode 100644 index 0000000000..7bedb69dff --- /dev/null +++ b/requirements/develop_modules/structure.mdx @@ -0,0 +1,133 @@ +--- +page_title: Standard Module Structure +description: >- + Learn about the recommended file and directory structure for developing reusable modules distributed as separate repositories. +# START AUTO GENERATED METADATA, DO NOT EDIT +created_at: 2025-11-19T13:27:46Z +last_modified: 2025-11-19T13:27:46Z +# END AUTO GENERATED METADATA +--- + +# Standard Module Structure + +The standard module structure is a file and directory layout we recommend for +reusable modules distributed in separate repositories. Terraform tooling is +built to understand the standard module structure and use that structure to +generate documentation, index modules for the module registry, and more. + +The standard module structure expects the layout documented below. The list may +appear long, but everything is optional except for the root module. Most modules +don't need to do any extra work to follow the standard structure. + +* **Root module**. This is the **only required element** for the standard + module structure. Terraform files must exist in the root directory of + the repository. This should be the primary entrypoint for the module and is + expected to be opinionated. For the + [Consul module](https://registry.terraform.io/modules/hashicorp/consul) + the root module sets up a complete Consul cluster. It makes a lot of assumptions + however, and we expect that advanced users will use specific _nested modules_ + to more carefully control what they want. + +* **README**. The root module and any nested modules should have README + files. This file should be named `README` or `README.md`. The latter will + be treated as markdown. There should be a description of the module and + what it should be used for. If you want to include an example for how this + module can be used in combination with other resources, put it in an [examples + directory like this](https://github.com/hashicorp/terraform-aws-consul/tree/master/examples). + Consider including a visual diagram depicting the infrastructure resources + the module may create and their relationship. + + The README doesn't need to document inputs or outputs of the module because + tooling will automatically generate this. If you are linking to a file or + embedding an image contained in the repository itself, use a commit-specific + absolute URL so the link won't point to the wrong version of a resource in the + future. + +* **LICENSE**. The license under which this module is available. If you are + publishing a module publicly, many organizations will not adopt a module + unless a clear license is present. We recommend always having a license + file, even if it is not an open source license. + +* **`main.tf`, `variables.tf`, `outputs.tf`**. These are the recommended filenames for + a minimal module, even if they're empty. `main.tf` should be the primary + entrypoint. For a simple module, this may be where all the resources are + created. For a complex module, resource creation may be split into multiple + files but any nested module calls should be in the main file. `variables.tf` + and `outputs.tf` should contain the declarations for variables and outputs, + respectively. + +* **Variables and outputs should have descriptions.** All variables and + outputs should have one or two sentence descriptions that explain their + purpose. This is used for documentation. See the documentation for + [variable configuration](/terraform/language/block/variable) and + [output configuration](/terraform/language/block/output) for more details. + +* **Nested modules**. Nested modules should exist under the `modules/` + subdirectory. Any nested module with a `README.md` is considered usable + by an external user. If a README doesn't exist, it is considered for internal + use only. These are purely advisory; Terraform will not actively deny usage + of internal modules. Nested modules should be used to split complex behavior + into multiple small modules that advanced users can carefully pick and + choose. For example, the + [Consul module](https://registry.terraform.io/modules/hashicorp/consul) + has a nested module for creating the Cluster that is separate from the + module to setup necessary IAM policies. This allows a user to bring in their + own IAM policy choices. + + If the root module includes calls to nested modules, they should use relative + paths like `./modules/consul-cluster` so that Terraform will consider them + to be part of the same repository or package, rather than downloading them + again separately. + + If a repository or package contains multiple nested modules, they should + ideally be [composable](/terraform/language/modules/develop/composition) by the caller, rather than + calling directly to each other and creating a deeply-nested tree of modules. + +* **Examples**. Examples of using the module should exist under the + `examples/` subdirectory at the root of the repository. Each example may have + a README to explain the goal and usage of the example. Examples for + submodules should also be placed in the root `examples/` directory. + + Because examples will often be copied into other repositories for + customization, any `module` blocks should have their `source` set to the + address an external caller would use, not to a relative path. + +A minimal recommended module following the standard structure is shown below. +While the root module is the only required element, we recommend the structure +below as the minimum: + +```sh +$ tree minimal-module/ +. +├── README.md +├── main.tf +├── variables.tf +├── outputs.tf +``` + +A complete example of a module following the standard structure is shown below. +This example includes all optional elements and is therefore the most +complex a module can become: + +```sh +$ tree complete-module/ +. +├── README.md +├── main.tf +├── variables.tf +├── outputs.tf +├── ... +├── modules/ +│   ├── nestedA/ +│   │   ├── README.md +│   │   ├── variables.tf +│   │   ├── main.tf +│   │   ├── outputs.tf +│   ├── nestedB/ +│   ├── .../ +├── examples/ +│   ├── exampleA/ +│   │   ├── main.tf +│   ├── exampleB/ +│   ├── .../ +``` diff --git a/requirements/module-best-practices.md b/requirements/module-best-practices.md new file mode 100644 index 0000000000..cddd9a1317 --- /dev/null +++ b/requirements/module-best-practices.md @@ -0,0 +1,908 @@ +In this comprehensive blog, I have shared detailed Terraform module development best practices. + +Whether you're creating modules for your team or contributing to open-source projects, these tips will help you write clean, reusable, and easy-to-maintain infrastructure code. + +By the end of this guide, you will have learned: + +How to build reusable and well-structured Terraform modules. +Best practices for naming, inputs, outputs, and file organization. +How to test, document, and version your modules effectively. +How to use CI/CD to automate checks and keep your modules up to date. +How to prepare modules for open-source use with proper GitHub setup. +The article series is divided into two parts. + +In Part 1, we'll look at best practices for developing open-source Terraform modules and talk about the commonly used Terraform module repository template pattern that many organizations use. +In Part 2, we'll dive into a well-designed, pre-configured template that implements all best practices from Part 1, meets the requirements of most Terraform projects, and can be bootstrapped in 5 minutes. +Terraform Modules +Terraform modules are a key paradigm for managing infrastructure as code. They reduce complexity, increase reusability, and ensure consistency across projects. + +By encapsulating infrastructure configurations into modular components, teams can standardize resource setups, reduce errors, and share best practices across projects. + +However, the value of these modules is dependent on their internal structure. A well-structured module guarantees that infrastructure stays understandable and manageable as complexity grows. + +It allows development teams to make changes over time without introducing additional risks or obstacles. + +In contrast, badly constructed modules can cause confusion, make debugging more difficult, and require more maintenance due to hidden bugs and other issues. + +⚠️ +Please keep in mind that HashiCorp changed the Terraform licensing, leading the community to create a fork called OpenTofu, which is licensed under the MPL-2.0 license. All tips, tricks, and best practices shared in this article are fully compatible with OpenTofu. +Let’s get started! + +Terraform Module Development Best Practices +In this section, we will look at best practices for Terraform module development that may help for a wide range of users. + +I recommend you adopt only those practices that are relevant to your project or organization. Although they are suggestions, implementing as many of these into practice as you can will improve the quality and maintainability of your Terraform code. + +Terraform-related configuration +In this section we will look at best practices realated to terraform configurations. + +#1: Use clear input and output definitions +Name: To demonstrate the purpose of variables and outputs, give them descriptive names. +Type: Always declare variable types to ensure consistency. Avoid using any type or missing type. +Default value: Set default values whenever possible. Having a default value is always better to leaving it empty. +Validations: To verify inputs, use Terraform's built-in validation. +Sensitive: Mark variables or outputs handling secrets as sensitive = true to safeguard sensitive data. +Write documentation: Document variables and outputs, including their purpose, types, and default values. +💡 +Use tools like terraform-docs to generate this automatically. +Think about backward compatibility: Adding a variable with a default value is backwards-compatible. Removing a variable is backwards-incompatible; therefore, avoid it wherever possible. +Minimize unnecessary variables: Before creating a variable, consider: "Will this value ever need to change?" If the response is no, use the local value. +#2: Keep Logical Structure +Use a dedicated repository for each Terraform module. Submodules can be included in the module, but they must be directly related to the main module and not implement unrelated functionality. + +💡 +Use the standard module structure for Terraform modules +Organize module files logically: + +main.tf for core resources. +variables.tf for input variables. +outputs.tf for outputs.versions.tf for the minimal terraform version, and provider configurations (if required). +README.md for the repository's basic documentation of the module and all of its nested folders. +To improve readability, split logic into separate files. + +For instance, you may split the main.tf file into something like this if your module is in charge of setting up an Application Load Balancer (ALB), EC2 instances, and S3 buckets: + +network.tf: Contains the configuration for the Application Load Balancer (ALB) and related networking resources. +vms.tf or ec2.tf: Manages the configuration for EC2 instances. +bucket.tf or s3.tf: Includes the S3 bucket configuration. +Use the following directories: + +modules/ for nested or reusable sub-modules. +examples/ for sample configurations and usage examples. +docs/ for additional documentation, such as detailed design or architecture diagrams. +tests/ for infrastructure tests +scripts/ for custom scripts that Terraform may call during execution +helpers/ for organizing helper scripts required for automation or maintenance but not immediately invoked by Terraform. +files/ for static files that Terraform references but does not run. It can be startup scripts, configuration files +wrappers/ for implementing Terragrunt wrapper pattern. +#3: Use nested submodules for complex logic (modules/ directory) +When you dealing with complex Terraform configurations, use nested submodules to break down logic into smaller, reusable units. Follow these best practices: + +Place submodules in the modules/ directory using the naming pattern modules/${module_name}. + +modules/ +├── network/ +├── compute/ +└── storage/ +Nested modules should be considered as private unless explicitly stated in the module’s documentation. + +Users should avoid using nested modules directly unless the developer has specified that they are safe and designed for external usage. Consider it, and remember to update the sub-module information in the documentation during the development. +Always refer to the provided documentation for guidance on module usage. +Terraform does not track refactored resources. When you move resources from a top-level module to submodule or just rename them, Terraform may consider them new resources and try to recreate them. This can lead to significant issues for users, such as resource outages or data loss. + +To mitigate this behavior, use moved in scope of refactoring. + +moved { + from = aws_instance.old_name + to = module.new_submodule.aws_instance.new_name +} +If refactoring is unavoidable, let users know about it as a significant change in the CHANGELOG and give them detailed guidance on how to manage the changeover. + +#4: Try to avoid custom scripts (scripts/ directory) +General rule: Custom scripts should be considered a last resort. Try to avoid them if it is possible. Resources created by such scripts are not tracked or managed by Terraform state, which can lead to inconsistencies. + +Exceptions to the rule: Custom scripts may be necessary when Terraform does not support the desired functionality. + +Before starting to use scripts, consider these alternatives: + +Provider-defined functions: Explore the existing capabilities of your Terraform provider +Try to use alternative providers: For example, in the case of AWS you can use awscc (AWS Cloud Control API) which is generated from CloudFormation resource definitions and has different resource scopes. +3. Use something like TerraCurl for unsupported resources. + +You can check more information about it in the article Writing Terraform for unsupported resources. + +If custom scripts are indeed necessary and if none of the above options work, follow these two key steps: + +Document scripts clearly: Provide a detailed explanation for why the custom script exists. Include a deprecation plan, if possible, and write clear documentation for the script and its usage. +2. Use Provisioners for Execution: Use Terraform’s provisioners (e.g., local-exec) to call custom scripts when needed. + +#5: Automate toil with helper scripts (helpers/ directory) +For any repetitive or manual tasks that need automation, create simple scripts and store them in the helpers/ directory. These scripts should abide by the following principles: + +Clear Documentation +Provide a brief overview of the script's purpose and why it exists. +Include usage instructions and any other relevant context. +2. Usability Standards + +Implement argument validation to handle incorrect or missing inputs gracefully (in case of using bash, you may check “How to Use Bash Getopts With Examples" article). +Include a --help option that explains the script’s functionality, arguments, and examples of usage. +#6: Store static files in a separate directory (files/ directory) +Templates, assets, and other items that Terraform references but does not run directly are known as static files. To properly organize these files, adhere to the following rules: + +Separate lengthy HereDocs +Move lengthy HereDoc content into external files for better readability. +Use the file() function to reference these files in your Terraform code. +2. Use .tftpl or .tpl file extensions for templates + +When working with Terraform's templatefile() function, use .tftpl or .tpl extensions for template files. While Terraform doesn't enforce this naming standard (you can use any that you like), this extension will help your editor understand the content and might offer a better editing experience as a result. +3. Place Templates in a templates/ subdirectory + +Organize all template files within a templates/ subdirectory inside the files/ directory. +A typical example of content within the files/ directory is an AWS Lambda function, commonly placed in files//. + +In this case, the files// directory can be considered the root folder, and if the Lambda function is written in Python, this directory may contain an internal structure such as Python packages, dependencies, build scripts or other related files. + +#7: Implement Terragrunt Wrapper +Many people use Terragrunt to follow DRY principles. Terragrunt can handle a wide range of tasks across your stack, but module development requires some preparation as well. + +In some cases, it is not feasible to use Terraform's native for_each feature. Hence, the Terragrunt wrapper, also known as the single-module wrapper pattern, comes into play. This technique helps in situations like: + +You need to deploy multiple copies of a module with different configurations. +Your environment's limitations or complexities prevent you from using native for_each Terraform function. +To apply this approach, establish a new directory wrappers/ with at least three files: + +main.tf +outputs.tf +variables.tf +Let’s check them in more detail. + +main.tf file contains: + +Terraform source +for_each loop over an items variable +All inputs that the module has in the following format: `my_input_variable = try(each.value.my_input_variable, var.defaults.my_input_variable, {})` +For example: + +module "wrapper" { + source = "../" + + for_each = var.items + + my_input_variable = try(each.value.my_input_variable, + var.defaults.my_input_variable, {}) +} +variables.tf file contains: + +defaults variable, that is, a map of default values. These default values will be used as a backup for each input variable if it is empty or missing. +items variable, which maps items to create a wrapper from +For example: + +variable "defaults" { + description = "A map of default values. These default values will be used as a backup for each input variable if it is empty or missing." + type = any + default = {} +} + +variable "items" { + description = "Maps of items to create a wrapper from. Values are passed through to the module." + type = any + default = {} +} +outputs.tf file contains: + +wrapper output with a map of outputs from a wrapper. +For example: + +output "wrapper" { + description = "Map of outputs from a wrapper." + value = module.wrapper +} +#8: Avoid providers or backend configurations +Shared modules must not configure providers or backends. These settings must be defined in root modules in order to maintain flexibility and reusability. + +It is also a good practice to use the required providers block to define only the bare minimum of necessary provider versions. If you need to let the user know about state management in your module, include this information in the README.md file. + +versions.tf Example: + +terraform { + required_version = ">= 1.0" + required_providers { + google = { + source = "hashicorp/google" + version = ">= 4.0.0" + } + } +} + +As shown in the example, it also contains a minimal Terraform version. + +Establishing it is a good practice if you know your code needs a specific version or newer to work properly. If you are unclear about the minimum version required for your module, set >=1.0 as the default definition. + +Test the Terraform code +Testing your Terraform code is just as important as testing other types of programming code. A lot of options exist in the Terraform ecosystem to assist you in ensuring the dependability and correctness of your modules. + +Let's look at some of the best practices for module testing. + +Static analysis +Typically, this involves analyzing the syntax, structure, and deployment configuration using linters, static code analysis tools, or Terraform dry-runs. + +These tools are used before the deployment. This topic will be discussed in further detail in the "Code Linting and Quality" section. + +Module integration testing +Testing modules in isolation is a useful way to confirm that they work as expected. Module integration testing entails deploying the module to a test environment and ensuring that it generates the expected resources. The following testing frameworks can help you write and manage tests: + +Terraform-tests: a built-in Terraform feature that allows you to write tests with HCL. +Terraform-check: a built-in Terraform feature that may help you to validate infrastructure outside the usual resource lifecycle. +Terratest: a Go library that provides patterns and helper functions for testing infrastructure. To gain hands-on experience, start with the article "Automating AWS Infrastructure Testing With Terratest". +terraform-compliance: a lightweight, security and compliancefocused BDD test framework +Google tools +Google's blueprint testing framework +InSpec +End-to-end testing +Integration testing is important to verify that multiple modules work together properly. + +In the scope of this method, all modules that exist in your environment are deployed together. Following that, tests are checking that everything works properly using API calls or any other methods. + +Although this method is long and expensive, it offers the highest level of verification and can ensure that changes do not cause problems with your production systems. + +Some highlights +Don't use old testing tools (like Kitchen-Terraform) and remember to keep the version of tools updated. +Only the first two approaches, static analysis and module integration testing, are relevant in the context of module development. This is because we can't predict how or where the user will use the module. As a result, end-to-end testing is ineffective in this scenario. +All of the tools I outlined for integration testing are also applicable to end-to-end testing. +To learn more about Terraform testing, check the following links; +Testing HashiCorp Terraform +Implement end-to-end Terratest testing on Terraform projects +Write a documentation +A Terraform module's success depends on clear and effective documentation. + +A well-structured README.md makes usage easier, reduces complexity, and improves the overall user experience. Conversely, badly organized documentation might have a negative impact on the module's popularity and usefulness. + +To create high-quality module documentation, adopt the following principles: + +Provide comprehensive examples: Include detailed sample configurations that explain module usage. It's best to give as many examples as possible. Use the examples/ folder for this. +Automate documentation generation: Use terraform-docs to automatically generate and maintain up-to-date documentation +Generate input/output documentation automatically. +Add documentation for: +Detailed examples in the examples/ directory. +Submodules: They should be properly documented, with explanations of their purpose, usage, and specific configurations. +Create a well-structured README.md .Essentially, include the following sections: +An overview of the module. +Examples of usage. +Input and output definitions. +Version compatibility and known limits. +Any badges like “Terraform support” or “OpenTofu support” +Git configuration +Git has various configuration settings that improve project management. The following are essential .git configuration files and their purposes. + +.gitignore +This file specifies which files and directories Git should ignore, so they are not tracked or committed to the repository. The Terraform codebase's gitignore file has to include the following exclusions: + +Terraform +state files +lock files +init directory +extra tfvars files +Any build artifacts generated by non-Terraform code, such as Python bytecode files (.pyc) or anything else +Log files +Example: + +**/.terraform/* +*.tfstate +*.tfstate.* +crash.log +crash.*.log +*.tfvars +*.tfvars.json + +override.tf +override.tf.json +*_override.tf +*_override.tf.json + +.terraform.lock.hcl +.terraform.tfstate.lock.info + +.terraformrc +terraform.rc + +*.pyc +.gitattributes +This file keeps uniform file formatting and prevents line ending issues, especially in projects with contributors from different operating systems. Here is an example of essential configuration: + +# Normalize Terraform files +*.tf text + +# Enforce LF (Unix-style) line endings +*.sh eol=lf +*.ts text eol=lf +*.json text eol=lf +*.md text eol=lf +*.tf text eol=lf +*.txt text eol=lf + +language-configuration.json linguist-language=jsonc +.vscode/**.json linguist-language=jsonc +.gitconfig +This file defines project-specific Git settings for enforcing standards and managing author information. By default, it's good to include the following configuration: + +[core] +autocrlf = input +longpaths = true +autocrlf: Manages line-ending normalization across operating systems (e.g., Windows, Linux, MacOS). When set to input, Git only changes Windows-style line endings (CRLF) to Unix-style line endings (LF) only when a file is committed + +longpaths: Allows Git to support file paths longer than 260 characters, the default maximum path length on Windows. This option prevents errors when working with deeply nested directories or repositories that have long file names. + +Use a trunk-based development +Trunk-based development is the most frequently recommended Git branching strategy, according to DevOps Research and Assessment (DORA), and Google DevOps capabilities. With this method, you can always maintain the main branch clean of unnecessary files or artifacts and ready for deployment. + +Trunk-based development for Terraform module repositories can be adopted in the following way: + +main branch +The main branch contains the most recent code and serves as the main development branch. + +It should always remain clean, protected, and ready for use. + +Feature and bug-fix branches +Development takes place in the feature and bug-fix branches. These branches are chopped off from the main branch. + +Naming convention: + +Feature branches: feature/$feature_name or feat/$feature_name +Bug-fix branches: fix/$bugfix_name +Release Branches +A release branch is used to prepare code for the upcoming release. When the release branch is stable and confirmed, it should be merged with the main branch. + +Naming convention: release/$version_number (e.g., release/v1.2.0). + +Pull Requests +Once a feature or bug fix has been completed, use a pull request to merge it back into the main branch. + +To reduce merge conflicts, rebase branches before merging. + +GitHub configuration +GitHub provides a range of powerful built-in features to enhance repository management and collaboration. + +These features can be activated by adding specific files to your repository, typically within the .github folder. Using these features can help attract contributors, build interest in your repository, and improve the development experience. + + +tofuutils/tenv: A repository that uses almost every GitHub feature. +PULL_REQUEST_TEMPLATE.md +Adding this file to your repository enables project contributors to see a predefined structure in the pull request body automatically. + +It’s considered best practice to include key instructions or questions to help streamline the review process. + +Example: + +## Description +Provide a summary of the PR changes. + +## Checklist +- [ ] Code is formatted (`terraform fmt`). +- [ ] Documentation is updated. +- [ ] Tests are added or updated. + +ISSUE_TEMPLATE directory +This directory contains a collection of .md files that are designed to standardize the submission process for issues. + +These structured templates provide clear instructions to contributors when they report problems, seek enhancements, or deal with other types of issues. + + +A dialog window with pre-defined issue templates +In addition to templates, you may change issue submission behavior by including a .github/ISSUE_TEMPLATE/config.yml file in your repository. This configuration file allows you to specify the default behaviors and settings for issue templates. Example: + +blank_issues_enabled: true +In this file blank_issues_enabled means: + +true: Allows users to submit blank issues without requiring a template. +false: Blocks blank issues entirely, ensuring that all issues are submitted using only predefined templates. +CONTRIBUTING.md +The CONTRIBUTING.md file contains explicit guidelines for contributing to the project. It may include: + +Set up the development environment: Follow step-by-step instructions to clone the repository, install dependencies, and run the project locally. +Submitting pull request: Clearly outline the workflow for creating pull requests, including branch naming rules, explicit commit messages, and code standards. +Code review expectations: Define what contributors should expect from the review process and how feedback will be provided. +Issues and feature requests: Explain how contributors can submit bug reports, suggest new features, and participate in discussions. +LICENSE +The LICENSE file specifies the terms under which your project may be used, modified, and distributed. It ensures that users and contributors understand their rights and responsibilities. + +It's better to choose a license that aligns with your project's goals: + +MIT: A permissive license that allows virtually unrestricted use, modification, and distribution. +Apache 2.0: Provides similar freedoms to MIT but adds protections such as patent rights. +GPL: A copyleft license that mandates derivative works to be open source under the same license. +If you're confused about which license to choose, visit the Choose a License. It provides you with simple instructions and examples to help you choose the best license for your purposes. + +Also, as a default license, you can use the MIT license, which is ideal for a wide range of projects, including Terraform modules. + +CODE_OF_CONDUCT.md +The CODE_OF_CONDUCT.md file allows you to create community standards, indicate a friendly and inclusive project, and specify anticipated behavior. + +Typically, this file may: + +Define acceptable and inappropriate user behavior. +Specify how community members can report violations. +Outline the procedures for resolving disputes and ensuring enforcement. +Emphasize creating a welcoming space for everyone, regardless of background or experience. +FUNDING.yml +The FUNDING.md file allows the repository to display funding options. It might provide links to platforms where contributors can make donations to support the project. Examples of such platforms are: + +Patreon +Open Collective +GitHub Sponsors +Other appropriate platforms based on your requirements. +Don't forget to explain how the funds will be used, whether for development, community projects, or operational purposes. + + +A banner that GitHub displays if FUNDING.md is enabled. +CODEOWNERS +The CODEOWNERS file assigns ownership to specified files or directories, which automates pull request reviews. Example: + +# Assign maintainers for specific parts of the project. +*.tf @team-infra +*.md @alexander-sharov + +For further details, check the official GitHub documentation. + +MAINTAINERS.md +The MAINTAINERS.md file contains a list of project maintainers, their duties, and contact information. Example: + +# MAINTAINERS + +## Maintainers List + +| Name | Role | GitHub Handle | Contact | +|---------------|---------------------|--------------------|---------------------| +| Alice Johnson | Lead Developer | @alice-johnson | alice@example.com | +| Bob Smith | Documentation Lead | @bob-smith | bob@example.com | + +## Responsibilities +- **Lead Developer**: Manages core development and architecture. +- **Documentation Lead**: Ensures project documentation is correct and up-to-date. +SECURITY.md +The SECURITY.md file defines the security policy and vulnerability-handling procedure for your project. Example: + +# SECURITY POLICY + +## Reporting a Vulnerability + +If you discover a security vulnerability, please report it by emailing **security@example.com**. Provide as much detail as possible to help us address the issue promptly. + +## What to Expect +- We will acknowledge receipt of your report within 72 hours. +- A timeline for the investigation and resolution will be communicated. +SUPPORT.md +The SUPPORT.md file instructs users on how to properly report problems or obtain assistance. + +For instance: + +# SUPPORT + +## How to Get Help + +- For general inquiries or usage questions, please open an issue in the repository. +- If you encounter a bug, file a detailed issue with the following information: + - Steps to reproduce the problem + - Expected behavior + - Actual behavior + - Logs or screenshots, if applicable + +## Contact Us + +For urgent or private matters, contact **support@example.com**. We aim to respond within 72 hours. +Continuous integration configuration +Continuous Integration (CI) plays an essential role for open-source repositories since it ensures the reliability, quality, and maintainability of the codebase. When many people make changes, CI pipelines help to ensure consistency and prevent bugs from being merged into the main branch. + +Although there is no single "ideal" CI pipeline, certain key components should always be covered, including: + +Linting +Formatting +Code Validation +Compliance Checks +PR verification automation +All of the mentioned checks can be automated with CI tools like Jenkins, GitLab CI, and GitHub Actions. + +Every time code is pushed or a pull request (PR) is sent, these checks ought to be executed as CI jobs. In addition, since many people are contributing through pull requests, PR validation and main branch validation must be your top priorities. + +To maximize CI pipeline efficiency, implement as many features as possible from the "Code Linting and Quality" section. + +Beyond code validation, CI automation should also cover: + +Documentation updates: Ensure that documentation stays in sync with code changes. +Dependency Management: Handle package updates efficiently. +Versioning & Releases: Automate version control, release management, and change tracking. +Test automation: CI pipeline should include automated checks for all types of tests. These processes align with best practices stated in the "Test Terraform Code" section. +Now let's take a closer look at such automation. + +Setup Versioning +Each module should have a version to manage its usage effectively. For your releases, I advise using semantic versioning (e.g., v1.0.0): + +MAJOR version (1.x.x): Introduces breaking changes. +MINOR version (x.1.x): Adds functionality in a backward-compatible manner. +PATCH version (x.x.1): Fixes bugs or includes improvements without introducing new functionality or breaking backward compatibility. +Ideally, version management should be automated using your CI tool, such as Jenkins, GitLab CI/CD, GitHub Actions, or other similar platforms. In the bulk of my projects, I utilize the GitHub action kvendingoldo/semver-action and other similar platforms, which do the following automatically: + +Set the semantic version as a Git tag for each commit. +Create release branches +Create GitHub releases +This configuration is an excellent starting point, but feel free to experiment with different tools that best suit your workflow. Check such links as: + +Semantic release +Semantic release action +💡 +Note: It's important to use the GitHub Releases features with public modules. If you decide to use a custom versioning process, make sure that you integrate GitHub Releases into your workflow so that your users have clear visibility and accessibility. + +GitHub release produced by kvendingoldo/semver-action. +Automate CHANGELOG updates +A CHANGELOG.md is a file that provides a curated, chronologically ordered list of notable changes for each version of a project. While it can be optional for personal projects, it is mandatory for public projects. + +This file helps users easily track significant changes between releases or versions. + +In the context of Terraform module development, it is important to follow these best practices: + +Maintain a CHANGELOG.md file to document changes across versions. + +For example: + + +# Changelog + +## [1.0.0] - 2025-01-01 +### Added +- Initial release of the Terraform module. + +## [0.1.0] - 2024-12-01 +### Added +- Added basic AWS VPC code. + +### Fixed +- Resolved some typos. +Include changelog updates in pull requests (PRs): Add a note that the PR should include text describing any changes that will be included in the changelog to ensure proper documentation of updates. + +Automate changelog updates to streamline the process and maintain consistency. You can use the following approaches: + +Release Drafter tool +Conventional Changelog tool +Go-changelog from Hashicorp (You can check Cloudflare as an example) +💡 +For additional information on building and maintaining a changelog, visit keep a changelog +Setup Dependency Management +Currently, there are two standard products available for automatic dependency updates that can be used for public Terraform modules. + +Let's look at both of them. + +1. Dependabot +Dependabot is a GitHub-native tool that automatically checks and updates dependencies. You can configure it to manage Terraform modules, GitHub Actions, Python PIP packages, and any other ecosystem. + +Here is a sample configuration for managing Terraform dependencies that checks changes daily and, if found, creates a pull request with a new version: + +# .github/dependabot.yml +version: 2 +updates: + - package-ecosystem: "terraform" + directory: "/" + schedule: + interval: "daily" +Key features of Dependabot: + +Automatically opens pull requests for dependency updates. +Supports a wide range of ecosystems, including Terraform, npm, Python, GitHub Actions, and more. +Includes built-in GitHub security alarms for dependencies' vulnerabilities. +GitHub native +2. Renovate +Renovate is yet another very customisable tool for managing dependencies. It has more flexibility than Dependabot, making it ideal for projects with complicated dependency management requirements. + +Here is an renovate.json sample that does the same function as the DepedaBot configuration: + +{ + "extends": ["config:base"], + "terraform": { + "enabled": true + }, + "schedule": ["daily"] + "automerge": false +} +Key features of Renovate: + +Manages several repositories and ecosystems at once. +Provides granular control over update rules, including grouping updates or setting specific policies. +Automatically updates configuration files, such as Terraform’s versions.tf or .tf.lock.hcl. +Integrates with self-hosted and cloud platforms +Work better with non-GitHub platforms like GitLab CI. +Configure jobs to reduce toil +Maintaining a public project frequently requires you to oversee multiple tasks at once. Any project management task that is performed at least twice ought to be automated to optimize your workflow and time management. + +Here are two useful automations that you may use while developing Terraform modules to improve project efficiency. + +A job that simply closes existing issues and PRs without a new activity. + +name: 'Close stale issues and PRs' +on: + schedule: + - cron: '0 0 * * *' + +jobs: + stale: + runs-on: ubuntu-22.04 + steps: + - uses: actions/stale@v9 + with: + repo-token: ${{ secrets.GITHUB_TOKEN }} + days-before-stale: 30 + stale-issue-label: stale + stale-pr-label: stale + stale-issue-message: | + This issue has been automatically marked as stale because it has been open 30 days + with no activity. Remove stale label or comment or this issue will be closed in 10 days + stale-pr-message: | + This PR has been automatically marked as stale because it has been open 30 days + with no activity. Remove stale label or comment or this PR will be closed in 10 days + # Not stale if have this labels or part of milestone + exempt-issue-labels: bug,wip,on-hold + exempt-pr-labels: bug,wip,on-hold + exempt-all-milestones: true + # Close issue operations + # Label will be automatically removed if the issues are no longer closed nor locked. + days-before-close: 10 + delete-branch: true + close-issue-message: This issue was automatically closed because of stale in 10 days + close-pr-message: This PR was automatically closed because of stale in 10 days +A job that validate PR title + +name: 'Validate PR title' + +on: + pull_request_target: + types: + - opened + - edited + - synchronize + +jobs: + main: + name: Validate PR title + runs-on: ubuntu-22.04 + steps: + - uses: amannn/action-semantic-pull-request@v5.5.3 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + with: + # Configure which types are allowed. + # Default: https://github.com/commitizen/conventional-commit-types + types: | + fix + feat + docs + ci + chore + # Configure that a scope must always be provided. + requireScope: false + # Configure additional validation for the subject based on a regex. + # This example ensures the subject starts with an uppercase character. + subjectPattern: ^[A-Z].+$ + # If `subjectPattern` is configured, you can use this property to override + # the default error message that is shown when the pattern doesn't match. + # The variables `subject` and `title` can be used within the message. + subjectPatternError: | + The subject "{subject}" found in the pull request title "{title}" + didn't match the configured pattern. Please ensure that the subject + starts with an uppercase character. + # For work-in-progress PRs you can typically use draft pull requests + # from Github. However, private repositories on the free plan don't have + # this option and therefore this action allows you to opt-in to using the + # special "[WIP]" prefix to indicate this state. This will avoid the + # validation of the PR title and the pull request checks remain pending. + # Note that a second check will be reported if this is enabled. + wip: true + # When using "Squash and merge" on a PR with only one commit, GitHub + # will suggest using that commit message instead of the PR title for the + # merge commit, and it's easy to commit this by mistake. Enable this option + # to also validate the commit message for one commit PRs. + validateSingleCommit: false +GitHub actions usage +Existing GitHub Actions are important building blocks for creating GitHub pipelines. + +👨‍💻 +My key advice: avoid writing custom actions wherever possible. Instead, explore the GitHub Marketplace to find pre-existing actions that fit your requirements. If a relevant action lacks a specific feature, consider contributing to its development rather than rebuilding the wheel. +Creating custom actions is time-consuming and rarely justified for small to medium-sized organizations. Leveraging existing solutions not only saves effort but also ensures better maintenance and community support. + +Code Linting and Quality +Linting is an essential practice for maintaining high-quality Terraform code. + +It helps ensure consistency, compliance with best practices, and early detection of potential issues. Well-linted code is easier to review, debug, and manage; therefore, it's an important stage in the development lifecycle. + +Let us have a look at linting best practices. + +Make sure no sensitive data has been committed to a repository. Use the tools listed below for that: +Gitleaks or Git-Secret CI jobs +Talisman pre-commit checks +Use Infracost to get an approximate breakdown of the cloud infrastructure costs. +Use linters like tflint or terraform validate to catch syntax errors and enforce Terraform best practices. +Format all code using terraform fmt and enforce this via pre-commit hooks. +To detect compliance and security issues, use the security linters listed below: +Checkov: An open-source static code analysis tool developed by Bridgecrew. It supports multiple IaC frameworks, including Terraform, CloudFormation, Kubernetes, Helm charts, and Dockerfiles. Checkov performs comprehensive scans to detect misconfigurations and policy violations, helping maintain adherence to security best practices. +Terrascan: Developed by Tenable, Terrascan is designed to detect compliance and security violations across various IaC frameworks, including Terraform, CloudFormation, ARM Templates, Kubernetes, Helm, Kustomize, and Dockerfiles. It uses the Open Policy Agent (OPA) for policy as code, allowing for customizable and extensive policy enforcement. +Trivy: Aqua Security's Trivy is a versatile security scanner that can detect vulnerabilities in container images, file systems, and Git repositories. It has Terraform support, which allows you to detect security vulnerabilities in your IaC configurations. Notably, tfsec, previously recommended for Terraform security scanning, has been incorporated into Trivy, combining their capabilities (tfsec#1994). +KICS (Keeping Infrastructure as Code Secure): Checkmarx developed KICS, which identifies security vulnerabilities, compliance issues, and infrastructure misconfigurations early in the development cycle. It supports multiple types of IaC frameworks, allowing you to secure your infrastructure before deployment. +To learn more about static code analysis tools, see "IaC Security Analysis: Checkov vs. tfsec vs. Terrascan – A Comparative Evaluation" and "Comparing Checkov vs. tfsec vs. Terrascan" articles. +Use Open Policy Agent (OPA). It is a policy-as-code engine that enables flexible and automated enforcement of compliance, security, and operational rules within Terraform IAC configurations. + +OPA uses the Rego language to establish custom policies, validate Terraform files, and plan outputs for misconfigurations. + +It works smoothly with CI/CD pipelines to guarantee policy compliance before deployment and includes pre-built policy libraries for typical use cases. OPA contributes to the maintenance of secure and consistent infrastructure standards by identifying concerns early on. +If you use GitLab, then use GitLab IaC Scanning. Infrastructure as code scanning is a built-in feature of the platform, available even on the free plan. It includes a preset CI/CD template that performs helpful static analysis tests on the IaC files in your project. +Use GitHub Super-Linter to maintain consistent code quality across multiple file types, such as YAML, JSON, Markdown, and Terraform. + +If Super-Linter is not an acceptable choice (e.g., you can have self-hosted Jenkins), try using yamllint or jsonlint to effectively validate and format YAML and JSON files. Example of a super-linter GitHub workflow: + +name: Super-Linter +on: [push, pull_request] +jobs: + lint: + runs-on: ubuntu-22.04 + steps: + - uses: actions/checkout@v3 + - uses: github/super-linter@v4 + env: + VALIDATE_TERRAFORM: true + VALIDATE_YAML: true + VALIDATE_JSON: true + VALIDATE_MARKDOWN: true +Developer-environment configuration +Now let's look at some of the key Developer-environment configurations. + +#1: Configure editor +Developers use a wide range of IDEs and text editors, from lightweight alternatives like Vim to powerful, full-featured IDEs like IntelliJ IDEA or VSCode. + +It is highly recommended to add a .editorconfig file in your repository to ensure uniform code formatting and standards across different development environments. + +The .editorconfig file is supported by most modern IDEs and text editors. + +By setting shared code conventions, teams can ensure that developers, regardless of the tools they use, follow the same formatting guidelines. This minimizes issues caused by inconsistent code formatting, streamlines collaboration, and promotes a cleaner codebase. + +Example: + +# EditorConfig is awesome: https://editorconfig.org + +# top-most EditorConfig file +root = true + +[*] +charset = utf-8 +end_of_line = lf +indent_size = 2 +indent_style = space +insert_final_newline = true +max_line_length = 80 +trim_trailing_whitespace = true + +[*.{tf,tfvars}] +indent_size = 2 +indent_style = space + +[*.md] +max_line_length = 0 +trim_trailing_whitespace = false + +[Makefile] +tab_width = 2 +indent_style = tab +[COMMIT_EDITMSG] +max_line_length = 0 +Aside from editorconfig, the second good option is to keep the configuration for VSCode, which will be used by the majority of developers in 2025. + +Example of .vscode/settings.json: + +{ + "editor.formatOnSave": true, + "terraform.format": { + "enable": true + }, + "editor.tabSize": 2 +} +Either is not a recommendation, but it is a good idea to specify all plugins that may be relevant during development. + +Example of .vscode/extensions.json: + +{ + "recommendations": [ + "hashicorp.terraform", + "redhat.vscode-yaml", + "streetsidesoftware.code-spell-checker" + ] +} +#2: Use a pre-commit framework +The pre-commit framework is a powerful tool for automating code quality checks and enforcing standards before changes are committed to your repository. It provides immediate feedback to developers, helping them catch and address issues in their personal workspace before committing the code. + +These hooks check the state of the code before committing and can stop the process if tests fail, ensuring that only high-quality code is pushed to the repository. + +To use it, you can create .pre-commit-config.yaml in the root of your repository, and run it manually before the commit via pre-commit run --all-files. + +The following example contains multiple best practices, which we have already examined in the article: + +# .pre-commit-config.yaml + +repos: + - repo: https://github.com/pre-commit/pre-commit-hooks + rev: v4.5.0 + hooks: + - id: check-yaml + - id: check-json + - id: trailing-whitespace + args: ["--markdown-linebreak-ext=md"] + - id: check-added-large-files + - id: check-executables-have-shebangs + - id: check-shebang-scripts-are-executable + - id: check-merge-conflict + - id: check-vcs-permalinks + - id: detect-private-key + - id: detect-aws-credentials + args: ["--allow-missing-credentials"] + - id: end-of-file-fixer + - id: no-commit-to-branch + - id: pretty-format-json + args: + - --autofix + - repo: https://github.com/zricethezav/gitleaks + rev: v8.18.2 + hooks: + - id: gitleaks + - repo: https://github.com/bridgecrewio/checkov.git + rev: 3.2.43 + hooks: + - id: checkov + args: + - --config-file + - .checkov.yaml + - repo: https://github.com/antonbabenko/pre-commit-terraform + rev: v1.88.3 + hooks: + - id: terraform_fmt + - id: terraform_validate + args: + - --hook-config=--retry-once-with-cleanup=true + - id: terraform_docs + args: + - --args=--config=.terraform-docs.yaml + - id: terraform_tflint + args: + - --args=--config=__GIT_WORKING_DIR__/.tflint.hcl +You may use the AWS Terraform template repository as a starting point for developing your pre-commit configuration. + +It offers a large number of standard checks that you can apply to your project. Furthermore, pay attention to these two repositories that provide common pre-commit hooks for Terraform/OpenTofu: + +Pre-commit hooks for Terraform +Pre-commit hooks for OpenTofu +Terraform-module template pattern +Previously in this post, we explored several guidelines relevant to most Terraform modules. + +However, managing tens or hundreds of modules distributed across multiple repositories makes it practically impossible to keep all configurations, CI processes, and linting rules up to date. Using a copy-pasting method is neither efficient nor sustainable. + +To make a maintenance task easier, use a GitHub/GitLab repository or GitHub fork methodology for your Terraform modules. This repository can contain all of the essential CI pipelines, linters, tools, and code configurations, which are implemented as a common template for your Terraform modules. + +In my projects, I use both of these approaches under the moniker "Terraform Module Template Repository". This repository allows me to constantly keep my configurations up to date and deliver updates rapidly. + +Furthermore, users can quickly build a new module by clicking via "Use this template button" within the template repository. + +Before we go any further, I'd want to discuss the differences between a fork and a template repository, which will be used as a template for future repositories. + +If you use the GitHub template, it may be difficult to transmit updates from the parent repository to the child repository because they are technically independent. + +In the case of forks, the repositories are linked, but I do not suggest backmerging, because you may spend a lot of time at merge conflicts. + +In the scope of both methods, I advocate implementing a custom CI job in the template repository that will distribute updates to children via easy Git and bash commands manually or by CRON. We'll look at an example of such a job in Part 2 of this article series. + +Returning to the "Terraform Module Template Repository" approach, it's important to note that applying it for personal projects or smaller-scale infrastructure may be overkill due to the amount of required work (believe me!). + +The upfront costs of creating and maintaining a reusable, parameterized template may exceed the benefits. If you only have a few repositories, it can be much easier to copy-paste configurations, without implementing an additional automation. + +Finally, if you expect to manage dozens or hundreds of modules, you should create a Terraform Module Template Repository from the ground up at the beginning stage of project development. + +For smaller-scale cases, you can look for open-source alternatives (one of which will be discussed in Part 2 of this series), or just copy and paste your preferred configurations between Terraform modules. + +Conclusion +Thank you for reading! I hope you found this guide helpful. Stay tuned for Part 2, where I will cover: + +A ready-to-use Terraform module template that implements the majority of best practices outlined in this article. +An example of CI jobs that deliver updates from the template repository to their children repositories. +Practical steps to adopt a ready-to-use module template for your organization in 5 minutes +More code samples, as well as tips and advice based on projects that have already used the template repository and other best practices outlined in this article. diff --git a/requirements/terraform-best-practices.md b/requirements/terraform-best-practices.md new file mode 100644 index 0000000000..d8caf73f12 --- /dev/null +++ b/requirements/terraform-best-practices.md @@ -0,0 +1,752 @@ +# Welcome + +[Terraform](https://www.terraform.io) is powerful (if not the most powerful out there now) and one of the most used tools which allow management of infrastructure as code. It allows developers to do a lot of things and does not restrict them from doing things in ways that will be hard to support or integrate with. + +Some information described in this book may not seem like the best practices. I know this, and to help readers to separate what are established best practices and what is just another opinionated way of doing things, I sometimes use hints to provide some context and icons to specify the level of maturity on each subsection related to best practices. + +The book was started in sunny Madrid in 2018, available for free here at [https://www.terraform-best-practices.com/](https://www.terraform-best-practices.com). + +A few years later it has been updated with more actual best practices available with Terraform 1.0. Eventually, this book should contain most of the indisputable best practices and recommendations for Terraform users. + +# Key concepts + +The official Terraform documentation describes [all aspects of configuration in details](https://www.terraform.io/docs/configuration/index.html). Read it carefully to understand the rest of this section. + +This section describes key concepts which are used inside the book. + +## Resource + +Resource is `aws_vpc`, `aws_db_instance`, etc. A resource belongs to a provider, accepts arguments, outputs attributes, and has a lifecycle. A resource can be created, retrieved, updated, and deleted. + +## Resource module + +Resource module is a collection of connected resources which together perform the common action (for e.g., [AWS VPC Terraform module](https://github.com/terraform-aws-modules/terraform-aws-vpc/) creates VPC, subnets, NAT gateway, etc). It depends on provider configuration, which can be defined in it, or in higher-level structures (e.g., in infrastructure module). + +## Infrastructure module + +An infrastructure module is a collection of resource modules, which can be logically not connected, but in the current situation/project/setup serves the same purpose. It defines the configuration for providers, which is passed to the downstream resource modules and to resources. It is normally limited to work in one entity per logical separator (e.g., AWS Region, Google Project). + +For example, [terraform-aws-atlantis](https://github.com/terraform-aws-modules/terraform-aws-atlantis/) module uses resource modules like [terraform-aws-vpc](https://github.com/terraform-aws-modules/terraform-aws-vpc/) and [terraform-aws-security-group](https://github.com/terraform-aws-modules/terraform-aws-security-group/) to manage the infrastructure required for running [Atlantis](https://www.runatlantis.io) on [AWS Fargate](https://aws.amazon.com/fargate/). + +Another example is [terraform-aws-cloudquery](https://github.com/cloudquery/terraform-aws-cloudquery) module where multiple modules by [terraform-aws-modules](https://github.com/terraform-aws-modules/) are being used together to manage the infrastructure as well as using Docker resources to build, push, and deploy Docker images. All in one set. + +## Composition + +Composition is a collection of infrastructure modules, which can span across several logically separated areas (e.g.., AWS Regions, several AWS accounts). Composition is used to describe the complete infrastructure required for the whole organization or project. + +A composition consists of infrastructure modules, which consist of resources modules, which implement individual resources. + +![Simple infrastructure composition](https://118479043-files.gitbook.io/~/files/v0/b/gitbook-x-prod.appspot.com/o/spaces%2Fe1Mp2scOX6OnQbifCen3%2Fuploads%2F6DQkdvTnhqwFfVx2btjq%2Fcomposition-1.png?alt=media) + +## Data source + +Data source performs a read-only operation and is dependant on provider configuration, it is used in a resource module and an infrastructure module. + +Data source `terraform_remote_state` acts as a glue for higher-level modules and compositions. + +The [external](https://registry.terraform.io/providers/hashicorp/external/latest/docs/data-sources/external) data source allows an external program to act as a data source, exposing arbitrary data for use elsewhere in the Terraform configuration. Here is an example from the [terraform-aws-lambda module](https://github.com/terraform-aws-modules/terraform-aws-lambda/blob/258e82b50adc451f51544a2b57fd1f6f8f4a61e4/package.tf#L5-L7) where the filename is computed by calling an external Python script. + +The [http](https://registry.terraform.io/providers/hashicorp/http/latest/docs/data-sources/http) data source makes an HTTP GET request to the given URL and exports information about the response which is often useful to get information from endpoints where a native Terraform provider does not exist. + +## Remote state + +Store [Terraform state](https://www.terraform.io/docs/language/state/index.html) for each infrastructure module and composition in a remote backend, configured with ACLs, versioning, and logging. This single, authoritative source of truth keeps environments consistent and typically includes disaster-recovery features such as automated backups. Managing state locally can lead to collaboration issues and race conditions when multiple developers run Terraform at the same time, resulting in unpredictable outcomes. + +## Provider, provisioner, etc + +Providers, provisioners, and a few other terms are described very well in the official documentation and there is no point to repeat it here. To my opinion, they have little to do with writing good Terraform modules. + +## Why so *difficult*? + +While individual resources are like atoms in the infrastructure, resource modules are molecules (consisting of atoms). A module is the smallest versioned and shareable unit. It has an exact list of arguments, implement basic logic for such a unit to do the required function. e.g., [terraform-aws-security-group](https://github.com/terraform-aws-modules/terraform-aws-security-group) module creates `aws_security_group` and `aws_security_group_rule` resources based on input. This resource module by itself can be used together with other modules to create the infrastructure module. + +Access to data across molecules (resource modules and infrastructure modules) is performed using the modules' outputs and data sources. + +Access between compositions is often performed using remote state data sources. There are [multiple ways to share data between configurations](https://www.terraform.io/docs/language/state/remote-state-data.html#alternative-ways-to-share-data-between-configurations). + +When putting concepts described above in pseudo-relations it may look like this: + +``` +composition-1 { + infrastructure-module-1 { + data-source-1 => d1 + + resource-module-1 { + data-source-2 => d2 + resource-1 (d1, d2) + resource-2 (d2) + } + + resource-module-2 { + data-source-3 => d3 + resource-3 (d1, d3) + resource-4 (d3) + } + } +} +``` + +# Code structure + +Questions related to Terraform code structure are by far the most frequent in the community. Everyone thought about the best code structure for the project at some point also. + +## How should I structure my Terraform configurations? + +This is one of the questions where lots of solutions exist and it is very hard to give universal advice, so let's start with understanding what are we dealing with. + +* What is the complexity of your project? + * Number of related resources + * Number of Terraform providers (see note below about "logical providers") +* How often does your infrastructure change? + * **From** once a month/week/day + * **To** continuously (every time when there is a new commit) +* Code change initiators? *Do you let the CI server update the repository when a new artifact is built?* + * Only developers can push to the infrastructure repository + * Everyone can propose a change to anything by opening a PR (including automated tasks running on the CI server) +* Which deployment platform or deployment service do you use? + * AWS CodeDeploy, Kubernetes, or OpenShift require a slightly different approach +* How environments are grouped? + * By environment, region, project + +{% hint style="info" %} +*Logical providers* work entirely within Terraform's logic and very often don't interact with any other services, so we can think about their complexity as O(1). The most common logical providers include [random](https://registry.terraform.io/providers/hashicorp/random/latest/docs), [local](https://registry.terraform.io/providers/hashicorp/local/latest/docs), [terraform](https://www.terraform.io/docs/providers/terraform/index.html), [null](https://registry.terraform.io/providers/hashicorp/null/latest/docs), [time](https://registry.terraform.io/providers/hashicorp/time/latest). +{% endhint %} + +## Getting started with the structuring of Terraform configurations + +Putting all code in `main.tf` is a good idea when you are getting started or writing an example code. In all other cases you will be better having several files split logically like this: + +* `main.tf` - call modules, locals, and data sources to create all resources +* `variables.tf` - contains declarations of variables used in `main.tf` +* `outputs.tf` - contains outputs from the resources created in `main.tf` +* `versions.tf` - contains version requirements for Terraform and providers + +`terraform.tfvars` should not be used anywhere except [composition](https://www.terraform-best-practices.com/key-concepts#composition). + +## How to think about Terraform configuration structure? + +{% hint style="info" %} +Please make sure that you understand key concepts - [resource module](https://www.terraform-best-practices.com/key-concepts#resource-module), [infrastructure module](https://www.terraform-best-practices.com/key-concepts#infrastructure-module), and [composition](https://www.terraform-best-practices.com/key-concepts#composition), as they are used in the following examples. +{% endhint %} + +### Common recommendations for structuring code + +* It is easier and faster to work with a smaller number of resources + * `terraform plan` and `terraform apply` both make cloud API calls to verify the status of resources + * If you have your entire infrastructure in a single composition this can take some time +* A blast radius (in case of security breach) is smaller with fewer resources + * Insulating unrelated resources from each other by placing them in separate compositions reduces the risk if something goes wrong +* Start your project using remote state because: + * Your laptop is no place for your infrastructure source of truth + * Managing a `tfstate` file in git is a nightmare + * Later when infrastructure layers start to grow in multiple directions (number of dependencies or resources) it will be easier to keep things under control +* Practice a consistent structure and [naming](https://www.terraform-best-practices.com/naming) convention: + * Like procedural code, Terraform code should be written for people to read first, consistency will help when changes happen six months from now + * It is possible to move resources in Terraform state file but it may be harder to do if you have inconsistent structure and naming +* Keep resource modules as plain as possible +* Don't hardcode values that can be passed as variables or discovered using data sources +* Use data sources and `terraform_remote_state` specifically as a glue between infrastructure modules within the composition + +In this book, example projects are grouped by *complexity* - from small to very-large infrastructures. This separation is not strict, so check other structures also. + +### Orchestration of infrastructure modules and compositions + +Having a small infrastructure means that there is a small number of dependencies and few resources. As the project grows the need to chain the execution of Terraform configurations, connecting different infrastructure modules, and passing values within a composition becomes obvious. + +There are at least 5 distinct groups of orchestration solutions that developers use: + +1. Terraform only. Very straightforward, developers have to know only Terraform to get the job done. +2. Terragrunt. Pure orchestration tool which can be used to orchestrate the entire infrastructure as well as handle dependencies. Terragrunt operates with infrastructure modules and compositions natively, so it reduces duplication of code. +3. In-house scripts. Often this happens as a starting point towards orchestration and before discovering Terragrunt. +4. Ansible or similar general purpose automation tool. Usually used when Terraform is adopted after Ansible, or when Ansible UI is actively used. +5. [Crossplane](https://crossplane.io) and other Kubernetes-inspired solutions. Sometimes it makes sense to utilize the Kubernetes ecosystem and employ a reconciliation loop feature to achieve the desired state of your Terraform configurations. View video [Crossplane vs Terraform](https://www.youtube.com/watch?v=ELhVbSdcqSY) for more information. + +With that in mind, this book reviews the first two of these project structures, Terraform only and Terragrunt. + +See examples of code structures for [Terraform](https://www.terraform-best-practices.com/examples/terraform) or [Terragrunt](https://www.terraform-best-practices.com/examples/terragrunt) in the next chapter. + +# Small-size infrastructure with Terraform + +Source: + +This example contains code as an example of structuring Terraform configurations for a small-size infrastructure, where no external dependencies are used. + +{% hint style="success" %} + +* Perfect to get started and refactor as you go +* Perfect for small resource modules +* Good for small and linear infrastructure modules (eg, [terraform-aws-atlantis](https://github.com/terraform-aws-modules/terraform-aws-atlantis)) +* Good for a small number of resources (fewer than 20-30) + {% endhint %} + +{% hint style="warning" %} +Single state file for all resources can make the process of working with Terraform slow if the number of resources is growing (consider using an argument `-target` to limit the number of resources) +{% endhint %} + +# Medium-size infrastructure with Terraform + +Source: + +This example contains code as an example of structuring Terraform configurations for a medium-size infrastructure which uses: + +* 2 AWS accounts +* 2 separate environments (`prod` and `stage` which share nothing). Each environment lives in a separate AWS account +* Each environment uses a different version of the off-the-shelf infrastructure module (`alb`) sourced from [Terraform Registry](https://registry.terraform.io/) +* Each environment uses the same version of an internal module `modules/network` since it is sourced from a local directory. + +{% hint style="success" %} + +* Perfect for projects where infrastructure is logically separated (separate AWS accounts) +* Good when there is no need to modify resources shared between AWS accounts (one environment = one AWS account = one state file) +* Good when there is no need in the orchestration of changes between the environments +* Good when infrastructure resources are different per environment on purpose and can't be generalized (eg, some resources are absent in one environment or in some regions) + {% endhint %} + +{% hint style="warning" %} +As the project grows, it will be harder to keep these environments up-to-date with each other. Consider using infrastructure modules (off-the-shelf or internal) for repeatable tasks. +{% endhint %} + +# Large-size infrastructure with Terraform + +Source: + +This example contains code as an example of structuring Terraform configurations for a large-size infrastructure which uses: + +* 2 AWS accounts +* 2 regions +* 2 separate environments (`prod` and `stage` which share nothing). Each environment lives in a separate AWS account and span resources between 2 regions +* Each environment uses a different version of the off-the-shelf infrastructure module (`alb`) sourced from [Terraform Registry](https://registry.terraform.io/) +* Each environment uses the same version of an internal module `modules/network` since it is sourced from a local directory. + +{% hint style="info" %} +In a large project like described here the benefits of using Terragrunt become very visible. See [Code Structures examples with Terragrunt](https://www.terraform-best-practices.com/examples/terragrunt). +{% endhint %} + +{% hint style="success" %} + +* Perfect for projects where infrastructure is logically separated (separate AWS accounts) +* Good when there is no need to modify resources shared between AWS accounts (one environment = one AWS account = one state file) +* Good when there is no need for the orchestration of changes between the environments +* Good when infrastructure resources are different per environment on purpose and can't be generalized (eg, some resources are absent in one environment or in some regions) + {% endhint %} + +{% hint style="warning" %} +As the project grows, it will be harder to keep these environments up-to-date with each other. Consider using infrastructure modules (off-the-shelf or internal) for repeatable tasks. +{% endhint %} + +# Naming conventions + +## General conventions + +{% 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 if using a version before Terraform `1.9`). 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). Use the same wording as the upstream documentation when applicable. +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. +10. Avoid double negatives: use positive variable names to prevent confusion. For example, use `encryption_enabled` instead of `encryption_disabled`. +11. For variables that should never be `null`, set `nullable = false`. This ensures that passing `null` uses the default value instead of `null`. If `null` is an acceptable value, you can omit nullable or set it to `true`. + +## 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 + * `{name}` for `data "aws_subnet" "private"` is `private` + * `{name}` for `resource "aws_vpc_endpoint_policy" "test"` is `test` + 2. `{type}` is a resource or data source type without a provider prefix + * `{type}` for `data "aws_subnet" "private"` is `subnet` + * `{type}` for `resource "aws_vpc_endpoint_policy" "test"` is `vpc_endpoint_policy` + 3. `{attribute}` is an attribute returned by the output + 4. [See examples](#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](#code-examples-of-output). +4. If the returned value is a list it should have a plural name. [See example](#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 %} + +# Code styling + +{% hint style="info" %} + +* Examples and Terraform modules should contain documentation explaining features and how to use them. +* All links in README.md files should be absolute to make Terraform Registry website show them correctly. +* Documentation may include diagrams created with [mermaid](https://github.com/mermaid-js/mermaid) and blueprints created with [cloudcraft.co](https://cloudcraft.co). +* Use [Terraform pre-commit hooks](https://github.com/antonbabenko/pre-commit-terraform) to make sure that the code is valid, properly formatted, and automatically documented before it is pushed to git and reviewed by humans. + {% endhint %} + +## Formatting + +Terraform’s `terraform fmt` command enforces the canonical style for configuration files. The tool is intentionally opinionated and non-configurable, guaranteeing a uniform format across codebases so reviewers can focus on substance rather than style. Integrate it with [Terraform pre-commit hooks](https://github.com/antonbabenko/pre-commit-terraform) to validate and format code automatically before it reaches version control. + +For example: + +```yaml +# .pre-commit-config.yaml +repos: + - repo: https://github.com/antonbabenko/pre-commit-terraform + rev: v1.99.4 + hooks: + - id: terraform_fmt +``` + +In CI pipelines, use `terraform fmt -check` to verify compliance. It exits with status 0 when all files are correctly formatted; otherwise, it returns a non-zero code and lists the offending files. Centralizing formatting in this way removes merge friction and enforces a consistent standard across teams. + +## Editor Configuration + +* **Use `.editorconfig`**: [EditorConfig](https://editorconfig.org/) helps maintain consistent coding styles for multiple developers working on the same project across various editors and IDEs. Include an `.editorconfig` file in your repositories to maintain consistent whitespace and indentation. + +**Example `.editorconfig`:** + +```editorconfig +[*] +indent_style = space +indent_size = 2 +trim_trailing_whitespace = true + +[*.{tf,tfvars}] +indent_style = space +indent_size = 2 + +[Makefile] +indent_style = tab +``` + +## Documentation + +### Automatically generated documentation + +[pre-commit](https://pre-commit.com/) is a framework for managing and maintaining multi-language pre-commit hooks. It is written in Python and is a powerful tool to do something automatically on a developer's machine before code is committed to a git repository. Normally, it is used to run linters and format code (see [supported hooks](https://pre-commit.com/hooks.html)). + +With Terraform configurations `pre-commit` can be used to format and validate code, as well as to update documentation. + +Check out the [pre-commit-terraform repository](https://github.com/antonbabenko/pre-commit-terraform/blob/master/README.md) to familiarize yourself with it, and existing repositories (eg, [terraform-aws-vpc](https://github.com/terraform-aws-modules/terraform-aws-vpc)) where this is used already. + +### terraform-docs + +[terraform-docs](https://github.com/segmentio/terraform-docs) is a tool that does the generation of documentation from Terraform modules in various output formats. You can run it manually (without pre-commit hooks), or use [pre-commit-terraform hooks](https://github.com/antonbabenko/pre-commit-terraform) to get the documentation updated automatically. + +### Comment style + +Use `#` for comments. Avoid `//` or block comments. + +**Example:** + +```hcl +# This is a comment explaining the resource +resource "aws_instance" "this" { +# ... +} +``` + +**Section Headers**: Delimit section headers in code with `# -----` or `######` for clarity. + +**Example:** + +```hcl +# -------------------------------------------------- +# AWS EC2 Instance Configuration +# -------------------------------------------------- + +resource "aws_instance" "this" { +# ... +} +``` + +@todo: Document module versions, release, GH actions + +## Resources + +1. [pre-commit framework homepage](https://pre-commit.com/) +2. [Collection of git hooks for Terraform to be used with pre-commit framework](https://github.com/antonbabenko/pre-commit-terraform) +3. Blog post by [Dean Wilson](https://github.com/deanwilson): [pre-commit hooks and terraform - a safety net for your repositories](https://www.unixdaemon.net/tools/terraform-precommit-hooks/) + +# FAQ + +## What are the tools I should be aware of and consider using? + +* [**Terragrunt**](https://terragrunt.gruntwork.io/) - Orchestration tool +* [**tflint**](https://github.com/terraform-linters/tflint) - Code linter +* [**tfenv**](https://github.com/tfutils/tfenv) - Version manager +* [**Atmos**](https://atmos.tools/) - A modern composable framework for Terraform backed by YAML +* [**asdf-hashicorp**](https://github.com/asdf-community/asdf-hashicorp) - HashiCorp plugin for the [asdf](https://github.com/asdf-vm/asdf) version manager +* [**Atlantis**](https://www.runatlantis.io/) - Pull Request automation +* [**pre-commit-terraform**](https://github.com/antonbabenko/pre-commit-terraform) - Collection of git hooks for Terraform to be used with [pre-commit framework](https://pre-commit.com/) +* [**Infracost**](https://www.infracost.io) - Cloud cost estimates for Terraform in pull requests. Works with Terragrunt, Atlantis and pre-commit-terraform too. + +## What are the solutions to [dependency hell](https://en.wikipedia.org/wiki/Dependency_hell) with modules? + +Versions of resource and infrastructure modules should be specified. Providers should be configured outside of modules, but only in composition. Version of providers and Terraform can be locked also. + +There is no master dependency management tool, but there are some tips to make dependency specifications less problematic. For example, [Dependabot](https://dependabot.com/) can be used to automate dependency updates. Dependabot creates pull requests to keep your dependencies secure and up-to-date. Dependabot supports Terraform configurations. + +# Writing Terraform configurations + +## Use `locals` to specify explicit dependencies between resources + +Helpful way to give a hint to Terraform that some resources should be deleted before even when there is no direct dependency in Terraform configurations. + + + +## Terraform 0.12 - Required vs Optional arguments + +1. Required argument `index_document` must be set, if `var.website` is not an empty map. +2. Optional argument `error_document` can be omitted. + +{% code title="main.tf" %} + +```hcl +variable "website" { + type = map(string) + default = {} +} + +resource "aws_s3_bucket" "this" { + # omitted... + + dynamic "website" { + for_each = length(keys(var.website)) == 0 ? [] : [var.website] + + content { + index_document = website.value.index_document + error_document = lookup(website.value, "error_document", null) + } + } +} +``` + +{% endcode %} + +{% code title="terraform.tfvars" %} + +```hcl +website = { + index_document = "index.html" +} +``` + +{% endcode %} + +### Optional Object Attributes (Terraform 1.3+) + +Use optional attributes in objects to provide default values for non-required fields: + +{% code title="variables.tf" %} + +```hcl +variable "database_settings" { + description = "Database configuration with optional parameters" + type = object({ + name = string + engine = string + instance_class = string + backup_retention = optional(number, 7) + monitoring_enabled = optional(bool, true) + tags = optional(map(string), {}) + }) +} +``` + +{% endcode %} + +## Managing Secrets in Terraform + +Secrets are sensitive data that can be anything from passwords and encryption keys to API tokens and service certificates. They are typically used to set up authentication and authorization for cloud resources. Safeguarding these sensitive resources is crucial because exposure could lead to security breaches. It’s highly recommended to avoid storing secrets in Terraform config and state, as anyone with access to version control can access them. Instead, consider using external data sources to fetch secrets from external sources at runtime. For instance, if you’re using AWS Secrets Manager, you can use the `aws_secretsmanager_secret_version` data source to access the secret value. The following example uses write-only arguments, which are supported in Terraform 1.11+, and keep the value out of Terraform state. + +{% code title="main.tf" %} + +```hcl +# Fetch the secret’s metadata +data "aws_secretsmanager_secret" "db_password" { + name = "my-database-password" +} + +# Get the latest secret value +data "aws_secretsmanager_secret_version" "db_password" { + secret_id = data.aws_secretsmanager_secret.db_password.id +} + +# Use the secret without persisting it to state +resource "aws_db_instance" "example" { + engine = "mysql" + instance_class = "db.t3.micro" + name = "exampledb" + username = "admin" + + # write-only: Terraform sends it to AWS then forgets it + password_wo = data.aws_secretsmanager_secret_version.db_password.secret_string +``` + +{% endcode %} + +## Variable Validation and Input Handling + +{% hint style="info" %} +Variable validation helps catch errors early, provides clear feedback, and ensures inputs meet your requirements. +{% endhint %} + +### Basic Variable Validation + +Use validation blocks to ensure variables meet specific criteria: + +{% code title="variables.tf" %} + +```hcl +variable "environment" { + description = "Environment name for resource tagging" + type = string + default = "dev" + + validation { + condition = contains(["dev", "staging", "prod"], var.environment) + error_message = "Environment must be one of: dev, staging, prod." + } +} +``` + +{% endcode %} + +### Object and List Validation + +Validate complex data structures to ensure they contain expected values: + +{% code title="variables.tf" %} + +```hcl +variable "database_config" { + description = "Database configuration" + type = object({ + engine = string + instance_class = string + allocated_storage = number + }) + + validation { + condition = contains(["mysql", "postgres"], var.database_config.engine) + error_message = "Database engine must be either 'mysql' or 'postgres'." + } +} + +variable "allowed_cidr_blocks" { + description = "List of CIDR blocks allowed to access resources" + type = list(string) + + validation { + condition = alltrue([ + for cidr in var.allowed_cidr_blocks : can(cidrhost(cidr, 0)) + ]) + error_message = "All CIDR blocks must be valid IPv4 CIDR notation." + } +} +``` + +{% endcode %} From be1616f04e65467d037db93b6a80a64ae7806b7e Mon Sep 17 00:00:00 2001 From: Robert Crandall Date: Thu, 12 Mar 2026 15:20:57 -0700 Subject: [PATCH 02/10] Move skill around --- .../terraform-provider-pr-review/SKILL.md | 4 +- .../references}/composition.mdx | 0 .../references}/index.mdx | 0 .../references}/module-best-practices.md | 0 .../references}/providers.mdx | 0 .../references}/publish.mdx | 0 .../references}/refactoring.mdx | 0 .../references}/structure.mdx | 0 requirements/terraform-best-practices.md | 752 ------------------ 9 files changed, 2 insertions(+), 754 deletions(-) rename {requirements/develop_modules => .github/skills/terraform-provider-pr-review/references}/composition.mdx (100%) rename {requirements/develop_modules => .github/skills/terraform-provider-pr-review/references}/index.mdx (100%) rename {requirements => .github/skills/terraform-provider-pr-review/references}/module-best-practices.md (100%) rename {requirements/develop_modules => .github/skills/terraform-provider-pr-review/references}/providers.mdx (100%) rename {requirements/develop_modules => .github/skills/terraform-provider-pr-review/references}/publish.mdx (100%) rename {requirements/develop_modules => .github/skills/terraform-provider-pr-review/references}/refactoring.mdx (100%) rename {requirements/develop_modules => .github/skills/terraform-provider-pr-review/references}/structure.mdx (100%) delete mode 100644 requirements/terraform-best-practices.md diff --git a/.github/skills/terraform-provider-pr-review/SKILL.md b/.github/skills/terraform-provider-pr-review/SKILL.md index 15a0ec63b9..0dd31745ec 100644 --- a/.github/skills/terraform-provider-pr-review/SKILL.md +++ b/.github/skills/terraform-provider-pr-review/SKILL.md @@ -55,8 +55,8 @@ This section summarizes the Terraform module and provider concepts most relevant - Tests (`*_test.go`, especially acceptance tests) - Docs/site content under `website/` - Example configurations under `examples/` -3. For schema changes, verify backward compatibility using the rules in *Terraform Background*: flag attribute removal/renames, type changes, new `ForceNew`, or `Optional`→`Required` transitions. -4. For example/configuration changes, confirm they follow standard module structure (`main.tf`, `variables.tf`, `outputs.tf`, `README.md`), do not embed `provider` blocks in child modules, and include `description` on variables/outputs. +3. For schema changes, verify backward compatibility using the rules in *Terraform Background*: flag attribute removal/renames, type changes, new `ForceNew`, or `Optional`→`Required` transitions. If a PR restructures resources or renames modules, consult [Refactoring Modules](./references/refactoring.mdx) for `moved` block requirements. +4. For example/configuration changes, confirm they follow [Standard Module Structure](./references/structure.mdx) (`main.tf`, `variables.tf`, `outputs.tf`, `README.md`), do not embed `provider` blocks in child modules (see [Providers Within Modules](./references/providers.mdx)), and include `description` on variables/outputs. 5. Review against the checklist in [Terraform Provider Review Checklist](./references/review-checklist.md). 6. Prioritize findings by severity and provide actionable fixes. 7. Report residual risk and testing gaps when uncertain. diff --git a/requirements/develop_modules/composition.mdx b/.github/skills/terraform-provider-pr-review/references/composition.mdx similarity index 100% rename from requirements/develop_modules/composition.mdx rename to .github/skills/terraform-provider-pr-review/references/composition.mdx diff --git a/requirements/develop_modules/index.mdx b/.github/skills/terraform-provider-pr-review/references/index.mdx similarity index 100% rename from requirements/develop_modules/index.mdx rename to .github/skills/terraform-provider-pr-review/references/index.mdx diff --git a/requirements/module-best-practices.md b/.github/skills/terraform-provider-pr-review/references/module-best-practices.md similarity index 100% rename from requirements/module-best-practices.md rename to .github/skills/terraform-provider-pr-review/references/module-best-practices.md diff --git a/requirements/develop_modules/providers.mdx b/.github/skills/terraform-provider-pr-review/references/providers.mdx similarity index 100% rename from requirements/develop_modules/providers.mdx rename to .github/skills/terraform-provider-pr-review/references/providers.mdx diff --git a/requirements/develop_modules/publish.mdx b/.github/skills/terraform-provider-pr-review/references/publish.mdx similarity index 100% rename from requirements/develop_modules/publish.mdx rename to .github/skills/terraform-provider-pr-review/references/publish.mdx diff --git a/requirements/develop_modules/refactoring.mdx b/.github/skills/terraform-provider-pr-review/references/refactoring.mdx similarity index 100% rename from requirements/develop_modules/refactoring.mdx rename to .github/skills/terraform-provider-pr-review/references/refactoring.mdx diff --git a/requirements/develop_modules/structure.mdx b/.github/skills/terraform-provider-pr-review/references/structure.mdx similarity index 100% rename from requirements/develop_modules/structure.mdx rename to .github/skills/terraform-provider-pr-review/references/structure.mdx diff --git a/requirements/terraform-best-practices.md b/requirements/terraform-best-practices.md deleted file mode 100644 index d8caf73f12..0000000000 --- a/requirements/terraform-best-practices.md +++ /dev/null @@ -1,752 +0,0 @@ -# Welcome - -[Terraform](https://www.terraform.io) is powerful (if not the most powerful out there now) and one of the most used tools which allow management of infrastructure as code. It allows developers to do a lot of things and does not restrict them from doing things in ways that will be hard to support or integrate with. - -Some information described in this book may not seem like the best practices. I know this, and to help readers to separate what are established best practices and what is just another opinionated way of doing things, I sometimes use hints to provide some context and icons to specify the level of maturity on each subsection related to best practices. - -The book was started in sunny Madrid in 2018, available for free here at [https://www.terraform-best-practices.com/](https://www.terraform-best-practices.com). - -A few years later it has been updated with more actual best practices available with Terraform 1.0. Eventually, this book should contain most of the indisputable best practices and recommendations for Terraform users. - -# Key concepts - -The official Terraform documentation describes [all aspects of configuration in details](https://www.terraform.io/docs/configuration/index.html). Read it carefully to understand the rest of this section. - -This section describes key concepts which are used inside the book. - -## Resource - -Resource is `aws_vpc`, `aws_db_instance`, etc. A resource belongs to a provider, accepts arguments, outputs attributes, and has a lifecycle. A resource can be created, retrieved, updated, and deleted. - -## Resource module - -Resource module is a collection of connected resources which together perform the common action (for e.g., [AWS VPC Terraform module](https://github.com/terraform-aws-modules/terraform-aws-vpc/) creates VPC, subnets, NAT gateway, etc). It depends on provider configuration, which can be defined in it, or in higher-level structures (e.g., in infrastructure module). - -## Infrastructure module - -An infrastructure module is a collection of resource modules, which can be logically not connected, but in the current situation/project/setup serves the same purpose. It defines the configuration for providers, which is passed to the downstream resource modules and to resources. It is normally limited to work in one entity per logical separator (e.g., AWS Region, Google Project). - -For example, [terraform-aws-atlantis](https://github.com/terraform-aws-modules/terraform-aws-atlantis/) module uses resource modules like [terraform-aws-vpc](https://github.com/terraform-aws-modules/terraform-aws-vpc/) and [terraform-aws-security-group](https://github.com/terraform-aws-modules/terraform-aws-security-group/) to manage the infrastructure required for running [Atlantis](https://www.runatlantis.io) on [AWS Fargate](https://aws.amazon.com/fargate/). - -Another example is [terraform-aws-cloudquery](https://github.com/cloudquery/terraform-aws-cloudquery) module where multiple modules by [terraform-aws-modules](https://github.com/terraform-aws-modules/) are being used together to manage the infrastructure as well as using Docker resources to build, push, and deploy Docker images. All in one set. - -## Composition - -Composition is a collection of infrastructure modules, which can span across several logically separated areas (e.g.., AWS Regions, several AWS accounts). Composition is used to describe the complete infrastructure required for the whole organization or project. - -A composition consists of infrastructure modules, which consist of resources modules, which implement individual resources. - -![Simple infrastructure composition](https://118479043-files.gitbook.io/~/files/v0/b/gitbook-x-prod.appspot.com/o/spaces%2Fe1Mp2scOX6OnQbifCen3%2Fuploads%2F6DQkdvTnhqwFfVx2btjq%2Fcomposition-1.png?alt=media) - -## Data source - -Data source performs a read-only operation and is dependant on provider configuration, it is used in a resource module and an infrastructure module. - -Data source `terraform_remote_state` acts as a glue for higher-level modules and compositions. - -The [external](https://registry.terraform.io/providers/hashicorp/external/latest/docs/data-sources/external) data source allows an external program to act as a data source, exposing arbitrary data for use elsewhere in the Terraform configuration. Here is an example from the [terraform-aws-lambda module](https://github.com/terraform-aws-modules/terraform-aws-lambda/blob/258e82b50adc451f51544a2b57fd1f6f8f4a61e4/package.tf#L5-L7) where the filename is computed by calling an external Python script. - -The [http](https://registry.terraform.io/providers/hashicorp/http/latest/docs/data-sources/http) data source makes an HTTP GET request to the given URL and exports information about the response which is often useful to get information from endpoints where a native Terraform provider does not exist. - -## Remote state - -Store [Terraform state](https://www.terraform.io/docs/language/state/index.html) for each infrastructure module and composition in a remote backend, configured with ACLs, versioning, and logging. This single, authoritative source of truth keeps environments consistent and typically includes disaster-recovery features such as automated backups. Managing state locally can lead to collaboration issues and race conditions when multiple developers run Terraform at the same time, resulting in unpredictable outcomes. - -## Provider, provisioner, etc - -Providers, provisioners, and a few other terms are described very well in the official documentation and there is no point to repeat it here. To my opinion, they have little to do with writing good Terraform modules. - -## Why so *difficult*? - -While individual resources are like atoms in the infrastructure, resource modules are molecules (consisting of atoms). A module is the smallest versioned and shareable unit. It has an exact list of arguments, implement basic logic for such a unit to do the required function. e.g., [terraform-aws-security-group](https://github.com/terraform-aws-modules/terraform-aws-security-group) module creates `aws_security_group` and `aws_security_group_rule` resources based on input. This resource module by itself can be used together with other modules to create the infrastructure module. - -Access to data across molecules (resource modules and infrastructure modules) is performed using the modules' outputs and data sources. - -Access between compositions is often performed using remote state data sources. There are [multiple ways to share data between configurations](https://www.terraform.io/docs/language/state/remote-state-data.html#alternative-ways-to-share-data-between-configurations). - -When putting concepts described above in pseudo-relations it may look like this: - -``` -composition-1 { - infrastructure-module-1 { - data-source-1 => d1 - - resource-module-1 { - data-source-2 => d2 - resource-1 (d1, d2) - resource-2 (d2) - } - - resource-module-2 { - data-source-3 => d3 - resource-3 (d1, d3) - resource-4 (d3) - } - } -} -``` - -# Code structure - -Questions related to Terraform code structure are by far the most frequent in the community. Everyone thought about the best code structure for the project at some point also. - -## How should I structure my Terraform configurations? - -This is one of the questions where lots of solutions exist and it is very hard to give universal advice, so let's start with understanding what are we dealing with. - -* What is the complexity of your project? - * Number of related resources - * Number of Terraform providers (see note below about "logical providers") -* How often does your infrastructure change? - * **From** once a month/week/day - * **To** continuously (every time when there is a new commit) -* Code change initiators? *Do you let the CI server update the repository when a new artifact is built?* - * Only developers can push to the infrastructure repository - * Everyone can propose a change to anything by opening a PR (including automated tasks running on the CI server) -* Which deployment platform or deployment service do you use? - * AWS CodeDeploy, Kubernetes, or OpenShift require a slightly different approach -* How environments are grouped? - * By environment, region, project - -{% hint style="info" %} -*Logical providers* work entirely within Terraform's logic and very often don't interact with any other services, so we can think about their complexity as O(1). The most common logical providers include [random](https://registry.terraform.io/providers/hashicorp/random/latest/docs), [local](https://registry.terraform.io/providers/hashicorp/local/latest/docs), [terraform](https://www.terraform.io/docs/providers/terraform/index.html), [null](https://registry.terraform.io/providers/hashicorp/null/latest/docs), [time](https://registry.terraform.io/providers/hashicorp/time/latest). -{% endhint %} - -## Getting started with the structuring of Terraform configurations - -Putting all code in `main.tf` is a good idea when you are getting started or writing an example code. In all other cases you will be better having several files split logically like this: - -* `main.tf` - call modules, locals, and data sources to create all resources -* `variables.tf` - contains declarations of variables used in `main.tf` -* `outputs.tf` - contains outputs from the resources created in `main.tf` -* `versions.tf` - contains version requirements for Terraform and providers - -`terraform.tfvars` should not be used anywhere except [composition](https://www.terraform-best-practices.com/key-concepts#composition). - -## How to think about Terraform configuration structure? - -{% hint style="info" %} -Please make sure that you understand key concepts - [resource module](https://www.terraform-best-practices.com/key-concepts#resource-module), [infrastructure module](https://www.terraform-best-practices.com/key-concepts#infrastructure-module), and [composition](https://www.terraform-best-practices.com/key-concepts#composition), as they are used in the following examples. -{% endhint %} - -### Common recommendations for structuring code - -* It is easier and faster to work with a smaller number of resources - * `terraform plan` and `terraform apply` both make cloud API calls to verify the status of resources - * If you have your entire infrastructure in a single composition this can take some time -* A blast radius (in case of security breach) is smaller with fewer resources - * Insulating unrelated resources from each other by placing them in separate compositions reduces the risk if something goes wrong -* Start your project using remote state because: - * Your laptop is no place for your infrastructure source of truth - * Managing a `tfstate` file in git is a nightmare - * Later when infrastructure layers start to grow in multiple directions (number of dependencies or resources) it will be easier to keep things under control -* Practice a consistent structure and [naming](https://www.terraform-best-practices.com/naming) convention: - * Like procedural code, Terraform code should be written for people to read first, consistency will help when changes happen six months from now - * It is possible to move resources in Terraform state file but it may be harder to do if you have inconsistent structure and naming -* Keep resource modules as plain as possible -* Don't hardcode values that can be passed as variables or discovered using data sources -* Use data sources and `terraform_remote_state` specifically as a glue between infrastructure modules within the composition - -In this book, example projects are grouped by *complexity* - from small to very-large infrastructures. This separation is not strict, so check other structures also. - -### Orchestration of infrastructure modules and compositions - -Having a small infrastructure means that there is a small number of dependencies and few resources. As the project grows the need to chain the execution of Terraform configurations, connecting different infrastructure modules, and passing values within a composition becomes obvious. - -There are at least 5 distinct groups of orchestration solutions that developers use: - -1. Terraform only. Very straightforward, developers have to know only Terraform to get the job done. -2. Terragrunt. Pure orchestration tool which can be used to orchestrate the entire infrastructure as well as handle dependencies. Terragrunt operates with infrastructure modules and compositions natively, so it reduces duplication of code. -3. In-house scripts. Often this happens as a starting point towards orchestration and before discovering Terragrunt. -4. Ansible or similar general purpose automation tool. Usually used when Terraform is adopted after Ansible, or when Ansible UI is actively used. -5. [Crossplane](https://crossplane.io) and other Kubernetes-inspired solutions. Sometimes it makes sense to utilize the Kubernetes ecosystem and employ a reconciliation loop feature to achieve the desired state of your Terraform configurations. View video [Crossplane vs Terraform](https://www.youtube.com/watch?v=ELhVbSdcqSY) for more information. - -With that in mind, this book reviews the first two of these project structures, Terraform only and Terragrunt. - -See examples of code structures for [Terraform](https://www.terraform-best-practices.com/examples/terraform) or [Terragrunt](https://www.terraform-best-practices.com/examples/terragrunt) in the next chapter. - -# Small-size infrastructure with Terraform - -Source: - -This example contains code as an example of structuring Terraform configurations for a small-size infrastructure, where no external dependencies are used. - -{% hint style="success" %} - -* Perfect to get started and refactor as you go -* Perfect for small resource modules -* Good for small and linear infrastructure modules (eg, [terraform-aws-atlantis](https://github.com/terraform-aws-modules/terraform-aws-atlantis)) -* Good for a small number of resources (fewer than 20-30) - {% endhint %} - -{% hint style="warning" %} -Single state file for all resources can make the process of working with Terraform slow if the number of resources is growing (consider using an argument `-target` to limit the number of resources) -{% endhint %} - -# Medium-size infrastructure with Terraform - -Source: - -This example contains code as an example of structuring Terraform configurations for a medium-size infrastructure which uses: - -* 2 AWS accounts -* 2 separate environments (`prod` and `stage` which share nothing). Each environment lives in a separate AWS account -* Each environment uses a different version of the off-the-shelf infrastructure module (`alb`) sourced from [Terraform Registry](https://registry.terraform.io/) -* Each environment uses the same version of an internal module `modules/network` since it is sourced from a local directory. - -{% hint style="success" %} - -* Perfect for projects where infrastructure is logically separated (separate AWS accounts) -* Good when there is no need to modify resources shared between AWS accounts (one environment = one AWS account = one state file) -* Good when there is no need in the orchestration of changes between the environments -* Good when infrastructure resources are different per environment on purpose and can't be generalized (eg, some resources are absent in one environment or in some regions) - {% endhint %} - -{% hint style="warning" %} -As the project grows, it will be harder to keep these environments up-to-date with each other. Consider using infrastructure modules (off-the-shelf or internal) for repeatable tasks. -{% endhint %} - -# Large-size infrastructure with Terraform - -Source: - -This example contains code as an example of structuring Terraform configurations for a large-size infrastructure which uses: - -* 2 AWS accounts -* 2 regions -* 2 separate environments (`prod` and `stage` which share nothing). Each environment lives in a separate AWS account and span resources between 2 regions -* Each environment uses a different version of the off-the-shelf infrastructure module (`alb`) sourced from [Terraform Registry](https://registry.terraform.io/) -* Each environment uses the same version of an internal module `modules/network` since it is sourced from a local directory. - -{% hint style="info" %} -In a large project like described here the benefits of using Terragrunt become very visible. See [Code Structures examples with Terragrunt](https://www.terraform-best-practices.com/examples/terragrunt). -{% endhint %} - -{% hint style="success" %} - -* Perfect for projects where infrastructure is logically separated (separate AWS accounts) -* Good when there is no need to modify resources shared between AWS accounts (one environment = one AWS account = one state file) -* Good when there is no need for the orchestration of changes between the environments -* Good when infrastructure resources are different per environment on purpose and can't be generalized (eg, some resources are absent in one environment or in some regions) - {% endhint %} - -{% hint style="warning" %} -As the project grows, it will be harder to keep these environments up-to-date with each other. Consider using infrastructure modules (off-the-shelf or internal) for repeatable tasks. -{% endhint %} - -# Naming conventions - -## General conventions - -{% 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 if using a version before Terraform `1.9`). 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). Use the same wording as the upstream documentation when applicable. -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. -10. Avoid double negatives: use positive variable names to prevent confusion. For example, use `encryption_enabled` instead of `encryption_disabled`. -11. For variables that should never be `null`, set `nullable = false`. This ensures that passing `null` uses the default value instead of `null`. If `null` is an acceptable value, you can omit nullable or set it to `true`. - -## 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 - * `{name}` for `data "aws_subnet" "private"` is `private` - * `{name}` for `resource "aws_vpc_endpoint_policy" "test"` is `test` - 2. `{type}` is a resource or data source type without a provider prefix - * `{type}` for `data "aws_subnet" "private"` is `subnet` - * `{type}` for `resource "aws_vpc_endpoint_policy" "test"` is `vpc_endpoint_policy` - 3. `{attribute}` is an attribute returned by the output - 4. [See examples](#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](#code-examples-of-output). -4. If the returned value is a list it should have a plural name. [See example](#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 %} - -# Code styling - -{% hint style="info" %} - -* Examples and Terraform modules should contain documentation explaining features and how to use them. -* All links in README.md files should be absolute to make Terraform Registry website show them correctly. -* Documentation may include diagrams created with [mermaid](https://github.com/mermaid-js/mermaid) and blueprints created with [cloudcraft.co](https://cloudcraft.co). -* Use [Terraform pre-commit hooks](https://github.com/antonbabenko/pre-commit-terraform) to make sure that the code is valid, properly formatted, and automatically documented before it is pushed to git and reviewed by humans. - {% endhint %} - -## Formatting - -Terraform’s `terraform fmt` command enforces the canonical style for configuration files. The tool is intentionally opinionated and non-configurable, guaranteeing a uniform format across codebases so reviewers can focus on substance rather than style. Integrate it with [Terraform pre-commit hooks](https://github.com/antonbabenko/pre-commit-terraform) to validate and format code automatically before it reaches version control. - -For example: - -```yaml -# .pre-commit-config.yaml -repos: - - repo: https://github.com/antonbabenko/pre-commit-terraform - rev: v1.99.4 - hooks: - - id: terraform_fmt -``` - -In CI pipelines, use `terraform fmt -check` to verify compliance. It exits with status 0 when all files are correctly formatted; otherwise, it returns a non-zero code and lists the offending files. Centralizing formatting in this way removes merge friction and enforces a consistent standard across teams. - -## Editor Configuration - -* **Use `.editorconfig`**: [EditorConfig](https://editorconfig.org/) helps maintain consistent coding styles for multiple developers working on the same project across various editors and IDEs. Include an `.editorconfig` file in your repositories to maintain consistent whitespace and indentation. - -**Example `.editorconfig`:** - -```editorconfig -[*] -indent_style = space -indent_size = 2 -trim_trailing_whitespace = true - -[*.{tf,tfvars}] -indent_style = space -indent_size = 2 - -[Makefile] -indent_style = tab -``` - -## Documentation - -### Automatically generated documentation - -[pre-commit](https://pre-commit.com/) is a framework for managing and maintaining multi-language pre-commit hooks. It is written in Python and is a powerful tool to do something automatically on a developer's machine before code is committed to a git repository. Normally, it is used to run linters and format code (see [supported hooks](https://pre-commit.com/hooks.html)). - -With Terraform configurations `pre-commit` can be used to format and validate code, as well as to update documentation. - -Check out the [pre-commit-terraform repository](https://github.com/antonbabenko/pre-commit-terraform/blob/master/README.md) to familiarize yourself with it, and existing repositories (eg, [terraform-aws-vpc](https://github.com/terraform-aws-modules/terraform-aws-vpc)) where this is used already. - -### terraform-docs - -[terraform-docs](https://github.com/segmentio/terraform-docs) is a tool that does the generation of documentation from Terraform modules in various output formats. You can run it manually (without pre-commit hooks), or use [pre-commit-terraform hooks](https://github.com/antonbabenko/pre-commit-terraform) to get the documentation updated automatically. - -### Comment style - -Use `#` for comments. Avoid `//` or block comments. - -**Example:** - -```hcl -# This is a comment explaining the resource -resource "aws_instance" "this" { -# ... -} -``` - -**Section Headers**: Delimit section headers in code with `# -----` or `######` for clarity. - -**Example:** - -```hcl -# -------------------------------------------------- -# AWS EC2 Instance Configuration -# -------------------------------------------------- - -resource "aws_instance" "this" { -# ... -} -``` - -@todo: Document module versions, release, GH actions - -## Resources - -1. [pre-commit framework homepage](https://pre-commit.com/) -2. [Collection of git hooks for Terraform to be used with pre-commit framework](https://github.com/antonbabenko/pre-commit-terraform) -3. Blog post by [Dean Wilson](https://github.com/deanwilson): [pre-commit hooks and terraform - a safety net for your repositories](https://www.unixdaemon.net/tools/terraform-precommit-hooks/) - -# FAQ - -## What are the tools I should be aware of and consider using? - -* [**Terragrunt**](https://terragrunt.gruntwork.io/) - Orchestration tool -* [**tflint**](https://github.com/terraform-linters/tflint) - Code linter -* [**tfenv**](https://github.com/tfutils/tfenv) - Version manager -* [**Atmos**](https://atmos.tools/) - A modern composable framework for Terraform backed by YAML -* [**asdf-hashicorp**](https://github.com/asdf-community/asdf-hashicorp) - HashiCorp plugin for the [asdf](https://github.com/asdf-vm/asdf) version manager -* [**Atlantis**](https://www.runatlantis.io/) - Pull Request automation -* [**pre-commit-terraform**](https://github.com/antonbabenko/pre-commit-terraform) - Collection of git hooks for Terraform to be used with [pre-commit framework](https://pre-commit.com/) -* [**Infracost**](https://www.infracost.io) - Cloud cost estimates for Terraform in pull requests. Works with Terragrunt, Atlantis and pre-commit-terraform too. - -## What are the solutions to [dependency hell](https://en.wikipedia.org/wiki/Dependency_hell) with modules? - -Versions of resource and infrastructure modules should be specified. Providers should be configured outside of modules, but only in composition. Version of providers and Terraform can be locked also. - -There is no master dependency management tool, but there are some tips to make dependency specifications less problematic. For example, [Dependabot](https://dependabot.com/) can be used to automate dependency updates. Dependabot creates pull requests to keep your dependencies secure and up-to-date. Dependabot supports Terraform configurations. - -# Writing Terraform configurations - -## Use `locals` to specify explicit dependencies between resources - -Helpful way to give a hint to Terraform that some resources should be deleted before even when there is no direct dependency in Terraform configurations. - - - -## Terraform 0.12 - Required vs Optional arguments - -1. Required argument `index_document` must be set, if `var.website` is not an empty map. -2. Optional argument `error_document` can be omitted. - -{% code title="main.tf" %} - -```hcl -variable "website" { - type = map(string) - default = {} -} - -resource "aws_s3_bucket" "this" { - # omitted... - - dynamic "website" { - for_each = length(keys(var.website)) == 0 ? [] : [var.website] - - content { - index_document = website.value.index_document - error_document = lookup(website.value, "error_document", null) - } - } -} -``` - -{% endcode %} - -{% code title="terraform.tfvars" %} - -```hcl -website = { - index_document = "index.html" -} -``` - -{% endcode %} - -### Optional Object Attributes (Terraform 1.3+) - -Use optional attributes in objects to provide default values for non-required fields: - -{% code title="variables.tf" %} - -```hcl -variable "database_settings" { - description = "Database configuration with optional parameters" - type = object({ - name = string - engine = string - instance_class = string - backup_retention = optional(number, 7) - monitoring_enabled = optional(bool, true) - tags = optional(map(string), {}) - }) -} -``` - -{% endcode %} - -## Managing Secrets in Terraform - -Secrets are sensitive data that can be anything from passwords and encryption keys to API tokens and service certificates. They are typically used to set up authentication and authorization for cloud resources. Safeguarding these sensitive resources is crucial because exposure could lead to security breaches. It’s highly recommended to avoid storing secrets in Terraform config and state, as anyone with access to version control can access them. Instead, consider using external data sources to fetch secrets from external sources at runtime. For instance, if you’re using AWS Secrets Manager, you can use the `aws_secretsmanager_secret_version` data source to access the secret value. The following example uses write-only arguments, which are supported in Terraform 1.11+, and keep the value out of Terraform state. - -{% code title="main.tf" %} - -```hcl -# Fetch the secret’s metadata -data "aws_secretsmanager_secret" "db_password" { - name = "my-database-password" -} - -# Get the latest secret value -data "aws_secretsmanager_secret_version" "db_password" { - secret_id = data.aws_secretsmanager_secret.db_password.id -} - -# Use the secret without persisting it to state -resource "aws_db_instance" "example" { - engine = "mysql" - instance_class = "db.t3.micro" - name = "exampledb" - username = "admin" - - # write-only: Terraform sends it to AWS then forgets it - password_wo = data.aws_secretsmanager_secret_version.db_password.secret_string -``` - -{% endcode %} - -## Variable Validation and Input Handling - -{% hint style="info" %} -Variable validation helps catch errors early, provides clear feedback, and ensures inputs meet your requirements. -{% endhint %} - -### Basic Variable Validation - -Use validation blocks to ensure variables meet specific criteria: - -{% code title="variables.tf" %} - -```hcl -variable "environment" { - description = "Environment name for resource tagging" - type = string - default = "dev" - - validation { - condition = contains(["dev", "staging", "prod"], var.environment) - error_message = "Environment must be one of: dev, staging, prod." - } -} -``` - -{% endcode %} - -### Object and List Validation - -Validate complex data structures to ensure they contain expected values: - -{% code title="variables.tf" %} - -```hcl -variable "database_config" { - description = "Database configuration" - type = object({ - engine = string - instance_class = string - allocated_storage = number - }) - - validation { - condition = contains(["mysql", "postgres"], var.database_config.engine) - error_message = "Database engine must be either 'mysql' or 'postgres'." - } -} - -variable "allowed_cidr_blocks" { - description = "List of CIDR blocks allowed to access resources" - type = list(string) - - validation { - condition = alltrue([ - for cidr in var.allowed_cidr_blocks : can(cidrhost(cidr, 0)) - ]) - error_message = "All CIDR blocks must be valid IPv4 CIDR notation." - } -} -``` - -{% endcode %} From 20ca9715d9ee5ed5e0cb746bc5908978b72ba603 Mon Sep 17 00:00:00 2001 From: Robert Crandall Date: Tue, 17 Mar 2026 14:55:41 -0700 Subject: [PATCH 03/10] Prevent read after update --- .github/skills/terraform-provider-pr-review/SKILL.md | 1 + .../references/review-checklist.md | 3 +++ 2 files changed, 4 insertions(+) diff --git a/.github/skills/terraform-provider-pr-review/SKILL.md b/.github/skills/terraform-provider-pr-review/SKILL.md index 0dd31745ec..beb9d23af6 100644 --- a/.github/skills/terraform-provider-pr-review/SKILL.md +++ b/.github/skills/terraform-provider-pr-review/SKILL.md @@ -84,5 +84,6 @@ If no issues are found, explicitly state `No blocking findings found` and list r - Prefer matching resource/data source tests when implementation behavior changes. - When schema or state semantics change, treat as high-risk unless compatibility is clearly preserved. - Breaking changes must follow semantic versioning: attribute removal/rename or new required fields warrant a major version bump. +- Create/update functions in this provider intentionally do **not** call the read function afterward. This reduces API call volume against GitHub's rate limits and avoids stale reads from eventually-consistent endpoints (see [#2892](https://github.com/integrations/terraform-provider-github/issues/2892)). Do not flag this as an issue. - Example configurations under `examples/` should be self-contained, follow standard module structure, and not include `provider` blocks inside nested modules. - Sensitive attributes (tokens, secrets, private keys) must be marked `Sensitive: true` in the schema and must not appear in log output. diff --git a/.github/skills/terraform-provider-pr-review/references/review-checklist.md b/.github/skills/terraform-provider-pr-review/references/review-checklist.md index 75e241ebe2..efa19e4421 100644 --- a/.github/skills/terraform-provider-pr-review/references/review-checklist.md +++ b/.github/skills/terraform-provider-pr-review/references/review-checklist.md @@ -2,12 +2,15 @@ Use this checklist when reviewing PRs in `terraform-provider-github`. +ALWAYS acknowledge you are using this checklist when reviewing a PR! + ## 1. Correctness and Behavior - Verify CRUD/read logic correctly maps GitHub API responses to Terraform schema/state. - Check for nil handling, default-value drift, and state flattening/expansion mismatches. - Confirm update paths do not accidentally force replacement or wipe optional fields. - Validate retry/backoff and error classification for API failures. +- **Do not flag** create/update functions that return `nil` instead of calling the read function. This provider intentionally avoids read-after-write to minimize API calls against GitHub's rate limits and to avoid stale reads from eventually-consistent endpoints (see [#2892](https://github.com/integrations/terraform-provider-github/issues/2892)). ## 2. Schema and State Compatibility From 03e857f3283d8c259f063fd02eba78aaa8d2b949 Mon Sep 17 00:00:00 2001 From: Robert Crandall Date: Wed, 13 May 2026 11:51:06 -0700 Subject: [PATCH 04/10] Make review compatible with CCR --- .github/copilot-instructions.md | 160 +++ .github/instructions/docs.instructions.md | 42 + .github/instructions/examples.instructions.md | 56 ++ .../schema-and-state.instructions.md | 74 ++ .github/instructions/tests.instructions.md | 36 + .../terraform-provider-pr-review/SKILL.md | 89 -- .../references/composition.mdx | 380 -------- .../references/index.mdx | 89 -- .../references/module-best-practices.md | 908 ------------------ .../references/providers.mdx | 367 ------- .../references/publish.mdx | 45 - .../references/refactoring.mdx | 448 --------- .../references/review-checklist.md | 77 -- .../references/structure.mdx | 133 --- 14 files changed, 368 insertions(+), 2536 deletions(-) create mode 100644 .github/copilot-instructions.md create mode 100644 .github/instructions/docs.instructions.md create mode 100644 .github/instructions/examples.instructions.md create mode 100644 .github/instructions/schema-and-state.instructions.md create mode 100644 .github/instructions/tests.instructions.md delete mode 100644 .github/skills/terraform-provider-pr-review/SKILL.md delete mode 100644 .github/skills/terraform-provider-pr-review/references/composition.mdx delete mode 100644 .github/skills/terraform-provider-pr-review/references/index.mdx delete mode 100644 .github/skills/terraform-provider-pr-review/references/module-best-practices.md delete mode 100644 .github/skills/terraform-provider-pr-review/references/providers.mdx delete mode 100644 .github/skills/terraform-provider-pr-review/references/publish.mdx delete mode 100644 .github/skills/terraform-provider-pr-review/references/refactoring.mdx delete mode 100644 .github/skills/terraform-provider-pr-review/references/review-checklist.md delete mode 100644 .github/skills/terraform-provider-pr-review/references/structure.mdx diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 0000000000..a5db108d14 --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,160 @@ +# Copilot Code Review Instructions + +These instructions guide Copilot Code Review (CCR) for the +`integrations/terraform-provider-github` repository. They apply to every +pull request review. Path-specific guidance lives under `.github/instructions/`. + +ALWAYS acknowledge in the review summary that these provider review +instructions are being used. + +## Review Goals + +- Find correctness bugs, regressions, and provider behavior changes. +- Validate schema/state compatibility for Terraform users. +- Check test coverage (unit and acceptance), docs, and examples. +- Identify risk around GitHub API usage, permissions, and error handling. + +## Terraform Background + +Use this background when judging schema, examples, or state changes. + +- **Resources vs. data sources.** A `resource` block manages a CRUD lifecycle + object. A `data` block only reads existing objects. Both declare a typed + schema. +- **Schema attributes.** Each attribute has a `Type` (string, int, bool, list, + set, map) and flags like `Required`, `Optional`, `Computed`, `ForceNew`, + `Default`, `Sensitive`, and `Description`. Changing any flag alters + user-visible behavior. +- **State.** Terraform stores last-known attribute values in state. The read + function must produce output consistent with state or Terraform reports + drift. +- **Plan and apply.** Bugs in schema or read logic cause perpetual diffs, + surprise replacements, or silent data loss. +- **Imports.** Users adopt existing infrastructure with `terraform import`. The + read function must populate full state from just the resource ID. + +### Backward Compatibility Rules + +- **Safe (minor/patch):** adding new optional attributes with defaults; adding + new resources or data sources. +- **Breaking (major):** removing or renaming attributes; changing `Optional` to + `Required`; changing `Type`; adding `ForceNew` to an existing attribute. +- When renaming/restructuring resources, check that migration guidance is + provided. Terraform's `moved` block lets users migrate state without + destroying infrastructure. Removing a `moved` block is itself a breaking + change. + +## Repository-Specific Rules (read carefully) + +- **Do not flag** create/update functions that return `nil` instead of calling + the read function afterward. This provider intentionally avoids + read-after-write to minimize API calls against GitHub's rate limits and to + avoid stale reads from eventually-consistent endpoints (see + [#2892](https://github.com/integrations/terraform-provider-github/issues/2892)). +- Acceptance and manual validation are important. See `CONTRIBUTING.md`. +- Prefer matching resource/data source tests when implementation behavior + changes. +- When schema or state semantics change, treat as high-risk unless + compatibility is clearly preserved. +- Breaking changes must follow semantic versioning: attribute removal/rename + or new required fields warrant a major version bump. +- Example configurations under `examples/` should be self-contained, follow + standard module structure, and not include `provider` blocks inside nested + modules. +- Sensitive attributes (tokens, secrets, private keys) must be marked + `Sensitive: true` in the schema and must not appear in log output. + +## Universal Review Checklist + +### 1. Correctness and Behavior + +- Verify CRUD/read logic correctly maps GitHub API responses to schema/state. +- Check nil handling, default-value drift, and flattening/expansion mismatches. +- Confirm update paths do not accidentally force replacement or wipe optional + fields. +- Validate retry/backoff and error classification for API failures. + +### 2. Schema and State Compatibility + +- Any `schema.Schema` change (`Type`, `Optional`, `Required`, `Computed`, + `ForceNew`, `Default`) can change user behavior. +- Confirm imports still work and read functions produce stable state. +- Flag any behavior that may break existing state without migration + notes/tests. +- Watch for attribute rename/removal without a deprecation path. +- New or changed attributes should include `ValidateFunc`/`ValidateDiagFunc` + to catch invalid values at plan time rather than at apply time. +- All attributes should have a `Description` string. +- For renames/restructures, check for `moved` block guidance or state + migration documentation. +- Mark secret-holding attributes with `Sensitive: true`. + +### 3. Terraform UX and Drift + +- Ensure diff suppression, normalization, and API canonicalization avoid + perpetual diffs. +- Check that empty vs. null handling is intentional. +- Verify list/set ordering behavior and deterministic state output. + +### 4. Testing Expectations + +- For behavior changes, check matching tests under `github/*_test.go`. +- Prefer acceptance tests for API-facing changes (`TF_ACC=1` scenarios). +- Ensure tests assert the bugfix/regression target, not only happy path. +- Flag missing tests when logic changed but coverage did not. + +### 5. Docs and Examples + +- If resource/data source behavior changed, review website docs updates under + `website/`. +- If user workflow changed, review corresponding example updates under + `examples/`. +- Confirm examples still reflect current schema and argument names. +- Example directories should follow standard module structure (`main.tf`, + `variables.tf`, `outputs.tf`) with a `README.md`. +- Variables and outputs in examples should include `description` and `type`. +- If a PR adds or changes a resource, verify there is at least one example + showing typical usage. + +### 6. Security and Permissions + +- Verify sensitive values are not logged or exposed in state. +- Check token/credential handling and least-privilege assumptions. +- Document permission scope requirements for new API calls. +- Confirm no secrets or credentials are hardcoded in examples. +- Verify debug/trace logging does not print sensitive attribute values. +- Sensitive outputs should be marked `sensitive = true`. + +### 7. Performance and API Safety + +- Flag new N+1 patterns, excessive API calls, or missing pagination handling. +- Check for rate-limit-sensitive loops and absent caching where needed. +- Confirm context cancellation/timeouts are respected in long operations. + +### 8. Versioning and Changelog + +- Breaking changes (attribute removal/rename, forced replacement, new required + fields) must be called out for a MAJOR version bump. +- Backward-compatible additions (new optional attributes with defaults, new + resources/data sources) correspond to MINOR version bumps. +- Bug fixes with no schema change correspond to PATCH version bumps. +- Verify the PR description or `CHANGELOG.md` includes a clear summary of what + changed and the user impact. + +## Review Report Format + +Return findings first, ordered by severity: + +1. `HIGH`/`MEDIUM`/`LOW` title — short impact statement +2. File reference: `path/to/file.go:line` +3. Why this is a problem (runtime behavior, Terraform UX, upgrade risk) +4. Suggested fix (concise) + +Then include: + +- `Open Questions / Assumptions` +- `Residual Risk` +- `Change Summary` (brief) + +If no issues are found, explicitly state `No blocking findings found` and list +remaining risk areas. diff --git a/.github/instructions/docs.instructions.md b/.github/instructions/docs.instructions.md new file mode 100644 index 0000000000..d251abdb7a --- /dev/null +++ b/.github/instructions/docs.instructions.md @@ -0,0 +1,42 @@ +--- +applyTo: "website/**" +--- + +# Website / Docs Review + +These rules apply to documentation under `website/`. Combine with the +repo-wide checklist in `.github/copilot-instructions.md`. + +## Keep Docs in Sync with Schema + +- Any schema change in `github/` (new attribute, renamed attribute, + changed `Required`/`Optional`/`Computed`/`Default`, new `ForceNew`, + removed attribute) must have a matching docs update. +- Argument tables should list attributes with the same name, type, and + required/optional status as the schema. +- Deprecated attributes must be clearly marked and include guidance on the + replacement. + +## Imports + +- Resources that support `terraform import` must document the import ID + format with at least one example command. + +## Permissions and Scopes + +- For any GitHub API call that requires a specific token scope or app + permission, the docs should call this out so users can configure their + authentication correctly. + +## Examples in Docs + +- Inline example HCL should reflect current argument names and be + syntactically valid. +- Sensitive attributes should not appear with real-looking secrets, even as + examples. + +## Style and Links + +- Internal links between docs pages should resolve. +- New resources/data sources need at least one usage example and a list of + every supported argument and attribute. diff --git a/.github/instructions/examples.instructions.md b/.github/instructions/examples.instructions.md new file mode 100644 index 0000000000..c0134e5bc2 --- /dev/null +++ b/.github/instructions/examples.instructions.md @@ -0,0 +1,56 @@ +--- +applyTo: "examples/**" +--- + +# Example Configuration Review + +These rules apply to Terraform configurations under `examples/`. Combine +with the repo-wide checklist in `.github/copilot-instructions.md`. + +## Standard Module Structure + +Each example directory should follow the +[standard module structure](https://developer.hashicorp.com/terraform/language/modules/develop/structure): + +- `main.tf` — primary resources/module calls +- `variables.tf` — input variable declarations +- `outputs.tf` — output declarations +- `README.md` — purpose and usage of the example + +Empty stub files are acceptable when an example legitimately has no inputs +or outputs. + +## Variables and Outputs + +- Every `variable` and `output` block should include a `description` and + `type`. +- Outputs that surface sensitive values must be marked `sensitive = true`. +- Variables that accept secrets should be marked `sensitive = true`. + +## Provider Configuration + +- **Do not** embed `provider` blocks inside nested or child modules. + Provider configuration belongs in root modules only. A module with a + `provider` block is not compatible with `count`, `for_each`, or + `depends_on`. +- Each example module should declare `required_providers` with a minimum + version constraint (`>=`) for the `integrations/github` provider. + +## Coverage + +- If a PR adds or meaningfully changes a resource or data source, verify + there is at least one example demonstrating typical usage. +- Examples must reflect the current schema: argument names, required vs. + optional, default values. + +## Security + +- No hardcoded tokens, passwords, or other secrets. Reference variables or + environment-sourced values instead. +- Example READMEs should call out any non-obvious permission requirements. + +## Refactoring and `moved` Blocks + +When an example demonstrates resource renames or restructures, prefer +`moved` blocks over destroy-and-recreate. Removing a previously published +`moved` block is itself a breaking change for downstream users. diff --git a/.github/instructions/schema-and-state.instructions.md b/.github/instructions/schema-and-state.instructions.md new file mode 100644 index 0000000000..86509aaf3c --- /dev/null +++ b/.github/instructions/schema-and-state.instructions.md @@ -0,0 +1,74 @@ +--- +applyTo: "github/**/*.go" +--- + +# Provider Source Review (Schema, State, API) + +These rules apply to all provider Go source files under `github/`. Combine +with the repo-wide checklist in `.github/copilot-instructions.md`. + +## Schema Changes Are User-Visible + +Any change to `schema.Schema` (`Type`, `Optional`, `Required`, `Computed`, +`ForceNew`, `Default`, `Sensitive`, `Description`) is potentially breaking. +Flag all schema diffs and verify: + +- Attribute removals or renames have a deprecation cycle or `moved`/state + migration guidance. +- `Optional` → `Required` transitions are called out as breaking. +- New `ForceNew` flags on existing attributes are called out as breaking + (forces resource replacement). +- New attributes are `Optional` with a `Default` where reasonable to avoid + forcing existing users to update their configs. +- All attributes carry a `Description` string (used for docs generation). +- Attributes accepting bounded values declare `ValidateFunc` or + `ValidateDiagFunc` so invalid input fails at plan time, not apply time. +- Attributes holding tokens, secrets, or private keys are marked + `Sensitive: true`. + +## State and Drift + +- Read functions must populate every state attribute from API responses so + `terraform import` works from the resource ID alone. +- Verify the read path does not produce values that differ from what create/ + update wrote (causes perpetual diffs). +- Watch list/set ordering: prefer `schema.TypeSet` or stable sort when the + GitHub API does not return deterministic order. +- Empty vs. null handling must be intentional and consistent between create, + read, and update. +- Diff suppression (`DiffSuppressFunc`) and normalization should be reviewed + for correctness whenever schema is touched. + +## CRUD Behavior + +- Update paths must not accidentally force resource replacement or wipe + optional fields that the user did not change. +- Nil-check API response fields before dereferencing. +- Classify errors: 404 from GitHub typically means "remove from state" in + read; other errors should bubble up. +- Respect `context.Context` cancellation and any configured timeouts. + +## Repository-Specific: No Read-After-Write + +**Do not flag** create or update functions that return `nil` instead of +calling the resource's read function. This is intentional in this provider to +minimize API calls against GitHub rate limits and to avoid stale reads from +eventually-consistent endpoints. See +[#2892](https://github.com/integrations/terraform-provider-github/issues/2892). + +## API Safety and Performance + +- Flag new N+1 access patterns over GitHub APIs. +- Verify pagination is handled (`ListOptions` / `NextPage` loops) on any + endpoint that returns a list. +- Check for rate-limit-sensitive loops; consider caching or batching where + appropriate. +- Sensitive values must never appear in log output, even at debug/trace + level. + +## Security + +- Token, credential, and webhook secret handling must follow least + privilege. +- New API calls should document the GitHub permission scope they require. +- Do not hardcode secrets anywhere in source. diff --git a/.github/instructions/tests.instructions.md b/.github/instructions/tests.instructions.md new file mode 100644 index 0000000000..56f52c4f4c --- /dev/null +++ b/.github/instructions/tests.instructions.md @@ -0,0 +1,36 @@ +--- +applyTo: "github/**/*_test.go" +--- + +# Provider Test Review + +These rules apply to test files under `github/`. Combine with the repo-wide +checklist in `.github/copilot-instructions.md`. + +## Coverage Expectations + +- When behavior in a resource or data source changes, there must be a + matching test change. Flag PRs where production code changed but tests + did not. +- Prefer acceptance tests (`TF_ACC=1`) for any API-facing change. Unit-only + coverage is rarely sufficient for schema or CRUD changes. +- Tests should assert the specific bugfix or regression being targeted, not + only the happy path. + +## Test Structure + +- Acceptance tests should exercise create → read → import → update → destroy + where applicable. Import steps are particularly valuable because they + validate that the read function can reconstruct state from the ID alone. +- Use realistic fixture data; avoid asserting on transient or + environment-specific fields without normalization. +- Avoid hardcoded secrets or tokens in test files; use environment variables + or test helpers. + +## When Reviewing Test Changes + +- If a test was deleted or weakened, explain why in the report and flag as + at least MEDIUM unless the corresponding production code was also removed. +- New skip conditions or `t.Skip` calls must include a clear justification. +- Tests that depend on specific organization/repo names should use the + shared test helpers/config, not hardcoded values. diff --git a/.github/skills/terraform-provider-pr-review/SKILL.md b/.github/skills/terraform-provider-pr-review/SKILL.md deleted file mode 100644 index beb9d23af6..0000000000 --- a/.github/skills/terraform-provider-pr-review/SKILL.md +++ /dev/null @@ -1,89 +0,0 @@ ---- -name: terraform-provider-pr-review -description: "Review pull requests for the Terraform GitHub provider. Use when asked to review a PR, audit Terraform provider changes, check schema or state behavior, verify acceptance tests, or catch regressions in resources/data sources/docs/examples." -argument-hint: "PR number/URL or 'active/open PR' plus focus areas (schema, tests, docs, migration, security)" -user-invocable: true ---- - -# Terraform Provider PR Review - -Use this skill to perform a high-signal code review for this repository (`integrations/terraform-provider-github`). - -## Review Goals - -- Find correctness bugs, regressions, and provider behavior changes. -- Validate schema/state compatibility for Terraform users. -- Check test coverage (unit and acceptance), docs, and examples. -- Identify risk around GitHub API usage, permissions, and error handling. - -## Terraform Background for Reviewers - -This section summarizes the Terraform module and provider concepts most relevant to PR review. Refer back here when evaluating schema, examples, or state changes. - -### Key Concepts - -- **Resources and Data Sources**: A *resource* (`resource` block) manages a lifecycle object (create, read, update, delete). A *data source* (`data` block) only reads existing objects. Both declare a *schema* of typed attributes. -- **Schema Attributes**: Each attribute has a `Type` (string, int, bool, list, set, map), and flags like `Required`, `Optional`, `Computed`, `ForceNew`, `Default`, `Sensitive`, and `Description`. Changing any flag alters user-visible behavior. -- **State**: Terraform stores the last-known attribute values of every managed resource in *state*. The *read* function must produce output consistent with state or Terraform will detect drift and propose changes. -- **Plan and Apply**: `terraform plan` computes a diff between desired config and current state. `terraform apply` executes that diff. Bugs in schema or read logic cause perpetual diffs, surprise replacements, or silent data loss. -- **Imports**: Users can adopt existing infrastructure with `terraform import`. The resource's read function must be able to populate full state from just the resource ID. - -### Module Structure (for reviewing `examples/`) - -- A module is a directory of `.tf` files. The recommended layout is `main.tf`, `variables.tf`, `outputs.tf`, and a `README.md`. -- Variables and outputs should always include `description` and `type`. -- Child modules must **not** contain `provider` blocks — provider configuration belongs exclusively in root modules. -- Each module should declare `required_providers` with a minimum version constraint (`>=`). - -### Backward Compatibility Rules - -- **Safe (minor/patch)**: Adding new optional attributes with defaults, adding new resources or data sources. -- **Breaking (major)**: Removing or renaming attributes, changing `Optional` to `Required`, changing `Type`, adding `ForceNew` to an existing attribute. -- When renaming resources or restructuring modules, Terraform's `moved` block lets users migrate state without destroying infrastructure. If a PR restructures resources, check that migration guidance is provided. -- Removing a `moved` block is itself a breaking change. - -## Inputs - -- PR identifier: URL, number, or `active/open PR`. -- Optional focus areas: `schema`, `state`, `tests`, `docs`, `examples`, `security`, `performance`. - -## Procedure - -1. Gather PR context. -2. Inspect changed files and classify them: - - Provider/resource/data-source code under `github/` - - Tests (`*_test.go`, especially acceptance tests) - - Docs/site content under `website/` - - Example configurations under `examples/` -3. For schema changes, verify backward compatibility using the rules in *Terraform Background*: flag attribute removal/renames, type changes, new `ForceNew`, or `Optional`→`Required` transitions. If a PR restructures resources or renames modules, consult [Refactoring Modules](./references/refactoring.mdx) for `moved` block requirements. -4. For example/configuration changes, confirm they follow [Standard Module Structure](./references/structure.mdx) (`main.tf`, `variables.tf`, `outputs.tf`, `README.md`), do not embed `provider` blocks in child modules (see [Providers Within Modules](./references/providers.mdx)), and include `description` on variables/outputs. -5. Review against the checklist in [Terraform Provider Review Checklist](./references/review-checklist.md). -6. Prioritize findings by severity and provide actionable fixes. -7. Report residual risk and testing gaps when uncertain. - -## Output Format - -Return findings first, ordered by severity. - -1. `HIGH`/`MEDIUM`/`LOW` title - short impact statement -2. File reference: `path/to/file.go:line` -3. Why this is a problem (runtime behavior, Terraform UX, upgrade risk) -4. Suggested fix (concise) - -Then include: - -- `Open Questions / Assumptions` -- `Residual Risk` -- `Change Summary` (brief) - -If no issues are found, explicitly state `No blocking findings found` and list remaining risk areas. - -## Repository Notes - -- Acceptance and manual validation are important in this provider. See contribution guidance in `CONTRIBUTING.md`. -- Prefer matching resource/data source tests when implementation behavior changes. -- When schema or state semantics change, treat as high-risk unless compatibility is clearly preserved. -- Breaking changes must follow semantic versioning: attribute removal/rename or new required fields warrant a major version bump. -- Create/update functions in this provider intentionally do **not** call the read function afterward. This reduces API call volume against GitHub's rate limits and avoids stale reads from eventually-consistent endpoints (see [#2892](https://github.com/integrations/terraform-provider-github/issues/2892)). Do not flag this as an issue. -- Example configurations under `examples/` should be self-contained, follow standard module structure, and not include `provider` blocks inside nested modules. -- Sensitive attributes (tokens, secrets, private keys) must be marked `Sensitive: true` in the schema and must not appear in log output. diff --git a/.github/skills/terraform-provider-pr-review/references/composition.mdx b/.github/skills/terraform-provider-pr-review/references/composition.mdx deleted file mode 100644 index e14a6bad6b..0000000000 --- a/.github/skills/terraform-provider-pr-review/references/composition.mdx +++ /dev/null @@ -1,380 +0,0 @@ ---- -page_title: Module Composition -description: |- - Module composition allows infrastructure to be described from modular - building blocks. -# START AUTO GENERATED METADATA, DO NOT EDIT -created_at: 2025-11-19T13:27:46Z -last_modified: 2025-11-19T13:27:46Z -# END AUTO GENERATED METADATA ---- - -# Module Composition - -In a simple Terraform configuration with only one root module, we create a -flat set of resources and use Terraform's expression syntax to describe the -relationships between these resources: - -```hcl -resource "aws_vpc" "example" { - cidr_block = "10.1.0.0/16" -} - -resource "aws_subnet" "example" { - vpc_id = aws_vpc.example.id - - availability_zone = "us-west-2b" - cidr_block = cidrsubnet(aws_vpc.example.cidr_block, 4, 1) -} -``` - -When we introduce `module` blocks, our configuration becomes hierarchical -rather than flat: each module contains its own set of resources, and possibly -its own child modules, which can potentially create a deep, complex tree of -resource configurations. - -However, in most cases we strongly recommend keeping the module tree flat, -with only one level of child modules, and use a technique similar to the -above of using expressions to describe the relationships between the modules: - -```hcl -module "network" { - source = "./modules/aws-network" - - base_cidr_block = "10.0.0.0/8" -} - -module "consul_cluster" { - source = "./modules/aws-consul-cluster" - - vpc_id = module.network.vpc_id - subnet_ids = module.network.subnet_ids -} -``` - -We call this flat style of module usage _module composition_, because it -takes multiple [composable](https://en.wikipedia.org/wiki/Composability) -building-block modules and assembles them together to produce a larger system. -Instead of a module _embedding_ its dependencies, creating and managing its -own copy, the module _receives_ its dependencies from the root module, which -can therefore connect the same modules in different ways to produce different -results. - -The rest of this page discusses some more specific composition patterns that -may be useful when describing larger systems with Terraform. - -## Dependency Inversion - -In the example above, we saw a `consul_cluster` module that presumably describes -a cluster of [HashiCorp Consul](https://www.consul.io/) servers running in -an AWS VPC network, and thus it requires as arguments the identifiers of both -the VPC itself and of the subnets within that VPC. - -An alternative design would be to have the `consul_cluster` module describe -its _own_ network resources, but if we did that then it would be hard for -the Consul cluster to coexist with other infrastructure in the same network, -and so where possible we prefer to keep modules relatively small and pass in -their dependencies. - -This [dependency inversion](https://en.wikipedia.org/wiki/Dependency_inversion_principle) -approach also improves flexibility for future -refactoring, because the `consul_cluster` module doesn't know or care how -those identifiers are obtained by the calling module. A future refactor may -separate the network creation into its own configuration, and thus we may -pass those values into the module from data sources instead: - -```hcl -data "aws_vpc" "main" { - tags = { - Environment = "production" - } -} - -data "aws_subnet_ids" "main" { - vpc_id = data.aws_vpc.main.id -} - -module "consul_cluster" { - source = "./modules/aws-consul-cluster" - - vpc_id = data.aws_vpc.main.id - subnet_ids = data.aws_subnet_ids.main.ids -} -``` - -### Conditional Creation of Objects - -In situations where the same module is used across multiple environments, -it's common to see that some necessary object already exists in some -environments but needs to be created in other environments. - -For example, this can arise in development environment scenarios: for cost -reasons, certain infrastructure may be shared across multiple development -environments, while in production the infrastructure is unique and managed -directly by the production configuration. - -Rather than trying to write a module that itself tries to detect whether something -exists and create it if not, we recommend applying the dependency inversion -approach: making the module accept the object it needs as an argument, via -an input variable. - -For example, consider a situation where a Terraform module deploys compute -instances based on a disk image, and in some environments there is a -specialized disk image available while other environments share a common -base disk image. Rather than having the module itself handle both of these -scenarios, we can instead declare an input variable for an object representing -the disk image. Using AWS EC2 as an example, we might declare a common subtype -of the `aws_ami` resource type and data source schemas: - -```hcl -variable "ami" { - type = object({ - # Declare an object using only the subset of attributes the module - # needs. Terraform will allow any object that has at least these - # attributes. - id = string - architecture = string - }) -} -``` - -The caller of this module can now itself directly represent whether this is -an AMI to be created inline or an AMI to be retrieved from elsewhere: - -```hcl -# In situations where the AMI will be directly managed: - -resource "aws_ami_copy" "example" { - name = "local-copy-of-ami" - source_ami_id = "ami-abc123" - source_ami_region = "eu-west-1" -} - -module "example" { - source = "./modules/example" - - ami = aws_ami_copy.example -} -``` - -```hcl -# Or, in situations where the AMI already exists: - -data "aws_ami" "example" { - owner = "9999933333" - - tags = { - application = "example-app" - environment = "dev" - } -} - -module "example" { - source = "./modules/example" - - ami = data.aws_ami.example -} -``` - -This is consistent with Terraform's declarative style: rather than creating -modules with complex conditional branches, we directly describe what -should already exist and what we want Terraform to manage itself. - -By following this pattern, we can be explicit about in which situations we -expect the AMI to already be present and which we don't. A future reader -of the configuration can then directly understand what it is intending to do -without first needing to inspect the state of the remote system. - -In the above example, the object to be created or read is simple enough to -be given inline as a single resource, but we can also compose together multiple -modules as described elsewhere on this page in situations where the -dependencies themselves are complicated enough to benefit from abstractions. - -## Assumptions and Guarantees - -Every module has implicit assumptions and guarantees that define what data it expects and what data it produces for consumers. - -- **Assumption:** A condition that must be true in order for the configuration of a particular resource to be usable. For example, an `aws_instance` configuration can have the assumption that the given AMI will always be configured for the `x86_64` CPU architecture. -- **Guarantee:** A characteristic or behavior of an object that the rest of the configuration should be able to rely on. For example, an `aws_instance` configuration can have the guarantee that an EC2 instance will be running in a network that assigns it a private DNS record. - -We recommend [validating your configuration](/terraform/language/validate) to help capture and test for assumptions and guarantees. This helps future maintainers understand the configuration design and intent. Configuration validation returns useful information about errors earlier and in context, helping consumers more easily diagnose issues in their configurations. - -The following examples creates a precondition that checks whether the EC2 instance has an encrypted root volume. - -```hcl -output "api_base_url" { - value = "https://${aws_instance.example.private_dns}:8433/" - - # The EC2 instance must have an encrypted root volume. - precondition { - condition = data.aws_ebs_volume.example.encrypted - error_message = "The server's root volume is not encrypted." - } -} -``` - -## Multi-cloud Abstractions - -Terraform itself intentionally does not attempt to abstract over similar -services offered by different vendors, because we want to expose the full -functionality in each offering and yet unifying multiple offerings behind a -single interface will tend to require a "lowest common denominator" approach. - -However, through composition of Terraform modules it is possible to create -your own lightweight multi-cloud abstractions by making your own tradeoffs -about which platform features are important to you. - -Opportunities for such abstractions arise in any situation where multiple -vendors implement the same concept, protocol, or open standard. For example, -the basic capabilities of the domain name system are common across all vendors, -and although some vendors differentiate themselves with unique features such -as geolocation and smart load balancing, you may conclude that in your use-case -you are willing to eschew those features in return for creating modules that -abstract the common DNS concepts across multiple vendors: - -```hcl -module "webserver" { - source = "./modules/webserver" -} - -locals { - fixed_recordsets = [ - { - name = "www" - type = "CNAME" - ttl = 3600 - records = [ - "webserver01", - "webserver02", - "webserver03", - ] - }, - ] - server_recordsets = [ - for i, addr in module.webserver.public_ip_addrs : { - name = format("webserver%02d", i) - type = "A" - records = [addr] - } - ] -} - -module "dns_records" { - source = "./modules/route53-dns-records" - - route53_zone_id = var.route53_zone_id - recordsets = concat(local.fixed_recordsets, local.server_recordsets) -} -``` - -In the above example, we've created a lightweight abstraction in the form of -a "recordset" object. This contains the attributes that describe the general -idea of a DNS recordset that should be mappable onto any DNS provider. - -We then instantiate one specific _implementation_ of that abstraction as a -module, in this case deploying our recordsets to Amazon Route53. - -If we later wanted to switch to a different DNS provider, we'd need only to -replace the `dns_records` module with a new implementation targeting that -provider, and all of the configuration that _produces_ the recordset -definitions can remain unchanged. - -We can create lightweight abstractions like these by defining Terraform object -types representing the concepts involved and then using these object types -for module input variables. In this case, all of our "DNS records" -implementations would have the following variable declared: - -```hcl -variable "recordsets" { - type = list(object({ - name = string - type = string - ttl = number - records = list(string) - })) -} -``` - -While DNS serves as a simple example, there are many more opportunities to -exploit common elements across vendors. A more complex example is Kubernetes, -where there are now many different vendors offering hosted Kubernetes clusters -and even more ways to run Kubernetes yourself. - -If the common functionality across all of these implementations is sufficient -for your needs, you may choose to implement a set of different modules that -describe a particular Kubernetes cluster implementation and all have the common -trait of exporting the hostname of the cluster as an output value: - -```hcl -output "hostname" { - value = azurerm_kubernetes_cluster.main.fqdn -} -``` - -You can then write _other_ modules that expect only a Kubernetes cluster -hostname as input and use them interchangeably with any of your Kubernetes -cluster modules: - -```hcl -module "k8s_cluster" { - source = "modules/azurerm-k8s-cluster" - - # (Azure-specific configuration arguments) -} - -module "monitoring_tools" { - source = "modules/monitoring_tools" - - cluster_hostname = module.k8s_cluster.hostname -} -``` - -## Data-only Modules - -Most modules contain `resource` blocks and thus describe infrastructure to be -created and managed. It may sometimes be useful to write modules that do not -describe any new infrastructure at all, but merely retrieve information about -existing infrastructure that was created elsewhere using -[data sources](/terraform/language/data-sources). - -As with conventional modules, we suggest using this technique only when the -module raises the level of abstraction in some way, in this case by -encapsulating exactly how the data is retrieved. - -A common use of this technique is when a system has been decomposed into several -subsystem configurations but there is certain infrastructure that is shared -across all of the subsystems, such as a common IP network. In this situation, -we might write a shared module called `join-network-aws` which can be called -by any configuration that needs information about the shared network when -deployed in AWS: - -```hcl -module "network" { - source = "./modules/join-network-aws" - - environment = "production" -} - -module "k8s_cluster" { - source = "./modules/aws-k8s-cluster" - - subnet_ids = module.network.aws_subnet_ids -} -``` - -The `network` module itself could retrieve this data in a number of different -ways: it could query the AWS API directly using -[`aws_vpc`](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/vpc) -and -[`aws_subnet_ids`](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/subnet_ids) -data sources, or it could read saved information from a Consul cluster using -[`consul_keys`](https://registry.terraform.io/providers/hashicorp/consul/latest/docs/data-sources/keys), -or it might read the outputs directly from the state of the configuration that -manages the network using -[`terraform_remote_state`](/terraform/language/state/remote-state-data). - -The key benefit of this approach is that the source of this information can -change over time without updating every configuration that depends on it. -Furthermore, if you design your data-only module with a similar set of outputs -as a corresponding management module, you can swap between the two relatively -easily when refactoring. diff --git a/.github/skills/terraform-provider-pr-review/references/index.mdx b/.github/skills/terraform-provider-pr-review/references/index.mdx deleted file mode 100644 index 5e1eab23c3..0000000000 --- a/.github/skills/terraform-provider-pr-review/references/index.mdx +++ /dev/null @@ -1,89 +0,0 @@ ---- -page_title: Creating Modules -description: >- - Modules are containers for multiple resources that are used together in a - configuration. Learn when to create modules and about module structure. -# START AUTO GENERATED METADATA, DO NOT EDIT -created_at: 2025-11-19T13:27:46Z -last_modified: 2025-11-19T13:27:46Z -# END AUTO GENERATED METADATA ---- - -# Creating Modules - -> **Hands-on:** Try the [Reuse Configuration with Modules](/terraform/tutorials/modules?utm_source=WEBSITE&utm_medium=WEB_IO&utm_offer=ARTICLE_PAGE&utm_content=DOCS) tutorials. - -A _module_ is a container for multiple resources that are used together. -You can use modules to create lightweight abstractions, so that you can -describe your infrastructure in terms of its architecture, rather than -directly in terms of physical objects. - -The `.tf` files in your working directory when you run [`terraform plan`](/terraform/cli/commands/plan) -or [`terraform apply`](/terraform/cli/commands/apply) together form the _root_ -module. That module may [call other modules](/terraform/language/modules/syntax#calling-a-child-module) -and connect them together by passing output values from one to input values -of another. - -To learn how to _use_ modules, see [the Modules configuration section](/terraform/language/modules). -This section is about _creating_ re-usable modules that other configurations -can include using `module` blocks. - -## Module structure - -Re-usable modules are defined using all of the same -[configuration language](/terraform/language) concepts we use in root modules. -Most commonly, modules use: - -* [Input variables](/terraform/language/block/variable) to accept values from - the calling module. -* [Output values](/terraform/language/block/output) to return results to the - calling module, which it can then use to populate arguments elsewhere. -* [Resources](/terraform/language/resources) to define one or more - infrastructure objects that the module will manage. - -To define a module, create a new directory for it and place one or more `.tf` -files inside just as you would do for a root module. Terraform can load modules -either from local relative paths or from remote repositories; if a module will -be re-used by lots of configurations you may wish to place it in its own -version control repository. - -Modules can also call other modules using a `module` block, but we recommend -keeping the module tree relatively flat and using [module composition](/terraform/language/modules/develop/composition) -as an alternative to a deeply-nested tree of modules, because this makes -the individual modules easier to re-use in different combinations. - -## When to write a module - -In principle any combination of resources and other constructs can be factored -out into a module, but over-using modules can make your overall Terraform -configuration harder to understand and maintain, so we recommend moderation. - -A good module should raise the level of abstraction by describing a new concept -in your architecture that is constructed from resource types offered by -providers. - -For example, `aws_instance` and `aws_elb` are both resource types belonging to -the AWS provider. You might use a module to represent the higher-level concept -"[HashiCorp Consul](https://www.consul.io/) cluster running in AWS" which -happens to be constructed from these and other AWS provider resources. - -We _do not_ recommend writing modules that are just thin wrappers around single -other resource types. If you have trouble finding a name for your module that -isn't the same as the main resource type inside it, that may be a sign that -your module is not creating any new abstraction and so the module is -adding unnecessary complexity. Just use the resource type directly in the -calling module instead. - -### No-Code Provisioning in HCP Terraform - -You can also create no-code ready modules to enable the no-code provisioning workflow in HCP Terraform. No-code provisioning lets users deploy a module's resources in HCP Terraform without writing any Terraform configuration. - -No-code ready modules have additional requirements and considerations. Refer to [Designing No-Code Ready Modules](/terraform/cloud-docs/no-code-provisioning/module-design) in the HCP Terraform documentation for details. - -## Refactoring module resources - -You can include [refactoring blocks](/terraform/language/modules/develop/refactoring) to record how resource -names and module structure have changed from previous module versions. -Terraform uses that information during planning to reinterpret existing objects -as if they had been created at the corresponding new addresses, eliminating a -separate workflow step to replace or migrate existing objects. diff --git a/.github/skills/terraform-provider-pr-review/references/module-best-practices.md b/.github/skills/terraform-provider-pr-review/references/module-best-practices.md deleted file mode 100644 index cddd9a1317..0000000000 --- a/.github/skills/terraform-provider-pr-review/references/module-best-practices.md +++ /dev/null @@ -1,908 +0,0 @@ -In this comprehensive blog, I have shared detailed Terraform module development best practices. - -Whether you're creating modules for your team or contributing to open-source projects, these tips will help you write clean, reusable, and easy-to-maintain infrastructure code. - -By the end of this guide, you will have learned: - -How to build reusable and well-structured Terraform modules. -Best practices for naming, inputs, outputs, and file organization. -How to test, document, and version your modules effectively. -How to use CI/CD to automate checks and keep your modules up to date. -How to prepare modules for open-source use with proper GitHub setup. -The article series is divided into two parts. - -In Part 1, we'll look at best practices for developing open-source Terraform modules and talk about the commonly used Terraform module repository template pattern that many organizations use. -In Part 2, we'll dive into a well-designed, pre-configured template that implements all best practices from Part 1, meets the requirements of most Terraform projects, and can be bootstrapped in 5 minutes. -Terraform Modules -Terraform modules are a key paradigm for managing infrastructure as code. They reduce complexity, increase reusability, and ensure consistency across projects. - -By encapsulating infrastructure configurations into modular components, teams can standardize resource setups, reduce errors, and share best practices across projects. - -However, the value of these modules is dependent on their internal structure. A well-structured module guarantees that infrastructure stays understandable and manageable as complexity grows. - -It allows development teams to make changes over time without introducing additional risks or obstacles. - -In contrast, badly constructed modules can cause confusion, make debugging more difficult, and require more maintenance due to hidden bugs and other issues. - -⚠️ -Please keep in mind that HashiCorp changed the Terraform licensing, leading the community to create a fork called OpenTofu, which is licensed under the MPL-2.0 license. All tips, tricks, and best practices shared in this article are fully compatible with OpenTofu. -Let’s get started! - -Terraform Module Development Best Practices -In this section, we will look at best practices for Terraform module development that may help for a wide range of users. - -I recommend you adopt only those practices that are relevant to your project or organization. Although they are suggestions, implementing as many of these into practice as you can will improve the quality and maintainability of your Terraform code. - -Terraform-related configuration -In this section we will look at best practices realated to terraform configurations. - -#1: Use clear input and output definitions -Name: To demonstrate the purpose of variables and outputs, give them descriptive names. -Type: Always declare variable types to ensure consistency. Avoid using any type or missing type. -Default value: Set default values whenever possible. Having a default value is always better to leaving it empty. -Validations: To verify inputs, use Terraform's built-in validation. -Sensitive: Mark variables or outputs handling secrets as sensitive = true to safeguard sensitive data. -Write documentation: Document variables and outputs, including their purpose, types, and default values. -💡 -Use tools like terraform-docs to generate this automatically. -Think about backward compatibility: Adding a variable with a default value is backwards-compatible. Removing a variable is backwards-incompatible; therefore, avoid it wherever possible. -Minimize unnecessary variables: Before creating a variable, consider: "Will this value ever need to change?" If the response is no, use the local value. -#2: Keep Logical Structure -Use a dedicated repository for each Terraform module. Submodules can be included in the module, but they must be directly related to the main module and not implement unrelated functionality. - -💡 -Use the standard module structure for Terraform modules -Organize module files logically: - -main.tf for core resources. -variables.tf for input variables. -outputs.tf for outputs.versions.tf for the minimal terraform version, and provider configurations (if required). -README.md for the repository's basic documentation of the module and all of its nested folders. -To improve readability, split logic into separate files. - -For instance, you may split the main.tf file into something like this if your module is in charge of setting up an Application Load Balancer (ALB), EC2 instances, and S3 buckets: - -network.tf: Contains the configuration for the Application Load Balancer (ALB) and related networking resources. -vms.tf or ec2.tf: Manages the configuration for EC2 instances. -bucket.tf or s3.tf: Includes the S3 bucket configuration. -Use the following directories: - -modules/ for nested or reusable sub-modules. -examples/ for sample configurations and usage examples. -docs/ for additional documentation, such as detailed design or architecture diagrams. -tests/ for infrastructure tests -scripts/ for custom scripts that Terraform may call during execution -helpers/ for organizing helper scripts required for automation or maintenance but not immediately invoked by Terraform. -files/ for static files that Terraform references but does not run. It can be startup scripts, configuration files -wrappers/ for implementing Terragrunt wrapper pattern. -#3: Use nested submodules for complex logic (modules/ directory) -When you dealing with complex Terraform configurations, use nested submodules to break down logic into smaller, reusable units. Follow these best practices: - -Place submodules in the modules/ directory using the naming pattern modules/${module_name}. - -modules/ -├── network/ -├── compute/ -└── storage/ -Nested modules should be considered as private unless explicitly stated in the module’s documentation. - -Users should avoid using nested modules directly unless the developer has specified that they are safe and designed for external usage. Consider it, and remember to update the sub-module information in the documentation during the development. -Always refer to the provided documentation for guidance on module usage. -Terraform does not track refactored resources. When you move resources from a top-level module to submodule or just rename them, Terraform may consider them new resources and try to recreate them. This can lead to significant issues for users, such as resource outages or data loss. - -To mitigate this behavior, use moved in scope of refactoring. - -moved { - from = aws_instance.old_name - to = module.new_submodule.aws_instance.new_name -} -If refactoring is unavoidable, let users know about it as a significant change in the CHANGELOG and give them detailed guidance on how to manage the changeover. - -#4: Try to avoid custom scripts (scripts/ directory) -General rule: Custom scripts should be considered a last resort. Try to avoid them if it is possible. Resources created by such scripts are not tracked or managed by Terraform state, which can lead to inconsistencies. - -Exceptions to the rule: Custom scripts may be necessary when Terraform does not support the desired functionality. - -Before starting to use scripts, consider these alternatives: - -Provider-defined functions: Explore the existing capabilities of your Terraform provider -Try to use alternative providers: For example, in the case of AWS you can use awscc (AWS Cloud Control API) which is generated from CloudFormation resource definitions and has different resource scopes. -3. Use something like TerraCurl for unsupported resources. - -You can check more information about it in the article Writing Terraform for unsupported resources. - -If custom scripts are indeed necessary and if none of the above options work, follow these two key steps: - -Document scripts clearly: Provide a detailed explanation for why the custom script exists. Include a deprecation plan, if possible, and write clear documentation for the script and its usage. -2. Use Provisioners for Execution: Use Terraform’s provisioners (e.g., local-exec) to call custom scripts when needed. - -#5: Automate toil with helper scripts (helpers/ directory) -For any repetitive or manual tasks that need automation, create simple scripts and store them in the helpers/ directory. These scripts should abide by the following principles: - -Clear Documentation -Provide a brief overview of the script's purpose and why it exists. -Include usage instructions and any other relevant context. -2. Usability Standards - -Implement argument validation to handle incorrect or missing inputs gracefully (in case of using bash, you may check “How to Use Bash Getopts With Examples" article). -Include a --help option that explains the script’s functionality, arguments, and examples of usage. -#6: Store static files in a separate directory (files/ directory) -Templates, assets, and other items that Terraform references but does not run directly are known as static files. To properly organize these files, adhere to the following rules: - -Separate lengthy HereDocs -Move lengthy HereDoc content into external files for better readability. -Use the file() function to reference these files in your Terraform code. -2. Use .tftpl or .tpl file extensions for templates - -When working with Terraform's templatefile() function, use .tftpl or .tpl extensions for template files. While Terraform doesn't enforce this naming standard (you can use any that you like), this extension will help your editor understand the content and might offer a better editing experience as a result. -3. Place Templates in a templates/ subdirectory - -Organize all template files within a templates/ subdirectory inside the files/ directory. -A typical example of content within the files/ directory is an AWS Lambda function, commonly placed in files//. - -In this case, the files// directory can be considered the root folder, and if the Lambda function is written in Python, this directory may contain an internal structure such as Python packages, dependencies, build scripts or other related files. - -#7: Implement Terragrunt Wrapper -Many people use Terragrunt to follow DRY principles. Terragrunt can handle a wide range of tasks across your stack, but module development requires some preparation as well. - -In some cases, it is not feasible to use Terraform's native for_each feature. Hence, the Terragrunt wrapper, also known as the single-module wrapper pattern, comes into play. This technique helps in situations like: - -You need to deploy multiple copies of a module with different configurations. -Your environment's limitations or complexities prevent you from using native for_each Terraform function. -To apply this approach, establish a new directory wrappers/ with at least three files: - -main.tf -outputs.tf -variables.tf -Let’s check them in more detail. - -main.tf file contains: - -Terraform source -for_each loop over an items variable -All inputs that the module has in the following format: `my_input_variable = try(each.value.my_input_variable, var.defaults.my_input_variable, {})` -For example: - -module "wrapper" { - source = "../" - - for_each = var.items - - my_input_variable = try(each.value.my_input_variable, - var.defaults.my_input_variable, {}) -} -variables.tf file contains: - -defaults variable, that is, a map of default values. These default values will be used as a backup for each input variable if it is empty or missing. -items variable, which maps items to create a wrapper from -For example: - -variable "defaults" { - description = "A map of default values. These default values will be used as a backup for each input variable if it is empty or missing." - type = any - default = {} -} - -variable "items" { - description = "Maps of items to create a wrapper from. Values are passed through to the module." - type = any - default = {} -} -outputs.tf file contains: - -wrapper output with a map of outputs from a wrapper. -For example: - -output "wrapper" { - description = "Map of outputs from a wrapper." - value = module.wrapper -} -#8: Avoid providers or backend configurations -Shared modules must not configure providers or backends. These settings must be defined in root modules in order to maintain flexibility and reusability. - -It is also a good practice to use the required providers block to define only the bare minimum of necessary provider versions. If you need to let the user know about state management in your module, include this information in the README.md file. - -versions.tf Example: - -terraform { - required_version = ">= 1.0" - required_providers { - google = { - source = "hashicorp/google" - version = ">= 4.0.0" - } - } -} - -As shown in the example, it also contains a minimal Terraform version. - -Establishing it is a good practice if you know your code needs a specific version or newer to work properly. If you are unclear about the minimum version required for your module, set >=1.0 as the default definition. - -Test the Terraform code -Testing your Terraform code is just as important as testing other types of programming code. A lot of options exist in the Terraform ecosystem to assist you in ensuring the dependability and correctness of your modules. - -Let's look at some of the best practices for module testing. - -Static analysis -Typically, this involves analyzing the syntax, structure, and deployment configuration using linters, static code analysis tools, or Terraform dry-runs. - -These tools are used before the deployment. This topic will be discussed in further detail in the "Code Linting and Quality" section. - -Module integration testing -Testing modules in isolation is a useful way to confirm that they work as expected. Module integration testing entails deploying the module to a test environment and ensuring that it generates the expected resources. The following testing frameworks can help you write and manage tests: - -Terraform-tests: a built-in Terraform feature that allows you to write tests with HCL. -Terraform-check: a built-in Terraform feature that may help you to validate infrastructure outside the usual resource lifecycle. -Terratest: a Go library that provides patterns and helper functions for testing infrastructure. To gain hands-on experience, start with the article "Automating AWS Infrastructure Testing With Terratest". -terraform-compliance: a lightweight, security and compliancefocused BDD test framework -Google tools -Google's blueprint testing framework -InSpec -End-to-end testing -Integration testing is important to verify that multiple modules work together properly. - -In the scope of this method, all modules that exist in your environment are deployed together. Following that, tests are checking that everything works properly using API calls or any other methods. - -Although this method is long and expensive, it offers the highest level of verification and can ensure that changes do not cause problems with your production systems. - -Some highlights -Don't use old testing tools (like Kitchen-Terraform) and remember to keep the version of tools updated. -Only the first two approaches, static analysis and module integration testing, are relevant in the context of module development. This is because we can't predict how or where the user will use the module. As a result, end-to-end testing is ineffective in this scenario. -All of the tools I outlined for integration testing are also applicable to end-to-end testing. -To learn more about Terraform testing, check the following links; -Testing HashiCorp Terraform -Implement end-to-end Terratest testing on Terraform projects -Write a documentation -A Terraform module's success depends on clear and effective documentation. - -A well-structured README.md makes usage easier, reduces complexity, and improves the overall user experience. Conversely, badly organized documentation might have a negative impact on the module's popularity and usefulness. - -To create high-quality module documentation, adopt the following principles: - -Provide comprehensive examples: Include detailed sample configurations that explain module usage. It's best to give as many examples as possible. Use the examples/ folder for this. -Automate documentation generation: Use terraform-docs to automatically generate and maintain up-to-date documentation -Generate input/output documentation automatically. -Add documentation for: -Detailed examples in the examples/ directory. -Submodules: They should be properly documented, with explanations of their purpose, usage, and specific configurations. -Create a well-structured README.md .Essentially, include the following sections: -An overview of the module. -Examples of usage. -Input and output definitions. -Version compatibility and known limits. -Any badges like “Terraform support” or “OpenTofu support” -Git configuration -Git has various configuration settings that improve project management. The following are essential .git configuration files and their purposes. - -.gitignore -This file specifies which files and directories Git should ignore, so they are not tracked or committed to the repository. The Terraform codebase's gitignore file has to include the following exclusions: - -Terraform -state files -lock files -init directory -extra tfvars files -Any build artifacts generated by non-Terraform code, such as Python bytecode files (.pyc) or anything else -Log files -Example: - -**/.terraform/* -*.tfstate -*.tfstate.* -crash.log -crash.*.log -*.tfvars -*.tfvars.json - -override.tf -override.tf.json -*_override.tf -*_override.tf.json - -.terraform.lock.hcl -.terraform.tfstate.lock.info - -.terraformrc -terraform.rc - -*.pyc -.gitattributes -This file keeps uniform file formatting and prevents line ending issues, especially in projects with contributors from different operating systems. Here is an example of essential configuration: - -# Normalize Terraform files -*.tf text - -# Enforce LF (Unix-style) line endings -*.sh eol=lf -*.ts text eol=lf -*.json text eol=lf -*.md text eol=lf -*.tf text eol=lf -*.txt text eol=lf - -language-configuration.json linguist-language=jsonc -.vscode/**.json linguist-language=jsonc -.gitconfig -This file defines project-specific Git settings for enforcing standards and managing author information. By default, it's good to include the following configuration: - -[core] -autocrlf = input -longpaths = true -autocrlf: Manages line-ending normalization across operating systems (e.g., Windows, Linux, MacOS). When set to input, Git only changes Windows-style line endings (CRLF) to Unix-style line endings (LF) only when a file is committed - -longpaths: Allows Git to support file paths longer than 260 characters, the default maximum path length on Windows. This option prevents errors when working with deeply nested directories or repositories that have long file names. - -Use a trunk-based development -Trunk-based development is the most frequently recommended Git branching strategy, according to DevOps Research and Assessment (DORA), and Google DevOps capabilities. With this method, you can always maintain the main branch clean of unnecessary files or artifacts and ready for deployment. - -Trunk-based development for Terraform module repositories can be adopted in the following way: - -main branch -The main branch contains the most recent code and serves as the main development branch. - -It should always remain clean, protected, and ready for use. - -Feature and bug-fix branches -Development takes place in the feature and bug-fix branches. These branches are chopped off from the main branch. - -Naming convention: - -Feature branches: feature/$feature_name or feat/$feature_name -Bug-fix branches: fix/$bugfix_name -Release Branches -A release branch is used to prepare code for the upcoming release. When the release branch is stable and confirmed, it should be merged with the main branch. - -Naming convention: release/$version_number (e.g., release/v1.2.0). - -Pull Requests -Once a feature or bug fix has been completed, use a pull request to merge it back into the main branch. - -To reduce merge conflicts, rebase branches before merging. - -GitHub configuration -GitHub provides a range of powerful built-in features to enhance repository management and collaboration. - -These features can be activated by adding specific files to your repository, typically within the .github folder. Using these features can help attract contributors, build interest in your repository, and improve the development experience. - - -tofuutils/tenv: A repository that uses almost every GitHub feature. -PULL_REQUEST_TEMPLATE.md -Adding this file to your repository enables project contributors to see a predefined structure in the pull request body automatically. - -It’s considered best practice to include key instructions or questions to help streamline the review process. - -Example: - -## Description -Provide a summary of the PR changes. - -## Checklist -- [ ] Code is formatted (`terraform fmt`). -- [ ] Documentation is updated. -- [ ] Tests are added or updated. - -ISSUE_TEMPLATE directory -This directory contains a collection of .md files that are designed to standardize the submission process for issues. - -These structured templates provide clear instructions to contributors when they report problems, seek enhancements, or deal with other types of issues. - - -A dialog window with pre-defined issue templates -In addition to templates, you may change issue submission behavior by including a .github/ISSUE_TEMPLATE/config.yml file in your repository. This configuration file allows you to specify the default behaviors and settings for issue templates. Example: - -blank_issues_enabled: true -In this file blank_issues_enabled means: - -true: Allows users to submit blank issues without requiring a template. -false: Blocks blank issues entirely, ensuring that all issues are submitted using only predefined templates. -CONTRIBUTING.md -The CONTRIBUTING.md file contains explicit guidelines for contributing to the project. It may include: - -Set up the development environment: Follow step-by-step instructions to clone the repository, install dependencies, and run the project locally. -Submitting pull request: Clearly outline the workflow for creating pull requests, including branch naming rules, explicit commit messages, and code standards. -Code review expectations: Define what contributors should expect from the review process and how feedback will be provided. -Issues and feature requests: Explain how contributors can submit bug reports, suggest new features, and participate in discussions. -LICENSE -The LICENSE file specifies the terms under which your project may be used, modified, and distributed. It ensures that users and contributors understand their rights and responsibilities. - -It's better to choose a license that aligns with your project's goals: - -MIT: A permissive license that allows virtually unrestricted use, modification, and distribution. -Apache 2.0: Provides similar freedoms to MIT but adds protections such as patent rights. -GPL: A copyleft license that mandates derivative works to be open source under the same license. -If you're confused about which license to choose, visit the Choose a License. It provides you with simple instructions and examples to help you choose the best license for your purposes. - -Also, as a default license, you can use the MIT license, which is ideal for a wide range of projects, including Terraform modules. - -CODE_OF_CONDUCT.md -The CODE_OF_CONDUCT.md file allows you to create community standards, indicate a friendly and inclusive project, and specify anticipated behavior. - -Typically, this file may: - -Define acceptable and inappropriate user behavior. -Specify how community members can report violations. -Outline the procedures for resolving disputes and ensuring enforcement. -Emphasize creating a welcoming space for everyone, regardless of background or experience. -FUNDING.yml -The FUNDING.md file allows the repository to display funding options. It might provide links to platforms where contributors can make donations to support the project. Examples of such platforms are: - -Patreon -Open Collective -GitHub Sponsors -Other appropriate platforms based on your requirements. -Don't forget to explain how the funds will be used, whether for development, community projects, or operational purposes. - - -A banner that GitHub displays if FUNDING.md is enabled. -CODEOWNERS -The CODEOWNERS file assigns ownership to specified files or directories, which automates pull request reviews. Example: - -# Assign maintainers for specific parts of the project. -*.tf @team-infra -*.md @alexander-sharov - -For further details, check the official GitHub documentation. - -MAINTAINERS.md -The MAINTAINERS.md file contains a list of project maintainers, their duties, and contact information. Example: - -# MAINTAINERS - -## Maintainers List - -| Name | Role | GitHub Handle | Contact | -|---------------|---------------------|--------------------|---------------------| -| Alice Johnson | Lead Developer | @alice-johnson | alice@example.com | -| Bob Smith | Documentation Lead | @bob-smith | bob@example.com | - -## Responsibilities -- **Lead Developer**: Manages core development and architecture. -- **Documentation Lead**: Ensures project documentation is correct and up-to-date. -SECURITY.md -The SECURITY.md file defines the security policy and vulnerability-handling procedure for your project. Example: - -# SECURITY POLICY - -## Reporting a Vulnerability - -If you discover a security vulnerability, please report it by emailing **security@example.com**. Provide as much detail as possible to help us address the issue promptly. - -## What to Expect -- We will acknowledge receipt of your report within 72 hours. -- A timeline for the investigation and resolution will be communicated. -SUPPORT.md -The SUPPORT.md file instructs users on how to properly report problems or obtain assistance. - -For instance: - -# SUPPORT - -## How to Get Help - -- For general inquiries or usage questions, please open an issue in the repository. -- If you encounter a bug, file a detailed issue with the following information: - - Steps to reproduce the problem - - Expected behavior - - Actual behavior - - Logs or screenshots, if applicable - -## Contact Us - -For urgent or private matters, contact **support@example.com**. We aim to respond within 72 hours. -Continuous integration configuration -Continuous Integration (CI) plays an essential role for open-source repositories since it ensures the reliability, quality, and maintainability of the codebase. When many people make changes, CI pipelines help to ensure consistency and prevent bugs from being merged into the main branch. - -Although there is no single "ideal" CI pipeline, certain key components should always be covered, including: - -Linting -Formatting -Code Validation -Compliance Checks -PR verification automation -All of the mentioned checks can be automated with CI tools like Jenkins, GitLab CI, and GitHub Actions. - -Every time code is pushed or a pull request (PR) is sent, these checks ought to be executed as CI jobs. In addition, since many people are contributing through pull requests, PR validation and main branch validation must be your top priorities. - -To maximize CI pipeline efficiency, implement as many features as possible from the "Code Linting and Quality" section. - -Beyond code validation, CI automation should also cover: - -Documentation updates: Ensure that documentation stays in sync with code changes. -Dependency Management: Handle package updates efficiently. -Versioning & Releases: Automate version control, release management, and change tracking. -Test automation: CI pipeline should include automated checks for all types of tests. These processes align with best practices stated in the "Test Terraform Code" section. -Now let's take a closer look at such automation. - -Setup Versioning -Each module should have a version to manage its usage effectively. For your releases, I advise using semantic versioning (e.g., v1.0.0): - -MAJOR version (1.x.x): Introduces breaking changes. -MINOR version (x.1.x): Adds functionality in a backward-compatible manner. -PATCH version (x.x.1): Fixes bugs or includes improvements without introducing new functionality or breaking backward compatibility. -Ideally, version management should be automated using your CI tool, such as Jenkins, GitLab CI/CD, GitHub Actions, or other similar platforms. In the bulk of my projects, I utilize the GitHub action kvendingoldo/semver-action and other similar platforms, which do the following automatically: - -Set the semantic version as a Git tag for each commit. -Create release branches -Create GitHub releases -This configuration is an excellent starting point, but feel free to experiment with different tools that best suit your workflow. Check such links as: - -Semantic release -Semantic release action -💡 -Note: It's important to use the GitHub Releases features with public modules. If you decide to use a custom versioning process, make sure that you integrate GitHub Releases into your workflow so that your users have clear visibility and accessibility. - -GitHub release produced by kvendingoldo/semver-action. -Automate CHANGELOG updates -A CHANGELOG.md is a file that provides a curated, chronologically ordered list of notable changes for each version of a project. While it can be optional for personal projects, it is mandatory for public projects. - -This file helps users easily track significant changes between releases or versions. - -In the context of Terraform module development, it is important to follow these best practices: - -Maintain a CHANGELOG.md file to document changes across versions. - -For example: - - -# Changelog - -## [1.0.0] - 2025-01-01 -### Added -- Initial release of the Terraform module. - -## [0.1.0] - 2024-12-01 -### Added -- Added basic AWS VPC code. - -### Fixed -- Resolved some typos. -Include changelog updates in pull requests (PRs): Add a note that the PR should include text describing any changes that will be included in the changelog to ensure proper documentation of updates. - -Automate changelog updates to streamline the process and maintain consistency. You can use the following approaches: - -Release Drafter tool -Conventional Changelog tool -Go-changelog from Hashicorp (You can check Cloudflare as an example) -💡 -For additional information on building and maintaining a changelog, visit keep a changelog -Setup Dependency Management -Currently, there are two standard products available for automatic dependency updates that can be used for public Terraform modules. - -Let's look at both of them. - -1. Dependabot -Dependabot is a GitHub-native tool that automatically checks and updates dependencies. You can configure it to manage Terraform modules, GitHub Actions, Python PIP packages, and any other ecosystem. - -Here is a sample configuration for managing Terraform dependencies that checks changes daily and, if found, creates a pull request with a new version: - -# .github/dependabot.yml -version: 2 -updates: - - package-ecosystem: "terraform" - directory: "/" - schedule: - interval: "daily" -Key features of Dependabot: - -Automatically opens pull requests for dependency updates. -Supports a wide range of ecosystems, including Terraform, npm, Python, GitHub Actions, and more. -Includes built-in GitHub security alarms for dependencies' vulnerabilities. -GitHub native -2. Renovate -Renovate is yet another very customisable tool for managing dependencies. It has more flexibility than Dependabot, making it ideal for projects with complicated dependency management requirements. - -Here is an renovate.json sample that does the same function as the DepedaBot configuration: - -{ - "extends": ["config:base"], - "terraform": { - "enabled": true - }, - "schedule": ["daily"] - "automerge": false -} -Key features of Renovate: - -Manages several repositories and ecosystems at once. -Provides granular control over update rules, including grouping updates or setting specific policies. -Automatically updates configuration files, such as Terraform’s versions.tf or .tf.lock.hcl. -Integrates with self-hosted and cloud platforms -Work better with non-GitHub platforms like GitLab CI. -Configure jobs to reduce toil -Maintaining a public project frequently requires you to oversee multiple tasks at once. Any project management task that is performed at least twice ought to be automated to optimize your workflow and time management. - -Here are two useful automations that you may use while developing Terraform modules to improve project efficiency. - -A job that simply closes existing issues and PRs without a new activity. - -name: 'Close stale issues and PRs' -on: - schedule: - - cron: '0 0 * * *' - -jobs: - stale: - runs-on: ubuntu-22.04 - steps: - - uses: actions/stale@v9 - with: - repo-token: ${{ secrets.GITHUB_TOKEN }} - days-before-stale: 30 - stale-issue-label: stale - stale-pr-label: stale - stale-issue-message: | - This issue has been automatically marked as stale because it has been open 30 days - with no activity. Remove stale label or comment or this issue will be closed in 10 days - stale-pr-message: | - This PR has been automatically marked as stale because it has been open 30 days - with no activity. Remove stale label or comment or this PR will be closed in 10 days - # Not stale if have this labels or part of milestone - exempt-issue-labels: bug,wip,on-hold - exempt-pr-labels: bug,wip,on-hold - exempt-all-milestones: true - # Close issue operations - # Label will be automatically removed if the issues are no longer closed nor locked. - days-before-close: 10 - delete-branch: true - close-issue-message: This issue was automatically closed because of stale in 10 days - close-pr-message: This PR was automatically closed because of stale in 10 days -A job that validate PR title - -name: 'Validate PR title' - -on: - pull_request_target: - types: - - opened - - edited - - synchronize - -jobs: - main: - name: Validate PR title - runs-on: ubuntu-22.04 - steps: - - uses: amannn/action-semantic-pull-request@v5.5.3 - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - with: - # Configure which types are allowed. - # Default: https://github.com/commitizen/conventional-commit-types - types: | - fix - feat - docs - ci - chore - # Configure that a scope must always be provided. - requireScope: false - # Configure additional validation for the subject based on a regex. - # This example ensures the subject starts with an uppercase character. - subjectPattern: ^[A-Z].+$ - # If `subjectPattern` is configured, you can use this property to override - # the default error message that is shown when the pattern doesn't match. - # The variables `subject` and `title` can be used within the message. - subjectPatternError: | - The subject "{subject}" found in the pull request title "{title}" - didn't match the configured pattern. Please ensure that the subject - starts with an uppercase character. - # For work-in-progress PRs you can typically use draft pull requests - # from Github. However, private repositories on the free plan don't have - # this option and therefore this action allows you to opt-in to using the - # special "[WIP]" prefix to indicate this state. This will avoid the - # validation of the PR title and the pull request checks remain pending. - # Note that a second check will be reported if this is enabled. - wip: true - # When using "Squash and merge" on a PR with only one commit, GitHub - # will suggest using that commit message instead of the PR title for the - # merge commit, and it's easy to commit this by mistake. Enable this option - # to also validate the commit message for one commit PRs. - validateSingleCommit: false -GitHub actions usage -Existing GitHub Actions are important building blocks for creating GitHub pipelines. - -👨‍💻 -My key advice: avoid writing custom actions wherever possible. Instead, explore the GitHub Marketplace to find pre-existing actions that fit your requirements. If a relevant action lacks a specific feature, consider contributing to its development rather than rebuilding the wheel. -Creating custom actions is time-consuming and rarely justified for small to medium-sized organizations. Leveraging existing solutions not only saves effort but also ensures better maintenance and community support. - -Code Linting and Quality -Linting is an essential practice for maintaining high-quality Terraform code. - -It helps ensure consistency, compliance with best practices, and early detection of potential issues. Well-linted code is easier to review, debug, and manage; therefore, it's an important stage in the development lifecycle. - -Let us have a look at linting best practices. - -Make sure no sensitive data has been committed to a repository. Use the tools listed below for that: -Gitleaks or Git-Secret CI jobs -Talisman pre-commit checks -Use Infracost to get an approximate breakdown of the cloud infrastructure costs. -Use linters like tflint or terraform validate to catch syntax errors and enforce Terraform best practices. -Format all code using terraform fmt and enforce this via pre-commit hooks. -To detect compliance and security issues, use the security linters listed below: -Checkov: An open-source static code analysis tool developed by Bridgecrew. It supports multiple IaC frameworks, including Terraform, CloudFormation, Kubernetes, Helm charts, and Dockerfiles. Checkov performs comprehensive scans to detect misconfigurations and policy violations, helping maintain adherence to security best practices. -Terrascan: Developed by Tenable, Terrascan is designed to detect compliance and security violations across various IaC frameworks, including Terraform, CloudFormation, ARM Templates, Kubernetes, Helm, Kustomize, and Dockerfiles. It uses the Open Policy Agent (OPA) for policy as code, allowing for customizable and extensive policy enforcement. -Trivy: Aqua Security's Trivy is a versatile security scanner that can detect vulnerabilities in container images, file systems, and Git repositories. It has Terraform support, which allows you to detect security vulnerabilities in your IaC configurations. Notably, tfsec, previously recommended for Terraform security scanning, has been incorporated into Trivy, combining their capabilities (tfsec#1994). -KICS (Keeping Infrastructure as Code Secure): Checkmarx developed KICS, which identifies security vulnerabilities, compliance issues, and infrastructure misconfigurations early in the development cycle. It supports multiple types of IaC frameworks, allowing you to secure your infrastructure before deployment. -To learn more about static code analysis tools, see "IaC Security Analysis: Checkov vs. tfsec vs. Terrascan – A Comparative Evaluation" and "Comparing Checkov vs. tfsec vs. Terrascan" articles. -Use Open Policy Agent (OPA). It is a policy-as-code engine that enables flexible and automated enforcement of compliance, security, and operational rules within Terraform IAC configurations. - -OPA uses the Rego language to establish custom policies, validate Terraform files, and plan outputs for misconfigurations. - -It works smoothly with CI/CD pipelines to guarantee policy compliance before deployment and includes pre-built policy libraries for typical use cases. OPA contributes to the maintenance of secure and consistent infrastructure standards by identifying concerns early on. -If you use GitLab, then use GitLab IaC Scanning. Infrastructure as code scanning is a built-in feature of the platform, available even on the free plan. It includes a preset CI/CD template that performs helpful static analysis tests on the IaC files in your project. -Use GitHub Super-Linter to maintain consistent code quality across multiple file types, such as YAML, JSON, Markdown, and Terraform. - -If Super-Linter is not an acceptable choice (e.g., you can have self-hosted Jenkins), try using yamllint or jsonlint to effectively validate and format YAML and JSON files. Example of a super-linter GitHub workflow: - -name: Super-Linter -on: [push, pull_request] -jobs: - lint: - runs-on: ubuntu-22.04 - steps: - - uses: actions/checkout@v3 - - uses: github/super-linter@v4 - env: - VALIDATE_TERRAFORM: true - VALIDATE_YAML: true - VALIDATE_JSON: true - VALIDATE_MARKDOWN: true -Developer-environment configuration -Now let's look at some of the key Developer-environment configurations. - -#1: Configure editor -Developers use a wide range of IDEs and text editors, from lightweight alternatives like Vim to powerful, full-featured IDEs like IntelliJ IDEA or VSCode. - -It is highly recommended to add a .editorconfig file in your repository to ensure uniform code formatting and standards across different development environments. - -The .editorconfig file is supported by most modern IDEs and text editors. - -By setting shared code conventions, teams can ensure that developers, regardless of the tools they use, follow the same formatting guidelines. This minimizes issues caused by inconsistent code formatting, streamlines collaboration, and promotes a cleaner codebase. - -Example: - -# EditorConfig is awesome: https://editorconfig.org - -# top-most EditorConfig file -root = true - -[*] -charset = utf-8 -end_of_line = lf -indent_size = 2 -indent_style = space -insert_final_newline = true -max_line_length = 80 -trim_trailing_whitespace = true - -[*.{tf,tfvars}] -indent_size = 2 -indent_style = space - -[*.md] -max_line_length = 0 -trim_trailing_whitespace = false - -[Makefile] -tab_width = 2 -indent_style = tab -[COMMIT_EDITMSG] -max_line_length = 0 -Aside from editorconfig, the second good option is to keep the configuration for VSCode, which will be used by the majority of developers in 2025. - -Example of .vscode/settings.json: - -{ - "editor.formatOnSave": true, - "terraform.format": { - "enable": true - }, - "editor.tabSize": 2 -} -Either is not a recommendation, but it is a good idea to specify all plugins that may be relevant during development. - -Example of .vscode/extensions.json: - -{ - "recommendations": [ - "hashicorp.terraform", - "redhat.vscode-yaml", - "streetsidesoftware.code-spell-checker" - ] -} -#2: Use a pre-commit framework -The pre-commit framework is a powerful tool for automating code quality checks and enforcing standards before changes are committed to your repository. It provides immediate feedback to developers, helping them catch and address issues in their personal workspace before committing the code. - -These hooks check the state of the code before committing and can stop the process if tests fail, ensuring that only high-quality code is pushed to the repository. - -To use it, you can create .pre-commit-config.yaml in the root of your repository, and run it manually before the commit via pre-commit run --all-files. - -The following example contains multiple best practices, which we have already examined in the article: - -# .pre-commit-config.yaml - -repos: - - repo: https://github.com/pre-commit/pre-commit-hooks - rev: v4.5.0 - hooks: - - id: check-yaml - - id: check-json - - id: trailing-whitespace - args: ["--markdown-linebreak-ext=md"] - - id: check-added-large-files - - id: check-executables-have-shebangs - - id: check-shebang-scripts-are-executable - - id: check-merge-conflict - - id: check-vcs-permalinks - - id: detect-private-key - - id: detect-aws-credentials - args: ["--allow-missing-credentials"] - - id: end-of-file-fixer - - id: no-commit-to-branch - - id: pretty-format-json - args: - - --autofix - - repo: https://github.com/zricethezav/gitleaks - rev: v8.18.2 - hooks: - - id: gitleaks - - repo: https://github.com/bridgecrewio/checkov.git - rev: 3.2.43 - hooks: - - id: checkov - args: - - --config-file - - .checkov.yaml - - repo: https://github.com/antonbabenko/pre-commit-terraform - rev: v1.88.3 - hooks: - - id: terraform_fmt - - id: terraform_validate - args: - - --hook-config=--retry-once-with-cleanup=true - - id: terraform_docs - args: - - --args=--config=.terraform-docs.yaml - - id: terraform_tflint - args: - - --args=--config=__GIT_WORKING_DIR__/.tflint.hcl -You may use the AWS Terraform template repository as a starting point for developing your pre-commit configuration. - -It offers a large number of standard checks that you can apply to your project. Furthermore, pay attention to these two repositories that provide common pre-commit hooks for Terraform/OpenTofu: - -Pre-commit hooks for Terraform -Pre-commit hooks for OpenTofu -Terraform-module template pattern -Previously in this post, we explored several guidelines relevant to most Terraform modules. - -However, managing tens or hundreds of modules distributed across multiple repositories makes it practically impossible to keep all configurations, CI processes, and linting rules up to date. Using a copy-pasting method is neither efficient nor sustainable. - -To make a maintenance task easier, use a GitHub/GitLab repository or GitHub fork methodology for your Terraform modules. This repository can contain all of the essential CI pipelines, linters, tools, and code configurations, which are implemented as a common template for your Terraform modules. - -In my projects, I use both of these approaches under the moniker "Terraform Module Template Repository". This repository allows me to constantly keep my configurations up to date and deliver updates rapidly. - -Furthermore, users can quickly build a new module by clicking via "Use this template button" within the template repository. - -Before we go any further, I'd want to discuss the differences between a fork and a template repository, which will be used as a template for future repositories. - -If you use the GitHub template, it may be difficult to transmit updates from the parent repository to the child repository because they are technically independent. - -In the case of forks, the repositories are linked, but I do not suggest backmerging, because you may spend a lot of time at merge conflicts. - -In the scope of both methods, I advocate implementing a custom CI job in the template repository that will distribute updates to children via easy Git and bash commands manually or by CRON. We'll look at an example of such a job in Part 2 of this article series. - -Returning to the "Terraform Module Template Repository" approach, it's important to note that applying it for personal projects or smaller-scale infrastructure may be overkill due to the amount of required work (believe me!). - -The upfront costs of creating and maintaining a reusable, parameterized template may exceed the benefits. If you only have a few repositories, it can be much easier to copy-paste configurations, without implementing an additional automation. - -Finally, if you expect to manage dozens or hundreds of modules, you should create a Terraform Module Template Repository from the ground up at the beginning stage of project development. - -For smaller-scale cases, you can look for open-source alternatives (one of which will be discussed in Part 2 of this series), or just copy and paste your preferred configurations between Terraform modules. - -Conclusion -Thank you for reading! I hope you found this guide helpful. Stay tuned for Part 2, where I will cover: - -A ready-to-use Terraform module template that implements the majority of best practices outlined in this article. -An example of CI jobs that deliver updates from the template repository to their children repositories. -Practical steps to adopt a ready-to-use module template for your organization in 5 minutes -More code samples, as well as tips and advice based on projects that have already used the template repository and other best practices outlined in this article. diff --git a/.github/skills/terraform-provider-pr-review/references/providers.mdx b/.github/skills/terraform-provider-pr-review/references/providers.mdx deleted file mode 100644 index 5bd39d2c21..0000000000 --- a/.github/skills/terraform-provider-pr-review/references/providers.mdx +++ /dev/null @@ -1,367 +0,0 @@ ---- -page_title: Providers Within Modules - Configuration Language -description: >- - Use providers within Terraform modules. Learn about version constraints, aliases, implicit inheritance, and passing providers to Terraform modules. -# START AUTO GENERATED METADATA, DO NOT EDIT -created_at: 2025-11-19T13:27:46Z -last_modified: 2025-11-19T13:27:46Z -# END AUTO GENERATED METADATA ---- - -# Providers Within Modules - -[inpage-providers]: #providers-within-modules - -In a configuration with multiple modules, there are some special considerations -for how resources are associated with provider configurations. - -Each resource in the configuration must be associated with one provider -configuration. Provider configurations, unlike most other concepts in -Terraform, are global to an entire Terraform configuration and can be shared -across module boundaries. Provider configurations can be defined only in a -root Terraform module. - -Providers can be passed down to descendant modules in two ways: either -_implicitly_ through inheritance, or _explicitly_ via the `providers` argument -within a `module` block. These two options are discussed in more detail in the -following sections. - -A module intended to be called by one or more other modules must not contain -any `provider` blocks. A module containing its own provider configurations is -not compatible with the `for_each`, `count`, and `depends_on` arguments that -were introduced in Terraform v0.13. For more information, see -[Legacy Shared Modules with Provider Configurations](#legacy-shared-modules-with-provider-configurations). - -Provider configurations are used for all operations on associated resources, -including destroying remote objects and refreshing state. Terraform retains, as -part of its state, a reference to the provider configuration that was most -recently used to apply changes to each resource. When a `resource` block is -removed from the configuration, this record in the state will be used to locate -the appropriate configuration because the resource's `provider` argument -(if any) will no longer be present in the configuration. - -As a consequence, you must ensure that all resources that belong to a -particular provider configuration are destroyed before you can remove that -provider configuration's block from your configuration. If Terraform finds -a resource instance tracked in the state whose provider configuration block is -no longer available then it will return an error during planning, prompting you -to reintroduce the provider configuration. - -## Provider Version Constraints in Modules - -Although provider _configurations_ are shared between modules, each module must -declare its own [provider requirements](/terraform/language/providers/requirements), so that -Terraform can ensure that there is a single version of the provider that is -compatible with all modules in the configuration and to specify the -[source address](/terraform/language/providers/requirements#source-addresses) that serves as -the global (module-agnostic) identifier for a provider. - -To declare that a module requires particular versions of a specific provider, -use a `required_providers` block inside a `terraform` block: - -```hcl -terraform { - required_providers { - aws = { - source = "hashicorp/aws" - version = ">= 2.7.0" - } - } -} -``` - -A provider requirement says, for example, "This module requires version v2.7.0 -of the provider `hashicorp/aws` and will refer to it as `aws`." It doesn't, -however, specify any of the configuration settings that determine what remote -endpoints the provider will access, such as an AWS region; configuration -settings come from provider _configurations_, and a particular overall Terraform -configuration can potentially have -[several different configurations for the same provider](/terraform/language/block/provider#select-an-alternate-provider-configuration). - -## Provider Aliases Within Modules - -To declare multiple configuration names for a provider within a module, add the -`configuration_aliases` argument: - -```hcl -terraform { - required_providers { - aws = { - source = "hashicorp/aws" - version = ">= 2.7.0" - configuration_aliases = [ aws.alternate ] - } - } -} -``` - -The above requirements are identical to the previous, with the addition of the -alias provider configuration name `aws.alternate`, which can be referenced by -resources using the `provider` argument. - -If you are writing a shared Terraform module, constrain only the minimum -required provider version using a `>=` constraint. This should specify the -minimum version containing the features your module relies on, and thus allow a -user of your module to potentially select a newer provider version if other -features are needed by other parts of their overall configuration. - -## Implicit Provider Inheritance - -For convenience in simple configurations, a child module automatically inherits -[default provider configurations](/terraform/language/block/provider#alias) from its parent. This means that -explicit `provider` blocks appear only in the root module, and downstream -modules can simply declare resources for that provider and have them -automatically associated with the root provider configurations. - -For example, the root module might contain only a `provider` block and a -`module` block to instantiate a child module: - -```hcl -provider "aws" { - region = "us-west-1" -} - -module "child" { - source = "./child" -} -``` - -The child module can then use any resource from this provider with no further -provider configuration required: - -```hcl -resource "aws_s3_bucket" "example" { - bucket = "provider-inherit-example" -} -``` - -We recommend using this approach when a single configuration for each provider -is sufficient for an entire configuration. - -~> **Note:** Only provider configurations are inherited by child modules, not provider source or version requirements. Each module must [declare its own provider requirements](/terraform/language/providers/requirements). This is especially important for non-HashiCorp providers. - -In more complex situations there may be -[multiple provider configurations](/terraform/language/block/provider#using-an-alternate-provider-configuration), -or a child module may need to use different provider settings than -its parent. For such situations, you must pass providers explicitly. - -## Passing Providers Explicitly - -When child modules each need a different configuration of a particular -provider, or where the child module requires a different provider configuration -than its parent, you can use the `providers` argument within a `module` block -to explicitly define which provider configurations are available to the -child module. For example: - -```hcl -# The default "aws" configuration is used for AWS resources in the root -# module where no explicit provider instance is selected. -provider "aws" { - region = "us-west-1" -} - -# An alternate configuration is also defined for a different -# region, using the alias "usw2". -provider "aws" { - alias = "usw2" - region = "us-west-2" -} - -# An example child module is instantiated with the alternate configuration, -# so any AWS resources it defines will use the us-west-2 region. -module "example" { - source = "./example" - providers = { - aws = aws.usw2 - } -} -``` - -The `providers` argument within a `module` block is similar to -[the `provider` argument](/terraform/language/block/resource#provider) -within a resource, but is a map rather than a single string because a module may -contain resources from many different providers. - -The keys of the `providers` map are provider configuration names as expected by -the child module, and the values are the names of corresponding configurations -in the _current_ module. - -Modules implicitly inherit default providers. Setting a `providers` argument within a `module` block overrides the default inheritance behavior for that provider. - -Additional provider configurations (those with the `alias` argument set) are -_never_ inherited automatically by child modules, and so must always be passed -explicitly using the `providers` map. For example, a module -that configures connectivity between networks in two AWS regions is likely -to need both a source and a destination region. In that case, the root module -may look something like this: - -```hcl -provider "aws" { - alias = "usw1" - region = "us-west-1" -} - -provider "aws" { - alias = "usw2" - region = "us-west-2" -} - -module "tunnel" { - source = "./tunnel" - providers = { - aws.src = aws.usw1 - aws.dst = aws.usw2 - } -} -``` - -The subdirectory `./tunnel` must then declare the configuration aliases for the -provider so the calling module can pass configurations with these names in its `providers` argument: - -```hcl -terraform { - required_providers { - aws = { - source = "hashicorp/aws" - version = ">= 2.7.0" - configuration_aliases = [ aws.src, aws.dst ] - } - } -} -``` - -Each resource should then have its own `provider` attribute set to either -`aws.src` or `aws.dst` to choose which of the two provider configurations to -use. - -## Legacy Shared Modules with Provider Configurations - -In Terraform v0.10 and earlier there was no explicit way to use different -configurations of a provider in different modules in the same configuration, -and so module authors commonly worked around this by writing `provider` blocks -directly inside their modules, making the module have its own separate -provider configurations separate from those declared in the root module. - -However, that pattern had a significant drawback: because a provider -configuration is required to destroy the remote object associated with a -resource instance as well as to create or update it, a provider configuration -must always stay present in the overall Terraform configuration for longer -than all of the resources it manages. If a particular module includes -both resources and the provider configurations for those resources then -removing the module from its caller would violate that constraint: both the -resources and their associated providers would, in effect, be removed -simultaneously. - -Terraform v0.11 introduced the mechanisms described in earlier sections to -allow passing provider configurations between modules in a structured way, and -thus we explicitly recommended against writing a child module with its own -provider configuration blocks. However, that legacy pattern continued to work -for compatibility purposes -- though with the same drawback -- until Terraform -v0.13. - -Terraform v0.13 introduced the possibility for a module itself to use the -`for_each`, `count`, and `depends_on` arguments, but the implementation of -those unfortunately conflicted with the support for the legacy pattern. - -To retain the backward compatibility as much as possible, Terraform v0.13 -continues to support the legacy pattern for module blocks that do not use these -new features, but a module with its own provider configurations is not -compatible with `for_each`, `count`, or `depends_on`. Terraform will produce an -error if you attempt to combine these features. For example: - -``` -Error: Module does not support count - - on main.tf line 15, in module "child": - 15: count = 2 - -Module "child" cannot be used with count because it contains a nested provider -configuration for "aws", at child/main.tf:2,10-15. - -This module can be made compatible with count by changing it to receive all of -its provider configurations from the calling module, by using the "providers" -argument in the calling module block. -``` - -To make a module compatible with the new features, you must remove all of the -`provider` blocks from its definition. - -If the new version of the module declares `configuration_aliases`, or if the -calling module needs the child module to use different provider configurations -than its own default provider configurations, the calling module must then -include an explicit `providers` argument to describe which provider -configurations the child module will use: - -```hcl -provider "aws" { - region = "us-west-1" -} - -provider "aws" { - region = "us-east-1" - alias = "east" -} - -module "child" { - count = 2 - providers = { - # By default, the child module would use the - # default (unaliased) AWS provider configuration - # using us-west-1, but this will override it - # to use the additional "east" configuration - # for its resources instead. - aws = aws.east - } -} -``` - -Since the association between resources and provider configurations is -static, module calls using `for_each` or `count` cannot pass different -provider configurations to different instances. If you need different -instances of your module to use different provider configurations then you -must use a separate `module` block for each distinct set of provider -configurations: - -```hcl -provider "aws" { - alias = "usw1" - region = "us-west-1" -} - -provider "aws" { - alias = "usw2" - region = "us-west-2" -} - -provider "google" { - alias = "usw1" - credentials = file("account.json") - project = "my-project-id" - region = "us-west1" - zone = "us-west1-a" -} - -provider "google" { - alias = "usw2" - credentials = file("account.json") - project = "my-project-id" - region = "us-west2" - zone = "us-west2-a" -} - -module "bucket_w1" { - source = "./publish_bucket" - providers = { - aws.src = aws.usw1 - google.src = google.usw1 - } -} - -module "bucket_w2" { - source = "./publish_bucket" - providers = { - aws.src = aws.usw2 - google.src = google.usw2 - } -} -``` diff --git a/.github/skills/terraform-provider-pr-review/references/publish.mdx b/.github/skills/terraform-provider-pr-review/references/publish.mdx deleted file mode 100644 index 5d83b6665d..0000000000 --- a/.github/skills/terraform-provider-pr-review/references/publish.mdx +++ /dev/null @@ -1,45 +0,0 @@ ---- -page_title: Publishing Modules -description: A module is a container for multiple resources that are used together. -# START AUTO GENERATED METADATA, DO NOT EDIT -created_at: 2025-11-19T13:27:46Z -last_modified: 2025-11-19T13:27:46Z -# END AUTO GENERATED METADATA ---- - -# Publishing Modules - -If you've built a module that you intend to be reused, we recommend -[publishing the module](/terraform/registry/modules/publish) on the -[Terraform Registry](https://registry.terraform.io). This will version -your module, generate documentation, and more. - -Published modules can be easily consumed by Terraform, and users can -[constrain module versions](/terraform/language/modules/syntax#version) -for safe and predictable updates. The following example shows how a caller -might use a module from the Terraform Registry: - -```hcl -module "consul" { - source = "hashicorp/consul/aws" -} -``` - -If you do not wish to publish your modules in the public registry, you can -instead use a [private registry](/terraform/registry/private) to get -the same benefits. - -We welcome contributions of Terraform modules from our community members, partners, and customers. Our ecosystem is made richer by each new module created or an existing one updated, as they reflect the wide range of experience and technical requirements of the community that uses them. Our cloud provider partners often seek to develop specific modules for popular or challenging use cases on their platform and utilize them as valuable learning experiences to empathize with their users. Similarly, our community module developers incorporate a variety of opinions and use cases from the broader Terraform community. Both types of modules have their place in the Terraform registry, accessible to practitioners who can decide which modules best fit their requirements. - -## Distribution via other sources - -Although the registry is the native mechanism for distributing re-usable -modules, Terraform can also install modules from -[various other sources](/terraform/language/modules/sources). The alternative sources -do not support the first-class versioning mechanism, but some sources have -their own mechanisms for selecting particular VCS commits, etc. - -We recommend that modules distributed via other protocols still use the -[standard module structure](/terraform/language/modules/develop/structure) so that they can -be used in a similar way as a registry module or be published on the registry -at a later time. diff --git a/.github/skills/terraform-provider-pr-review/references/refactoring.mdx b/.github/skills/terraform-provider-pr-review/references/refactoring.mdx deleted file mode 100644 index b1f0782cac..0000000000 --- a/.github/skills/terraform-provider-pr-review/references/refactoring.mdx +++ /dev/null @@ -1,448 +0,0 @@ ---- -page_title: Refactor modules -description: How to make backward-compatible changes to modules already in use. -# START AUTO GENERATED METADATA, DO NOT EDIT -created_at: 2025-11-19T13:27:46Z -last_modified: 2025-12-02T06:26:00-08:00 -# END AUTO GENERATED METADATA ---- - -# Refactor modules - -By default, Terraform interprets a change as an instruction to destroy the existing resource and create a new resource at the new address. You can use a `moved` block to update a resource address without destroying it. - - -> **Hands On:** Try the [Use Configuration to Move Resources](/terraform/tutorials/configuration-language/move-config) tutorial. - -## Overview - -Add a `moved` block to your configuration to move or rename an object. Before creating a new plan for the resource specified in the `to` argument, Terraform checks the state for an existing resource at the `from` address. Terraform includes the update to the resource address in the next execution plan. The address change does not destroy the resource. - -You can use `moved` blocks for the following use cases: - -- [Move a resource or module](#move-a-resource-or-module) -- [Rename a resource](#rename-a-resource) -- [Create multiple instances](#create-multiple-instances) -- [Rename a module call](#rename-a-module-call) -- [Split a module](#split-a-module) - -When the module or resource is no longer necessary, you can [remove `moved` blocks](#remove-a-moved-block) from the configuration. Note that removing a `moved` block is a breaking change. - -## Requirements - -Terraform v1.1 and later is required to use `moved` blocks to explicitly refactor module addresses. Instead, use the [`terraform state mv` CLI command](/terraform/cli/commands/state/mv). - -## Move a resource or module - -Add a [`moved` block](/terraform/language/block/moved) to your configuration and specify the following arguments: - -- `from`: The previous address of the module or resource. -- `to`: The new address of the module or resource. - -In the following example, the resource named `aws_instance.a` has been moved to `aws_instance.b`: - -```hcl -moved { - from = aws_instance.a - to = aws_instance.b -} -``` - -Before creating a new plan for `aws_instance.b`, Terraform first checks -whether there is an existing object for `aws_instance.a` recorded in the state. -If there is an existing object, Terraform renames that object to -`aws_instance.b` and then proceeds with creating a plan. The resulting plan is -as if the object had originally been created at `aws_instance.b`, avoiding any -need to destroy it during apply. - -The `from` and `to` addresses both use a special addressing syntax that allows -selecting modules, resources, and resources inside child modules. - -## Rename a resource - -Consider this example module with a resource configuration: - -```hcl -resource "aws_instance" "a" { - count = 2 - - # (resource-type-specific configuration) -} -``` - -Applying this configuration for the first time would cause Terraform to -create `aws_instance.a[0]` and `aws_instance.a[1]`. - -If you later choose a different name for this resource, then you can change the -name label in the `resource` block and record the old name inside a `moved` block: - -```hcl -resource "aws_instance" "b" { - count = 2 - - # (resource-type-specific configuration) -} - -moved { - from = aws_instance.a - to = aws_instance.b -} -``` - -When creating the next plan for each configuration using this module, Terraform -treats any existing objects belonging to `aws_instance.a` as if they had -been created for `aws_instance.b`: `aws_instance.a[0]` will be treated as -`aws_instance.b[0]`, and `aws_instance.a[1]` as `aws_instance.b[1]`. - -New instances of the module, which _never_ had an -`aws_instance.a`, will ignore the `moved` block and propose to create -`aws_instance.b[0]` and `aws_instance.b[1]` as normal. - -Both of the addresses in this example referred to a resource as a whole, and -so Terraform recognizes the move for all instances of the resource. That is, -it covers both `aws_instance.a[0]` and `aws_instance.a[1]` without the need -to identify each one separately. - -Each resource type has a separate schema so objects of different types -are not typically compatible. You can always use the `moved` block to change -the name of a resource, but some providers also let you change an object from -one resource type to another. Refer to the provider documentation for details -on which resources can move between types. You _cannot_ use the `moved` -block to change a managed resource (a `resource` block) into a data -resource (a `data` block). - -## Create multiple instances - -You can use a `moved` block when creating multiple instances of a resource or module call while preserving the original instance. - -### Enable `count` or `for_each` for a resource - -The following example module contains a single-instance resource bound to the address `aws_instance.a`: - -```hcl -resource "aws_instance" "a" { - # (resource-type-specific configuration) -} -``` - -Later, you use [`for_each`](/terraform/language/meta-arguments#for_each) with this -resource to systematically declare multiple instances. To preserve an object -that was previously associated with `aws_instance.a`, you must add a -`moved` block to specify which instance key the object will take in the new -configuration. - -In the following example, Terraform does not plan to destroy any existing object at -`aws_instance.a`. Instead, Terraform treats that object as if it were originally -created as `aws_instance.a["small"]`: - -```hcl -locals { - instances = tomap({ - big = { - instance_type = "m3.large" - } - small = { - instance_type = "t2.medium" - } - }) -} - -resource "aws_instance" "a" { - for_each = local.instances - - instance_type = each.value.instance_type - # (other resource-type-specific configuration) -} - -moved { - from = aws_instance.a - to = aws_instance.a["small"] -} -``` - - -When at least one of the two addresses includes an instance key, such as -`["small"]` in the previous example, Terraform understands both addresses as -referring to specific instances of a resource rather than the resource as a -whole. That means you can use `moved` to switch between keys and to add and -remove keys as you switch between `count`, `for_each`, or neither. - -The following examples are also valid `moved` blocks that record -changes to resource instance keys in a similar way: - -```hcl -# Both old and new configuration used "for_each", but the -# "small" element was renamed to "tiny". -moved { - from = aws_instance.b["small"] - to = aws_instance.b["tiny"] -} - -# The old configuration used "count" and the new configuration -# uses "for_each", with the following mappings from -# index to key: -moved { - from = aws_instance.c[0] - to = aws_instance.c["small"] -} -moved { - from = aws_instance.c[1] - to = aws_instance.c["tiny"] -} - -# The old configuration used "count", and the new configuration -# uses neither "count" nor "for_each", and you want to keep -# only the object at index 2. -moved { - from = aws_instance.d[2] - to = aws_instance.d -} -``` - -When you add `count` to an existing resource that didn't previously have the argument, -Terraform automatically proposes moving the original object to instance `0` -unless you write a `moved` block that explicitly mentions that resource. -However, we recommend writing out the corresponding `moved` block -explicitly to make the change clearer to future readers of the module. - -### Enable `count` or `for_each` for a module call - -The following example is a single-instance module that creates objects whose -addresses begin with `module.a`: - - -```hcl -module "a" { - source = "../modules/example" - - # (module arguments) -} -``` - -In later module versions, you may need to use -[`count`](/terraform/language/meta-arguments#count) with this resource to systematically -declare multiple instances. To preserve an object that was previously associated -with `aws_instance.a` alone, you can add a `moved` block to specify which -instance key that object will take in the new configuration. - -The following configuration above directs Terraform to treat all objects in `module.a` as -if they were originally created in `module.a[2]`. As a result, Terraform plans -to create new objects only for `module.a[0]` and `module.a[1]`: - -```hcl -module "a" { - source = "../modules/example" - count = 3 - - # (module arguments) -} - -moved { - from = module.a - to = module.a[2] -} -``` - -When at least one of the two addresses includes an instance key, such as -`[2]` in the previous example, Terraform understands both addresses as -referring to specific instances of a module call rather than the module -call as a whole. That means you can use `moved` to switch between keys and to -add and remove keys as you switch between `count`, `for_each`, or neither. - -## Rename a module call - -You can rename a call to a module in a similar way as renaming a resource. -Consider the following original module version: - -```hcl -module "a" { - source = "../modules/example" - - # (module arguments) -} -``` - -When applying this configuration, Terraform would prefix the addresses for -any resources declared in this module with the module path `module.a`. -For example, a resource `aws_instance.example` would have the full address -`module.a.aws_instance.example`. - -If you later choose a better name for this module call, then you can change the -name label in the `module` block and record the old name inside a `moved` block: - -```hcl -module "b" { - source = "../modules/example" - - # (module arguments) -} - -moved { - from = module.a - to = module.b -} -``` - -When creating the next plan for each configuration using this module, Terraform -will treat any existing object addresses beginning with `module.a` as if -they had instead been created in `module.b`. `module.a.aws_instance.example` -would be treated as `module.b.aws_instance.example`. - -Both of the addresses in this example referred to a module call as a whole, and -so Terraform recognizes the move for all instances of the call. If this -module call used `count` or `for_each` then it would apply to all of the -instances, without the need to specify each one separately. - - - -## Split a module - -As a module grows to support new requirements, it might eventually grow big -enough to warrant splitting into two separate modules. - -Consider this example module: - -```hcl -resource "aws_instance" "a" { - # (other resource-type-specific configuration) -} - -resource "aws_instance" "b" { - # (other resource-type-specific configuration) -} - -resource "aws_instance" "c" { - # (other resource-type-specific configuration) -} -``` - -You can split this into two modules as follows: - -* `aws_instance.a` now belongs to module "x". -* `aws_instance.b` also belongs to module "x". -* `aws_instance.c` belongs module "y". - -To achieve this refactoring without replacing existing objects bound to the -old resource addresses, you must: - -1. Write module "x", copying over the two resources it should contain. -1. Write module "y", copying over the one resource it should contain. -1. Edit the original module to no longer include any of these resources, and - instead to contain only shim configuration to migrate existing users. - -The new modules "x" and "y" should contain only `resource` blocks: - -```hcl -# module "x" - -resource "aws_instance" "a" { - # (other resource-type-specific configuration) -} - -resource "aws_instance" "b" { - # (other resource-type-specific configuration) -} -``` - -```hcl -# module "y" - -resource "aws_instance" "c" { - # (other resource-type-specific configuration) -} -``` - -The original module, now only a shim for backward-compatibility, calls the -two new modules and indicates that the resources moved into them: - -```hcl -module "x" { - source = "../modules/x" - - # ... -} - -module "y" { - source = "../modules/y" - - # ... -} - -moved { - from = aws_instance.a - to = module.x.aws_instance.a -} - -moved { - from = aws_instance.b - to = module.x.aws_instance.b -} - -moved { - from = aws_instance.c - to = module.y.aws_instance.c -} -``` - -When an existing user of the original module upgrades to the new "shim" -version, Terraform notices these three `moved` blocks and behaves -as if the objects associated with the three old resource addresses were -originally created inside the two new modules. - -New users of this family of modules may use either the combined shim module -_or_ the two new modules separately. You may wish to communicate to your -existing users that the old module is now deprecated and so they should use -the two separate modules for any new needs. - -The multi-module refactoring situation is unusual in that it violates the -typical rule that a parent module sees its child module as a "closed box", -unaware of exactly which resources are declared inside it. This compromise -assumes that all three of these modules are maintained by the same people -and distributed together in a single -[module package](/terraform/language/modules/sources#modules-in-package-sub-directories). - -Terraform resolves module references in `moved` blocks relative to the module -instance they are defined in. For example, if the original module above were -already a child module named `module.original`, the reference to -`module.x.aws_instance.a` would resolve as -`module.original.module.x.aws_instance.a`. A module may only make `moved` -statements about its own objects and objects of its child modules. - -If you need to refer to resources within a module that was called using -`count` or `for_each` meta-arguments, you must specify a specific instance -key to use in order to match with the new location of the resource -configuration: - -```hcl -moved { - from = aws_instance.example - to = module.new[2].aws_instance.example -} -``` - -## Remove a `moved` block - -Over time, a long-lasting module may accumulate many `moved` blocks. - -Removing a `moved` block is a breaking change because any configurations that refer to the old address will plan to delete the existing object instead of move it. We strongly recommend that you retain all historical `moved` blocks from earlier versions of your modules to preserve the upgrade path for users of any previous version. - -If you do decide to remove `moved` blocks, proceed with caution. It can be safe to remove `moved` blocks when you are maintaining private modules within an organization and you are certain that all users have successfully run `terraform apply` with your new module version. - -If you need to rename or move the same object twice, we recommend chaining `moved` blocks to document the full change history: - -```hcl -moved { - from = aws_instance.a - to = aws_instance.b -} - -moved { - from = aws_instance.b - to = aws_instance.c -} -``` - -Recording a sequence of moves in this way allows for successful upgrades for -both configurations with objects at `aws_instance.a` and configurations with -objects at `aws_instance.b`. In both cases, Terraform treats the existing -object as if it had been originally created as `aws_instance.c`. diff --git a/.github/skills/terraform-provider-pr-review/references/review-checklist.md b/.github/skills/terraform-provider-pr-review/references/review-checklist.md deleted file mode 100644 index efa19e4421..0000000000 --- a/.github/skills/terraform-provider-pr-review/references/review-checklist.md +++ /dev/null @@ -1,77 +0,0 @@ -# Terraform Provider Review Checklist - -Use this checklist when reviewing PRs in `terraform-provider-github`. - -ALWAYS acknowledge you are using this checklist when reviewing a PR! - -## 1. Correctness and Behavior - -- Verify CRUD/read logic correctly maps GitHub API responses to Terraform schema/state. -- Check for nil handling, default-value drift, and state flattening/expansion mismatches. -- Confirm update paths do not accidentally force replacement or wipe optional fields. -- Validate retry/backoff and error classification for API failures. -- **Do not flag** create/update functions that return `nil` instead of calling the read function. This provider intentionally avoids read-after-write to minimize API calls against GitHub's rate limits and to avoid stale reads from eventually-consistent endpoints (see [#2892](https://github.com/integrations/terraform-provider-github/issues/2892)). - -## 2. Schema and State Compatibility - -- Any `schema.Schema` changes (`Type`, `Optional`, `Required`, `Computed`, `ForceNew`, `Default`) can change user behavior. -- Confirm imports still work and read functions produce stable state. -- Flag any behavior that may break existing state without migration notes/tests. -- Watch for attribute rename/removal without deprecation path. -- Adding an optional attribute with a default is backward-compatible; removing or renaming an attribute is a breaking change that needs a deprecation cycle. -- New or changed attributes should include `ValidateFunc`/`ValidateDiagFunc` to catch invalid values at plan time rather than at apply time. -- All attributes should have a `Description` string for documentation generation. -- When resources are renamed or restructured, check for `moved` block guidance or state migration documentation so existing users don't face resource destruction on upgrade. -- Mark attributes that hold secrets with `Sensitive: true` in the schema to prevent leaking values in plan output and state. - -## 3. Terraform UX and Drift - -- Ensure diff suppression, normalization, and API canonicalization avoid perpetual diffs. -- Check that empty vs null handling is intentional. -- Verify list/set ordering behavior and deterministic state output. - -## 4. Testing Expectations - -- For behavior changes, check matching tests under `github/*_test.go`. -- Prefer acceptance tests for API-facing changes (`TF_ACC=1` scenarios). -- Ensure tests assert the bugfix/regression target, not only happy path. -- Flag missing tests when logic changed but test coverage did not. - -## 5. Docs and Examples - -- If resource/data source behavior changed, review website docs updates under `website/`. -- If user workflow changed, review corresponding example updates under `examples/`. -- Confirm examples still reflect current schema and argument names. -- Example directories should follow standard module structure (`main.tf`, `variables.tf`, `outputs.tf`) with a `README.md` explaining purpose and usage. -- Example configurations should not embed `provider` blocks inside child modules; provider configuration belongs in root modules only. -- Variables and outputs in examples should include `description` and `type`. -- If a PR adds or changes a resource, verify there is at least one example showing typical usage. - -## 6. Security and Permissions - -- Verify sensitive values are not logged or accidentally exposed in state. -- Check token/credential handling and least-privilege assumptions. -- Ensure permission scope requirements are documented for new API calls. -- Confirm no secrets or credentials are hardcoded in example configurations. -- Verify that debug/trace logging does not print sensitive attribute values. -- Sensitive outputs should be marked `sensitive = true` so Terraform redacts them in CLI output. - -## 7. Performance and API Safety - -- Flag new N+1 patterns, excessive API calls, or missing pagination handling. -- Check for rate-limit-sensitive loops and absent caching where needed. -- Confirm context cancellation/timeouts are respected in long operations. - -## 8. Versioning and Changelog - -- Breaking changes (attribute removal/rename, forced replacement, new required fields) must be called out for a MAJOR version bump under semantic versioning. -- Backward-compatible additions (new optional attributes with defaults, new resources/data sources) correspond to MINOR version bumps. -- Bug fixes with no schema change correspond to PATCH version bumps. -- Verify the PR description or CHANGELOG includes a clear summary of what changed and the user impact. - -## 9. Review Report Rules - -- Report findings first, ordered by severity. -- Include precise file references like `github/resource_x.go:123`. -- Explain impact in Terraform terms: plan/apply behavior, drift, state compatibility. -- Mention residual risks if tests or docs are incomplete. diff --git a/.github/skills/terraform-provider-pr-review/references/structure.mdx b/.github/skills/terraform-provider-pr-review/references/structure.mdx deleted file mode 100644 index 7bedb69dff..0000000000 --- a/.github/skills/terraform-provider-pr-review/references/structure.mdx +++ /dev/null @@ -1,133 +0,0 @@ ---- -page_title: Standard Module Structure -description: >- - Learn about the recommended file and directory structure for developing reusable modules distributed as separate repositories. -# START AUTO GENERATED METADATA, DO NOT EDIT -created_at: 2025-11-19T13:27:46Z -last_modified: 2025-11-19T13:27:46Z -# END AUTO GENERATED METADATA ---- - -# Standard Module Structure - -The standard module structure is a file and directory layout we recommend for -reusable modules distributed in separate repositories. Terraform tooling is -built to understand the standard module structure and use that structure to -generate documentation, index modules for the module registry, and more. - -The standard module structure expects the layout documented below. The list may -appear long, but everything is optional except for the root module. Most modules -don't need to do any extra work to follow the standard structure. - -* **Root module**. This is the **only required element** for the standard - module structure. Terraform files must exist in the root directory of - the repository. This should be the primary entrypoint for the module and is - expected to be opinionated. For the - [Consul module](https://registry.terraform.io/modules/hashicorp/consul) - the root module sets up a complete Consul cluster. It makes a lot of assumptions - however, and we expect that advanced users will use specific _nested modules_ - to more carefully control what they want. - -* **README**. The root module and any nested modules should have README - files. This file should be named `README` or `README.md`. The latter will - be treated as markdown. There should be a description of the module and - what it should be used for. If you want to include an example for how this - module can be used in combination with other resources, put it in an [examples - directory like this](https://github.com/hashicorp/terraform-aws-consul/tree/master/examples). - Consider including a visual diagram depicting the infrastructure resources - the module may create and their relationship. - - The README doesn't need to document inputs or outputs of the module because - tooling will automatically generate this. If you are linking to a file or - embedding an image contained in the repository itself, use a commit-specific - absolute URL so the link won't point to the wrong version of a resource in the - future. - -* **LICENSE**. The license under which this module is available. If you are - publishing a module publicly, many organizations will not adopt a module - unless a clear license is present. We recommend always having a license - file, even if it is not an open source license. - -* **`main.tf`, `variables.tf`, `outputs.tf`**. These are the recommended filenames for - a minimal module, even if they're empty. `main.tf` should be the primary - entrypoint. For a simple module, this may be where all the resources are - created. For a complex module, resource creation may be split into multiple - files but any nested module calls should be in the main file. `variables.tf` - and `outputs.tf` should contain the declarations for variables and outputs, - respectively. - -* **Variables and outputs should have descriptions.** All variables and - outputs should have one or two sentence descriptions that explain their - purpose. This is used for documentation. See the documentation for - [variable configuration](/terraform/language/block/variable) and - [output configuration](/terraform/language/block/output) for more details. - -* **Nested modules**. Nested modules should exist under the `modules/` - subdirectory. Any nested module with a `README.md` is considered usable - by an external user. If a README doesn't exist, it is considered for internal - use only. These are purely advisory; Terraform will not actively deny usage - of internal modules. Nested modules should be used to split complex behavior - into multiple small modules that advanced users can carefully pick and - choose. For example, the - [Consul module](https://registry.terraform.io/modules/hashicorp/consul) - has a nested module for creating the Cluster that is separate from the - module to setup necessary IAM policies. This allows a user to bring in their - own IAM policy choices. - - If the root module includes calls to nested modules, they should use relative - paths like `./modules/consul-cluster` so that Terraform will consider them - to be part of the same repository or package, rather than downloading them - again separately. - - If a repository or package contains multiple nested modules, they should - ideally be [composable](/terraform/language/modules/develop/composition) by the caller, rather than - calling directly to each other and creating a deeply-nested tree of modules. - -* **Examples**. Examples of using the module should exist under the - `examples/` subdirectory at the root of the repository. Each example may have - a README to explain the goal and usage of the example. Examples for - submodules should also be placed in the root `examples/` directory. - - Because examples will often be copied into other repositories for - customization, any `module` blocks should have their `source` set to the - address an external caller would use, not to a relative path. - -A minimal recommended module following the standard structure is shown below. -While the root module is the only required element, we recommend the structure -below as the minimum: - -```sh -$ tree minimal-module/ -. -├── README.md -├── main.tf -├── variables.tf -├── outputs.tf -``` - -A complete example of a module following the standard structure is shown below. -This example includes all optional elements and is therefore the most -complex a module can become: - -```sh -$ tree complete-module/ -. -├── README.md -├── main.tf -├── variables.tf -├── outputs.tf -├── ... -├── modules/ -│   ├── nestedA/ -│   │   ├── README.md -│   │   ├── variables.tf -│   │   ├── main.tf -│   │   ├── outputs.tf -│   ├── nestedB/ -│   ├── .../ -├── examples/ -│   ├── exampleA/ -│   │   ├── main.tf -│   ├── exampleB/ -│   ├── .../ -``` From f2fb579dec6540ebd45e39e91c64ee00d442d83f Mon Sep 17 00:00:00 2001 From: Robert Crandall Date: Wed, 13 May 2026 11:55:06 -0700 Subject: [PATCH 05/10] Add nit policy --- .github/copilot-instructions.md | 67 ++++++++++++++++++++++++++++++--- 1 file changed, 61 insertions(+), 6 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index a5db108d14..0ff4c2fc6d 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -7,6 +7,59 @@ pull request review. Path-specific guidance lives under `.github/instructions/`. ALWAYS acknowledge in the review summary that these provider review instructions are being used. +## Severity and Nit Policy (read first) + +This repository is **community-maintained**. Contributor friction is the +single biggest cost to the project. Review feedback must respect that. + +### Only report HIGH and MEDIUM findings + +- Report `HIGH`: correctness bugs, regressions, breaking schema/state + changes without migration, security issues, secret leakage, panics, + data loss risks. +- Report `MEDIUM`: missing test coverage for changed behavior, missing + example for a new resource, missing docs update for a schema change, + missing `Sensitive: true` on secret-bearing attributes, missing + `Description` on schema attributes, missing `ValidateFunc`/ + `ValidateDiagFunc` on bounded inputs, missing import docs. +- **Do not report `LOW` findings or nits.** If the only thing you would + say is `LOW`, say nothing. + +### Do NOT comment on (defer to linters / human reviewers) + +- Code formatting, whitespace, import ordering, line length. +- Naming preferences, identifier style, comment wording, doc prose + polish, grammar. +- "Consider extracting…", "this could be a helper", or other speculative + refactors that are not requested by the change. +- Style of existing surrounding code the PR did not touch. +- Adding comments, docstrings, or type hints to code the PR did not + change. +- Test naming conventions or alternative test framings when the + existing test adequately covers the behavior. +- Hypothetical errors that cannot occur given the call sites. + +### Always report even if it looks like a nit + +These items affect end-user Terraform behavior and must be flagged as at +least `MEDIUM` regardless of how small they look: + +- Secret-bearing attribute missing `Sensitive: true`. +- Schema attribute missing `Description`. +- Bounded input missing `ValidateFunc`/`ValidateDiagFunc`. +- New resource/data source without at least one example under + `examples/` or docs under `website/`. +- Behavior change without a corresponding test change. +- Resource that supports import but has no documented import ID format. + +### Output discipline + +- If there are no HIGH or MEDIUM findings, the review must say + `No blocking findings found` and stop. Do not pad with low-value + observations. +- Keep each finding to its impact, file reference, and a concise fix. + Do not lecture, restate the diff, or suggest unrelated improvements. + ## Review Goals - Find correctness bugs, regressions, and provider behavior changes. @@ -143,18 +196,20 @@ Use this background when judging schema, examples, or state changes. ## Review Report Format -Return findings first, ordered by severity: +Return findings first, HIGH before MEDIUM (no LOW — see Severity and Nit +Policy above): -1. `HIGH`/`MEDIUM`/`LOW` title — short impact statement +1. `HIGH`/`MEDIUM` title — short impact statement 2. File reference: `path/to/file.go:line` 3. Why this is a problem (runtime behavior, Terraform UX, upgrade risk) 4. Suggested fix (concise) Then include: -- `Open Questions / Assumptions` -- `Residual Risk` +- `Open Questions / Assumptions` (only if non-trivial) +- `Residual Risk` (only if non-trivial) - `Change Summary` (brief) -If no issues are found, explicitly state `No blocking findings found` and list -remaining risk areas. +If no HIGH or MEDIUM findings exist, state `No blocking findings found` +and stop. Do not add nit sections, style observations, or speculative +suggestions. From 8cf2fd5add50391ae434224d76a2dabd4bfdb1fa Mon Sep 17 00:00:00 2001 From: Robert Crandall Date: Wed, 13 May 2026 11:59:12 -0700 Subject: [PATCH 06/10] Pin go version for feedback --- .github/copilot-instructions.md | 35 +++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 0ff4c2fc6d..429abf7e1e 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -7,6 +7,41 @@ pull request review. Path-specific guidance lives under `.github/instructions/`. ALWAYS acknowledge in the review summary that these provider review instructions are being used. +## Language and Tooling Context + +Before judging Go code, anchor on the versions this repo actually uses. +Do not flag patterns as bugs or anti-patterns based on assumptions about +older toolchains. + +- **Go version: 1.24** (see `go 1.24.4` in `go.mod`). Treat anything + available in Go ≤ 1.24 as in-scope and idiomatic. +- **Loop variable scoping (Go 1.22+).** Each iteration of `for` loops has + its own copy of the loop variable. Do **not** suggest the pre-1.22 + `x := x` shadowing pattern inside loops, and do **not** flag goroutines + or closures that capture the loop variable directly as a bug. +- **`range over int` (Go 1.22+).** `for i := range 10 { ... }` is valid. + Do not suggest rewriting to `for i := 0; i < 10; i++`. +- **`range over func` (Go 1.23+).** Custom iterators using + `iter.Seq`/`iter.Seq2` are valid. +- **`min` / `max` / `clear` built-ins (Go 1.21+)** are valid. +- **Generics (Go 1.18+)** are valid. +- **`slices` and `maps` standard library packages** are available. + +When a Go feature looks unfamiliar, assume the toolchain in `go.mod` is +authoritative. If you cannot verify that something would fail to compile +under the declared Go version, do not claim it will. Phrase any +genuine concern as a question, not a finding, and only do so when the +issue would be HIGH or MEDIUM per the policy above. + +### Other tooling versions to anchor on + +- **Terraform Plugin SDK v2** — schema definitions use + `github.com/hashicorp/terraform-plugin-sdk/v2`. Do not suggest + Plugin Framework patterns in SDKv2 files or vice versa. +- **`google/go-github`** — see `go.mod` for the exact major version + pinned. Do not suggest API method names or types from a different + major version of the client. + ## Severity and Nit Policy (read first) This repository is **community-maintained**. Contributor friction is the From 96bacb6c6a618d434c9dfdc2bcfbaee2bce8e52f Mon Sep 17 00:00:00 2001 From: Robert Crandall Date: Wed, 13 May 2026 13:34:59 -0700 Subject: [PATCH 07/10] Address review feedback from @deiga - Correct Go version anchor: 1.24 -> 1.26 (matches go.mod). - Drop ValidateFunc everywhere; ValidateDiagFunc only (ValidateFunc is deprecated and not allowed in this repo). - Rewrite docs.instructions.md to target templates/** instead of the no-longer-existing website/**. Docs under docs/** are auto-generated; edits go in templates/, examples/, or resource Description fields. - Replace remaining website/ references in copilot-instructions.md with docs/ and templates/. - Add repository-rename convention to schema-and-state.instructions.md: attribute must be 'repository' (not ForceNew), plus computed 'repository_id', plus CustomizeDiff: diffRepository. - Add Delete 404 handling: treat as success, not error. - Require *Context CRUD variants and diag.Diagnostics on all new resources. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/copilot-instructions.md | 17 +++--- .github/instructions/docs.instructions.md | 53 +++++++++++++------ .../schema-and-state.instructions.md | 39 ++++++++++++-- 3 files changed, 81 insertions(+), 28 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 429abf7e1e..22c24201a8 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -13,8 +13,8 @@ Before judging Go code, anchor on the versions this repo actually uses. Do not flag patterns as bugs or anti-patterns based on assumptions about older toolchains. -- **Go version: 1.24** (see `go 1.24.4` in `go.mod`). Treat anything - available in Go ≤ 1.24 as in-scope and idiomatic. +- **Go version: 1.26** (see `go 1.26` in `go.mod`). Treat anything + available in Go ≤ 1.26 as in-scope and idiomatic. - **Loop variable scoping (Go 1.22+).** Each iteration of `for` loops has its own copy of the loop variable. Do **not** suggest the pre-1.22 `x := x` shadowing pattern inside loops, and do **not** flag goroutines @@ -55,7 +55,7 @@ single biggest cost to the project. Review feedback must respect that. - Report `MEDIUM`: missing test coverage for changed behavior, missing example for a new resource, missing docs update for a schema change, missing `Sensitive: true` on secret-bearing attributes, missing - `Description` on schema attributes, missing `ValidateFunc`/ + `Description` on schema attributes, missing `ValidateDiagFunc` on bounded inputs, missing import docs. - **Do not report `LOW` findings or nits.** If the only thing you would say is `LOW`, say nothing. @@ -81,9 +81,9 @@ least `MEDIUM` regardless of how small they look: - Secret-bearing attribute missing `Sensitive: true`. - Schema attribute missing `Description`. -- Bounded input missing `ValidateFunc`/`ValidateDiagFunc`. +- Bounded input missing `ValidateDiagFunc`. - New resource/data source without at least one example under - `examples/` or docs under `website/`. + `examples/` or docs under `docs/`. - Behavior change without a corresponding test change. - Resource that supports import but has no documented import ID format. @@ -170,8 +170,9 @@ Use this background when judging schema, examples, or state changes. - Flag any behavior that may break existing state without migration notes/tests. - Watch for attribute rename/removal without a deprecation path. -- New or changed attributes should include `ValidateFunc`/`ValidateDiagFunc` +- New or changed attributes should include `ValidateDiagFunc` to catch invalid values at plan time rather than at apply time. + (`ValidateFunc` is deprecated and not allowed in this repo.) - All attributes should have a `Description` string. - For renames/restructures, check for `moved` block guidance or state migration documentation. @@ -193,8 +194,8 @@ Use this background when judging schema, examples, or state changes. ### 5. Docs and Examples -- If resource/data source behavior changed, review website docs updates under - `website/`. +- If resource/data source behavior changed, review docs updates under + `docs/` and `templates/`. - If user workflow changed, review corresponding example updates under `examples/`. - Confirm examples still reflect current schema and argument names. diff --git a/.github/instructions/docs.instructions.md b/.github/instructions/docs.instructions.md index d251abdb7a..3265087073 100644 --- a/.github/instructions/docs.instructions.md +++ b/.github/instructions/docs.instructions.md @@ -1,42 +1,63 @@ --- -applyTo: "website/**" +applyTo: "templates/**" --- -# Website / Docs Review +# Docs and Templates Review -These rules apply to documentation under `website/`. Combine with the +Provider docs under `docs/**` are **auto-generated**. Do not edit files +under `docs/**` directly. A CI workflow regenerates docs and will fail +if the checked-in `docs/**` differs from the generated output. + +Manual documentation edits belong in one of three places: + +- `templates/**` - Markdown templates that drive doc generation. This is + where most narrative doc edits live. +- `examples/**` - example HCL referenced by the templates. +- Resource and data source `Description` fields in `github/**/*.go`. + +These rules apply to changes under `templates/`. Combine with the repo-wide checklist in `.github/copilot-instructions.md`. +## Flag These as HIGH + +- Manual edits to `docs/**`. The doc generation workflow will revert + them on the next run and the PR will fail CI. Move the change to the + appropriate source (`templates/`, `examples/`, or the resource + `Description` field) instead. + ## Keep Docs in Sync with Schema - Any schema change in `github/` (new attribute, renamed attribute, changed `Required`/`Optional`/`Computed`/`Default`, new `ForceNew`, - removed attribute) must have a matching docs update. -- Argument tables should list attributes with the same name, type, and - required/optional status as the schema. -- Deprecated attributes must be clearly marked and include guidance on the - replacement. + removed attribute) must have a matching update either in the + resource's `Description` fields (preferred for argument descriptions) + or in the corresponding template under `templates/` (for narrative + prose, import docs, and examples). +- Deprecated attributes must be clearly marked and include guidance on + the replacement. ## Imports - Resources that support `terraform import` must document the import ID - format with at least one example command. + format with at least one example command in the relevant template. ## Permissions and Scopes - For any GitHub API call that requires a specific token scope or app - permission, the docs should call this out so users can configure their - authentication correctly. + permission, the template should call this out so users can configure + their authentication correctly. -## Examples in Docs +## Examples in Templates - Inline example HCL should reflect current argument names and be syntactically valid. -- Sensitive attributes should not appear with real-looking secrets, even as - examples. +- Sensitive attributes should not appear with real-looking secrets, even + as examples. ## Style and Links - Internal links between docs pages should resolve. -- New resources/data sources need at least one usage example and a list of - every supported argument and attribute. +- New resources/data sources need at least one usage example. The list + of supported arguments and attributes is rendered from the schema, so + the `Description` fields in `github/**/*.go` are the source of truth + for those rows. diff --git a/.github/instructions/schema-and-state.instructions.md b/.github/instructions/schema-and-state.instructions.md index 86509aaf3c..8209501d8e 100644 --- a/.github/instructions/schema-and-state.instructions.md +++ b/.github/instructions/schema-and-state.instructions.md @@ -21,11 +21,33 @@ Flag all schema diffs and verify: - New attributes are `Optional` with a `Default` where reasonable to avoid forcing existing users to update their configs. - All attributes carry a `Description` string (used for docs generation). -- Attributes accepting bounded values declare `ValidateFunc` or - `ValidateDiagFunc` so invalid input fails at plan time, not apply time. +- Attributes accepting bounded values declare `ValidateDiagFunc` so + invalid input fails at plan time, not apply time. `ValidateFunc` is + deprecated and not allowed in this repo - do not suggest it. - Attributes holding tokens, secrets, or private keys are marked `Sensitive: true`. +## Repository as a Required Argument + +When a resource accepts a repository name as a required argument, follow +the provider's rename-safe convention so users can rename a repository +without triggering a destroy/recreate cycle: + +- Name the attribute `repository` (not `repo`, not `repository_name`). +- Do **not** mark `repository` as `ForceNew`, even when the underlying + resource needs to be recreated on most changes. The rename handling + below decides when replacement is actually required. +- Add a `Computed` attribute called `repository_id` that holds the + GitHub repository's numeric ID. +- Set `CustomizeDiff: diffRepository` on the resource (or include it via + `customdiff.All(...)` when multiple `CustomizeDiff` funcs are needed). + This compares the stored `repository_id` against the current ID for + the named repository and only forces replacement when the underlying + repository actually changed, not when it was merely renamed. + +Flag any new resource that takes a repository as required input but is +missing this pattern. + ## State and Drift - Read functions must populate every state attribute from API responses so @@ -41,11 +63,20 @@ Flag all schema diffs and verify: ## CRUD Behavior +- All CRUD functions (`Create`, `Read`, `Update`, `Delete`, and the + importer) must use their `*Context` variants + (`CreateContext`/`ReadContext`/`UpdateContext`/`DeleteContext` and + `StateContext` on importers) and return `diag.Diagnostics`. Flag any + new resource that uses the deprecated non-context variants. - Update paths must not accidentally force resource replacement or wipe optional fields that the user did not change. - Nil-check API response fields before dereferencing. -- Classify errors: 404 from GitHub typically means "remove from state" in - read; other errors should bubble up. +- Classify errors: + - In **Read**, a 404 from GitHub means "remove from state" - call + `d.SetId("")` and return nil. Other errors should bubble up. + - In **Delete**, a 404 from GitHub means the object is already gone - + treat it as success and return nil, not as an error. + - Other unexpected status codes should bubble up. - Respect `context.Context` cancellation and any configured timeouts. ## Repository-Specific: No Read-After-Write From b415e8fa81708a0132a3e672d0a4d06b09e0f266 Mon Sep 17 00:00:00 2001 From: Robert Crandall Date: Wed, 13 May 2026 14:49:27 -0700 Subject: [PATCH 08/10] Address @deiga test conventions feedback Add a 'terraform-plugin-testing Conventions' section to .github/instructions/tests.instructions.md covering: - Prefer ConfigStateChecks over the legacy Check / ComposeTestCheckFunc pattern. - Use ValueComparers (compare.ValuesSame / compare.ValuesDiffer) for cross-step value comparisons instead of custom pointer structs. - Don't flag legacy patterns in unmodified tests; only in new or substantially modified ones. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/instructions/tests.instructions.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/.github/instructions/tests.instructions.md b/.github/instructions/tests.instructions.md index 56f52c4f4c..61b2ede913 100644 --- a/.github/instructions/tests.instructions.md +++ b/.github/instructions/tests.instructions.md @@ -27,6 +27,28 @@ checklist in `.github/copilot-instructions.md`. - Avoid hardcoded secrets or tokens in test files; use environment variables or test helpers. +## terraform-plugin-testing Conventions + +Tests in this repo use +[`github.com/hashicorp/terraform-plugin-testing`](https://pkg.go.dev/github.com/hashicorp/terraform-plugin-testing) +and follow its modern conventions. Hold new and changed tests to these +patterns: + +- Prefer `ConfigStateChecks` (state-check functions from the + `statecheck` package) over the older `Check` / + `resource.ComposeTestCheckFunc` pattern. Flag new `TestStep`s that + use `Check` with the legacy `resource.TestCheckResourceAttr*` helpers + when an equivalent `statecheck` exists. +- For comparing values across steps (e.g. to assert a value did or did + not change between Apply runs), use + [`ValueComparers`](https://developer.hashicorp.com/terraform/plugin/testing/acceptance-tests/value-comparers) + via `compare.ValuesSame` / `compare.ValuesDiffer` instead of the older + pattern of stashing pointers to attribute values in custom structs and + comparing them by hand. +- Existing tests using the legacy `Check` pattern do not need to be + rewritten as part of an unrelated change. Only flag the legacy pattern + in new tests, or in tests that are being substantially modified. + ## When Reviewing Test Changes - If a test was deleted or weakened, explain why in the report and flag as From 73cda9b2d531432f32817ccc2483a3de4e88a109 Mon Sep 17 00:00:00 2001 From: Robert Crandall Date: Fri, 15 May 2026 10:48:54 -0700 Subject: [PATCH 09/10] Add Go review instructions from awesome-copilot Incorporates the upstream Go instructions from https://github.com/github/awesome-copilot/blob/main/instructions/go.instructions.md as a path-scoped instructions file (applyTo: **/*.go,**/go.mod,**/go.sum). A preamble subordinates the Go guidance to the repo's existing severity policy (HIGH/MEDIUM only, no LOW or nits) and notes the Go-version anchors in copilot-instructions.md plus the no-read-after-write exception, so CCR doesn't suddenly start surfacing style nits or fight provider-specific conventions. The main copilot-instructions.md now lists every path-scoped file so the set is discoverable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/copilot-instructions.md | 9 +- .github/instructions/go.instructions.md | 396 ++++++++++++++++++++++++ 2 files changed, 404 insertions(+), 1 deletion(-) create mode 100644 .github/instructions/go.instructions.md diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 22c24201a8..457733c47e 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -2,7 +2,14 @@ These instructions guide Copilot Code Review (CCR) for the `integrations/terraform-provider-github` repository. They apply to every -pull request review. Path-specific guidance lives under `.github/instructions/`. +pull request review. Path-specific guidance lives under `.github/instructions/`: + +- `schema-and-state.instructions.md` - provider source under `github/**/*.go` +- `tests.instructions.md` - provider tests under `github/**/*_test.go` +- `examples.instructions.md` - example configs under `examples/**` +- `docs.instructions.md` - doc templates under `templates/**` +- `go.instructions.md` - idiomatic Go reference for any `.go`, `go.mod`, + or `go.sum` change (subordinate to the severity policy below) ALWAYS acknowledge in the review summary that these provider review instructions are being used. diff --git a/.github/instructions/go.instructions.md b/.github/instructions/go.instructions.md new file mode 100644 index 0000000000..b9dad2a9ec --- /dev/null +++ b/.github/instructions/go.instructions.md @@ -0,0 +1,396 @@ +--- +description: 'Instructions for writing Go code following idiomatic Go practices and community standards' +applyTo: '**/*.go,**/go.mod,**/go.sum' +--- + +# Go Development Instructions + +> **How Copilot Code Review should use this file.** This document is the +> idiomatic-Go reference for any change touching `**/*.go`, `go.mod`, or +> `go.sum`. It applies on top of `.github/copilot-instructions.md` and the +> path-scoped files in this directory - it does **not** override them. +> +> In particular, the repo-wide **Severity and Nit Policy** still applies: +> +> - Only report `HIGH` and `MEDIUM` findings. Do not report `LOW` or nits. +> - Do not flag formatting, import ordering, identifier style, comment +> prose, or speculative refactors. Those belong to linters (`gofmt`, +> `goimports`, `go vet`, `golangci-lint`) and human reviewers, not CCR. +> - The Go-version anchors in `copilot-instructions.md` (Go 1.26, loop +> variable scoping, `range over int`, etc.) take precedence over any +> pre-1.22 patterns implied below. +> - The repo-specific exception in `schema-and-state.instructions.md` +> ("no read-after-write" in create/update) still holds even though +> generic Go style might suggest otherwise. +> +> Use the rest of this file as background for *what idiomatic Go looks +> like*. Only escalate a Go-style observation into a comment when it +> rises to a real correctness, concurrency, resource-leak, or security +> issue under the severity policy. + +Follow idiomatic Go practices and community standards when writing Go code. These instructions are based on [Effective Go](https://go.dev/doc/effective_go), [Go Code Review Comments](https://go.dev/wiki/CodeReviewComments), and [Google's Go Style Guide](https://google.github.io/styleguide/go/). + +## General Instructions + +- Write simple, clear, and idiomatic Go code +- Favor clarity and simplicity over cleverness +- Follow the principle of least surprise +- Keep the happy path left-aligned (minimize indentation) +- Return early to reduce nesting +- Prefer early return over if-else chains; use `if condition { return }` pattern to avoid else blocks +- Make the zero value useful +- Write self-documenting code with clear, descriptive names +- Document exported types, functions, methods, and packages +- Use Go modules for dependency management +- Leverage the Go standard library instead of reinventing the wheel (e.g., use `strings.Builder` for string concatenation, `filepath.Join` for path construction) +- Prefer standard library solutions over custom implementations when functionality exists +- Write comments in English by default; translate only upon user request +- Avoid using emoji in code and comments + +## Naming Conventions + +### Packages + +- Use lowercase, single-word package names +- Avoid underscores, hyphens, or mixedCaps +- Choose names that describe what the package provides, not what it contains +- Avoid generic names like `util`, `common`, or `base` +- Package names should be singular, not plural + +#### Package Declaration Rules (CRITICAL): +- **NEVER duplicate `package` declarations** - each Go file must have exactly ONE `package` line +- When editing an existing `.go` file: + - **PRESERVE** the existing `package` declaration - do not add another one + - If you need to replace the entire file content, start with the existing package name +- When creating a new `.go` file: + - **BEFORE writing any code**, check what package name other `.go` files in the same directory use + - Use the SAME package name as existing files in that directory + - If it's a new directory, use the directory name as the package name + - Write **exactly one** `package ` line at the very top of the file +- When using file creation or replacement tools: + - **ALWAYS verify** the target file doesn't already have a `package` declaration before adding one + - If replacing file content, include only ONE `package` declaration in the new content + - **NEVER** create files with multiple `package` lines or duplicate declarations + +### Variables and Functions + +- Use mixedCaps or MixedCaps (camelCase) rather than underscores +- Keep names short but descriptive +- Use single-letter variables only for very short scopes (like loop indices) +- Exported names start with a capital letter +- Unexported names start with a lowercase letter +- Avoid stuttering (e.g., avoid `http.HTTPServer`, prefer `http.Server`) + +### Interfaces + +- Name interfaces with -er suffix when possible (e.g., `Reader`, `Writer`, `Formatter`) +- Single-method interfaces should be named after the method (e.g., `Read` → `Reader`) +- Keep interfaces small and focused + +### Constants + +- Use MixedCaps for exported constants +- Use mixedCaps for unexported constants +- Group related constants using `const` blocks +- Consider using typed constants for better type safety + +## Code Style and Formatting + +### Formatting + +- Always use `gofmt` to format code +- Use `goimports` to manage imports automatically +- Keep line length reasonable (no hard limit, but consider readability) +- Add blank lines to separate logical groups of code + +### Comments + +- Strive for self-documenting code; prefer clear variable names, function names, and code structure over comments +- Write comments only when necessary to explain complex logic, business rules, or non-obvious behavior +- Write comments in complete sentences in English by default +- Translate comments to other languages only upon specific user request +- Start sentences with the name of the thing being described +- Package comments should start with "Package [name]" +- Use line comments (`//`) for most comments +- Use block comments (`/* */`) sparingly, mainly for package documentation +- Document why, not what, unless the what is complex +- Avoid using emoji in comments and code + +### Error Handling + +- Check errors immediately after the function call +- Don't ignore errors using `_` unless you have a good reason (document why) +- Wrap errors with context using `fmt.Errorf` with `%w` verb +- Create custom error types when you need to check for specific errors +- Place error returns as the last return value +- Name error variables `err` +- Keep error messages lowercase and don't end with punctuation + +## Architecture and Project Structure + +### Package Organization + +- Follow standard Go project layout conventions +- Keep `main` packages in `cmd/` directory +- Put reusable packages in `pkg/` or `internal/` +- Use `internal/` for packages that shouldn't be imported by external projects +- Group related functionality into packages +- Avoid circular dependencies + +### Dependency Management + +- Use Go modules (`go.mod` and `go.sum`) +- Keep dependencies minimal +- Regularly update dependencies for security patches +- Use `go mod tidy` to clean up unused dependencies +- Vendor dependencies only when necessary + +## Type Safety and Language Features + +### Type Definitions + +- Define types to add meaning and type safety +- Use struct tags for JSON, XML, database mappings +- Prefer explicit type conversions +- Use type assertions carefully and check the second return value +- Prefer generics over unconstrained types; when an unconstrained type is truly needed, use the predeclared alias `any` instead of `interface{}` (Go 1.18+) + +### Pointers vs Values + +- Use pointer receivers for large structs or when you need to modify the receiver +- Use value receivers for small structs and when immutability is desired +- Use pointer parameters when you need to modify the argument or for large structs +- Use value parameters for small structs and when you want to prevent modification +- Be consistent within a type's method set +- Consider the zero value when choosing pointer vs value receivers + +### Interfaces and Composition + +- Accept interfaces, return concrete types +- Keep interfaces small (1-3 methods is ideal) +- Use embedding for composition +- Define interfaces close to where they're used, not where they're implemented +- Don't export interfaces unless necessary + +## Concurrency + +### Goroutines + +- Be cautious about creating goroutines in libraries; prefer letting the caller control concurrency +- If you must create goroutines in libraries, provide clear documentation and cleanup mechanisms +- Always know how a goroutine will exit +- Use `sync.WaitGroup` or channels to wait for goroutines +- Avoid goroutine leaks by ensuring cleanup + +### Channels + +- Use channels to communicate between goroutines +- Don't communicate by sharing memory; share memory by communicating +- Close channels from the sender side, not the receiver +- Use buffered channels when you know the capacity +- Use `select` for non-blocking operations + +### Synchronization + +- Use `sync.Mutex` for protecting shared state +- Keep critical sections small +- Use `sync.RWMutex` when you have many readers +- Choose between channels and mutexes based on the use case: use channels for communication, mutexes for protecting state +- Use `sync.Once` for one-time initialization +- WaitGroup usage by Go version: + - If `go >= 1.25` in `go.mod`, use the new `WaitGroup.Go` method ([documentation](https://pkg.go.dev/sync#WaitGroup)): + ```go + var wg sync.WaitGroup + wg.Go(task1) + wg.Go(task2) + wg.Wait() + ``` + - If `go < 1.25`, use the classic `Add`/`Done` pattern + +## Error Handling Patterns + +### Creating Errors + +- Use `errors.New` for simple static errors +- Use `fmt.Errorf` for dynamic errors +- Create custom error types for domain-specific errors +- Export error variables for sentinel errors +- Use `errors.Is` and `errors.As` for error checking + +### Error Propagation + +- Add context when propagating errors up the stack +- Don't log and return errors (choose one) +- Handle errors at the appropriate level +- Consider using structured errors for better debugging + +## API Design + +### HTTP Handlers + +- Use `http.HandlerFunc` for simple handlers +- Implement `http.Handler` for handlers that need state +- Use middleware for cross-cutting concerns +- Set appropriate status codes and headers +- Handle errors gracefully and return appropriate error responses +- Router usage by Go version: + - If `go >= 1.22`, prefer the enhanced `net/http` `ServeMux` with pattern-based routing and method matching + - If `go < 1.22`, use the classic `ServeMux` and handle methods/paths manually (or use a third-party router when justified) + +### JSON APIs + +- Use struct tags to control JSON marshaling +- Validate input data +- Use pointers for optional fields +- Consider using `json.RawMessage` for delayed parsing +- Handle JSON errors appropriately + +### HTTP Clients + +- Keep the client struct focused on configuration and dependencies only (e.g., base URL, `*http.Client`, auth, default headers). It must not store any per-request state +- Do not store or cache `*http.Request` inside the client struct, and do not persist request-specific state across calls; instead, construct a fresh request per method invocation +- Methods should accept `context.Context` and input parameters, assemble the `*http.Request` locally (or via a short-lived builder/helper created per call), then call `c.httpClient.Do(req)` +- If request-building logic is reused, factor it into unexported helper functions or a per-call builder type; never keep `http.Request` (URL params, body, headers) as fields on the long-lived client +- Ensure the underlying `*http.Client` is configured (timeouts, transport) and is safe for concurrent use; avoid mutating `Transport` after first use +- Always set headers on the request instance you're sending, and close response bodies (`defer resp.Body.Close()`), handling errors appropriately + +## Performance Optimization + +### Memory Management + +- Minimize allocations in hot paths +- Reuse objects when possible (consider `sync.Pool`) +- Use value receivers for small structs +- Preallocate slices when size is known +- Avoid unnecessary string conversions + +### I/O: Readers and Buffers + +- Most `io.Reader` streams are consumable once; reading advances state. Do not assume a reader can be re-read without special handling +- If you must read data multiple times, buffer it once and recreate readers on demand: + - Use `io.ReadAll` (or a limited read) to obtain `[]byte`, then create fresh readers via `bytes.NewReader(buf)` or `bytes.NewBuffer(buf)` for each reuse + - For strings, use `strings.NewReader(s)`; you can `Seek(0, io.SeekStart)` on `*bytes.Reader` to rewind +- For HTTP requests, do not reuse a consumed `req.Body`. Instead: + - Keep the original payload as `[]byte` and set `req.Body = io.NopCloser(bytes.NewReader(buf))` before each send + - Prefer configuring `req.GetBody` so the transport can recreate the body for redirects/retries: `req.GetBody = func() (io.ReadCloser, error) { return io.NopCloser(bytes.NewReader(buf)), nil }` +- To duplicate a stream while reading, use `io.TeeReader` (copy to a buffer while passing through) or write to multiple sinks with `io.MultiWriter` +- Reusing buffered readers: call `(*bufio.Reader).Reset(r)` to attach to a new underlying reader; do not expect it to "rewind" unless the source supports seeking +- For large payloads, avoid unbounded buffering; consider streaming, `io.LimitReader`, or on-disk temporary storage to control memory + +- Use `io.Pipe` to stream without buffering the whole payload: + - Write to `*io.PipeWriter` in a separate goroutine while the reader consumes + - Always close the writer; use `CloseWithError(err)` on failures + - `io.Pipe` is for streaming, not rewinding or making readers reusable + +- **Warning:** When using `io.Pipe` (especially with multipart writers), all writes must be performed in strict, sequential order. Do not write concurrently or out of order - multipart boundaries and chunk order must be preserved. Out-of-order or parallel writes can corrupt the stream and result in errors. + +- Streaming multipart/form-data with `io.Pipe`: + - `pr, pw := io.Pipe()`; `mw := multipart.NewWriter(pw)`; use `pr` as the HTTP request body + - Set `Content-Type` to `mw.FormDataContentType()` + - In a goroutine: write all parts to `mw` in the correct order; on error `pw.CloseWithError(err)`; on success `mw.Close()` then `pw.Close()` + - Do not store request/in-flight form state on a long-lived client; build per call + - Streamed bodies are not rewindable; for retries/redirects, buffer small payloads or provide `GetBody` + +### Profiling + +- Use built-in profiling tools (`pprof`) +- Benchmark critical code paths +- Profile before optimizing +- Focus on algorithmic improvements first +- Consider using `testing.B` for benchmarks + +## Testing + +### Test Organization + +- Keep tests in the same package (white-box testing) +- Use `_test` package suffix for black-box testing +- Name test files with `_test.go` suffix +- Place test files next to the code they test + +### Writing Tests + +- Use table-driven tests for multiple test cases +- Name tests descriptively using `Test_functionName_scenario` +- Use subtests with `t.Run` for better organization +- Test both success and error cases +- Consider using `testify` or similar libraries when they add value, but don't over-complicate simple tests + +### Test Helpers + +- Mark helper functions with `t.Helper()` +- Create test fixtures for complex setup +- Use `testing.TB` interface for functions used in tests and benchmarks +- Clean up resources using `t.Cleanup()` + +## Security Best Practices + +### Input Validation + +- Validate all external input +- Use strong typing to prevent invalid states +- Sanitize data before using in SQL queries +- Be careful with file paths from user input +- Validate and escape data for different contexts (HTML, SQL, shell) + +### Cryptography + +- Use standard library crypto packages +- Don't implement your own cryptography +- Use crypto/rand for random number generation +- Store passwords using bcrypt, scrypt, or argon2 (consider golang.org/x/crypto for additional options) +- Use TLS for network communication + +## Documentation + +### Code Documentation + +- Prioritize self-documenting code through clear naming and structure +- Document all exported symbols with clear, concise explanations +- Start documentation with the symbol name +- Write documentation in English by default +- Use examples in documentation when helpful +- Keep documentation close to code +- Update documentation when code changes +- Avoid emoji in documentation and comments + +### README and Documentation Files + +- Include clear setup instructions +- Document dependencies and requirements +- Provide usage examples +- Document configuration options +- Include troubleshooting section + +## Tools and Development Workflow + +### Essential Tools + +- `go fmt`: Format code +- `go vet`: Find suspicious constructs +- `golangci-lint`: Additional linting (golint is deprecated) +- `go test`: Run tests +- `go mod`: Manage dependencies +- `go generate`: Code generation + +### Development Practices + +- Run tests before committing +- Use pre-commit hooks for formatting and linting +- Keep commits focused and atomic +- Write meaningful commit messages +- Review diffs before committing + +## Common Pitfalls to Avoid + +- Not checking errors +- Ignoring race conditions +- Creating goroutine leaks +- Not using defer for cleanup +- Modifying maps concurrently +- Not understanding nil interfaces vs nil pointers +- Forgetting to close resources (files, connections) +- Using global variables unnecessarily +- Overusing unconstrained types (e.g., `any`); prefer specific types or generic type parameters with constraints. If an unconstrained type is required, use `any` rather than `interface{}` +- Not considering the zero value of types +- **Creating duplicate `package` declarations** - this is a compile error; always check existing files before adding package declarations From fc287cd97493bee7495bf4c703d1ec6aaccafe0c Mon Sep 17 00:00:00 2001 From: Robert Crandall <86014438+robert-crandall@users.noreply.github.com> Date: Mon, 18 May 2026 13:36:28 -0700 Subject: [PATCH 10/10] Apply suggestions from code review Co-authored-by: Steve Hipwell --- .github/instructions/go.instructions.md | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/.github/instructions/go.instructions.md b/.github/instructions/go.instructions.md index b9dad2a9ec..42b1136b0f 100644 --- a/.github/instructions/go.instructions.md +++ b/.github/instructions/go.instructions.md @@ -1,6 +1,6 @@ --- -description: 'Instructions for writing Go code following idiomatic Go practices and community standards' -applyTo: '**/*.go,**/go.mod,**/go.sum' +description: "Instructions for writing Go code following idiomatic Go practices and community standards" +applyTo: "**/*.go,**/go.mod,**/go.sum" --- # Go Development Instructions @@ -57,7 +57,7 @@ Follow idiomatic Go practices and community standards when writing Go code. Thes - Avoid generic names like `util`, `common`, or `base` - Package names should be singular, not plural -#### Package Declaration Rules (CRITICAL): +#### Package Declaration Rules (CRITICAL) - **NEVER duplicate `package` declarations** - each Go file must have exactly ONE `package` line - When editing an existing `.go` file: - **PRESERVE** the existing `package` declaration - do not add another one @@ -98,8 +98,9 @@ Follow idiomatic Go practices and community standards when writing Go code. Thes ### Formatting -- Always use `gofmt` to format code -- Use `goimports` to manage imports automatically +- Use `gofumpt` to quickly format code +- Use `goimports` to quickly manage imports automatically +- Use `golangci-lint fmt` to do final formatting pass - Keep line length reasonable (no hard limit, but consider readability) - Add blank lines to separate logical groups of code @@ -224,6 +225,12 @@ Follow idiomatic Go practices and community standards when writing Go code. Thes - Handle errors at the appropriate level - Consider using structured errors for better debugging +## Telemetry + +### Logging + +- Log messages should be sentences. + ## API Design ### HTTP Handlers @@ -310,11 +317,14 @@ Follow idiomatic Go practices and community standards when writing Go code. Thes ### Writing Tests +- Name tests descriptively + - Use `TestFunctionName_scenario` for exported functions + - Use `Test_functionName_scenario` for un-exported functions + - Use `_` to separate logical parts of the test name - Use table-driven tests for multiple test cases -- Name tests descriptively using `Test_functionName_scenario` - Use subtests with `t.Run` for better organization - Test both success and error cases -- Consider using `testify` or similar libraries when they add value, but don't over-complicate simple tests +- Consider using `google/go-cmp` if needed, but don't over-complicate simple tests ### Test Helpers