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 is created with the Terraform user added as an extra member #527

Closed
morancj opened this issue Aug 6, 2020 · 6 comments
Closed
Labels
Good first issue Good for newcomers r/team Status: Stale Used by stalebot to clean house Type: Bug Something isn't working as documented
Milestone

Comments

@morancj
Copy link

morancj commented Aug 6, 2020

Terraform Version

Terraform v0.12.24

  • provider.aws v2.59.0
  • provider.github v2.8.1

Affected Resource(s)

  • github_team
  • (optionally) github_team_membership

Terraform Configuration Files

resource "github_team" "tftest" {
  name        = "tftest"
  description = "Test team"
  privacy     = "closed"
}

Debug Output

Not relevant: trivial to reproduce. Let me know otherwise, because redacting will take some effort.

Expected Behavior

Team created with no members

Actual Behavior

The user under which terraform is run is silently added as a member to the new team. This membership does not appear in the output or state file.

Steps to Reproduce

Create a new team with or without any github_team_membership entries.
Team will be created with members + terraform_user, if terraform_user is not a member.

Important Factoids

Creating the team through GitHub's Web UI (BUI) automatically adds the user creating the team as a member.

References

Presumably, this is because of:
https://github.com/google/go-github/blob/954e7c82b2994a9f418f300c3f1e147f6a51bd29/github/teams.go#L172:24
and
https://developer.github.com/v3/teams/#create-a-team
The latter states:

When you create a new team, you automatically become a team maintainer without explicitly adding yourself to the optional array of maintainers. For more information, see "About teams" in the GitHub Help documentation.

@sfdc-afraley
Copy link

This is especially confusing when you're testing with your account and creating teams where you yourself are a member. It adds you as a maintainer, but then a second terraform run will want to change your role. For example:

resource "github_team" "teams" {
  name        = "team1"
  privacy     = "closed"
}

resource "github_team_membership" "some_team_membership" {
  team_id  = github_team.teams.id
  username = "myuser"
  role     = "member"
}

Second TF apply:

# module.main.github_team_membership.membership["myuser:team1"] will be updated in-place
  ~ resource "github_team_membership" "some_team_membership" {
        id       = "1234567:myuser"
      ~ role     = "maintainer" -> "member"
        team_id  = "1234567"
        username = "myuser"
    }

@jcudit jcudit added Type: Bug Something isn't working as documented r/team labels Nov 26, 2020
@thekbb
Copy link
Contributor

thekbb commented Dec 16, 2020

Still relevant with
Terraform 0.13.5
provider.github 2.9.2

For context: This would simplify things for me as we're using teams and CODEOWNERS for some approval workflows, and the service account that creates the team shouldn't be able to approve things.

@jcudit jcudit added this to the v4.1.2 milestone Jan 4, 2021
jcudit pushed a commit that referenced this issue Jan 18, 2021
This adds a possible fix to the following issues by providing users with an option to remove the automatic addition of a default maintainer to a team during creation.

/cc #527
/cc #104
/cc #130
@ghost
Copy link

ghost commented Feb 4, 2021

It would be great if we could pass a map of members/role to the resource. It could then diff what we define and what's there, thus removing that auto-added creator.

jcudit pushed a commit that referenced this issue Feb 5, 2021
This adds a possible fix to the following issues by providing users with an option to remove the automatic addition of a default maintainer to a team during creation.

/cc #527
/cc #104
/cc #130
jcudit pushed a commit that referenced this issue Feb 5, 2021
* Add `create_default_maintainer` option to `github_team`

This adds a possible fix to the following issues by providing users with an option to remove the automatic addition of a default maintainer to a team during creation.

/cc #527
/cc #104
/cc #130
@jcudit
Copy link
Contributor

jcudit commented Feb 5, 2021

Ah, that's a good idea! Read it too late though and merged the workaround instead 🙃 . Seems like the right fix though and we can aim to get that option added in a future release.

@jcudit jcudit added the Good first issue Good for newcomers label Feb 5, 2021
k24dizzle added a commit to lyft/terraform-provider-github that referenced this issue Feb 22, 2021
* v4.1.0

