integrations / terraform-provider-github

Terraform GitHub provider
https://www.terraform.io/docs/providers/github/
MIT License
888 stars 726 forks source link

[BUG]: `github_team_members` Fails to Differentiate between Members and Child Team Members #2004

Open GarrettBlinkhorn opened 10 months ago

GarrettBlinkhorn commented 10 months ago

Expected Behavior

I am currently importing an existing set of Github teams into Terraform management using this provider. After I import the team to my github_team_members resource, I expect that the terraform plan will return No changes required. because I have defined a members variable which assigns all of the relevant team members and their roles to the resource.

No changes required. is therefore the happy state which indicates a successful import of the existing team into Terraform management.

Actual Behavior

This bug only occurs in a situation where you have a child team that has members that are not members of the parent team.

For clarity and as an example, ParentTeam contains UserA, UserB and ChildTeam contains UserA, UserC.

In the above case, the terraform plan actually returns a plan indicating that Terraform intends to remove UserC from the github_team_members.ParentTeam resource.

This is unexpected because UserC has never been a member of ParentTeam so they shouldn't need to be removed. If you accept the plan and carry on with a terraform apply to confirm the removal, Terraform will make the modifications like such:

module.MyModule.module.MyTeam.github_team_members.members: Modifying... [id=4076768]
module.MyModule.module.MyTeam.github_team_members.members: Modifications complete after 1s [id=4076768]

However, a subsequent terraform plan will still return the original result: a plan which will attempt to remove the UserC from ParentTeam. Therefore, it is impossible to use this resource to properly manage a parent team when one of the child teams has members that do not exist in the parent team, as you end up stuck with a plan trying to remove an individual membership that doesn't actually exist and thus you can never reach the No changes required happy state.


Through my own attempts to debug this issue, I've learned the following:

Terraform Version

Terraform v1.5.2 MacOS Ventura 13.6

github = {
  source  = "integrations/github"
  version = "~> 5.0"
}

Affected Resource(s)

Terraform Configuration Files

variable "name" {
  description = "The name of the Github Team"
  type = string
}

variable "members" {
  description = "A map of Github User IDs to Github Team Roles"
  type = map(string)
}

variable "description" {
  description = "(Optional) A description to associate with the Github team"
  type = string
  default = ""
}

variable "privacy" {
  description = "(Optional) The level of privacy for the team. Must be one of 'secret' or 'closed'"
  type = string
  default = "closed"
}

variable "parent_team_id" {
  description = "(Optional) The ID or slug of the parent team, if this is a nested team"
  type = string
  default = null
}

resource "github_team" "team" {
  name = var.name
  description  = var.description
  privacy = var.privacy
  parent_team_id = var.parent_team_id
}

resource "github_team_members" "members" {
  team_id = github_team.team.id

  dynamic "members" {
    for_each = var.members
    content {
      username = members.key
      role = members.value
    }
  }

}

Steps to Reproduce

First, create two Github teams (a Parent and a Child subteam) where the Child subteam has a team member that is not a member of the Parent team. For example, ParentTeam contains UserA, UserB and ChildTeam contains UserA, UserC.

Then, run a terraform plan against ParentTeam. The plan should return an attempt to remove UserC from ParentTeam as described above. I've included my configuration files so I hope that reproducing this error is not too much effort.

Debug Output

These logs correspond to the provided example:

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # module.MyModule.module.MyTeam.github_team_members.members will be updated in-place
  ~ resource "github_team_members" "members" {
        id      = "ParentTeam"
        # (2 unchanged attributes hidden)

      - members {
          - role     = "member" -> null
          - username = "UserC" -> null
        }

        # (8 unchanged blocks hidden)
    }

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

Panic Output

No response

Code of Conduct

github-actions[bot] commented 10 months ago

👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labled with Status: Up for grabs. You & others like you are the reason all of this works! So thank you & happy coding! 🚀

GarrettBlinkhorn commented 9 months ago

Hey @kfcampbell thanks for triaging this and I can see its made it in to your backlog. Can definitely appreciate that your team has quite a few other items in there as well - can you give me a sense of when/if someone might be able to take a closer look at this?

This is part of an on-going body of work that my team is engaged in, so it would be nice to know if we can wait until your team is able to resolve this issue or if we should start brainstorming another approach to achieve our goals. I'm not looking for a specific commitment so much as to get a better idea of how incoming work moves through your backlog

o-sama commented 9 months ago

I'm abroad right now and it's almost midnight, but just wanted to throw this out here in case it helps with the issue.

IIRC the GitHub API itself (backend, not go-github client) doesn't have a feature to choose whether or not the team members are direct team members or child team members through the list members endpoint. It was available through the deprecated endpoint I think but no longer is supported. Not sure if the UI makes a call in the backend to the GraphQL API but that might be the case. Please don't go off my word without double checking but I just wanted to point you in the right direction (hopefully).

kfcampbell commented 9 months ago

@o-sama thank you for the thoughts!

@GarrettBlinkhorn unfortunately GitHub's SDK team does not have the bandwidth now to prioritize and pick up new features. Community discussion and PRs are very much appreciated!

murukesh-mohanan-paidy commented 1 month ago

@kfcampbell isn't this the same as #1193, which was marked as completed via #1786? Am I missing something?

kfcampbell commented 1 month ago

@murukesh-mohanan-paidy I believe it is, thanks for bringing it up. I'm going to close this for now but we can reopen or open a new issue if this rears its head again.