fix(syslog): quote payloadKey path to prevent VRL evaluation as expression#3327
Conversation
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughSyslog ChangesSyslog payload_key handling
Estimated code review effort: 2 (Simple) | ~12 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/test functional-target |
2 similar comments
|
/test functional-target |
|
/test functional-target |
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/generator/vector/output/syslog/syslog.go (1)
126-143: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winEscape
payloadKeybefore embedding it in VRL
payloadKeyallows quoted field-path segments (for example{."foo.bar"}), butfmt.Sprintf(payloadKeyConfiguredVRL, key)inserts the raw path intopayload_key = "%s". Any embedded"or\will produce invalid VRL and prevent the syslog transform from compiling. Escape the value here, or reject quoted segments.🤖 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/generator/vector/output/syslog/syslog.go` around lines 126 - 143, The payloadKeyConfiguredVRL template in syslog.go is embedding the raw payloadKey into a quoted VRL string, which can break compilation when the key contains quoted path segments or escape characters. Update the syslog VRL generation path that formats payloadKeyConfiguredVRL so the key is safely escaped before being passed to fmt.Sprintf, or validate/reject unsupported quoted segments in the payload key handling logic.
🤖 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.
Outside diff comments:
In `@internal/generator/vector/output/syslog/syslog.go`:
- Around line 126-143: The payloadKeyConfiguredVRL template in syslog.go is
embedding the raw payloadKey into a quoted VRL string, which can break
compilation when the key contains quoted path segments or escape characters.
Update the syslog VRL generation path that formats payloadKeyConfiguredVRL so
the key is safely escaped before being passed to fmt.Sprintf, or validate/reject
unsupported quoted segments in the payload key handling logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 4aeb287b-52d2-4e20-88c8-afd51d67a6b4
📒 Files selected for processing (5)
Makefileinternal/generator/vector/output/syslog/syslog.gointernal/generator/vector/output/syslog/tls_with_field_references.tomlinternal/generator/vector/output/syslog/udp_with_every_setting.tomltest/functional/outputs/syslog/rfc5424_test.go
|
/approve |
openshift-ci
Bot
added
the
approved
|
/retest-required |
2 similar comments
|
/retest-required |
|
/retest-required |
openshift-ci
Bot
added
the
do-not-merge/hold
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jcantrill, vparfonov The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
1 similar comment
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jcantrill, vparfonov The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest-required |
…ssion The payloadKey was injected unquoted into VRL, causing it to be evaluated as a field access expression rather than kept as a path string. This meant the resolved value was then re-interpreted as a path via split+get, which always failed, producing repeated "payload_key not found" warnings. Fixes: LOG-9562 Signed-off-by: Vitalii Parfonov <vparfono@redhat.com>
|
/lgtm |
|
/hold cancel |
openshift-ci
Bot
removed
the
do-not-merge/hold
|
@vparfonov: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
openshift-merge-bot
Bot
merged commit 5d1df70
into
openshift:master
2 participants
Description
Root Cause:
The payloadKey path (e.g., .message) was injected unquoted into the VRL template:
payload_key = .message # evaluates as VRL expression, reads the VALUE of .messageThis resolved
.messageto its runtime value (e.g., "Hello World"), which was then treated as a field path looking for a field literally named "Hello World", which doesn't exist.For payloadKey:
"{.message}"this only produced noisy warnings (since.messagewas left unchanged). For any other field (e.g.,{.kubernetes.namespace_name}), it silently failed to set.messageto the intended value.Fix:
Quote the path as a string literal and strip the leading dot before splitting:
/cc @Clee2691
/assign @jcantrill
Links
Summary by CodeRabbit