Add an AWS deployment planning skill for Prebid Server Go - #1166
ChristianPavilonis wants to merge 11 commits into
Conversation
Gather deployment requirements before choosing AWS services and generating Terraform or runtime files. Keep cloud execution behind separate approval and document safe testing, state ownership, and credential handling. Refs #1163
aram356
left a comment
There was a problem hiding this comment.
This is not a complete review, but I wanted to make sure we keep the CLI clean and consistent. We already have a prebid subcommand, so I would recommend building on top of that.
For the new command:
ts prebid server ...
And the existing command would become:
ts prebid client ...
prk-Jr
left a comment
There was a problem hiding this comment.
Summary
Adds an agent planning skill for self-hosted Prebid Server Go on AWS, a new experimental ts prebid server CLI namespace (local inspection and validation, guarded Secrets Manager writes, EC2 status), and renames ts prebid bundle to ts prebid client. The CLI subsystem is carefully built — sanitized &'static str error payloads, an injected Interaction trait so tests can't touch a terminal, secrets kept out of argv, owner-only temp payloads cleaned up on both success and failure — and the negative-path test suite is genuinely strong. All 20 CI checks pass.
One blocking item: .tool-versions pins every tool except the AWS CLI this PR adds, which is exactly the binary the credential-write path shells out to.
3 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. Each was verified in an isolated worktree, individually and as a batch, againstcargo fmt --all -- --check, both host-targettrusted-server-cliclippy invocations with-D warnings, andcargo test --package trusted-server-cli. The remaining comments describe the fix in prose because the change touches multiple files, lands outside the diff hunks, or didn't survivecargo fmtin the reviewed form.
Blocking
🔧 wrench
- AWS CLI pinned to
latestwhile every other tool is exact — see inline at.tool-versions:6
Non-blocking
♻️ refactor
rpassword/serde_yaml_ngbypass workspace dependency inheritance — see inline atcrates/trusted-server-cli/Cargo.toml:27- The determinism check compares a pure function against itself — see inline at
crates/trusted-server-cli/src/commands/pbs/config.rs:378 - Hand-rolled JSON escaping round-trip in
bidder_list— see inline atcrates/trusted-server-cli/src/commands/pbs/inspect.rs:91(suggestion)
🤔 thinking
ts prebid bundle→ts prebid clientis a breaking rename with no alias — see inline atcrates/trusted-server-cli/src/run.rs:75(suggestion)- Transport failure on
put-secret-valuedoesn't flag the outcome as uncertain — see inline atcrates/trusted-server-cli/src/commands/pbs/secrets.rs:173 - A dated
Status: Implementedspec is rewritten retroactively — see inline atdocs/superpowers/specs/2026-06-17-prebid-bundle-cli-design.md:5 - The
ts prebid servernamespace is undocumented indocs/— see inline atdocs/guide/cli.md:274 - Missing
python3makes two tests pass for the wrong reason — see inline atcrates/trusted-server-cli/tests/pbs_cli.rs:23 identifier()rejects AWS profile names containing.— see inline atcrates/trusted-server-cli/src/commands/pbs/config.rs:104
⛏ nitpick
- Unparenthesized
&&/||on the noninteractive-write gate — see inline atcrates/trusted-server-cli/src/commands/pbs/secrets.rs:112(suggestion)
🌱 seedling
- Unbounded recursion over operator YAML — see inline at
crates/trusted-server-cli/src/commands/pbs/config.rs:318
📝 note
- Identity failure aborts the whole status report; resource-query failure degrades — see inline at
crates/trusted-server-cli/src/commands/pbs/status.rs:26
👍 praise
- Negative-path test discipline — see inline at
crates/trusted-server-cli/tests/pbs_cli.rs:39
Cross-cutting / body-level findings
- 📌 Three separable concerns in one PR — this bundles (a) agent-only markdown under
.claude/skills/with zero runtime impact, (b) a ~1800-LOC experimental CLI subsystem that writes AWS credentials, and (c) a breaking rename of an already-shipped command. They have different audiences, different risk profiles, and different revert stories: the rename is the one most likely to need a fast follow-up or a release note, and it's currently welded to a large feature branch. Not a change request on this PR — but if the rename landed separately it could be communicated and reverted on its own cadence. Flagging underAGENTS.md's "every change should impact as little code as possible".
CI Status
- integration tests (Fastly EC lifecycle): PASS
- integration tests: PASS
- browser integration tests: PASS
- CodeQL: PASS
- cargo test (ts CLI, native): PASS
- vitest: PASS
- format-typescript: PASS (required)
- Analyze (javascript-typescript): PASS
- cargo fmt: PASS (required)
- cargo test (axum native): PASS
- Analyze (rust): PASS
- cargo check (cloudflare native + wasm32-unknown-unknown): PASS
- cargo test: PASS (required)
- format-docs: PASS (required)
- cargo check/build/test (spin native + wasm32-wasip1): PASS
- cargo test (cross-adapter parity): PASS
- CLAUDE.md symlink guard: PASS
- prepare integration artifacts: PASS
- Analyze (actions): PASS
No failed, cancelled, or pending checks.
jevansnyc
left a comment
There was a problem hiding this comment.
Nothing major so posting in here as single comment:
Breaking rename with no alias — [run.rs:76] ts prebid bundle became ts prebid client. Any existing script gets error: unrecognized subcommand 'bundle'. Confirmed against the built binary. Either add a hidden alias or call the break out in the PR description.
.tool-versions aws plugin name is wrong — [.tool-versions:6] The entry is aws 2.36.45, but asdf/mise call that plugin awscli. asdf install fails on the exact onboarding path the docs point at. CI is unaffected since the workflows grep only rust/nodejs/viceroy.
Regional module has no provider pin — [modules/regional/terraform.tf:3] No AWS provider version constraint, and [.gitignore:3] excludes its lock file, yet RUNBOOK.md tells operators to init/test inside the module. So the module test runs against an unpinned provider that will drift away from the root's = 6.64.0.
Confirm prompt rejects long input instead of declining — [pbs/mod.rs:228] Terminal::confirm caps the answer at 16 bytes and returns "input exceeds size limit" rather than treating it as a no. A 17-character answer makes the operator re-enter the secret through the hidden prompt.
CPU alarm pages on stopped hosts — [modules/regional/monitoring.tf:13] The per-instance high-CPU alarm sets treat_missing_data = "breaching", so a stopped or replaced instance pages as saturated.
ONE QUESTION for Christian:
The test plan lists terraform fmt, init, and validate, but not terraform test, even though both .tftest.hcl files ship and the README/RUNBOOK instruct operators to run them. Worth confirming those two suites actually ran. validate leaves module inputs unknown and evaluates very little of the plan.
Summary
ts prebid: browser bundle generation ists prebid client, while self-hosted Prebid Server operations are underts prebid server.deploy/pbs-example/.The earlier post-merge inspection compatibility blocker is resolved at the current head. The CLI tests now pass against the current configuration schema.
Changes
.claude/skills/planning-prebid-aws/ts prebid server.crates/trusted-server-cli/src/commands/pbs/crates/trusted-server-cli/src/run.rsts prebid.crates/trusted-server-cli/tests/pbs_cli.rsand module testscrates/trusted-server-cli/README.mdandexamples/pbs/deploy/pbs-example/ts prebid bundlereferences withts prebid client..tool-versionsCLI layout
Client
This generates the publisher-specific browser bundle. It replaces the former
ts prebid bundlecommand.Server
ts prebid server inspect --config <file>ts prebid server check --deployment <file>ts prebid server secrets set <bidder> --deployment <file> --region <region>ts prebid server status --deployment <file>Add
--jsonanywhere underts prebid serverfor machine-readable output.Secret values stay out of process arguments and reports. AWS request payloads use owner-only temporary files on Unix and are removed on normal success and error paths. Raw AWS stderr is withheld. Abrupt termination can leave temporary files, so operators must use protected temporary storage. Windows ACL behavior has not been validated.
Example deployment
deploy/pbs-example/is a committed, locally checked example. It models:us-east-1andus-west-2.v4.7.0pinned to a verified image digest.The example uses fictional account, certificate, hosted-zone, AMI, instance, CIDR, and bidder values. It is not deployable as-is. It has no remote Terraform backend, WAF, runtime secret loader, deployment command, rollback command, or load-test evidence.
Boundaries
The deployment descriptor supports
ec2-composeonly. The planning skill may recommend ECS or another approved architecture, but that requires separate CLI support.There are no
deployorrollbackserver subcommands. This PR does not provision infrastructure, install a runtime secret loader, replace containers, adopt sandbox state, activate bidders, or change caller traffic. The example files document those deferred operations rather than presenting them as implemented.The planning skill now treats AWS RTB Fabric as an optional outbound path per bidder and region. It records partner participation and acceptance, PBS endpoint mapping, regional quotas and timeouts, cost, fallback, monitoring, and Terraform link lifecycle limitations. It does not provision gateways or links, automate partner acceptance, or replace the current EC2/Compose descriptor.
Live AWS operations still require explicit operator authorization. No real AWS calls or secret writes were made during implementation.
Open questions
ec2-compose, or should this skill stay focused on the currently supported Compose/EC2 path? ECS would improve managed task replacement and deployment behavior, but it adds a second runtime and CLI contract.ts prebid server statusdescriptor accepts explicit instance IDs, not ASG membership.Test plan
./scripts/test-cli.shcargo fmt --all -- --checkcargo clippy --package trusted-server-cli --all-targets --target x86_64-unknown-linux-gnu -- -D warningscd crates/trusted-server-js/lib && npm run formatcd crates/trusted-server-js/lib && npx vitest runwith 901 tests passingcd crates/trusted-server-js/lib && node build-all.mjsts prebid,ts prebid client --help, andts prebid server --helpterraform fmt -check -recursive deploy/pbs-exampleterraform -chdir=deploy/pbs-example init -backend=false -input=falseterraform -chdir=deploy/pbs-example validatecargo run_cli_linux prebid server check --deployment deploy/pbs-example/deployment.yaml --jsondocker compose --env-file deploy/pbs-example/runtime/examples/compose.env -f deploy/pbs-example/runtime/compose.yaml config --quietdeploy/pbs-example/runtime/secret-bindings.jsonwith Pythondeploy/pbs-test/remains untracked and untouchedDeferred evidence
Checklist
disable-model-invocation: true.unwrap()calls.Closes
Closes #1163