search

cloudposse / terraform-aws-lambda-function

A module for launching Lambda Fuctions
https://cloudposse.com/accelerate
Apache License 2.0
30 stars 40 forks source link

fix: make sure attachments are ready before creating lambda function #41

Open manifestori opened 1 year ago

manifestori commented 1 year ago

This PR fixes the lambda creation issue, where permissions are required.

For example, if you add vpc_config, the lambda will fail on the first attempt since no attachment has been made to the IAM role. That's the only edge case.

Fixing by adding depends_on to attachments as they are implicitly required to exist before the lambda resource creation.

Gowiem commented 11 months ago

@natemccurdy did you run into this issue ever? I'm wondering if this is actually something that needs to be addressed or not still...

Gowiem commented 11 months ago

/terratest

natemccurdy commented 11 months ago

@Gowiem I did not hit this specific issue. But I did find and fix a semi-related ordering issue for the aws_iam_role_policy_attachment.custom resource, but the root cause of that was an incorrect for_each block (https://github.com/cloudposse/terraform-aws-lambda-function/pull/46).

I'm not claiming to be an expert on AWS Lambda API requirements, but I'm not totally sure that this statement is correct:

...to attachments as they are implicitly required to exist before the lambda resource creation.

It'd be good to validate that statement to nail down exactly what situations lead to an edge case.

mergify[bot] commented 6 months ago

Thanks @manifestori for creating this pull request!

A maintainer will review your changes shortly. Please don't be discouraged if it takes a while.

While you wait, make sure to review our contributor guidelines.

[!TIP]

Need help or want to ask for a PR review to be expedited?

Join us on Slack in the #pr-reviews channel.