fix(x-pack/winlogbeat): run ingest pipeline tests on CI #47395
fix(x-pack/winlogbeat): run ingest pipeline tests on CI #47395andrewkroh wants to merge 5 commits into
Conversation
At some point the CI tests for the ingest pipeline in Winlogbeat stopped being tested on CI because there was no linux host running the unit tests. The unit tests for the pipeline tests require Docker for running Elasticsearch. This adds a new Buildkite step for x-pack/winlogbeat. It fixes the tests there broken. It removes the request body from an eslegclient debug statement that was very noisy, and in some cases extremely lengthy. It removes the deprecated 'version' from the docker-compose.yaml file used to run ES. The pipelines were mostly fine because they have been copied from elastic/integrations where they are independently testing.
[git-generate] go -C ./x-pack/winlogbeat/module/security test ./... -v -update go -C ./x-pack/winlogbeat/module/sysmon test ./... -v -update go -C ./x-pack/winlogbeat/module/powershell test ./... -v -update
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
|
This pull request does not have a backport label.
To fixup this pull request, you need to add the backport labels for the needed
|
|
This pull request doesn't have a |
🔍 Preview links for changed docs |
|
Pinging @elastic/sec-windows-platform (Team:Security-Windows Platform) |
|
Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane) |
colleenmcginnis
left a comment
There was a problem hiding this comment.
Docs change looks okay.
leehinman
left a comment
There was a problem hiding this comment.
LGTM. One question with SidList but it isn't blocking.
| "computer_name": "WIN-BVM4LI1L1Q6.TEST.local", | ||
| "event_data": { | ||
| "SidList": [ | ||
| "", |
There was a problem hiding this comment.
Is this expected in the SidList?
There was a problem hiding this comment.
No, but the intention is for the "standard" pipeline to match what's in the Fleet integration at https://github.com/elastic/integrations/blob/80b968725f9c769f86522c69a9cdc52a30d9e231/packages/windows/data_stream/forwarded/elasticsearch/ingest_pipeline/security_standard.yml.
And currently this is how the windows.forwarded pipeline is behaving as per https://github.com/elastic/integrations/pull/15797/files#diff-a31adecc2e9c3b480a32c5672cf718fe6d4777d94c8939c1d52d16953a0c6a2aR42
However, also as identified in my integrations PR, the system.security pipeline does not have this problem. The solution will be to get Fleet's system and windows pipelines synced up by using the file reference feature and then sync beats. https://github.com/elastic/integrations/pull/15797/files#r2474625479 🤯
|
@nfritts Your review is needed here :-) |
|
@nfritts we still need your approval here |
|
This pull request is now in conflicts. Could you fix it? 🙏 |
✅ Vale Linting ResultsNo issues found on modified lines! The Vale linter checks documentation changes against the Elastic Docs style guide. To use Vale locally or report issues, refer to Elastic style guide for Vale. |
📝 WalkthroughWalkthroughThis PR updates Winlogbeat to ECS 8.17.0 across all test fixtures, adds a Suggested labels
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
x-pack/winlogbeat/module/wintest/simulate.go (1)
137-157:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPer-document simulate failures are now silently ignored by the existing checks.
Lines 138-140 append the raw Elasticsearch simulate error object, but downstream validation still calls
wintest.ErrorMessage, which only recognizes processed events carryingerror.message(seex-pack/winlogbeat/module/testing.goLines 159-162 andx-pack/winlogbeat/module/wintest/simulate_test.goLines 153-156). A simulate failure returned as{"type": "...", "reason": "..."}will therefore normalize successfully and be treated as a passing event. Please either convert per-doc simulate errors into the sameerror.messageshape the rest of the harness expects, or teachErrorMessageto parse Elasticsearch simulate error objects too.🤖 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 `@x-pack/winlogbeat/module/wintest/simulate.go` around lines 137 - 157, The per-document simulate failures (pipelineIngestedDocument.Error) are being appended as raw ES error objects so downstream ErrorMessage doesn't recognize them; change the loop in simulate.go that iterates response.Docs to, when doc.Error != nil, convert the raw ES error into the harness-expected shape (an object with error.message) before appending to events (e.g. build an event where "error": {"message": "<type>: <reason>" or use the raw error JSON merged into "error.message"}), keeping the rest of the logic the same; reference simulatePipelineResponse, pipelineIngestedDocument, and ErrorMessage to ensure the produced shape matches what ErrorMessage expects.
🤖 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 @.buildkite/x-pack/pipeline.xpack.winlogbeat.yml:
- Line 95: The pipeline references an undeclared env var
GCP_DEFAULT_MACHINE_TYPE; fix by declaring GCP_DEFAULT_MACHINE_TYPE in the env
section (set it to the desired machine type) or update the step to use an
existing env var such as GCP_WIN_MACHINE_TYPE or GCP_HI_PERF_MACHINE_TYPE so the
step uses a defined variable; ensure the change touches the env block and the
step that currently uses GCP_DEFAULT_MACHINE_TYPE.
---
Outside diff comments:
In `@x-pack/winlogbeat/module/wintest/simulate.go`:
- Around line 137-157: The per-document simulate failures
(pipelineIngestedDocument.Error) are being appended as raw ES error objects so
downstream ErrorMessage doesn't recognize them; change the loop in simulate.go
that iterates response.Docs to, when doc.Error != nil, convert the raw ES error
into the harness-expected shape (an object with error.message) before appending
to events (e.g. build an event where "error": {"message": "<type>: <reason>" or
use the raw error JSON merged into "error.message"}), keeping the rest of the
logic the same; reference simulatePipelineResponse, pipelineIngestedDocument,
and ErrorMessage to ensure the produced shape matches what ErrorMessage expects.
🪄 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: Enterprise
Run ID: 574ae3ed-3eeb-4479-a415-31b7e48ab208
📒 Files selected for processing (126)
.buildkite/x-pack/pipeline.xpack.winlogbeat.ymldocs/reference/winlogbeat/exported-fields-powershell.mdlibbeat/esleg/eslegclient/connection.gox-pack/winlogbeat/module/.gitignorex-pack/winlogbeat/module/powershell/_meta/fields.ymlx-pack/winlogbeat/module/powershell/fields.gox-pack/winlogbeat/module/powershell/test/testdata/ingest/400.golden.jsonx-pack/winlogbeat/module/powershell/test/testdata/ingest/403.golden.jsonx-pack/winlogbeat/module/powershell/test/testdata/ingest/4103.golden.jsonx-pack/winlogbeat/module/powershell/test/testdata/ingest/4104.golden.jsonx-pack/winlogbeat/module/powershell/test/testdata/ingest/4105.golden.jsonx-pack/winlogbeat/module/powershell/test/testdata/ingest/4106.golden.jsonx-pack/winlogbeat/module/powershell/test/testdata/ingest/600.golden.jsonx-pack/winlogbeat/module/powershell/test/testdata/ingest/800.golden.jsonx-pack/winlogbeat/module/security/ingest/security.ymlx-pack/winlogbeat/module/security/ingest/security_standard.ymlx-pack/winlogbeat/module/security/test/testdata/ingest/1100.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/1102.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/1104.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/1105.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/4670_WindowsSrv2016.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/4706_WindowsSrv2016.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/4707_WindowsSrv2016.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/4713_WindowsSrv2016.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/4716_WindowsSrv2016.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/4717_WindowsSrv2016.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/4718_WindowsSrv2016.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/4719.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/4719_WindowsSrv2016.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/4739_WindowsSrv2016.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/4741.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/4742.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/4743.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/4744.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/4745.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/4746.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/4747.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/4748.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/4749.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/4750.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/4751.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/4752.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/4753.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/4759.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/4760.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/4761.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/4762.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/4763.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/4778.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/4779.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/4797.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/4817_WindowsSrv2016.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/4902_WindowsSrv2016.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/4904_WindowsSrv2016.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/4905_WindowsSrv2016.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/4906_WindowsSrv2016.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/4907_WindowsSrv2016.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/4908_WindowsSrv2016.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/5379.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/5380.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/5381.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/5382.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2012_4673.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2012_4674.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2012_4697.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2012_4698.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2012_4699.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2012_4700.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2012_4701.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2012_4702.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2012_4768.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2012_4769.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2012_4770.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2012_4771.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2012_4776.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2012_4778.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2012_4779.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2012r2-logon.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2016-4672.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2016-logoff.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2016_4720_Account_Created.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2016_4722_Account_Enabled.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2016_4723_Password_Change.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2016_4724_Password_Reset.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2016_4725_Account_Disabled.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2016_4726_Account_Deleted.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2016_4727.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2016_4728.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2016_4729.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2016_4730.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2016_4731.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2016_4732.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2016_4733.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2016_4734.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2016_4735.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2016_4737.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2016_4738_Account_Changed.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2016_4740_Account_Locked_Out.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2016_4754.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2016_4755.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2016_4756.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2016_4757.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2016_4758.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2016_4764.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2016_4767_Account_Unlocked.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2016_4781_Account_Renamed.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2016_4798.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2016_4799.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2016_4964.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2019_4688_Process_Created.golden.jsonx-pack/winlogbeat/module/security/test/testdata/ingest/security-windows2019_4689_Process_Exited.golden.jsonx-pack/winlogbeat/module/sysmon/test/testdata/ingest/sysmon-10.2-dns.golden.jsonx-pack/winlogbeat/module/sysmon/test/testdata/ingest/sysmon-11-filedelete.golden.jsonx-pack/winlogbeat/module/sysmon/test/testdata/ingest/sysmon-11-filedeletedetected.golden.jsonx-pack/winlogbeat/module/sysmon/test/testdata/ingest/sysmon-11-registry.golden.jsonx-pack/winlogbeat/module/sysmon/test/testdata/ingest/sysmon-12-loadimage.golden.jsonx-pack/winlogbeat/module/sysmon/test/testdata/ingest/sysmon-12-processcreate.golden.jsonx-pack/winlogbeat/module/sysmon/test/testdata/ingest/sysmon-13-clipboardchange.golden.jsonx-pack/winlogbeat/module/sysmon/test/testdata/ingest/sysmon-13-processtampering.golden.jsonx-pack/winlogbeat/module/sysmon/test/testdata/ingest/sysmon-9.01.golden.jsonx-pack/winlogbeat/module/sysmon/test/testdata/ingest/sysmon-no-evtx.golden.jsonx-pack/winlogbeat/module/testing.gox-pack/winlogbeat/module/wintest/docker.gox-pack/winlogbeat/module/wintest/docker_test.gox-pack/winlogbeat/module/wintest/simulate.gox-pack/winlogbeat/module/wintest/simulate_test.go
👮 Files not reviewed due to content moderation or server errors (16)
- x-pack/winlogbeat/module/security/test/testdata/ingest/4706_WindowsSrv2016.golden.json
- x-pack/winlogbeat/module/security/test/testdata/ingest/4707_WindowsSrv2016.golden.json
- x-pack/winlogbeat/module/security/test/testdata/ingest/4713_WindowsSrv2016.golden.json
- x-pack/winlogbeat/module/security/test/testdata/ingest/4716_WindowsSrv2016.golden.json
- x-pack/winlogbeat/module/security/test/testdata/ingest/4717_WindowsSrv2016.golden.json
- x-pack/winlogbeat/module/security/test/testdata/ingest/4718_WindowsSrv2016.golden.json
- x-pack/winlogbeat/module/security/test/testdata/ingest/4719.golden.json
- x-pack/winlogbeat/module/security/test/testdata/ingest/4719_WindowsSrv2016.golden.json
- x-pack/winlogbeat/module/security/test/testdata/ingest/4739_WindowsSrv2016.golden.json
- x-pack/winlogbeat/module/security/test/testdata/ingest/4741.golden.json
- x-pack/winlogbeat/module/security/test/testdata/ingest/4742.golden.json
- x-pack/winlogbeat/module/security/test/testdata/ingest/4743.golden.json
- x-pack/winlogbeat/module/security/test/testdata/ingest/4744.golden.json
- x-pack/winlogbeat/module/security/test/testdata/ingest/4745.golden.json
- x-pack/winlogbeat/module/security/test/testdata/ingest/4746.golden.json
- x-pack/winlogbeat/module/security/test/testdata/ingest/4747.golden.json
| agents: | ||
| provider: "gcp" | ||
| image: "${IMAGE_UBUNTU_X86_64}" | ||
| machineType: "${GCP_DEFAULT_MACHINE_TYPE}" |
There was a problem hiding this comment.
Undefined variable: GCP_DEFAULT_MACHINE_TYPE is not defined in the env section.
This variable is referenced but not declared (lines 4-28 define GCP_WIN_MACHINE_TYPE and GCP_HI_PERF_MACHINE_TYPE but not GCP_DEFAULT_MACHINE_TYPE). The step will fail or use unexpected defaults.
Suggested fix
Add to env section:
GCP_WIN_MACHINE_TYPE: "n2-standard-8"
GCP_HI_PERF_MACHINE_TYPE: "c2d-highcpu-16"
+ GCP_DEFAULT_MACHINE_TYPE: "n2-standard-8"Or use an existing variable:
- machineType: "${GCP_DEFAULT_MACHINE_TYPE}"
+ machineType: "${GCP_WIN_MACHINE_TYPE}"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| machineType: "${GCP_DEFAULT_MACHINE_TYPE}" | |
| machineType: "${GCP_WIN_MACHINE_TYPE}" |
🤖 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 @.buildkite/x-pack/pipeline.xpack.winlogbeat.yml at line 95, The pipeline
references an undeclared env var GCP_DEFAULT_MACHINE_TYPE; fix by declaring
GCP_DEFAULT_MACHINE_TYPE in the env section (set it to the desired machine type)
or update the step to use an existing env var such as GCP_WIN_MACHINE_TYPE or
GCP_HI_PERF_MACHINE_TYPE so the step uses a defined variable; ensure the change
touches the env block and the step that currently uses GCP_DEFAULT_MACHINE_TYPE.
TL;DRThe failing Buildkite step is a security ingest golden mismatch: the pipeline now emits Remediation
Investigation detailsRoot Cause
The security ingest pipeline explicitly adds But the expected fixture for the failing case lacks this field ( Evidence
Verification
Follow-upIf additional Note 🔒 Integrity filter blocked 2 itemsThe following items were blocked because they don't meet the GitHub integrity level.
To allow these resources, lower tools:
github:
min-integrity: approved # merged | approved | unapproved | noneWhat is this? | From workflow: PR Buildkite Detective Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not. |
|
This problem is being taken up in #51913. The PR never received the necessary code owner reviews and is very stale so closing. |
Reviewer note: First commit contains code changes. Second commit is the generated golden file changes.
Proposed commit message
Checklist
stresstest.shscript to run them under stress conditions and race detector to verify their stability../changelog/fragmentsusing the changelog tool.Disruptive User Impact
Author's Checklist
How to test this PR locally
Related issues
Use cases
Screenshots
Logs