terraform-provider-github icon indicating copy to clipboard operation
terraform-provider-github copied to clipboard

github_team creates team with no members that is unmanageable

Open ghost opened this issue 7 years ago • 14 comments

This issue was originally opened by @steven-edgar as hashicorp/terraform#18655. It was migrated here as a result of the provider split. The original body of the issue is below.


Terraform Version

Terraform v0.11.7

Terraform Configuration Files

resource "github_team" "my-team" { 
name = "my-team" 
description = "This is my team" 
privacy = "closed" 
}

resource "github_team" "my-subteam" { 
name = "my-subteam" 
description = "This is my subteam" 
privacy = "closed" 
parent_team_id = "${github_team.my-team.id}" 
}

resource "github_team_membership" "my-team_github-user-1" { 
team_id = "${github_team.my-subteam.id}" 
username = "github-user-1" 
role = "maintainer" 
}

Debug Output

Crash Output

Expected Behavior

Created a github team with the creators account as maintainer

Actual Behavior

Created a github team with no members, which was therefor unmaintainable. We've had to contact Github support to have that resolved.

Steps to Reproduce

Github team "my-team" already existed and had been imported into the terraform state. We then added my-subteam and ran: terraform init terraform apply

Additional Context

References

ghost avatar Aug 11 '18 03:08 ghost

Hi there @steven-edgar thanks for opening this issue. It looks like you're bumping into the exact same issue as is described in https://github.com/terraform-providers/terraform-provider-github/pull/104

Can you please take time to read that short thread and respond there (with scopes of the token used), so we get a chance to reproduce this behaviour and decide what next steps to take?

radeksimko avatar Aug 13 '18 05:08 radeksimko

Hi @radeksimko

I have some proposals for fixing this. It feels wrong to add this discussion to #104 (where I'm actively participating) as there could be multiple solution PRs and the discussion should remain somewhat central. First to restate the problem:

When creating a team via the github UI, the user creating the team is automatically added as a member to the team with maintainer role. This is same whether the user is an organisation admin or regular organisation member creating a team.

When terraform creates a github_team resource it it created without any members, and any team membership is controlled github_team_membership resource. This isn't a problem for token from a organisation admin user as they have access to all teams. It is a problem for regular organisation member's token when creating a team because their user isn't a member with maintainer role of the team when it tries to add itself as a member.

You may say just use the organisation admin user's token. This isn't always possible especially in larger organisations either due to delegated responsibilities or security concerns. You can see from the participation in #104 that there are multiple people affected. For example, I am in a github organisation with around 1500 repositories, where I have delegated responsibility for about 100 of them.

I'm not in favour of the current PR in #104 as we will run into a conflict of managing team membership roles through both github_team:maintainers and github_team_membership Trying to manage the team membership role through github_team:maintainers will be problematic. The github api for creation of new teams has a maintainers parameter to add to maintainers to the new team at create time. However it's not symmetric with getTeam functions which deals with team members only and not maintainers.

My suggestion is to to mirror the behaviour of github and add a create_default_maintainer boolean to the resource which would add the user associated with the token as a default maintainer to the team.

 resource "github_team" "foo" {
          name = "myfooteam"
          description = "foo"
          privacy = "secret"
          create_default_maintainer = true
  }

This could be done by these changes to the schema below and

  • resourceGithubTeamRead() will always set create_default_maintainer to false
  • a diff suppression function.
"create_default_maintainer": {
	Type:     schema.TypeBool,
	Optional: true,
	Default:  false,
},

This would ;

  • only add a maintainer on creation of a team, and not updates
  • not appear in diff/plans
  • Import correctly
  • Leave the team membership and role to be managed by github_team_membership

The only problem I could see is that we would want to ensure the idempotent behaviour of github_team_membership resource when adding team membership to a team where the user is already a member with maintainer role

I appreciate that I may be misusing the schema by adding a required field that stores an option, so some guidance from @radeksimko if this seems like a workable solution would be great. Alternatively does anyone have any better ideas for how to solve this?

carinadigital avatar Mar 06 '19 20:03 carinadigital

It's technically manageable by org maintainers, I personally do not want anyone as team maintainers at my org as they should only be able to make team changes in github but I realize not every setup is the same and can see why this would be desired. I support this behind a feature flag with the old defaults to maintain backwards compatibility.

majormoses avatar Jul 16 '19 23:07 majormoses

I support this behind a feature flag with the old defaults to maintain backwards compatibility.

create_default_maintainer defaulting to false is the same as the previous behaviour.

carinadigital avatar Jul 17 '19 08:07 carinadigital

I've just begun using this provider, and ran into this issue today for the first time. I am an org admin, so I don't need to be included in teams I create (and I usually remove myself as I don't want to receive notifications sent to the team). Before I read this issue and the two associated PRs, I assumed we could just completely suppress the automatic behavior from GitHub because there wouldn't be any case where the 'creating user' needs to be a member of the team.

Now I see that's not true, and there are completely reasonable situations where that is required. Unfortunately this conflicts with the entire concept of Terraform management of the team; it will have a member which Terraform does not have reflected in the state, and if the person managing the Terraform configuration adds a github_team_membership resource for the same user then the result will be illogical (especially if that resource is later deleted, which would remove the person from the team).

In my mind I wondered if it would be possible for a provider to tell Terraform that additional resources were automatically created as a side-effect of a resource creation operation, but that would only get the maintainer added to the state; they wouldn't be added to the HCL files, and the next plan/apply cycle would remove them!

I can't think of a good way to resolve this, because even attempting to generate a warning to the user that their HCL files will result in a team they cannot manage would require making two passes over the configuration, and Terraform doesn't do that. The best I can think of is to have the top-level github object find out whether the token in use has admin:org scope, and if it does not, then any github_team resources which have create_default_maintainer set to false would generate an error.

kpfleming avatar Dec 05 '19 00:12 kpfleming

Taking that one step further, the top-level provider could also obtain the username of the user who owns the token, and then if a github_team_membership resource is seen for that same user with permissions less than maintainer, an error would be generated.

kpfleming avatar Dec 05 '19 01:12 kpfleming

Not quite a year later I'm coming across this issue. It would be ideal to be able to suppress the automatic maintainer add, as I now have a user that got added to every team I created with terraform. Since this is a service principal acting at organization level it should be transparent without needing to be a maintainer in every group. There are valid cases where this can lock up, but I would argue that those are cases that the github API needs to solve, not terraform. I would happily :+1: a PR that adds a flag to create teams that don't have the creating user as a member.

the-maldridge avatar Oct 13 '20 21:10 the-maldridge

👋 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[bot] avatar Dec 12 '22 02:12 github-actions[bot]

👋 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[bot] avatar Apr 25 '24 01:04 github-actions[bot]

Still a bug.

the-maldridge avatar Apr 25 '24 03:04 the-maldridge

👋 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[bot] avatar Jan 23 '25 02:01 github-actions[bot]

Still a bug.

the-maldridge avatar Jan 23 '25 02:01 the-maldridge

👋 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[bot] avatar Dec 07 '25 02:12 github-actions[bot]

@the-maldridge Hey there 👋

Could you describe what you are experiencing?

The Original issue was that the TF user wasn't added as maintainer to a team automatically and AFAIK that is not a bug, since the TF user could be a GH App.

And adding maintainers to a team via TF AFAIK works.

deiga avatar Dec 09 '25 07:12 deiga