[AWS] Add common variables to all SQS-based AWS inputs #9865

kpollich · 2024-05-14T19:36:28Z

Summary

Proposed commit message

Add endpoint, region, and custom common variables to all aws-s3 inputs

Checklist

I have reviewed tips for building integrations and this pull request is aligned with them.
I have verified that all data streams collect metrics or logs.
I have added an entry to my package's changelog.yml file.
I have verified that Kibana version constraints are current according to guidelines.

Author's Checklist

It'd be nice if someone could help me with testing this as I'm not sure how to manually verify

How to test this PR locally

😅

Related issues

FYI this is related to an ongoing escalation related to pulling logs out of an SQS queue.

kpollich · 2024-05-14T19:37:15Z

packages/aws/_dev/benchmark/rally/billing-benchmark/config.yml

Seems like this file just got changed by the linter 🤷

Haiku:
Code repository,
Today, not the time to lint,
Tomorrow, perhaps.

elasticmachine · 2024-05-14T21:04:41Z

🚀 Benchmarks report

To see the full report comment with /test benchmark fullreport

strawgate · 2024-05-14T22:08:44Z

Building on elastic/beats#38189

strawgate · 2024-05-14T22:15:21Z

packages/aws/manifest.yml

+    required: false
+    show_user: false
+    default: ""
+    description: URL of the entry point for an AWS web service


perhaps we use this as an opportunity to make this say that endpoint can also be a domain?

What do you propose for the new description value, in that case?

URL/domain of the entry point for an AWS web service

Is this sufficient?

I'm actually pretty sure I lied and it can't be a domain. I'll verify on Monday

kpollich · 2024-05-16T12:33:21Z

@elastic/obs-ds-hosted-services @elastic/security-service-integrations @elastic/obs-infraobs-integrations - Would you be able to take a look this soon? I'd also appreciate some help testing this change as I don't have an environment setup where I can test the endpoint behavior for these data streams. Are there instructions to do this anywhere that I could reference?

kaiyan-sheng

Thanks for adding this parameter! I didn't realize we are missing this one for the whole time...

kpollich · 2024-05-20T11:58:21Z

FYI I opted to just bump to 2.15.3 rather than doing a prerelease build

strawgate · 2024-05-20T22:00:05Z

edit: everything i typed below is irrelevant, see next post

Looking at this, the endpoint field is required but it's not enough.

It looks like default_region isn't wired up everywhere and that we have inputs that require region to be explicitly set if it cant be parsed from the URL (which it kinda looks like it can never be parsed from the query_url...?)

In the SQS input the region pulling logic is

	if (u.Scheme == "https" || u.Scheme == "http") && u.Host != "" {
		queueHostSplit := strings.SplitN(u.Host, ".", 3)
		if len(queueHostSplit) == 3 {
			if queueHostSplit[2] == endpoint || (endpoint == "" && strings.HasPrefix(queueHostSplit[2], "amazonaws.")) {
				region = queueHostSplit[1]
				if defaultRegion != "" && region != defaultRegion {
					return defaultRegion, regionMismatchError{queueURLRegion: region, defaultRegion: defaultRegion}
				}
				return region, nil
			}
		} else if defaultRegion != "" {
			return defaultRegion, nil
		}
	}
	return "", errBadQueueURL
}

But endpoint always has a scheme and queueHostSplit[2] will never have a scheme so this cannot match.

I believe we will have to expose a "region" field or a "custom_settings" for the SQS inputs in the integration.

it also looks like the getRegionFromQueue expects the defaultRegion param but we actually just pass the region param

		regionName, err := getRegionFromQueueURL(in.config.QueueURL, in.config.AWSConfig.Endpoint, in.config.RegionName)

So based on this we have 4 options as I see it:

Add the endpoint and region field onto each part of the AWS integration with an SQS url and S3 bucket
Add the endpoint and region field onto the top-level settings of the AWS integration and apply it to all policy components
Add a custom settings field onto each part of the AWS integration like we have in the AWS Custom Logs integration
Add a custom settings field onto the top-level settings of the AWS integration and apply it to all policy components

My order of preference is 1,4,2,3 but I think all would solve the customer's problem

strawgate · 2024-05-21T02:44:19Z

I chatted with @faec for a bit and I think endpoint isn't even used for this flow and all we actually need is to either:

