Details
- Reviewers
varun bartek - Group Reviewers
Restricted Owners Package (Owns No Changed Paths) - Commits
- rCOMM4caa544f818b: [terraform] [4/n] search_index_lambda terraform configuration options
Verified terraform configuration on staging
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Requires further discussion and planning of error handling. We'll need to set retry limits, batch sizes, and a failed-event destination so we can store failed entries in a queue.
| services/terraform/modules/shared/search_index_lambda.tf | ||
|---|---|---|
| 58 ↗ | (On Diff #34814) | Will change example to search-index-lambda |
Will add dummy file and script for terraform apply and cargo lambda build
| services/terraform/remote/aws_iam.tf | ||
|---|---|---|
| 302 ↗ | (On Diff #34814) | This will be changed to the actual arn of the lambda, not the iam_role |
| services/terraform/remote/aws_iam.tf | ||
|---|---|---|
| 321 ↗ | (On Diff #34814) | Try to avoid unnecessary changes like this by reviewing git diff before you put a diff up |
If bootstrap.zip is a dummy, why is it so large? 3 MiB is a lot to add to the Git repo
I was able to reduce the dummy to 725 KB by removing any extraneous dependencies. It's currently a minimally sized lambda but packaging the runtime makes this still decently large.
Hopefully, me and Bartek can figure out S3 buckets relatively soon as to replace this temporary solution. If not, another consideration might be to create a Node.js bootstrap.zip lambda which does not need to include a Rust runtime.
Thanks for explaining! If you end up needing to commit the bootstrap.zip file to the repo, then before you land this diff, can you link a Linear task tracking migrating the file to an S3 bucket?
how long does it take to build the lambda? if we're just adding the zip file to satisfy buildkite we could instead build and zip the lambda with a script similar to run.sh and call that script in the CI steps
| services/docker-compose.yml | ||
|---|---|---|
| 99 ↗ | (On Diff #35117) | why do we need this? |
| services/terraform/modules/shared/search_index_lambda.tf | ||
| 39 ↗ | (On Diff #35117) | might be worth adding a comment here explaining why the count is 0 for dev |
| services/terraform/remote/run.sh | ||
| 11 ↗ | (On Diff #35117) | this seems a little dangerous on staging/prod (especially prod). can we still require the user to confirm the changes before they're applied? |
It doesn't take long at all to build the lambda. You're correct; the Zip file is purely there to satisfy buildkite. I definitely believe that removing the zip file and running a script in CI is the better solution.
| services/docker-compose.yml | ||
|---|---|---|
| 99 ↗ | (On Diff #35117) | Localstack, when simulating lambdas, uses Docker to run the rust binaries. This line enables the Docker socket in the Localstack container. If removed, there will be a "Docker not available" error "/var/run/docker.sock:/var/run/docker.sock" |
| services/terraform/modules/shared/search_index_lambda.tf | ||
|---|---|---|
| 39 ↗ | (On Diff #35117) | Making this count = 0 is a mistake. Now that I'm trying to enable local dev with localstack, I'll remove this |
| services/terraform/remote/run.sh | ||
|---|---|---|
| 11 ↗ | (On Diff #35230) | It will build the lambda bootstrap.zip and move it to the search-index-lambda folder so that the terraform config files can pick up on it. Afterwards, we can just run terraform apply and it'll wait for user confirmation. If you don't run it, terraform apply will complain that there is no bootstrap.zip file at the specified path. |
| services/terraform/modules/shared/search_index_lambda.tf | ||
|---|---|---|
| 39 ↗ | (On Diff #35117) | Yeah, that's right. Omitting count guarantees a single instance of this resource. does it default to 1? So I don't believe that's exactly how it works. Whenever count is used in a resource, instances of the resource are then indexed. So if it was count = 1, we'd access this by aws_lambda_event_source_mapping.trigger[0]. If we never use count, there shouldn't be an index. |
| services/terraform/modules/shared/search_index_lambda.tf | ||
|---|---|---|
| 39 ↗ | (On Diff #35117) |
Exactly, count is a built-in Terraform meta-param that spawns multiple instances of a resource and gives ways to iterate through them, without code duplication |