From 21629279a3ddc53c042a342dcd65accc9a4f55a7 Mon Sep 17 00:00:00 2001 From: Ludovico Magnocavallo Date: Sat, 28 Mar 2026 08:26:00 +0100 Subject: [PATCH] Refactor agent documentation and establish core guidelines (#3820) * update the factories overview * update agent rules * update main GEMINI file * add preferred workflow to GEMINI file --- .agents/rules/context-interpolation.md | 48 ++++++++++++++++++++++++++ .agents/rules/dev-workflow.md | 3 ++ .agents/rules/fast-factory-driven.md | 4 +-- .agents/rules/style-guide.md | 27 +++++++++++++++ .agents/rules/testing-guidelines.md | 31 +++++++++++++++++ FACTORIES.md | 7 ++-- GEMINI.md | 41 +++++++++++++++++++--- 7 files changed, 151 insertions(+), 10 deletions(-) create mode 100644 .agents/rules/context-interpolation.md create mode 100644 .agents/rules/style-guide.md create mode 100644 .agents/rules/testing-guidelines.md diff --git a/.agents/rules/context-interpolation.md b/.agents/rules/context-interpolation.md new file mode 100644 index 000000000..e2e8cd618 --- /dev/null +++ b/.agents/rules/context-interpolation.md @@ -0,0 +1,48 @@ +--- +trigger: always_on +--- + +# Context-Based Interpolation + +When designing factory patterns or datasets, you MUST leverage the **context-based interpolation** system. + +A `context` object variable is used to hold maps of known, existing resource IDs (such as `project_ids`, `folder_ids`, `networks`, `iam_principals`). + +## Usage Pattern + +1. **Variables:** Add a `context` variable in `variables.tf`. + ```hcl + variable "context" { + description = "Context-specific interpolations." + type = object({ + project_ids = optional(map(string), {}) + folder_ids = optional(map(string), {}) + }) + default = {} + } + ``` + +2. **Locals:** Build `ctx` and `ctx_p` local variables in `main.tf` by flattening the `var.context` object. + ```hcl + locals { + ctx = { + for k, v in var.context : k => { + for kk, vv in v : "${local.ctx_p}${k}:${kk}" => vv + } + } + ctx_p = "$" + } + ``` + +3. **Lookups:** Apply lookups inside resources. + ```hcl + project = lookup(local.ctx.project_ids, var.project_id, var.project_id) + ``` + +4. **YAML Interpolation:** In factory YAML files, use the `$` prefix convention to reference the lookup map keys. + ```yaml + # Instead of hardcoding the folder ID: parent: folders/1234567890 + parent: $folder_ids:teams/team-a + ``` + +This pattern makes YAML configuration files highly portable across installations and environments by substituting mnemonic keys for hardcoded values. \ No newline at end of file diff --git a/.agents/rules/dev-workflow.md b/.agents/rules/dev-workflow.md index 62b697a9c..dac4f3f14 100644 --- a/.agents/rules/dev-workflow.md +++ b/.agents/rules/dev-workflow.md @@ -5,6 +5,9 @@ trigger: always_on # Always make sure edited code passes linting checks - run `tools/tfdoc.py` if variable or output definitions changed +- run `tools/check_documentation.py` to ensure variables are sorted alphabetically and READMEs are consistent +- run `tools/check_boilerplate.py` to ensure license headers are present - run `terraform fmt` on any edited Terraform file, and hcl examples in README files +- run `yamllint -c .yamllint --no-warnings ` on any edited YAML files - a schema change should be reflected in all the other places that use the same schema, those are documented in `tools/duplicate-diff.py` - always make sure both example and module (or stage) level tests run for all the modules/stages where code was edited \ No newline at end of file diff --git a/.agents/rules/fast-factory-driven.md b/.agents/rules/fast-factory-driven.md index 8334715f6..1de2b9562 100644 --- a/.agents/rules/fast-factory-driven.md +++ b/.agents/rules/fast-factory-driven.md @@ -4,7 +4,7 @@ trigger: always_on # Use factory datasets for resource configuration -**FAST stages should generally not implement factories, but leverage those defined in modules and their associated schemas.** +**FAST stages should leverage module-backed factories (e.g., `project-factory`) when available, but must implement their own stage-level factories when macro-modules don't exist.** Stages in the FAST folder are split between Terraform code and datasets. @@ -12,7 +12,7 @@ Code is used to implement and wire together "factories" that implement resource - YAML configurations are grouped in "datasets" which implement a complete design for the stage - each factory has a reference JSON schema used to describe and validate the YAML data -- factories are generally implemented in the underlying modules, not in FAST stages +- factories are generally implemented in the underlying modules, not in FAST stages, *unless* the stage needs to iterate over standard modules or resources (e.g., `dns` zones, `net-firewall-policy`, `ncc_hubs`). - modules deal with one specific resource set (eg an instance and its disks, a project and its org policies, IAM, etc.) and generally implement a single factory - the project and VPC factory modules are the exception, as they are designed as "macro modules" to support interrelated creation of resources pertaining to a larger scope - modules that do not manage "sets" of resources (e.g. one project, one folder, one dataset, etc.) typically do not have an associated factory, or only do for sub-resources (e.g. rules in a single policy), those factories are either implemented in the "macro modules" or directly in FAST diff --git a/.agents/rules/style-guide.md b/.agents/rules/style-guide.md new file mode 100644 index 000000000..537b7b09e --- /dev/null +++ b/.agents/rules/style-guide.md @@ -0,0 +1,27 @@ +--- +trigger: always_on +--- + +# Strictly Adhere to the Fabric Style Guide + +When modifying or generating Terraform code, you MUST follow these specific style conventions: + +- **Line Length:** Enforce a 79-character line length limit for legibility. This rule is relaxed *only* for long resource attribute names and descriptions. +- **Ternary Operators:** Wrap complex ternary operators in parentheses and break lines to align the `?` and `:` tokens clearly. Example: + ```hcl + locals { + parent_id = ( + var.parent == null || startswith(coalesce(var.parent, "-"), "$") + ? var.parent + : split("/", var.parent)[1] + ) + } + ``` +- **Function Calls:** Split function calls with many arguments across multiple lines, typically breaking after the opening parenthesis and before the closing one. +- **Alphabetical Ordering:** You MUST ensure variables and outputs are strictly sorted alphabetically. Run `tools/check_documentation.py` to verify this. +- **Locals Separation:** + - Use module-level locals for values referenced directly by resources or outputs (e.g., `svpc_host_config`). + - Use block-level "private" locals prefixed with an underscore (`_`) for intermediate transformations only referenced within the same block (e.g., `_svpc_service_iam`). +- **Complex Transformations:** Move complex data transformations in `for` or `for_each` loops to `locals` to keep resource blocks clean. +- **Variable Spaces:** Leverage `optional()` defaults extensively within object variables to reduce verbosity. +- **Naming:** NEVER use random strings for resource naming. Instead, implement and use an optional `prefix` variable in all modules. diff --git a/.agents/rules/testing-guidelines.md b/.agents/rules/testing-guidelines.md new file mode 100644 index 000000000..6de060f3c --- /dev/null +++ b/.agents/rules/testing-guidelines.md @@ -0,0 +1,31 @@ +--- +trigger: always_on +--- + +# Use Example-Based Testing in README.md + +When testing modules, you MUST prefer example-based testing over writing legacy Python `pytest` functions. + +Example-based tests are triggered from HCL Markdown fenced code blocks inside a module's `README.md` file using a special `# tftest` comment at the bottom. + +## How to structure a test +1. Write a clear HCL code block demonstrating the functionality. +2. Add the `tftest` directive, declaring expected counts. +3. If validating values, point to a YAML inventory file. + +```hcl +module "my-module" { + source = "./modules/my-module" + name = "test-example" +} +# tftest modules=1 resources=2 inventory=my-inventory.yaml +``` + +## Inventory Files +Inventory files are YAML datasets used to assert that the generated plan matches expectations. +- Place them in the `tests/modules//examples/` directory. +- You can generate an inventory automatically by running a successful plan using `pytest -s` (refer to `CONTRIBUTING.md` for the exact command format). +- **DO NOT hand-code inventory files from scratch.** Extract only the necessary bits relevant to the test scenario from the generated output. + +## tftest-based tests (Advanced) +If a module lacks an example in its `README.md` or you are testing a FAST stage without examples, use `tftest`-based tests using `tfvars` and `yaml` files. Refer to the "Testing via `tfvars` and `yaml`" section in `CONTRIBUTING.md`. \ No newline at end of file diff --git a/FACTORIES.md b/FACTORIES.md index d8e1a7f2e..35e492bc3 100644 --- a/FACTORIES.md +++ b/FACTORIES.md @@ -17,7 +17,6 @@ The following table provides a granular overview of modules that implement facto | :--- | :--- | :--- | :--- | :--- | | **analytics-hub** | Analytics Hub Exchange | `listings` | Analytics Hub Listings | `project_id`, `region` | | **billing-account** | Billing Account (Config) | `budgets_data_path` | Billing Budgets | `id` (Billing Account ID) | -| **data-catalog-policy-tag** | Data Catalog Taxonomy | `taxonomy` | Policy Tags | `project_id`, `location`, `name` (Taxonomy Name) | | **data-catalog-tag** | N/A | `tags` | Data Catalog Tags | `tags` (Merged with factory data) | | **data-catalog-tag-template** | N/A | `tag_templates` | Tag Templates | `project_id`, `region` | | **dataplex-aspect-types** | N/A | `aspect_types` | Aspect Types | `project_id`, `location` | @@ -76,21 +75,23 @@ The following table details how FAST stages implement factory patterns. | Stage | Factory (Key/Feature) | Implementation Type | Underlying Module/Resource | | :--- | :--- | :--- | :--- | | **0-org-setup** | `projects`, `folders`, `budgets` | Module-Backed (Factory) | `project-factory` | -| **1-vpcsc** | `access_levels`, `perimeters`, `policies` | Module-Backed (Factory) | `vpc-sc` | +| **1-vpcsc** | `access_levels`, `egress_policies`, `ingress_policies`, `perimeters` | Module-Backed (Factory) | `vpc-sc` | | **2-networking** | `vpcs` | Module-Backed (Factory) | `net-vpc-factory` | | **2-networking** | `projects` | Module-Backed (Factory) | `project-factory` | | **2-networking** | `dns` (Zones) | Stage-Implemented (Module) | `dns` | | **2-networking** | `dns_response_policies` | Stage-Implemented (Module) | `dns-response-policy` | | **2-networking** | `firewall_policies` | Stage-Implemented (Module) | `net-firewall-policy` | | **2-networking** | `vpns` | Stage-Implemented (Module) | `net-vpn-ha` | +| **2-networking** | `vlan_attachments` | Stage-Implemented (Module) | `net-vlan-attachment` | | **2-networking** | `ncc_hubs` | Stage-Implemented (Resource) | `google_network_connectivity_hub` | | **2-networking** | `ncc_groups` | Stage-Implemented (Resource) | `google_network_connectivity_group` | | **2-networking** | `nvas` | Native (Complex) | `compute-vm`, `net-lb-int` | | **2-project-factory** | `projects`, `folders`, `budgets` | Module-Backed (Factory) | `project-factory` | -| **2-project-factory** | `vpcs` | Module-Backed (Factory) | `net-vpc-factory` | | **2-security** | `projects` | Module-Backed (Factory) | `project-factory` | | **2-security** | `certificate_authorities` | Stage-Implemented (Module) | `certificate-authority-service` | | **2-security** | `keyrings` (KMS) | Stage-Implemented (Module) | `kms` | +| **3-data-platform-dev** | `aspect_types` | Module-Backed (Factory) | `dataplex-aspect-types` | +| **3-data-platform-dev** | `data_domains` | Native (Complex) | Multiple | | **3-secops-dev** | `rules`, `reference_lists` | Module-Backed (Factory) | `secops-rules` | ## Maintenance Guide diff --git a/GEMINI.md b/GEMINI.md index 3b994b9b5..0932e50e4 100644 --- a/GEMINI.md +++ b/GEMINI.md @@ -17,13 +17,15 @@ Cloud Foundation Fabric is a comprehensive suite of Terraform modules and end-to * Self-contained: Dependency injection via context variables is preferred over complex remote state lookups within modules. * Flat: avoid using sub-modules to reduce complexity and minimize layer traversals. * **Naming:** Avoid random suffixes; use explicit `prefix` variables. +* **Factories:** Many modules implement a data-driven "factory" pattern (often via a `factories_config` variable) to manage resources at scale using YAML data files. See `FACTORIES.md` for a comprehensive list. + * **Validation:** Factory YAML files must conform to JSON schemas (typically stored in a `schemas/` folder). Use a modeline (e.g., `# yaml-language-server: $schema=../schemas/project.schema.json`) to enable IDE validation. * **Usage:** Modules are designed to be forked/owned or referenced via Git tags (e.g., `source = "github.com/...//modules/project?ref=v30.0.0"`). ### 2. FAST (`/fast`) * **Purpose:** Rapidly set up a secure, scalable GCP organization. * **Architecture:** Divided into sequential "stages" (0-org-setup, 1-vpcsc, 2-security, 2-networking, etc.). -* **Factories:** Uses YAML-based "factories" (e.g., Project Factory) to drive configuration at scale. +* **Factories:** Extensively uses YAML-based datasets and module factory patterns to drive configuration at scale, acting as a "translation machine" that expresses different architectural designs without changing the underlying stage code. ### 3. Tools (`/tools`) @@ -61,6 +63,7 @@ terraform fmt -recursive modules/ python3 tools/check_documentation.py modules/ # Regenerate README variables/outputs tables when check fails +# Note: tfdoc uses special HTML comments () in READMEs. Do not manually edit these sections. python3 tools/tfdoc.py --replace modules/ # YAML linting @@ -74,7 +77,20 @@ python3 tools/check_boilerplate.py --scan-files #### 2. Testing -Tests are written in Python using `pytest` and the [`tftest`](https://pypi.org/project/tftest/) library. +Our testing philosophy is simple: test to ensure the code works and does not break due to dependency changes. **Example-based testing via `README.md` is the preferred approach.** + +Tests are triggered from HCL Markdown fenced code blocks using a special `# tftest` directive at the end of the block. + +```hcl +module "my-module" { + source = "./modules/my-module" + # ... +} +# tftest modules=1 resources=2 inventory=my-inventory.yaml +``` + +* **Inventory files (`YAML`):** Used to assert specific values, resource counts, or outputs from the terraform plan against an expected dataset. +* **Legacy Tests:** Python-based tests using `pytest` and `tftest` are supported but example-based tests should be used whenever possible. ```bash # Run all tests @@ -83,8 +99,8 @@ pytest tests # Run specific module examples pytest -k 'modules and :' tests/examples -# Run tests from a specific file -pytest tests/examples/test_plan.py +# Automatically generate an inventory file from a successful plan +pytest -s 'tests/examples/test_plan.py::test_example[terraform:modules/:Heading Name:Index]' ``` **Note:** `TF_PLUGIN_CACHE_DIR` is recommended to speed up tests. @@ -181,9 +197,24 @@ Modify one existing README example (do not add a new one) to demonstrate context ## Architecture & Conventions -* **Variables:** Prefer object variables (e.g., `iam = { ... }`) over many individual scalar variables. +* **Variables & Interfaces:** + * Prefer object variables (e.g., `iam = { ... }`) over many individual scalar variables. + * Design compact variable spaces by leveraging Terraform's `optional()` function with defaults extensively. + * Use maps instead of lists for multiple items to ensure stable keys in state and avoid `for_each` dynamic value issues. +* **Naming:** Never use random strings for resource naming. Rely on an optional `prefix` variable implemented consistently across modules. * **IAM:** Implemented within resources (authoritative `_binding` or additive `_member`) via standard interfaces. * **Outputs:** Explicitly depend on internal resources to ensure proper ordering (`depends_on`). * **File Structure:** * Move away from `main.tf`, `variables.tf`, `outputs.tf`. * Use descriptive filenames: `iam.tf`, `gcs.tf`, `mounts.tf`. +* **Style & Formatting:** + * **Line Length:** Enforce a 79-character line length limit for legibility (relaxed for long resource attributes and descriptions). + * **Ternary Operators & Functions:** Wrap complex ternary operators in parentheses and break lines to align `?` and `:`. Split function calls with many arguments across multiple lines. + * **Locals Separation:** Use module-level locals for values referenced directly by resources/outputs. Use block-level "private" locals prefixed with an underscore (`_`) for intermediate transformations. + * **Complex Transformations:** Move complex data transformations in `for` or `for_each` loops to `locals` to keep resource blocks clean. + +## Preferred Workflow + +- Always break down complex requests into small, iterative tasks. +- For each task, propose the necessary edits and explicitly wait for user confirmation or discussion before proceeding. +- Always use the `replace` tool to both perform and cleanly display the proposed text modifications. Do not silently overwrite files or use shell commands for text edits.