Terraform: GitHub users being re-created

Created on 20 Dec 2016  ยท  10Comments  ยท  Source: hashicorp/terraform

Terraform Version

Terraform v0.8.1

Affected Resource(s)

  • github_membership

Terraform Configuration Files

variable "members" {
    type = "list"
    default = [
        "user1",
        "user2",
        "user3"
    ]
}

resource "github_membership" "member" {
    count = "${length(var.members)}"
    username = "${element(var.members, count.index)}"
    role = "member"
}

Debug Output

-/+ github_membership.member.28
    role:     "member" => "member"
    username: "user2" => "user3" (forces new resource)

Expected Behavior

Existing user resources are not being re-created.

Actual Behavior

Existing user resources are being re-created.

Steps to Reproduce

Please list the steps required to reproduce the issue, for example:

  1. terraform plan

Other

The issue is due to how the resource identifier are created (based on count). Hence if the array index changes it will remove and add all users.

config enhancement

Most helpful comment

This is working as intended, but it is of course not doing what you want, so I'm sorry about that... :confounded:

To address this we will need a core feature to create a resource for each item in an array, so this is more general than just this github user resource. In some other issue (which I will dig up later when I have more time to dig in) we were talking about something like this:

resource "github_membership" "member" {
    foreach = "${var.members}"
    username = "${element(var.members, count.index)}"
    role = "member"
}

By having first-class support for this Terraform would be able to remember the relationships between the values in the list and the resources it created, rather than doing it purely by index.


For now I would suggest that rather than trying to do this with a single resource block and a variable you instead make a little program that generates the Terraform configuration, with one resource block for each user named after that user. I do something very similar at work and it does the job just fine... since the resources are named after the users, adding and removing users just affects single resources rather than causing the entire list to offset.

You can generate JSON files (named .tf.json) as a simpler alternative to generating HCL.

All 10 comments

This is working as intended, but it is of course not doing what you want, so I'm sorry about that... :confounded:

To address this we will need a core feature to create a resource for each item in an array, so this is more general than just this github user resource. In some other issue (which I will dig up later when I have more time to dig in) we were talking about something like this:

resource "github_membership" "member" {
    foreach = "${var.members}"
    username = "${element(var.members, count.index)}"
    role = "member"
}

By having first-class support for this Terraform would be able to remember the relationships between the values in the list and the resources it created, rather than doing it purely by index.


For now I would suggest that rather than trying to do this with a single resource block and a variable you instead make a little program that generates the Terraform configuration, with one resource block for each user named after that user. I do something very similar at work and it does the job just fine... since the resources are named after the users, adding and removing users just affects single resources rather than causing the entire list to offset.

You can generate JSON files (named .tf.json) as a simpler alternative to generating HCL.

@apparentlymart Thanks for the explanation and the suggestion of a workaround. I guess foreach vs count makes sense. Another option would be to be able to specify the resource identifier, but i guess foreach is simpler and less error prone.

@apparentlymart Do you know if there is work going on related to "foreach" implementation? I wonder because I can't find discussions or open issues on github, though the problem seems rather popular.

+1 for foreach!

I have the exact same issue. When I remove someone or re-order the list it re-creates all user information. I just tell my team that it is time for a key rotation :) However would like this not to happen at all.

@itsguy I have just implemented what @apparentlymart has mentioned (json generator), but still having native foreach would be nice to have. Look here - https://github.com/antonbabenko/cd-terraform-demo/commit/760db4a8a6abe0a44ee837bbb16a310df01b954e#diff-9f268eefbfc06c07ae41cf7fb7d548a7R4

@antonbabenko Thanks for the pointer. I am trying to stick to my principals of not generating terraform code. If terraform cannot manage it I will look to ansible. Its a fine line on some of this 'Infrastructure' stuff.... but fits the standard configuration management model well.

There is another workaround, which is more painful but in may case a good solution.

On a large project, one should put everything inside of a module and than create a TF file for each instance of the module.

In the end of the day some script will have to be written to generate terraform files, but the entries should be simpler, which means the script will be simpler, the template will be simpler and in an emergency one can easily create/modify the output manually.

Hi @antonbabenko... sorry I didn't catch your earlier question about implementation of foreach.

At the time I wrote that I was an open source contributor to Terraform. Now I'm a HashiCorp employee on the Terraform team, so I can speak with more certainty on what's going on.

Since there were quite a lot of assorted usability issues with the configuration language, rather than trying to tackle each in isolation we decided to collect as many of them as possible and try to come up with a holistic set of changes that would solve many of the problems with as little additional complexity as possible.

The foreach idea was in that set, but we learned during some prototyping that some other work needs to happen first before that can be done reasonably, since Terraform's current support for lists and maps is weak. Therefore some more fundamental work needs to happen first on how we deal with value types internally, which is the next thing we plan to work on in this area. Once things are working well, we should be able to get foreach working much more robustly, but unfortunately it's going to take us some time to get there so I can only ask for your patience while we work through it.


In the mean time, along with my earlier suggestion of generating config we've seen users have success making use of modules to minimize repetition without generated code. Rather than making an array and then using count with it to produce multiple instances of the same resource, this pattern instead puts the resource (potentially multiple resources) in a child module and then instantiates it multiple times in the root:

module "membership_apparentlymart" {
  source   = "./github_membership"
  username = "apparentlymart"
}
module "membership_antonbabenko" {
  source   = "./github_membership"
  username = "antonbabenko"
}
module "membership_marcosdiez" {
  source   = "./github_membership"
  username = "marcosdiez"
}

This formulation is not as concise as the list of usernames, but it's more compact than repeating the entire resource "github_membership" block for each. Since modules are retained in state by their names rather than by indices, adding and removing module instances can be done without disturbing other instances.

(I think this may be what @marcosdiez was suggesting above, though without the code generation step. The hope is that the module blocks are compact enough that manually writing them out is not _as_ arduous as it would be to write out the entire resource blocks each time.)

I'm well aware that this is not an ideal state of affairs, and we do still intend to make this more concisely expressible, but I hope either this approach or the code generation approach are useful in the mean time to address some of these limitations.

We're using #19291 as the canonical issue to represent the for_each feature now, so I'm going to close this just to consolidate over there.

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.

Was this page helpful?
0 / 5 - 0 ratings