Skip to content

fix(cfn): retire the CloudFormation install stack - #1577

Merged
IAmJSD merged 1 commit into
LeanerCloud:mainfrom
IAmJSD:fix/issue-1432-retire-cfn-stack
Oct 28, 2026
Merged

IAmJSD merged 1 commit into
LeanerCloud:mainfrom
IAmJSD:fix/issue-1432-retire-cfn-stack

Conversation

@IAmJSD

@IAmJSD IAmJSD commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator

Root cause

cloudformation/stacks/CUDly/template.yaml was written for the old DynamoDB backend. It still created six AWS::DynamoDB::Table resources and passed CONFIG_TABLE … SESSIONS_TABLE to the Lambda. It set no DB_HOST and created no Postgres database, credentials secret or VPC access. The server is Postgres-only: initConfigStore returns database configuration required: DB_HOST must be set. A stack deployed from this template therefore failed every request and every scheduled task.

Fix (owner decision D29, option 2: retire the stack)

  • Delete cloudformation/stacks/CUDly/ (the whole template).
  • Docs: docs/DEPLOYMENT.md now says there is no CloudFormation install, explains why, and points to terraform/environments/aws (Lambda or Fargate). The CloudFormation bullets in the cleanup and finalize-revocations schedule lists and the CloudFormation row in the cross-account table are gone. README.md, CONTRIBUTING.md and CLAUDE.md describe terraform/ as the only install path and cloudformation/, arm/ and iac/ as target-account role templates. CHANGELOG.md has a new Removed entry under Unreleased. The live status lines in known_issues/33_executions_retention_cleanup.md no longer list CloudFormation, and its CloudFormation bullet is struck through with a pointer to this issue, following the file's existing style.
  • IAM parity scripts:
    • scripts/check-aws-iam-parity.sh: the runtime comparison is now TF Lambda vs TF Fargate. The CFN stack is removed from the required-actions and runtime-organizations loops.
    • scripts/test-aws-iam-parity.sh: the stack is out of the fixture seed and out of cases 2, 11 and 16. The YAML bare-* cases (17, 18, 19, 24, 26, 28) inject into iac/federation/aws-cross-account/cloudformation/template.yaml, which is read only and never edited. That file is in a parity group and has the same - Sid: CostExplorer anchor at the same indentation, so the YAML parsing paths stay covered.
  • Tests that read the stack:
    • terraform/modules/compute/scheduled_task_cfn_test.go: deleted. It only tested this template.
    • scheduled_task_coverage_test.go: the cloudformation platform is gone, along with the CFN-only cfnRule scope, the "action": Input-JSON pattern and the two CFN negative cases. No .tf file uses that JSON form; I checked with grep.
    • function_url_cors_test.go: the CFN test is removed.
    • runtime_iam_coverage_test.go and cross_account_sts_guard_test.go: the stack is removed from the file and site lists.
    • internal/testutil/lambdatimeout: the CFN Timeout check is removed.
  • Comments: the cfn-lint hook comment in .pre-commit-config.yaml and the aws-iam-parity job comment in ci.yml. The hook's files: pattern and the job name are unchanged. The pattern still covers CUDly-CrossAccount and the federation templates.
  • New regression guards in terraform/modules/compute/aws/retired_cfn_stack_test.go:
    • TestNoIaCProvisionsDynamoDB walks terraform/, cloudformation/, iac/, arm/ and internal/iacfiles/templates/. It fails on any AWS::DynamoDB::Table or aws_dynamodb_table, and on a root where it read no files.
    • TestRetiredCloudFormationStackIsNotReferenced fails if the directory comes back or if any current file (all files outside frontend/, docs/audits/, known_issues/ and CHANGELOG.md) names cloudformation/stacks/CUDly. CUDly-CrossAccount paths do not match.

Closes #1432

Verification

Unit, script and fixture evidence only. Nothing was deployed and no terraform plan or apply was run.

Fail-before: the new guard file, copied into a clean origin/main (3ec21a0) worktree:

$ go test -count=1 -run 'TestNoIaCProvisionsDynamoDB|TestRetiredCloudFormationStackIsNotReferenced' ./terraform/modules/compute/aws/
--- FAIL: TestNoIaCProvisionsDynamoDB
    cloudformation/stacks/CUDly/template.yaml:342 provisions a DynamoDB table; ...
