Repository navigation
fix(tests): mark the terraform-cloud target Terraform-only in the matrix - #430
Merged
jsteinich merged 2 commits intoSep 14, 2026
Merged
Conversation
`test/typescript/terraform-cloud/test.ts` passes `TERRAFORM_VERSION` — the matrix axis — as the `terraformVersion` of the HCP Terraform workspace it creates. HCP Terraform runs Terraform remotely and exposes no OpenTofu equivalent through that API, so a non-Terraform runtime has no correct value to put there, and the resulting failure would say nothing about the cloud backend the test covers. Closes open-constructs#392. Since open-constructs#405 the matrix builder owns runtime selection per target, so the constraint belongs there. `terraformOnly` is checked against each target's *resolved* runtimes rather than against `pinnedRuntimes` alone, so both a pin and a hypothetical non-Terraform default axis fail at matrix-build time with a named target and product, before any job is scheduled. Rejected the two alternatives: - Skipping inside the test when the runtime is not Terraform trades a loud failure for a green job that asserted nothing. A pin added later would look like tofu coverage while proving nothing. - Pinning `terraformVersion` to a fixed release decouples the remote version from the local CLI the matrix selects. Today they match; hardcoding one side introduces deliberate version skew across the remote backend on every entry and collapses remote-version coverage to a single version, so both matrix entries would create identical workspaces. Terraform coverage is unchanged: the target still follows `tested` (1.5.7 and 1.16.1), and no emitted entry differs for any target. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Terraform-only guard added in the previous commit, and the pinnedRuntimes validation from open-constructs#405, are both keyed by test path. A key that matches no discovered target makes them silently inert - a typo in `terraformOnly` lets a tofu-pinned terraform-cloud entry through exactly as if the guard were not there, which defeats the point of adding it. Validated once against the discovered target list, after discovery, so both maps fail closed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
jsteinich
marked this pull request as ready for review
September 12, 2026 16:35
so0k
approved these changes
Sep 13, 2026
so0k
left a comment
Contributor
There was a problem hiding this comment.
No blocking or non-blocking findings.
tools/build-test-matrix.mjsvalidates bothpinnedRuntimesandterraformOnlykeys against Jest-discovered targets, preventing renamed or mistyped paths from silently disabling the configuration.- The Terraform-only check runs after runtime resolution, so it catches both an explicit OpenTofu pin and any future non-Terraform default runtime.
- The designation is consistent with
test/typescript/terraform-cloud/test.ts, which sendsTERRAFORM_VERSIONto HCP Terraform’s remote workspace API. - No workflow files or security-sensitive execution paths changed.
Contributor
|
I'm going to lock this pull request because it has been closed for 30 days. This helps our maintainers find and focus on the active issues. If you've found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related issue
Fixes #392. Unblocked by #405, which landed the per-test runtime pinning this builds on.
Description
test/typescript/terraform-cloud/test.tsfeedsTERRAFORM_VERSION— set from the matrix axis — into theterraformVersionattribute of an HCP Terraform workspace. If the matrix ever resolves that target to OpenTofu, the test asks HCP Terraform for a workspace pinned to a Terraform version that does not exist, and there is no correct value to pass: that API executes Terraform remotely and has no OpenTofu equivalent.The target is declared Terraform-only in the matrix builder, and the builder throws if it ever resolves otherwise:
The check runs against each target's resolved runtime rather than against
pinnedRuntimes, so it catches both a pin and a hypothetical future non-Terraform default axis. Filtering the default axis instead would have silently dropped the target — the same class of quiet misconfiguration the issue is about.Why not the other two options the issue lists
Skip inside the test trades a loud failure for a green job that asserted nothing. If someone later pinned this target to tofu, CI would read as OpenTofu coverage while proving nothing.
Hardcode
terraformVersionto a fixed release is the one worth pushing back on. TodayTERRAFORM_VERSIONmakes the workspace's remote Terraform version match the local CLI the matrix selected. Hardcoding one side introduces deliberate local/remote skew on every entry that isn't the hardcoded version, and collapses remote-version coverage to one version — so both matrix entries would create identical workspaces and half the jobs become redundant. It also would not settle a tofu pin: you would still be driving a remote-execution Terraform workspace withtofu, failing confusingly rather than loudly.terraform-cloud/test.tsis untouched.Second commit: making the guard trustworthy
Both this guard and
pinnedRuntimesfrom #405 are keyed by test path, and a key matching no discovered target makes them silently inert. Verified: with a typo'dterraformOnlykey, a tofu-pinnedterraform-cloudentry sails through exactly as if the guard were absent. That defeats the purpose of adding it, so the keys are now validated once against the discovered target list, after discovery, and both maps fail closed.Testing
tools/build-test-matrix.mjsdoes not run natively on Windows (execFileSync("npx")needs.cmd; thetestDirprefix strip assumes forward slashes), so the real logic was run against a stubbed discovery list — only theexecFileSyncblock is replaced, with stubs built using the same`${testDir}/`prefix so the prefix-strip path is still exercised. Pins, guards, HCL detection and binary naming are untouched source.terraform-cloudgetsterraform1.5.7+terraform1.16.1terraform-cloudError: typescript/terraform-cloud/test.ts is Terraform-only, but the matrix resolved it to opentofu 1.12.6.terraformOnlykeyError: "…/tests.ts" is configured in pinnedRuntimes or terraformOnly but is not a discovered test target.prettier --checkandnode --checkclean.Latent coupling elsewhere: checked, this is the only one
A repo-wide sweep for
TERRAFORM_VERSIONfoundtest/typescript/terraform-cloud/test.ts(lines 7 and 47) to be the only test consumer; every other hit is a producer (integration.yml,provider-integration.yml) or a Docker build arg. A sweep of everyprocess.envread undertest/turned up onlyTF_EXECUTE_LOCAL,TF_VAR_myvar,SWITCH_STACK,USE_CONSTRAINED_FUNCTIONandTERRAFORM_CLOUD_*— none version- or product-coupled. It is also the only target touchingapp.terraform.io.Not verified
The test itself cannot be run here: it needs
TERRAFORM_CLOUD_TOKENand talks to HCP Terraform. The stubbed harness proves the selection logic, not jest's discovery — so that the real target list is unchanged, and that the path key matches whatjest --listTestsemits, are confirmed by the first CI run ofprepare-integration-tests. The key format is the same one the existingprovider-featurespin uses, and that pin resolves correctly in these runs.Adjacent, deliberately not changed
examples.yml:171,pr-unit.yml:173,unit.yml:80andprovider-integration.yml:102still string-concatenate"terraform${{ … }}"forTERRAFORM_BINARY_NAME, whereintegration.ymlnow usesmatrix.binary. Those workflows have no OpenTofu axis today, so it is latent rather than broken.🤖 Generated with Claude Code