Make sure to check if we can make API calls

We didn't check for the error, which could crash the tekton controller. That call is the first API call we make so is the first one that would crash. Report the run as error if that happen, since it's probably a GitHub API outage (and a lot of other things would be wrong) Signed-off-by: Chmouel Boudjnah <[email protected]>
openshift-pipelines · Jun 4, 2024 · 225cd25 · 225cd25
1 parent 721ea87
commit 225cd25
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 10 deletions.
diff --git a/pkg/provider/github/github.go b/pkg/provider/github/github.go
@@ -241,6 +241,14 @@ func parseTS(headerTS string) (time.Time, error) {
 // the issue was.
 func (v *Provider) checkWebhookSecretValidity(ctx context.Context, cw clockwork.Clock) error {
 	rl, resp, err := v.Client.RateLimit.Get(ctx)
+	if resp.StatusCode == http.StatusNotFound {
+		v.Logger.Info("skipping checking if token has expired, rate_limit api is not enabled on token")
+		return nil
+	}
+
+	if err != nil {
+		return fmt.Errorf("error making request to the GitHub API checking rate limit: %w", err)
+	}
 	if resp.Header.Get("GitHub-Authentication-Token-Expiration") != "" {
 		ts, err := parseTS(resp.Header.Get("GitHub-Authentication-Token-Expiration"))
 		if err != nil {
@@ -253,16 +261,6 @@ func (v *Provider) checkWebhookSecretValidity(ctx context.Context, cw clockwork.
 		}
 	}
 
-	if resp.StatusCode == http.StatusNotFound {
-		v.Logger.Info("skipping checking if token has expired, rate_limit api is not enabled on token")
-		return nil
-	}
-
-	// some other error happened that is not rate limited related
-	if err != nil {
-		return fmt.Errorf("error using token to access API: %w", err)
-	}
-
 	if rl.SCIM.Remaining == 0 {
 		return fmt.Errorf("token is ratelimited, it will be available again at %s", rl.SCIM.Reset.Format(time.RFC1123))
 	}
@@ -281,6 +279,9 @@ func (v *Provider) SetClient(ctx context.Context, run *params.Run, event *info.E
 	if v.Client == nil {
 		v.Client = client
 	}
+	if v.Client == nil {
+		return fmt.Errorf("no github client has been initialized")
+	}
 
 	v.APIURL = apiURL
 

diff --git a/pkg/provider/github/github_test.go b/pkg/provider/github/github_test.go
@@ -836,6 +836,7 @@ func TestProvider_checkWebhookSecretValidity(t *testing.T) {
 		expHeaderSet   bool
 		apiNotEnabled  bool
 		wantLogSnippet string
+		report500      bool
 	}{
 		{
 			name:         "remaining scim calls",
@@ -865,6 +866,11 @@ func TestProvider_checkWebhookSecretValidity(t *testing.T) {
 			remaining:  0,
 			wantSubErr: "token is ratelimited",
 		},
+		{
+			name:       "api error",
+			wantSubErr: "error making request to the GitHub API checking rate limit",
+			report500:  true,
+		},
 		{
 			name:           "not enabled",
 			apiNotEnabled:  true,
@@ -879,6 +885,10 @@ func TestProvider_checkWebhookSecretValidity(t *testing.T) {
 
 			if !tt.apiNotEnabled {
 				mux.HandleFunc("/rate_limit", func(rw http.ResponseWriter, _ *http.Request) {
+					if tt.report500 {
+						rw.WriteHeader(http.StatusInternalServerError)
+						return
+					}
 					s := &github.RateLimits{
 						SCIM: &github.Rate{
 							Remaining: tt.remaining,