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

GitHub Runner Scaler - RateLimit Bug for GitHub Enterprise #4784

Closed
mke-devoteam opened this issue Jul 12, 2023 · 7 comments · Fixed by #4786
Closed

GitHub Runner Scaler - RateLimit Bug for GitHub Enterprise #4784

mke-devoteam opened this issue Jul 12, 2023 · 7 comments · Fixed by #4786
Labels
bug Something isn't working

Comments

@mke-devoteam
Copy link

mke-devoteam commented Jul 12, 2023

Report

When you use the github_runner_scaler for GitHub Enterprise with your own GitHub Appliance, you encounter an issue. The scaler fails and shows a GitHub REST API error, even though the response code of 200 means the request was successful. The problem seems to be on line 525 of github_runner_scaler.go where it tries to fetch the "X-RateLimit-Remaining" response header. However, this header is missing in responses from the GitHub Enterprise Appliance.

Error Message:

{
    "TimeStamp": "2023-07-11 13:07:02 \u002B0000 UTC",
    "Type": "Warning",
    "ContainerAppName": "_masked_",
    "RevisionName": "_masked_",
    "ReplicaName": "",
    "Msg": "the GitHub REST API returned error. url: <githubAPIURL>/repos/<owner>/<repo>/actions/runs status: 200 response: {\u0022total_count\u0022:0,\u0022workflow_runs\u0022:[]}",
    "Reason": "KEDAScalerFailed",
    "EventSource": "KEDA",
    "Count": 2
}

Relevant code (Line 525 -527):

if r.StatusCode != 200 || r.Header.Get("X-RateLimit-Remaining") == "" {
	return []byte{}, fmt.Errorf("the GitHub REST API returned error. url: %s status: %d response: %s", url, r.StatusCode, string(b))
}

Expected Behavior

"When you use the github_runner_scaler with your own githubAPIURL, the code shouldn't try to fetch the "X-RateLimit-Remaining" response header."

Actual Behavior

The code tries to fetch the "X-RateLimit-Remaining" response header and throws an error, since the header is missing in responses from the GitHub Enterprise Appliance. This is a false error.

Steps to Reproduce the Problem

  1. Implement the Github Runner Scaler for an Enterprise account with its own GitHub Appliance
  2. For metadata, set the githubAPIURL to your Enterprise API URL
  3. Use personalAccessToken as Authentication Parameter
  4. Check Logs

Logs from KEDA operator

{
    "TimeStamp": "2023-07-11 13:07:02 \u002B0000 UTC",
    "Type": "Warning",
    "ContainerAppName": "_masked_",
    "RevisionName": "_masked_",
    "ReplicaName": "",
    "Msg": "the GitHub REST API returned error. url: <githubAPIURL>/repos/<owner>/<repo>/actions/runs status: 200 response: {\u0022total_count\u0022:0,\u0022workflow_runs\u0022:[]}",
    "Reason": "KEDAScalerFailed",
    "EventSource": "KEDA",
    "Count": 2
}

KEDA Version

None

Kubernetes Version

None

Platform

Microsoft Azure

Scaler Details

Github Runner Scaler

Anything else?

No response

@mke-devoteam mke-devoteam added the bug Something isn't working label Jul 12, 2023
@JorTurFer
Copy link
Member

JorTurFer commented Jul 12, 2023

Hello,
Do you know which version of GH enterprise are you running? I have checked the docs and it looks that even in enterprise the header should be there (which is weird because I thought that there isn't rate limiting in GH enterprise).
@Eldarrin , could we just check the remaining rate limit only if the status code isn't 200? I mean, if API has returned 200 is because we haven't hit the rate limit yet

@JorTurFer JorTurFer reopened this Jul 12, 2023
@JorTurFer
Copy link
Member

Please ignore this:
image

I clicked the wrong button 🤦

@mke-devoteam
Copy link
Author

Hi Jorge,
we are running GitHub Enterprise Server 3.8.3. I checked the headers I get in the response by making the same call with Postman.
This is the list of headers I get in the response from Postman:
image

Hope this helps.

@Eldarrin
Copy link
Contributor

You're correct, the error should be pinged on 403, not 200s. Want me to patch it?

@JorTurFer
Copy link
Member

If you can it'd be nice, if you can't, I'll do it :)

@JorTurFer
Copy link
Member

This is a nice addition to the hotfix release that we are preparing :)

@Eldarrin Eldarrin mentioned this issue Jul 12, 2023
3 tasks
@Eldarrin
Copy link
Contributor

PR submitted

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants