Skip to content

feat(cli): v0.3 generate terraform/opentofu (phase 1 — check/notification/team, import + secrets-as-vars) - #2

Closed
pawel-cygal wants to merge 13 commits into
impl/v0.2-crudfrom
impl/v0.3-generate
Closed

feat(cli): v0.3 generate terraform/opentofu (phase 1 — check/notification/team, import + secrets-as-vars)#2
pawel-cygal wants to merge 13 commits into
impl/v0.2-crudfrom
impl/v0.3-generate

Conversation

@pawel-cygal

Copy link
Copy Markdown
Member

Phase 2 of the "100%" CLI vision (v0.2 CRUD → v0.3 generate → v0.4 export). Spec/plan in docs/superpowers/.

Stacked on impl/v0.2-crud (PR #1) — base is v0.2, since it uses the v0.2 registry read closures. Retarget to main after v0.2 merges.

Adds syschecks generate terraform|opentofu --org <org> [--type|--project|--check] --out ./iac/ — renders an account's live resources into HCL + import{} blocks the published systeampl/systeam provider adopts on plan (goal: no-op). This phase covers check, notification-channel, team; the rest of the 15 types + Pulumi/Ansible are v0.3.x.

How it works

  • New internal/generate package: a per-resource attribute spec (schema.go) — the provider's writable attributes, with Secret derived from the provider's Sensitive:true and JSON-normalized (jsontypes) blob attrs excluded (they'd render as HCL objects into string attrs and break plan); an HCL renderer (hcl.go value/label + render.go resource/import/variable) with context-sensitive ${/%{ escaping and collision-free labels.
  • The command reads via the v0.2 registry (list/get only — read-only, never mutates the account), renders per-type files + imports.tf, and emits secrets as sensitive variables in variables.tf (never inlined; also redacts any backend-masked "******" value). terraform/opentofu share one renderer.

Correctness gate

docs/e2e-generate.md runbook: generatetofu inittofu plan must be "No changes" (import blocks adopt live state, rendered attrs match). A create/change/replace plan pinpoints the generator bug.

Quality

Built task-by-task TDD (9 commits), per-task + final whole-branch review (on the most capable model). The final review caught two plan-breakers (JSON-blob attrs → HCL objects; secret-key set diverging from the backend → "******" placeholders) — both fixed. HCL escaping verified against the real hashicorp/hcl/v2 parser. Read-only asserted (only GET); no plaintext secret in output. go build/vet/test ./... green; the new hcl parser dep is test-scoped; go.mod stays go 1.24.

Pawel Cygal added 13 commits August 6, 2026 17:34
…n/team

Derive writable HCL attribute lists for check, notification-channel and
team from terraform-provider-systeam's resource schemas (Required/Optional
only, Computed-only fields like id/status/member_count dropped) so a
future `generate terraform` emits configuration the provider actually
accepts. Includes an extraction script to keep the spec in sync when the
provider schema changes.
Attr.Secret only covered 3 of the provider's 7 Sensitive:true check
attributes (auth_password, db_password, auth_bearer_token), missing
ftp_password, http_headers, http_form_login_data and api_scenario_secrets.
An HCL emitter keying off Attr.Secret would have written those four as
plaintext. Make Secret mirror Sensitive:true 1:1 instead of a hardcoded
subset, extend the extraction script to surface Sensitive:true per
attribute, and pin the invariant with a test asserting the check secret
count equals the provider's sensitive count.
…dupe

Review (checked against hashicorp/hcl/v2's real parser) found three ways
generated HCL could fail to parse or silently corrupt values:

- quoteHCLString did not escape "${" / "%{" template interpolation/
  directive markers, so an SDK value containing either sequence was
  reinterpreted as an HCL expression instead of written back literally
  (e.g. `"${1+1}"` evaluates to 2, not the string "${1+1}").
- Only "\n" was escaped; a raw "\r" (CRLF input) produced an invalid
  multi-line string literal.
- labelSet.unique tracked a per-base counter, not the full set of labels
  already handed out, so a base colliding with a literal suffixed name
  handed out earlier (e.g. unique("web_2") then unique("web") again)
  could re-emit a duplicate label.

Fixed by escaping "$"/"%" only when immediately followed by "{" (doubling
unconditionally, as first proposed, was verified against the real parser
to corrupt the very common case of a bare "$"/"%", e.g. "a$b" round-trips
as "a$$b"), adding "\r"/"\t" escapes, and switching labelSet to track
every label ever issued so it always skips taken candidates.

Also, while re-verifying against the parser: guarded the float64->int64
branch against implementation-defined overflow for whole numbers outside
int64's range (now falls back to plain decimal via FormatFloat), and made
hclLabel's empty-sanitization fallback vary by input (fnv hash) instead of
a fixed "r_1" for every unrelated empty/punctuation-only input.

Tests now round-trip generated HCL through hashicorp/hcl/v2's real parser
(new go.mod dependency, go1.24-compatible: hcl v2.24.0 + go-cty v1.16.3)
rather than only comparing rendered strings, plus a cross-base label
collision regression test and a nested-map parseability test.
Wires the Task 1-3 renderer into `syschecks generate terraform`: resolves
--org, lists+gets each in-scope resource kind through the existing v0.2
registry (read-only), renders resource/import/variable blocks with a single
shared label set across all types, and writes provider.tf/imports.tf/
variables.tf plus one .tf file per kind into -o's directory. Narrows scope
via --type/--check/--project and warns on stderr about sensitive variables
that must be set before terraform apply.

Adds a small exported facade (internal/generate/generate.go) onto the
package-private render/label helpers so internal/cli can call them, and a
seenMethods() test helper to assert generate never mutates.
Two review findings against the v0.3 generate build:

1. api_scenario_steps, oidc_config, dns_records_config, and
   response_assertions are declared string (JSON) attrs by the
   provider, but the SDK returns them already decoded as
   map[string]any/[]any. Rendering them via the plain AttrString path
   emitted an HCL object/tuple into a string attribute, which
   `terraform plan` hard-errors on for any check that has one set.
   Drop them from specs["check"].Attrs — Phase 1 already defers
   complex/nested attrs, so omitting is drift-at-worst instead of a
   hard plan error.

2. specs["notification-channel"].SecretMapKeys only covered a guessed
   subset of the keys the backend actually masks to "******"
   (SECRET_CONFIG_KEYS in healthchecks-backend). Widen it to the union
   with the backend's set, and add a mask-sentinel check in
   renderMapAttr so any config value equal to "******" is redacted to
   a var reference regardless of its key — closing the footgun for
   keys the backend masks that the CLI hasn't caught up with yet.
check_source_critical is the same class of bug as the 4 attrs excluded
in the prior fix wave: it's a jsontypes.NormalizedType StringAttribute
that the SDK returns already decoded as a map, so rendering it via
AttrString emitted an HCL object into a string attr and hard-errored
`terraform plan`. It's set on most checks, so this was load-bearing,
not a corner case.

Rather than hand-list a 5th exception, make the rule mechanical:
extract-provider-schema.sh now detects any writable attr with a
CustomType: field (the jsontypes.Normalized* marker) and reports it as
excluded-jsontype-nonsecret unless it's also Sensitive: true — the 3
sensitive jsontypes attrs (http_headers, http_form_login_data,
api_scenario_secrets) stay safe because Attr.Secret redirects
rendering to a var reference instead of ever running the decoded
value through hclValue. Re-running the script against the provider
confirms it now reproduces exactly the corrected spec: all 5
non-secret jsontypes attrs flagged excluded, all 3 secret ones still
writable/sensitive.
The systeampl/systeam provider's check.type validator doesn't accept
"http", but the backend treats "http" as a legacy alias for "uptime"
(identical everywhere; all real checks are stored as "uptime"). A live
check surfaced with the legacy value rendered a `type = "http"`
attribute that failed `terraform plan` validation. Normalize it at
render time via a small, targeted alias map scoped to the check
resource's type attribute only.
@pawel-cygal
pawel-cygal deleted the branch impl/v0.2-crud August 8, 2026 12:10
@pawel-cygal pawel-cygal closed this Aug 8, 2026
@pawel-cygal
pawel-cygal deleted the impl/v0.3-generate branch August 8, 2026 12:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant