Skip to content

Add an AWS deployment planning skill for Prebid Server Go - #1166

Open
ChristianPavilonis wants to merge 11 commits into
mainfrom
feature/terraform-skill
Open

ChristianPavilonis wants to merge 11 commits into
mainfrom
feature/terraform-skill

Conversation

@ChristianPavilonis

@ChristianPavilonis ChristianPavilonis commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Add an explicitly invoked skill for planning Prebid Server Go deployments on AWS and generating approved Terraform/runtime files from an operator interview.
  • Add experimental Prebid Server inspection, validation, secret management, and EC2 status commands to the existing Trusted Server CLI.
  • Organize the CLI under ts prebid: browser bundle generation is ts prebid client, while self-hosted Prebid Server operations are under ts prebid server.
  • Add a production-shaped, locally checked EC2/Compose Terraform example under deploy/pbs-example/.
  • Keep planning and file generation separate from AWS execution. Deployment, rollback, and runtime secret injection remain deferred.

The earlier post-merge inspection compatibility blocker is resolved at the current head. The CLI tests now pass against the current configuration schema.

Changes

Files Change
.claude/skills/planning-prebid-aws/ Defines explicit invocation, requirements gathering, design approval, generation, and evidence gates. Adds an optional AWS RTB Fabric outbound bidder-connectivity interview and lifecycle guidance. Routes supported operations through ts prebid server.
crates/trusted-server-cli/src/commands/pbs/ Implements local inspection and validation, guarded Secrets Manager writes, and EC2 infrastructure status.
crates/trusted-server-cli/src/run.rs Exposes the client and server command groups under ts prebid.
crates/trusted-server-cli/tests/pbs_cli.rs and module tests Exercise the real CLI against a fake AWS executable, plus discovery, merging, validation, redaction, target refusal, retries, and partial status.
crates/trusted-server-cli/README.md and examples/pbs/ Document usage, schemas, authorization boundaries, recovery, and limitations with fictional fixtures.
deploy/pbs-example/ Adds a two-region, two-AZ-per-region example with regional ALBs, Route 53 latency aliases, private EC2 Compose hosts, per-AZ NAT, monitoring, Secrets Manager metadata, runtime examples, a plan, and a runbook.
Prebid guides, examples, and diagnostics Replace ts prebid bundle references with ts prebid client.
.tool-versions Adds AWS CLI tooling and aligns the existing entries.

CLI layout

Client

ts prebid client [--config <path>] [--out <dir>]

This generates the publisher-specific browser bundle. It replaces the former ts prebid bundle command.

Server

Command Current scope
ts prebid server inspect --config <file> Reads selected local configuration without changing it. Sensitive account, endpoint, and bid-parameter values are withheld.
ts prebid server check --deployment <file> Validates descriptor and binding structure plus deterministic regional YAML merges without AWS calls.
ts prebid server secrets set <bidder> --deployment <file> --region <region> Writes a complete JSON payload to an existing declared secret after identity, metadata, history, and confirmation checks.
ts prebid server status --deployment <file> Reads declared EC2 instances and reports infrastructure state, not PBS health or readiness.

Add --json anywhere under ts prebid server for 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:

  • Route 53 latency routing to one ALB in each of us-east-1 and us-west-2.
  • Two AZs per region, with one private PBS EC2 host per AZ.
  • Docker Compose as the host runtime.
  • Per-AZ NAT gateways for outbound bidder access.
  • Secrets Manager metadata and EC2 read permissions without committing secret values.
  • PBS Go v4.7.0 pinned 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-compose only. The planning skill may recommend ECS or another approved architecture, but that requires separate CLI support.

There are no deploy or rollback server 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

  • ECS support: Should a future version add an ECS/Fargate example and descriptor support alongside 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.
  • Instance replacement: Should the example move from fixed EC2 instances to launch templates and Auto Scaling Groups? That would improve host replacement, but the current ts prebid server status descriptor accepts explicit instance IDs, not ASG membership.
  • Terraform state: Should a later production profile use a separately bootstrapped S3 backend with native lock-file locking instead of the demo's local state?
  • Capacity evidence: What real peak QPS, bidder fan-out, caller timeout, regional failover target, and latency SLO should replace the fictional planning assumptions before anyone treats the topology as capacity-tested?

