cloudposse / terraform-aws-lambda-function

A module for launching Lambda Fuctions
https://cloudposse.com/accelerate
Apache License 2.0
30 stars 40 forks source link

v0.6.0 breaks `aws_cloudwatch_log_group` names #78

Closed MaxymVlasov closed 2 weeks ago

MaxymVlasov commented 2 weeks ago

Describe the Bug

74 actually breaks aws_cloudwatch_log_group names

# module.lambda_cloudtrail_lookup[0].module.cloudwatch_log_group.aws_cloudwatch_log_group.default[0] must be replaced
-/+ resource "aws_cloudwatch_log_group" "default" {
      ~ arn               = "arn:aws:logs:us-east-1:012345678910:log-group:/aws/lambda/compromised-keys-cloudtrail-lookup" -> (known after apply)
      ~ id                = "/aws/lambda/compromised-keys-cloudtrail-lookup" -> (known after apply)
      ~ log_group_class   = "STANDARD" -> (known after apply)
      ~ name              = "/aws/lambda/compromised-keys-cloudtrail-lookup" -> "namespace-environment-stage-/aws/lambda/compromised-keys-cloudtrail-lookup" # forces replacement
      + name_prefix       = (known after apply)
      ~ tags              = {
          ~ "Name"                         = "namespace-environment-stage-compromised-keys" -> "namespace-environment-stage-/aws/lambda/compromised-keys-cloudtrail-lookup"
        }
      ~ tags_all          = {
          ~ "Name"                         = "namespace-environment-stage-compromised-keys" -> "namespace-environment-stage-/aws/lambda/compromised-keys-cloudtrail-lookup"
            # (15 unchanged elements hidden)
        }
        # (2 unchanged attributes hidden)
    }

  # module.lambda_delete_key[0].module.cloudwatch_log_group.aws_cloudwatch_log_group.default[0] must be replaced
-/+ resource "aws_cloudwatch_log_group" "default" {
      ~ arn               = "arn:aws:logs:us-east-1:012345678910:log-group:/aws/lambda/compromised-keys-delete-key" -> (known after apply)
      ~ id                = "/aws/lambda/compromised-keys-delete-key" -> (known after apply)
      ~ log_group_class   = "STANDARD" -> (known after apply)
      ~ name              = "/aws/lambda/compromised-keys-delete-key" -> "namespace-environment-stage-/aws/lambda/compromised-keys-delete-key" # forces replacement
      + name_prefix       = (known after apply)
      ~ tags              = {
          ~ "Name"                         = "namespace-environment-stage-compromised-keys" -> "namespace-environment-stage-/aws/lambda/compromised-keys-delete-key"
        }
      ~ tags_all          = {
          ~ "Name"                         = "namespace-environment-stage-compromised-keys" -> "namespace-environment-stage-/aws/lambda/compromised-keys-delete-key"
            # (15 unchanged elements hidden)
        }
        # (2 unchanged attributes hidden)
    }

  # module.lambda_notify_security[0].module.cloudwatch_log_group.aws_cloudwatch_log_group.default[0] must be replaced
-/+ resource "aws_cloudwatch_log_group" "default" {
      ~ arn               = "arn:aws:logs:us-east-1:012345678910:log-group:/aws/lambda/compromised-keys-notify-security" -> (known after apply)
      ~ id                = "/aws/lambda/compromised-keys-notify-security" -> (known after apply)
      ~ log_group_class   = "STANDARD" -> (known after apply)
      ~ name              = "/aws/lambda/compromised-keys-notify-security" -> "namespace-environment-stage-/aws/lambda/compromised-keys-notify-security" # forces replacement
      + name_prefix       = (known after apply)
      ~ tags              = {
          ~ "Name"                         = "namespace-environment-stage-compromised-keys" -> "namespace-environment-stage-/aws/lambda/compromised-keys-notify-security"
        }
      ~ tags_all          = {
          ~ "Name"                         = "namespace-environment-stage-compromised-keys" -> "namespace-environment-stage-/aws/lambda/compromised-keys-notify-security"
            # (15 unchanged elements hidden)
        }
        # (2 unchanged attributes hidden)
    }

Plan: 3 to add, 0 to change, 3 to destroy.

Expected Behavior

name should be not changed https://github.com/cloudposse/terraform-aws-lambda-function/pull/74/files#diff-dc46acf24afd63ef8c556b77c126ccc6e578bc87e3aa09a931f33d9bf2532fbbR15

Possible solution: Provide all context variables except name

Steps to Reproduce

  1. Create https://github.com/cloudposse/terraform-aws-lambda-function/blob/main/examples/complete/main.tf on v0.5.5
  2. Update to v0.6.0

Screenshots

No response

Environment

No response

Additional Context

No response

MaxymVlasov commented 2 weeks ago

@milldr also I'm curious - do you have tests that check upgrade (print out tf plan changes) from current version to changes in PR, or only test that PR pass creation-removal resources tests?

milldr commented 2 weeks ago

@milldr also I'm curious - do you have tests that check upgrade (print out tf plan changes) from current version to changes in PR, or only test that PR pass creation-removal resources tests?

In most modules we do, but looks like we don't here. I'll revert the last release

nnsense commented 2 weeks ago

To be honest, I think hardcoding /aws/lambda/ is creating issues. I understand that's by default what lambda does if you create it on the UI but we have the freedom to allow the user to chose their pattern if they wish. I will propose a new PR trying to deal with that, let me give another go :)

milldr commented 2 weeks ago

I've opened PR #79 to revert the changes and add tests and necessary outputs to catch any breaking changes going forward