Conversation
|
LGTM, just make sure AWS functions the same when applying :) |
Wait for two reviews before merging though! |
| } | ||
| #1. IAM Role and Security Group for Lambda | ||
| module "lambda_role_and_sg" { | ||
| source = "git::https://github.com/ONSdigital/ons-terraform-modular.git//terraform/lambda/lambda_role_and_sg?ref=KEH-1797-shared-terraform-repo" |
There was a problem hiding this comment.
I think this source name needs to be updated?
From ons-terraform-modular to ons-shared-terraform-repository
Other than that, LGTM otherwise
There was a problem hiding this comment.
I think this source name needs to be updated?
From ons-terraform-modular to ons-shared-terraform-repository
Other than that, LGTM otherwise
The old name seems to still point to the new repo - I guess its still better to have the new name instead
| image_uri = "${var.aws_account_id}.dkr.ecr.${var.region}.amazonaws.com/${var.ecr_repository}@${data.aws_ecr_image.lambda_image.image_digest}" | ||
| # 2. Create the Lambda function | ||
| module "tech_audit_lambda" { | ||
| source = "git::https://github.com/ONSdigital/ons-terraform-modular.git//terraform/lambda?ref=KEH-1797-shared-terraform-repo" |
There was a problem hiding this comment.
parroting on what was mentioned before with the new/old URL
|
|
||
| The Lambda Terraform now composes shared modules for the IAM role/security group and the Lambda resource itself. You still provide the image details via `ecr_repository` and `container_ver` (tag). Optionally, you can pin to an immutable image digest using `container_digest` to avoid tag lookup issues. | ||
|
|
||
| After apply, you can confirm the exact image (including digest) via the `lambda_image_uri` Terraform output. |
There was a problem hiding this comment.
nitpick - "After applying"
delterr
left a comment
delterr
left a comment
There was a problem hiding this comment.
Looks good, just a few nitpicks with regards to URLs
4 participants
What type of PR is this? (check all applicable)
What
Modified the
terraform/lambda/main.tf, outputs.tf and variables.tffiles.Moved resources to a shared repository
https://github.com/ONSdigital/ons-terraform-modular.The "IAM role", "policies" and "security group" are now created by
module.lambda_role_and_sginstead of inline.The "Lambda function" is now created by
module.tech_audit_lambda.lambda_function_arn / lambda_function_nameinstead of inline.Testing
Have any new tests been added as part of this issue? If not, try to explain why test coverage is not needed here.
The modification to the terraform does not have any dependent tests, therefore not require new tests or modification to existing tests.
Documentation
Has any new documentation been written as part of this issue? We should try to keep documentation up to date
as new code is added, rather than leaving it for the future.
Please write a brief description of why documentation is not necessary here.
Related issues
KEH-1797
How to review
check if the terraform changes are up to the tickets acceptance