Skip to content

Add sample file support to generator configuration and orchestration#4

Merged
yusufozturk merged 3 commits into
mainfrom
generator-inputfile-update
Jun 7, 2026
Merged

Add sample file support to generator configuration and orchestration#4
yusufozturk merged 3 commits into
mainfrom
generator-inputfile-update

Conversation

@namles

@namles namles commented Jun 6, 2026

Copy link
Copy Markdown
Member

Introduce support for a sample input file in the generator configuration, allowing the generator to replay file contents instead of synthesizing lines. This update includes necessary changes to the configuration structure and adds tests to verify the functionality.

Summary by CodeRabbit

  • New Features

    • Generators can replay lines from a case-relative sample file (cycled to meet target) and optionally rewrite RFC3164 syslog timestamps on replay; replay is selectable via environment settings and is exposed in compose output as optional mounts/env.
  • Bug Fixes / Validation

    • Sample-file paths must be case-relative (no absolute paths or .. traversal); compose generation validates sample-file existence.
  • Tests

    • Added tests for compose rendering and per-generator sample-file behavior.
  • Chores

    • Orchestration now receives absolute case directory for accurate resolution.

@coderabbitai

coderabbitai Bot commented Jun 6, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

Pull request was closed or merged during review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d74a69f2-1ac7-4ed6-a689-1579c6916216

📥 Commits

Reviewing files that changed from the base of the PR and between 1fc1b22 and 806fae8.

📒 Files selected for processing (5)
  • containers/generator/main.go
  • internal/config/case.go
  • internal/orchestrator/compose_render_test.go
  • internal/orchestrator/docker.go
  • internal/runner/runner.go

Walkthrough

The PR adds generator sample-file replay: case-relative files are preloaded and cycled verbatim (with optional RFC3164 timestamp rewriting). Changes span config schema/validation, generator runtime, Docker Compose wiring and tests, and runner case-dir plumbing.

Changes

Sample File Replay Feature

Layer / File(s) Summary
Configuration Schema and RunConfig Extension
internal/config/case.go, internal/orchestrator/docker.go
GeneratorConfig adds SampleFile and RewriteTimestamp YAML fields; TestCase.Validate checks sample paths are case-relative. RunConfig gains CaseDir to resolve case-relative inputs.
Generator Sample-File Replay Implementation
containers/generator/main.go
Adds regexp import; reads GENERATOR_SAMPLE_FILE and GENERATOR_REWRITE_TIMESTAMP; preloads sample lines at startup; sendLinesConn uses the preloaded lines as the primary source (disabling sequenced buffering when active); the send loop prioritizes replay, then sequenced, then sampled/timestamped, then template fallback. Helpers normalize lines and optionally rewrite RFC3164 header dates while preserving <PRI>.
Docker Compose Template and Variable Wiring
internal/orchestrator/docker.go
Plural and singular generator compose templates now conditionally mount case-relative sample files read-only and inject GENERATOR_SAMPLE_FILE and GENERATOR_REWRITE_TIMESTAMP. Adds template fields and resolveSampleHost to validate/resolve host/container paths.
Compose Rendering Test Coverage
internal/orchestrator/compose_render_test.go
Adds TestSingularComposeRendersSampleFile and TestPluralComposeRendersSampleFile and writeSampleFile helper to verify per-case mount and env var rendering for single and multiple generators.
Runner CaseDir Wiring and Subject Override
internal/runner/runner.go
Runner computes absolute per-test-case CaseDir and sets orchestrator.RunConfig.CaseDir for Run and persistence variants; applySubjectOverrides applies optional SubjectVersion after image override.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~40 minutes

Poem

🐰 I found a file beneath the tree,
Lines to replay, again for me,
Dates refreshed to dance and spin,
Mounted safe — read-only within,
Hopping through compose, runner, and generator glee!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 69.23% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main changes across all modified files, which focus on adding sample file replay support to the generator configuration and orchestration layers.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 generator-inputfile-update

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

