🌱 Retry external network calls when publishing results (#1191)

* 🌱 Retries for signing the results with rekor. Signed-off-by: Spencer Schrock <[email protected]> Co-authored-by: naveensrinivasan <[email protected]> * add retry for webapp post Signed-off-by: Spencer Schrock <[email protected]> * add package comment to silence linter. Signed-off-by: Spencer Schrock <[email protected]> * Switch e2e test to unit test. And test backoff feature. Signed-off-by: Spencer Schrock <[email protected]> * Add test which counts number of retries. Signed-off-by: Spencer Schrock <[email protected]> --------- Signed-off-by: Spencer Schrock <[email protected]> Co-authored-by: naveensrinivasan <[email protected]>
ossf · Jun 22, 2023 · 8808ed2 · 8808ed2
1 parent 0eed6cb
commit 8808ed2
Show file tree

Hide file tree

Showing 2 changed files with 170 additions and 26 deletions.
diff --git a/signing/signing.go b/signing/signing.go
@@ -14,6 +14,7 @@
 //
 // SPDX-License-Identifier: Apache-2.0
 
+// Package signing provides functionality to sign and upload results to the Scorecard API.
 package signing
 
 import (
@@ -23,6 +24,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"log"
 	"net/http"
 	"net/url"
 	"os"
@@ -39,6 +41,13 @@ import (
 var (
 	errorEmptyToken   = errors.New("error token empty")
 	errorInvalidToken = errors.New("invalid token")
+
+	// backoff schedule for interactions with cosign/rekor and our web API.
+	backoffSchedule = []time.Duration{
+		1 * time.Second,
+		3 * time.Second,
+		10 * time.Second,
+	}
 )
 
 // Signing is a signing structure.
@@ -79,10 +88,22 @@ func (s *Signing) SignScorecardResult(scorecardResultsFile string) error {
 		SkipConfirmation: true, // skip cosign's privacy confirmation prompt as we run non-interactively
 	}
 
-	// This command will use the provided OIDCIssuer to authenticate into Fulcio, which will generate the
-	// signing certificate on the scorecard result. This attestation is then uploaded to the Rekor transparency log.
-	// The output bytes (signature) and certificate are discarded since verification can be done with just the payload.
-	if _, err := sign.SignBlobCmd(rootOpts, keyOpts, scorecardResultsFile, true, "", "", true); err != nil {
+	var err error
+	for _, backoff := range backoffSchedule {
+		// This command will use the provided OIDCIssuer to authenticate into Fulcio, which will generate the
+		// signing certificate on the scorecard result. This attestation is then uploaded to the Rekor transparency log.
+		// The output bytes (signature) and certificate are discarded since verification can be done with just the payload.
+		_, err = sign.SignBlobCmd(rootOpts, keyOpts, scorecardResultsFile, true, "", "", true)
+		if err == nil {
+			break
+		}
+		log.Printf("error signing scorecard results: %v\n", err)
+		log.Printf("retrying in %v...\n", backoff)
+		time.Sleep(backoff)
+	}
+
+	// retries failed
+	if err != nil {
 		return fmt.Errorf("error signing payload: %w", err)
 	}
 
@@ -133,15 +154,34 @@ func (s *Signing) ProcessSignature(jsonPayload []byte, repoName, repoRef string)
 		return fmt.Errorf("marshalling json results: %w", err)
 	}
 
-	// Call scorecard-webapp-api to process and upload signature.
-	// Setup HTTP request and context.
 	apiURL := os.Getenv(options.EnvInputInternalPublishBaseURL)
 	rawURL := fmt.Sprintf("%s/projects/github.com/%s", apiURL, repoName)
-	parsedURL, err := url.Parse(rawURL)
+	postURL, err := url.Parse(rawURL)
 	if err != nil {
 		return fmt.Errorf("parsing Scorecard API endpoint: %w", err)
 	}
-	req, err := http.NewRequest("POST", parsedURL.String(), bytes.NewBuffer(payloadBytes))
+
+	for _, backoff := range backoffSchedule {
+		// Call scorecard-webapp-api to process and upload signature.
+		err = postResults(postURL, payloadBytes)
+		if err == nil {
+			break
+		}
+		log.Printf("error sending scorecard results to webapp: %v\n", err)
+		log.Printf("retrying in %v...\n", backoff)
+		time.Sleep(backoff)
+	}
+
+	// retries failed
+	if err != nil {
+		return fmt.Errorf("error sending scorecard results to webapp: %w", err)
+	}
+
+	return nil
+}
+
+func postResults(endpoint *url.URL, payload []byte) error {
+	req, err := http.NewRequest("POST", endpoint.String(), bytes.NewBuffer(payload))
 	if err != nil {
 		return fmt.Errorf("creating HTTP request: %w", err)
 	}

diff --git a/signing/signing_test.go b/signing/signing_test.go
@@ -17,9 +17,11 @@
 package signing
 
 import (
-	"fmt"
+	"net/http"
+	"net/http/httptest"
 	"os"
 	"testing"
+	"time"
 
 	"github.com/ossf/scorecard-action/options"
 )
@@ -75,26 +77,128 @@ import (
 // 	}
 // }
 
-// Test using scorecard results that have already been signed & uploaded.
-func Test_ProcessSignature(t *testing.T) {
-	t.Parallel()
-
-	jsonPayload, err := os.ReadFile("testdata/results.json")
-	repoName := "ossf-tests/scorecard-action"
-	repoRef := "refs/heads/main"
-	accessToken := os.Getenv("GITHUB_AUTH_TOKEN")
-	os.Setenv(options.EnvInputInternalPublishBaseURL, "https://api.securityscorecards.dev")
+//nolint:paralleltest // we are using t.Setenv
+func TestProcessSignature(t *testing.T) {
+	tests := []struct {
+		name        string
+		payloadPath string
+		status      int
+		wantErr     bool
+	}{
+		{
+			name:        "post succeeded",
+			status:      http.StatusCreated,
+			payloadPath: "testdata/results.json",
+			wantErr:     false,
+		},
+		{
+			name:        "post failed",
+			status:      http.StatusBadRequest,
+			payloadPath: "testdata/results.json",
+			wantErr:     true,
+		},
+	}
+	// use smaller backoffs for the test so they run faster
+	setBackoffs(t, []time.Duration{0, time.Millisecond, 2 * time.Millisecond})
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			jsonPayload, err := os.ReadFile(tt.payloadPath)
+			if err != nil {
+				t.Fatalf("Unexpected error reading testdata: %v", err)
+			}
+			repoName := "ossf-tests/scorecard-action"
+			repoRef := "refs/heads/main"
+			//nolint:gosec // dummy credentials
+			accessToken := "ghs_foo"
+			server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				w.WriteHeader(tt.status)
+			}))
+			t.Setenv(options.EnvInputInternalPublishBaseURL, server.URL)
+			t.Cleanup(server.Close)
 
-	if err != nil {
-		t.Errorf("Error reading testdata:, %v", err)
+			s, err := New(accessToken)
+			if err != nil {
+				t.Fatalf("Unexpected error New: %v", err)
+			}
+			err = s.ProcessSignature(jsonPayload, repoName, repoRef)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("ProcessSignature() error: %v, wantErr: %v", err, tt.wantErr)
+			}
+		})
 	}
