Repository navigation
chore(gha): adopt the new jsii-terraform image and test Terraform 1.5.7 + 1.16.1 - #398
Merged
so0k merged 3 commits intoSep 9, 2026
Merged
Conversation
jsteinich
force-pushed
the
chore/bump-jsii-terraform-digest
branch
from
September 8, 2026 11:05
4761a46 to
41f78d7
Compare
jsteinich
changed the title
7 of 8 tasks
This comment was marked as resolved.
This comment was marked as resolved.
6 of 8 tasks
…TF 1.16.1 Points all 27 pinned references across the 11 workflow files at the image published from 2f29d5a (open-constructs#394), which is the first one carrying OpenTofu 1.12.6, Terraform 1.5.7 and 1.16.1, the fail-fast install layers and the post-install binary verification. This is purely the digest bump. `tested` still runs 1.5.5 and 1.6.5, so no job counts or version coverage change here — that comes next, now that the binaries are actually present in the image CI runs. Digest confirmed to be the build of 2f29d5a: the immutable 0.24.0-2f29d5a8d45b047c5450eee9640e1cbaaee98c8d tag and :latest both resolve to sha256:62c00a31... Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1.5.7 is the newest pre-fork release and the exact floor cdktn already declares: DEFAULT_TARGET_VERSIONS in packages/cdktn/src/validations/target-versions.ts is `terraform: ">=1.5.7"`. The matrix was testing 1.5.5, which nothing declares support for. This is a rename of an existing leg, not a new one — same number of jobs, same cross-product. It relies on the image digest bumped in the previous commit, which is the first to carry a terraform1.5.7 binary. The two hardcoded terraform_version lists in pr-unit.yml and release.yml move with it. They duplicate `tested` from .terraform.versions.json rather than deriving from it; worth fixing separately so a version change is a one-file edit. Deliberately not included: moving the upper leg from 1.6.5 to 1.16.1, and `default` with it. `cdktn get` is broken from Terraform 1.15.7 on for any module declaring configuration_aliases (open-constructs#379); the fix is open in open-constructs#383 and is not in main, so raising the ceiling now would land red. That half should follow open-constructs#383. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Unblocked by open-constructs#383, which landed in 54a8217. `cdktn get` was broken from Terraform 1.15.7 on for any module declaring configuration_aliases (open-constructs#379), so until that fix was in main this half would have landed red. Like the floor-leg move, this renames the upper leg rather than adding one: `tested` goes from 1.5.7 + 1.6.5 to 1.5.7 + 1.16.1, so the integration cross-product keeps the same job count. Together the two legs now span the declared floor and current stable instead of two adjacent 2023 releases, which is what open-constructs#337 needed to exercise the newer-protocol schema sections at all. `default` moves to 1.16.1 as well. That is what the examples matrix builds against (tools/build-example-matrix.mjs reads `.default`) and, less obviously, what every provider-integration run uses: the terraform_version input in provider-integration.yml is defaulted rather than passed, and none of its three callers override it. The duplicated unit-test matrices in pr-unit.yml and release.yml move with it. `available` is deliberately left alone — 1.5.5 and 1.6.5 are now unreferenced, but pruning them only takes effect once the workflows repin to a rebuilt image, so it belongs with the next digest bump rather than here. Not touched: tools/generate-function-bindings/function-availability/ functions-matrix.json, which lists 1.6.5 among every other release as generated function-availability data, not as a CI version. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
so0k
force-pushed
the
chore/bump-jsii-terraform-digest
branch
from
September 9, 2026 09:27
41f78d7 to
2f51249
Compare
so0k
approved these changes
Sep 9, 2026
jsteinich
added a commit
that referenced
this pull request
Sep 12, 2026
### Related issue Follow-up from the review on #400, point 2. #400 is closed; this is the durable half of its regression coverage. ### Description `test/python/edge/test.ts` has been `describe.skip`'d since 3acb935 (**1 December 2022**), waiting on [aws/jsii#3866](aws/jsii#3866), with the stated intent to "re-add them when updating JSII". That fix shipped shortly after, and the JSII update has since landed (#395: jsii 6.0, jsii-pacmak 1.140.0) — but the skip was never lifted. Python is the only language whose edge test does not run. TypeScript, Go, Java and C# all do. That gap has a concrete cost: it is why #311's cross-language compile coverage never saw the broken Python provider import, which only surfaced once CI could reach Terraform >= 1.8 (#398) — and then via the `python-documentation` example rather than a test aimed at it. `main.py` calls `edge.provider.EdgeProvider(...)`, so `beforeAll`'s synth imports the generated `edge.provider` module — exactly the module whose `__init__.py` carried the unresolvable import. A regression there fails the suite loudly, at the import. ### Two stale assertions, and why they were wrong Re-enabling produced **18 passed / 2 failed**. The suite is substantially intact after three years dormant; both failures were expectations that had never once executed. **1. A typo.** The `numList` expectation read `...reqnum)}` — a stray closing paren — where synth produces `...reqnum}`. **2. A wrong assumption about `Fn.lookup`.** The map expectations assumed every `Fn.lookup(map, key, default)` renders as a Terraform `lookup()`. It does not. `Fn.lookup` (`packages/cdktn/src/terraform-functions.ts:41`) emits `lookup()` only when the default is **truthy**: ```ts static lookup(inputMap: any, key: string, defaultValue?: any) { if (defaultValue) return Fn._lookup(inputMap, key, [defaultValue]); return asAny(propertyAccess(inputMap, [key])); // -> renders inputMap[key] } ``` The fixture passes the same `Fn.lookup(...)` call for all six attributes, with different defaults: | attribute | default | truthy | renders as | | --- | --- | --- | --- | | `reqMap` | `false` | no | `${map_resource.map.reqMap.key1}` | | `optMap` | `"missing"` | yes | `${lookup(map_resource.map.optMap, "key1", "missing")}` | | `computedMap` | `0` | no | `${map_resource.map.computedMap.key1}` | So the deciding factor is the truthiness of the default, **not** whether the attribute is required, optional or computed. The corrected expectations match, and are now identical to `test/typescript/edge/test.ts`. Only the first failing assertion in each block is reported by jest, so the rest of the map block was corrected at the same time rather than surfacing them one CI run at a time. ### These pin current behaviour, not necessarily correct behaviour (#416) Worth flagging rather than burying: that truthiness guard looks like a bug. `Fn.lookup(map, "key", false)` reads as "return `false` if the key is missing", but the falsy default is discarded and the emitted expression becomes a bare property access — which errors on a missing key instead of returning the default. The guard should almost certainly be `defaultValue !== undefined`. Filed as #416. I have deliberately **not** changed it here. This PR is test coverage; altering `Fn.lookup` is a behavioural change to the construct library with its own blast radius, and it would be wrong to smuggle it in. If it is fixed, these expectations move with it — and at that point the edge tests will be the thing that proves the fix, which is rather the point of un-skipping them. Note this also means the equivalent TypeScript assertions were changed to match this behaviour in 09ac17b ("chore: fix assertions", Aug 2023) with no rationale recorded. ### Testing CI on this branch: **257 pass, 0 fail**, 1 skip (`windows_integration`, `if: false`). Both `python/edge` jobs pass, on Terraform 1.5.7 and 1.16.1. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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
Follow-up to #394. Step 2 of the sequencing agreed in #337. Rebased on #383, which unblocks the ceiling half.
Description
Three commits, all consequences of #394 having published a new image and #383 having fixed
cdktn geton modern Terraform.1. Bump the pinned image digest — all 27 references across 11 workflow files move to
sha256:62c00a31..., the image built from 2f29d5a. Confirmed to be that build::latestand the immutable0.24.0-2f29d5a8d45b047c5450eee9640e1cbaaee98c8dtag resolve to the same digest. First image carrying OpenTofu 1.12.6, Terraform 1.5.7 and 1.16.1, the fail-fast install layers and the post-install binary verification.2. Move the floor leg from Terraform 1.5.5 to 1.5.7 — 1.5.7 is the newest pre-fork release and the exact floor cdktn already declares.
DEFAULT_TARGET_VERSIONSinpackages/cdktn/src/validations/target-versions.tsis{ terraform: ">=1.5.7", opentofu: ">=1.6.0" }, so every project that declares notargetVersionsis told 1.5.7 is supported, while CI was testing 1.5.5 — a version nothing declares.3. Raise the ceiling from 1.6.5 to 1.16.1 — unblocked by #383 (54a8217).
cdktn getwas broken from Terraform 1.15.7 onward for any module declaringconfiguration_aliases(#379); until that fix was in main this would have landed red.Both version changes are renames of existing legs, not additions —
testedgoes from["1.5.5", "1.6.5"]to["1.5.7", "1.16.1"], so the integration cross-product keeps the same job count and the CI bill is unchanged. The two legs now span the declared floor and current stable instead of two adjacent 2023 releases, which is what #337 needed in order to exercise the newer-protocol schema sections (functions,ephemeral_resource_schemas,write_only) at all — the previous 1.6.5 ceiling structurally could not emit them.defaultmoves to 1.16.1 with it. That drives the examples matrix (tools/build-example-matrix.mjsreads.default) and — less obviously — every provider-integration run:provider-integration.yml'sterraform_versioninput is defaulted rather than passed, and none of its three callers override it. The duplicated unit-test matrices inpr-unit.ymlandrelease.ymlmove with it too.Deliberately not included
available. They are unreferenced now, but editing.terraform.versions.jsononly rebuilds the image on merge — the workflows stay pinned to the current digest, so the prune would not take effect until a later bump. It belongs with the next digest bump.tools/generate-function-bindings/function-availability/functions-matrix.jsonstill lists 1.6.5. That is generated function-availability data covering every release, not a CI version.Follow-ups
terraform_versionmatrices inpr-unit.ymlandrelease.ymlfrom.terraform.versions.jsoninstead of duplicatingtestedin three places, so a version change is a one-file edit.Checklist
🤖 Generated with Claude Code