* Fix unable to resolve node id for branch_protection (integrations#610)

* Don't check node id for length

* Check if node id is valid base 64

Co-authored-by: Willem Gillis <willem.gillis@imec.be>

* temporarily disable PR acceptance testing

these jobs all fail and are confusing to contributors when launched from 
a PR raised by a fork.  there are ways to get around this, but will 
defer until the repository is transferred.  disabling for now.

* remove `ForceNew` on `template*` as they are concerns only at creation time (integrations#609)

There are a number of resources that have been marked as `ForceNew: true` out of a desire in "correctness" by those that do not actually understand how these resources are used in the real world and damage that can be done. No one wants to blow up a repository to change something like this, if they need to there is a mechanism built into terraform called [taint](https://www.terraform.io/docs/commands/taint.html). While there are some things that make sense for using `ForceNew` a repository for source control on properties that the API will ignore outside of creation is not one of them.

Signed-off-by: Ben Abrams <me@benabrams.it>

* Add diff suppression function to the branch protection resource (integrations#614)

Adding a diff suppression function to the branch protection resource to
ignore the strict status check field if no contexts have been specified.

This resolves the issue with the GraphQL API returning a strict status
check value of "true" by default, regardless of contexts being set or
not.

* Add Apps to actor types in branch protection (integrations#615)

Adding Github Apps to actor types in the branch protection resource.

NOTE: Apps as an actor type is only available in push restrictions.

* Added `allowsDeletions`and `allowsForcePushes`settings (integrations#623)

* Added `allowsDeletions`and `allowsForcePushes`settings https://developer.github.com/v4/changelog/2020-11-13-schema-changes/ (#1)

* complete documentation

* update module github.com/shurcooL/githubv4 with `go get github.com/shurcooL/githubv4`

* vendor latest githubv4

* add test for deletions and force pushes

Co-authored-by: Jeremy Udit <jcudit@github.com>

* Fix syntax error

* Conditionally Run GHES Test Suite

* Run gofmt (integrations#645)

Signed-off-by: Stephen Hoekstra <shoekstra@schubergphilis.com>

* Allow dependabot to check github actions (integrations#643)

* Typo: s/visiblity/visibility (integrations#629)

Small typo in the docs.

* github_repository_webhook: describe content_type options (integrations#510)

* change private to visibility (integrations#635)

* Use commit SHA to lookup commit info in github_repository_file resource (integrations#644)

* Fix references to "master"

Signed-off-by: Stephen Hoekstra <shoekstra@schubergphilis.com>

* Use commit SHA to lookup commit info

Currently the provider loops through commits in a repo until it finds the most recent commit containing the managed file. This is inefficient and could lead to you being rate limited if managing a few files that were updated a long time ago.

This commit fixes that by storing the commit SHA when updating the file and using that SHA to lookup the commit info instead of looping through all commits.

Signed-off-by: Stephen Hoekstra <shoekstra@schubergphilis.com>

* add release automation for terraform registry

/cc https://www.terraform.io/docs/registry/providers/publishing.html

* Add v4.2.0 Release (integrations#641)

* add v4.1.1 release items

* correct semver version

* Document Additional Breaking Change For v3.0.0

* add `github_repository_file` bugfix

* move to correct release

* add goreleaser configuration to enable release automation

* resource/repository: add support for enabling github pages (integrations#490)

* add support for enabling github pages

* update resource comments

* add additional comments in expand methods

* add formatting fixes

Co-authored-by: Jeremy Udit <jcudit@github.com>

* Add `github_branch_protection_v3` Resource (integrations#642)

* Add `branch_protection_v3` Resource

- add new resource to `website/github.erb`
- add new resource to `website/docs/r/<resource>.html.markdown`
- add new resource to `github/provider.go`
- add tests for resource in `github/resource_<resource>_test.go`
- implement new resource in `github/resource_<resource>.go`

* fixup! gofmt fixes

* add changelog entries for v4.3.0 release (integrations#658)

* Remove github.com/hashicorp/terraform from dependencies (integrations#628)

* remove github.com/hashicorp/terraform from dependencies

* go mod tidy

* refactor: execute fmt

* Allow dependabot to check go dependencies (integrations#653)

* Fix link to Milestones page (integrations#663)

* github_branch_default: send only fields that changed. Fixes integrations#625 integrations#620. (integrations#666)

* github_branch_default: send only fields that changed. Fixes integrations#625 integrations#620.

* fix failing test and update docs

Co-authored-by: Jeremy Udit <jcudit@github.com>

* Fix error handling (integrations#668)

Do not silently proceed further on receiving an error response.

Signed-off-by: rustyclock <rustyclock@protonmail.com>

* Update CHANGELOG.md

* Remove Obsolete Test

My understanding is our use of Terraform Registry makes this failing test unnecessary.

* Update GitHub organization references (integrations#672)

Following the project transfer from `terraform-providers` organization to `integrations`.

* Add Example For `github_team_repository` (integrations#676)

* Add Example For `github_team_repository`

also documents a limitation with `for_each` in this scenario

* fixup! add newline

* changelog: manually fix links to issues (integrations#673)

They were pointing to the old archived repo, which doesn't have newer issues.

* Handle base64 decodable repo names (integrations#684)

* github/config: Fix detection of individual, non-org accounts (integrations#685)

- Previously, whenever an individual user tried to interact with their
  repos, the provider would return an error, rendering v4.3.1 unusable
  _for individuals_:

  ```
  ➜ terraform plan
  Error: GET https://api.github.com/orgs/issyl0: 404 Not Found []
  on /Users/issyl0/repos/terraform/github.tf line 1, in provider "github":
  ```

- `ConfigureOwner` works such that if the `owner.name` is not blank (ie,
  the user had specified `owner = <username>` in their Terraform file),
  the code progresses to check if the `owner.name` is an org.
  Importantly, that check (prior to this change) returned an error if
  `owner.name` was not an org. That final error meant it was impossible
  to run if not using an organisation account.

- Reproduction steps: https://gist.github.com/issyl0/cd61e4cb59de2c2e1e8f45e3cf7c12f5

* add changelog entry for v4.3.2

* Add `create_default_maintainer` Option To `github_team` (integrations#661)

* Add `create_default_maintainer` option to `github_team`

This adds a possible fix to the following issues by providing users with an option to remove the automatic addition of a default maintainer to a team during creation.

/cc integrations#527
/cc integrations#104
/cc integrations#130

* Refresh CONTRIBUTING Documentation (integrations#682)

* refresh contributing docs

* add quick instructions

* add updates for newer versions of terraform

* Add Diff Suppression Option To `repository_collaborator` (integrations#683)

* add diff suppression option to `repository_collaborator`

* fixup! remove comment

* remove hardcoded username from test

* Update CHANGELOG for v4.4.0 release

* Add repo context to error message when branch is not found (integrations#691)

* Add repo context to error message when branch is not found

* fix failing `TestAccGithubRepositoryFile` test

access committer data through `Commit` struct

Co-authored-by: Jeremy Udit <jcudit@github.com>

* fix based on linter (integrations#694)

* add v4.4.1 release notes

* Modify github_team_repository to accept slug as a valid team_id as well (integrations#693)

* First attempt

* Add comment

* Attempt to modify unit tests

* Edit docs to reflect change

* Make sure team_id is set appropriately

* fixing lint

* add passing tests for `github_team_repository`

Co-authored-by: Jeremy Udit <jcudit@github.com>

* add v4.5.0 release notes

* temporarily ignore `darwin/arm64` to unblock releases

/cc integrations#695

* update release notes for v4.5.0

Co-authored-by: tf-release-bot <terraform@hashicorp.com>
Co-authored-by: Polygens <willem.gillis@gmail.com>
Co-authored-by: Willem Gillis <willem.gillis@imec.be>
Co-authored-by: Jeremy Udit <jcudit@github.com>
Co-authored-by: Ben Abrams <me@benabrams.it>
Co-authored-by: Patrick Marabeas <patrick@marabeas.io>
Co-authored-by: Francois BAYART <francois.bayart@kensu.io>
Co-authored-by: Stephen Hoekstra <shoekstra@schubergphilis.com>
Co-authored-by: John Losito <lositojohnj@gmail.com>
Co-authored-by: Bret <166301+bcomnes@users.noreply.github.com>
Co-authored-by: Jakub Holy <jakubholy@jakubholy.net>
Co-authored-by: Ichinose Shogo <shogo82148@gmail.com>
Co-authored-by: angie pinilla <angelinepinilla@gmail.com>
Co-authored-by: Shu Kutsuzawa <cappyzawa@yahoo.ne.jp>
Co-authored-by: Oleksandr Dievri <o.dievri@gmail.com>
Co-authored-by: Dee Kryvenko <109895+dee-kryvenko@users.noreply.github.com>
Co-authored-by: Ravi <1299606+rustycl0ck@users.noreply.github.com>
Co-authored-by: Alexis Gauthiez <alexis.gauthiez@gmail.com>
Co-authored-by: Christian Höltje <docwhat@gerf.org>
Co-authored-by: Issy Long <me@issyl0.co.uk>
Co-authored-by: Michael Barany <1434605+mbarany@users.noreply.github.com>
kfcampbell pushed a commit to kfcampbell/terraform-provider-github that referenced this issue Jul 26, 2022
…#661)

* Add `create_default_maintainer` option to `github_team`

This adds a possible fix to the following issues by providing users with an option to remove the automatic addition of a default maintainer to a team during creation.

/cc integrations#527
/cc integrations#104
/cc integrations#130
@github-actions
Copy link

github-actions bot commented Dec 8, 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 8, 2022
@github-actions github-actions bot removed the Status: Stale Used by stalebot to clean house label Dec 9, 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
Good first issue Good for newcomers 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