diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 2a795c01f..e84896090 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -114,6 +114,7 @@ jobs: # Include them here to avoid exporting the Docker container as an artifact # - name: Login to GCR + if: ${{ github.event_name != 'pull_request' }} uses: docker/login-action@650006c6eb7dba73a995cc03b0b2d7f5ca915bee # v4 with: registry: gcr.io @@ -121,6 +122,7 @@ jobs: password: ${{ secrets.GCR_JSON_KEY }} - name: Push release image to gcr + if: ${{ github.event_name != 'pull_request' }} run: ./godelw docker push --tags=latest,version ci-all: diff --git a/policy/approval/approve_test.go b/policy/approval/approve_test.go index 2c9270212..8e1c03dce 100644 --- a/policy/approval/approve_test.go +++ b/policy/approval/approve_test.go @@ -31,10 +31,6 @@ import ( "github.com/stretchr/testify/require" ) -func ptr[T any](v T) *T { - return &v -} - var defaultOptions = Options{ Methods: DefaultMethods(), } diff --git a/server/handler/eval_context_test.go b/server/handler/eval_context_test.go index 162d1c759..037e74a25 100644 --- a/server/handler/eval_context_test.go +++ b/server/handler/eval_context_test.go @@ -42,7 +42,7 @@ func TestParseConfigPostsStatusForSeenPolicy(t *testing.T) { assert.Nil(t, evaluator) require.NotNil(t, ec.Status) assert.Equal(t, "error", ec.Status.GetState()) - assert.Equal(t, "policy-bot: main", ec.Status.GetContext()) + assert.Equal(t, "policy-bot", ec.Status.GetContext()) assert.Equal(t, "Error loading policy from testorg/testrepo@main", ec.Status.GetDescription()) } diff --git a/server/handler/fetcher.go b/server/handler/fetcher.go index c00aab654..83d4d57c9 100644 --- a/server/handler/fetcher.go +++ b/server/handler/fetcher.go @@ -109,7 +109,7 @@ func isServerError(err error) bool { var ghErr *github.ErrorResponse if errors.As(err, &ghErr) { switch ghErr.Response.StatusCode { - case http.StatusInternalServerError, http.StatusServiceUnavailable, http.StatusGatewayTimeout: + case http.StatusInternalServerError, http.StatusBadGateway, http.StatusServiceUnavailable, http.StatusGatewayTimeout: return true } } diff --git a/server/handler/fetcher_test.go b/server/handler/fetcher_test.go index 47e548fb3..5d2758075 100644 --- a/server/handler/fetcher_test.go +++ b/server/handler/fetcher_test.go @@ -17,6 +17,7 @@ package handler import ( "context" "errors" + "net/http" "testing" "github.com/google/go-github/v85/github" @@ -112,3 +113,55 @@ func TestConfigFetcherScopesSeenPolicyByBranch(t *testing.T) { require.Error(t, fc.LoadError) assert.False(t, fc.SeenPolicy) } + +func githubStatusError(code int) error { + return &github.ErrorResponse{ + Response: &http.Response{StatusCode: code}, + } +} + +func TestIsServerError(t *testing.T) { + for _, test := range []struct { + code int + retryable bool + }{ + {http.StatusInternalServerError, true}, + {http.StatusBadGateway, true}, + {http.StatusServiceUnavailable, true}, + {http.StatusGatewayTimeout, true}, + {http.StatusForbidden, false}, + {http.StatusNotFound, false}, + {http.StatusUnprocessableEntity, false}, + } { + assert.Equal(t, test.retryable, isServerError(githubStatusError(test.code)), "status %d", test.code) + } + + assert.False(t, isServerError(errors.New("request failed"))) +} + +func TestConfigFetcherRetriesBadGateway(t *testing.T) { + calls := 0 + + fetcher := ConfigFetcher{ + Loader: mockConfigLoader{ + loadConfig: func(ctx context.Context, client *github.Client, owner, repo, ref string) (appconfig.Config, error) { + calls++ + if calls == 1 { + return appconfig.Config{}, githubStatusError(http.StatusBadGateway) + } + return appconfig.Config{ + Content: []byte("policy:\n approval:\n - rule\n"), + Source: "testorg/testrepo@main", + Path: ".policy.yml", + }, nil + }, + }, + SeenPolicyCache: NewSeenPolicyCache(), + } + + fc := fetcher.ConfigForRepositoryBranch(context.Background(), nil, "testorg", "testrepo", "main") + require.NoError(t, fc.LoadError) + require.NoError(t, fc.ParseError) + assert.Equal(t, 2, calls) + assert.True(t, fc.SeenPolicy) +}