From 15460813c975f524c73175f3882f7b26b2e08e8b Mon Sep 17 00:00:00 2001 From: Johannes Rudolph Date: Mon, 7 Sep 2026 18:33:33 +0200 Subject: [PATCH 1/3] refactor(meshstack/noop): take the runner's GCP project from test_context MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The runner e2e module read its GCP project from TF_VAR_gcp_project_id. That is an environment fact the smoke-test harness already publishes in test_context (fixtures.gcp.project_id), and keeping it as an env var means it is wired in two places that nothing checks against each other — which is how this test broke silently once already. The harness keeps the Actions variable wired only because this module declares the root variable, so dropping it here is what lets that go too. The meshStack endpoint stays a root variable: it is not secret either, but the harness does not publish it in test_context yet. Co-Authored-By: Claude Opus 5 --- modules/meshstack/noop/e2e/runner/main.tf | 2 +- modules/meshstack/noop/e2e/runner/provider.tf | 2 +- modules/meshstack/noop/e2e/runner/variables.tf | 14 +++++++++----- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/modules/meshstack/noop/e2e/runner/main.tf b/modules/meshstack/noop/e2e/runner/main.tf index 36738312..7616bfe5 100644 --- a/modules/meshstack/noop/e2e/runner/main.tf +++ b/modules/meshstack/noop/e2e/runner/main.tf @@ -3,7 +3,7 @@ module "backplane" { meshstack_workspace_identifier = var.test_context.workspace meshstack_endpoint = var.meshstack_endpoint - gcp_project_id = var.gcp_project_id + gcp_project_id = var.test_context.fixtures.gcp.project_id gcp_region = var.gcp_region gcp_resource_name_prefix = "noop-runner-${var.test_context.name_suffix}" runner_display_name = "smoke-test-noop-runner-${var.test_context.name_suffix}" diff --git a/modules/meshstack/noop/e2e/runner/provider.tf b/modules/meshstack/noop/e2e/runner/provider.tf index 7e8eebd1..cf5cded0 100644 --- a/modules/meshstack/noop/e2e/runner/provider.tf +++ b/modules/meshstack/noop/e2e/runner/provider.tf @@ -1,4 +1,4 @@ provider "google" { - project = var.gcp_project_id + project = var.test_context.fixtures.gcp.project_id region = var.gcp_region } diff --git a/modules/meshstack/noop/e2e/runner/variables.tf b/modules/meshstack/noop/e2e/runner/variables.tf index 6b9240f5..07d71f83 100644 --- a/modules/meshstack/noop/e2e/runner/variables.tf +++ b/modules/meshstack/noop/e2e/runner/variables.tf @@ -4,21 +4,25 @@ variable "test_context" { workspace = string project = string name_suffix = string + + # Project the runner's Cloud Run service and Secret Manager secrets live in. + fixtures = object({ + gcp = object({ + project_id = string + }) + }) }) nullable = false } -variable "gcp_project_id" { - type = string - description = "GCP project ID for the runner Cloud Run service and Secret Manager secrets." -} - variable "gcp_region" { type = string default = "europe-west1" description = "GCP region for the Cloud Run service and Secret Manager replicas." } +# Not secret, so this belongs in test_context (see the e2e-test skill) — but the harness does not +# publish it there yet; it reaches CI through the secret pipe. Move it over once it does. variable "meshstack_endpoint" { type = string description = "Base URL of the meshStack API. Written into the runner config for API polling." From 1103dd90f1569cac810b089db1758e5afff845e2 Mon Sep 17 00:00:00 2001 From: Johannes Rudolph Date: Mon, 7 Sep 2026 18:33:45 +0200 Subject: [PATCH 2/3] docs(e2e-test): no fixture comes from the environment Record the rule the change above follows: an e2e module takes every environment fact from test_context, and only secrets arrive as TF_VAR_*. Secrets stay the exception because GitHub masks values one by one, so they cannot ride in a single grab-bag object. Co-Authored-By: Claude Opus 5 --- .agents/skills/e2e-test/SKILL.md | 5 +++++ AGENTS.md | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/.agents/skills/e2e-test/SKILL.md b/.agents/skills/e2e-test/SKILL.md index d459909d..50e7171a 100644 --- a/.agents/skills/e2e-test/SKILL.md +++ b/.agents/skills/e2e-test/SKILL.md @@ -92,6 +92,10 @@ Conventions that keep this clean and correct: statically evaluated) in both modes. - **Cloud resource IDs live under `fixtures`** (e.g. `var.test_context.fixtures.stackit.project_id`), never as a flat top-level field. +- **A non-secret environment fact always arrives in `test_context`, never as a `TF_VAR_*`.** A + `TF_VAR_*` fixture must be wired in the harness workflow as well, and nothing checks that the two + still agree — that is how the `meshstack/noop` runner test broke silently once. Secrets are the + one exception, for the reason below. - **`test_context` describes the environment, not the test case.** A flag that selects *which variant of the module under test to build* (e.g. a sync vs async implementation) does not belong in `test_context` — it belongs in a **root variable of the `e2e/` module**, pinned per test file. See @@ -489,6 +493,7 @@ source setup-override-provider.sh - [ ] `fixtures` is `optional()` with its inner shape fully required (no half-populated fixtures) - [ ] Always-shared fields (`workspace`, `name_suffix`, `hub_git_ref`) are required, not `optional()` - [ ] Cloud resource IDs sourced from `var.test_context.fixtures.*` (not flat `test_context` fields) +- [ ] No fixture read from the environment — only secrets arrive as `TF_VAR_*` - [ ] Scalar secrets are top-level `nullable` vars with `default = null` (foundation mode omits them) - [ ] Module sourced via relative path (not a GitHub URL), gated with `count = var.test_context.bbd_version_ref == null ? 1 : 0` - [ ] `hub.git_ref = var.test_context.hub_git_ref` — no hardcoded `"main"` diff --git a/AGENTS.md b/AGENTS.md index 91ab8468..9800f20e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -472,6 +472,10 @@ the diagram checklist. Modules that can be smoke-tested against a live meshStack instance should include an `e2e/` directory alongside the module root. +An `e2e/` module takes every environment fact from `var.test_context`; only secrets arrive as +scalar `TF_VAR_*`. A `TF_VAR_*` fixture has to be wired in the harness repo as well, which is a +second place for it to go missing. + See [.agents/skills/e2e-test/SKILL.md](.agents/skills/e2e-test/SKILL.md) (the `e2e-test` skill) for the full e2e testing conventions, including the `e2e/` structure, `test_context` wiring, `e2e/main.tf` and `*.tftest.hcl` conventions, the new-test checklist, and how to run and debug tests via the smoke-test runner. From d444d661b5f2eebfa4d4f5cea6271ef5b9334336 Mon Sep 17 00:00:00 2001 From: Johannes Rudolph Date: Mon, 7 Sep 2026 21:44:52 +0200 Subject: [PATCH 3/3] doc: clarify secret handling --- .agents/skills/e2e-test/SKILL.md | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/.agents/skills/e2e-test/SKILL.md b/.agents/skills/e2e-test/SKILL.md index 50e7171a..bd421b04 100644 --- a/.agents/skills/e2e-test/SKILL.md +++ b/.agents/skills/e2e-test/SKILL.md @@ -91,11 +91,8 @@ Conventions that keep this clean and correct: - **Always-shared fields are required**: `workspace`, `name_suffix`, and `hub_git_ref` are used (or statically evaluated) in both modes. - **Cloud resource IDs live under `fixtures`** (e.g. `var.test_context.fixtures.stackit.project_id`), - never as a flat top-level field. -- **A non-secret environment fact always arrives in `test_context`, never as a `TF_VAR_*`.** A - `TF_VAR_*` fixture must be wired in the harness workflow as well, and nothing checks that the two - still agree — that is how the `meshstack/noop` runner test broke silently once. Secrets are the - one exception, for the reason below. + never as a flat top-level field. This ensures we have one common union type of fixture inputs reusable + across our hub modules. - **`test_context` describes the environment, not the test case.** A flag that selects *which variant of the module under test to build* (e.g. a sync vs async implementation) does not belong in `test_context` — it belongs in a **root variable of the `e2e/` module**, pinned per test file. See @@ -105,13 +102,13 @@ Conventions that keep this clean and correct: ### Secrets -**No secret is ever a `test_context` field.** The grab-bag is built from state a CI job can read, so -a secret in it would have to be persisted somewhere it does not belong. +**No secret go into the `test_context` field.** The test_context object is built from a tofu state +read and this must not store secrets. A secret reaches the module one of two ways: -- The provider reads it from **its own standard environment variable** (cloud credentials). The - module declares nothing. +- The provider reads it from **its own standard environment variable**. The + module declares nothing and we rely on the e2e test harness to setup the environment accordingly. - The module declares a **flat root variable** for it, when the value is also needed as an input to the module under test (e.g. `stackit_git_forgejo_token`, `github_app_private_key`). The smoke-test runner exports every secret it holds as `TF_VAR_`, so declaring the variable is all it takes