diff --git a/pkg/sources/github/connector.go b/pkg/sources/github/connector.go index 126657cf7f55..480ccd2ff494 100644 --- a/pkg/sources/github/connector.go +++ b/pkg/sources/github/connector.go @@ -36,6 +36,9 @@ func newConnector(ctx context.Context, source *Source) (Connector, error) { apiEndpoint = cloudV3Endpoint } + // Ensure GitHub Enterprise Server endpoints include /api/v3 for proper API routing + apiEndpoint = normalizeGitHubEnterpriseEndpoint(apiEndpoint) + switch cred := source.conn.GetCredential().(type) { case *sourcespb.GitHub_GithubApp: log.RedactGlobally(cred.GithubApp.GetPrivateKey()) @@ -90,3 +93,29 @@ func createGraphqlClient(ctx context.Context, client *http.Client, apiEndpoint s return githubv4.NewEnterpriseClient(graphqlEndpoint, client), nil } + +// normalizeGitHubEnterpriseEndpoint ensures GitHub Enterprise Server endpoints include /api/v3 +// This is required for libraries like ghinstallation that construct URLs like: +// {BaseURL}/app/installations/{id}/access_tokens +// +// GitHub Enterprise Server requires: https://hostname/api/v3/app/installations/{id}/access_tokens +// By default, the GitHub Enterprise URL format should be http(s)://[hostname]/api/v3/ or you will +// always receive the 406 status code. +// GitHub.com uses: https://api.github.com/app/installations/{id}/access_tokens +func normalizeGitHubEnterpriseEndpoint(endpoint string) string { + endpoint = strings.TrimSuffix(endpoint, "/") + + // GitHub.com endpoints (api.github.com) don't need /api/v3 appended + // They already use the correct format + if strings.Contains(endpoint, "api.github.com") { + return endpoint + } + + // For GitHub Enterprise Server, ensure /api/v3 is present + // Example: https://github.company.com -> https://github.company.com/api/v3 + if !strings.HasSuffix(endpoint, "/api/v3") { + endpoint = endpoint + "/api/v3" + } + + return endpoint +} diff --git a/pkg/sources/github/connector_test.go b/pkg/sources/github/connector_test.go new file mode 100644 index 000000000000..5c7ebdf0e58e --- /dev/null +++ b/pkg/sources/github/connector_test.go @@ -0,0 +1,91 @@ +package github + +import ( + "testing" +) + +func TestNormalizeGitHubEnterpriseEndpoint(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + // GitHub.com endpoints - should NOT be modified + { + name: "github.com api endpoint", + input: "https://api.github.com", + expected: "https://api.github.com", + }, + { + name: "github.com api endpoint with trailing slash", + input: "https://api.github.com/", + expected: "https://api.github.com", + }, + { + name: "github.com api endpoint with http", + input: "http://api.github.com", + expected: "http://api.github.com", + }, + + // GitHub Enterprise without /api/v3 - should ADD it + { + name: "enterprise endpoint without api/v3", + input: "https://github.company.com", + expected: "https://github.company.com/api/v3", + }, + { + name: "enterprise endpoint with http protocol", + input: "http://github.company.com", + expected: "http://github.company.com/api/v3", + }, + { + name: "enterprise endpoint with trailing slash", + input: "https://github.company.com/", + expected: "https://github.company.com/api/v3", + }, + + // GitHub Enterprise WITH /api/v3 - should NOT modify + { + name: "enterprise endpoint already has api/v3", + input: "https://github.company.com/api/v3", + expected: "https://github.company.com/api/v3", + }, + { + name: "enterprise endpoint with api/v3 and trailing slash", + input: "https://github.company.com/api/v3/", + expected: "https://github.company.com/api/v3", + }, + { + name: "enterprise endpoint with api/v3 (http)", + input: "http://github.company.com/api/v3", + expected: "http://github.company.com/api/v3", + }, + + // Edge cases + { + name: "enterprise subdomain", + input: "https://git.enterprise.example.com", + expected: "https://git.enterprise.example.com/api/v3", + }, + { + name: "enterprise with port", + input: "https://github.company.com:8443", + expected: "https://github.company.com:8443/api/v3", + }, + { + name: "enterprise with port and api/v3", + input: "https://github.company.com:8443/api/v3", + expected: "https://github.company.com:8443/api/v3", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := normalizeGitHubEnterpriseEndpoint(tt.input) + if result != tt.expected { + t.Errorf("normalizeGitHubEnterpriseEndpoint(%q) = %q, want %q", + tt.input, result, tt.expected) + } + }) + } +} diff --git a/pkg/sources/github/github_test.go b/pkg/sources/github/github_test.go index 53e20d6a2401..fa1a53a7afcd 100644 --- a/pkg/sources/github/github_test.go +++ b/pkg/sources/github/github_test.go @@ -805,6 +805,44 @@ func TestEnumerateWithApp(t *testing.T) { assert.True(t, gock.IsDone()) } +func TestEnumerateWithApp_EnterpriseServer(t *testing.T) { + defer gock.Off() + + privateKey := createPrivateKey() + + // Test that the normalized endpoint is used for token refresh + gock.New("https://github.company.com"). + Post("/api/v3/app/installations/1337/access_tokens"). + Reply(200). + JSON(map[string]string{"token": "test-installation-token", "expires_at": "2025-12-31T23:59:59Z"}) + + gock.New("https://github.company.com"). + Get("/api/v3/installation/repositories"). + Reply(200). + JSON(map[string]any{ + "repositories": []map[string]string{ + {"clone_url": "https://github.company.com/org/repo.git", "full_name": "org/repo"}, + }, + }) + + s := initTestSource(&sourcespb.GitHub{ + Endpoint: "https://github.company.com", // No /api/v3, should be normalized + Credential: &sourcespb.GitHub_GithubApp{ + GithubApp: &credentialspb.GitHubApp{ + PrivateKey: privateKey, + InstallationId: "1337", + AppId: "4141", + }, + }, + }) + err := s.enumerateWithApp(context.Background(), s.connector.(*appConnector).InstallationClient(), noopReporter()) + assert.Nil(t, err) + assert.Equal(t, 1, s.filteredRepoCache.Count()) + assert.True(t, s.filteredRepoCache.Exists("org/repo")) + assert.False(t, gock.HasUnmatchedRequest()) + assert.True(t, gock.IsDone()) +} + // This only tests the resume info slice portion of setProgressCompleteWithRepo. func Test_setProgressCompleteWithRepo_resumeInfo(t *testing.T) { tests := []struct { @@ -1103,3 +1141,148 @@ func noopReporter() sources.UnitReporter { }, } } + +func TestNewConnector_NormalizesEnterpriseEndpoint(t *testing.T) { + defer gock.Off() + + tests := []struct { + name string + inputEndpoint string + expectedBaseURL string + credential *sourcespb.GitHub_Token + }{ + { + name: "GitHub.com endpoint unchanged", + inputEndpoint: "https://api.github.com", + expectedBaseURL: "https://api.github.com", + credential: &sourcespb.GitHub_Token{Token: "test-token"}, + }, + { + name: "Enterprise endpoint without /api/v3 gets normalized", + inputEndpoint: "https://github.company.com", + expectedBaseURL: "https://github.company.com/api/v3", + credential: &sourcespb.GitHub_Token{Token: "test-token"}, + }, + { + name: "Enterprise endpoint with /api/v3 unchanged", + inputEndpoint: "https://github.company.com/api/v3", + expectedBaseURL: "https://github.company.com/api/v3", + credential: &sourcespb.GitHub_Token{Token: "test-token"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Mock the /user endpoint since token connector calls it + gock.New(tt.expectedBaseURL). + Get("/user"). + Reply(200). + JSON(map[string]string{"login": "test-user"}) + + s := &Source{ + conn: &sourcespb.GitHub{ + Endpoint: tt.inputEndpoint, + Credential: tt.credential, + }, + } + + connector, err := newConnector(context.Background(), s) + require.NoError(t, err) + require.NotNil(t, connector) + + // Verify the APIClient was created with the normalized endpoint + apiClient := connector.APIClient() + require.NotNil(t, apiClient) + + // For GitHub.com, BaseURL should be api.github.com + // For Enterprise, BaseURL should include /api/v3/ + if strings.Contains(tt.expectedBaseURL, "api.github.com") { + assert.Equal(t, "https://api.github.com/", apiClient.BaseURL.String()) + } else { + assert.Equal(t, tt.expectedBaseURL+"/", apiClient.BaseURL.String()) + } + }) + } +} + +func TestNewConnector_GitHubApp_NormalizesEnterpriseEndpoint(t *testing.T) { + defer gock.Off() + + privateKey := createPrivateKey() + + tests := []struct { + name string + inputEndpoint string + expectedBaseURL string + }{ + { + name: "GitHub.com app endpoint unchanged", + inputEndpoint: "https://api.github.com", + expectedBaseURL: "https://api.github.com", + }, + { + name: "Enterprise app endpoint without /api/v3 gets normalized", + inputEndpoint: "https://git.random.ch", + expectedBaseURL: "https://git.random.ch/api/v3", + }, + { + name: "Enterprise app endpoint with /api/v3 unchanged", + inputEndpoint: "https://git.random.ch/api/v3", + expectedBaseURL: "https://git.random.ch/api/v3", + }, + { + name: "Enterprise app endpoint with trailing slash", + inputEndpoint: "https://github.company.com/", + expectedBaseURL: "https://github.company.com/api/v3", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Mock the token refresh endpoint that ghinstallation library calls + gock.New(tt.expectedBaseURL). + Post("/app/installations/1337/access_tokens"). + Reply(200). + JSON(map[string]string{"token": "test-token", "expires_at": "2025-12-31T23:59:59Z"}) + + // Mock the installations endpoint + gock.New(tt.expectedBaseURL). + Get("/app/installations"). + Reply(200). + JSON([]map[string]any{}) + + s := &Source{ + conn: &sourcespb.GitHub{ + Endpoint: tt.inputEndpoint, + Credential: &sourcespb.GitHub_GithubApp{ + GithubApp: &credentialspb.GitHubApp{ + PrivateKey: privateKey, + InstallationId: "1337", + AppId: "4141", + }, + }, + }, + } + + connector, err := newConnector(context.Background(), s) + require.NoError(t, err) + require.NotNil(t, connector) + + appConn, ok := connector.(*appConnector) + require.True(t, ok, "expected appConnector type") + require.NotNil(t, appConn.APIClient()) + require.NotNil(t, appConn.InstallationClient()) + + // Verify the API client was created with the normalized endpoint + apiClient := appConn.APIClient() + if strings.Contains(tt.expectedBaseURL, "api.github.com") { + assert.Equal(t, "https://api.github.com/", apiClient.BaseURL.String()) + } else { + assert.Equal(t, tt.expectedBaseURL+"/", apiClient.BaseURL.String()) + } + + // Verify no unmatched HTTP mocks (all expected calls were made) + assert.False(t, gock.HasUnmatchedRequest(), "had unmatched HTTP requests") + }) + } +}