Firewall rules support

[yusufozturk]

Summary by CodeRabbit

• New Features
  • Added support for a new HTTP source scenario, drop, alongside the existing deliver and reject options.
  • Updated scenario validation and verification so dropped requests are treated distinctly from rejected ones, with clearer expected outcomes for request acceptance and delivery counts.

[cloudflare-workers-and-pages]

Deploying pipebench with    Cloudflare Pages

Latest commit:	998c7de
Status:	✅  Deploy successful!
Preview URL:	https://63650ce6.pipebench.pages.dev
Branch Preview URL:	https://dev-firewall-rules.pipebench.pages.dev

View logs

[coderabbitai]

Walkthrough

This change adds support for a new "drop" scenario in HTTP source correctness testing. The config validator now accepts "drop" alongside "deliver" and "reject", and the runner's test driver adds logic to verify sender-side acceptance combined with zero receiver-side delivery.

Changes

Drop Scenario Support

Layer / File(s)	Summary
Config validation for drop scenario internal/config/case.go	validateHTTPSource now accepts "drop" as a valid http_source.scenario value in addition to "deliver" and "reject".
Runner driver logic for drop scenario internal/runner/runner.go	Adds a new hs.Scenario == "drop" branch in runHTTPSourceCorrectness that waits for sender-side acceptance of all requests, verifies receiver count stays at zero, and fails on any delivered records or under-accepted sender evidence; also updates the scenario documentation comment.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Runner
  participant Sender
  participant Receiver
  Runner->>Sender: poll /stats for accepted request count
  Sender-->>Runner: accepted count reaches expected total
  Runner->>Receiver: poll delivered record count
  Receiver-->>Runner: count remains zero
  Runner->>Runner: fail if records delivered or sender under-accepted

Loading

Poem

A hop, a drop, a scenario new,
Requests accepted, but nothing gets through!
I twitch my nose and check the stats,
Zero delivered — no dodgy rats!
Carrots for the code that keeps drops true 🥕🐇

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The title suggests firewall rule work, but the changes are about HTTP source scenario validation and runner behavior for drop/reject handling.	Rename it to describe the HTTP source scenario update, such as "Add drop scenario support to HTTP source checks".

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ 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 dev-firewall-rules

---

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

[coderabbitai]

🧹 Nitpick comments (2)

internal/runner/runner.go (1)

5233-5314: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff

Reduce duplication between "reject" and "drop" branches.

The new "drop" branch (Lines 5261-5292) is nearly identical to the existing "reject" branch (Lines 5233-5260): same polling loop, same ccfWaitStable call, same got > 0 check, differing only in which sender-stat field is validated (rejected vs accepted) and the message text. Consider extracting the shared polling/verification logic into a helper parameterized by scenario name and the expected-count field, reducing risk of divergence when one branch is updated but not the other.

🤖 Prompt for 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.

In `@internal/runner/runner.go` around lines 5233 - 5314, The reject and drop
paths in runner.go duplicate the same polling and delivery-check flow, so
extract the shared logic from the Scenario-specific block in
ccfWaitStable/httpSenderStats handling into a helper. Keep the scenario-specific
validation as parameters (reject vs accept field, and the corresponding
success/error text), and have the helper drive the common wait-for-sender,
stable-read, and got>0 checks to avoid drift between the two branches.

internal/config/case.go (1)

1516-1520: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Update Scenario field doc comment to mention "drop".

The HTTPSourceConfig.Scenario comment only documents "deliver" and "reject", but the validator now also accepts "drop" (Line 1566 in this file). Leaving the doc stale will confuse config authors.

📝 Proposed doc update

 	// Scenario: "deliver" (a valid credential → all Count events reach the
 	// receiver) or "reject" (a bad credential → the source rejects every request,
-	// nothing is delivered, and the sender records Count rejections).
+	// nothing is delivered, and the sender records Count rejections), or "drop"
+	// (a valid credential accepted by the source, but a downstream pipeline
+	// drops every record before it reaches the receiver).
 	Scenario string `yaml:"scenario"`

🤖 Prompt for 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.

In `@internal/config/case.go` around lines 1516 - 1520, Update the
HTTPSourceConfig.Scenario doc comment to include the newly accepted "drop"
scenario alongside "deliver" and "reject". Edit the comment on HTTPSourceConfig
so it accurately reflects the validator behavior in the same type, keeping the
wording concise and consistent with the existing scenario descriptions.

🤖 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.

Nitpick comments:
In `@internal/config/case.go`:
- Around line 1516-1520: Update the HTTPSourceConfig.Scenario doc comment to
include the newly accepted "drop" scenario alongside "deliver" and "reject".
Edit the comment on HTTPSourceConfig so it accurately reflects the validator
behavior in the same type, keeping the wording concise and consistent with the
existing scenario descriptions.

In `@internal/runner/runner.go`:
- Around line 5233-5314: The reject and drop paths in runner.go duplicate the
same polling and delivery-check flow, so extract the shared logic from the
Scenario-specific block in ccfWaitStable/httpSenderStats handling into a helper.
Keep the scenario-specific validation as parameters (reject vs accept field, and
the corresponding success/error text), and have the helper drive the common
wait-for-sender, stable-read, and got>0 checks to avoid drift between the two
branches.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f9771e87-1bce-4bc7-8b38-2020666f7705

📥 Commits

Reviewing files that changed from the base of the PR and between 36871e2 and 998c7de.

📒 Files selected for processing (2)

• internal/config/case.go
• internal/runner/runner.go