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

github_team.id can't be used in a for_each expression to create multiple github_team_repository resources #500

Closed
jspiro opened this issue Jun 26, 2020 · 6 comments
Labels
r/team_repository r/team Status: Stale Used by stalebot to clean house Type: Bug Something isn't working as documented
Milestone

Comments

@jspiro
Copy link
Contributor

jspiro commented Jun 26, 2020

Terraform Version

0.12.28

Affected Resource(s)

  • github_team
  • github_team_repository

Terraform Configuration Files

Not working:

data "github_team" "writers" {
  slug = "writer-team"
}

resource "github_team_repository" "writers" {
  for_each   = data.github_team.writers.id
  # or for multiple teams something like:
  # for_each   = { for obj in [data.github_team.writers] : obj.id => obj.id }
  team_id    = each.value
  repository = "repo"
  permission = "push"
}

Working:

data "github_team" "writers" {
  slug = "foo-bar"
}

resource "github_team_repository" "writers" {
  team_id    = data.github_team.writers.id
  repository = "repo"
  permission = "push"
}

Expected Behavior

I've been trying to do a simple thing: Provide a list (whether data or resource) of teams to github_team_repository using 0.12 for_each syntax. I expect that the for_each syntax would work.

Actual Behavior

  on modules/github/repository/main.tf line 58, in resource "github_team_repository" "writers":
  XX:   for_each   = data.github_team.writers.id

The "for_each" value depends on resource attributes that cannot be determined
until apply, so Terraform cannot predict how many instances will be created.
To work around this, use the -target argument to first apply only the
resources that the for_each depends on.

Steps to Reproduce

  1. terraform plan

Important Factoids

I built a team module that handles membership, and a repo module that hooks up those teams to admin/writer/reader roles. Passing one team into the other is a natural thing to do.

But TF cannot seem to get the dependencies right. It doesn't infer that the data needs to be looked up first, or that it's a computed value, or the team needs to be created first (if using github_team resources) when given in a for_each, but it works perfectly if I provide the team id directly.

I've tried to provide the ID a dozen different ways, through different vars, locals, lists, sets, you name it. I've tried elaborate and explicit layers of depends_on in both variables, resources, and the like. The above is the simplest reproduction.

Workaround

If I create the teams first, and then hook them up to the repo, no problem.

But it's not really possible to run this in a CI environment without creating the teams first in one PR, then the repos in another – that's two different PRs, two different applications–I can't get my team to accept the workaround, they'd rather dump terraform.

Ultimately, this is annoying and should work, but maybe I'm missing something obvious. At this point I've taken it personally and have spent hours trying to figure it out 😩

References

Are there any other GitHub issues (open or closed) or Pull Requests that should be linked here? For example:

@anGie44
Copy link
Contributor

anGie44 commented Jun 29, 2020

hi @jspiro, thank you for creating this issue! re:

Provide a list (whether data or resource) of teams to github_team_repository using 0.12 for_each syntax.

using the example you provided, a map created with for_each = { for obj in [data.github_team.writers] : obj.id => obj.id } seems to create the loop as expected 👍 was there another issue you were facing with that syntax?

regarding the error log, however, a for_each = data.github_team.writers.id does not succeed b/c terraform cannot iterate over this singular string id. you can also workaround having to make an additional map by instead dynamically creating a list of team ids if you use the count attribute e.g.

For 1 team:

data "github_team" "writers" {
  slug = "foo-bar"
}

locals {
  teams = [data.github_team.writers.id]
}

resource "github_team_repository" "writers" {
  count = length(local.teams)
  team_id = local.teams[count.index]
  repository = "repo"
  permission = "push"
}

For more than 1 team:

data "github_team" "writers" {
  slug = "foo-bar"
}

data "github_team" "writers_new" {
  slug = "foo-bar-new"
}

locals {
  teams = [data.github_team.writers.id, data.github_team.writers_new.id]
}

resource "github_team_repository" "writers" {
  count = length(local.teams)
  team_id = local.teams[count.index]
  repository = "repo"
  permission = "push"
}

hope this helps! if any further questions arise, let me know 😄

@jspiro
Copy link
Contributor Author

jspiro commented Jun 30, 2020

@anGie44 Hey, thanks so much for the detailed reply! I had not thought to try it with count. I suppose you could then say that my bug report is regarding for_each, as you demonstrated in your example.

It's possibly a bug in terraform, or possibly by design (hashicorp/terraform#23529 or hashicorp/terraform#4149)–unclear. I brought it here in case anyone else had tried it (I imagine many who use this module would have encountered the use case) and had suggestions.

If there isn't anything that can be done in the Go for the provider, then perhaps it would be worth documenting the limitation with for_each and the workaround with count.

What do you think?

@jcudit jcudit added Type: Bug Something isn't working as documented r/team r/team_repository labels Nov 30, 2020
@jcudit jcudit added this to the v4.4.0 milestone Jan 16, 2021
@jcudit
Copy link
Contributor

jcudit commented Jan 26, 2021

Happy to add this limitation / workaround to our docs. Will put something up for review for the next release.

@majormoses
Copy link
Contributor

majormoses commented Feb 13, 2021

To add some further validation to this general approach, here are some similar snippets from our module.

Array of objects that represent a team and the permission to grant:

# snip from permissions.tf in modules/repository/permissions.tf
resource "github_team_repository" "permission" {
  for_each = { for team in var.additional_permissions : team.team_id => team }

  # we need to use an ID but this is a poor user experience to have to look it up.
  # we may want to suggest using local variables rather than the standard .tfvars
  team_id    = each.value.team_id
  repository = github_repository.repo.name
  permission = each.value.permission
}

The caller then passes in a set of maps:

# snip from modules/repository/variables.tf
variable "additional_permissions" {
  description = <<DESCRIPTION
    This is an array of mappings of teams with permissions for the repository
    for example:
    additional_users = [
      {
        "team_id"    = data.github_team.one.id,
        "permission" = "push"
      },
      {
        "team_id"     = data.github_team.two.id,
        "permission"  = "admin"
      },
      {
        "team_id"     = "1234567",
        "permission   = "pull"
      }
    ]
DESCRIPTION
  # we may wish to remove the default in the future
  # we will make the breaking changes later
  default = []
  type = list(
    object(
      {
        # TODO: keep an eye on new expiremental feature that would be nice here
        # https://www.terraform.io/docs/configuration/functions/defaults.html
        team_id    = string,
        permission = string
      }
    )
  )
}

Also another good note is to avoid (where possible) data sources within your modules if you need to instantiate it a bunch of times you will likely exhaust your github api rate limit budget.

@github-actions
Copy link

github-actions bot commented Dec 9, 2022

👋 Hey Friends, this issue has been automatically marked as stale because it has no recent activity. It will be closed if no further activity occurs. Please add the Status: Pinned label if you feel that this issue needs to remain open/active. Thank you for your contributions and help in keeping things tidy!

@github-actions github-actions bot added the Status: Stale Used by stalebot to clean house label Dec 9, 2022
@github-actions github-actions bot removed the Status: Stale Used by stalebot to clean house label Dec 10, 2022
Copy link

👋 Hey Friends, this issue has been automatically marked as stale because it has no recent activity. It will be closed if no further activity occurs. Please add the Status: Pinned label if you feel that this issue needs to remain open/active. Thank you for your contributions and help in keeping things tidy!

@github-actions github-actions bot added the Status: Stale Used by stalebot to clean house label Apr 24, 2024
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
r/team_repository r/team Status: Stale Used by stalebot to clean house Type: Bug Something isn't working as documented
Projects
None yet
Development

No branches or pull requests

6 participants