@namles namles requested a review from yusufozturk June 6, 2026 08:54

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 4

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

Inline comments:
In `@containers/generator/main.go`:
- Around line 833-847: The sample file is being loaded inside the per-connection
path (the block that calls loadSampleLines when cfg.SampleFile != ""), causing
repeated parallel reads; move the loadSampleLines call to the program
startup/config initialization and store the resulting [][]byte on the config
(e.g., add a field like Config.SampleLines) so workers reuse it; update
sendLinesConn (and any callers) to check cfg.SampleLines instead of reloading,
remove the per-connection load logic, and keep the same error checks (empty
file) during startup so workers assume preloaded data.

In `@internal/config/case.go`:
- Around line 196-207: Validate that SampleFile is a case-relative path and
reject absolute or escaping paths: in the Case struct validation (where
SampleFile is defined) ensure filepath.IsAbs(SampleFile) is false, clean the
path with filepath.Clean, then compute rel, err := filepath.Rel(CaseDir,
filepath.Join(CaseDir, cleaned)) and require err == nil and that rel does not
start with ".." before accepting; return a validation error otherwise so the
join/mount logic used by the Docker generator code (the join logic that consumes
CaseDir + SampleFile) cannot escape the case directory.

In `@internal/orchestrator/docker.go`:
- Around line 853-857: The code silently skips mounting/setting env when
g.SampleFile is set but cfg.CaseDir is empty and does not validate the sample
path; update the logic that calculates sampleHost/sampleDst/sampleFile
(referencing g.SampleFile and cfg.CaseDir in the docker compose preparation
function) to fail fast: if g.SampleFile is non-empty then resolve the host path
by joining cfg.CaseDir and g.SampleFile (or a singular/plural resolver you
provide), verify the resulting path exists and is a regular file (return an
error if CaseDir is empty or the file is missing/not regular), and only then set
sampleHost, sampleDst and sampleFile; apply the same validation for the parallel
block around the lines handling the other sample logic (the 914-919 region) so
compose rendering never proceeds with an unresolved sample_file.

In `@internal/runner/runner.go`:
- Around line 222-223: The CaseDir constructed with
filepath.Join(r.opts.CasesDir, tc.Name) should be resolved to an absolute path
before handing it to orchestration; change the assignment that sets CaseDir to
use filepath.Abs (or filepath.Join followed by filepath.Abs) on r.opts.CasesDir/
tc.Name so the host bind mount for sample_file always points to the correct
absolute location; update the same pattern in all four run paths where CaseDir
is set (instances referencing CaseDir, r.opts.CasesDir, tc.Name and
filepath.Join) so orchestration receives an absolute path.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e52b1794-0b9c-442b-ac45-a7ad487ea03b

📥 Commits

Reviewing files that changed from the base of the PR and between fec4370 and cea5db1.

📒 Files selected for processing (5)
  • containers/generator/main.go
  • internal/config/case.go
  • internal/orchestrator/compose_render_test.go
  • internal/orchestrator/docker.go
  • internal/runner/runner.go

Comment thread containers/generator/main.go Outdated
Comment thread internal/config/case.go
Comment thread internal/orchestrator/docker.go Outdated
Comment thread internal/runner/runner.go Outdated
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 6, 2026

Copy link
Copy Markdown

Deploying pipebench with  Cloudflare Pages  Cloudflare Pages

Latest commit: 806fae8
Status: ✅  Deploy successful!
Preview URL: https://12492e0a.pipebench.pages.dev
Branch Preview URL: https://generator-inputfile-update.pipebench.pages.dev

View logs

…pdate

# Conflicts:
#	internal/config/case.go
#	internal/orchestrator/compose_render_test.go
#	internal/orchestrator/docker.go
@yusufozturk

Copy link
Copy Markdown
Member

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jun 7, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@yusufozturk yusufozturk merged commit c2fb224 into main Jun 7, 2026
3 of 4 checks passed
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.

2 participants