Test plan

  • ./scripts/test-cli.sh
  • cargo fmt --all -- --check
  • cargo clippy --package trusted-server-cli --all-targets --target x86_64-unknown-linux-gnu -- -D warnings
  • cd crates/trusted-server-js/lib && npm run format
  • cd crates/trusted-server-js/lib && npx vitest run with 901 tests passing
  • cd crates/trusted-server-js/lib && node build-all.mjs
  • Smoke-tested ts prebid, ts prebid client --help, and ts prebid server --help
  • terraform fmt -check -recursive deploy/pbs-example
  • terraform -chdir=deploy/pbs-example init -backend=false -input=false
  • terraform -chdir=deploy/pbs-example validate
  • cargo run_cli_linux prebid server check --deployment deploy/pbs-example/deployment.yaml --json
  • docker compose --env-file deploy/pbs-example/runtime/examples/compose.env -f deploy/pbs-example/runtime/compose.yaml config --quiet
  • Parsed deploy/pbs-example/runtime/secret-bindings.json with Python
  • Verified deploy/pbs-test/ remains untracked and untouched

Deferred evidence

  • Fresh-agent skill invocation and end-to-end file-generation exercise
  • Live AWS authentication and service integration
  • Real-terminal interaction, PBS startup, and runtime delivery
  • Real bidder authorization, credential injection, and optional AWS RTB Fabric connectivity
  • Representative load, failover, replacement, alert delivery, and latency measurements
  • macOS and Windows validation

Checklist

  • Skill requires explicit invocation with disable-model-invocation: true.
  • New code has unit and process-level tests; production code adds no unwrap() calls.
  • Credentials are absent from committed examples and reports; test payloads use dummy values.
  • Unsupported operations and unverified integration behavior are documented rather than reported as working.
  • The example deployment uses fictional AWS and domain values and does not authorize cloud execution.

Closes

Closes #1163

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
@ChristianPavilonis
ChristianPavilonis marked this pull request as ready for review September 15, 2026 23:06

@aram356 aram356 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 prk-Jr left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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, against cargo fmt --all -- --check, both host-target trusted-server-cli clippy invocations with -D warnings, and cargo 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 survive cargo fmt in the reviewed form.

Blocking

🔧 wrench

  • AWS CLI pinned to latest while every other tool is exact — see inline at .tool-versions:6

Non-blocking

♻️ refactor

  • rpassword / serde_yaml_ng bypass workspace dependency inheritance — see inline at crates/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 at crates/trusted-server-cli/src/commands/pbs/inspect.rs:91 (suggestion)

🤔 thinking

  • ts prebid bundlets prebid client is a breaking rename with no alias — see inline at crates/trusted-server-cli/src/run.rs:75 (suggestion)
  • Transport failure on put-secret-value doesn't flag the outcome as uncertain — see inline at crates/trusted-server-cli/src/commands/pbs/secrets.rs:173
  • A dated Status: Implemented spec is rewritten retroactively — see inline at docs/superpowers/specs/2026-06-17-prebid-bundle-cli-design.md:5
  • The ts prebid server namespace is undocumented in docs/ — see inline at docs/guide/cli.md:274
  • Missing python3 makes two tests pass for the wrong reason — see inline at crates/trusted-server-cli/tests/pbs_cli.rs:23
  • identifier() rejects AWS profile names containing . — see inline at crates/trusted-server-cli/src/commands/pbs/config.rs:104

⛏ nitpick

  • Unparenthesized &&/|| on the noninteractive-write gate — see inline at crates/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 under AGENTS.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.

Comment thread .tool-versions Outdated
Comment thread crates/trusted-server-cli/Cargo.toml Outdated
Comment thread crates/trusted-server-cli/src/commands/pbs/config.rs
Comment thread crates/trusted-server-cli/src/commands/pbs/inspect.rs Outdated
Comment thread crates/trusted-server-cli/src/run.rs
Comment thread crates/trusted-server-cli/src/commands/pbs/status.rs
Comment thread crates/trusted-server-cli/src/commands/pbs/secrets.rs Outdated
Comment thread crates/trusted-server-cli/src/commands/pbs/config.rs
Comment thread crates/trusted-server-cli/src/commands/pbs/config.rs Outdated
Comment thread crates/trusted-server-cli/tests/pbs_cli.rs
@aram356
aram356 requested a review from jevansnyc September 17, 2026 15:27

@jevansnyc jevansnyc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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.

Create terraform deployment skill for prebid server dependency for TS

4 participants