Skip to content

Commit ee897f0

Browse files
luxasluxas
authored
Merge pull request #26 from fluxcd/github_client_interface
Factor out a high-level githubClient interface for all operations.
2 parents 971412c + 13f3fbe commit ee897f0

13 files changed

+432
-208
lines changed

‎github/auth.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,9 @@ func buildTransportChain(ctx context.Context, opts *clientOptions) (http.RoundTr
305305

306306
// If a custom roundtripper was set, pipe it through the transport too
307307
if opts.RoundTripperFactory != nil {
308+
// TODO: Document usage of a nil transport here
309+
// TODO: Provide some kind of debug logging if/when the httpcache is used
310+
// One can see if the request hit the cache using: resp.Header[httpcache.XFromCache]
308311
customTransport := opts.RoundTripperFactory.Transport(transport)
309312
if customTransport == nil {
310313
// The lint failure here is a false positive, for some (unknown) reason

‎github/client.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ import (
2626
const ProviderID = gitprovider.ProviderID("github")
2727

2828
func newClient(c *github.Client, domain string, destructiveActions bool) *Client {
29-
ctx := &clientContext{c, domain, destructiveActions}
29+
ghClient := &githubClientImpl{c, destructiveActions}
30+
ctx := &clientContext{ghClient, domain, destructiveActions}
3031
return &Client{
3132
clientContext: ctx,
3233
orgs: &OrganizationsClient{
@@ -42,7 +43,7 @@ func newClient(c *github.Client, domain string, destructiveActions bool) *Client
4243
}
4344

4445
type clientContext struct {
45-
c *github.Client
46+
c githubClient
4647
domain string
4748
destructiveActions bool
4849
}

‎github/client_organization_teams.go‎

Lines changed: 8 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ package github
1818

1919
import (
2020
"context"
21-
"fmt"
2221

2322
"github.com/google/go-github/v32/github"
2423

@@ -41,25 +40,16 @@ type TeamsClient struct {
4140
//
4241
// ErrNotFound is returned if the resource does not exist.
4342
func (c *TeamsClient) Get(ctx context.Context, teamName string) (gitprovider.Team, error) {
44-
apiObjs := []*github.User{}
45-
opts := &github.TeamListTeamMembersOptions{}
46-
err := allPages(&opts.ListOptions, func() (*github.Response, error) {
47-
// GET /orgs/{org}/teams/{team_slug}/members
48-
pageObjs, resp, listErr := c.c.Teams.ListTeamMembersBySlug(ctx, c.ref.Organization, teamName, opts)
49-
apiObjs = append(apiObjs, pageObjs...)
50-
return resp, listErr
51-
})
43+
// GET /orgs/{org}/teams/{team_slug}/members
44+
apiObjs, err := c.c.ListOrgTeamMembers(ctx, c.ref.Organization, teamName)
5245
if err != nil {
5346
return nil, err
5447
}
5548

49+
// Collect a list of the members' names. Login is validated to be non-nil in ListOrgTeamMembers.
5650
logins := make([]string, 0, len(apiObjs))
5751
for _, apiObj := range apiObjs {
58-
// Make sure login isn't nil
59-
if apiObj.Login == nil {
60-
return nil, fmt.Errorf("didn't expect login to be nil for user: %+v: %w", apiObj, gitprovider.ErrInvalidServerData)
61-
}
62-
52+
// Login is validated to be non-nil in ListOrgTeamMembers
6353
logins = append(logins, *apiObj.Login)
6454
}
6555

@@ -77,28 +67,17 @@ func (c *TeamsClient) Get(ctx context.Context, teamName string) (gitprovider.Tea
7767
//
7868
// List returns all available organizations, using multiple paginated requests if needed.
7969
func (c *TeamsClient) List(ctx context.Context) ([]gitprovider.Team, error) {
80-
// List all teams, using pagination. This does not contain information about the members
81-
apiObjs := []*github.Team{}
82-
opts := &github.ListOptions{}
83-
err := allPages(opts, func() (*github.Response, error) {
84-
// GET /orgs/{org}/teams
85-
pageObjs, resp, listErr := c.c.Teams.ListTeams(ctx, c.ref.Organization, opts)
86-
apiObjs = append(apiObjs, pageObjs...)
87-
return resp, listErr
88-
})
70+
// GET /orgs/{org}/teams
71+
apiObjs, err := c.c.ListOrgTeams(ctx, c.ref.Organization)
8972
if err != nil {
9073
return nil, err
9174
}
9275

9376
// Use .Get() to get detailed information about each member
9477
teams := make([]gitprovider.Team, 0, len(apiObjs))
9578
for _, apiObj := range apiObjs {
96-
// Make sure name isn't nil
97-
if apiObj.Slug == nil {
98-
return nil, fmt.Errorf("didn't expect slug to be nil for team: %+v: %w", apiObj, gitprovider.ErrInvalidServerData)
99-
}
100-
101-
// Get information about individual teams
79+
// Get detailed information about individual teams (including members).
80+
// Slug is validated to be non-nil in ListOrgTeams.
10281
team, err := c.Get(ctx, *apiObj.Slug)
10382
if err != nil {
10483
return nil, err

‎github/client_organizations.go‎

Lines changed: 5 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,6 @@ package github
1818

1919
import (
2020
"context"
21-
"fmt"
22-
23-
"github.com/google/go-github/v32/github"
2421

2522
"github.com/fluxcd/go-git-providers/gitprovider"
2623
)
@@ -44,11 +41,9 @@ func (c *OrganizationsClient) Get(ctx context.Context, ref gitprovider.Organizat
4441
}
4542

4643
// GET /orgs/{org}
47-
apiObj, _, err := c.c.Organizations.Get(ctx, ref.Organization)
48-
// TODO: Provide some kind of debug logging if/when the httpcache is used
49-
// One can see if the request hit the cache using: resp.Header[httpcache.XFromCache]
44+
apiObj, err := c.c.GetOrg(ctx, ref.Organization)
5045
if err != nil {
51-
return nil, handleHTTPError(err)
46+
return nil, err
5247
}
5348

5449
return newOrganization(c.clientContext, apiObj, ref), nil
@@ -58,25 +53,15 @@ func (c *OrganizationsClient) Get(ctx context.Context, ref gitprovider.Organizat
5853
//
5954
// List returns all available organizations, using multiple paginated requests if needed.
6055
func (c *OrganizationsClient) List(ctx context.Context) ([]gitprovider.Organization, error) {
61-
apiObjs := []*github.Organization{}
62-
opts := &github.ListOptions{}
63-
err := allPages(opts, func() (*github.Response, error) {
64-
// GET /user/orgs
65-
pageObjs, resp, listErr := c.c.Organizations.List(ctx, "", opts)
66-
apiObjs = append(apiObjs, pageObjs...)
67-
return resp, listErr
68-
})
56+
// GET /user/orgs
57+
apiObjs, err := c.c.ListOrgs(ctx)
6958
if err != nil {
7059
return nil, err
7160
}
7261

7362
orgs := make([]gitprovider.Organization, 0, len(apiObjs))
7463
for _, apiObj := range apiObjs {
75-
// Make sure name isn't nil
76-
if apiObj.Login == nil {
77-
return nil, fmt.Errorf("didn't expect login to be nil for org: %+v: %w", apiObj, gitprovider.ErrInvalidServerData)
78-
}
79-
64+
// apiObj.Login is already validated to be non-nil in ListOrgs
8065
orgs = append(orgs, newOrganization(c.clientContext, apiObj, gitprovider.OrganizationRef{
8166
Domain: c.domain,
8267
Organization: *apiObj.Login,

‎github/client_repositories_org.go‎

Lines changed: 7 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,8 @@ func (c *OrgRepositoriesClient) Get(ctx context.Context, ref gitprovider.OrgRepo
4141
if err := validateOrgRepositoryRef(ref, c.domain); err != nil {
4242
return nil, err
4343
}
44-
apiObj, err := getRepository(ctx, c.c, ref)
44+
// GET /repos/{owner}/{repo}
45+
apiObj, err := c.c.GetRepo(ctx, ref.GetIdentity(), ref.GetRepository())
4546
if err != nil {
4647
return nil, err
4748
}
@@ -57,27 +58,16 @@ func (c *OrgRepositoriesClient) List(ctx context.Context, ref gitprovider.Organi
5758
return nil, err
5859
}
5960

60-
// Get all of the repositories in the organization using pagination.
61-
var apiObjs []*github.Repository
62-
opts := &github.RepositoryListByOrgOptions{}
63-
err := allPages(&opts.ListOptions, func() (*github.Response, error) {
64-
// GET /orgs/{org}/repos
65-
pageObjs, resp, listErr := c.c.Repositories.ListByOrg(ctx, ref.Organization, opts)
66-
apiObjs = append(apiObjs, pageObjs...)
67-
return resp, listErr
68-
})
61+
// GET /orgs/{org}/repos
62+
apiObjs, err := c.c.ListOrgRepos(ctx, ref.Organization)
6963
if err != nil {
7064
return nil, err
7165
}
7266

7367
// Traverse the list, and return a list of OrgRepository objects
7468
repos := make([]gitprovider.OrgRepository, 0, len(apiObjs))
7569
for _, apiObj := range apiObjs {
76-
// Make sure apiObj is valid
77-
if err := validateRepositoryAPI(apiObj); err != nil {
78-
return nil, err
79-
}
80-
70+
// apiObj is already validated at ListOrgRepos
8171
repos = append(repos, newOrgRepository(c.clientContext, apiObj, gitprovider.OrgRepositoryRef{
8272
OrganizationRef: ref,
8373
RepositoryName: *apiObj.Name,
@@ -130,13 +120,7 @@ func (c *OrgRepositoriesClient) Reconcile(ctx context.Context, ref gitprovider.O
130120
return actual, actionTaken, err
131121
}
132122

133-
func getRepository(ctx context.Context, c *github.Client, ref gitprovider.RepositoryRef) (*github.Repository, error) {
134-
// GET /repos/{owner}/{repo}
135-
apiObj, _, err := c.Repositories.Get(ctx, ref.GetIdentity(), ref.GetRepository())
136-
return validateRepositoryAPIResp(apiObj, err)
137-
}
138-
139-
func createRepository(ctx context.Context, c *github.Client, ref gitprovider.RepositoryRef, orgName string, req gitprovider.RepositoryInfo, opts ...gitprovider.RepositoryCreateOption) (*github.Repository, error) {
123+
func createRepository(ctx context.Context, c githubClient, ref gitprovider.RepositoryRef, orgName string, req gitprovider.RepositoryInfo, opts ...gitprovider.RepositoryCreateOption) (*github.Repository, error) {
140124
// First thing, validate and default the request to ensure a valid and fully-populated object
141125
// (to minimize any possible diffs between desired and actual state)
142126
if err := gitprovider.ValidateAndDefaultInfo(&req); err != nil {
@@ -153,15 +137,7 @@ func createRepository(ctx context.Context, c *github.Client, ref gitprovider.Rep
153137
data := repositoryToAPI(&req, ref)
154138
applyRepoCreateOptions(&data, o)
155139

156-
return createRepositoryData(ctx, c, orgName, &data)
157-
}
158-
159-
func createRepositoryData(ctx context.Context, c *github.Client, orgName string, data *github.Repository) (*github.Repository, error) {
160-
// POST /user/repos or
161-
// POST /orgs/{org}/repos
162-
// depending on orgName
163-
apiObj, _, err := c.Repositories.Create(ctx, orgName, data)
164-
return validateRepositoryAPIResp(apiObj, err)
140+
return c.CreateRepo(ctx, orgName, &data)
165141
}
166142

167143
func reconcileRepository(ctx context.Context, actual gitprovider.UserRepository, req gitprovider.RepositoryInfo) (bool, error) {

‎github/client_repositories_user.go‎

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@ import (
2020
"context"
2121
"errors"
2222

23-
"github.com/google/go-github/v32/github"
24-
2523
"github.com/fluxcd/go-git-providers/gitprovider"
2624
)
2725

@@ -41,7 +39,8 @@ func (c *UserRepositoriesClient) Get(ctx context.Context, ref gitprovider.UserRe
4139
if err := validateUserRepositoryRef(ref, c.domain); err != nil {
4240
return nil, err
4341
}
44-
apiObj, err := getRepository(ctx, c.c, ref)
42+
// GET /repos/{owner}/{repo}
43+
apiObj, err := c.c.GetRepo(ctx, ref.GetIdentity(), ref.GetRepository())
4544
if err != nil {
4645
return nil, err
4746
}
@@ -57,27 +56,16 @@ func (c *UserRepositoriesClient) List(ctx context.Context, ref gitprovider.UserR
5756
return nil, err
5857
}
5958

60-
// Get all of the user's repositories using pagination.
61-
var apiObjs []*github.Repository
62-
opts := &github.RepositoryListOptions{}
63-
err := allPages(&opts.ListOptions, func() (*github.Response, error) {
64-
// GET /users/{username}/repos
65-
pageObjs, resp, listErr := c.c.Repositories.List(ctx, ref.GetIdentity(), opts)
66-
apiObjs = append(apiObjs, pageObjs...)
67-
return resp, listErr
68-
})
59+
// GET /users/{username}/repos
60+
apiObjs, err := c.c.ListUserRepos(ctx, ref.UserLogin)
6961
if err != nil {
7062
return nil, err
7163
}
7264

7365
// Traverse the list, and return a list of UserRepository objects
7466
repos := make([]gitprovider.UserRepository, 0, len(apiObjs))
7567
for _, apiObj := range apiObjs {
76-
// Make sure apiObj is valid
77-
if err := validateRepositoryAPI(apiObj); err != nil {
78-
return nil, err
79-
}
80-
68+
// apiObj is already validated at ListUserRepos
8169
repos = append(repos, newUserRepository(c.clientContext, apiObj, gitprovider.UserRepositoryRef{
8270
UserRef: ref,
8371
RepositoryName: *apiObj.Name,

‎github/client_repository_deploykey.go‎

Lines changed: 8 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ func (c *DeployKeyClient) List(ctx context.Context) ([]gitprovider.DeployKey, er
6464
if err != nil {
6565
return nil, err
6666
}
67-
// Cast to []gitprovider.DeployKey
67+
// Cast to the generic []gitprovider.DeployKey
6868
keys := make([]gitprovider.DeployKey, 0, len(dks))
6969
for _, dk := range dks {
7070
keys = append(keys, dk)
@@ -73,27 +73,17 @@ func (c *DeployKeyClient) List(ctx context.Context) ([]gitprovider.DeployKey, er
7373
}
7474

7575
func (c *DeployKeyClient) list(ctx context.Context) ([]*deployKey, error) {
76-
// List all keys, using pagination.
77-
apiObjs := []*github.Key{}
78-
opts := &github.ListOptions{}
79-
err := allPages(opts, func() (*github.Response, error) {
80-
// GET /repos/{owner}/{repo}/keys
81-
pageObjs, resp, listErr := c.c.Repositories.ListKeys(ctx, c.ref.GetIdentity(), c.ref.GetRepository(), opts)
82-
apiObjs = append(apiObjs, pageObjs...)
83-
return resp, listErr
84-
})
76+
// GET /repos/{owner}/{repo}/keys
77+
apiObjs, err := c.c.ListKeys(ctx, c.ref.GetIdentity(), c.ref.GetRepository())
8578
if err != nil {
8679
return nil, err
8780
}
8881

8982
// Map the api object to our DeployKey type
9083
keys := make([]*deployKey, 0, len(apiObjs))
9184
for _, apiObj := range apiObjs {
92-
k, err := newDeployKey(c, apiObj)
93-
if err != nil {
94-
return nil, err
95-
}
96-
keys = append(keys, k)
85+
// apiObj is already validated at ListKeys
86+
keys = append(keys, newDeployKey(c, apiObj))
9787
}
9888

9989
return keys, nil
@@ -107,7 +97,7 @@ func (c *DeployKeyClient) Create(ctx context.Context, req gitprovider.DeployKeyI
10797
if err != nil {
10898
return nil, err
10999
}
110-
return newDeployKey(c, apiObj)
100+
return newDeployKey(c, apiObj), nil
111101
}
112102

113103
// Reconcile makes sure the given desired state (req) becomes the actual state in the backing Git provider.
@@ -148,18 +138,12 @@ func (c *DeployKeyClient) Reconcile(ctx context.Context, req gitprovider.DeployK
148138
return actual, true, actual.Update(ctx)
149139
}
150140

151-
func createDeployKey(ctx context.Context, c *github.Client, ref gitprovider.RepositoryRef, req gitprovider.DeployKeyInfo) (*github.Key, error) {
141+
func createDeployKey(ctx context.Context, c githubClient, ref gitprovider.RepositoryRef, req gitprovider.DeployKeyInfo) (*github.Key, error) {
152142
// First thing, validate and default the request to ensure a valid and fully-populated object
153143
// (to minimize any possible diffs between desired and actual state)
154144
if err := gitprovider.ValidateAndDefaultInfo(&req); err != nil {
155145
return nil, err
156146
}
157-
158-
return createDeployKeyData(ctx, c, ref, deployKeyToAPI(&req))
159-
}
160-
161-
func createDeployKeyData(ctx context.Context, c *github.Client, ref gitprovider.RepositoryRef, data *github.Key) (*github.Key, error) {
162147
// POST /repos/{owner}/{repo}/keys
163-
apiObj, _, err := c.Repositories.CreateKey(ctx, ref.GetIdentity(), ref.GetRepository(), data)
164-
return apiObj, handleHTTPError(err)
148+
return c.CreateKey(ctx, ref.GetIdentity(), ref.GetRepository(), deployKeyToAPI(&req))
165149
}

0 commit comments

Comments
 (0)