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

Targets of event rules are duplicated if delete permission is missing #286

Closed
cloetzi opened this issue Aug 26, 2016 · 7 comments
Closed

Comments

@cloetzi
Copy link
Contributor

cloetzi commented Aug 26, 2016

I am experimenting with minimal permissions for the IAM user used by zappa. If the permission for deleting a target/rule is missing, the targets in a schedule are duplicated without that an error is raised. This means, the lambda function gets triggered multiple times simultaneously.

The issue can be reproduced by restricting the event permissions of the Zappa AWS user to the policy below and run zappa update.

{
  "Action": [
    "events:PutRule",
    "events:PutTargets",
    "events:ListRules"
  ],
  "Resource": "*",
  "Effect": "Allow"
}
@Miserlou
Copy link
Owner

Damn, that's a great bug.

Can you confirm that the function is actually executed multiple times concurrently?

I guess the issue is that we should raise a warning when this occurs, or do you expect another behavior?

@cloetzi
Copy link
Contributor Author

cloetzi commented Aug 26, 2016

Yes, it seems that it executes concurrently. See Cloudwatch graph below from the Lambda execution, at the end I had 5 targets running every minute.
image

I'd expect to see the AWS error, for example if you remove the ListRules permission, Zappa nicely tells you that this permission is missing. For the delete targets, this error must be swallowed somehow. I had a brief look in the code but I could not find where it actually happens, will have another look now.

@Miserlou
Copy link
Owner

Miserlou commented Aug 26, 2016

Ah, dang. Okay, good bug report. This code got messed with recently. Is this against 0.21.0 or master head?

@cloetzi
Copy link
Contributor Author

cloetzi commented Aug 26, 2016

It is against master head.

@cloetzi
Copy link
Contributor Author

cloetzi commented Aug 26, 2016

Found the issue: https://github.com/Miserlou/Zappa/blob/master/zappa/zappa.py#L1275
I was missing the permission ListTargetsByRule, this error is swallowed and skips the actual delete call.
A possible solution could be to check if the ClientError is an AccessDeniedException and if so, raise it.

EDIT: Added missing words.

@Miserlou
Copy link
Owner

Excellent sleuthing! Sounds like a good solution to me if you can properly type damn boto3 errors, want to send a PR? 😄

@cloetzi
Copy link
Contributor Author

cloetzi commented Aug 26, 2016

Working on it.

kalkehcoisa pushed a commit to kalkehcoisa/Zappa that referenced this issue Jan 30, 2024
kalkehcoisa pushed a commit to kalkehcoisa/Zappa that referenced this issue Jan 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants