From 088385e6866795a344e7d94032b316e328512a52 Mon Sep 17 00:00:00 2001
From: Kavindu Dodanduwa <kavindu.dodanduwa@elastic.co>
Date: Wed, 30 Oct 2024 09:10:44 -0700
Subject: [PATCH] handle empty tags, add tests and close underlying body

Signed-off-by: Kavindu Dodanduwa <kavindu.dodanduwa@elastic.co>
---
 .../add_cloud_metadata/provider_aws_ec2.go    | 48 ++++++++++++-------
 .../provider_aws_ec2_test.go                  | 27 +++++++++++
 2 files changed, 59 insertions(+), 16 deletions(-)

diff --git a/libbeat/processors/add_cloud_metadata/provider_aws_ec2.go b/libbeat/processors/add_cloud_metadata/provider_aws_ec2.go
index 735fccb235f..1e98fae2c21 100644
--- a/libbeat/processors/add_cloud_metadata/provider_aws_ec2.go
+++ b/libbeat/processors/add_cloud_metadata/provider_aws_ec2.go
@@ -153,7 +153,7 @@ func getTags(ctx context.Context, imdsClient IMDSClient, ec2Client EC2Client, in
 	logger.Info("Tag extraction from IMDS failed, fallback to DescribeTags API to obtain EKS cluster name.")
 	clusterName, err := clusterNameFromDescribeTag(ctx, ec2Client, instanceId)
 	if err != nil {
-		logger.Warnf("error obtaining cluster name: %s.", err)
+		logger.Warnf("error obtaining cluster name: %v.", err)
 		return tags
 	}
 
@@ -169,38 +169,54 @@ func getTags(ctx context.Context, imdsClient IMDSClient, ec2Client EC2Client, in
 func getTagsFromIMDS(ctx context.Context, client IMDSClient, logger *logp.Logger) (tags map[string]string, ok bool) {
 	tags = make(map[string]string)
 
-	metadata, err := client.GetMetadata(ctx, &imds.GetMetadataInput{Path: tagsCategory})
+	b, err := getMetadataHelper(ctx, client, tagsCategory, logger)
 	if err != nil {
-		logger.Warnf("error from IMDS tags category request: %s", err)
-		return tags, false
-	}
-
-	b, err := io.ReadAll(metadata.Content)
-	if err != nil {
-		logger.Warnf("error extracting tags category payload: %s", err)
+		logger.Warnf("error obtaining tags category: %v", err)
 		return tags, false
 	}
 
 	for _, tag := range strings.Split(string(b), "\n") {
 		tagPath := fmt.Sprintf("%s/%s", tagsCategory, tag)
-		metadata, err := client.GetMetadata(ctx, &imds.GetMetadataInput{Path: tagPath})
+		b, err := getMetadataHelper(ctx, client, tagPath, logger)
 		if err != nil {
-			logger.Warnf("error from IMDS tag request: %s", err)
+			logger.Warnf("error extracting tag value of %s: %v", tag, err)
 			return tags, false
 		}
 
-		b, err := io.ReadAll(metadata.Content)
-		if err != nil {
-			logger.Warnf("error extracting tag value payload: %s", err)
-			return tags, false
+		tagValue := string(b)
+		if tagValue == "" {
+			logger.Infof("Ignoring tag key %s as value is empty", tag)
+			continue
 		}
 
-		tags[tag] = string(b)
+		tags[tag] = tagValue
 	}
 
 	return tags, true
 }
 
+// getMetadataHelper performs the IMDS call for the given path and returns the response content after closing the underlying content reader.
+func getMetadataHelper(ctx context.Context, client IMDSClient, path string, logger *logp.Logger) (content []byte, err error) {
+	metadata, err := client.GetMetadata(ctx, &imds.GetMetadataInput{Path: path})
+	if err != nil {
+		return nil, fmt.Errorf("error from IMDS metadata request: %w", err)
+	}
+
+	defer func(Content io.ReadCloser) {
+		err := Content.Close()
+		if err != nil {
+			logger.Warnf("error closing IMDS metadata response body: %v", err)
+		}
+	}(metadata.Content)
+
+	content, err = io.ReadAll(metadata.Content)
+	if err != nil {
+		return nil, fmt.Errorf("error extracting metadata from the IMDS response: %w", err)
+	}
+
+	return content, nil
+}
+
 // clusterNameFromDescribeTag is a helper to extract EKS cluster name using DescribeTag.
 func clusterNameFromDescribeTag(ctx context.Context, ec2Client EC2Client, instanceID string) (string, error) {
 	input := &ec2.DescribeTagsInput{
diff --git a/libbeat/processors/add_cloud_metadata/provider_aws_ec2_test.go b/libbeat/processors/add_cloud_metadata/provider_aws_ec2_test.go
index fefd0455c7b..d4e8c14a4f6 100644
--- a/libbeat/processors/add_cloud_metadata/provider_aws_ec2_test.go
+++ b/libbeat/processors/add_cloud_metadata/provider_aws_ec2_test.go
@@ -482,6 +482,33 @@ func Test_getTags(t *testing.T) {
 				}},
 			want: map[string]string{},
 		},
+		{
+			name: "Empty tags values are ignored",
+			imdsClient: &MockIMDSClient{
+				GetMetadataFunc: func(ctx context.Context, input *imds.GetMetadataInput, f ...func(*imds.Options)) (*imds.GetMetadataOutput, error) {
+					if input.Path == tagsCategory {
+						// tag category request
+						return &imds.GetMetadataOutput{
+							Content: io.NopCloser(strings.NewReader(customTagKey)),
+						}, nil
+					}
+
+					// tag request
+					if strings.HasSuffix(input.Path, customTagKey) {
+						return &imds.GetMetadataOutput{
+							Content: io.NopCloser(strings.NewReader("")),
+						}, nil
+					}
+
+					return nil, errors.New("invalid request")
+				},
+			},
+			ec2Client: &MockEC2Client{
+				DescribeTagsFunc: func(ctx context.Context, params *ec2.DescribeTagsInput, optFns ...func(*ec2.Options)) (*ec2.DescribeTagsOutput, error) {
+					return nil, errors.New("some error from DescribeTag")
+				}},
+			want: map[string]string{},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {