Skip to content
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

Terraform 0.11 incorrectly deferred data source read; 0.12 now behaves correctly, changing behavior #22730

Closed
dynamike opened this issue Sep 6, 2019 · 5 comments
Labels
bug config v0.11 Issues (primarily bugs) reported against v0.11 releases

Comments

@dynamike
Copy link

dynamike commented Sep 6, 2019

Terraform Version

Terraform v0.12.8
+ provider.aws v2.27.0

Terraform Configuration Files

locals {
  region = "us-west-2"
}

data "aws_ecr_repository" "main" {
  name = "apples"
}

data "aws_iam_policy_document" "main" {
  statement {
    actions = [
      "ecr:PutImage",
    ]

    resources = [
      "${data.aws_ecr_repository.main.arn}",
    ]
  }
}

provider "aws" {
  version = "~> 2.27.0"
  region  = "${local.region}"
}

Expected Behavior

Running terraform plan with Terraform 0.11.14 and identical AWS provider version I get the following plan

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
 <= read (data resources)

Terraform will perform the following actions:

 <= data.aws_ecr_repository.main
      id:                            <computed>
      arn:                           <computed>
      name:                          "apples"
      registry_id:                   <computed>
      repository_url:                <computed>
      tags.%:                        <computed>

 <= data.aws_iam_policy_document.main
      id:                            <computed>
      json:                          <computed>
      statement.#:                   "1"
      statement.0.actions.#:         "1"
      statement.0.actions.977834259: "ecr:PutImage"
      statement.0.effect:            "Allow"
      statement.0.resources.#:       <computed>
      version:                       "2012-10-17"


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

Actual Behavior

In Terraform 0.12.8 incorrectly tries to resolve the data source when running terraform plan

data.aws_ecr_repository.main: Refreshing state...

Error: Null value found in list

  on main.tf line 15, in data "aws_iam_policy_document" "main":
  15:     resources = [
  16:       data.aws_ecr_repository.main.arn,
  17:     ]

Steps to Reproduce

  1. terraform init
  2. terraform apply

Additional Context

This doesn't appear to be an AWS provider issue because it's only affected by the upgrading to 0.12.x. Based on the documentation, terraform shouldn't try to resolve the data source until I go to apply the change. I can't find anywhere in the documentation around a change in behavior for data sources between 0.11.x and 0.12.x. Lastly, hashicorp/terraform-provider-aws#9947 appears to be a related issue, but I disagree with the expected behavior. This should not result in an error.

@cblkwell
Copy link

cblkwell commented Sep 6, 2019

This is definitely an issue I'm running into with some work I'm doing now with https://github.com/trussworks/terraform-aws-ecs-service trying to update it for Terraform 0.12; this prevents the creation of the ECS service when the cluster doesn't exist, which worked with Terraform 0.11.

@hashibot hashibot added bug config v0.12 Issues (primarily bugs) reported against v0.12 releases labels Sep 7, 2019
@teamterraform
Copy link
Contributor

Hi @dynamike,

After reviewing the linked issue hashicorp/terraform-provider-aws#9947, our initial instinct is that this does seem to be a primarily provider bug. The Terraform Core team doesn't have all the context on the behavior of the aws_ecr_repository data resource, but the described behavior reads to us as a provider bug for the following reason:

A data source is expected to either return a result or to fail. Specific data sources may be defined such that an empty result makes sense, such as if they are querying a set of objects based on a filter and the filter matches nothing, but the common case for data resources is to match exactly one object, and that seems to be the intent of aws_ecr_repository too.

The linked issue suggests that instead of failing, this data source is setting arn to null. That in turn means that the value is known during plan (Terraform can see that it's null) and thus the configuration of aws_iam_policy_document is known too, meaning Terraform will attempt to read it during the plan phase.

From a quick skim of the relevant provider code, the bug on the provider side seems to arise from having borrowed some code from the corresponding aws_ecr_repository resource type. For resource types it is correct to signal a missing object without an error so that Terraform can see that the object needs to be recreated, but it was incorrect to carry that logic over to the data resource.


With that said, there is still some odd core behavior unexplained here which we'd like to understand better. In your 0.11 example we can see that data.aws_ecr_repository.main itself was also deferred to the apply phase, which suggests that Terraform 0.11 considered its configuration to be partially-unknown during planning. However, we can see in the configuration you shared that this isn't the case:

data "aws_ecr_repository" "main" {
  name = "apples"
}

name is the only argument being set and it's set to a constant string, so the correct behavior here would be for Terraform to try to read this data source at plan time: it already knows everything that's needed to submit the request.

This therefore feels like a bug in Terraform 0.11 that was inadvertently fixed as a side-effect of other work in 0.12. In order to explain that more fully we'll need to try this on Terraform 0.11 ourselves and read the trace log to understand what caused Terraform to believe it had to defer this data read until apply time.

Our focus is naturally on Terraform 0.12 now, so we likely won't be able to spend further time investigating this apparent Terraform 0.11 bug in the near future.


Unfortunately that doesn't really help with what you are trying to do here: Terraform 0.12 appears to be working as designed, but that design is not aligned with what you were trying to achieve here. Generally-speaking we'd recommend not trying to create and read a particular object within the same configuration, since Terraform currently has no way to guarantee the necessary ordering due to the need to refresh data resources as early as possible for other reasons.

If you do need to create an ECR repository and use it in the same configuration, we'd recommend using the techniques described in the Module Composition guide, passing in the ECR repository object to the module as an object variable rather than having the module try to read it for itself:

resource "aws_ecr_repository" "example" {
  # ...
}

module "uses_ecr" {
  source = "./modules/uses-ecr"

  ecr_repository = aws_ecr_repository.example
}

...and then in the uses-ecr module:

variable "ecr_repository" {
  # Describe the attributes from aws_ecr_repository that the module needs
  type = object({
    arn            = string
    repository_url = string
  })
}

Terraform 0.11's limited type system made it hard to pass around resource objects like this, and so we can see that passing an id and then using a data source to retrieve the full object might've been a compelling workaround. However, now that Terraform 0.12 fully supports passing around resource instances as object values we'd recommend making use of that capability so that Terraform can better understand how data flows through the configuration and thus perform actions in the correct order.

@teamterraform teamterraform added v0.11 Issues (primarily bugs) reported against v0.11 releases and removed v0.12 Issues (primarily bugs) reported against v0.12 releases labels Sep 7, 2019
@teamterraform teamterraform changed the title No longer able to handle dynamic data sources in 0.12 Terraform 0.11 incorrectly deferred data source read; 0.12 now behaves correctly, changing behavior Sep 7, 2019
@cblkwell
Copy link

cblkwell commented Sep 9, 2019

Passing the object to the module is definitely a better plan; doing that fixes the issue I was running into.

@dynamike
Copy link
Author

dynamike commented Sep 9, 2019

I really appreciate the thorough response on this and very much agree that better path forward is to pass the object directly into the module (I didn't know you could do this!). I'm going to close this as the focus on 0.12 is the right call IMO.

@dynamike dynamike closed this as completed Sep 9, 2019
@ghost
Copy link

ghost commented Oct 10, 2019

I'm going to lock this issue 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 similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Oct 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug config v0.11 Issues (primarily bugs) reported against v0.11 releases
Projects
None yet
Development

No branches or pull requests

4 participants