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

Multiple S3 buckets support for input, dynamic bucket names from SQS, and queue url override #364

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

abdullahzen
Copy link

@abdullahzen abdullahzen commented Dec 24, 2020

Changes made:

  • Allowing multiple s3 bucket names in the input s3 plugin and preserving the previous functionality with a single bucket name
  • Added dynamic fetching of objects by bucket names returned by SQS. (while checking that the returned bucket name is one of the bucket names that were provided in the s3_buckets and were verified)
  • Added queue url to the sqs section in the parameters to override calling get_queue_url in some cases where get_queue_url is not allowed for specific IAM users.
  • Changed the default tag to include the bucket name

Fixes #342 : Usually this error is returned when the user doesn't have permission to call get_queue_url

Added queue url to sqs section to override calling get_queue_url

Changed the default tag to include the bucket name

Signed-off-by: abdullahzen <[email protected]>
…test multiple s3 bucket names and queue url override for sqs

Signed-off-by: abdullahzen <[email protected]>
Fixed wrong variable name in the input plugin

Signed-off-by: abdullahzen <[email protected]>
@abdullahzen
Copy link
Author

@cosmo0920 Can I get a review? thank you!

desc "S3 bucket name"
config_param :s3_bucket, :string
desc "S3 bucket name(s) separated by commas in case of multiple bucket names"
config_param :s3_buckets, :string
Copy link
Contributor

Choose a reason for hiding this comment

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

Changing parameter name breaks existing configurations.
We couldn't accept this change.
Instead, using deprecated attr in config_param and adding parameter conversion code are needed to accept this PR.

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for the review. I'll do the requested changes to make it backwards compatible and get back to you 👍

@github-actions
Copy link

github-actions bot commented Jul 6, 2021

This PR has been automatically marked as stale because it has been open 90 days with no activity. Remove stale label or comment or this PR will be closed in 30 days

@github-actions github-actions bot added the stale label Jul 6, 2021
@kenhys kenhys added enhancement and removed stale labels Jul 9, 2021
@github-actions
Copy link

github-actions bot commented Oct 7, 2021

This PR has been automatically marked as stale because it has been open 90 days with no activity. Remove stale label or comment or this PR will be closed in 30 days

@github-actions github-actions bot added the stale label Oct 7, 2021
@kenhys kenhys removed the stale label Oct 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SQS The specified queue does not exist or you do not have access to it.
3 participants