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

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

Open
1 task done
GarrettBlinkhorn opened this issue Nov 6, 2023 · 6 comments
Labels
Status: Up for grabs Issues that are ready to be worked on by anyone Type: Bug Something isn't working as documented

Comments

@GarrettBlinkhorn
Copy link

GarrettBlinkhorn commented Nov 6, 2023

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:

  • If I take UserC and add them to var.members which is used to generate the ParentTeam github_team_members, then Terraform will return No changes required. even though UserC will not actually show up as ParentTeam team member. This "solves" the plan but actually makes the situation more confusing, because UserC is defined as a member of ParentTeam in code but not in reality. I would expect that any user defined using the member block would be a member of the team.
  • When viewing a given team on the Github UI, they differentiate between members and child team members - the github provider though does not seem to make this distinction, which I suspect is why the provider treats them like an actual member instead.

Terraform Version

Terraform v1.5.2
MacOS Ventura 13.6

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

Affected Resource(s)

  • github_team_members

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

  • I agree to follow this project's Code of Conduct
@GarrettBlinkhorn GarrettBlinkhorn added Status: Triage This is being looked at and prioritized Type: Bug Something isn't working as documented labels Nov 6, 2023
Copy link

github-actions bot commented Nov 6, 2023

👋 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! 🚀

@kfcampbell kfcampbell added Status: Up for grabs Issues that are ready to be worked on by anyone and removed Status: Triage This is being looked at and prioritized labels Nov 9, 2023
@GarrettBlinkhorn
Copy link
Author

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
Copy link
Contributor

o-sama commented Nov 20, 2023

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
Copy link
Member

@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
Copy link

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

@kfcampbell
Copy link
Member

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Up for grabs Issues that are ready to be worked on by anyone Type: Bug Something isn't working as documented
Projects
None yet
Development

No branches or pull requests

4 participants