-
Notifications
You must be signed in to change notification settings - Fork 90
feat: Terraform example #1458
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
feat: Terraform example #1458
Conversation
To install Terraform if you don't have it yet, you can follow the [Install Terraform Guide](https://developer.hashicorp.com/terraform/downloads?product_intent=terraform). | ||
|
||
## Configuration | ||
Serverless Framework uses [serverless.yml](./serverless.yml) to define the application's AWS resources. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
terraform...
} | ||
|
||
# Allows API gateway to invoke lambda | ||
resource "aws_lambda_permission" "hello_world_lambda_testinvoke" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is this test-invoke-stage ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Required stage to test the lambda invocation from the API Gateway console. Can be removed, but CDK example deploys similar permissions, including that test-invoke-stage.
https://awscli.amazonaws.com/v2/documentation/api/latest/reference/apigateway/test-invoke-method.html
} | ||
|
||
# Allows API gateway to invoke lambda | ||
resource "aws_lambda_permission" "hello_world_stream_lambda_testinvoke" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here?
variables = { | ||
POWERTOOLS_LOG_LEVEL = "INFO" | ||
POWERTOOLS_LOGGER_SAMPLE_RATE = "0.1" | ||
POWERTOOLS_LOGGER_LOG_EVENT = "true" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove this line, we actually don't handle this env var...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Terraform example is based on the CDK example configuration which include these environment variables. Are you sure?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, we need to cleanup all the examples (#1444)
variables = { | ||
POWERTOOLS_LOG_LEVEL = "INFO" | ||
POWERTOOLS_LOGGER_SAMPLE_RATE = "0.7" | ||
POWERTOOLS_LOGGER_LOG_EVENT = "true" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove this line too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few changes to do. I'll need to test also...
Codecov ReportAll modified lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #1458 +/- ##
=========================================
Coverage 78.58% 78.58%
Complexity 650 650
=========================================
Files 74 74
Lines 2508 2508
Branches 259 259
=========================================
Hits 1971 1971
Misses 455 455
Partials 82 82 ☔ View full report in Codecov by Sentry. |
@@ -0,0 +1,4 @@ | |||
# terraform modules |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do we know that we haven't broken this - can we lint it as part of the build?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could we do a terraform validate
at least ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is also this: https://github.com/terraform-linters/setup-tflint. Should we add it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@scottgerring we should something similar for SAM and the others... sam build
/ sam validate --lint --template template.yml
(for another issue/pr)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok I will add some form of linting/validation
Merge main
…ls-lambda-java into feat/terraform-examples
…ls-lambda-java into feat/terraform-examples
Kudos, SonarCloud Quality Gate passed!
|
Issue #1445:
Description of changes:
Added a core-example with Terraform deployment
Just added a new terraform folder under powertools-examples-core which contain appropriate instructions and template to deploy the sample Lambdas and API Gateway with Terraform
Checklist
Breaking change checklist
RFC issue #:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.