[AWS] [S3] Remove url.QueryUnescape() from aws-s3 input in polling mode #38125

zmoog · 2024-02-23T15:43:10Z

Proposed commit message

Remove url.QueryUnescape() from aws-s3 input in polling mode because it is not required.

We introduced the url.QueryUnescape() function to unescape object keys from S3 notifications in SQS messages when the input runs in SQS. We also used it for object keys from the S3 list objects when the input runs in polling mode. However, the object keys in the S3 list object responses (polling mode) do not require unescape.

We must remove the unescape to avoid unintended changes to the S3 object key.

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.

How to test this PR locally

go test -v ./x-pack/filebeat/input/awss3/... -run TestS3Poller

Related issues

Closes [awss3 input] unescape characters in s3 file names #38012

mergify · 2024-02-23T15:43:51Z

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @zmoog? 🙏.
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

zmoog · 2024-02-23T16:32:19Z

Reopening (closed accidentally)

mergify · 2024-02-23T16:33:47Z

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @zmoog? 🙏.
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

zmoog · 2024-02-23T16:46:08Z

The "x-pack/libbeat: Load dynamic pipeline's steps" is failing, but I don't think it's related to the changes in this PR.

mergify · 2024-02-23T19:43:00Z

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 zmoog/remove-unescape-from-aws-s3 upstream/zmoog/remove-unescape-from-aws-s3
git merge upstream/main
git push upstream zmoog/remove-unescape-from-aws-s3

We introduced [^1] the `url.QueryUnescape()` function to unescape object keys from S3 notification in SQS messages. However, the object keys in the S3 list object responses do not require [^2] unescape. We must remove the unescape to avoid unintended changes to the S3 object key. [^1]: elastic#18370 [^2]: elastic#38012 (comment)

elasticmachine · 2024-03-04T10:33:05Z

💚 Build Succeeded

Buildkite Build
Commit: c148243

History

💚 Build #2957 succeeded f303b7a
💚 Build #2842 succeeded 62e9992
💚 Build #2840 succeeded db704dc
💚 Build #2839 succeeded b5146ca
💚 Build #2836 succeeded a11ed6e
💚 Build #2835 succeeded a4323ab

cc @zmoog

elasticmachine · 2024-03-04T10:33:07Z

💚 Build Succeeded

Buildkite Build
Commit: c148243

History

💚 Build #1750 succeeded f303b7a
💚 Build #1635 succeeded 62e9992
💚 Build #1633 succeeded db704dc
💚 Build #1632 succeeded b5146ca
💚 Build #1629 succeeded a11ed6e
💚 Build #1628 succeeded a4323ab

cc @zmoog

elasticmachine · 2024-03-04T10:33:08Z

💚 Build Succeeded

Buildkite Build
Commit: c148243

History

💚 Build #1743 succeeded f303b7a
💚 Build #1627 succeeded 62e9992
💚 Build #1625 succeeded db704dc
💚 Build #1624 succeeded b5146ca
💚 Build #1621 succeeded a11ed6e
💚 Build #1620 succeeded a4323ab

cc @zmoog

elasticmachine · 2024-03-04T10:33:25Z

💚 Build Succeeded

Buildkite Build
Commit: c148243

History

💚 Build #906 succeeded f303b7a
💚 Build #791 succeeded 62e9992
💚 Build #789 succeeded db704dc
💚 Build #788 succeeded b5146ca
💚 Build #785 succeeded a11ed6e
💚 Build #784 succeeded a4323ab

cc @zmoog

elasticmachine · 2024-03-04T10:36:30Z

💚 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: 2024-03-04T10:30:42.687+0000
Duration: 134 min 29 sec

Test stats 🧪

Test	Results
Failed	0
Passed	3320
Skipped	180
Total	3500

💚 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!)

elasticmachine · 2024-03-04T10:50:32Z

💚 Build Succeeded

Buildkite Build
Commit: c148243

History

💚 Build #2039 succeeded f303b7a
💚 Build #1924 succeeded 62e9992
💚 Build #1918 succeeded a11ed6e

cc @zmoog

elasticmachine · 2024-03-04T11:12:26Z

💚 Build Succeeded

Buildkite Build
Commit: c148243

History

💔 Build #3263 failed 62e9992

cc @zmoog

…de (#38125) * Remove url.QueryUnescape() We introduced [^1] the `url.QueryUnescape()` function to unescape object keys from S3 notification in SQS messages. However, the object keys in the S3 list object responses do not require [^2] unescape. We must remove the unescape to avoid unintended changes to the S3 object key. [^1]: #18370 [^2]: #38012 (comment) --------- Co-authored-by: Andrea Spacca <andrea.spacca@elastic.co> (cherry picked from commit 5f1e656)

…s-s3 input in polling mode (#38165) * [AWS] [S3] Remove url.QueryUnescape() from aws-s3 input in polling mode (#38125) We introduced [^1] the `url.QueryUnescape()` function to unescape object keys from S3 notification in SQS messages. However, the object keys in the S3 list object responses do not require [^2] unescape. We must remove the unescape to avoid unintended changes to the S3 object key. [^1]: #18370 [^2]: #38012 (comment) --------- Co-authored-by: Andrea Spacca <andrea.spacca@elastic.co>

botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Feb 23, 2024

zmoog changed the title ~~[AWS] [S3] Remove url.QueryUnescape() from aws-s3 input (polling mode)~~ [AWS] [S3] Remove url.QueryUnescape() from aws-s3 input in polling mode Feb 23, 2024

mergify bot assigned zmoog Feb 23, 2024

zmoog added the Team:Cloud-Monitoring Label for the Cloud Monitoring team label Feb 23, 2024

botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Feb 23, 2024

zmoog added bug needs_team Indicates that the issue/PR needs a Team:* label labels Feb 23, 2024

botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Feb 23, 2024

zmoog force-pushed the zmoog/remove-unescape-from-aws-s3 branch from a4323ab to a11ed6e Compare February 23, 2024 15:49

zmoog closed this Feb 23, 2024

zmoog force-pushed the zmoog/remove-unescape-from-aws-s3 branch from b5146ca to a27e439 Compare February 23, 2024 16:25

zmoog reopened this Feb 23, 2024

zmoog marked this pull request as ready for review February 23, 2024 16:46

zmoog requested a review from a team as a code owner February 23, 2024 16:46

aspacca approved these changes Feb 26, 2024

View reviewed changes

zmoog added 2 commits March 4, 2024 11:01

Update changelog

f0e2b8f

zmoog force-pushed the zmoog/remove-unescape-from-aws-s3 branch from 62e9992 to f303b7a Compare March 4, 2024 10:01

zmoog added the backport-v8.13.0 Automated backport with mergify label Mar 4, 2024

Merge branch 'main' into zmoog/remove-unescape-from-aws-s3

c148243

zmoog enabled auto-merge (squash) March 4, 2024 11:24

zmoog merged commit 5f1e656 into elastic:main Mar 4, 2024
38 of 39 checks passed

mergify bot mentioned this pull request Mar 4, 2024

[8.13](backport #38125) [AWS] [S3] Remove url.QueryUnescape() from aws-s3 input in polling mode #38165

Merged

zmoog deleted the zmoog/remove-unescape-from-aws-s3 branch March 4, 2024 15:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[AWS] [S3] Remove url.QueryUnescape() from aws-s3 input in polling mode #38125

[AWS] [S3] Remove url.QueryUnescape() from aws-s3 input in polling mode #38125

zmoog commented Feb 23, 2024 •

edited

Loading

mergify bot commented Feb 23, 2024

zmoog commented Feb 23, 2024 •

edited

Loading

mergify bot commented Feb 23, 2024

zmoog commented Feb 23, 2024

mergify bot commented Feb 23, 2024

elasticmachine commented Mar 4, 2024

elasticmachine commented Mar 4, 2024

elasticmachine commented Mar 4, 2024

elasticmachine commented Mar 4, 2024

elasticmachine commented Mar 4, 2024 •

edited

Loading

Build stats

Test stats 🧪

elasticmachine commented Mar 4, 2024

elasticmachine commented Mar 4, 2024

[AWS] [S3] Remove url.QueryUnescape() from aws-s3 input in polling mode #38125

[AWS] [S3] Remove url.QueryUnescape() from aws-s3 input in polling mode #38125

Conversation

zmoog commented Feb 23, 2024 • edited Loading

Proposed commit message

Checklist

How to test this PR locally

Related issues

mergify bot commented Feb 23, 2024

zmoog commented Feb 23, 2024 • edited Loading

mergify bot commented Feb 23, 2024

zmoog commented Feb 23, 2024

mergify bot commented Feb 23, 2024

elasticmachine commented Mar 4, 2024

💚 Build Succeeded

History

elasticmachine commented Mar 4, 2024

💚 Build Succeeded

History

elasticmachine commented Mar 4, 2024

💚 Build Succeeded

History

elasticmachine commented Mar 4, 2024

💚 Build Succeeded

History

elasticmachine commented Mar 4, 2024 • edited Loading

💚 Build Succeeded

Build stats

Test stats 🧪

💚 Flaky test report

🤖 GitHub comments

elasticmachine commented Mar 4, 2024

💚 Build Succeeded

History

elasticmachine commented Mar 4, 2024

💚 Build Succeeded

History

zmoog commented Feb 23, 2024 •

edited

Loading

zmoog commented Feb 23, 2024 •

edited

Loading

elasticmachine commented Mar 4, 2024 •

edited

Loading