--- FAIL: TestRetiredCloudFormationStackIsNotReferenced
    the CloudFormation install stack directory exists again (stat err: <nil>); ...
    .pre-commit-config.yaml:167 still points at the retired CloudFormation install stack (#1432) ...
    docs/DEPLOYMENT.md:606 ...
    internal/testutil/lambdatimeout/lambdatimeout.go:46 ...
    scripts/check-aws-iam-parity.sh:11 ...
    scripts/test-aws-iam-parity.sh:44 ...
    terraform/modules/compute/scheduled_task_cfn_test.go:24 ...
    terraform/modules/compute/scheduled_task_coverage_test.go:85 ...
FAIL

Pass-after on this branch: both tests PASS.

Commands run on this branch (with AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null AWS_EC2_METADATA_DISABLED=true):

  • go build ./..., go vet ./..., go vet -tags integration ./...: clean.
  • go test -count=1 ./terraform/modules/compute/...: ok (compute, aws, azure, gcp).
  • go test -count=1 -run TestLambdaTimeoutCovers ./internal/api/ ./internal/purchase/: all three lambdatimeout consumers pass.
  • go test -count=1 ./internal/mocks/ ./internal/iacfiles/ ./iac/: ok.
  • golangci-lint run --allow-parallel-runners ./terraform/modules/compute/... ./internal/testutil/lambdatimeout/ (v2.10.1): 0 issues.
  • gocyclo -over 10 -ignore "_test\.go" internal/testutil/lambdatimeout/lambdatimeout.go: clean.
  • bash scripts/check-aws-iam-parity.sh: OK. bash scripts/test-aws-iam-parity.sh: 28 passed, 0 failed.
  • Mutation probe on the retargeted YAML cases: with bare_star_action disabled (END { }), cases 14, 17, 18, 19, 20, 21, 22, 23 and 28 fail as expected (19 passed, 9 failed), so the YAML cases still exercise the extractor.
  • shellcheck on both parity scripts and actionlint .github/workflows/ci.yml: clean.
  • pre-commit run for cfn-lint (on CUDly-CrossAccount and both federation templates; only the known W3005 warning), markdownlint, check-yaml, trailing-whitespace, end-of-file-fixer, go-mod-tidy and check-merge-conflict on the changed files: all pass.

Combined-tree verification. Tested on a local, never-pushed tree: queue/platform @ 40dbbdaf = origin/main 6e2033cd plus:

Branch Tested head
fix/issue-1494-listing-settlements d7aa121
fix/issue-1529-ambiguous-return 8ecf506
fix/issue-1434-lambda-budget 8b6eebe
fix/issue-1432-retire-cfn-stack 9cb7de8
fix/issue-712-fire-default-aws-schedule 8bd7095

Linux, Go 1.26.9, AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null AWS_EC2_METADATA_DISABLED=true; Docker Postgres via testcontainers.

Out of scope / left as is

  • Federation CloudFormation templates (iac/federation/*/cloudformation/, internal/iacfiles/templates/*cfn*, handler_federation) are unchanged.
  • cloudformation/stacks/CUDly-CrossAccount/template.yaml stays. It is a target-account role, not an install. Its PrimaryRoleName default (CUDly-LambdaRole) and description still assume a hub deployed from the retired stack under the name "CUDly". Terraform users already have to override it. Changing the default is a behavior change for existing users of that template, so it is left for a follow-up.
  • cross_account_sts_guard_test.go still accepts CrossAccountTargetAccountIds in accountSourceRefs and still walks cloudformation/stacks/. That keeps the guard working for YAML if a grant ever shows up there again.
  • The historical audit docs (docs/audits/), the older known-issues entries and the existing CHANGELOG Notices still mention the stack as it was, on purpose.
  • Rebuilding the stack on Postgres (option 1) was not done, per the owner decision.

Maintainer: please mirror the issue's triage labels onto this PR: priority/p2, severity/high, urgency/this-quarter, impact/few, effort/l, type/bug, triaged. The issue also carries status/needs-info. The owner decision now answers it, so that label can come off the issue.

🤖 Generated with Claude Code

The CloudFormation install stack (cloudformation/stacks/CUDly) still created
six DynamoDB tables and passed their *_TABLE names to the Lambda, but set no
DB_HOST and created no Postgres database. The server is Postgres-only, so
initConfigStore failed at startup and the stack could not serve any request
or scheduled task.

Per the acting owner's decision, delete the stack instead of rebuilding it on
Postgres, and remove every reference to it: the deployment guide, README,
CONTRIBUTING, CLAUDE.md, the cfn-lint hook comment, the CI parity job
comment, the AWS IAM parity script and its self-tests (the YAML bare "*"
cases now inject into the federation cross-account CloudFormation template),
the Lambda timeout helper, and the Terraform module tests that read it.
Readers are pointed at the Terraform AWS install.

The target-account role templates (cloudformation/stacks/CUDly-CrossAccount
and iac/federation/**) are unchanged.

New guards: no shipped IaC may provision a DynamoDB table, and no current
file may point at the retired stack directory.

Closes LeanerCloud#1432

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@IAmJSD IAmJSD added priority/p2 Backlog-worthy effort/l Weeks triaged Item has been triaged severity/high Significant harm urgency/this-quarter Within the quarter impact/few Limited audience type/bug Defect labels Oct 28, 2026
@coderabbitai

coderabbitai Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Caution

Review failed

The pull request is closed.

Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@IAmJSD

IAmJSD commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator Author

Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): VERDICT: APPROVE 9cb7de8ae085438e2654f47efdc93db91aa92715

Blocking:

  • none

Non-blocking:

  • cloudformation/stacks/CUDly-CrossAccount/template.yaml:21-29: the PrimaryRoleName default (CUDly-LambdaRole) and its description ("Default matches a primary CUDly stack deployed with the stack name CUDly") now describe a hub that can no longer be deployed from this repo. The PR body discloses this and defers it; changing the default is a behavior change, so a follow-up issue is the right place. At minimum the description text could be reworded later to point at the Terraform role name.
  • CHANGELOG.md:49: the new Removed bullet ends without a period ((#1432)), unlike neighbouring entries. Cosmetic.
  • TestRetiredCloudFormationStackIsNotReferenced skips frontend/. I grepped the whole tree (frontend included, docs/audits excluded) for stacks/CUDly not followed by -: the only hits are the CHANGELOG entry and the historical audit doc, so nothing is missed today.

Attacked:

  • Hunted for leftover references the guard can't see: grepped the whole tree (incl. frontend, workflows, Makefiles, dotfiles like .trivyignore, .snyk, .coderabbit.yaml, .gitallowed) for cloudformation/stacks, CUDly/template, "CFN/hub stack" wording and CloudFormation-as-install wording in docs/. The only path-level survivor is the cfn-lint files: glob, which still matches CUDly-CrossAccount and the federation templates. internal/iacfiles does not embed the stack.
  • Parity: bash scripts/check-aws-iam-parity.sh OK. bash scripts/test-aws-iam-parity.sh 28/28. Checked that the retargeted YAML bare-* cases inject into a file that is in a parity group (federation cross-account CFN vs TF), so they still exercise the YAML extractor. The awk anchor fails loudly if it is missing.
  • Fail-before: in the scratch checkout I restored cloudformation/stacks/CUDly/ and docs/DEPLOYMENT.md from origin/main. TestNoIaCProvisionsDynamoDB failed (template.yaml:342) and so did TestRetiredCloudFormationStackIsNotReferenced (dir exists, DEPLOYMENT.md:606). Restored afterwards; tree clean.
  • Ran on 9cb7de8: go build ./..., go vet ./..., go vet -tags integration ./... (clean); go test -race -short -count=1 ./terraform/modules/compute/... ./internal/testutil/... ok; -run LambdaTimeout ./internal/api ./internal/purchase ok (lambdatimeout consumers after the CFN check was dropped); golangci-lint v2.10.1 0 issues; gocyclo clean; shellcheck clean on both parity scripts. No integration-tagged code changed.
  • Issue fix(cfn): CloudFormation CUDly stack can't start the current server (no DB_HOST; still provisions DynamoDB) #1432 option 2 AC (template and every reference removed or redirected, docs no longer offer a CFN install): met. Closes is appropriate.

Body-stale:

  • none

Reviewer: rM1. Bar while the maintainers are away (set by IAmJSD as acting owner): one approving independent review on the head, two for money, auth and security paths.

@IAmJSD

IAmJSD commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator Author

Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): VERDICT: APPROVE 9cb7de8ae085438e2654f47efdc93db91aa92715

Blocking:

  • none

Non-blocking:

  • cloudformation/stacks/CUDly-CrossAccount/template.yaml: PrimaryRoleName still defaults to CUDly-LambdaRole, the role of the hub this PR deletes. The PR body already says this, but nothing tracks it. A follow-up issue should change the default, or at least the description, so the template no longer points at a hub that can't exist.
  • retired_cfn_stack_test.go: the reference guard walks the whole repo except frontend/, and that includes extension-less files up to 4 MB, such as locally built binaries. That is fine in CI. Locally a stale build artifact could be read, but it won't contain the path, so this only costs time.

Attacked:

  • Searched for any remaining reference to the retired path outside the historical files: git grep over this repo (frontend, workflows, scripts, the pre-commit files: pattern) and over the cli/go/mcp repos at origin/main. Nothing outside docs/audits, known_issues and the CHANGELOG. The cfn-lint pattern still covers CUDly-CrossAccount and the federation templates. arm/ and iac/ hold only target-account role templates, so the new README and CLAUDE.md wording is accurate.
  • Fail-before: I restored cloudformation/stacks/CUDly and docs/DEPLOYMENT.md from origin/main in the scratch tree. TestNoIaCProvisionsDynamoDB failed (template.yaml:342) and so did TestRetiredCloudFormationStackIsNotReferenced (DEPLOYMENT.md:606). Both pass at the head.
  • Ran: go build ./..., go vet ./..., go vet -tags integration ./...; go test -race -short -count=1 ./terraform/modules/compute/... ./internal/testutil/... ./iac/ ./internal/iacfiles/ (ok) and -run TestLambdaTimeoutCovers ./internal/api ./internal/purchase (ok); bash scripts/check-aws-iam-parity.sh (OK) and scripts/test-aws-iam-parity.sh (28 passed, 0 failed); golangci-lint v2.10.1 on ./terraform/modules/compute/... ./internal/testutil/lambdatimeout/ (0 issues).
  • Combined tree: this head merged with fix(iac): fire delayed purchases by default; collect AWS recommendations at 02:00 UTC #1580, fix(purchase): shorten execution budget to 3m30s so the failure alert fits the Lambda timeout #1578 and fix(revoke): keep the in-flight flag on an ambiguous Azure Return failure and refuse a late Lambda claim #1579. No conflicts. Build and vet are clean, and go test ./terraform/modules/compute/... plus every TestLambdaTimeoutCovers* pass.

Body-stale:

  • none

Reviewer: rM2. Bar while the maintainers are away (set by IAmJSD as acting owner): one approving independent review on the head, two for money, auth and security paths.

@IAmJSD

IAmJSD commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator Author

Local verification at 9cb7de8ae085438e2654f47efdc93db91aa92715 merged onto main (6e2033cd), together with #1577, #1578, #1579 in that order (combined tree bf700d64, Linux, Go 1.26.10): go build ./..., go vet with and without -tags integration, the full go test -race -short -count=1 ./..., and go test -race -tags integration -count=1 on every changed package, against Docker Postgres. All pass. CI Success and pre-commit are green on this head, with 2 approving independent review(s) on it. Nothing was run against any cloud.

@IAmJSD
IAmJSD marked this pull request as ready for review October 28, 2026 22:26
@IAmJSD
IAmJSD requested a review from cristim as a code owner October 28, 2026 22:26
@IAmJSD
IAmJSD merged commit 2aabee0 into LeanerCloud:main Oct 28, 2026
41 checks passed
@IAmJSD

IAmJSD commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator Author

Merged by IAmJSD's agent at IAmJSD's request (IAmJSD is acting owner while the maintainers are away), under the bar IAmJSD set: 2 approving independent review(s) on 9cb7de8a (two required: money, auth or security path), green CI, and the combined-tree verification above. Squash-merged at exactly 9cb7de8ae085438e2654f47efdc93db91aa92715.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/l Weeks impact/few Limited audience priority/p2 Backlog-worthy severity/high Significant harm triaged Item has been triaged type/bug Defect urgency/this-quarter Within the quarter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(cfn): CloudFormation CUDly stack can't start the current server (no DB_HOST; still provisions DynamoDB)

1 participant