Skip to content

Commit 96f7f24

Browse files
pzmi-f3pzmi-f3
authored
Move away from deprecated Teams API Endpoints (#32)
On March 15, 2022 at 14:00 UTC GitHub will deprecate an endpoint under path /teams/:team_id Source: https://github.blog/changelog/2022-02-22-sunset-notice-deprecated-teams-api-endpoints/ To use a new endpoint this PR upgrades `google/go-github` module to version v42.0.0. This is a minimal set of changes to mitigate effects of the deprecation: - replace call to `githubClient.Teams.ListTeamMembers` with `githubClient.Teams.ListTeamMembersByID` - add a call to `githubClient.Organizations.Get` - to retrieve an organisation ID, which is required `ListTeamMembersByID`, from only it's name - adjust test accordingly
1 parent 214dff2 commit 96f7f24

23 files changed

Lines changed: 579 additions & 212 deletions

‎go.mod‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ require (
1010
github.com/form3tech-oss/go-pact-testing v1.4.1
1111
github.com/form3tech-oss/logrus-logzio-hook v1.0.0
1212
github.com/golang/snappy v0.0.1 // indirect
13-
github.com/google/go-github/v28 v28.1.1
13+
github.com/google/go-github/v42 v42.0.0
1414
github.com/google/tcpproxy v0.0.0-20180808230851-dfa16c61dad2
1515
github.com/google/uuid v1.3.0
1616
github.com/gorilla/mux v1.8.0
@@ -25,6 +25,4 @@ require (
2525
github.com/spf13/jwalterweatherman v1.1.0 // indirect
2626
github.com/spf13/viper v1.7.1
2727
github.com/stretchr/testify v1.7.0
28-
golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550 // indirect
29-
golang.org/x/net v0.0.0-20200226121028-0de0cce0169b // indirect
3028
)

‎go.sum‎

Lines changed: 21 additions & 148 deletions
Large diffs are not rendered by default.

‎internal/api/approval/approval.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import (
1414

1515
ghclient "github.com/form3tech-oss/github-team-approver/internal/api/github"
1616

17-
"github.com/google/go-github/v28/github"
17+
"github.com/google/go-github/v42/github"
1818
)
1919

2020
const (
@@ -388,4 +388,3 @@ func filterAllowedAndIgnoreReviewers(members []*github.User, commits []*github.R
388388

389389
return allowed, ignored
390390
}
391-

‎internal/api/approval/approval_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
package approval
22

33
import (
4-
"github.com/google/go-github/v28/github"
4+
"github.com/google/go-github/v42/github"
55
"github.com/stretchr/testify/assert"
66
"github.com/stretchr/testify/require"
77
"testing"
@@ -136,8 +136,8 @@ func TestContentsUrlToRelDir(t *testing.T) {
136136

137137
func TestFilterAllowedAndIgnoreReviewers(t *testing.T) {
138138
tests := map[string]struct {
139-
commits []*github.RepositoryCommit
140-
members []*github.User
139+
commits []*github.RepositoryCommit
140+
members []*github.User
141141
allowed []string
142142
ignored []string
143143
}{

‎internal/api/approval/state.go‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,17 @@ package approval
22

33
import (
44
"fmt"
5-
"github.com/google/go-github/v28/github"
5+
"github.com/google/go-github/v42/github"
66
log "github.com/sirupsen/logrus"
77
"strings"
88
)
99

1010
const (
11-
statusEventDescriptionApprovedFormatString = "Approved by:\n%s"
12-
statusEventDescriptionForciblyApproved = "Forcibly approved."
13-
statusEventDescriptionNoReviewsRequested = "No teams have been identified as having to be requested for a review."
14-
statusEventDescriptionNoRulesMatched = "The PR's body doesn't meet the requirements."
15-
statusEventDescriptionPendingFormatString = "Needs approval from:\n%s"
11+
statusEventDescriptionApprovedFormatString = "Approved by:\n%s"
12+
statusEventDescriptionForciblyApproved = "Forcibly approved."
13+
statusEventDescriptionNoReviewsRequested = "No teams have been identified as having to be requested for a review."
14+
statusEventDescriptionNoRulesMatched = "The PR's body doesn't meet the requirements."
15+
statusEventDescriptionPendingFormatString = "Needs approval from:\n%s"
1616
)
1717

1818
type state struct {
@@ -54,7 +54,7 @@ func (s *state) addPendingTeamNames(name string) {
5454
}
5555

5656
func (s *state) setApprovingReviewers(reviews []*github.PullRequestReview) {
57-
approving := map[string]bool{}
57+
approving := map[string]bool{}
5858

5959
for _, review := range reviews {
6060
if *review.State == pullRequestReviewStateApproved {
@@ -63,6 +63,7 @@ func (s *state) setApprovingReviewers(reviews []*github.PullRequestReview) {
6363
}
6464
s.approvingReviewers = approving
6565
}
66+
6667
// The members that will be mentioned in the bot comment when it publishes ignored PR reviewers comment.
6768
// We only mention reviewers who approved a PR and also contributed to the PR thus being ignored as valid reviewers.
6869
// Without this filtering, we would be mentioning all contributing authors even if they didn't review the PR.
@@ -130,4 +131,4 @@ func computeReviewsToRequest(log *log.Entry, teams []*github.Team, pendingTeams
130131
}
131132
log.Tracef("Reviews will be requested from the following teams: %v", reviewsToRequest)
132133
return reviewsToRequest
133-
}
134+
}

‎internal/api/event.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,11 @@ package api
22

33
import (
44
"fmt"
5-
"github.com/google/go-github/v28/github"
5+
"github.com/google/go-github/v42/github"
66
)
7+
78
const (
8-
pullRequestActionClosed = "closed"
9+
pullRequestActionClosed = "closed"
910
)
1011

1112
type event interface {
@@ -28,4 +29,4 @@ func getSupportedEvent(eventType string) (event, error) {
2829
}
2930

3031
return nil, fmt.Errorf("%s: %w", eventType, errIgnoredEvent)
31-
}
32+
}

‎internal/api/github/client.go‎

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import (
1616

1717
"github.com/bradleyfalzon/ghinstallation"
1818
"github.com/form3tech-oss/github-team-approver-commons/pkg/configuration"
19-
"github.com/google/go-github/v28/github"
19+
"github.com/google/go-github/v42/github"
2020
"github.com/gregjones/httpcache"
2121
log "github.com/sirupsen/logrus"
2222
)
@@ -48,13 +48,14 @@ func (c *Client) GetConfiguration(ctx context.Context, ownerLogin, repoName stri
4848
// Try to download the contents of the configuration file.
4949
ctxTimeout, fn := context.WithTimeout(ctx, DefaultGitHubOperationTimeout)
5050
defer fn()
51-
r, err := c.githubClient.Repositories.DownloadContents(ctxTimeout, ownerLogin, repoName, configuration.ConfigurationFilePath, nil)
51+
r, res, err := c.githubClient.Repositories.DownloadContents(ctxTimeout, ownerLogin, repoName, configuration.ConfigurationFilePath, nil)
5252
if err != nil {
53-
if strings.Contains(err.Error(), "No file named") { // No better way of distinguishing between errors.
53+
if strings.Contains(strings.ToLower(err.Error()), "no file named") { // fixme: look at status code instead of body
5454
return nil, ErrNoConfigurationFile
5555
}
5656
return nil, fmt.Errorf("error downloading configuration: %w", err)
5757
}
58+
defer res.Body.Close()
5859
defer r.Close()
5960
// Parse the configuration file.
6061
v, err := configuration.ReadConfiguration(r)
@@ -68,7 +69,7 @@ func (c *Client) GetPullRequestReviews(ctx context.Context, ownerLogin, repoName
6869
reviews := make([]*github.PullRequestReview, 0, 0)
6970

7071
opts := &github.ListOptions{
71-
Page: 1,
72+
Page: 1,
7273
PerPage: defaultListOptionsPerPage,
7374
}
7475

@@ -109,7 +110,7 @@ func (c *Client) GetPullRequestCommitFiles(ctx context.Context, ownerLogin, repo
109110
commitFiles := make([]*github.CommitFile, 0, 0)
110111

111112
opts := &github.ListOptions{
112-
Page: 1,
113+
Page: 1,
113114
PerPage: defaultListOptionsPerPage,
114115
}
115116

@@ -149,7 +150,7 @@ func (c *Client) GetTeams(ctx context.Context, organisation string) ([]*github.T
149150
teams := make([]*github.Team, 0, 0)
150151

151152
opts := &github.ListOptions{
152-
Page: 1,
153+
Page: 1,
153154
PerPage: defaultListOptionsPerPage,
154155
}
155156

@@ -247,7 +248,7 @@ func (c *Client) GetTeamMembers(ctx context.Context, teams []*github.Team, organ
247248

248249
opts := &github.TeamListTeamMembersOptions{
249250
ListOptions: github.ListOptions{
250-
Page: 1,
251+
Page: 1,
251252
PerPage: defaultListOptionsPerPage,
252253
},
253254
}
@@ -264,21 +265,36 @@ func (c *Client) GetTeamMembers(ctx context.Context, teams []*github.Team, organ
264265
logger.WithFields(log.Fields{"page": opts.Page}).Tracef("requesting")
265266

266267
ctxTimeout, fn := context.WithTimeout(ctx, DefaultGitHubOperationTimeout)
267-
m, res, err := c.githubClient.Teams.ListTeamMembers(ctxTimeout, team.GetID(), opts)
268+
org, resorg, err := c.githubClient.Organizations.Get(ctx, organisation)
269+
if err != nil {
270+
fn()
271+
return nil, fmt.Errorf("error getting an organisation %q: %w", organisation, err)
272+
}
273+
if resorg.StatusCode >= 300 {
274+
fn()
275+
return nil, fmt.Errorf("error getting an organisation organisation %q (status: %d): %s", organisation, resorg.StatusCode, readAllClose(resorg.Body))
276+
}
277+
fn()
278+
defer resorg.Body.Close()
279+
280+
ctxTimeout, fn = context.WithTimeout(ctx, DefaultGitHubOperationTimeout)
281+
m, resteam, err := c.githubClient.Teams.ListTeamMembersByID(ctxTimeout, org.GetID(), team.GetID(), opts)
268282
if err != nil {
269283
fn()
270284
return nil, fmt.Errorf("error listing members for team %q in organisation %q: %w", name, organisation, err)
271285
}
272-
if res.StatusCode >= 300 {
286+
if resteam.StatusCode >= 300 {
273287
fn()
274-
return nil, fmt.Errorf("error listing members for team %q in organisation %q (status: %d): %s", name, organisation, res.StatusCode, readAllClose(res.Body))
288+
return nil, fmt.Errorf("error listing members for team %q in organisation %q (status: %d): %s", name, organisation, resteam.StatusCode, readAllClose(resteam.Body))
275289
}
276290
fn()
291+
defer resteam.Body.Close()
292+
277293
users = append(users, m...)
278-
if res.NextPage == 0 {
294+
if resteam.NextPage == 0 {
279295
break
280296
}
281-
opts.Page = res.NextPage
297+
opts.Page = resteam.NextPage
282298
}
283299
return users, nil
284300
}
@@ -452,7 +468,7 @@ func (c *Client) GetLabels(ctx context.Context, ownerLogin, repoName string, prN
452468

453469
labels := make([]string, 0, 0)
454470
opts := &github.ListOptions{
455-
Page: 1,
471+
Page: 1,
456472
PerPage: defaultListOptionsPerPage,
457473
}
458474

‎internal/api/github/stages/client_stage.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import (
55
ghclient "github.com/form3tech-oss/github-team-approver/internal/api/github"
66
"github.com/form3tech-oss/github-team-approver/internal/api/secret"
77
"github.com/form3tech-oss/github-team-approver/internal/api/stages/fakegithub"
8-
"github.com/google/go-github/v28/github"
8+
"github.com/google/go-github/v42/github"
99
"github.com/stretchr/testify/require"
1010
"os"
1111
"testing"
@@ -38,8 +38,12 @@ func (c *ClientStage) FakeGHRunning() *ClientStage {
3838
}
3939

4040
func (c *ClientStage) Organisation() *ClientStage {
41+
id := int64(1)
4142
c.fakeGitHub.SetOrg(&fakegithub.Org{
4243
OwnerName: "form3tech",
44+
OrgDetails: &github.Organization{
45+
ID: &id,
46+
},
4347
})
4448

4549
return c

‎internal/api/handler.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import (
66
"errors"
77
"fmt"
88
ghclient "github.com/form3tech-oss/github-team-approver/internal/api/github"
9-
"github.com/google/go-github/v28/github"
9+
"github.com/google/go-github/v42/github"
1010
"github.com/sirupsen/logrus"
1111
"io/ioutil"
1212
"net/http"

‎internal/api/pacts/pull_request_opened_pending.json‎

Lines changed: 68 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,72 @@
5353
"body": "pull_request_approval_rules:\n- target_branches:\n - master\n rules:\n - regex: \"- \\\\[x\\\\] Yes - this change impacts customers\"\n approving_team_handles:\n - CAB - Foo\n - CAB - Bar\n labels:\n - needs-cab-approval\n - regex: \"- \\\\[x\\\\] Yes - this change impacts documentation\"\n approving_team_handles:\n - CAB - Documentation\n labels:\n - needs-doc-approval\n"
5454
}
5555
},
56+
{
57+
"description": "Get Org (#5)",
58+
"request": {
59+
"method": "GET",
60+
"path": "/orgs/form3tech"
61+
},
62+
"response": {
63+
"status": 200,
64+
"headers": {
65+
"Content-Type": "application/json; charset=utf-8"
66+
},
67+
"body": {
68+
"login": "github",
69+
"id": 1,
70+
"node_id": "MDEyOk9yZ2FuaXphdGlvbjE=",
71+
"url": "https://api.github.com/orgs/github",
72+
"repos_url": "https://api.github.com/orgs/github/repos",
73+
"events_url": "https://api.github.com/orgs/github/events",
74+
"hooks_url": "https://api.github.com/orgs/github/hooks",
75+
"issues_url": "https://api.github.com/orgs/github/issues",
76+
"members_url": "https://api.github.com/orgs/github/members{/member}",
77+
"public_members_url": "https://api.github.com/orgs/github/public_members{/member}",
78+
"avatar_url": "https://github.com/images/error/octocat_happy.gif",
79+
"description": "A great organization",
80+
"name": "github",
81+
"company": "GitHub",
82+
"blog": "https://github.com/blog",
83+
"location": "San Francisco",
84+
"email": "octocat@github.com",
85+
"twitter_username": "github",
86+
"is_verified": true,
87+
"has_organization_projects": true,
88+
"has_repository_projects": true,
89+
"public_repos": 2,
90+
"public_gists": 1,
91+
"followers": 20,
92+
"following": 0,
93+
"html_url": "https://github.com/octocat",
94+
"created_at": "2008-01-14T04:33:35Z",
95+
"updated_at": "2014-03-03T18:58:10Z",
96+
"type": "Organization",
97+
"total_private_repos": 100,
98+
"owned_private_repos": 100,
99+
"private_gists": 81,
100+
"disk_usage": 10000,
101+
"collaborators": 8,
102+
"billing_email": "mona@github.com",
103+
"plan": {
104+
"name": "Medium",
105+
"space": 400,
106+
"private_repos": 20,
107+
"filled_seats": 4,
108+
"seats": 5
109+
},
110+
"default_repository_permission": "read",
111+
"members_can_create_repositories": true,
112+
"two_factor_requirement_enabled": true,
113+
"members_allowed_repository_creation_type": "all",
114+
"members_can_create_public_repositories": false,
115+
"members_can_create_private_repositories": false,
116+
"members_can_create_internal_repositories": false,
117+
"members_can_create_pages": true,
118+
"members_can_fork_private_repositories": false
119+
}
120+
}
121+
},
56122
{
57123
"description": "Get Team IDs (Page 1, Per Page: 100) (#5)",
58124
"request": {
@@ -85,7 +151,7 @@
85151
],
86152
"request": {
87153
"method": "GET",
88-
"path": "/teams/1/members"
154+
"path": "/organizations/1/team/1/members"
89155
},
90156
"response": {
91157
"status": 200,
@@ -108,7 +174,7 @@
108174
],
109175
"request": {
110176
"method": "GET",
111-
"path": "/teams/2/members"
177+
"path": "/organizations/1/team/2/members"
112178
},
113179
"response": {
114180
"status": 200,

0 commit comments

Comments
 (0)