Skip to content

Commit

Permalink
address pr comment
Browse files Browse the repository at this point in the history
  • Loading branch information
efd6 committed May 24, 2023
1 parent d2b237c commit 2572777
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 15 deletions.
2 changes: 1 addition & 1 deletion x-pack/filebeat/docs/inputs/input-aws-s3.asciidoc
Expand Up @@ -261,7 +261,7 @@ URL of the AWS SQS queue that messages will be received from. (Required when `bu
==== `region_name`

The name of the AWS region of the end point for testing purposes. This option
must not be set when using amazonaws.com end points.
must not be set when using amazonaws end points that provide a region.

[float]
==== `visibility_timeout`
Expand Down
13 changes: 4 additions & 9 deletions x-pack/filebeat/input/awss3/config.go
Expand Up @@ -7,8 +7,6 @@ package awss3
import (
"errors"
"fmt"
"net/url"
"strings"
"time"

"github.com/dustin/go-humanize"
Expand Down Expand Up @@ -81,13 +79,10 @@ func (c *config) Validate() error {
return fmt.Errorf("number_of_workers <%v> must be greater than 0", c.NumberOfWorkers)
}

if c.QueueURL != "" && c.RegionName != "" {
u, err := url.Parse(c.QueueURL)
if err != nil {
return fmt.Errorf("invalid queue_url: %w", err)
}
if strings.HasSuffix(u.Host, ".amazonaws.com") {
return fmt.Errorf("region_name <%s> must not be set with an amazonaws.com queue_url", c.RegionName)
if c.QueueURL != "" {
region, _ := getRegionFromQueueURL(c.QueueURL, c.AWSConfig.Endpoint)
if region != "" && c.RegionName != "" {
return fmt.Errorf("region_name <%s> must not be set with a queue_url containing a region name", c.RegionName)
}
}

Expand Down
35 changes: 33 additions & 2 deletions x-pack/filebeat/input/awss3/config_test.go
Expand Up @@ -120,26 +120,57 @@ func TestConfig(t *testing.T) {
config: mapstr.M{
"queue_url": queueURL,
"region_name": "region",
"endpoint": "ep",
},
expectedErr: "",
expectedCfg: func(queueURL, s3Bucket string, nonAWSS3Bucket string) config {
c := makeConfig(queueURL, "", "")
c.RegionName = "region"
c.AWSConfig.Endpoint = "ep"
return c
},
},
{
name: "AWS endpoint with explicit region",
name: "explicit AWS endpoint with explicit region",
queueURL: queueURL,
s3Bucket: "",
nonAWSS3Bucket: "",
config: mapstr.M{
"queue_url": "https://sqs.us-east-1.amazonaws.com/627959692251/test-s3-logs",
"region_name": "region",
"endpoint": "amazonaws.com",
},
expectedErr: "region_name <region> must not be set with an amazonaws.com queue_url accessing config",
expectedErr: "region_name <region> must not be set with a queue_url containing a region name",
expectedCfg: nil,
},
{
name: "inferred AWS endpoint with explicit region",
queueURL: queueURL,
s3Bucket: "",
nonAWSS3Bucket: "",
config: mapstr.M{
"queue_url": "https://sqs.us-east-1.amazonaws.com/627959692251/test-s3-logs",
"region_name": "region",
},
expectedErr: "region_name <region> must not be set with a queue_url containing a region name",
expectedCfg: nil,
},
{
name: "localstack_with_region_name",
queueURL: "http://localhost:4566/000000000000/sample-queue",
s3Bucket: "",
nonAWSS3Bucket: "",
config: mapstr.M{
"queue_url": "http://localhost:4566/000000000000/sample-queue",
"region_name": "myregion",
},
expectedErr: "",
expectedCfg: func(queueURL, s3Bucket string, nonAWSS3Bucket string) config {
c := makeConfig(queueURL, "", "")
c.RegionName = "myregion"
return c
},
},
{
name: "error on no queueURL and s3Bucket and nonAWSS3Bucket",
queueURL: "",
Expand Down
8 changes: 5 additions & 3 deletions x-pack/filebeat/input/awss3/input.go
Expand Up @@ -311,9 +311,11 @@ func getRegionFromQueueURL(queueURL string, endpoint string) (string, error) {
return "", fmt.Errorf(queueURL + " is not a valid URL")
}
if url.Scheme == "https" && url.Host != "" {
queueHostSplit := strings.Split(url.Host, ".")
if len(queueHostSplit) > 2 && (strings.Join(queueHostSplit[2:], ".") == endpoint || (endpoint == "" && queueHostSplit[2] == "amazonaws")) {
return queueHostSplit[1], nil
queueHostSplit := strings.SplitN(url.Host, ".", 3)
if len(queueHostSplit) == 3 {
if queueHostSplit[2] == endpoint || (endpoint == "" && strings.HasPrefix(queueHostSplit[2], "amazonaws.")) {
return queueHostSplit[1], nil
}
}
}
return "", fmt.Errorf("QueueURL is not in format: https://sqs.{REGION_ENDPOINT}.{ENDPOINT}/{ACCOUNT_NUMBER}/{QUEUE_NAME}")
Expand Down

0 comments on commit 2572777

Please sign in to comment.