Expose a region field on each SQS-S3 based component of the AWS integration
Expose a region field at the integration level
Expose a custom configurations field on each component of the AWS integration like we do for aws custom_logs
Expose a custom configuration field at the integration level

as an aside: It's unclear to me why setting defaultregion doesn't work in this situation. If it works as a default then having defaultregion and region at the integration level seems odd but having defaultregion at the integration level and region at the component level makes some sense.

default_region: Default region to query if no other region is set. Most AWS services offer a regional endpoint that can be used to make requests. Some services, such as IAM, do not support regions. If a region is not provided by any other way (environment variable, credential or instance profile), the value set here will be used.

strawgate · 2024-05-24T18:20:41Z

Ok so we found a bunch of beats issues we're going to fix separately. Kyle and I agreed that we will expose region and endpoint on each aws integration component and also a custom_configuration at the top-level and at the integration component level.

Due to bugs in beats elastic/beats#39709 these will work for s3 polling but not for the sqs queue part until 8.14

agithomas · 2024-05-27T10:14:43Z

@elastic/obs-ds-hosted-services @elastic/security-service-integrations @elastic/obs-infraobs-integrations - Would you be able to take a look this soon? I'd also appreciate some help testing this change as I don't have an environment setup where I can test the endpoint behavior for these data streams. Are there instructions to do this anywhere that I could reference?

@kaiyan-sheng , are there specific instructions that can be followed to test this feature?

efd6 · 2024-05-28T00:50:14Z

packages/aws/data_stream/apigateway_logs/manifest.yml

@@ -107,6 +123,13 @@ streams:
        type: bool
        multi: false
        default: false
+      - name: custom_configurations


This doesn't appear to be being used in this data stream.

Yep this was a WIP commit. I'm still actively working on finalizing this PR. Thanks for taking a look!

@efd6 - These should all be updated and accurate now. Thanks again for reviewing 🙏

efd6 · 2024-05-28T00:50:36Z

packages/aws/data_stream/cloudfront_logs/manifest.yml

@@ -73,3 +89,10 @@ streams:
        default: 5
        required: false
        show_user: false
+      - name: custom_configurations


Same here and below with exception noted.

efd6 · 2024-05-28T00:51:47Z

packages/aws/data_stream/ec2_logs/manifest.yml

@@ -69,6 +85,13 @@ streams:
        default: 5
        required: false
        show_user: false
+      - name: custom_configurations


This one is used.

efd6 · 2024-05-28T00:53:11Z

packages/aws/data_stream/ecs_metrics/agent/stream/stream.yml.hbs

+{{#if endpoint}}
+endpoint: {{endpoint}}
+{{/if}}


I don't see endpoint in the manifest. Also, is region intentionally not added here?

efd6 · 2024-05-28T00:59:59Z

packages/aws/data_stream/vpn/agent/stream/stream.yml.hbs

+{{#if endpoint}}
+endpoint: {{endpoint}}
+{{/if}}


I don't see endpoint in the manifest, and missing region?

kpollich · 2024-05-28T18:04:09Z

In an effort to not balloon the scope of this PR to almost the entire AWS integration, I'm taking the following stretch goals from the description and moving them into a separate issue to put on the backlog:

(Stretch goal) Expose a custom field for all Cloudwatch inputs on all policy templates
(Stretch goal) Expose a custom field at the package level

I've created #10002 to capture this work. The ingest team is okay with shipping these changes only for S3 inputs in an effort to resolve the ongoing escalation more quickly.

efd6

Just noting that the custom configuration field is the sharpest user-facing tool that I've seen.

efd6 · 2024-05-28T23:32:09Z

packages/aws/data_stream/cloudtrail/manifest.yml

@@ -359,3 +382,4 @@ streams:
        type: bool
        multi: false
        default: false
+


Blank line needed? Ah, I see elastic-package check is adding it. Why?

efd6 · 2024-05-28T23:33:24Z

packages/aws/data_stream/guardduty/manifest.yml

+        show_user: false
+        description: >
+          Additional settings to be added to the configuration. Be careful using this as it might break the input as those settings are not validated and can override the settings specified above. See [`aws-s3` input settings docs](https://www.elastic.co/guide/en/beats/filebeat/current/filebeat-input-aws-s3.html) for details.
+


Blank line.

I was able to remove the blank line in the cloudtrail manifest, which I think was just erroneous, but elastic-package check always seems to add a newline after multiline YAML values like we're using in description here.

I also tried forcing the string onto multiple lines with line breaks, and the linter rewrites it to a single line with a blank line afterwards. I'm not sure why this happens. Maybe @jsoriano would know.

efd6 · 2024-05-28T23:33:39Z

packages/aws/data_stream/route53_resolver_logs/manifest.yml

+        show_user: false
+        description: >
+          Additional settings to be added to the configuration. Be careful using this as it might break the input as those settings are not validated and can override the settings specified above. See [`aws-s3` input settings docs](https://www.elastic.co/guide/en/beats/filebeat/current/filebeat-input-aws-s3.html) for details.
+


Blank line.

efd6 · 2024-05-28T23:33:51Z

packages/aws/data_stream/s3access/manifest.yml

+        show_user: false
+        description: >
+          Additional settings to be added to the configuration. Be careful using this as it might break the input as those settings are not validated and can override the settings specified above. See [`aws-s3` input settings docs](https://www.elastic.co/guide/en/beats/filebeat/current/filebeat-input-aws-s3.html) for details.
+


Blank line.

strawgate · 2024-05-29T20:00:14Z

Looking forward to getting this out the door, let me know if it looks like we can't merge by the end of the week!

kaiyan-sheng · 2024-05-29T20:04:19Z

@kaiyan-sheng , are there specific instructions that can be followed to test this feature?

Sorry we don't have any documented instructions for it. Maybe the AWS documentation will help?

kpollich · 2024-05-30T12:00:50Z

This needs a final codeowner review from @elastic/obs-infraobs-integrations.

@strawgate - You mentioned you have an environment where this could be tested. Is that something you can help with before this gets merged?

strawgate · 2024-05-30T12:11:02Z

One way or another I'll get this tested today!

elasticmachine · 2024-05-30T13:39:26Z

💚 Build Succeeded

Buildkite Build
Commit: c5f7f53

History

💚 Build #12000 succeeded 52b0ff7
💚 Build #11838 succeeded 572297b
💚 Build #11830 succeeded 52e0d49
💚 Build #11690 succeeded f3dc2a6
💚 Build #11444 succeeded 9a97542

cc @kpollich

elastic-sonarqube · 2024-05-30T13:39:31Z

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarQube

strawgate · 2024-05-30T13:55:10Z

To test this you setup an S3 bucket, enable Collect logs from S3 bucket, provide the bucket ARN, run the integration and make sure it works (or that you get some sort of ingest pipeline issue and not an agent/filebeat issue):

Then add the region and endpoint fields. If your bucket is in us-east-1 you'd set something like this in the region and endpoint fields.

region: us-east-1
endpoint: https://s3.us-east-1.amazonaws.com

works

Run the integration and make sure it works (or that you get some sort of ingest pipeline issue and not an Agent/Filebeat issue).

Next, set those two settings via the custom configurations block and make sure it still works.

Then to test the failure case, set region and endpoint to something wrong:

default_region: us-east-100
region: us-west-1
endpoint: https://sqs.us-east-1.amazonaws.com

[elastic_agent.filebeat][error] Input 'aws-s3' failed with: failed to initialize s3 poller: failed to get AWS region for bucket: operation error S3: GetBucketLocation, https response error StatusCode: 400, RequestID: ...., api error AuthorizationHeaderMalformed: The authorization header is malformed; the region 'us-east-100' is wrong; expecting 'us-east-1'

Will be fixed in 8.14

region: us-east-100
endpoint: https://sqs.us-east-1.amazonaws.com

[elastic_agent.filebeat][error] Input 'aws-s3' failed with: failed to initialize s3 poller: failed to get AWS region for bucket: operation error S3: GetBucketLocation, https response error StatusCode: 400, RequestID: QNX0GY2KEDD49X0J, HostID: 4BlyLvpZI4e27YByy870Sj9BD/AuTeWDpSJ0CmbXEReTzMt4ge1UJPB59PH1Ix8ZDtZqxVWjYAg=, api error AuthorizationHeaderMalformed: The authorization header is malformed; the region 'us-east-100' is wrong; expecting 'us-east-1'

region: us-east-1
endpoint: https://sqs.us-east-100.amazonaws.com

 Input 'aws-s3' failed with: failed to initialize s3 poller: failed to get AWS region for bucket: operation error S3: GetBucketLocation, https response error StatusCode: 0, RequestID: , HostID: , request send failed, Get "https://billeaston-sqs-bucket.s3.us-east-100.amazonaws.com/?location=": dial tcp: lookup billeaston-sqs-bucket.s3.us-east-100.amazonaws.com on 127.0.0.11:53: no such host

region: us-potato-1
endpoint: https://sqs.us-east-1.amazonaws.com

*no error, but should error, fixed in 8.14+*

region: us-potato-1
endpoint: https://weaston.org

And ensure it stops working.

strawgate · 2024-05-31T12:21:36Z

I performed the testing as described in the above issue. Is there any additional testing anyone wants before this gets merged?

kpollich · 2024-05-31T12:28:58Z

I'm satisfied with the level of testing here now. I just need codeowner review from @elastic/obs-infraobs-integrations. @lalit-satapathy can you help with landing this?

Add endpoint var to AWS integration + data streams

21dbabc

kpollich added the enhancement New feature or request label May 14, 2024

kpollich self-assigned this May 14, 2024

kpollich requested review from a team as code owners May 14, 2024 19:36

Update PR in changelog

9a97542

kpollich commented May 14, 2024

View reviewed changes

strawgate reviewed May 14, 2024

View reviewed changes

kaiyan-sheng approved these changes May 17, 2024

View reviewed changes

Merge branch 'main' into aws/add-endpoint-var

f3dc2a6

Add endpoint + region var to all SQS based policy templates

a881ffa

kpollich changed the title ~~[AWS] Add endpoint var to AWS integration + data streams~~ [AWS] Add common variables to all SQS-based AWS inputs May 23, 2024

kpollich added 4 commits May 23, 2024 11:30

Remove endpoint from non-SQS inputs

8d7bb24

fixup! Remove endpoint from non-SQS inputs

d9ef5eb

fixup! fixup! Remove endpoint from non-SQS inputs

52e0d49

(wip) Add custom_configurations variable

572297b

This was referenced May 24, 2024

[8.14] Fix handling of custom Endpoint when using S3 + SQS elastic/beats#39709

Merged

[main] Fix handling of custom Endpoint when using S3 + SQS elastic/beats#39722

Open

efd6 reviewed May 28, 2024

View reviewed changes

kpollich added 2 commits May 28, 2024 13:25

Add custom configurations to S3 inputs

8e7687e

Remove endpoint from ecs_metrics

31f1d68

kpollich added 4 commits May 28, 2024 13:30

Remove top-level integration custom settings for now

fbd458b

Fix ec2 logs

b30738d

Run elastic-package check

9b4314a

Fix cloudtrail

52b0ff7

kpollich mentioned this pull request May 28, 2024

[AWS] Add custom variable to all Cloudwatch inputs and at the package level #10002

Open

kpollich requested a review from efd6 May 28, 2024 18:04

efd6 approved these changes May 28, 2024

View reviewed changes

Remove blank line from cloudtrail manifest

c5f7f53

[AWS] Add common variables to all SQS-based AWS inputs #9865

Are you sure you want to change the base?

[AWS] Add common variables to all SQS-based AWS inputs #9865

Conversation

kpollich commented May 14, 2024 • edited

Summary

Proposed commit message

Checklist

Author's Checklist

How to test this PR locally

Related issues

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented May 14, 2024 • edited

🚀 Benchmarks report

strawgate commented May 14, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kpollich commented May 16, 2024

kaiyan-sheng left a comment

Choose a reason for hiding this comment

kpollich commented May 20, 2024

strawgate commented May 20, 2024 • edited

strawgate commented May 21, 2024 • edited

strawgate commented May 24, 2024

agithomas commented May 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kpollich commented May 28, 2024

efd6 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

strawgate commented May 29, 2024

kaiyan-sheng commented May 29, 2024

kpollich commented May 30, 2024

strawgate commented May 30, 2024

elasticmachine commented May 30, 2024

💚 Build Succeeded

History

elastic-sonarqube bot commented May 30, 2024

Quality Gate passed

strawgate commented May 30, 2024 • edited

strawgate commented May 31, 2024

kpollich commented May 31, 2024

kpollich commented May 14, 2024 •

edited

elasticmachine commented May 14, 2024 •

edited

strawgate commented May 20, 2024 •

edited

strawgate commented May 21, 2024 •

edited

strawgate commented May 30, 2024 •

edited