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.

Sign up for GitHub

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

Jump to bottom

Fix permissions issues for SQS #10265

Merged
merged 1 commit into from
Jan 26, 2019
Merged

Fix permissions issues for SQS #10265

merged 1 commit into from
Jan 26, 2019

Conversation

ph
Copy link
Contributor

@ph ph commented Jan 22, 2019 •
edited
Loading

NOTES: This PR is based on top of #10116

Correctly add the permissions to the lambda role when monitoring
SQS queue.

Fixes: #9152

@ph ph requested a review from kvch January 22, 2019 21:02
kvch
kvch reviewed Jan 23, 2019
View reviewed changes
x-pack/functionbeat/provider/aws/sqs.go
map[string]interface{}{
"Action": []string{
"sqs:ChangeMessageVisibility",
"sqs:DeleteMessage",
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious: Why is this permission needed?

Copy link
Contributor Author

@ph ph Jan 23, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

It's linked to how SQS works, when you receive a batch of events from SQS you have to change the visibility of the events from visible to hidden, that visibility property has a timeout, this allows an event to be used by another workers if your initial worker crashed or something else bad happened.

When you have successfully processed the event you have to delete the event from the queue so no other worker (or function) can take it.

Now looking at the official doc
They list the following:

- sqs:ReceiveMessage
- sqs:DeleteMessage
- sqs:GetQueueAttributes

And another official doc (I can't find it) also mention sqs:ChangeMessageVisibility.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for the explanation.

@ph ph added the needs_backport PR is waiting to be backported to other branches. label Jan 23, 2019
@ph
Fix permissions issues for SQS
da94016 
Correctly creates add the permission to the lambda role when monitoring
SQS queue.

Fixes: elastic#9152
@ph ph force-pushed the fn/permission-sqs branch from 36ef020 to da94016 Compare January 25, 2019 19:33
@ph ph merged commit dc963c4 into elastic:master Jan 26, 2019
ph added a commit to ph/beats that referenced this pull request Jan 29, 2019
@ph
Fix permissions issues for SQS (elastic#10265)
3042df6 
**NOTES:**  This PR is based on top of elastic#10116 

Correctly add the permissions to the lambda role when monitoring
SQS queue.

Fixes: elastic#9152

(cherry picked from commit dc963c4)
@ph ph mentioned this pull request Jan 29, 2019
Merged
@ph ph added v6.7.0 and removed needs_backport PR is waiting to be backported to other branches. labels Jan 29, 2019
ph added a commit that referenced this pull request Jan 30, 2019
@ph
Fix permissions issues for SQS (#10265) (#10411)
0739a6a 
**NOTES:**  This PR is based on top of #10116 

Correctly add the permissions to the lambda role when monitoring
SQS queue.

Fixes: #9152

(cherry picked from commit dc963c4)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kvch kvch kvch approved these changes

Assignees
No one assigned
Labels
Functionbeat review v6.7.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ph @kvch