fix: lambda customization config not working

[Dogacel]

The error I was getting:

│ Error: Reference to undeclared resource
│
│   on .terraform/modules/cognito_user_pool/main.tf line 76, in resource "aws_cognito_user_pool" "pool":
│   76:           lambda_arn     = lookup(for_each.value, "lambda_arn", null)
│
│ A managed resource "for_each" "value" has not been declared in
│ module.cognito_user_pool.
╵

So I decided to test a couple things, each.value, each.key etc. but none of them worked.

│ A reference to "each.value" has been used in a context in which it
│ unavailable, such as when the configuration no longer contains the value in
│ its "for_each" expression. Remove this reference to each.value in your
│ configuration to work around this error.

So I just decided to make it consistent with other lambda blocks by copy-pasting the lookup blocks 🙂.

This has been tested in my local.

@lgallard Here is a final fix, sorry I was unavailable yesterday most of the time.

[lgallard]

@Dogacel can you share you definition code? This way I can test it too 🙏🏻

Thanks in advance!

[Dogacel]

@lgallard I'm sorry it is pretty complicated and contains lots of company production configuration 🙂 Different ssm parameters, local values, client configurations, iam coming from custom modules etc.

Here is our block for only lambda config,

  lambda_config = {
    pre_token_generation_config = {
      lambda_arn     = data.aws_lambda_function.cognito_access_token_customization.arn
      lambda_version = "V2_0"
    }
    user_migration      = data.aws_lambda_function.cognito_user_migration.arn
    post_authentication = data.aws_lambda_function.cognito_post_authentication_trigger.arn
    post_confirmation   = data.aws_lambda_function.cognito_post_confirmation_trigger.arn
    custom_sms_sender = {
      lambda_arn     = data.aws_lambda_function.cognito_custom_sms_sender_trigger.arn,
      lambda_version = "V1_0"
    }
    custom_email_sender = {
      lambda_arn     = data.aws_lambda_function.cognito_custom_email_sender_trigger.arn,
      lambda_version = "V1_0"
    }
    kms_key_id = aws_kms_key.cognito_custom_sender.arn
  }

[lgallard]

@Dogacel I was checking your PR and I would like to have another approach:

dynamic "pre_token_generation_config" {
  for_each = lookup(var.lambda_config, "pre_token_generation_config", null) != null ? [var.lambda_config["pre_token_generation_config"]] : []
  content {
    lambda_arn     = for_each.value["lambda_arn"]
    lambda_version = for_each.value["lambda_version"]
  }
}

This way if var.lambda_config exists (i.e., it is not null), the map itself is wrapped in a list to make it compatible with for_each, which requires a collection. If it doesn't exist, an empty list is used, effectively skipping the generation of any content block.

Inside the content block, the values for lambda_arn and lambda_version are accessed directly using the map keys from for_each.value. This direct access is safer because the for_each construction ensures that the block is only processed if the pre_token_generation_config actually exists. This prevents null reference errors that might occur if pre_token_generation_config was not provided.

This approach to managing pre_token_generation_config enhances clarity and maintainability. By only creating settings when needed, we avoid errors and keep the code straightforward. This makes it easier to handle and update, reducing the chance of mistakes.

Does it make sense to you?

[Dogacel]

@Dogacel I was checking your PR and I would like to have another approach:

dynamic "pre_token_generation_config" {
  for_each = lookup(var.lambda_config, "pre_token_generation_config", null) != null ? [var.lambda_config["pre_token_generation_config"]] : []
  content {
    lambda_arn     = for_each.value["lambda_arn"]
    lambda_version = for_each.value["lambda_version"]
  }
}

This way if var.lambda_config exists (i.e., it is not null), the map itself is wrapped in a list to make it compatible with for_each, which requires a collection. If it doesn't exist, an empty list is used, effectively skipping the generation of any content block.

Inside the content block, the values for lambda_arn and lambda_version are accessed directly using the map keys from for_each.value. This direct access is safer because the for_each construction ensures that the block is only processed if the pre_token_generation_config actually exists. This prevents null reference errors that might occur if pre_token_generation_config was not provided.

This approach to managing pre_token_generation_config enhances clarity and maintainability. By only creating settings when needed, we avoid errors and keep the code straightforward. This makes it easier to handle and update, reducing the chance of mistakes.

Does it make sense to you?

You can try it however I don't think it will work. I tested myself before,

https://developer.hashicorp.com/terraform/language/meta-arguments/for_each

First, it is not called for_each.value, right? It should be each.value. And as I explained in my PR description, each argument doesn't work for whatever reason.

Based on https://developer.hashicorp.com/terraform/language/expressions/dynamic-blocks, we should use pre_token_generation_config["lambda_arn"] as an example.

However the thing I noticed is style of this code doesn't match with the rest. That seems like a tech debt 🙂 So I decided to go with the consistent approach, wdy think?

[lgallard]

@Dogacel You are right about the wrong reference, it should be each.value and each.key.

You also right about the consistency in code, it might be odd and this refactoring can be treated in a another issue / PR.

I'm going to merge your PR instead as it was already tested on your side.

Thanks for the feedback!