-
Notifications
You must be signed in to change notification settings - Fork 9.6k
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
provider/aws: Add aws_lambda_permission #4826
provider/aws: Add aws_lambda_permission #4826
Conversation
76f057a
to
1d6d9ee
Compare
This is now ready for review. It is possible to use this with SNS. Acceptance tests may intermittently fail due to IAM - see #4447 |
if awsErr, ok := err.(awserr.Error); ok { | ||
// IAM is eventually consistent :/ | ||
if awsErr.Code() == "ResourceConflictException" { | ||
return fmt.Errorf("[WARN] Error creating ELB Listener with SSL Cert, retrying: %s", err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we Warning on creating ELB Listener with SSL Cert
? - copy/ paste error I think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is for a case where you rename the reference name, e.g.
resource "aws_lambda_permission" "one" {
to
resource "aws_lambda_permission" "two" {
It should probably be dealt with in core too, but Terraform now treats these as two different resources and the new one may be created before the old one gets deleted. I thought WARN
was the right level for this log message, but i can make it INFO
or DEBUG
- what do you suggest?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, I see, the message is obviously wrong! d'oh
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@radeksimko I mean more the actual error itself - creating ELB Listener with SSL Cert
- this resource has nothing to do with ELBs :)
1 small comment made but tests look good:
|
I will need to work around the intermittent issues with permissions being lost. Here's the full explanation including isolated reproduction case sent as support ticket to AWS: I reckon they will say it's the nature of the API (inconsistent in the same way as IAM). The solution I plan to implement includes mutex to prevent creating more permissions for the same function at the same time. It inherently makes creation of multiple resources of this type less efficient (slower), but there doesn't seem to be any other way around it. We already use mutex in AWS provider, as @stack72 mentioned - in |
3b6373a
to
e6ca769
Compare
Just a quick update from AWS support this morning - a bug on the API level has been confirmed by the Lambda team. They're working on a fix. I will try to make it work with the mutex anyway, so we can get this resource into |
22d9515
to
674f4a7
Compare
674f4a7
to
db0d48e
Compare
8a29b91
to
f8fac71
Compare
Just verified that the tests all work now 4 times :)
Very nice work @radeksimko |
[WIP] provider/aws: Add aws_lambda_permission
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
This should allow users to schedule Lambda functions via CloudWatch events (essentially crontab functionality) and/or connect functions to other AWS products, like SNS, S3 etc.
It is closing #2885 & #4454
TODO list follows:
ResourceConflictException
(retry) - similar problem to provider/aws: Retry Listener Creation for ELBs #4825qualifier
)function_name
asmyFunction
(as opposed to ARN)ResourceConflictException