test: cover TAG building block inputs end to end - #305
Conversation
The noop building block is the reference implementation of meshStack's Terraform interface, and TAG inputs are the one input source it did not cover. Three nullable list variables — one per taggable meshObject a tenant-level building block can read — plus their entries in the debug output the e2e tests assert on. No definition passes them yet, so they resolve to null and the existing expected fixture just gains three null keys. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A second definition is about to run the same building block, and that building block's variables have no defaults — so both definitions have to declare the identical input set. Sharing it is what keeps them in step. Pure move: `tofu validate` passes and the definition is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A TAG input reads a tag from the workspace, project, payment method or landing zone the building block belongs to. A workspace-level definition has no project, so only a tenant-level one reaches the latter three — hence a second definition rather than three more inputs on the existing one. It stays off unless a consumer names the tag keys, so nothing changes for anyone importing the module today. The keys are named rather than discovered because a definition stores only the key; meshStack resolves the value per building block. Needs provider 0.25.2, which is where TAG inputs landed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The three tags under test sit on a project, a payment method and a landing zone, and every one of them has to be owned by the test: the provider has no meshstack_project_tag or meshstack_payment_method_tag resource, so a shared fixture object cannot be tagged at all. A custom platform makes that chain reachable with no cloud behind it. The one catch is the tenant: the provider treats it as created once spec.platform_tenant_id is set, and a custom platform has no replicator to set it — hence the mandatory manual building block, which is how the provider's own landing zone tests solve the same problem. Two payment methods, because reassigning which one funds the project is the cheapest of meshStack's four tag-source reassignment triggers. Needs an admin-scoped API key: ADM_TAGDEFINITION_SAVE and ADM_PAYMENTMETHOD_SAVE have no workspace-scoped variant. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three runs sharing one file's state, which is new for this repo: a tag input can only be seen to follow a tag if the tag changes under a building block that already exists. The runs prove that the value a tag holds now reaches the next run. They do not prove the tag edit alone triggered it — the provider exposes no way to attribute a run to an edit, so each run bumps an input the provider does track and waits for a run of its own. The third run edits no tag at all and still sees a new value, because the project is funded from another payment method. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ange The rule against several run blocks in one file gave two concrete reasons, and neither one applies to a test whose subject is a change to a live building block's surroundings — a tag it reads moving under it. Spelling that out is what keeps the exception from being read as permission to share state in general. It also records the two things such a file has to get right, both of which cost a debugging round: bump an input the provider tracks, and let meshStack's own tag-triggered run get out of the way first. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Scorecard Check
📊 meshstack-hub Module Scorecard
📋 Per-Module Category SummaryScore per category per building block.
Core Structure — some checks failingBasic module file structure and documentation — applies to 1 modules
Core Structure — Summary
Integration — ✅ all passingmeshstack_integration.tf conventions — applies to 1 modules
Integration — Summary
AWS Backplane — not applicableAWS automation principal conventions (WIF or cross-account StackSet) — applies to 0 modules No applicable modules. Azure Backplane — not applicableAzure UAMI-based automation principal conventions — applies to 0 modules No applicable modules. GCP Backplane — not applicableGCP workload-identity-federation automation principal conventions — applies to 0 modules No applicable modules. STACKIT Backplane — not applicableSTACKIT WIF-based automation principal conventions — applies to 0 modules No applicable modules. Testing — ✅ all passingEnd-to-end test coverage — applies to 1 modules
Testing — Summary
|
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
| no external locking or concurrency group needed. Verify assumptions like this against the OpenTofu | ||
| version in use rather than trusting them. | ||
|
|
||
| ### The exception: a test whose subject *is* a change |
There was a problem hiding this comment.
d: I am not sure how helpful this text below is. I find its rather hard to read and understand. And basically only tells us that an output can change if a tag value somewhere else changes... I can also remove it if you think its nonesense here?
There was a problem hiding this comment.
yes, remove it, it doesn't add much value. you could move such details as README.md to the e2e/tag-inputs folder or split it up as succinct comments for the tf files?
grubmeshi
left a comment
grubmeshi
left a comment
There was a problem hiding this comment.
in general, nice test and i like the scenarios, i think though that we can simplify it and make it more maintainable in the future. see my detailed comments and huddle me in case of questions :D
| no external locking or concurrency group needed. Verify assumptions like this against the OpenTofu | ||
| version in use rather than trusting them. | ||
|
|
||
| ### The exception: a test whose subject *is* a change |
There was a problem hiding this comment.
yes, remove it, it doesn't add much value. you could move such details as README.md to the e2e/tag-inputs folder or split it up as succinct comments for the tf files?
| - [ ] One `.tftest.hcl` file per variant, pinning the flag in a file-level `variables` block — never several `run` blocks sharing one file's state | ||
| - [ ] …unless the subject of the test *is* a change to a live building block's surroundings — then one file with shared state, and see [The exception](#the-exception-a-test-whose-subject-is-a-change) for the two things it must get right |
There was a problem hiding this comment.
f: merge those two into one clear instruction and not into "do this ... unless"
or just relax the requirement of "one tftest.hcl per variant"?
|
|
||
| variable "project_tag" { | ||
| type = list(string) | ||
| default = null |
There was a problem hiding this comment.
default = null is redundant afaik, as nullable = true, or is terraform issuing a warning here?
concerning best practice, and to avoid nullability issues, maybe a default = [] is better here?
| locals { | ||
| # A platform type identifier can never be reused, not even after the type is deleted, so the run | ||
| # suffix is what keeps repeated and concurrent runs from colliding. Uppercase and dashes only. | ||
| platform_type_name = "NOOP-TAG-${var.test_context.name_suffix}" |
There was a problem hiding this comment.
f: is the name_suffix guranteed to be uppercase? maybe ensure this here using upper here?
| } | ||
|
|
||
| mandatory_building_block_refs = [ | ||
| { uuid = meshstack_building_block_definition.platform_tenant_id.metadata.uuid } |
There was a problem hiding this comment.
f: i think that should work now (ref all the things!!!):
| { uuid = meshstack_building_block_definition.platform_tenant_id.metadata.uuid } | |
| meshstack_building_block_definition.platform_tenant_id.ref |
| condition = contains(["initial", "changed_values", "reassigned_payment_method"], var.scenario) | ||
| error_message = "scenario must be one of initial, changed_values or reassigned_payment_method." |
There was a problem hiding this comment.
f: error_message can be dropped as condition is clear enough? that is similar to refining code comments, btw
| condition = contains(["initial", "changed_values", "reassigned_payment_method"], var.scenario) | |
| error_message = "scenario must be one of initial, changed_values or reassigned_payment_method." | |
| condition = contains(["initial", "changed_values", "reassigned_payment_method"], var.scenario) |
|
|
||
| variable "tag_settle_duration" { | ||
| type = string | ||
| default = "0s" |
There was a problem hiding this comment.
| default = "0s" | |
| nullable = false |
make the caller think about this input (note that nullable is true by default)
| @@ -1,12 +1,15 @@ | |||
| { | |||
| "flag": true, | |||
| "landing_zone_tag": null, | |||
There was a problem hiding this comment.
f: can be make this somehow non null (by reusing the code from the new scenarios?) it feels it's not a good assertion asserting nothing, in particular all other things here have some dummy value.
| project_tag_key = string | ||
| payment_method_tag_key = string | ||
| landing_zone_tag_key = string | ||
| platform_type_name = string |
There was a problem hiding this comment.
d: are we resuing some smoke test platform here? that would be ok, but don't use a platform which happens to be there "by chance" in meshtack-dev.
| # Only a tenant-level building block has a project, a payment method and a landing zone to read tags | ||
| # from — a workspace-level one can reach workspace tags and nothing else. So the TAG inputs need a | ||
| # definition of their own. It runs the same building block; the three inputs are the only difference. | ||
| resource "meshstack_building_block_definition" "tenant_tag_inputs" { |
There was a problem hiding this comment.
i'd aactually change the existing BBD and extend it with the three things you mention. that improves "coverage" for the already existing noop test run and saves us from having two BBDs, which is also somewhat surprising as "normal" meshstack_integration.tf files have only one BBD targeted at the buildingblock/ folder, which is a design principle I don't want to sacrifice for a test tbh.
2 participants
Why
meshStack now lets a building block definition input read a meshStack tag
(
assignmentType = TAG), and re-resolves it when the tag value changes or the object it isread from is reassigned. Nothing in the hub used the feature —
assignment_type = "TAG"appeared zero times — so nothing proved it works the way an end user meets it.
This adds the first e2e coverage, and the TAG input source the NoOp reference building block
was missing.
What the test covers
A
TENANT_LEVELbuilding block reading three tag targets, in one file whose runs share state:tag_inputs_resolve_on_createtag_value_changes_resolve_on_the_next_runreassigned_payment_method_resolves_its_own_tagpayment_method_tagfollows the new source, the other two are untouchedThe third case is meshfed's
onProjectPaymentMethodChanged, the cheapest of the fourreassignment triggers to exercise.
WORKSPACEis deliberately out of scope: tagging the shared test workspace needsmeshstack_workspace_tag, which the provider's own docs mark "not recommended for generaluse" (whole-workspace read-modify-write, races within a single apply).
What it proves, and what it does not
It proves the value a tag holds now reaches the next building block run. It does not prove
the tag edit alone triggered that run. meshStack does trigger one, but the provider exposes no
way to attribute a run to an edit —
status.latest_run_uuidexists, butReadnever blocks,so a refresh reads stale outputs. Each run therefore bumps an input the provider does track,
which makes it issue an update and await a run of its own. Both the test file and the skill
amendment say so rather than overclaiming.
Two design points worth a look
The fixture owns everything it tags. There is no
meshstack_project_tagormeshstack_payment_method_tagresource, so a shared fixture project or payment method cannotbe tagged at all — reusing
fixtures.<cloud>.mesh_tenant_idwould have covered none of thethree targets. A custom platform makes the whole chain reachable with no cloud behind it.
The tenant needs a platform tenant ID from somewhere. The provider counts a tenant as
created once
spec.platform_tenant_idis set, and a custom platform has no replicator to setit. The fixture adds a mandatory manual building block whose only output is assigned
PLATFORM_TENANT_ID— the same approach the provider's own landing zone tests use — sowait_for_completion = truestays and teardown does not race.Commits
Ordered for commit-by-commit review; each one is self-consistent and leaves the existing three
noop e2e tests green.
feat: read tag inputs in the noop building block— three nullable list variables, theirentries in the debug output, the expected-JSON fixture. Nothing wires them up yet.
refactor: lift the noop definition's inputs and outputs into locals— pure move, so thenext commit reads as "one new definition" rather than a 200-line diff.
feat: add an opt-in tenant-level noop definition with TAG inputs— off unless a consumernames the tag keys, so nothing changes for anyone importing the module today.
test: add a self-contained meshTenant fixture for the tag e2e— the wholee2e/tag-inputs/module, no assertions yet, runnable by hand withtofu apply.test: assert tag inputs resolve on create, on edit and on reassignmentdocs: allow one e2e test file to share state when the subject is a changeVerified locally
v0.25.2, resolved from the registry).The provider's TAG validator runs at
tofu validatetime, which confirms all three targetsare accepted on a
TENANT_LEVELdefinition and rejected onWORKSPACE_LEVEL.tofu fmtclean, no trailing whitespace,ci/validate_modules.shreports only thepre-existing missing-
pngquantwarning.mainfor every module.MESHSTACK_ENDPOINT.Not verified — needs a real run
No live meshStack was available. Four things could still block the first run:
ADM_TAGDEFINITION_SAVEand
ADM_PAYMENTMETHOD_SAVE; neither has a workspace-scoped variant. If the smoke-test keyis not admin-scoped the fix is a permission grant, not a redesign — there is no alternative
design, per the point about ownership above.
provider's acceptance tests, not on the dev instance.
tag_settle_duration = "180s"is a guess at how long meshStack's own tag-triggered runneeds to get out of the way. If an update during an in-flight run turns out not to 409, drop
it to
"0s"and save six minutes per run.confidentiality/environmentlocals are copied frommodules/stackit/project/e2e. A mandatory payment method tag on that instance would needadding too.
Also unrelated and left alone:
modules/meshstack/noop/backplane/versions.tfuses~> 4.0/~> 7.0/~> 0.12, which is the module's pre-existing scorecard failure.🤖 Generated with Claude Code