Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(sinks): fix endpoint strip for aws region #3070

Merged
merged 5 commits into from
Jul 15, 2020

Conversation

fanatid
Copy link
Contributor

@fanatid fanatid commented Jul 15, 2020

@fanatid fanatid added type: bug A code related bug. sink: aws_s3 Anything `aws_s3` sink related sink: aws_cloudwatch_logs Anything `aws_cloudwatch_logs` sink related sink: aws_kinesis_streams Anything `aws_kinesis_streams` sink related sink: aws_cloudwatch_metrics Anything `aws_cloudwatch_metrics` sink related sink: aws_kinesis_firehose Anything `aws_kinesis_firehose` sink related provider: aws Anything `aws` service provider related labels Jul 15, 2020
@fanatid fanatid requested review from Hoverbear and bruceg July 15, 2020 08:43
@fanatid fanatid self-assigned this Jul 15, 2020
@binarylogic
Copy link
Contributor

I'd like to get @bruceg's approval before merging this, just to make sure this doesn't regress a separate feature that we are not aware of.

Copy link
Member

@lukesteensen lukesteensen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to add a test for the case that showed us this bug.

Copy link
Member

@bruceg bruceg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @lukesteensen, this needs a test case to demonstrate the issue, preferably handling all of:

  1. No trailing slash on URL,
  2. With a trailing slash,
  3. With trailing slash and path,
  4. Maybe all of the above with a query string.
    I see I tested #1, and #3 with a query, but I obviously didn't check other cases.

@fanatid
Copy link
Contributor Author

fanatid commented Jul 15, 2020

Pushed test, but github not displayed new changes in branch..

Signed-off-by: Kirill Fomichev <[email protected]>
@Hoverbear
Copy link
Contributor

Did... GA break?

Signed-off-by: Ana Hobden <[email protected]>
@Hoverbear Hoverbear merged commit 2e29c36 into master Jul 15, 2020
@fanatid fanatid deleted the aws-fix-endpoint-strip branch July 15, 2020 19:02
mengesb pushed a commit to jacobbraaten/vector that referenced this pull request Dec 9, 2020
* chore(sinks): fix endpoint strip for aws region

Signed-off-by: Kirill Fomichev <[email protected]>

* add test

Signed-off-by: Kirill Fomichev <[email protected]>

* add yet one test

Signed-off-by: Kirill Fomichev <[email protected]>

* Github actions is buggy

Signed-off-by: Ana Hobden <[email protected]>

Co-authored-by: Ana Hobden <[email protected]>
Signed-off-by: Brian Menges <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
provider: aws Anything `aws` service provider related sink: aws_cloudwatch_logs Anything `aws_cloudwatch_logs` sink related sink: aws_cloudwatch_metrics Anything `aws_cloudwatch_metrics` sink related sink: aws_kinesis_firehose Anything `aws_kinesis_firehose` sink related sink: aws_kinesis_streams Anything `aws_kinesis_streams` sink related sink: aws_s3 Anything `aws_s3` sink related type: bug A code related bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug(aws_s3 sink): specified endpoint sometimes has URL mutated unexpectedly.
5 participants