+}
 
-	s, err := New(accessToken)
-	if err != nil {
-		panic(fmt.Sprintf("error SigningNew: %v", err))
+//nolint:paralleltest // we are using t.Setenv
+func TestProcessSignature_retries(t *testing.T) {
+	tests := []struct {
+		name          string
+		nFailures     int
+		wantNRequests int
+		wantErr       bool
+	}{
+		{
+			name:          "succeeds immediately",
+			nFailures:     0,
+			wantNRequests: 1,
+			wantErr:       false,
+		},
+		{
+			name:          "one retry",
+			nFailures:     1,
+			wantNRequests: 2,
+			wantErr:       false,
+		},
+		{
+			// limit corresponds to backoffs set in test body
+			name:          "retry limit exceeded",
+			nFailures:     4,
+			wantNRequests: 3,
+			wantErr:       true,
+		},
 	}
-	if err := s.ProcessSignature(jsonPayload, repoName, repoRef); err != nil {
-		t.Errorf("ProcessSignature() error:, %v", err)
-		return
+	// use smaller backoffs for the test so they run faster
+	setBackoffs(t, []time.Duration{0, time.Millisecond, 2 * time.Millisecond})
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var jsonPayload []byte
+			repoName := "ossf-tests/scorecard-action"
+			repoRef := "refs/heads/main"
+			//nolint:gosec // dummy credentials
+			accessToken := "ghs_foo"
+			var nRequests int
+			server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				nRequests++
+				status := http.StatusCreated
+				if tt.nFailures > 0 {
+					status = http.StatusBadRequest
+					tt.nFailures--
+				}
+				w.WriteHeader(status)
+			}))
+			t.Setenv(options.EnvInputInternalPublishBaseURL, server.URL)
+			t.Cleanup(server.Close)
+
+			s, err := New(accessToken)
+			if err != nil {
+				t.Fatalf("Unexpected error New: %v", err)
+			}
+			err = s.ProcessSignature(jsonPayload, repoName, repoRef)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("ProcessSignature() error: %v, wantErr: %v", err, tt.wantErr)
+			}
+			if nRequests != tt.wantNRequests {
+				t.Errorf("ProcessSignature() made %d requests, wanted %d", nRequests, tt.wantNRequests)
+			}
+		})
 	}
 }
+
+// temporarily sets the backoffs for a given test.
+func setBackoffs(t *testing.T, newBackoffs []time.Duration) {
+	t.Helper()
+	old := backoffSchedule
+	backoffSchedule = newBackoffs
+	t.Cleanup(func() {
+		backoffSchedule = old
+	})
+}