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_membership reports change in team ID, then fails to recreate #323

Closed
brendanjerwin opened this issue Jan 16, 2020 · 23 comments · Fixed by #325
Closed

github_team_membership reports change in team ID, then fails to recreate #323

brendanjerwin opened this issue Jan 16, 2020 · 23 comments · Fixed by #325

Comments

@brendanjerwin
Copy link

Terraform Version

0.12.16
provider "github" (hashicorp/github) 2.2.1...

Affected Resource(s)

  • github_team_membership

Behavior

We saw:

# module.foobar.github_team_membership.core[0] must be replaced | 153s
-- | --
1733 | -/+ resource "github_team_membership" "core" { | 154s
1734 | ~ etag     = "W/\"4979907f3b4ea610ff2d3a73b25ade0b\"" -> (known after apply) | 154s
1735 | ~ id       = "123456:foobar" -> (known after apply) | 154s
1736 | role     = "member" | 154s
1737 | + team_id  = "123456" # forces replacement | 154s
1738 | username = "foobar" | 154s
1739 | }

Followed by

module.foobar.github_team_membership.other_teams[0]: Destroying... [id=123456:foobar] | 164s
-- | --
1906 |   | 164s
1907 | Error: Unexpected ID format (""), expected numerical ID. strconv.ParseInt: parsing "": invalid syntax

I tested the API: https://api.github.com/teams/123456/memberships/foobar and got:

{"state":"active","role":"maintainer","url":"https://api.github.com/organizations/9876543/team/123456/memberships/foobar"}

Note the url attribute.

The code appears to parse the team ID out of the URL returned by this call: https://github.com/terraform-providers/terraform-provider-github/blob/2c03b951bb0aadc56d5f3e9a254f464d5ee018b8/github/resource_github_team_membership.go#L116

I put together this test: https://play.golang.org/p/3GPY1x3E7N3 The team doesn't parse from the URL, just the username.

Suggestion: It would appear that the Team ID is already known: https://github.com/terraform-providers/terraform-provider-github/blob/2c03b951bb0aadc56d5f3e9a254f464d5ee018b8/github/resource_github_team_membership.go#L89 There should be no reason to parse it from the returned URL.

@razorsedge
Copy link

I just ran into this as well.

@samrees
Copy link
Contributor

samrees commented Jan 16, 2020

Just ran into this.

@gihad
Copy link

gihad commented Jan 16, 2020

@brendanjerwin @razorsedge @samrees Any of you have found a workaround? Perhaps specifying an older version of the provider?

@brandoncamenisch
Copy link

I tested a few versions back but same issue 😭 afaict There hasn't been any updates to 2.2.1 for quite some time. 🤔 I wonder if there was an update somewhere else (GH maybe) and this provider just needs to catch up to that.

@gihad
Copy link

gihad commented Jan 16, 2020

@brandoncamenisch That's also my hunch, I also took a look at the code history and didn't see changes for a while that would explain this happening now.

@samrees
Copy link
Contributor

samrees commented Jan 16, 2020

This makes no sense to me. This started happening this morning, yet the code mentioned hasn't changed in 4 years.

https://github.com/terraform-providers/terraform-provider-github/blob/2c03b951bb0aadc56d5f3e9a254f464d5ee018b8/github/resource_github_team_membership.go#L148

changing this does seem to fix it?

