x-pack/filebeat/input/awss3: allow cross-region bucket configuration by efd6 · Pull Request #40309 · elastic/beats

efd6 · 2024-07-23T05:01:22Z

Proposed commit message

See title.

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.

Disruptive User Impact

Author's Checklist

[ ]

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

elasticmachine · 2024-07-23T07:47:34Z

Pinging @elastic/security-service-integrations (Team:Security-Service Integrations)

efd6 · 2024-07-23T23:44:43Z

x-pack/filebeat/input/awss3/interfaces.go

+	opts := a.client.Options()
+	if opts.Region == region {
+		return a.client
+	}
+	opts.Region = region
+	return s3.New(opts)


Depending on the frequency of the new-client path, this could generate a reasonable amount of work. Something that I considered but did not add is having awsS3API hold a map[string]*s3.Client that can cache (non-retiring) the clients for each region. Given that there are not large numbers of regions, it should be reasonable to retain clients that are constructed dynamically for the life of the awsS3API. WDYT?

I think this is a good idea. My intuition is that in the use-cases where this is needed that it can be in very high volumes (big orgs, lots of aws projects, lots of regions, all notifications centralized to one queue).

I've added caching and limited the size to 100. We should never hit this limit, but I wanted to make sure that we don't put ourselves in the situation that this results in an OoM crash.

mergify · 2024-07-23T23:45:20Z

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 22161-awss3 upstream/22161-awss3
git merge upstream/main
git push upstream 22161-awss3

andrewkroh · 2024-07-24T13:30:13Z

I added the backport label thinking that this can try to target v8.15.1.

andrewkroh

LGTM.

…40309) (cherry picked from commit e177fc5)

kaiyan-sheng · 2024-07-29T13:49:27Z

@efd6 Should we create a separate PR to update the documentation too? Thanks!

efd6 · 2024-07-29T21:30:17Z

@kaiyan-sheng I had not intended to include documentation since this feature needs no user understanding.

…on bucket configuration (#40371) * x-pack/filebeat/input/awss3: allow cross-region bucket configuration (#40309) (cherry picked from commit e177fc5) * remove irrelevant changelog entries --------- Co-authored-by: Dan Kortschak <dan.kortschak@elastic.co>

When a region is not specified in the SQS notification then reuse the existing S3 client instead of creating a new one based on an empty (unspecified) AWS region name. This problem affected custom SQS notification formats that did not specify a region name (like Crowdstrike FDR notifications). Fixes: elastic/integrations/elastic#10647 Relates: elastic#40309

When a region is not specified in the SQS notification then reuse the existing S3 client instead of creating a new one based on an empty (unspecified) AWS region name. This problem affected custom SQS notification formats that did not specify a region name (like Crowdstrike FDR notifications). This addresses errors like: S3 download failure: s3 GetObject failed: operation error S3: GetObject, resolve auth scheme: resolve endpoint: endpoint rule error, Invalid region: region was not a valid DNS name. Fixes: elastic/integrations/elastic#10647 Relates: elastic#40309

…#40628) When a region is not specified in the SQS notification then reuse the existing S3 client instead of creating a new one based on an empty (unspecified) AWS region name. This problem affected custom SQS notification formats that did not specify a region name (like Crowdstrike FDR notifications). This addresses errors like: S3 download failure: s3 GetObject failed: operation error S3: GetObject, resolve auth scheme: resolve endpoint: endpoint rule error, Invalid region: region was not a valid DNS name. Fixes: elastic/integrations/#10647 Relates: #40309

…#40628) When a region is not specified in the SQS notification then reuse the existing S3 client instead of creating a new one based on an empty (unspecified) AWS region name. This problem affected custom SQS notification formats that did not specify a region name (like Crowdstrike FDR notifications). This addresses errors like: S3 download failure: s3 GetObject failed: operation error S3: GetObject, resolve auth scheme: resolve endpoint: endpoint rule error, Invalid region: region was not a valid DNS name. Fixes: elastic/integrations/#10647 Relates: #40309 (cherry picked from commit 6d25d46)

…#40628) (#40632) When a region is not specified in the SQS notification then reuse the existing S3 client instead of creating a new one based on an empty (unspecified) AWS region name. This problem affected custom SQS notification formats that did not specify a region name (like Crowdstrike FDR notifications). This addresses errors like: S3 download failure: s3 GetObject failed: operation error S3: GetObject, resolve auth scheme: resolve endpoint: endpoint rule error, Invalid region: region was not a valid DNS name. Fixes: elastic/integrations/#10647 Relates: #40309 (cherry picked from commit 6d25d46) Co-authored-by: Andrew Kroh <andrew.kroh@elastic.co>

efd6 added Filebeat Filebeat backport-skip Skip notification from the automated backport with mergify Team:Security-Service Integrations Security Service Integrations Team labels Jul 23, 2024

efd6 self-assigned this Jul 23, 2024

botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Jul 23, 2024

efd6 force-pushed the 22161-awss3 branch from d998a42 to 927a67a Compare July 23, 2024 05:02

efd6 marked this pull request as ready for review July 23, 2024 07:47

efd6 requested a review from a team as a code owner July 23, 2024 07:47

narph requested a review from andrewkroh July 23, 2024 08:33

efd6 commented Jul 23, 2024

View reviewed changes

x-pack/filebeat/input/awss3: allow cross-region bucket configuration

986d9fa

efd6 force-pushed the 22161-awss3 branch from 927a67a to 986d9fa Compare July 24, 2024 00:10

andrewkroh added bug bugfix backport-8.15 Automated backport to the 8.15 branch with mergify and removed bug labels Jul 24, 2024

efd6 force-pushed the 22161-awss3 branch from 875742b to 9e2b4e1 Compare July 24, 2024 23:19

cache seen connections

d5a8298

efd6 force-pushed the 22161-awss3 branch from 9e2b4e1 to d5a8298 Compare July 25, 2024 00:06

efd6 removed the backport-skip Skip notification from the automated backport with mergify label Jul 25, 2024

andrewkroh approved these changes Jul 25, 2024

View reviewed changes

fix grammar

f9fc552

kaiyan-sheng approved these changes Jul 26, 2024

View reviewed changes

efd6 merged commit e177fc5 into elastic:main Jul 28, 2024

mergify bot pushed a commit that referenced this pull request Jul 28, 2024

x-pack/filebeat/input/awss3: allow cross-region bucket configuration (#…

d7224b7

…40309) (cherry picked from commit e177fc5)

mergify bot mentioned this pull request Jul 28, 2024

[8.15](backport #40309) x-pack/filebeat/input/awss3: allow cross-region bucket configuration #40371

Merged

6 tasks

andrewkroh mentioned this pull request Aug 23, 2024

[Stack 8.16.0-SNAPSHOT] [crowdstrike] Failing test daily: system test: default in crowdstrike.fdr elastic/integrations#10647

Closed

andrewkroh mentioned this pull request Aug 27, 2024

x-pack/filebeat/inputs/awss3 - Handle SQS notification without region #40628

Merged

6 tasks

mergify bot mentioned this pull request Aug 27, 2024

[8.15](backport #40628) x-pack/filebeat/inputs/awss3 - Handle SQS notification without region #40632

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

x-pack/filebeat/input/awss3: allow cross-region bucket configuration#40309

x-pack/filebeat/input/awss3: allow cross-region bucket configuration#40309
efd6 merged 3 commits intoelastic:mainfrom
efd6:22161-awss3

efd6 commented Jul 23, 2024

Uh oh!

elasticmachine commented Jul 23, 2024

Uh oh!

efd6 Jul 23, 2024

Uh oh!

andrewkroh Jul 24, 2024

Uh oh!

efd6 Jul 24, 2024

Uh oh!

mergify bot commented Jul 23, 2024

Uh oh!

andrewkroh commented Jul 24, 2024

Uh oh!

andrewkroh left a comment

Uh oh!

kaiyan-sheng commented Jul 29, 2024

Uh oh!

efd6 commented Jul 29, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

efd6 commented Jul 23, 2024

Proposed commit message

Checklist

Disruptive User Impact

Author's Checklist

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

Uh oh!

elasticmachine commented Jul 23, 2024

Uh oh!

efd6 Jul 23, 2024

Choose a reason for hiding this comment

Uh oh!

andrewkroh Jul 24, 2024

Choose a reason for hiding this comment

Uh oh!

efd6 Jul 24, 2024

Choose a reason for hiding this comment

Uh oh!

mergify bot commented Jul 23, 2024

Uh oh!

andrewkroh commented Jul 24, 2024

Uh oh!

andrewkroh left a comment

Choose a reason for hiding this comment

Uh oh!

kaiyan-sheng commented Jul 29, 2024

Uh oh!

efd6 commented Jul 29, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants