terraform-aws-modules / terraform-aws-app-runner

Terraform module to create AWS App Runner resources πŸ‡ΊπŸ‡¦
https://registry.terraform.io/modules/terraform-aws-modules/app-runner/aws
Apache License 2.0
33 stars 19 forks source link

fix: Inconsistent conditional result types #6

Closed diomalta closed 1 year ago

diomalta commented 1 year ago

Description

This pull request aims to fix an issue when configuring a custom access role for AWS App Runner, where the Terraform merge function would return an "Inconsistent conditional result types fail for complex types" error. This occurred because the possibility of adding more attributes in a map was not being handled properly.

{
  "@level": "error",
  "@message": "Error: Inconsistent conditional result types",
  "diagnostic": {
    "severity": "error",
    "summary": "Inconsistent conditional result types",
    "detail": "The true and false result expressions must have consistent types. The 'true' value includes object attribute \"authentication_configuration\", which is absent in the 'false' value.",
    "snippet": {
      "context": "locals",
      "code": "  source_configuration = local.create_access_iam_role ? merge(\n    var.source_configuration,\n    { authentication_configuration = { access_role_arn = aws_iam_role.access[0].arn } }\n  ) : var.source_configuration",
      "values": [
        {
          "traversal": "aws_iam_role.access[0].arn",
          "statement": "is a string"
        },
        {
          "traversal": "local.create_access_iam_role",
          "statement": "is true"
        },
        {
          "traversal": "var.source_configuration",
          "statement": "is object with 2 attributes"
        }
      ]
    }
  }
}

Motivation and Context

This change is required to ensure that users can configure custom access roles for AWS App Runner without encountering errors during the merge function. By handling the addition of more attributes in a map, the Terraform configuration will now function correctly and prevent the aforementioned error.

Breaking Changes

There are no breaking changes in this pull request. The changes are solely focused on resolving the issue mentioned above.

How Has This Been Tested?

To test these changes, I started with an existing example from the examples/* folder and deployed it. Then, I applied my changes and checked it against the deployed example to ensure there were no breaking or disruptive changes. Once verified, I deployed the changes again to validate their functionality. Finally, I executed pre-commit run -a on my pull request to ensure compliance with pre-commit standards.

bryantbiggs commented 1 year ago

Please provide a reproduction that demonstrates the issue first so we can discuss any changes

diomalta commented 1 year ago

I'm sorry, first time...

This is sample code to demonstrate the problem:

module "app_runner_private_image_base" {
  source  = "terraform-aws-modules/app-runner/aws"
  version = "1.2.0"

  service_name = "test-private-image-base"

  source_configuration = {
    auto_deployments_enabled = false
    image_repository = {
      image_configuration = {
        port = 8000
        runtime_environment_variables = {
          MY_VARIABLE = "hello!"
        }
      }
      image_repository_type = "ECR"
      image_identifier      = "123456789012.dkr.ecr.us-east-1.amazonaws.com/example-server:latest"
    }
  }

  create_access_iam_role = true
  private_ecr_arn        = "arn:aws:ecr:us-east-1:123456789012:repository/example-server"

  auto_scaling_configurations = {
    mini = {
      name            = "mini"
      max_concurrency = 20
      max_size        = 5
      min_size        = 1

      tags = {
        Type = "Mini"
      }
    }
  }
}

The output of the Terraform plan:

 Error: Inconsistent conditional result types
β”‚ 
β”‚   on .terraform/modules/app_runner_private_image_base/main.tf line 17, in locals:
β”‚   17:   source_configuration = local.create_access_iam_role ? merge(
β”‚   18:     var.source_configuration,
β”‚   19:     { authentication_configuration = { access_role_arn = aws_iam_role.access[0].arn } }
β”‚   20:   ) : var.source_configuration
β”‚     β”œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€
β”‚     β”‚ aws_iam_role.access[0].arn is a string
β”‚     β”‚ local.create_access_iam_role is true
β”‚     β”‚ var.source_configuration is object with 2 attributes
β”‚ 
β”‚ The true and false result expressions must have consistent types. The 'true' value includes object attribute "authentication_configuration", which is absent in the 'false' value.

Provider version (AWS): 4.62.0

Is more information needed?

github-actions[bot] commented 1 year ago

This PR has been automatically marked as stale because it has been open 30 days with no activity. Remove stale label or comment or this PR will be closed in 10 days

kevcube commented 1 year ago

this can be solved by #7 if we default the property authentication_configuration in that object to null, right? or maybe an empty map.

IIRC this error message comes from terraform defaulting to deep-merging maps, not sure if it's even configurable to not deep merge.

kevcube commented 1 year ago

tested, authentication_configuration = null does fix this

lays147 commented 1 year ago

Had the same issue and @kevcube solution worked.

github-actions[bot] commented 1 year ago

This PR has been automatically marked as stale because it has been open 30 days with no activity. Remove stale label or comment or this PR will be closed in 10 days

github-actions[bot] commented 1 year ago

This PR was automatically closed because of stale in 10 days

github-actions[bot] commented 1 year ago

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.