-               if urlSlice[v] == "teams" {
+               if urlSlice[v] == "team" {

https://developer.github.com/v3/teams/members/#get-team-membership shows "teams" plural.

@steakfest
Copy link

I came to the same conclusion that @samrees came too and came here to post about it.

I suspect that github pushed an update to their API today. Possibly thinking they were "fixing" an issue and of course broke our tool relying on this specific url pattern.

I would propose that instead of changing that line from teams to team that maybe a safer bet would be to add some OR logic so that it finds the value if the preceding slug is teams OR team. That way if github realizes they broke something with their api backwards compatibility and revert the change we're not in the same boat.

@samrees
Copy link
Contributor

samrees commented Jan 17, 2020

The pr I made removes the function that string parses the URL entirely as relying on that URL for anything other than a self referential URL is incorrect .

Githubs love of hypermedia APIs is admirable but causes this kind of confusion. Most devs I've seen don't want to write a crawler to interact with an API, in which case the right answer is to throw away all the URLs returned by the v3 api.

https://developer.github.com/v3/#hypermedia
https://en.wikipedia.org/wiki/Hypermedia#Application_programming_interfaces

@shouichi
Copy link
Contributor

shouichi commented Jan 17, 2020

@jcudit Could you take a look at this issue? It seems like GitHub changed their API and this provider is affected. As a result terraform apply always fails 😭

@jcudit
Copy link
Contributor

jcudit commented Jan 18, 2020

I’ve looked and found the likely change that caused this from the GitHub side. My next steps are to get passing acceptance tests for the proposed fix. I can post an update next week if it is yet to be released by then.

@willejs
Copy link

willejs commented Jan 20, 2020

@radeksimko do you think we could get the 'quickfix' in #326 merged and released as a stop gap solution? Building a fork of the plugin isnt easily going to get into my CD pipeline...

@jcudit
Copy link
Contributor

jcudit commented Jan 20, 2020

I've thrown this patch at the test suite. It seems incomplete as a fix:

$ go test -v -timeout 30m  ./... -run TestAccGithubTeamMembership_basic
...
2020/01/17 20:22:46 [DEBUG] Github API Request Details:
---[ REQUEST ]---------------------------------------
GET /teams/3606334/memberships/fuzzoli87 HTTP/1.1
Host: api.github.com
User-Agent: go-github
Accept: application/vnd.github.hellcat-preview+json
Accept-Encoding: gzip


-----------------------------------------------------
2020/01/17 20:22:46 [DEBUG] Sleeping 1s between write operations
2020/01/17 20:22:47 [DEBUG] Github API Response Details:
---[ RESPONSE ]--------------------------------------
HTTP/1.1 404 Not Found
Transfer-Encoding: chunked
Access-Control-Allow-Origin: *
Access-Control-Expose-Headers: ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type
Content-Security-Policy: default-src 'none'
Content-Type: application/json; charset=utf-8
Date: Sat, 18 Jan 2020 01:22:47 GMT
Referrer-Policy: origin-when-cross-origin, strict-origin-when-cross-origin
Server: GitHub.com
Status: 404 Not Found
Strict-Transport-Security: max-age=31536000; includeSubdomains; preload
X-Accepted-Oauth-Scopes: repo
X-Content-Type-Options: nosniff
X-Frame-Options: deny
X-Github-Media-Type: github.v3; param=hellcat-preview; format=json
X-Github-Request-Id: FABE:4655:15D7FC:29DF1A:5E225DE6
X-Oauth-Scopes: admin:gpg_key, admin:org, admin:public_key, admin:repo_hook, delete_repo, repo, user
X-Ratelimit-Limit: 5000
X-Ratelimit-Remaining: 4961
X-Ratelimit-Reset: 1579312573
X-Xss-Protection: 1; mode=block

70
{
 "message": "Not Found",
 "documentation_url": "https://developer.github.com/v3/teams/members/#get-team-membership"
}

I've got time blocked off today to continue investigating. I suspect the upstream library will need updates in response to the API change. I'll post updates here.

@DMonCode
Copy link

Was the /teams to /team change part of the api v3 update? or did GitHub change their API string mid version?

Does provider-github specify it's API version it's looking for or is it just pulling current?

@eerkunt
Copy link
Contributor

eerkunt commented Jan 20, 2020

Having exactly the same problem :( This is critically effecting our organisation right now :(

@jcudit
Copy link
Contributor

jcudit commented Jan 20, 2020

Status: 404 Not Found

As pointed out over here, the error I pasted above is likely due to my inputs to the test case. Apologies for that misdirection. I will continue to work towards passing acceptance tests for the proposed fix.

@thanasisk
Copy link

This is affecting us as well. I will be monitoring this issue for updates. Is there an ETA on a fix?

@stefansedich
Copy link

stefansedich commented Jan 21, 2020

Just hit by this today trying to provision a new-hire to our GH org 😢

@tibbon
Copy link

tibbon commented Jan 21, 2020

This is impacting me too. Interested to see a fix.

@steakfest
Copy link

I'm not sure you anybody else noticed this, so I'm share my experience.

I've found that because of the order of operations by the provider, that generally what I want to happen is still happening. Since the apply fails to delete after any new memberships have been created, this hans't blocked me from being able to add new members to teams, since terraform doesn't roll back.

@tibbon
Copy link

tibbon commented Jan 22, 2020

@radeksimko you closed this. Is it a fixed/non-issue?

@suever
Copy link

suever commented Jan 22, 2020

@radeksimko you closed this. Is it a fixed/non-issue?

@tibbon The fix was released today as v2.3.0:

@gihad
Copy link

gihad commented Jan 22, 2020

@tibbon The new version they released fix it for me: https://github.com/terraform-providers/terraform-provider-github/releases

I had to add version = "~> 2.3" to my provider config.

@tibbon
Copy link

tibbon commented Jan 22, 2020

Thank you! That just unblocked some stuff on my end, and it's super appreciated. Thanks for the work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.