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

chore: ensure graphql lambda function has the correct IAM permissions #371

Merged
merged 27 commits into from
Jul 31, 2024

Conversation

as1729
Copy link
Contributor

@as1729 as1729 commented Jul 25, 2024

There are 3 pieces to ensuring we are able to satisfy requirements of issue #348

  1. Front-end and GraphQL changes that accept payload from the user and parse it on the backend to submit to AWS.
  2. Terraform permissions updates so the GraphQL lambda can invoke the treasury file generation lambdas.
  3. GraphQL changes to aws.ts to support lambda invocation from the Redwoodjs lambda function.

This PR addresses point number 2.

Copy link

github-actions bot commented Jul 25, 2024

Terraform Summary

Step Result
🖌 Terraform Format & Style
⚙️ Terraform Initialization
🤖 Terraform Validation
📖 Terraform Plan

Hint: If "Terraform Format & Style" failed, run terraform fmt -recursive from the terraform/ directory and commit the results.

Output

Validation Output
Success! The configuration is valid.


Plan Summary
CHANGE RESOURCE
add aws_s3_object.origin_dist_artifact["static/js/app.778db0a8.js"]
aws_s3_object.origin_dist_artifact["static/js/app.778db0a8.js.LICENSE.txt"]
module.lambda_function-subrecipientTreasuryReportGen.aws_lambda_permission.current_version_triggers["StepFunctionTrigger"]
module.lambda_function-subrecipientTreasuryReportGen.aws_lambda_permission.unqualified_alias_triggers["StepFunctionTrigger"]
module.lambda_function-treasuryProjectFileGeneration.aws_cloudwatch_log_group.lambda[0]
module.lambda_function-treasuryProjectFileGeneration.aws_iam_policy.additional_inline[0]
module.lambda_function-treasuryProjectFileGeneration.aws_iam_policy.additional_jsons[0]
module.lambda_function-treasuryProjectFileGeneration.aws_iam_policy.logs[0]
module.lambda_function-treasuryProjectFileGeneration.aws_iam_role.lambda[0]
module.lambda_function-treasuryProjectFileGeneration.aws_iam_role_policy_attachment.additional_inline[0]
module.lambda_function-treasuryProjectFileGeneration.aws_iam_role_policy_attachment.additional_jsons[0]
module.lambda_function-treasuryProjectFileGeneration.aws_iam_role_policy_attachment.logs[0]
module.lambda_function-treasuryProjectFileGeneration.aws_lambda_function.this[0]
module.lambda_function-treasuryProjectFileGeneration.aws_lambda_permission.current_version_triggers["S3BucketNotification"]
module.lambda_function-treasuryProjectFileGeneration.aws_lambda_permission.current_version_triggers["StepFunctionTrigger"]
module.lambda_function-treasuryProjectFileGeneration.aws_lambda_permission.unqualified_alias_triggers["S3BucketNotification"]
module.lambda_function-treasuryProjectFileGeneration.aws_lambda_permission.unqualified_alias_triggers["StepFunctionTrigger"]
module.treasury_generation_step_function.aws_iam_policy.service["lambda"]
module.treasury_generation_step_function.aws_iam_policy_attachment.service["lambda"]
module.treasury_generation_step_function.aws_iam_role.this[0]
module.treasury_generation_step_function.aws_sfn_state_machine.this[0]
update aws_ecs_service.console
aws_s3_object.lambda_artifact-graphql
aws_s3_object.lambda_artifact-processValidationJson
aws_s3_object.origin_dist_artifact["200.html"]
aws_s3_object.origin_dist_artifact["build-manifest.json"]
aws_s3_object.origin_dist_artifact["chunk-references.json"]
aws_s3_object.origin_dist_artifact["index.html"]
module.lambda_function-cpfValidation.aws_lambda_function.this[0]
module.lambda_function-graphql.aws_iam_policy.additional_inline[0]
module.lambda_function-graphql.aws_lambda_function.this[0]
module.lambda_function-processValidationJson.aws_lambda_function.this[0]
module.lambda_function-subrecipientTreasuryReportGen.aws_lambda_function.this[0]
recreate aws_ecs_task_definition.console
aws_s3_object.lambda_artifact-python
module.lambda_function-cpfValidation.aws_lambda_permission.current_version_triggers["S3BucketNotification"]
module.lambda_function-graphql.aws_lambda_permission.current_version_triggers["APIGateway"]
module.lambda_function-processValidationJson.aws_lambda_permission.current_version_triggers["S3BucketNotification"]
delete aws_s3_object.origin_dist_artifact["static/js/app.4ab2a836.js"]
aws_s3_object.origin_dist_artifact["static/js/app.4ab2a836.js.LICENSE.txt"]
module.lambda_function-treasuryReportGeneration["1A"].aws_cloudwatch_log_group.lambda[0]
module.lambda_function-treasuryReportGeneration["1A"].aws_iam_policy.additional_inline[0]
module.lambda_function-treasuryReportGeneration["1A"].aws_iam_policy.additional_jsons[0]
module.lambda_function-treasuryReportGeneration["1A"].aws_iam_policy.logs[0]
module.lambda_function-treasuryReportGeneration["1A"].aws_iam_role.lambda[0]
module.lambda_function-treasuryReportGeneration["1A"].aws_iam_role_policy_attachment.additional_inline[0]
module.lambda_function-treasuryReportGeneration["1A"].aws_iam_role_policy_attachment.additional_jsons[0]
module.lambda_function-treasuryReportGeneration["1A"].aws_iam_role_policy_attachment.logs[0]
module.lambda_function-treasuryReportGeneration["1A"].aws_lambda_function.this[0]
module.lambda_function-treasuryReportGeneration["1A"].aws_lambda_permission.current_version_triggers["S3BucketNotification"]
module.lambda_function-treasuryReportGeneration["1A"].aws_lambda_permission.unqualified_alias_triggers["S3BucketNotification"]
module.lambda_function-treasuryReportGeneration["1B"].aws_cloudwatch_log_group.lambda[0]
module.lambda_function-treasuryReportGeneration["1B"].aws_iam_policy.additional_inline[0]
module.lambda_function-treasuryReportGeneration["1B"].aws_iam_policy.additional_jsons[0]
module.lambda_function-treasuryReportGeneration["1B"].aws_iam_policy.logs[0]
module.lambda_function-treasuryReportGeneration["1B"].aws_iam_role.lambda[0]
module.lambda_function-treasuryReportGeneration["1B"].aws_iam_role_policy_attachment.additional_inline[0]
module.lambda_function-treasuryReportGeneration["1B"].aws_iam_role_policy_attachment.additional_jsons[0]
module.lambda_function-treasuryReportGeneration["1B"].aws_iam_role_policy_attachment.logs[0]
module.lambda_function-treasuryReportGeneration["1B"].aws_lambda_function.this[0]
module.lambda_function-treasuryReportGeneration["1B"].aws_lambda_permission.current_version_triggers["S3BucketNotification"]
module.lambda_function-treasuryReportGeneration["1B"].aws_lambda_permission.unqualified_alias_triggers["S3BucketNotification"]
module.lambda_function-treasuryReportGeneration["1C"].aws_cloudwatch_log_group.lambda[0]
module.lambda_function-treasuryReportGeneration["1C"].aws_iam_policy.additional_inline[0]
module.lambda_function-treasuryReportGeneration["1C"].aws_iam_policy.additional_jsons[0]
module.lambda_function-treasuryReportGeneration["1C"].aws_iam_policy.logs[0]
module.lambda_function-treasuryReportGeneration["1C"].aws_iam_role.lambda[0]
module.lambda_function-treasuryReportGeneration["1C"].aws_iam_role_policy_attachment.additional_inline[0]
module.lambda_function-treasuryReportGeneration["1C"].aws_iam_role_policy_attachment.additional_jsons[0]
module.lambda_function-treasuryReportGeneration["1C"].aws_iam_role_policy_attachment.logs[0]
module.lambda_function-treasuryReportGeneration["1C"].aws_lambda_function.this[0]
module.lambda_function-treasuryReportGeneration["1C"].aws_lambda_permission.current_version_triggers["S3BucketNotification"]
module.lambda_function-treasuryReportGeneration["1C"].aws_lambda_permission.unqualified_alias_triggers["S3BucketNotification"]

Pusher: @as1729, Action: pull_request_target, Workflow: Continuous Integration

Copy link

github-actions bot commented Jul 25, 2024

QA Summary

See our documentation for tips on how to resolve failing QA checks.

QA Check Result
🌐 Web Tests
🔗 API Tests
🐍 Python Tests
📏 ESLint
🧼 Ruff
🧹 TFLint

Test Coverage

Coverage report for api suite
St File % Stmts % Branch % Funcs % Lines Uncovered Line #s
🔴 All files 45.85 27.8 51.63 46.28
🔴  src 0 100 0 0
🔴   server.ts 0 100 0 0 6-13
🟢  src/directives/requireAuth 100 100 100 100
🟢   requireAuth.ts 100 100 100 100
🟡  src/directives/skipAuth 50 100 0 50
🟡   skipAuth.ts 50 100 0 50 13
🔴  src/functions 0 100 0 0
🔴   graphql.ts 0 100 0 0 15-27
🔴  src/functions/processValidationJson 38.55 50 44.44 37.8
🟢   processValidationJson.scenarios.ts 100 100 100 100
🔴   processValidationJson.ts 37.8 50 44.44 37.03 59-98,118-119,157-165,177-178,193-196,201-242,255-256,270-309,321-324,332
🔴  src/graphql 0 100 100 0
🔴   agencies.sdl.ts 0 100 100 0 1
🔴   expenditureCategories.sdl.ts 0 100 100 0 1
🔴   inputTemplates.sdl.ts 0 100 100 0 1
🔴   organizations.sdl.ts 0 100 100 0 1
🔴   outputTemplates.sdl.ts 0 100 100 0 1
🔴   projects.sdl.ts 0 100 100 0 1
🔴   reportingPeriods.sdl.ts 0 100 100 0 1
🔴   subrecipientUploads.sdl.ts 0 100 100 0 1
🔴   subrecipients.sdl.ts 0 100 100 0 1
🔴   uploadValidations.sdl.ts 0 100 100 0 1
🔴   uploads.sdl.ts 0 100 100 0 1
🔴   users.sdl.ts 0 100 100 0 1
🔴   validationRuleses.sdl.ts 0 100 100 0 1
🔴  src/lib 11 8.92 8.69 11.26
🟡   auth.ts 64.7 48.38 57.14 67.34 70-71,77-78,94-95,117,124,127,132-139,163,167
🔴   aws.ts 9.25 12.5 7.14 9.25 29-97,121-157,172-259
🟢   constants.ts 100 100 100 100
🔴   db.ts 45.45 50 50 45.45 15-35,41,43,50
🔴   ec-codes.ts 0 100 100 0 1
🟢   logger.ts 100 100 100 100
🔴   persist-upload.js 0 0 0 0 16-295
🔴   preconditions.ts 0 0 0 0 2-3
🔴   records.js 0 0 0 0 12-214
🔴   templateRules.ts 0 0 0 0
🔴   tracer.ts 0 100 100 0 5-14
🔴   validate-upload.js 0 0 0 0 18-790
🟢   validation-error.ts 83.33 100 50 83.33 22
🔴   validation-rules.js 0 0 0 0 6-194
🟡  src/services/agencies 67.34 50 80 67.34
🟢   agencies.scenarios.ts 100 100 100 100
🟡   agencies.ts 65.21 50 75 65.21 40-51,60-64,97-98,104,113-121
🟡  src/services/expenditureCategories 78.57 66.66 88.88 78.57
🟢   expenditureCategories.scenarios.ts 100 100 100 100
🟡   expenditureCategories.ts 77.77 66.66 88.88 77.77 30-34,49-52,60,91
🟡  src/services/inputTemplates 77.77 66.66 85.71 77.77
🟢   inputTemplates.scenarios.ts 100 100 100 100
🟡   inputTemplates.ts 76.92 66.66 85.71 76.92 25-29,39-40,50,85
🟡  src/services/organizations 77.61 92.85 55.55 77.61
🟢   organizations.scenarios.ts 100 100 100 100
🟡   organizations.ts 76.56 92.85 50 76.56 36-40,75,147-177,200-224
🟡  src/services/outputTemplates 77.77 66.66 85.71 77.77
🟢   outputTemplates.scenarios.ts 100 100 100 100
🟡   outputTemplates.ts 76.92 66.66 85.71 76.92 25-29,39-40,50,81
🟡  src/services/passage 74.07 62.5 100 74.07
🟡   passage.ts 74.07 62.5 100 74.07 18-19,65-76
🟡  src/services/projects 80 100 62.5 80
🟢   projects.scenarios.ts 100 100 100 100
🟡   projects.ts 78.57 100 62.5 78.57 45-51
🟡  src/services/reportingPeriods 69.23 70 54.54 69.23
🟢   reportingPeriods.scenarios.ts 100 100 100 100
🟡   reportingPeriods.ts 68.42 70 54.54 68.42 25-29,39-40,55-58,74,105-121
🟢  src/services/subrecipientUploads 88.88 100 81.81 88.88
🟢   subrecipientUploads.scenarios.ts 100 100 100 100
🟢   subrecipientUploads.ts 84.61 100 71.42 84.61 46-51
🟢  src/services/subrecipients 85.71 100 71.42 85.71
🟢   subrecipients.scenarios.ts 100 100 100 100
🟢   subrecipients.ts 84.61 100 71.42 84.61 47-52
🟡  src/services/uploadValidations 57.14 100 14.28 57.14
🟢   uploadValidations.scenarios.ts 100 100 100 100
🟡   uploadValidations.ts 53.84 100 14.28 53.84 10,16,30,38,45-48
🟢  src/services/uploads 90.81 64.28 86.84 90.81
🟢   uploads.scenarios.ts 100 100 100 100
🟢   uploads.ts 87.5 64.28 66.66 87.5 30,96,124-138,245,276
🟢  src/services/users 85.96 79.41 92 85.96
🟢   users.scenarios.ts 100 100 100 100
🟢   users.ts 84.9 79.41 88.88 84.9 227-229,238-242,260-261,275-278,296-298,306-307,312,321-324
🟢  src/services/validationRuleses 85.71 100 71.42 85.71
🟢   validationRuleses.scenarios.ts 100 100 100 100
🟢   validationRuleses.ts 84.61 100 71.42 84.61 43-48
Coverage report for web suite
St File % Stmts % Branch % Funcs % Lines Uncovered Line #s
🔴 All files 13.09 13.84 10.86 12.44
🟢  api/src/lib 100 100 100 100
🟢   constants.ts 100 100 100 100
🔴  web/src 28.57 18.75 66.66 28.57
🔴   App.tsx 0 0 0 0 3-36
🟢   Routes.tsx 100 100 100 100
🟡   auth.ts 50 50 100 50 19-24
🔴   entry.client.tsx 0 0 100 0 10-22
🔴  web/src/auth 7.14 0 4.16 7.14
🔴   localAuth.ts 9.09 0 8.33 9.09 39-68,76-80
🔴   passageAuth.ts 5 0 0 5 22-25,31-60
🔴  web/src/components/Agency/Agencies 0 100 0 0
🔴   Agencies.tsx 0 100 0 0 9-21
🔴  web/src/components/Agency/AgenciesCell 0 100 0 0
🔴   AgenciesCell.tsx 0 100 0 0 8-39
🔴  web/src/components/Agency/Agency 0 0 0 0
🔴   Agency.tsx 0 0 0 0 10-78
🔴  web/src/components/Agency/AgencyCell 0 100 0 0
🔴   AgencyCell.tsx 0 100 0 0 7-27
🔴  web/src/components/Agency/AgencyForm 0 0 0 0
🔴   AgencyForm.tsx 0 0 0 0 25-45
🔴  web/src/components/Agency/EditAgencyCell 0 100 0 0
🔴   EditAgencyCell.tsx 0 100 0 0 10-59
🔴  web/src/components/Agency/NewAgency 0 100 0 0
🔴   NewAgency.tsx 0 100 0 0 9-35
🟢  web/src/components/Navigation 100 30 100 100
🟢   Navigation.tsx 100 30 100 100 26-81
🔴  web/src/components/Organization/EditOrganizationCell 0 100 0 0
🔴   EditOrganizationCell.tsx 0 100 0 0 13-62
🔴  web/src/components/Organization/EditOrganizationForm 0 0 0 0
🔴   EditOrganizationForm.tsx 0 0 0 0 27-41
🔴  web/src/components/Organization/NewOrganization 0 100 0 0
🔴   NewOrganization.tsx 0 100 0 0 9-37
🔴  web/src/components/Organization/NewOrganizationForm 0 0 0 0
🔴   NewOrganizationForm.tsx 0 0 0 0 25-54
🔴  web/src/components/Organization/Organization 0 0 0 0
🔴   Organization.tsx 0 0 0 0 10-70
🔴  web/src/components/Organization/OrganizationCell 0 100 0 0
🔴   OrganizationCell.tsx 0 100 0 0 7-27
🔴  web/src/components/Organization/OrganizationPickListsCell 40 0 27.27 36.36
🟡   OrganizationPickListsCell.mock.ts 50 100 0 100
🔴   OrganizationPickListsCell.stories.tsx 0 0 0 0 6-32
🟡   OrganizationPickListsCell.tsx 64.28 100 50 58.33 14-16,47-72
🔴  web/src/components/Organization/Organizations 0 100 0 0
🔴   Organizations.tsx 0 100 0 0 9-21
🔴  web/src/components/Organization/OrganizationsCell 0 100 0 0
🔴   OrganizationsCell.tsx 0 100 0 0 8-37
🔴  web/src/components/ReportingPeriod/EditReportingPeriodCell 0 100 0 0
🔴   EditReportingPeriodCell.tsx 0 100 0 0 13-74
🔴  web/src/components/ReportingPeriod/NewReportingPeriod 0 100 0 0
🔴   NewReportingPeriod.tsx 0 100 0 0 9-35
🔴  web/src/components/ReportingPeriod/ReportingPeriod 0 0 0 0
🔴   ReportingPeriod.tsx 0 0 0 0 12-101
🔴  web/src/components/ReportingPeriod/ReportingPeriodCell 0 100 0 0
🔴   ReportingPeriodCell.tsx 0 100 0 0 7-33
🔴  web/src/components/ReportingPeriod/ReportingPeriodForm 0 0 0 0
🔴   ReportingPeriodForm.tsx 0 0 0 0 18-43
🔴  web/src/components/ReportingPeriod/ReportingPeriods 0 0 0 0
🔴   ReportingPeriods.tsx 0 0 0 0 13-96
🔴  web/src/components/ReportingPeriod/ReportingPeriodsCell 0 100 0 0
🔴   ReportingPeriodsCell.tsx 0 100 0 0 8-43
🟡  web/src/components/ReportingPeriodsCell 57.14 0 60 50
🟢   ReportingPeriodsCell.mock.ts 100 100 100 100
🔴   ReportingPeriodsCell.stories.tsx 0 0 0 0 6-32
🟢   ReportingPeriodsCell.tsx 100 100 100 100
🔴  web/src/components/TableBuilder 0 0 0 0
🔴   DebouncedInput.tsx 0 0 0 0 13-32
🔴   Filter.tsx 0 0 0 0 6-15
🔴   TableBuilder.tsx 0 0 0 0 22-70
🔴   TableHeader.tsx 0 0 0 0 5-42
🔴   TableRow.tsx 0 100 0 0 3-7
🟡  web/src/components/TemplateUploadReportingPeriodCell 55 0 55.55 47.05
🟢   TemplateUploadReportingPeriodCell.mock.ts 100 100 100 100
🔴   TemplateUploadReportingPeriodCell.stories.tsx 0 0 0 0 11-37
🟢   TemplateUploadReportingPeriodCell.tsx 100 100 100 100
🔴  web/src/components/TreasuryGeneration/DownloadTreasuryFiles 0 100 0 0
🔴   DownloadTreasuryFiles.tsx 0 100 0 0 6-30
🔴  web/src/components/TreasuryGeneration/NewTreasuryGeneration 0 100 0 0
🔴   NewTreasuryGeneration.tsx 0 100 0 0 8-39
🔴  web/src/components/TreasuryGeneration/NewTreasuryGenerationForm 0 0 0 0
🔴   NewTreasuryGenerationForm.tsx 0 0 0 0 20-31
🔴  web/src/components/Upload/EditUploadCell 0 100 0 0
🔴   EditUploadCell.tsx 0 100 0 0 10-66
🔴  web/src/components/Upload/NewUpload 0 100 0 0
🔴   NewUpload.tsx 0 100 0 0 7-35
🔴  web/src/components/Upload/Upload 0 0 0 0
🔴   Upload.stories.tsx 0 100 100 0 5-93
🔴   Upload.tsx 0 0 0 0 11-61
🔴  web/src/components/Upload/UploadCell 0 100 0 0
🔴   UploadCell.tsx 0 100 0 0 7-59
🔴  web/src/components/Upload/UploadForm 0 0 0 0
🔴   UploadForm.tsx 0 0 0 0 23-108
🔴  web/src/components/Upload/UploadValidationButtonGroup 0 0 0 0
🔴   UploadValidationButtonGroup.stories.tsx 0 100 0 0 5-47
🔴   UploadValidationButtonGroup.tsx 0 0 0 0 24-57
🔴  web/src/components/Upload/UploadValidationResultsTable 42.85 100 100 42.85
🔴   UploadValidationResultsTable.stories.tsx 0 100 100 0 6-41
🟢   UploadValidationResultsTable.tsx 100 100 100 100
🔴  web/src/components/Upload/UploadValidationStatus 0 0 0 0
🔴   UploadValidationStatus.tsx 0 0 0 0 3-30
🔴  web/src/components/Upload/Uploads 0 0 0 0
🔴   Uploads.tsx 0 100 0 0 9-17
🔴   columns.tsx 0 0 0 0 7-67
🔴  web/src/components/Upload/UploadsCell 0 100 0 0
🔴   UploadsCell.tsx 0 100 0 0 8-55
🔴  web/src/components/User/EditUserCell 0 100 0 0
🔴   EditUserCell.tsx 0 100 0 0 10-58
🔴  web/src/components/User/NewUser 0 100 0 0
🔴   NewUser.tsx 0 100 0 0 9-32
🔴  web/src/components/User/User 0 0 0 0
🔴   User.tsx 0 0 0 0 10-94
🔴  web/src/components/User/UserCell 0 100 0 0
🔴   UserCell.tsx 0 100 0 0 7-30
🔴  web/src/components/User/UserForm 0 0 0 0
🔴   UserForm.tsx 0 0 0 0 28-202
🔴  web/src/components/User/Users 0 100 0 0
🔴   Users.tsx 0 100 0 0 9-24
🔴  web/src/components/User/UsersCell 0 100 0 ...[Comment body truncated]

terraform/functions.tf Outdated Show resolved Hide resolved
}

module "lambda_function-treasuryReportGeneration" {
for_each = toset(["1A", "1B", "1C"])
Copy link
Member

@TylerHendrickson TylerHendrickson Jul 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the extreme similarity between each of the Lambda functions generated by this module definition, I think it would be better to eliminate the for_each and define a single Lambda function for report generation.

Unless there's a clear operational benefit for having distinct functions (or clear detriment to having them combined), I think merging is justified given that each Lambda in the for_each shares the following criteria:

  1. Invocation source(s)
  2. Execution policy
  3. Handler callable (NB: inspecting a payload value feels like a more appropriate way to guide 1A/B/C selection than an env var)
  4. Runtime configuration, including env vars (other than PROJECT_USE_CODE, but see the previous point)
  5. Capacity requirements (the invocation scenario always calls all of these, so it seems unlikely that concurrency config would need to be independently adjusted)

Feel free to push back, but I don't personally see this otherwise being an example of a "monolithic Lambda".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keeping this lambda the same makes sense to me. The advantage I saw was mainly for observability in datadog. By filtering by function ARN it is easy to know whether the Project 1A/1B/1C files are failing to generate as opposed to adding a secondary filter on payload.

Since we can always add additional arguments to the log-context I'm good with making this change and simplifying this setup to have a single lambda.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see the need for different capacity requirements. I assume there are more of one type of project to generate over time, but I don't have any data at the moment to base this off-of so we can always separate it out when we notice this in the future.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see the need for different capacity requirements. I assume there are more of one type of project to generate over time, but I don't have any data at the moment to base this off-of so we can always separate it out when we notice this in the future.

FWIW, I think it will be a while before we're scaled to the point where we'll need to tune concurrency for these Lambda functions, the Lambda + SFN services should be able to work out scheduling executions on their own to a relatively high degree of scale.

Comment on lines 21 to 22
"Payload.$" : "$",
"FunctionName" : module.lambda_function-treasuryReportGeneration["1A"].lambda_function_arn
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my previous comment; I think it would be better to inject a "report type" parameter into the payload and have FunctionName point to module.lambda_function-treasuryReportGeneration.lambda_function_arn.

Comment on lines 49 to 50
"Payload.$" : "$",
"FunctionName" : module.lambda_function-treasuryReportGeneration["1B"].lambda_function_arn
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(see above)

Comment on lines 77 to 78
"Payload.$" : "$",
"FunctionName" : module.lambda_function-treasuryReportGeneration["1C"].lambda_function_arn
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(see above)


service_integrations = {
lambda = {
lambda = [module.lambda_function-treasuryReportGeneration["1A"].lambda_function_arn, module.lambda_function-treasuryReportGeneration["1B"].lambda_function_arn, module.lambda_function-treasuryReportGeneration["1C"].lambda_function_arn, module.lambda_function-subrecipientTreasuryReportGen.lambda_function_arn]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(see above)

Suggested change
lambda = [module.lambda_function-treasuryReportGeneration["1A"].lambda_function_arn, module.lambda_function-treasuryReportGeneration["1B"].lambda_function_arn, module.lambda_function-treasuryReportGeneration["1C"].lambda_function_arn, module.lambda_function-subrecipientTreasuryReportGen.lambda_function_arn]
lambda = [
module.lambda_function-treasuryReportGeneration.lambda_function_arn,
module.lambda_function-subrecipientTreasuryReportGen.lambda_function_arn,
]

Base automatically changed from 348-treasury-report-unblock-testing-of-file-generation-in-staging-environment-for-usdr_admin-users to main July 30, 2024 01:44
@as1729 as1729 enabled auto-merge (squash) July 31, 2024 02:33
Copy link
Member

@TylerHendrickson TylerHendrickson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@as1729 One follow-up comment for you to consider; I disabled auto-merge for this PR in case you want to make any more changes, but if my suggestion doesn't apply, consider this approved to merge.

@@ -76,7 +75,7 @@ def handle(event: S3Event, context: Context):

s3_client: S3Client = boto3.client("s3")

project_code = os.getenv("PROJECT_USE_CODE")
project_code = event["ProjectType"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recommend binding this to the logging context so that it's easy to filter in (Datadog) log analysis. I realize that the entire invocation event is already bound, so that could be fine, although I'd recommend ensuring that the invocation event is fairly small if the whole thing is going to be included in every log message.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, I'll update to remove the entire context in a separate PR since there's a few more changes I'm seeing that may be needed to that code.

@as1729 as1729 merged commit 2cde648 into main Jul 31, 2024
22 checks passed
@as1729 as1729 deleted the 348/add-terraform branch July 31, 2024 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants