[GitHub] Add a GraphQL client to the connector #3837
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,17 +1,92 @@ | ||
| package github | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "net/http" | ||
| "net/url" | ||
| "strings" | ||
|
|
||
| gogit "github.com/go-git/go-git/v5" | ||
| "github.com/google/go-github/v67/github" | ||
| "github.com/shurcooL/githubv4" | ||
|
|
||
| "github.com/trufflesecurity/trufflehog/v3/pkg/context" | ||
| "github.com/trufflesecurity/trufflehog/v3/pkg/log" | ||
| "github.com/trufflesecurity/trufflehog/v3/pkg/pb/sourcespb" | ||
| ) | ||
|
|
||
| const ( | ||
| cloudV3Endpoint = "https://api.github.com" | ||
| cloudGraphqlEndpoint = "https://api.github.com/graphql" // https://docs.github.com/en/graphql/guides/forming-calls-with-graphql#the-graphql-endpoint | ||
| ) | ||
|
|
||
| // Connector abstracts over the authenticated ways to interact with GitHub: cloning and API operations. | ||
| type Connector interface { | ||
| // APIClient returns a configured GitHub client that can be used for GitHub API operations. | ||
| APIClient() *github.Client | ||
| // GraphQLClient returns a client that can be used for GraphQL operations. | ||
| GraphQLClient() *githubv4.Client | ||
| // Clone clones a repository using the configured authentication information. | ||
| Clone(ctx context.Context, repoURL string, args ...string) (string, *gogit.Repository, error) | ||
| } | ||
|
|
||
| func newConnector(ctx context.Context, source *Source) (Connector, error) { | ||
| apiEndpoint := source.conn.Endpoint | ||
| if apiEndpoint == "" || endsWithGithub.MatchString(apiEndpoint) { | ||
| apiEndpoint = cloudV3Endpoint | ||
| } | ||
|
|
||
| switch cred := source.conn.GetCredential().(type) { | ||
| case *sourcespb.GitHub_GithubApp: | ||
| log.RedactGlobally(cred.GithubApp.GetPrivateKey()) | ||
| return NewAppConnector(ctx, apiEndpoint, cred.GithubApp) | ||
| case *sourcespb.GitHub_BasicAuth: | ||
| log.RedactGlobally(cred.BasicAuth.GetPassword()) | ||
| return NewBasicAuthConnector(ctx, apiEndpoint, source.conn.GetClonePath(), cred.BasicAuth) | ||
| case *sourcespb.GitHub_Token: | ||
| log.RedactGlobally(cred.Token) | ||
| return NewTokenConnector(ctx, apiEndpoint, cred.Token, source.conn.GetClonePath(), source.useAuthInUrl, func(c context.Context, err error) bool { | ||
| return source.handleRateLimit(c, err) | ||
| }) | ||
| case *sourcespb.GitHub_Unauthenticated: | ||
| return NewUnauthenticatedConnector(ctx, apiEndpoint, source.conn.GetClonePath()) | ||
| default: | ||
| return nil, fmt.Errorf("unknown connection type %T", source.conn.GetCredential()) | ||
| } | ||
| } | ||
|
|
||
| func createAPIClient(ctx context.Context, httpClient *http.Client, apiEndpoint string) (*github.Client, error) { | ||
| ctx.Logger().V(2).Info("Creating API client", "url", apiEndpoint) | ||
|
|
||
| // If we're using public GitHub, make a regular client. | ||
| // Otherwise, make an enterprise client. | ||
| if strings.EqualFold(apiEndpoint, cloudV3Endpoint) { | ||
| return github.NewClient(httpClient), nil | ||
| } | ||
|
|
||
| return github.NewClient(httpClient).WithEnterpriseURLs(apiEndpoint, apiEndpoint) | ||
| } | ||
|
|
||
| func createGraphqlClient(ctx context.Context, client *http.Client, apiEndpoint string) (*githubv4.Client, error) { | ||
| var graphqlEndpoint string | ||
| if apiEndpoint == cloudV3Endpoint { | ||
| graphqlEndpoint = cloudGraphqlEndpoint | ||
| } else { | ||
| // Use the root endpoint for the host. | ||
| // https://docs.github.com/en/[email protected]/graphql/guides/introduction-to-graphql | ||
| parsedURL, err := url.Parse(apiEndpoint) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("could not create GraphQL client: %w", err) | ||
| } | ||
|
|
||
| // GitHub Enterprise uses `/api/v3` for the base. (https://github.com/google/go-github/issues/958) | ||
| // Swap it, and anything before `/api`, with GraphQL. | ||
| before, _ := strings.CutSuffix(parsedURL.Path, "/api/v3") | ||
| parsedURL.Path = before + "/api/graphql" | ||
| graphqlEndpoint = parsedURL.String() | ||
| } | ||
|
|
||
| ctx.Logger().V(2).Info("Creating GraphQL client", "url", graphqlEndpoint) | ||
|
|
||
| return githubv4.NewEnterpriseClient(graphqlEndpoint, client), nil | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't decide if this is very nitpicky or a strong commitment to consistency, buuuuut I think we could improve this little thing by doing:
...and then anything calling
createGraphqlClientcan then wrap that with "error creating GraphQL client: %w". It is definitely double wrapping, but for someone looking into what might have gone wrong, it's more explicit about what happened and that's super helpful.Option 2 IMO is, it seems like most of what this function does is validate a REST API url and convert it to a GraphQL endpoint URL. That could be its own function, which would be very testable, and then an "error parsing URL" error makes even more sense. Or, it might not need its own function--there's a very natural spot for it up in
newConnector(line 38-ish), and then you can pass a known-validgraphqlEndpointURL to the different connector constructors, which then don't have to check for errors when creating their GraphQL clients as it (apparently) can't error.I appreciate that's more work though? I think just the little fix from option 1 is 👌🏻