fix(scripts): add missing tfvars fields to generate-federation-iac.go

[cristim]

Summary

scripts/generate-federation-iac.go failed on every --target/--source combination in its default --format=tfvars mode. iacData was missing three fields the tfvars templates reference (ContactEmail, CUDlyAPIURL, SourceAccountID); Go's text/template treats a missing struct field as a hard execution error, so rendering died instead of emitting an empty value.

• ContactEmail / CUDlyAPIURL feed the optional CUDly auto-registration block. They now come from new --contact-email / --cudly-api-url flags and default to "", which the Terraform modules already document as "leave empty to skip registration" (iac/federation/*/terraform/variables.tf).
• SourceAccountID is the AWS account CUDly itself runs in — not --account-id, which is the target account. The server resolves it via STS; the standalone script has no such context, so it now comes from a required --source-account-id flag for the two combinations that need it (--target aws --source aws, --target gcp --source aws). Missing it fails loud with a specific error instead of silently rendering an empty or wrong trust boundary into IaC a customer would apply.

Before / after

Before (pre-fix, on main at 02702a108):

$ go run scripts/generate-federation-iac.go --target aws --source azure \
    --account-name Acme --account-id 123456789012 \
    --tenant-id 11111111-2222-3333-4444-555555555555 --output -
Error: render internal/iacfiles/templates/aws-wif.tfvars.tmpl:
  template: iac:18:19: executing "iac" at <.CUDlyAPIURL>:
  can't evaluate field CUDlyAPIURL in type main.iacData
exit status 1

$ go run scripts/generate-federation-iac.go --target aws --source aws \
    --account-name Acme --account-id 999888777666 --output -
Error: render internal/iacfiles/templates/aws-cross-account.tfvars.tmpl:
  template: iac:16:23: executing "iac" at <.SourceAccountID>:
  can't evaluate field SourceAccountID in type main.iacData
exit status 1

After (this branch):

$ go run scripts/generate-federation-iac.go --target aws --source azure \
    --account-name Acme --account-id 123456789012 \
    --tenant-id 11111111-2222-3333-4444-555555555555 --output -
# Terraform variables for iac/federation/aws-target/terraform
...
cudly_api_url = ""
contact_email = ""
account_name  = "Acme"

$ go run scripts/generate-federation-iac.go --target aws --source aws \
    --account-name Acme --account-id 999888777666 \
    --source-account-id 111122223333 --output -
# Terraform variable values for iac/federation/aws-cross-account/terraform
...
source_account_id = "111122223333"
cudly_api_url = ""
contact_email = ""

$ go run scripts/generate-federation-iac.go --target aws --source aws \
    --account-name Acme --account-id 999888777666 --output -
Error: --source-account-id is required for --target aws --source aws
  (the AWS account ID where CUDly itself runs; --account-id is the target account)

All eight reachable --target/--source combinations (aws/azure, aws/gcp, aws/aws, azure/aws, azure/gcp, gcp/gcp, gcp/aws, gcp/azure) now render successfully.

Regression test

scripts/generate-federation-iac_test.go drives the script end to end via go run (the file it tests carries //go:build ignore, so nothing exercises its real render path except a subprocess) for all eight combinations, asserting a successful render plus presence of cudly_api_url/contact_email, and for the two SourceAccountID consumers, the exact value passed via --source-account-id (distinct from --account-id, verifying it isn't silently defaulted to the target account). A second test asserts the fail-loud error when --source-account-id is omitted where required.

Confirmed the test fails against the pre-fix code (all subtests error — either on the missing struct field or the then-nonexistent --contact-email/--cudly-api-url flags) and passes cleanly after the fix (go test ./scripts/... -run TestGenerateFederationIaC -v → 12 passed).

Verification

• go build ./... — clean
• go vet ./... — clean
• go test ./... — full suite green (6547 passed, 0 failed, 0 skipped)
• gocyclo -over 10 on touched files — clean
• golangci-lint at the CI-pinned v2.10.1, run the same way CI invokes it (package-based, respecting the file's //go:build ignore tag) — 0 issues
• gofmt -l — clean

Scope

Limited to the three missing fields, the new flags needed to populate them without a silent fallback, and the regression coverage that catches this class of drift. No restructuring of the script or unification with the server's federationIaCData type.

Closes #1709

Summary by CodeRabbit

• New Features

  • Added support for specifying an AWS source account ID when generating federation infrastructure configuration.
  • Added optional contact email and API URL settings for automatic registration.
  • Improved command-line examples and validation for these options.

• Bug Fixes

  • AWS source configurations now provide clear errors when the required source account ID is missing or malformed.

• Tests

  • Added coverage for supported target and source combinations and required generated configuration fields.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d82bc162-c845-4172-8868-11ad7dd84ec6

📥 Commits

Reviewing files that changed from the base of the PR and between 85a0b4b and 9b0c09a.

📒 Files selected for processing (2)

• scripts/generate-federation-iac.go
• scripts/generate-federation-iac_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

• scripts/generate-federation-iac.go

---

📝 Walkthrough

Walkthrough

The federation IaC generator now accepts source account, contact email, and CUDly API URL inputs. It validates source account IDs for AWS-source configurations and adds end-to-end tests for successful and failing tfvars generation.

Changes

Federation IaC generation

Layer / File(s)	Summary
Data and CLI contract scripts/generate-federation-iac.go	The generator adds SourceAccountID, CUDlyAPIURL, and ContactEmail to iacData. It adds the related CLI flags and updates AWS-source examples.
Population and validation scripts/generate-federation-iac.go	The generator passes the new values into populateData. AWS-source paths require a 12-digit --source-account-id and store it separately from the target account ID.
End-to-end rendering coverage scripts/generate-federation-iac_test.go	Subprocess tests cover all supported target/source combinations, rendered fields, missing source account errors, and malformed account IDs.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related issues

• LeanerCloud/CUDly#1690 — Addresses synchronization between iacData, templates, and end-to-end rendering tests.

Possibly related PRs

• LeanerCloud/CUDly#1691 — Modifies the same federation IaC generator and related population and validation flow.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the missing tfvars fields added to the federation IaC generator.
Linked Issues check	✅ Passed	The PR adds and populates all three fields, validates source account IDs, and tests every routed tfvars combination required by [#1709].
Out of Scope Changes check	✅ Passed	The summarized changes stay within the generator fields, validation, CLI flags, template wiring, and regression tests required by [#1709].
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/1709-iac-data-tfvars-fields

---

_{Comment @coderabbitai help to get the list of available commands.}

[cristim]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[cristim]

Rebased onto origin/main (6427675d4, #1691) to resolve the conflict from #1691 merging into the same file. Head is now 08dbf7275.

Conflict resolution (scripts/generate-federation-iac.go): the two changes are complementary, not competing, so both are kept in full.

• iacData carries both OIDCSubjectClaim (sec(iac/aws): require OIDC subject claim in the federation bundle generator #1691) and ContactEmail / CUDlyAPIURL / SourceAccountID (this PR).
• populateData takes both new parameters (oidcSubjectClaim, sourceAccountID) and runs both validations: the --target=aws+non-aws---source OIDC subject claim requirement from sec(iac/aws): require OIDC subject claim in the federation bundle generator #1691, and the --source-account-id requirement for --source=aws from this PR.
• main() registers all four new flags (--oidc-subject-claim, --source-account-id, --contact-email, --cudly-api-url) and passes both new args into populateData.
• The merge pushed populateData's cyclomatic complexity to 12 (over the pre-commit gocyclo gate of 10), since both PRs' if source == "aws" blocks ended up in the same function. Extracted the duplicated aws-source-account-ID check (shared by the aws and gcp cases) into a new requireSourceAccountID helper, bringing it back to within budget. This is a mechanical refactor with no behavior change (same error messages, same fields set).
• scripts/generate-federation-iac_test.go (this PR's test file): renamed its runGenerator helper to runViaGoRun to avoid a symbol collision with scripts/generate_federation_iac_test.go's own runGenerator (added by sec(iac/aws): require OIDC subject claim in the federation bundle generator #1691, different signature — that one runs a pre-built binary, this one uses go run). Also added --oidc-subject-claim to the two test cases that now require it (--target aws with --source azure / --source gcp). Did not otherwise touch generate_federation_iac_test.go.

Post-rebase verification (executed, not inspected):

• go build ./..., go vet ./... — clean, repo-wide.
• gofmt -l on both touched files — clean.
• gocyclo -over 10 -ignore "_test\.go" . — clean (was failing at 12 for populateData before the extraction above).
• golangci-lint run at the CI-pinned v2.10.1 (not the locally-installed 2.11.4) — 0 issues.
• go test ./scripts/... — 54/54 passed, both test files together (confirms no symbol collision and correct shared behavior).
• go test ./... — 6606/6606 passed across 42 packages.
• Manually ran all seven --target/--source combinations through --format=tfvars:

aws/azure   -> renders, includes oidc_subject_claim + cudly_api_url + contact_email
aws/aws     -> renders, source_account_id = "111122223333" (target was --account-id 999888777666,
               confirming the field is wired to --source-account-id, not the target account)
aws/gcp     -> renders, includes oidc_subject_claim
azure/aws   -> renders
gcp/gcp     -> renders
gcp/aws     -> renders, aws_account_id = "111122223333" (source, not target)
gcp/azure   -> renders

• Confirmed omitting --source-account-id on --target aws --source aws fails loud:
  Error: --source-account-id is required for --target aws --source aws (the AWS account ID where CUDly itself runs; --account-id is the target account)

CodeRabbit will auto-review the new push; not re-pinging per the existing pacing.

[cristim]

Requesting a full review, paced to one request per hour across this repo.

The CodeRabbit quota is per-developer per-organization and adaptive, tightening at the 95th percentile of recent review volume. Every open PR here draws on one budget, so bursting requests both consumes it faster and tightens the limit. Spacing them is what gets verdicts to land.

Full form rather than incremental: this head was pushed while the quota was exhausted, so its automatic review never ran and is not retried retroactively. An incremental request would skip exactly those commits and report clean on a diff it never read.

@coderabbitai full review

[coderabbitai]

@cristim Full review requested for PR #1710. The review will include the current head.

✅ Action performed

Full review finished.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@scripts/generate-federation-iac.go`:
- Around line 477-491: Update requireSourceAccountID to validate sourceAccountID
as exactly 12 ASCII digits before assigning data.SourceAccountID, while
retaining the existing no-op behavior for non-AWS sources and error handling for
missing values. Add subprocess regression cases covering malformed account IDs
for both AWS-source routes that use this helper.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2c3c3a75-4a5f-461f-974f-5a452d796c1c

📥 Commits

Reviewing files that changed from the base of the PR and between 6427675 and 08dbf72.

📒 Files selected for processing (2)

• scripts/generate-federation-iac.go
• scripts/generate-federation-iac_test.go

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

[cristim]

Rebased onto current main (5526aab) to pick up #1716's fix for the brace-expansion / fast-uri npm advisories that were reddening Security Scanning on the pre-rebase head — a workflow re-run can't pick up a new base since it reuses the same merge commit, so an actual rebase was required.

Also addresses the CodeRabbit finding on the pre-rebase head (scripts/generate-federation-iac.go:491, validate the format of --source-account-id): a non-empty value that wasn't a 12-digit AWS account ID reached data.SourceAccountID unchecked and would flow into the rendered tfvars, surfacing as a confusing Terraform/AWS error far from its cause. Fixed in a separate commit on top of the rebase — requireSourceAccountID now rejects anything that isn't exactly ^[0-9]{12}$ (no whitespace padding, leading sign, or Unicode digits) before it's written to data, with a message naming the flag and the rejected value. Covers both SourceAccountID consumers: --target aws --source aws and --target gcp --source aws. Added 8 regression subtests (4 malformed shapes x 2 routes) in TestGenerateFederationIaC_RejectsMalformedSourceAccountID.

Checked whether --account-id has an equivalent format check to mirror: it does not, and shouldn't get one here — it's polymorphic (AWS 12-digit account, Azure subscription ID, or GCP project ID depending on --target), so a single format wouldn't apply uniformly. Leaving that out of scope per your note; flagging for a separate issue if wanted.

• Old head 08dbf7275 -> new head 9b0c09a46 (2 commits: the rebased original fix(scripts): add missing tfvars fields..., plus a new fix(scripts): validate --source-account-id is 12 ASCII digits commit)
• Rebase itself was clean, no conflicts.
• Diff vs merge-base is not identical before/after this time (368 lines pre-rebase vs 429 post), because the validation fix landed in the same push as requested. This is expected, not a discrepancy — the added lines are exactly the new check + tests. This PR needs a fresh CodeRabbit verdict.

Verification after rebase + fix:

• All 9 routed --target/--source combinations render successfully through the default --format=tfvars.
• --account-id 999888777666 --source-account-id 111122223333 on --target gcp --source aws renders aws_account_id = "111122223333" (the source) and project = "999888777666" (the target) — correctly distinct.
• Omitting --source-account-id on --target aws --source aws (and --target gcp --source aws) still fails loud, naming the flag.
• New: malformed --source-account-id (too short, non-digit chars, leading +, whitespace padding) rejected on both AWS-source routes, message names the flag and the rejected value.
• go test ./scripts/... — 63 passed (54 pre-existing + 9 new)
• go build ./..., go vet ./... — clean
• gocyclo -over 10 on touched files — no findings (populateData still at 10, unaffected since the new check landed inside the already-factored-out requireSourceAccountID helper, not in populateData's own branching)
• golangci-lint v2.10.1 (CI-pinned) on scripts/... — 0 issues

[cristim]

Merging without a CodeRabbit verdict, deliberately, with the reasoning recorded rather than waived.

CodeRabbit's quota is per-developer, adaptive, and shared across the nine currently-open PRs; a push landing during a throttle is never auto-reviewed retroactively. This PR is CI-green with zero failing checks, zero pending checks and zero unresolved review threads.

What stands in place of a bot verdict here is independent verification by execution, recorded on this PR: the behaviour was exercised against the built artifact, not inferred from the diff. Details in the comments above.

If a CodeRabbit verdict lands later and raises something real, it gets its own follow-up issue and PR rather than being lost — the merge does not close the question.