Skip to content

✨ Enable GitHub Enterprise Server (GHES) support #2788

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.

Sign up for GitHub

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

Closed
rajbos wants to merge 4 commits into from
Closed

✨ Enable GitHub Enterprise Server (GHES) support #2788

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b443372
Support GitHub Enterprise Server hosts
rajbos Mar 24, 2023
8ab5e6d
Handle error
rajbos Apr 13, 2023
630d00d
Fix typo
rajbos Apr 13, 2023
b663652
Merge branch 'main' into ghes-support
rajbos Apr 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 30 additions & 3 deletions clients/githubrepo/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import (
"errors"
"fmt"
"net/http"
"os"
"strings"
"time"

"github.com/google/go-github/v38/github"
Expand Down Expand Up @@ -126,7 +128,7 @@ func (client *Client) InitRepo(inputRepo clients.Repo, commitSHA string, commitD

// URI implements RepoClient.URI.
func (client *Client) URI() string {
return fmt.Sprintf("github.com/%s/%s", client.repourl.owner, client.repourl.repo)
return fmt.Sprintf("%s/%s/%s", client.repourl.host, client.repourl.owner, client.repourl.repo)
Copy link
Member

@naveensrinivasan naveensrinivasan Apr 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please write a test for this?

}

// LocalPath implements RepoClient.LocalPath.
Expand Down Expand Up @@ -259,8 +261,33 @@ func CreateGithubRepoClientWithTransport(ctx context.Context, rt http.RoundTripp
httpClient := &http.Client{
Transport: rt,
}
client := github.NewClient(httpClient)
graphClient := githubv4.NewClient(httpClient)

Copy link
Member

@naveensrinivasan naveensrinivasan Apr 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please include tests for these?

Copy link
Author

@rajbos rajbos May 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@naveensrinivasan , can you give me some pointers / help on where to get started? (not a Go programmer, so testing in Go is new to me).

I see a generic setup for creating the clients during the tests here: clients/githubrepo/githubrepo_suite_test.go, and another generic mock here clients/mockclients/repo_client.go.

var client *github.Client
var graphClient *githubv4.Client
// check if GITHUB_API_URL is set
if githubAPIURL := os.Getenv("GITHUB_API_URL"); githubAPIURL != "https://api.github.com" {
// create a new enterprise client with custom URLs
githubSERVERURL := os.Getenv("GITHUB_SERVER_URL")
if githubSERVERURL == "" {
// load the server url from the api url
githubSERVERURL = strings.TrimSuffix(githubAPIURL, "/api/v3")
}
// trim trailing slash to prevent issues with the graphql client
githubSERVERURL = strings.TrimSuffix(githubSERVERURL, "/")
githubGRAPHQLURL := fmt.Sprintf("%s/api/graphql", githubSERVERURL)
Copy link

@esnible esnible Jun 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line didn't work for me. To run the scorecards, I used GITHUB_API_URL=https://api.github.ibm.com and changed this line to

githubGRAPHQLURL := fmt.Sprintf("%s/graphql", githubSERVERURL)

Using "%s/api/graphql" caused 404s, using /graphql allowed the scorecard to generate. I only have access to my own company's GHE, so I don't know if you should always use /graphql or if the path needs to be parameterized for different configurations.


var err error
client, err = github.NewEnterpriseClient(githubAPIURL, githubAPIURL, httpClient)
if err != nil {
panic(fmt.Errorf("error during CreateGithubRepoClientWithTransport: %v", err))
}

graphClient = githubv4.NewEnterpriseClient(githubGRAPHQLURL, httpClient)
} else {
// use the defaul url values from the github client
client = github.NewClient(httpClient)
graphClient = githubv4.NewClient(httpClient)
}

return &Client{
ctx: ctx,
Expand Down
6 changes: 0 additions & 6 deletions clients/githubrepo/repo.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,6 @@ func (r *repoURL) String() string {

// IsValid implements Repo.IsValid.
func (r *repoURL) IsValid() error {
switch r.host {
case "github.com":
default:
return sce.WithMessage(sce.ErrorUnsupportedHost, r.host)
}

if strings.TrimSpace(r.owner) == "" || strings.TrimSpace(r.repo) == "" {
return sce.WithMessage(sce.ErrorInvalidURL,
fmt.Sprintf("%v. Expected the full repository url", r.URI()))
Expand Down