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

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

manifestori
Copy link

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.

@manifestori manifestori requested review from a team as code owners June 26, 2023 11:29
@Gowiem
Copy link
Member

Gowiem commented Oct 12, 2023

@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 Gowiem added the patch A minor, backward compatible change label Oct 12, 2023
@Gowiem
Copy link
Member

Gowiem commented Oct 12, 2023

/terratest

@natemccurdy
Copy link
Contributor

@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 (#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.

@hans-d hans-d added wip Work in Progress: Not ready for final review or merge stale This PR has gone stale and removed wip Work in Progress: Not ready for final review or merge labels Mar 8, 2024
Copy link

mergify bot commented Mar 10, 2024

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.

@mergify mergify bot removed the stale This PR has gone stale label Mar 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch A minor, backward compatible change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants