diff --git a/pkg/provider/github/app/token.go b/pkg/provider/github/app/token.go index 949b4eecd4..2bb2ca70e4 100644 --- a/pkg/provider/github/app/token.go +++ b/pkg/provider/github/app/token.go @@ -10,29 +10,33 @@ import ( "github.com/golang-jwt/jwt/v4" "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1" + "github.com/openshift-pipelines/pipelines-as-code/pkg/events" "github.com/openshift-pipelines/pipelines-as-code/pkg/params" "github.com/openshift-pipelines/pipelines-as-code/pkg/provider/github" + "go.uber.org/zap" "knative.dev/pkg/logging" ) type Install struct { - request *http.Request - run *params.Run - repo *v1alpha1.Repository - ghClient *github.Provider - namespace string + request *http.Request + run *params.Run + repo *v1alpha1.Repository + ghClient *github.Provider + namespace string + eventEmitter *events.EventEmitter } -func NewInstallation(req *http.Request, run *params.Run, repo *v1alpha1.Repository, gh *github.Provider, namespace string) *Install { +func NewInstallation(req *http.Request, run *params.Run, repo *v1alpha1.Repository, gh *github.Provider, namespace string, eventEmitter *events.EventEmitter) *Install { if req == nil { req = &http.Request{} } return &Install{ - request: req, - run: run, - repo: repo, - ghClient: gh, - namespace: namespace, + request: req, + run: run, + repo: repo, + ghClient: gh, + namespace: namespace, + eventEmitter: eventEmitter, } } @@ -95,7 +99,14 @@ func (ip *Install) GetAndUpdateInstallationID(ctx context.Context) (string, stri installationID := *installation.ID token, err := ip.ghClient.GetAppToken(ctx, ip.run.Clients.Kube, enterpriseHost, installationID, ip.namespace) if err != nil { - logger.Warnf("Could not get a token for installation ID %d: %v", installationID, err) + msg := fmt.Sprintf("Could not get a token for installation ID %d: %v", installationID, err) + logger.Warn(msg) + + // Emit an event for better visibility of GitHub App token issues + if ip.eventEmitter != nil { + ip.eventEmitter.EmitMessage(ip.repo, zap.WarnLevel, "GitHubAppTokenGenerationFailed", msg) + } + // Return with the installation ID even if token generation fails, // as some operations might only need the ID. return enterpriseHost, "", installationID, nil diff --git a/pkg/provider/github/app/token_test.go b/pkg/provider/github/app/token_test.go index 8bea5e84dc..5c6457ef74 100644 --- a/pkg/provider/github/app/token_test.go +++ b/pkg/provider/github/app/token_test.go @@ -144,7 +144,7 @@ func Test_GenerateJWT(t *testing.T) { }, } - ip := NewInstallation(httptest.NewRequest(http.MethodGet, "http://localhost", strings.NewReader("")), run, &v1alpha1.Repository{}, &github.Provider{}, tt.namespace.GetName()) + ip := NewInstallation(httptest.NewRequest(http.MethodGet, "http://localhost", strings.NewReader("")), run, &v1alpha1.Repository{}, &github.Provider{}, tt.namespace.GetName(), nil) token, err := ip.GenerateJWT(ctx) if tt.wantErr { assert.Assert(t, err != nil) @@ -206,7 +206,7 @@ func Test_GetAndUpdateInstallationID(t *testing.T) { ctx = info.StoreCurrentControllerName(ctx, "default") ctx = info.StoreNS(ctx, testNamespace.GetName()) - ip := NewInstallation(httptest.NewRequest(http.MethodGet, "http://localhost", strings.NewReader("")), run, &v1alpha1.Repository{}, &github.Provider{}, testNamespace.GetName()) + ip := NewInstallation(httptest.NewRequest(http.MethodGet, "http://localhost", strings.NewReader("")), run, &v1alpha1.Repository{}, &github.Provider{}, testNamespace.GetName(), nil) jwtToken, err := ip.GenerateJWT(ctx) assert.NilError(t, err) req := httptest.NewRequest(http.MethodGet, "http://localhost", strings.NewReader("")) @@ -256,7 +256,7 @@ func Test_GetAndUpdateInstallationID(t *testing.T) { `{"total_count": 2,"repositories": [{"id":1,"html_url": "https://matched/%s/incoming"},{"id":2,"html_url": "https://anotherrepo/that/would/failit"}]}`, orgName) }) - ip = NewInstallation(req, run, repo, gprovider, testNamespace.GetName()) + ip = NewInstallation(req, run, repo, gprovider, testNamespace.GetName(), nil) _, token, installationID, err := ip.GetAndUpdateInstallationID(ctx) assert.NilError(t, err) assert.Equal(t, installationID, int64(wantID)) @@ -294,6 +294,24 @@ func TestGetAndUpdateInstallationID_Fallbacks(t *testing.T) { skip bool expectedErrorString string }{ + { + name: "repo installation succeeds directly", + repoURL: fmt.Sprintf("https://matched/%s/%s", orgName, repoName), + setupMux: func(mux *http.ServeMux, jwtToken string) { + mux.HandleFunc(fmt.Sprintf("/repos/%s/%s/installation", orgName, repoName), func(w http.ResponseWriter, _ *http.Request) { + _, _ = fmt.Fprintf(w, `{"id": %d}`, orgID) + }) + mux.HandleFunc(fmt.Sprintf("/app/installations/%d/access_tokens", orgID), func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r) + w.Header().Set("Authorization", "Bearer "+jwtToken) + w.Header().Set("Accept", "application/vnd.github+json") + _, _ = fmt.Fprintf(w, `{"token": "%s"}`, wantToken) + }) + }, + wantErr: false, + wantInstallationID: orgID, + wantToken: wantToken, + }, { name: "repo installation fails, org installation succeeds", repoURL: fmt.Sprintf("https://matched/%s/%s", orgName, repoName), @@ -357,13 +375,40 @@ func TestGetAndUpdateInstallationID_Fallbacks(t *testing.T) { expectedErrorString: "could not find repository, organization or user installation", }, { - name: "invalid repo url", - repoURL: "https://invalid/url", - setupMux: func(_ *http.ServeMux, _ string) { + name: "repo installation succeeds but token generation fails", + repoURL: fmt.Sprintf("https://matched/%s/%s", orgName, repoName), + setupMux: func(mux *http.ServeMux, jwtToken string) { + mux.HandleFunc(fmt.Sprintf("/repos/%s/%s/installation", orgName, repoName), func(w http.ResponseWriter, _ *http.Request) { + _, _ = fmt.Fprintf(w, `{"id": %d}`, orgID) + }) + mux.HandleFunc(fmt.Sprintf("/app/installations/%d/access_tokens", orgID), func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r) + w.Header().Set("Authorization", "Bearer "+jwtToken) + w.Header().Set("Accept", "application/vnd.github+json") + w.WriteHeader(http.StatusForbidden) + _, _ = fmt.Fprintf(w, `{"message": "Forbidden"}`) + }) }, - wantErr: true, - expectedErrorString: "invalid repository URL path", + wantErr: false, // Should not error, but return empty token + wantInstallationID: orgID, + wantToken: "", // Empty token when generation fails }, + { + name: "invalid repo url - too few path segments", + repoURL: "https://invalid/url", + setupMux: func(_ *http.ServeMux, _ string) { + }, + wantErr: true, + expectedErrorString: "invalid repository URL path", + }, + { + name: "invalid GitHub repo url - too many path segments", + repoURL: "https://github.com/group/subgroup/repo", + setupMux: func(_ *http.ServeMux, _ string) { + }, + wantErr: true, + expectedErrorString: "invalid repository URL path", + }, } for _, tt := range tests { @@ -393,7 +438,7 @@ func TestGetAndUpdateInstallationID_Fallbacks(t *testing.T) { ctx = info.StoreCurrentControllerName(ctx, "default") ctx = info.StoreNS(ctx, testNamespace.GetName()) - ip := NewInstallation(httptest.NewRequest(http.MethodGet, "http://localhost", strings.NewReader("")), run, &v1alpha1.Repository{}, &github.Provider{}, testNamespace.GetName()) + ip := NewInstallation(httptest.NewRequest(http.MethodGet, "http://localhost", strings.NewReader("")), run, &v1alpha1.Repository{}, &github.Provider{}, testNamespace.GetName(), nil) jwtToken, err := ip.GenerateJWT(ctx) assert.NilError(t, err) @@ -412,7 +457,7 @@ func TestGetAndUpdateInstallationID_Fallbacks(t *testing.T) { gprovider.SetGithubClient(fakeghclient) t.Setenv("PAC_GIT_PROVIDER_TOKEN_APIURL", serverURL) - ip = NewInstallation(httptest.NewRequest(http.MethodGet, "http://localhost", strings.NewReader("")), run, repo, gprovider, testNamespace.GetName()) + ip = NewInstallation(httptest.NewRequest(http.MethodGet, "http://localhost", strings.NewReader("")), run, repo, gprovider, testNamespace.GetName(), nil) _, token, installationID, err := ip.GetAndUpdateInstallationID(ctx) if tt.wantErr {