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 da18d63
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 15 deletions.
3 changes: 2 additions & 1 deletion x-pack/filebeat/docs/inputs/input-aws-s3.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,8 @@ 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, and must
be set for end points that do not.

[float]
==== `visibility_timeout`
Expand Down
16 changes: 7 additions & 9 deletions x-pack/filebeat/input/awss3/config.go
Original file line number Diff line number Diff line change
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,13 @@ 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)
switch {
case region != "" && c.RegionName != "":
return fmt.Errorf("region_name <%s> must not be set with a queue_url containing a region name", c.RegionName)
case region == "" && c.RegionName == "":
return errors.New("region_name must be set for a queue_url not containing a region name")
}
}

Expand Down
49 changes: 47 additions & 2 deletions x-pack/filebeat/input/awss3/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,26 +120,71 @@ 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: "localstack_no_region_name",
queueURL: "http://localhost:4566/000000000000/sample-queue",
s3Bucket: "",
nonAWSS3Bucket: "",
config: mapstr.M{
"queue_url": "http://localhost:4566/000000000000/sample-queue",
},
expectedErr: "region_name must be set for a queue_url not containing a region name accessing config",
expectedCfg: func(queueURL, s3Bucket string, nonAWSS3Bucket string) config {
c := makeConfig(queueURL, "", "")
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
Original file line number Diff line number Diff line change
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 da18d63

Please sign in to comment.