Repository navigation
feat: expose stack output values as the stackOutputs output - #12
Conversation
auto-approve-apply now runs `cdktn apply --outputs-file <file>` with the file under RUNNER_TEMP, and after a successful apply sets an `outputs` action output with the stack outputs as a JSON string. Later steps or the job's environment URL can use them, e.g. `fromJSON(steps.cdktn.outputs.outputs).my-stack.url`, without parsing the console output. Sensitive outputs are left out, as with the CLI's own default. Other modes don't write an outputs file and don't set the output. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Avoids the awkward steps.<id>.outputs.outputs path and confusion with the existing output (stdout/stderr) output. The name is unreleased, so this is not a breaking change.
stackOutputs output
so0k
left a comment
There was a problem hiding this comment.
Thanks for this, it's a useful addition. Your branch allows maintainer edits, so I've pushed a few commits on top rather than going back and forth on review:
- Renamed the output from
outputstostackOutputs(14fcc19). Every step output already lives understeps.<id>.outputs.<name>, so the old name gave workflowssteps.cdktn.outputs.outputs, right next to the existingoutput(combined stdout/stderr) output, which made the two easy to mix up.stackOutputsmatches the camelCase ofexitCode. Since the name isn't released yet, changing it now costs users nothing; after a release it would be a breaking change. I've updated the PR title and description to match. - Added
run()tests (0aeaa0c) with@actions/coreand@actions/execmocked. They check that a successful apply publishes the outputs as compact JSON, and that a failed apply or a plan publishes nothing. The helper tests already covered the pieces; these prove the wiring inrun(). - Comment and docs pass (
eefbf88,4981d53): tighter JSDoc, a note that cdktn-cli 0.24.0 and cdktf-cli 0.21.0 both support--outputs-fileand leave sensitive outputs out by default, and README/action.ymlnow say the JSON is keyed by stack name and set only after a successfulauto-approve-apply.dist/is rebuilt with the lockfile's rolldown. - Added a test-audit skill for agent-assisted test work (
e28a81d). It's repo tooling only and doesn't affect the action.
|
The In particular, Could we align the skill with this repository before merging it? Concretely:
The core |
…r tests The compact-JSON helper test duplicated the run() apply test; the missing-file case moves up to run(), so readStackOutputs no longer needs exporting.
…sing (#375) ### Related issue No GitHub issue. Related: #361 / #410 (the top-level failure path), which is why this crash surfaced as a raw stack trace. ### Description A `cdktn deploy` that applied successfully could then crash while printing its outputs: ``` TypeError: Cannot convert undefined or null to object at Object.entries (<anonymous>) at renderNested (helper/format.ts) at renderOutputs (helper/format.ts) at runDeploy (ui/deploy.ts) ``` #### Root cause `getConstructIdsForOutputs` maps the synthesized metadata (`"//".outputs` in `cdk.tf.json`) onto the result of `terraform output -json` with an unchecked lookup: ```ts return { ...acc, [key]: outputs[value] }; // undefined when terraform did not return it ``` An output declared in the metadata but absent from Terraform's result (state drift, `--skip-synth` against older state, an output renamed or removed between applies) was kept with the value `undefined`. A group is only pruned when every output in it is missing, so a partial miss survived and the renderer then called `Object.entries(undefined)`. The data path is unchanged since before 0.23. It surfaced in 0.24.0 because #264 removed the Ink renderer, whose `try/catch` used to turn this throw into a one-line message. #### The fix - **Protect at the source.** `getConstructIdsForOutputs` omits an output Terraform did not return (`undefined` or `null`) and logs one warning per stack naming the stack and every omitted output, by construct path and terraform output name: ``` 2 outputs declared by stack "app" were not returned by terraform and are omitted: app/network/a (a), app/db/port (db_port_ABC123) ``` The check is explicit, not a truthiness test, so an output whose value is `""`, `0` or `false` still renders. There is no distinction between cross-stack and user-declared outputs. - **`isTerraformOutput` excludes `null`**, where it previously threw on `null.sensitive`. - **One helper presents outputs for both commands.** `presentOutputs` (`cmds/helper/outputs.ts`) is used by `runDeploy` and `runOutput`: - no outputs: prints `No outputs found.`; - otherwise renders the table; a render failure is reported on stderr and does not fail the command, since its real work already succeeded; - with `--outputs-file`: prints the `written to` line, independent of whether the table rendered. The renderer (`helper/format.ts`) and `unpackTerraformOutput` are unchanged from `main`. Neither command can hand the renderer an `undefined` leaf any more, for two different reasons: - `deploy` presents the construct-id-mapped outputs, where the omission above now applies. - `output` presents the raw `terraform output -json` map (see *Scope* below), in which every key Terraform returns is a full `{ value, type, sensitive }` object. #### Scope: `deploy` and `output` present different maps `presentOutputs` is shared, but its two callers pass semantically different maps, and this PR keeps that as it is on `main`: - `runDeploy` presents `CdktfProject.outputsByConstructId`, mapped through the stack metadata and nested by construct path. - `runOutput` presents `CdktfProject.fetchOutputs()`, which returns each stack's raw terraform outputs (`CdktfStack.fetchOutputs` computes the mapped form for its state update and the warning, but returns `this.outputs`). So for a nested `TerraformOutput`, `deploy --outputs-file` writes `{"stack":{"parent":{"child":…}}}` while `output --outputs-file` writes `{"stack":{"parent_child_<HASH>":…}}`. Unifying them changes a file shape that users read, and deploy's shape is the one consumers depend on (cdktn-io/action#12 documents `stackOutputs` from `cdktn apply --outputs-file` as `.my-stack.url`), so it is tracked separately in #525 rather than changed here. #### Behaviour notes - With `--outputs-file` and no outputs, the CLI now prints `No outputs found.` followed by the `written to` line. The file is written in that case (it contains `{}`), so the line is true; `main` printed only `No outputs found.`. - The warning is logged once per stack, and again on each `watch` redeploy while the state stays out of date. It is also logged by `cdktn output`, which fetches the same way. - **Known limitation:** in an interactive terminal the warning can be garbled. It is a direct log4js write to stdout while the progress bar is live, and the bar's next repaint can overwrite that row. Non-TTY and CI output is unaffected. Routing it through the renderer means carrying the notice on the state update to the UI layer, which is more than the warning is worth here. #### Tests - Mapping: partial miss, whole group missing, `null`, the aggregated warning naming the stack and each output, pass-through without metadata. - Stack execution: on both `deploy` and `fetchOutputs`, a real `CdktfProject` warns with the stack's own name. - Helper: empty and undefined outputs, the table, a throwing render, the `written to` line with and without a path. - Per command: thin wiring tests (including the options `runOutput` forwards to `fetchOutputs`), plus the end-to-end regression (a missing output does not crash a successful deploy). Each test was checked against a broken source: removing the omission, the `null` handling, the warning, the empty check or the render guard fails at least one of them. --- #### Stacked PR #376 builds on this branch and makes the `--outputs-file` write awaited, inside the same helper. ### Checklist - [x] I have updated the PR title to match [CDKTN's style guide](https://github.com/open-constructs/cdk-terrain/blob/main/CONTRIBUTING.md#pull-requests-1) - [x] I have run the linter on my code locally - [x] I have performed a self-review of my code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the [documentation](https://github.com/open-constructs/cdk-terrain-docs/tree/main/content) if applicable — n/a, no documented behaviour changes - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works if applicable - [x] New and existing unit tests pass locally with my changes
Adds a
stackOutputsaction output. Inauto-approve-applymode, the action now runscdktn apply <stack> --auto-approve --outputs-file <file>, with the file underRUNNER_TEMP. After a successful apply it setsstackOutputsto the stack outputs as a JSON string.Why: a common use is turning stack outputs into the job's deployment URL, or passing them to later steps:
Today that means parsing the
outputconsole text.hashicorp/terraform-cdk-actiondidn't support this either. Some forks added it by passing--outputs-filethe same way.Behaviour:
auto-approve-applywrites the file and setsstackOutputs; the other modes are unchanged.stackOutputsisn't set. If no file was written, it's{}.--outputs-file-include-sensitive-outputsisn't passed).applyis an alias ofdeployincdktn-cli, which has the--outputs-fileoption.Changes:
engineCommandandbuildNpxArgstake the optional outputs file;readStackOutputshelper;action.ymland a README row with a usage example;dist/.npm run allpasses: check, typecheck, 36 tests and the bundle.🤖 Generated with Claude Code