r/aws_lambda_function: Remove replace_security_groups_on_destroy attribute

[jar-b]

Description

The replace_security_groups_on_destroy and replacement_security_group_ids attributes were deprecated in #31904. These should be removed, along with acceptance tests referencing these attributes.

References

Relates #31904
Relates #31520

Would you like to implement a fix?

None

[github-actions]

Community Note

Voting for Prioritization

• Please vote on this issue by adding a 👍 reaction to the original post to help the community and maintainers prioritize this request.
• Please see our prioritization guide for information on how we prioritize.
• Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.

Volunteering to Work on This Issue

• If you are interested in working on this issue, please leave a comment.
• If this would be your first contribution, please review the contribution guide.

[theipster]

Rather than removing these two attributes, replace_security_groups_on_destroy and replacement_security_group_ids, can we instead consider re-implementing them using the alternative approach described in #31520 (comment)?

Almost a year on, the underlying problem still exists in AWS, and so there is still value to be gained. Furthermore, we can confirm that the above approach does still successfully solve the problem too - making a huge difference.

In other words, rather than the now deprecated (read: now non-functional) approach whereby the provider looks up all related ENIs and modifies each individual ENI's security groups (which AWS no longer allows, for understandable reasons), instead the provider should simply modify the Lambda function's nominated security groups (and let Lambda deal with re-creating the new ENIs, which it can actually do very quickly in practice).

The overall objective (i.e. ending up with ENIs in a different security group, one way or another) and the overall goal (i.e. speeding up the Lambda function destroy) is still the same as before. So, the naming of these two attributes, replace_security_groups_on_destroy and replacement_security_group_ids, is still fairly accurate and doesn't need to change - which is convenient for backwards compatibility too.

If anyone is curious: we are adopting this approach in all our *.tftest.hcl test suites that involve VPC-enabled Lambda functions. We've effectively implemented it by adding an additional pre-teardown run { ... } block, whose sole purpose is to modify the aws_lambda_function.vpc_config.security_group_ids attribute, right at the end of the test suite just before allowing terraform test to undo its stack as usual. Adopting this approach consistently reduces our ~30m destroy to ~3m. Without this, our terraform test command would sporadically cause our CI pipeline jobs to time-out, leaving behind all the test suite resources for us to manually clean up (because, remember, resources created via terraform test have no persisted state and therefore there's no way to simply run terraform destroy on them! 😉), which is obviously a real pain.

If the aws_lambda_function resource could (continue to!) just handle all of this natively and transparently, this would simplify things on so many levels!

[jar-b]

Hey @theipster 👋 - Thanks for the detailed request, and especially for the context on how you're implementing this within Terraform test suites!

For some reason my previous understanding was that modifying the lambda security groups prior to deletion did not yield significant reductions in destroy times of the associated security groups. However, given you (and the commenter linked above) have evidence of significant time reductions, it seems my test configuration was too limited (and therefore missed the observed advantages), or AWS has made some internal changes which now make this more viable. Either way, this feels like a feature worth revisiting given the potential benefits.

I'm going to assign myself to this to investigate.

[jar-b]

First pass at this has yielded some definitive improvements on run time of acceptance tests (previously on the order of 1500s).

% make testacc PKG=lambda TESTS="TestAccLambdaFunction_VPC_replaceSGWithDefault|TestAccLambdaFunction_VPC_replaceSGWithCustom"
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.22.2 test ./internal/service/lambda/... -v -count 1 -parallel 20 -run='TestAccLambdaFunction_VPC_replaceSGWithDefault|TestAccLambdaFunction_VPC_replaceSGWithCustom'  -timeout 360m

--- PASS: TestAccLambdaFunction_VPC_replaceSGWithDefault (424.23s)
--- PASS: TestAccLambdaFunction_VPC_replaceSGWithCustom (615.75s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/lambda     620.855s

I also discovered my mistake with the previous attempt at this alternative - I missed waiting for the VPC configuration update to take effect prior to calling the delete operation. This effectively left the security groups intended to be "removed" still assigned to the lambda function during deletion, and therefore resulted in the same long-running destroy operations observed when this argument is not configured at all.

Thanks again for bringing this proposal up, @theipster ! I'm planning to get the full test suite run and a PR up shortly.

[theipster]

Thanks @jar-b for looking at this so promptly - much appreciated!

Yes, it seems to hinge on the timing of when the Lambda function gets deleted... or potentially (just a thought) more specifically when the Lambda function's IAM execution role gets deleted, given that execution roles for VPC-enabled Lambdas require those additional ec2:*NetworkInterface* permissions.

Let me know if there's anything I can help with; otherwise, I look forward to the improvement! 👍

[github-actions]

Warning

This issue has been closed, meaning that any additional comments are hard for our team to see. Please assume that the maintainers will not see them.

Ongoing conversations amongst community members are welcome, however, the issue will be locked after 30 days. Moving conversations to another venue, such as the AWS Provider forum, is recommended. If you have additional concerns, please open a new issue, referencing this one where needed.

[github-actions]

This functionality has been released in v5.51.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

[github-actions]

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.