Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🌱 Setup cron for running as GitHub App #2721

Merged
merged 11 commits into from
Mar 7, 2023

Conversation

spencerschrock
Copy link
Member

What kind of change does this PR introduce?

cron update

What is the current behavior?

The cron used personal access tokens

What is the new behavior (if this is a feature change)?**

  • Dropped the GITHUB AUTH server to 0 replicas since it doesn't support the GitHub App tokens
  • Added logic for retry-after headers and some metrics when/if we see it.
  • Disabled 4 checks which don't work well at the moment
    • Search based checks hit secondary rate limits frequently with the GitHub App tokens
    • Vulnerabilities due to an inefficiency in osv-scanner that has been fixed upstream but needs a new release cut
  • Runtime errors are ignored in the release worker due to known errors with the Branch-Protection check while using a GitHub App token

[] Tests for the changes have been added (for bug fixes/features)

Which issue(s) this PR fixes

NONE

Special notes for your reviewer

Does this PR introduce a user-facing change?

For user-facing changes, please add a concise, human-readable release note to
the release-note

(In particular, describe what changes users might need to make in their
application as a result of this pull request.)

NONE

@codecov
Copy link

codecov bot commented Mar 6, 2023

Codecov Report

Merging #2721 (e259690) into main (fb12a39) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2721   +/-   ##
=======================================
  Coverage   41.39%   41.39%           
=======================================
  Files         124      124           
  Lines       10164    10164           
=======================================
  Hits         4207     4207           
  Misses       5659     5659           
  Partials      298      298           

Copy link
Contributor

@azeemshaikh38 azeemshaikh38 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with comment/question.

clients/githubrepo/roundtripper/transport.go Outdated Show resolved Hide resolved
@spencerschrock spencerschrock temporarily deployed to integration-test March 7, 2023 00:15 — with GitHub Actions Inactive
@spencerschrock spencerschrock enabled auto-merge (squash) March 7, 2023 19:10
@spencerschrock spencerschrock temporarily deployed to integration-test March 7, 2023 19:10 — with GitHub Actions Inactive
@spencerschrock spencerschrock merged commit 0169c37 into ossf:main Mar 7, 2023
Shofiya2003 pushed a commit to Shofiya2003/scorecard that referenced this pull request Mar 10, 2023
* Update auth server to use GitHub App.

Signed-off-by: Spencer Schrock <[email protected]>

* Update release worker to use GitHub App tokens directly, as a workaround for the auth server not supporting it.

Signed-off-by: Spencer Schrock <[email protected]>

* Add Retry-After logic and stats.

Signed-off-by: Spencer Schrock <[email protected]>

* Change retry-after logic to support any status code. Disable troublesome checks.

Signed-off-by: Spencer Schrock <[email protected]>

* Use GitHub App Token instead of auth server.

Signed-off-by: Spencer Schrock <[email protected]>

* Temporarily disable additional chhecks.

Signed-off-by: Spencer Schrock <[email protected]>

* Disable github auth server as it doesn't work with the GitHub App Tokens.

Signed-off-by: Spencer Schrock <[email protected]>

* Re-enable Fuzzing check in the release test.

Signed-off-by: Spencer Schrock <[email protected]>

* Fix unit test for new check change.

Signed-off-by: Spencer Schrock <[email protected]>

* Move opencensus stat to the ratelimit roundtripped.

Signed-off-by: Spencer Schrock <[email protected]>

---------

Signed-off-by: Spencer Schrock <[email protected]>
Signed-off-by: Shofiya2003 <[email protected]>
Shofiya2003 pushed a commit to Shofiya2003/scorecard that referenced this pull request Mar 10, 2023
* Update auth server to use GitHub App.

Signed-off-by: Spencer Schrock <[email protected]>

* Update release worker to use GitHub App tokens directly, as a workaround for the auth server not supporting it.

Signed-off-by: Spencer Schrock <[email protected]>

* Add Retry-After logic and stats.

Signed-off-by: Spencer Schrock <[email protected]>

* Change retry-after logic to support any status code. Disable troublesome checks.

Signed-off-by: Spencer Schrock <[email protected]>

* Use GitHub App Token instead of auth server.

Signed-off-by: Spencer Schrock <[email protected]>

* Temporarily disable additional chhecks.

Signed-off-by: Spencer Schrock <[email protected]>

* Disable github auth server as it doesn't work with the GitHub App Tokens.

Signed-off-by: Spencer Schrock <[email protected]>

* Re-enable Fuzzing check in the release test.

Signed-off-by: Spencer Schrock <[email protected]>

* Fix unit test for new check change.

Signed-off-by: Spencer Schrock <[email protected]>

* Move opencensus stat to the ratelimit roundtripped.

Signed-off-by: Spencer Schrock <[email protected]>

---------

Signed-off-by: Spencer Schrock <[email protected]>
Signed-off-by: Shofiya2003 <[email protected]>
@spencerschrock spencerschrock deleted the feat/test-github-app-cron branch July 24, 2023 22:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants