kedacore / keda

KEDA is a Kubernetes-based Event Driven Autoscaling component. It provides event driven scale for any container running in Kubernetes
https://keda.sh
Apache License 2.0
8.5k stars 1.08k forks source link

GitHub Runner Scaler - RateLimit Bug for GitHub Enterprise #4784

Closed mke-devoteam closed 1 year ago

mke-devoteam commented 1 year ago

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

JorTurFer commented 1 year ago

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 commented 1 year ago

Please ignore this: image

I clicked the wrong button 🤦

mke-devoteam commented 1 year ago

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 commented 1 year ago

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

JorTurFer commented 1 year ago

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

JorTurFer commented 1 year ago

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

Eldarrin commented 1 year ago

PR submitted