fix: Add null/label context tags to the aws_lambda_function resource #44
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Gowiem
previously approved these changes
Oct 9, 2023
|
/terratest |
Gowiem
previously approved these changes
Oct 9, 2023
|
/terratest |
Gowiem
added
patch
Gowiem
reviewed
Oct 9, 2023
Gowiem
reviewed
Oct 9, 2023
Problem:
Prior to this, the `aws_lambda_function` resource was not getting tagged at all
when passing just the null/label context into the module.
For example, this would end up with a completely untagged Lambda function even
though I am passing the context from a standard null/label declaration:
```
module "test" {
source = "cloudposse/lambda-function/aws"
version = "0.5.1"
function_name = "${module.this.id}-test"
attributes = ["foo"]
description = var.lambda_description
s3_bucket = var.lambda_s3_bucket
s3_key = var.lambda_s3_key
runtime = var.lambda_runtime
handler = var.lambda_handler
context = module.this.context
}
```
To get any tags on the lambda, the `tags` attribute must be used:
```
module "test" {
source = "cloudposse/lambda-function/aws"
version = "0.5.1"
function_name = "${module.this.id}-test"
attributes = ["foo"]
description = var.lambda_description
s3_bucket = var.lambda_s3_bucket
s3_key = var.lambda_s3_key
runtime = var.lambda_runtime
handler = var.lambda_handler
context = module.this.context
tags = module.this.tags
}
```
The requirement of passing an explicit `tags` attribute is not how other
CloudPosse modules work. In most other CloudPosse modules, you can just pass
around a null/label context and the tags will automatically show up in child
resources.
Solution:
Use `tags = module.this.tags` on the `aws_lambda_function` resource.
Outcome:
* The `aws_lambda_function` resource is tagged with the implicit tags passed in via `context`.
* Tags from the `tags` variable are still present, but are now merged with the tags from `context`.
* This module follows the convetion of other CloudPosse modules.
* People used to CloudPosse modules will have an easier time using this module.
|
/terratest |
milldr
approved these changes
Oct 11, 2023
|
/terratest |
|
Thanks @milldr 🍻 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
Use
tags = module.this.tagson theaws_lambda_functionresource.Why
Prior to this, the
aws_lambda_functionresource was not getting tagged at allwhen passing just the null/label context into the module.
For example, this would end up with a completely untagged Lambda function even
though I am passing the context from a standard null/label declaration:
To get any tags on the lambda, the
tagsattribute must be used:This has a couple of problems:
attributeslist is missing from the resultant set of tags.tagsattribute is not how other CloudPosse modules work.Outcome
aws_lambda_functionresource is tagged with the implicit tags passed in viacontext.tagsvariable are still present, but are now merged with the tags fromcontext.