[Processor: add_cloud_metadata] Use AWS client to get instance metadata and EKS cluster name #35182

tetianakravchenko · 2023-04-24T08:01:41Z

What does this PR do?

Use AWS sdk instead of http calls
Get Identity document to set the same metadata as before
use EC2 describe tags call to get instance tags, that contains eks:cluster-name tag

NOTE: existing issue:

AWS and Openstack have compatible APIs, so in some cases (if explicitly not defined providers: ["aws"]) aws environment is detected as an openstack - it is an existing issue, due to the current implementation how the cloud provider is defined now
related issues:
add_cloud_metadata detecting wrong cloud provider (aws as openstack) #13816
Wrong detection of cloud.provider since Beats 8.0 (openstack as huawei) #31022

Why is it important?

Add cluster name metadata for EKS clusters, similar to the GKE

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.

Author's Checklist

[ ]

How to test this PR locally

Related issues

Use cases

Screenshots

when used configuration:

processors:
      - add_cloud_metadata:
          providers:
            - "aws"

Logs

Signed-off-by: Tetiana Kravchenko <[email protected]>

mergify · 2023-04-24T08:02:18Z

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

Signed-off-by: Tetiana Kravchenko <[email protected]>

elasticmachine · 2023-04-24T08:44:21Z

💚 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-05-31T07:26:24.750+0000
Duration: 46 min 54 sec

❕ Flaky test report

No test was executed to be analysed.

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

Signed-off-by: Tetiana Kravchenko <[email protected]>

gizas · 2023-04-27T09:03:55Z

libbeat/processors/add_cloud_metadata/provider_aws_ec2.go

-		logger.Warnf("error when read token request for getting IMDSv2 token: %s. No token in the metadata request will be used.", err)
-		return ""
+		logger.Debugf("error loading AWS default configuration: %s.", err)
+		result.err = errors.Wrapf(err, "failed loading AWS default configuration")


Suggested change

result.err = errors.Wrapf(err, "failed loading AWS default configuration")

result.err = fmt.Errorf("%w failed loading AWS default configuration", error)

gizas · 2023-04-27T09:06:04Z

libbeat/processors/add_cloud_metadata/provider_aws_ec2.go

+	"github.com/aws/aws-sdk-go-v2/feature/ec2/imds"
+	"github.com/aws/aws-sdk-go-v2/service/ec2"
+	"github.com/aws/aws-sdk-go-v2/service/ec2/types"
+	"github.com/pkg/errors"


Maybe just import fmt

done - 0241f22

gizas · 2023-04-27T09:06:37Z

libbeat/processors/add_cloud_metadata/provider_aws_ec2.go

+	instanceIdentity, err := awsClient.GetInstanceIdentityDocument(context.TODO(), &imds.GetInstanceIdentityDocumentInput{})
+	if err != nil {
+		logger.Debugf("error fetching EC2 Identity Document: %s.", err)
+		result.err = errors.Wrapf(err, "failed fetching EC2 Identity Document.")


Same as above with fmt.errorf

gizas · 2023-04-27T09:08:12Z

libbeat/processors/add_cloud_metadata/provider_aws_ec2.go

 	if err != nil {
-		logger.Warnf("error when reading token request for getting IMDSv2 token: %s. No token in the metadata request will be used.", err)
-		return ""
+		logger.Debugf("error fetching cluster name metadata: %s.", err)


This can be Warnf level and not debug?

@tetianakravchenko only minor if you think that we need to expose this to warnings or to errors , as it is quite important to print this by default, or repeat what you do in line

beats/libbeat/processors/add_cloud_metadata/provider_aws_ec2.go

Line 93 in 98b5cb5

result.err = errors.Wrapf(err, "failed fetching EC2 Identity Document.")

@gizas thank you for pointing it out! From my understanding it should be fine to use it this way, http fetcher uses this definition as well -

beats/libbeat/processors/add_cloud_metadata/http_fetcher.go

Line 94 in 98b5cb5

result.err = errors.Wrapf(err, "failed to create http request for %v", f.provider)

My comment has to do more with the level error vs debug vs warn. I would say not to use debug as it would hide any messages unless you raise the log level

this code is outdated - was replaced to Warn in 0241f22

Signed-off-by: Tetiana Kravchenko <[email protected]>

mergify · 2023-05-15T12:02:49Z

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 eks-cluster-name upstream/eks-cluster-name
git merge upstream/main
git push upstream eks-cluster-name

tetianakravchenko · 2023-05-15T14:31:37Z

hey @kaiyan-sheng as I pushed some changes after your approve, could you please have a look/approve again?

tetianakravchenko · 2023-05-15T18:26:29Z

@pierrehilbert @fearful-symmetry could you please review this PR and help with merging it?

I can't merge it, since I am missing the code owners review

tetianakravchenko · 2023-05-17T07:26:40Z

Hey @elastic/elastic-agent-data-plane, could you help with this PR? I can't merge it because Waiting on code owner review from elastic/elastic-agent-data-plane. #35182 (comment)

MichaelKatsoulis · 2023-05-17T09:33:53Z

@tetianakravchenko have you tested that on AWS Fargate ?

tetianakravchenko · 2023-05-17T11:02:07Z

@tetianakravchenko have you tested that on AWS Fargate ?

@MichaelKatsoulis no, only on the standard EKS setup. Do you think it is a blocker for this PR?

MichaelKatsoulis · 2023-05-17T12:55:02Z

@MichaelKatsoulis no, only on the standard EKS setup. Do you think it is a blocker for this PR?

Not a blocker, I was just interested on what happens in that case. I would expect to not get the cluster name.
It could be worthwhile testing to ensure that it doesn't cause anything undesired.

fearful-symmetry · 2023-05-26T16:39:56Z

libbeat/processors/add_cloud_metadata/provider_aws_ec2.go

 	if err != nil {
-		logger.Warnf("error when read token request for getting IMDSv2 token: %s. No token in the metadata request will be used.", err)
-		return ""
+		logger.Debugf("error loading AWS default configuration: %s.", err)


This is an error message, is there a reason the log level is debug? Should be Errorf.

add_cloud_metadata tries to fetch the metadata from all the available cloud providers, so if running on GCP - fetchMetadata for AWS will still run and in this case it might will be confusing to get errors on GCP regarding the AWS configuration. I've changed it to Warnf as it was done before - 19ab154

mergify · 2023-05-26T16:40:54Z

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 eks-cluster-name upstream/eks-cluster-name
git merge upstream/main
git push upstream eks-cluster-name

fearful-symmetry · 2023-05-26T16:40:59Z

Not sure why github set me as the code owner here, since I don't have a lot of context for the cloud processors. Tried to give a bit of feedback.

tetianakravchenko · 2023-05-30T10:38:51Z

Not sure why github set me as the code owner here, since I don't have a lot of context for the cloud processors. Tried to give a bit of feedback.

@fearful-symmetry I believe it is mainly due to adding new dependency

Signed-off-by: Tetiana Kravchenko <[email protected]>

mergify · 2023-05-30T14:12:14Z

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 eks-cluster-name upstream/eks-cluster-name
git merge upstream/main
git push upstream eks-cluster-name

tetianakravchenko · 2023-05-31T07:26:13Z

/test

…ta and EKS cluster name (#35182) * add generic metadata fetcher Signed-off-by: Tetiana Kravchenko <[email protected]> * merge main Signed-off-by: Tetiana Kravchenko <[email protected]> * clean up Signed-off-by: Tetiana Kravchenko <[email protected]> * move tagDescribe to different func Signed-off-by: Tetiana Kravchenko <[email protected]> * add tests for add_cloud_metadata Signed-off-by: Tetiana Kravchenko <[email protected]> * Tiltfile: fix docker_registry, use more generic value Signed-off-by: Tetiana Kravchenko <[email protected]> * add notice file Signed-off-by: Tetiana Kravchenko <[email protected]> * fix tests - add former test cases; fix linter issues Signed-off-by: Tetiana Kravchenko <[email protected]> * handle correctly result.err Signed-off-by: Tetiana Kravchenko <[email protected]> * add generic metadata fetcher Signed-off-by: Tetiana Kravchenko <[email protected]> * merge main Signed-off-by: Tetiana Kravchenko <[email protected]> * clean up Signed-off-by: Tetiana Kravchenko <[email protected]> * move tagDescribe to different func Signed-off-by: Tetiana Kravchenko <[email protected]> * add tests for add_cloud_metadata Signed-off-by: Tetiana Kravchenko <[email protected]> * Tiltfile: fix docker_registry, use more generic value Signed-off-by: Tetiana Kravchenko <[email protected]> * add notice file Signed-off-by: Tetiana Kravchenko <[email protected]> * fix tests - add former test cases; fix linter issues Signed-off-by: Tetiana Kravchenko <[email protected]> * handle correctly result.err Signed-off-by: Tetiana Kravchenko <[email protected]> * address reviews Signed-off-by: Tetiana Kravchenko <[email protected]> * Update dev-tools/kubernetes/Tiltfile Co-authored-by: kaiyan-sheng <[email protected]> * fix the types.TagDescription struct Signed-off-by: Tetiana Kravchenko <[email protected]> * remove not used variable; fix types.TagDescription struct Signed-off-by: Tetiana Kravchenko <[email protected]> * add a changelog record Signed-off-by: Tetiana Kravchenko <[email protected]> * change Debugf to Warnf Signed-off-by: Tetiana Kravchenko <[email protected]> --------- Signed-off-by: Tetiana Kravchenko <[email protected]> Co-authored-by: kaiyan-sheng <[email protected]>

DingGGu · 2023-07-26T02:39:36Z

This makes fetch IMDS Error on non EKS envs

{"log.level":"warn","@timestamp":"2023-07-26T02:20:11.324Z","log.logger":"add_cloud_metadata","log.origin":{"file.name":"add_cloud_metadata/provider_aws_ec2.go","file.line":102},"message":"error fetching cluster name metadata: error fetching EC2 Tags: operation error EC2: DescribeTags, failed to sign request: failed to retrieve credentials: failed to refresh cached credentials, failed to get nodes EC2 IMDS role credentials, operation error ec2imds: GetMetadata, request canceled, context deadline exceeded.","service.name":"filebeat","ecs.version":"1.6.0"}
{"log.level":"warn","@timestamp":"2023-07-26T02:20:16.470Z","log.logger":"add_cloud_metadata","log.origin":{"file.name":"add_cloud_metadata/provider_aws_ec2.go","file.line":102},"message":"error fetching cluster name metadata: error fetching EC2 Tags: operation error EC2: DescribeTags, exceeded maximum number of attempts, 3, failed to sign request: failed to retrieve credentials: failed to refresh cached credentials, failed to get nodes EC2 IMDS role credentials, operation error ec2imds: GetMetadata, exceeded maximum number of attempts, 3, http response error StatusCode: 500, request to EC2 IMDS failed.","service.name":"filebeat","ecs.version":"1.6.0"}

tetianakravchenko added 8 commits January 23, 2023 10:38

add generic metadata fetcher

8d26505

Signed-off-by: Tetiana Kravchenko <[email protected]>

Merge branch 'main' into eks-cluster-name

3da87ca

Merge branch 'main' into eks-cluster-name

39a690b

merge main

481f65c

Signed-off-by: Tetiana Kravchenko <[email protected]>

Merge branch 'main' into eks-cluster-name

4886d96

clean up

76a4da4

Signed-off-by: Tetiana Kravchenko <[email protected]>

move tagDescribe to different func

d659712

Signed-off-by: Tetiana Kravchenko <[email protected]>

add tests for add_cloud_metadata

737f3d3

Signed-off-by: Tetiana Kravchenko <[email protected]>

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

mergify bot assigned tetianakravchenko Apr 24, 2023

tetianakravchenko added 2 commits April 24, 2023 10:06

Tiltfile: fix docker_registry, use more generic value

9255c71

Signed-off-by: Tetiana Kravchenko <[email protected]>

add notice file

db67e1b

Signed-off-by: Tetiana Kravchenko <[email protected]>

tetianakravchenko added 2 commits April 26, 2023 15:36

fix tests - add former test cases; fix linter issues

bb5e429

Signed-off-by: Tetiana Kravchenko <[email protected]>

handle correctly result.err

98b5cb5

Signed-off-by: Tetiana Kravchenko <[email protected]>

tetianakravchenko requested review from a team, gizas and ChrsMark and removed request for a team April 27, 2023 08:23

gizas reviewed Apr 27, 2023

View reviewed changes

tetianakravchenko added 6 commits April 27, 2023 17:26

add generic metadata fetcher

fb0293e

Signed-off-by: Tetiana Kravchenko <[email protected]>

merge main

5b5e54d

Signed-off-by: Tetiana Kravchenko <[email protected]>

clean up

1769926

Signed-off-by: Tetiana Kravchenko <[email protected]>

move tagDescribe to different func

64439f3

Signed-off-by: Tetiana Kravchenko <[email protected]>

add tests for add_cloud_metadata

bbd928c

Signed-off-by: Tetiana Kravchenko <[email protected]>

Tiltfile: fix docker_registry, use more generic value

b193f5c

Signed-off-by: Tetiana Kravchenko <[email protected]>

add a changelog record

f4ae2cf

Signed-off-by: Tetiana Kravchenko <[email protected]>

tetianakravchenko dismissed gizas’s stale review via f4ae2cf May 15, 2023 12:02

tetianakravchenko requested a review from a team as a code owner May 15, 2023 12:02

Merge branch 'main' into eks-cluster-name

979778b

kaiyan-sheng approved these changes May 15, 2023

View reviewed changes

pierrehilbert added the Team:Elastic-Agent Label for the Agent team label May 19, 2023

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

fearful-symmetry reviewed May 26, 2023

View reviewed changes

tetianakravchenko added 2 commits May 30, 2023 12:43

change Debugf to Warnf

19ab154

Signed-off-by: Tetiana Kravchenko <[email protected]>

merge main; fix conflicts

b9ed1e8

Signed-off-by: Tetiana Kravchenko <[email protected]>

fearful-symmetry approved these changes May 30, 2023

View reviewed changes

Merge branch 'main' into eks-cluster-name

29c4e2e

tetianakravchenko merged commit c40fa0a into elastic:main May 31, 2023

gizas mentioned this pull request Dec 22, 2023

Investigate if it is possible to set orchestrator fields from Cloud provider kubernetes metadata #33081

Closed

tetianakravchenko mentioned this pull request Jan 9, 2024

[add_cloud_metadata] For the AWS cloud provider orchestrator.cluster shouldn't be stored under the cloud.* object #37583

Closed

tetianakravchenko mentioned this pull request Jan 23, 2024

[add_cloud_metadata] fix the orchestrator metadata for the aws cloud provider #37651

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Processor: add_cloud_metadata] Use AWS client to get instance metadata and EKS cluster name #35182

[Processor: add_cloud_metadata] Use AWS client to get instance metadata and EKS cluster name #35182

tetianakravchenko commented Apr 24, 2023 •

edited

Loading

mergify bot commented Apr 24, 2023

elasticmachine commented Apr 24, 2023 •

edited by jenkins-beats-ci bot

Loading

Build stats

gizas Apr 27, 2023

gizas Apr 27, 2023

tetianakravchenko May 2, 2023

gizas Apr 27, 2023

gizas Apr 27, 2023 •

edited

Loading

gizas May 9, 2023 •

edited

Loading

tetianakravchenko May 14, 2023

gizas May 29, 2023

tetianakravchenko May 30, 2023

mergify bot commented May 15, 2023

tetianakravchenko commented May 15, 2023

tetianakravchenko commented May 15, 2023

tetianakravchenko commented May 17, 2023

MichaelKatsoulis commented May 17, 2023

tetianakravchenko commented May 17, 2023

MichaelKatsoulis commented May 17, 2023

fearful-symmetry May 26, 2023

tetianakravchenko May 30, 2023 •

edited

Loading

mergify bot commented May 26, 2023

fearful-symmetry commented May 26, 2023

tetianakravchenko commented May 30, 2023 •

edited

Loading

mergify bot commented May 30, 2023

tetianakravchenko commented May 31, 2023

DingGGu commented Jul 26, 2023

	result.err = errors.Wrapf(err, "failed loading AWS default configuration")
	result.err = fmt.Errorf("%w failed loading AWS default configuration", error)

[Processor: add_cloud_metadata] Use AWS client to get instance metadata and EKS cluster name #35182

[Processor: add_cloud_metadata] Use AWS client to get instance metadata and EKS cluster name #35182

Conversation

tetianakravchenko commented Apr 24, 2023 • edited Loading

What does this PR do?

Why is it important?

Checklist

Author's Checklist

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

mergify bot commented Apr 24, 2023

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

💚 Build Succeeded

Build stats

❕ Flaky test report

🤖 GitHub comments

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

gizas Apr 27, 2023 • edited Loading

Choose a reason for hiding this comment

gizas May 9, 2023 • edited Loading

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

mergify bot commented May 15, 2023

tetianakravchenko commented May 15, 2023

tetianakravchenko commented May 15, 2023

tetianakravchenko commented May 17, 2023

MichaelKatsoulis commented May 17, 2023

tetianakravchenko commented May 17, 2023

MichaelKatsoulis commented May 17, 2023

Choose a reason for hiding this comment

tetianakravchenko May 30, 2023 • edited Loading

Choose a reason for hiding this comment

mergify bot commented May 26, 2023

fearful-symmetry commented May 26, 2023

tetianakravchenko commented May 30, 2023 • edited Loading

mergify bot commented May 30, 2023

tetianakravchenko commented May 31, 2023

DingGGu commented Jul 26, 2023

tetianakravchenko commented Apr 24, 2023 •

edited

Loading

elasticmachine commented Apr 24, 2023 •

edited by jenkins-beats-ci bot

Loading

gizas Apr 27, 2023 •

edited

Loading

gizas May 9, 2023 •

edited

Loading

tetianakravchenko May 30, 2023 •

edited

Loading

tetianakravchenko commented May 30, 2023 •

edited

Loading