Repository navigation
Unify the AWS example with CloudFront and durable Edge Cookie storage - #1540
ChristianPavilonis wants to merge 19 commits into
Conversation
prk-Jr
left a comment
There was a problem hiding this comment.
Requesting changes for one blocking defect; the other comments are non-blocking.
Blocking: deploy/examples/native-aws-workload/example-envelope.json fails the runtime's own integrity check. Run through VerifiedConfigBlob::parse (the path settings_from_config_blob uses at Axum startup), it is rejected with "app-config blob failed integrity verification": the stored sha256 (5d237581…) does not match the canonical hash of data (2c2f8a22…). The cause is tinybird.access_sample_rate: the file has 0, while the CLI emits 0.0 for this f64 field, as example-noindex-envelope.json does, and the canonical form distinguishes integer from float. With the hash recomputed, the content passes full settings validation, so only the checksum is stale. Details inline.
What I checked and found consistent: store-bindings TOML vs the pinned EdgeZero NativeStoreBindings/AppConfigAgentSettings/SecretsManagerSettings schema; logical store IDs vs edgezero.toml; every EDGEZERO__ADAPTER__* key is consumed; the documented ts config push flags exist; forwarded-header trust (ALB/CloudFront to EdgeZero x-forwarded normalization); all 40 AWS resources depend directly on draft_gate (brace-balanced parse); DynamoDB IAM actions/partitions vs the Axum backend's actual calls, and the table layout vs validate_table.
Not verified here: Terraform runs (the hosted offline-checks job passed at this head), and live-AWS behavior (Fargate volume ownership, Agent with a read-only root filesystem, the execution-role aws:SourceArn condition).
Keep the existing fictional workload and retained-DynamoDB examples above the reconstructed native stack. Operational ownership remains in trusted-server-deployer. No state, table, namespace, private configuration or cloud operation changes. Validate the reference TOML with the current typed CLI and run locked backend-free provider/schema and credential-free mock plans. Preserve all forty resource gates, exact fixed browser-host controls and the separate read-only offline workflow.
There was a problem hiding this comment.
Superseded by a follow-up review with inline comments: #1540 (review)
dhruv8sh
left a comment
There was a problem hiding this comment.
Summary
Adds the sanitized native AWS workload Terraform example (with an offline mock-plan harness and workflow) and a separate retained DynamoDB EC example. The offline checks pass at this head, and I reproduced fmt, init -backend=false and test-draft.mjs locally. Remaining comments are non-blocking.
2 of the inline comments below carry a one-click GitHub
suggestion. Use Commit suggestion (or Add suggestion to batch for several at once) to apply them as commits on the PR branch. The remaining comment describes the fix in prose because it touches several parts of the workflow file.
Non-blocking
♻️ refactor
- DynamoDB example has no CI coverage — see inline at
.github/workflows/native-aws-workload.yml:6
⛏ nitpick
movedblock is migration residue in a new example — see inline atdeploy/examples/native-aws-workload/ecs.tf:108
📝 note
- README reads as migration notes — see inline at
deploy/examples/native-ec-dynamodb/README.md:40
Cross-cutting / body-level findings
- 📝 PR description is stale — It lists head
ee7f68fcfwhile the PR is ateb6e19ef4. It calls itself a "Draft baseline review unit" while the PR is marked ready. It also says the DynamoDB example "passes backend-free initialization and Terraform validation", but its README (line 32) says provider initialization needs separate authorization, and no CI job runs that check. Please update the description so reviewers can trust it.
CI Status
- CI Node pin guard: PASS
- CLAUDE.md symlink guard: PASS
- Fastly response diff script: PASS
- HTML EOF runtime (Fastly): PASS
- browser integration tests: PASS
- cargo check (cloudflare native + wasm32-unknown-unknown): PASS
- cargo check/build/test (spin native + wasm32-wasip1): PASS
- cargo fmt: PASS
- cargo test: PASS
- cargo test (axum native): PASS
- cargo test (cross-adapter parity): PASS
- cargo test (ts CLI, native) (macos-latest): PASS
- cargo test (ts CLI, native) (ubuntu-latest): PASS
- docs-proposal scripts: PASS
- format-docs: PASS
- format-typescript: PASS
- integration tests: PASS
- integration tests (Fastly EC lifecycle): PASS
- offline-checks: PASS
- prepare integration artifacts: PASS
- release scripts: PASS
- trace runtime and browser acceptance: PASS
- ts dev lint domains (changed lines): PASS
- vitest: PASS
- worker-deadlines (cloudflare): PASS
- worker-deadlines (spin): PASS
|
Follow-up db1ba33 consolidates the two recipes into one AWS example with nested retained storage, default protected CloudFront and explicit DynamoDB EC wiring. All 12 inline threads have commit-linked responses. The description is updated to the actual ready-for-review state and current local evidence, addressing the body-level stale-description finding too. Hostname/cookie and genuine consent transport are documentation-only operator responsibilities; optional production additions remain deferred. Re-review requested from prk-Jr and dhruv8sh. Hosted current-head checks remain separate from the passing local results; no AWS operation or deployment was performed. |
dhruv8sh
left a comment
There was a problem hiding this comment.
Summary
This merges the AWS example into one guide with two Terraform roots, a retained DynamoDB table and a disposable Fargate workload, and enables protected CloudFront in the main tfvars. The task role's DynamoDB permissions match exactly what the Axum EC store calls. The retained table can't be deleted with the workload. Exact ec.storage agreement between Terraform and the envelope is enforced, and the offline checks pass. There is one non-blocking validation gap.
1 of the inline comments below carries a one-click GitHub
suggestion— use Commit suggestion to apply it as a commit on the PR branch.
Non-blocking
🤔 thinking
- Retained
namespaceis not validated until runtime startup — see inline atdeploy/examples/native-aws-workload/retained/variables.tf:20-23
👍 praise
- Least-privilege task policy —
iam.tfscopesDescribeTableto the exact table, limits identity operations to theec:<ns>partition andUpdateItemtoec-allocator:<ns>, and pins each with aNullLeadingKeyscheck. That is exactly what the Axum EC store calls (ec_kv.rs:describe_table, allocatorupdate_item, and get/put/query/delete on identity rows). - The retained table lives apart from the workload —
deletion_protection_enabledplusprevent_destroy, and the retained root has no dependency on the task role. Workload teardown can't take the table or leave a dangling policy attachment. - Origin protection — when CloudFront is enabled, the ALB is reachable only from the CloudFront origin-facing prefix list and needs CloudFront's secret header. Requests without it get 404. The forwarded host is fixed in CloudFront's configuration, not taken from the visitor.
CI Status
gh pr checks 1540reports no checks onbaseline/aws-examples-20261110(the base is notmain, so the repository workflows did not run). Run locally atdb1ba337a:- Terraform 1.16.2:
fmt -check -recursive;init -backend=false+validate(both roots);terraform test(main 4/4, retained 1/1): PASS node --test test-workflow.mjs(4/4) andtest-draft.mjs(Node 24.20.0): PASScargo test -p trusted-server-core --test aws_example_config(3/3) and the matching clippy-D warnings: PASSts dev lint domains --changed-vs origin/baseline/native-runtime-20261110: PASS
- Terraform 1.16.2:
| variable "namespace" { | ||
| description = "Exclusive app/environment identity namespace. Reserve its ec: and ec-allocator: partitions together." | ||
| type = string | ||
| } |
There was a problem hiding this comment.
🤔 thinking — namespace is not validated before plan, but the runtime rejects it at startup.
ec.storage.namespace must be 1–512 bytes with no control characters (crates/trusted-server-core/src/ec/storage.rs:135). This root accepts any string. With namespace = "", the plan succeeds and outputs owned_partitions.identity = "ec:". That value flows unchanged into the workload's ec_storage, the IAM LeadingKeys conditions and the published envelope. The failure only shows up when the Fargate task fails startup validation, after the workload has been provisioned. It fails closed, so this is non-blocking. Checking here moves the error to plan time, in the root that reserves the partitions.
| variable "namespace" { | |
| description = "Exclusive app/environment identity namespace. Reserve its ec: and ec-allocator: partitions together." | |
| type = string | |
| } | |
| variable "namespace" { | |
| description = "Exclusive app/environment identity namespace. Reserve its ec: and ec-allocator: partitions together." | |
| type = string | |
| validation { | |
| condition = length(var.namespace) > 0 && length(var.namespace) <= 512 && !can(regex("\\p{Cc}", var.namespace)) | |
| error_message = "namespace must be 1-512 characters without control characters, matching the runtime ec.storage.namespace rule." | |
| } | |
| } |
Verified in a scratch copy with Terraform 1.16.2: fmt -check, validate and the existing retained.tftest.hcl all pass. Mock-provider plan runs reject "", \n and U+0085 (\p{Cc} also covers the C1 controls that Rust's char::is_control rejects) and 513 characters, and accept 512. Terraform's length counts characters, not bytes, so a non-ASCII namespace between 171 and 512 characters can still exceed the runtime's byte limit. That's a rare edge, and the runtime still catches it.
Summary
Provide one operator-configurable native AWS deployment example with protected CloudFront enabled in the main tfvars and retained DynamoDB Edge Cookie storage wired into configuration and task IAM.
deploy/examples/native-aws-workload/retained/. One guide covers both roots, with separate operator-supplied locked state keys.dns-cutoverpath. Preserve origin admission, checked Geo/forwarding, disabled shared caching and explicit Geo Off support.[ec.storage]through the existing CLI envelope and require exact agreement with the retained handoff. Regenerate both envelopes byte-exact and add real runtime verification tests.This PR retains base
baseline/native-runtime-20261110and the existing dependency selection. Its follow-up is integration into RC #1541, not a merge tomainor deployment authorization. Operational deployment remains separately owned.Review fixes
Commit
db1ba337aaddresses the reported envelope checksum failure, nested Terraform CI coverage, policy ownership/name collision, AWS-owned default encryption, ignored private Terraform artifacts, inaccessible deployer dependency, stale migration narrative and unusedmovedblock. The PR description now reflects this ready-for-review example unit instead of stale draft/head claims.The runtime regression reproduced the old default envelope's integrity failure before regeneration. Both new exact CLI fixtures now pass
VerifiedConfigBlob::parseand settings validation with fictional secrets. Tests also reject data tampering and the original float-to-integer rewrite.Checks at db1ba33
Local checks passed:
The hosted credential-free AWS offline workflow passes at exact PR commit
db1ba337a: run 43306496934. It was explicitly dispatched after automatic PR runs did not appear. This is not a claim that the broader hosted suite completed.RC integration
Merged and pushed into #1541 through
86797e0b4, preserving the RC's existing work. All changed example, regression and workflow files exactly match the PR commit. Combined local checks pass: 3,437 core unit cases, 2 explicitly selected Node regressions, 18 envelope/storage/digest cases, 110 AWS-feature Axum cases, targeted strict Clippy, docs build, workflow tests, formatting and changed-line domain lint. The unchanged Terraform input fingerprints permit reuse of the 8+83+1 passing mock-plan results. Full hosted checks at the new RC head are running, not yet claimed passed.Limits and deferred work
The 40 workload AWS resource blocks remain behind the source-controlled qualification gate. Public account/resource/image values are fictional. No cloud identity checks, live plan/apply, secret writes, image publication, DNS change or live browser operation were performed.
DynamoDB Local and the external real Next.js fixture were not available/selected for the two ignored native cases. Actual AWS IAM/boundaries, table operations, capacity, Fargate image/volume/Agent behavior, TLS/DNS, genuine browser consent/cookie acceptance, persistence through ECS replacement and cleanup remain operator qualification work.
PITR/backups, alarms, HA, autoscaling, private networking/endpoints, WAF, customer-managed keys, IPv6, image automation, publisher/bidder/partner integrations, new signing/admin/S2S work and automated config rollback are explicitly deferred, not silently enabled.
Refs #1530. This unit does not close the overall RC tracker.