Repository: openshift-pipelines/pipelines-as-code
Pull request #300 (2021-10-13 18:14:41)

Fix private repo with Github Application
While reenabling the E2E tests on private repo, i figured that Github Apps was
broken and wasn't generating the private repo properly.

We have a hack in there for runing the e2e/unittests which probably need to be
figured later but that should not bring any defects.



Patch:
diff --git a/pkg/cmd/pipelineascode/pipelineascode.go b/pkg/cmd/pipelineascode/pipelineascode.go
index 1c0aa6a5..b795a9cc 100644
--- a/pkg/cmd/pipelineascode/pipelineascode.go
+++ b/pkg/cmd/pipelineascode/pipelineascode.go
@@ -96,6 +96,7 @@ func runWrap(ctx context.Context, cs *params.Run, vcx webvcs.Interface, kinterac
 	// If we already have the Token (ie: github apps) set as soon as possible the client,
 	// There is more things supported when we already have a github apps and some that are not
 	// (ie: /ok-to-test or /rerequest)
+	// TODO: probably not needed since we generate our token and not getting them beforehand
 	if cs.Info.Pac.VCSToken != "" {
 		err := vcx.SetClient(ctx, cs.Info.Pac)
 		if err != nil {
diff --git a/pkg/pipelineascode/pipelineascode.go b/pkg/pipelineascode/pipelineascode.go
index c129d524..50eb14f7 100644
--- a/pkg/pipelineascode/pipelineascode.go
+++ b/pkg/pipelineascode/pipelineascode.go
@@ -68,12 +68,13 @@ func Run(ctx context.Context, cs *params.Run, vcsintf webvcs.Interface, k8int ku
 		if err != nil {
 			return err
 		}
-		// We already SetClient before ParseWebhook in case if we already set
-		// the token (ie: github apps) and not coming from the repository
-		err = vcsintf.SetClient(ctx, cs.Info.Pac)
-		if err != nil {
-			return err
-		}
+	}
+
+	// Set the client, we should error out if there is a problem with
+	// token or secret or we won't be able to do much.
+	err = vcsintf.SetClient(ctx, cs.Info.Pac)
+	if err != nil {
+		return err
 	}
 
 	// Get the SHA commit info, we want to get the URL and commit title
diff --git a/pkg/webvcs/github/events.go b/pkg/webvcs/github/events.go
index eeaa641c..5e3f97e2 100644
--- a/pkg/webvcs/github/events.go
+++ b/pkg/webvcs/github/events.go
@@ -37,7 +37,7 @@ func (v *VCS) payloadFix(payload string) []byte {
 	return []byte(replacer.Replace(payload))
 }
 
-func (v *VCS) getAppToken() error {
+func (v *VCS) getAppToken(ctx context.Context, info *info.PacOpts) error {
 	installationIDEnv := os.Getenv("PAC_INSTALLATION_ID")
 	workspacePath := os.Getenv("PAC_WORKSPACE_SECRET")
 
@@ -79,13 +79,26 @@ func (v *VCS) getAppToken() error {
 		if !strings.HasPrefix(gheURL, "https://") {
 			gheURL = "https://" + gheURL
 		}
-
 		v.Client, _ = github.NewEnterpriseClient(gheURL, "", &http.Client{Transport: itr})
 		itr.BaseURL = strings.TrimSuffix(v.Client.BaseURL.String(), "/")
 	} else {
 		v.Client = github.NewClient(&http.Client{Transport: itr})
 	}
 
+	// This is a hack when we have auth and api dissascoiated
+	reqTokenURL := os.Getenv("PAC_WEBVCS_TOKEN_APIURL")
+	if reqTokenURL != "" {
+		itr.BaseURL = reqTokenURL
+	}
+
+	// Get a token ASAP because we need it for setting private repos
+	token, err := itr.Token(ctx)
+	if err != nil {
+		return err
+	}
+	v.Token = github.String(token)
+	info.VCSToken = token
+
 	return err
 }
 
@@ -95,7 +108,7 @@ func (v *VCS) ParsePayload(ctx context.Context, run *params.Run, payload string)
 	var processedevent *info.Event
 
 	// get the app token if it exist first
-	if err := v.getAppToken(); err != nil {
+	if err := v.getAppToken(ctx, run.Info.Pac); err != nil {
 		return nil, err
 	}
 
diff --git a/pkg/webvcs/github/github.go b/pkg/webvcs/github/github.go
index 303ed2d9..a5a57ca8 100644
--- a/pkg/webvcs/github/github.go
+++ b/pkg/webvcs/github/github.go
@@ -31,7 +31,6 @@ func (v *VCS) SetClient(ctx context.Context, info *info.PacOpts) error {
 		&oauth2.Token{AccessToken: info.VCSToken},
 	)
 	tc := oauth2.NewClient(ctx, ts)
-
 	var client *github.Client
 	apiURL := info.VCSAPIURL
 	if apiURL != "" {
@@ -52,7 +51,6 @@ func (v *VCS) SetClient(ctx context.Context, info *info.PacOpts) error {
 		v.Client = client
 	}
 	v.APIURL = &apiURL
-	v.Token = &info.VCSToken
 
 	return nil
 }
@@ -79,6 +77,11 @@ func (v *VCS) GetTektonDir(ctx context.Context, runevent *info.Event, path strin
 // GetCommitInfo get info (url and title) on a commit in runevent, this needs to
 // be run after parsewebhook while we already matched a token.
 func (v *VCS) GetCommitInfo(ctx context.Context, runevent *info.Event) error {
+	if v.Client == nil {
+		return fmt.Errorf("no github client has been initiliazed, " +
+			"exiting... (hint: did you forget setting a secret on your repo?)")
+	}
+
 	commit, _, err := v.Client.Git.GetCommit(ctx, runevent.Owner, runevent.Repository, runevent.SHA)
 	if err != nil {
 		return err


Test patch:
diff --git a/pkg/test/github/github.go b/pkg/test/github/github.go
index ab844aa1..8ab32b9b 100644
--- a/pkg/test/github/github.go
+++ b/pkg/test/github/github.go
@@ -13,7 +13,7 @@ import (
 const (
 	// baseURLPath is a non-empty Client.BaseURL path to use during tests,
 	// to ensure relative URLs are used for all endpoints. See issue #752.
-	githubBaseURLPath = "/api-v3"
+	githubBaseURLPath = "/api/v3"
 )
 
 // SetupGH Setup a GitHUB httptest connexion, from go-github test-suit
diff --git a/pkg/webvcs/github/events_test.go b/pkg/webvcs/github/events_test.go
index 56b97f03..3ee87c8a 100644
--- a/pkg/webvcs/github/events_test.go
+++ b/pkg/webvcs/github/events_test.go
@@ -308,6 +308,7 @@ func TestParsePayLoad(t *testing.T) {
 }
 
 func TestAppTokenGeneration(t *testing.T) {
+	fakeGithubAuthURL := "https://fake.gitub.auth/api/v3/"
 	tests := []struct {
 		name          string
 		wantErr       bool
@@ -396,7 +397,7 @@ func TestAppTokenGeneration(t *testing.T) {
 				"github-application-id": "2222",
 				"github-private-key":    fakePrivateKey,
 			},
-			resultBaseURL: "https://foo.bar.com/api/v3/",
+			resultBaseURL: fakeGithubAuthURL,
 		},
 		{
 			name: "Good/full ghe url",
@@ -409,19 +410,7 @@ func TestAppTokenGeneration(t *testing.T) {
 				"github-application-id": "2222",
 				"github-private-key":    fakePrivateKey,
 			},
-			resultBaseURL: "https://alpha.beta.com/api/v3/",
-		},
-		{
-			name: "Good/default github url",
-			envs: map[string]string{
-				"PAC_INSTALLATION_ID":  "11111",
-				"PAC_WORKSPACE_SECRET": "here",
-			},
-			wsSecretFiles: map[string]string{
-				"github-application-id": "2222",
-				"github-private-key":    fakePrivateKey,
-			},
-			resultBaseURL: "https://api.github.com/",
+			resultBaseURL: fakeGithubAuthURL,
 		},
 	}
 	for _, tt := range tests {
@@ -431,6 +420,15 @@ func TestAppTokenGeneration(t *testing.T) {
 				defer d.Remove()
 				tt.envs["PAC_WORKSPACE_SECRET"] = d.Path()
 			}
+
+			_, mux, url, teardown := ghtesthelper.SetupGH()
+			defer teardown()
+			mux.HandleFunc(fmt.Sprintf("/app/installations/%s/access_tokens", tt.envs["PAC_INSTALLATION_ID"]), func(w http.ResponseWriter, r *http.Request) {
+				_, _ = fmt.Fprint(w, `{"commit": {"message": "HELLO"}}`)
+			})
+			tt.envs["PAC_WEBVCS_APIURL"] = fakeGithubAuthURL
+			tt.envs["PAC_WEBVCS_TOKEN_APIURL"] = url + "/api/v3"
+
 			envRemove := env.PatchAll(t, tt.envs)
 			defer envRemove()
 
@@ -448,6 +446,7 @@ func TestAppTokenGeneration(t *testing.T) {
 				},
 				Info: info.Info{
 					Event: event,
+					Pac:   &info.PacOpts{},
 				},
 			}
 
diff --git a/pkg/webvcs/github/github_test.go b/pkg/webvcs/github/github_test.go
index a114c6a2..e289c65e 100644
--- a/pkg/webvcs/github/github_test.go
+++ b/pkg/webvcs/github/github_test.go
@@ -399,10 +399,11 @@ func TestGetStringPullRequestComment(t *testing.T) {
 
 func TestVCS_GetCommitInfo(t *testing.T) {
 	tests := []struct {
-		name             string
-		event            *info.Event
-		wantErr          bool
-		shaurl, shatitle string
+		name              string
+		event             *info.Event
+		noclient          bool
+		apiReply, wantErr string
+		shaurl, shatitle  string
 	}{
 		{
 			name: "good",
@@ -421,7 +422,13 @@ func TestVCS_GetCommitInfo(t *testing.T) {
 				Repository: "repository",
 				SHA:        "shacommitinfo",
 			},
-			wantErr: true,
+			apiReply: "hello moto",
+		},
+		{
+			name:     "noclient",
+			event:    &info.Event{},
+			wantErr:  "no github client has been initiliazed",
+			noclient: true,
 		},
 	}
 	for _, tt := range tests {
@@ -430,19 +437,22 @@ func TestVCS_GetCommitInfo(t *testing.T) {
 			defer teardown()
 			mux.HandleFunc(fmt.Sprintf("/repos/%s/%s/git/commits/%s",
 				tt.event.Owner, tt.event.Repository, tt.event.SHA), func(rw http.ResponseWriter, r *http.Request) {
-				if tt.wantErr {
-					fmt.Fprint(rw, "XXXXX")
-				} else {
-					fmt.Fprintf(rw, `{"html_url": "%s", "message": "%s"}`, tt.shaurl, tt.shatitle)
+				if tt.apiReply != "" {
+					fmt.Fprintf(rw, tt.apiReply)
+					return
 				}
+				fmt.Fprintf(rw, `{"html_url": "%s", "message": "%s"}`, tt.shaurl, tt.shatitle)
 			})
 			ctx, _ := rtesting.SetupFakeContext(t)
-			gvcs := &VCS{
-				Client: fakeclient,
+			vcs := &VCS{Client: fakeclient}
+			if tt.noclient {
+				vcs = &VCS{}
 			}
-
-			if err := gvcs.GetCommitInfo(ctx, tt.event); (err != nil) != tt.wantErr {
-				t.Errorf("GetCommitInfo() error = %v, wantErr %v", err, tt.wantErr)
+			gvcs := vcs
+			err := gvcs.GetCommitInfo(ctx, tt.event)
+			if tt.wantErr != "" {
+				assert.ErrorContains(t, err, tt.wantErr)
+				return
 			}
 			assert.Equal(t, tt.shatitle, tt.event.SHATitle)
 			assert.Equal(t, tt.shaurl, tt.event.SHAURL)
diff --git a/test/github_pullrequest_privaterepository_test.go b/test/github_pullrequest_privaterepository_test.go
index 956af87e..0dd3fe87 100644
--- a/test/github_pullrequest_privaterepository_test.go
+++ b/test/github_pullrequest_privaterepository_test.go
@@ -15,7 +15,6 @@ import (
 )
 
 func TestGithubPullRequestPrivateRepository(t *testing.T) {
-	t.Skip("PSI E2E Infra is borked atm")
 	for _, onWebhook := range []bool{false, true} {
 		targetNS := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-ns")
 		ctx := context.Background()