x-pack/filebeat : Sanitize cel input resource trace filename #35154

bhapas · 2023-04-20T19:01:01Z

What does this PR do?

This PR sanitizes the request.tracer.filename in filebeat config to replace : and / characters with _ character.

Also contains fixes for httpjson input

Why is it important?

There is a problem with different path separators between HFS+ (colon, ':') and UFS (slash, '/'). This also means that HFS+ file names may contain the slash character and not colons, while the opposite is true for UFS file names. Reference

So the file structure seems to be corrupted in MacOS if the filename has : and the same happens in Unix based systems if the filename has /. Hence replacing them with _.

Checklist

My code follows the style guidelines of this project
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have made corresponding change to the default configuration files
I have added tests that prove my fix is effective or that my feature works
I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

mergify · 2023-04-20T19:01:35Z

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @bhapas? 🙏.
For such, you'll need to label your PR with:

The upcoming major version of the Elastic Stack
The upcoming minor version of the Elastic Stack (if you're not pushing a breaking change)

To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

backport-v8./d.0 is the label to automatically backport to the 8./d branch. /d is the digit

* Also contains fixes for httpjson input

elasticmachine · 2023-04-20T19:58:05Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2023-04-24T15:42:16.569+0000
Duration: 74 min 58 sec

Test stats 🧪

Test	Results
Failed	0
Passed	2888
Skipped	175
Total	3063

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.
/package : Generate the packages and run the E2E tests.
/beats-tester : Run the installation tests with beats-tester.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

mergify · 2023-04-20T23:38:28Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b log-path-cel upstream/log-path-cel
git merge upstream/main
git push upstream log-path-cel

elasticmachine · 2023-04-21T14:49:17Z

Pinging @elastic/security-external-integrations (Team:Security-External Integrations)

mergify · 2023-04-21T19:13:29Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b log-path-cel upstream/log-path-cel
git merge upstream/main
git push upstream log-path-cel

x-pack/filebeat/input/httpjson/input_test.go

x-pack/filebeat/input/httpjson/input.go

x-pack/filebeat/input/cel/input.go

x-pack/filebeat/input/httpjson/input.go

x-pack/filebeat/input/httpjson/input_test.go

x-pack/filebeat/input/cel/input_test.go

x-pack/filebeat/input/httpjson/input_test.go

andrewkroh · 2023-04-24T23:03:42Z

x-pack/filebeat/input/httpjson/input_test.go

@@ -25,12 +26,14 @@ import (
 )

 func TestInput(t *testing.T) {
+	tempDirectory := t.TempDir()


I would rather see each sub-test use its own unique temp dir to ensure test isolation. But with only one test using this directory it's only a minor concern.

I had to change the structure to achieve this. To inject tempDir into the config which is currently a Map I felt difficult with current setup. As you said for one test using it , this should be a big change for the value it gives.

bhapas · 2023-04-25T04:58:54Z

Merging the PR as the golangci-lint errors are unrelated.

* Sanitize cel input resource trace filename * Also contains fixes for httpjson input * Send expected file through config * Fix windows tests (cherry picked from commit bebe337)

…#35201) * Sanitize cel input resource trace filename * Also contains fixes for httpjson input * Send expected file through config * Fix windows tests (cherry picked from commit bebe337) Co-authored-by: Bharat Pasupula <[email protected]>

…35154) (#35201)" This reverts commit 0371f91.

efd6 · 2023-04-26T02:32:36Z

x-pack/filebeat/input/cel/input_test.go

@@ -31,594 +31,655 @@ import (
 	"github.com/elastic/elastic-agent-libs/mapstr"
 )

-var inputTests = []struct {


I didn't notice this (hidden for GitHub folding changes). It should not have been changed; large change with no reason.

Fix #35220

…#35154) * Sanitize cel input resource trace filename * Also contains fixes for httpjson input * Send expected file through config * Fix windows tests

* Sanitize cel input resource trace filename * Also contains fixes for httpjson input * Send expected file through config * Fix windows tests

bhapas self-assigned this Apr 20, 2023

botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Apr 20, 2023

Sanitize cel input resource trace filename

13bae0a

* Also contains fixes for httpjson input

bhapas force-pushed the log-path-cel branch from 9190f8c to 13bae0a Compare April 20, 2023 19:03

bhapas added Team:Security-External Integrations backport-v8.7.0 Automated backport with mergify labels Apr 20, 2023

botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Apr 20, 2023

bhapas added the bugfix label Apr 20, 2023

bhapas mentioned this pull request Apr 20, 2023

x-pack/filebeat : Sanitize trace log filename to support multiple OS #35143

Merged

6 tasks

bhapas and others added 6 commits April 21, 2023 11:52

Send expected file through config

d2625e0

Merge branch 'main' into log-path-cel

484bf3e

Fix windows tests

0eb9be7

Merge branch 'main' into log-path-cel

2d9e2b6

Use os.tempdir

3cd29ad

Merge branch 'main' into log-path-cel

a2bac91

bhapas marked this pull request as ready for review April 21, 2023 14:49

bhapas requested a review from a team as a code owner April 21, 2023 14:49

andrewkroh reviewed Apr 22, 2023

View reviewed changes

bhapas added 3 commits April 22, 2023 09:01

Fix PR comments

9fc769a

Merge remote-tracking branch 'upstream/main' into log-path-cel

9b1e3ff

Fix doc

211f731

bhapas requested a review from andrewkroh April 22, 2023 07:17

efd6 reviewed Apr 22, 2023

View reviewed changes

x-pack/filebeat/input/cel/input.go Outdated Show resolved Hide resolved

x-pack/filebeat/input/httpjson/input.go Show resolved Hide resolved

Fix PR comments and add cel input tests

bc3ee57

bhapas requested a review from efd6 April 22, 2023 14:14

Fix Typo

d0f46e2

andrewkroh reviewed Apr 24, 2023

View reviewed changes

x-pack/filebeat/input/httpjson/input_test.go Outdated Show resolved Hide resolved

Merge branch 'main' into log-path-cel

042c5ad

efd6 reviewed Apr 24, 2023

View reviewed changes

x-pack/filebeat/input/cel/input_test.go Outdated Show resolved Hide resolved

x-pack/filebeat/input/httpjson/input_test.go Outdated Show resolved Hide resolved

bhapas and others added 4 commits April 24, 2023 15:48

Fix PR comments : use t.tempdir()

1253ebd

Merge branch 'main' into log-path-cel

66c702a

Fix PR comments

95174d4

Fix doc

6c40765

bhapas requested a review from andrewkroh April 24, 2023 18:19

andrewkroh approved these changes Apr 24, 2023

View reviewed changes

bhapas merged commit bebe337 into elastic:main Apr 25, 2023

bhapas deleted the log-path-cel branch April 25, 2023 04:59

mergify bot mentioned this pull request Apr 25, 2023

[8.7](backport #35154) x-pack/filebeat : Sanitize cel input resource trace filename #35201

Merged

bhapas added a commit that referenced this pull request Apr 25, 2023

Revert "x-pack/filebeat : Sanitize cel input resource trace filename (#…

057cca1

…35154) (#35201)" This reverts commit 0371f91.

efd6 reviewed Apr 26, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

x-pack/filebeat : Sanitize cel input resource trace filename #35154

x-pack/filebeat : Sanitize cel input resource trace filename #35154

bhapas commented Apr 20, 2023 •

edited

Loading

mergify bot commented Apr 20, 2023

elasticmachine commented Apr 20, 2023 •

edited by jenkins-beats-ci bot

Loading

Build stats

Test stats 🧪

mergify bot commented Apr 20, 2023

elasticmachine commented Apr 21, 2023

mergify bot commented Apr 21, 2023

andrewkroh Apr 24, 2023

bhapas Apr 25, 2023

bhapas commented Apr 25, 2023

efd6 Apr 26, 2023 •

edited

Loading

x-pack/filebeat : Sanitize cel input resource trace filename #35154

x-pack/filebeat : Sanitize cel input resource trace filename #35154

Conversation

bhapas commented Apr 20, 2023 • edited Loading

What does this PR do?

Why is it important?

Checklist

mergify bot commented Apr 20, 2023

elasticmachine commented Apr 20, 2023 • edited by jenkins-beats-ci bot Loading

💚 Build Succeeded

Build stats

Test stats 🧪

💚 Flaky test report

🤖 GitHub comments

mergify bot commented Apr 20, 2023

elasticmachine commented Apr 21, 2023

mergify bot commented Apr 21, 2023

andrewkroh Apr 24, 2023

Choose a reason for hiding this comment

bhapas Apr 25, 2023

Choose a reason for hiding this comment

bhapas commented Apr 25, 2023

efd6 Apr 26, 2023 • edited Loading

Choose a reason for hiding this comment

bhapas commented Apr 20, 2023 •

edited

Loading

elasticmachine commented Apr 20, 2023 •

edited by jenkins-beats-ci bot

Loading

efd6 Apr 26, 2023 •

edited

Loading