Skip to content

Commit

Permalink
Hotfix repo bug (kedacore#4792)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Eldarrin
Eldarrin committed Jul 13, 2023
1 parent 1753230 commit a1ae203
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 40 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ To learn more about active deprecations, we recommend checking [GitHub Discussio
- **General**: Metrics server exposes Prometheus metrics ([#4776](https://github.com/kedacore/keda/issues/4776))
- **AWS Pod Identity Authentication**: Use `default` service account if the workload doesn't set it ([#4767](https://github.com/kedacore/keda/issues/4767))
- **GitHub Runner Scaler**: Fix rate checking on GHEC when HTTP 200 ([#4786](https://github.com/kedacore/keda/issues/4786))
- **GitHub Runner Scaler**: Fix explicit repo check 404 to skip not crash ([#4790](https://github.com/kedacore/keda/issues/4790))
- **Pulsar Scaler**: Fix `msgBacklogThreshold` field being named wrongly as `msgBacklog` ([#4681](https://github.com/kedacore/keda/issues/4681))

### Deprecations
Expand Down
28 changes: 16 additions & 12 deletions pkg/scalers/github_runner_scaler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,7 @@ func (s *githubRunnerScaler) getRepositories(ctx context.Context) ([]string, err
default:
return nil, fmt.Errorf("runnerScope %s not supported", s.metadata.runnerScope)
}
body, err := getGithubRequest(ctx, url, s.metadata, s.httpClient)
body, _, err := getGithubRequest(ctx, url, s.metadata, s.httpClient)
if err != nil {
return nil, err
}
Expand All @@ -498,10 +498,10 @@ func (s *githubRunnerScaler) getRepositories(ctx context.Context) ([]string, err
return repoList, nil
}

func getGithubRequest(ctx context.Context, url string, metadata *githubRunnerMetadata, httpClient *http.Client) ([]byte, error) {
func getGithubRequest(ctx context.Context, url string, metadata *githubRunnerMetadata, httpClient *http.Client) ([]byte, int, error) {
req, err := http.NewRequestWithContext(ctx, "GET", url, nil)
if err != nil {
return []byte{}, err
return []byte{}, -1, err
}

req.Header.Set("Accept", "application/vnd.github.v3+json")
Expand All @@ -513,12 +513,12 @@ func getGithubRequest(ctx context.Context, url string, metadata *githubRunnerMet

r, err := httpClient.Do(req)
if err != nil {
return []byte{}, err
return []byte{}, -1, err
}

b, err := io.ReadAll(r.Body)
if err != nil {
return []byte{}, err
return []byte{}, -1, err
}
_ = r.Body.Close()

Expand All @@ -528,14 +528,14 @@ func getGithubRequest(ctx context.Context, url string, metadata *githubRunnerMet

if githubAPIRemaining == 0 {
resetTime, _ := strconv.ParseInt(r.Header.Get("X-RateLimit-Reset"), 10, 64)
return []byte{}, fmt.Errorf("GitHub API rate limit exceeded, resets at %s", time.Unix(resetTime, 0))
return []byte{}, r.StatusCode, fmt.Errorf("GitHub API rate limit exceeded, resets at %s", time.Unix(resetTime, 0))
}
}

return []byte{}, fmt.Errorf("the GitHub REST API returned error. url: %s status: %d response: %s", url, r.StatusCode, string(b))
return []byte{}, r.StatusCode, fmt.Errorf("the GitHub REST API returned error. url: %s status: %d response: %s", url, r.StatusCode, string(b))
}

return b, nil
return b, r.StatusCode, nil
}

func stripDeadRuns(allWfrs []WorkflowRuns) []WorkflowRun {
Expand All @@ -553,7 +553,7 @@ func stripDeadRuns(allWfrs []WorkflowRuns) []WorkflowRun {
// getWorkflowRunJobs returns a list of jobs for a given workflow run
func (s *githubRunnerScaler) getWorkflowRunJobs(ctx context.Context, workflowRunID int64, repoName string) ([]Job, error) {
url := fmt.Sprintf("%s/repos/%s/%s/actions/runs/%d/jobs", s.metadata.githubAPIURL, s.metadata.owner, repoName, workflowRunID)
body, err := getGithubRequest(ctx, url, s.metadata, s.httpClient)
body, _, err := getGithubRequest(ctx, url, s.metadata, s.httpClient)
if err != nil {
return nil, err
}
Expand All @@ -570,8 +570,10 @@ func (s *githubRunnerScaler) getWorkflowRunJobs(ctx context.Context, workflowRun
// getWorkflowRuns returns a list of workflow runs for a given repository
func (s *githubRunnerScaler) getWorkflowRuns(ctx context.Context, repoName string) (*WorkflowRuns, error) {
url := fmt.Sprintf("%s/repos/%s/%s/actions/runs", s.metadata.githubAPIURL, s.metadata.owner, repoName)
body, err := getGithubRequest(ctx, url, s.metadata, s.httpClient)
if err != nil {
body, statusCode, err := getGithubRequest(ctx, url, s.metadata, s.httpClient)
if err != nil && statusCode == 404 {
return nil, nil
} else if err != nil {
return nil, err
}

Expand Down Expand Up @@ -620,7 +622,9 @@ func (s *githubRunnerScaler) GetWorkflowQueueLength(ctx context.Context) (int64,
if err != nil {
return -1, err
}
allWfrs = append(allWfrs, *wfrs)
if wfrs != nil {
allWfrs = append(allWfrs, *wfrs)
}
}

var queueCount int64
Expand Down
89 changes: 61 additions & 28 deletions pkg/scalers/github_runner_scaler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ type parseGitHubRunnerMetadataTestData struct {
var testGitHubRunnerResolvedEnv = map[string]string{
"GITHUB_API_URL": "https://api.github.com",
"ACCESS_TOKEN": "sample",
"RUNNER_SCOPE": "org",
"RUNNER_SCOPE": ORG,
"ORG_NAME": "ownername",
"OWNER": "ownername",
"LABELS": "foo,bar",
Expand All @@ -49,35 +49,35 @@ var testGitHubRunnerMetadata = []parseGitHubRunnerMetadataTestData{
// nothing passed
{"empty", map[string]string{}, true, true, "no runnerScope given"},
// properly formed
{"properly formed", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": "org", "owner": "ownername", "repos": "reponame,otherrepo", "labels": "golang", "targetWorkflowQueueLength": "1"}, true, false, ""},
{"properly formed", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": ORG, "owner": "ownername", "repos": "reponame,otherrepo", "labels": "golang", "targetWorkflowQueueLength": "1"}, true, false, ""},
// properly formed with no labels and no repos
{"properly formed, no labels or repos", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": "repo", "owner": "ownername", "targetWorkflowQueueLength": "1"}, true, false, ""},
{"properly formed, no labels or repos", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": REPO, "owner": "ownername", "targetWorkflowQueueLength": "1"}, true, false, ""},
// string for int64
{"string for int64-1", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": "repo", "owner": "ownername", "targetWorkflowQueueLength": "a"}, true, false, ""},
{"string for int64-1", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": REPO, "owner": "ownername", "targetWorkflowQueueLength": "a"}, true, false, ""},
// formed from env
{"formed from env", map[string]string{"githubApiURLFromEnv": "GITHUB_API_URL", "runnerScopeFromEnv": "RUNNER_SCOPE", "ownerFromEnv": "OWNER", "reposFromEnv": "REPOS", "targetWorkflowQueueLength": "1"}, true, false, ""},
// missing runnerScope
{"missing runnerScope", map[string]string{"githubApiURL": "https://api.github.com", "owner": "ownername", "repos": "reponame,otherrepo", "labels": "golang", "targetWorkflowQueueLength": "1"}, true, true, "no runnerScope given"},
// empty runnerScope
{"empty runnerScope", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": "", "owner": "ownername", "repos": "reponame,otherrepo", "labels": "golang", "targetWorkflowQueueLength": "1"}, true, true, "no runnerScope given"},
// missing owner
{"missing owner", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": "repo", "repos": "reponame", "targetWorkflowQueueLength": "1"}, true, true, "no owner given"},
{"missing owner", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": REPO, "repos": "reponame", "targetWorkflowQueueLength": "1"}, true, true, "no owner given"},
// empty owner
{"empty owner", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": "repo", "owner": "", "repos": "reponame", "targetWorkflowQueueLength": "1"}, true, true, "no owner given"},
{"empty owner", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": REPO, "owner": "", "repos": "reponame", "targetWorkflowQueueLength": "1"}, true, true, "no owner given"},
// empty token
{"empty targetWorkflowQueueLength", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": "repo", "owner": "ownername", "repos": "reponame"}, true, false, ""},
{"empty targetWorkflowQueueLength", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": REPO, "owner": "ownername", "repos": "reponame"}, true, false, ""},
// missing installationID From Env
{"missing installationID Env", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": "org", "owner": "ownername", "repos": "reponame,otherrepo", "labels": "golang", "targetWorkflowQueueLength": "1", "applicationIDFromEnv": "APP_ID"}, true, true, "applicationID, installationID and applicationKey must be given"},
{"missing installationID Env", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": ORG, "owner": "ownername", "repos": "reponame,otherrepo", "labels": "golang", "targetWorkflowQueueLength": "1", "applicationIDFromEnv": "APP_ID"}, true, true, "applicationID, installationID and applicationKey must be given"},
// missing applicationID From Env
{"missing applicationId Env", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": "org", "owner": "ownername", "repos": "reponame,otherrepo", "labels": "golang", "targetWorkflowQueueLength": "1", "installationIDFromEnv": "INST_ID"}, true, true, "applicationID, installationID and applicationKey must be given"},
{"missing applicationId Env", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": ORG, "owner": "ownername", "repos": "reponame,otherrepo", "labels": "golang", "targetWorkflowQueueLength": "1", "installationIDFromEnv": "INST_ID"}, true, true, "applicationID, installationID and applicationKey must be given"},
// nothing passed
{"empty, no envs", map[string]string{}, false, true, "no runnerScope given"},
// empty githubApiURL
{"empty githubApiURL, no envs", map[string]string{"githubApiURL": "", "runnerScope": "org", "owner": "ownername", "repos": "reponame,otherrepo", "labels": "golang", "targetWorkflowQueueLength": "1"}, false, false, ""},
{"empty githubApiURL, no envs", map[string]string{"githubApiURL": "", "runnerScope": ORG, "owner": "ownername", "repos": "reponame,otherrepo", "labels": "golang", "targetWorkflowQueueLength": "1"}, false, false, ""},
// properly formed
{"properly formed, no envs", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": "org", "owner": "ownername", "repos": "reponame,otherrepo", "labels": "golang", "targetWorkflowQueueLength": "1"}, false, false, ""},
{"properly formed, no envs", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": ORG, "owner": "ownername", "repos": "reponame,otherrepo", "labels": "golang", "targetWorkflowQueueLength": "1"}, false, false, ""},
// properly formed with no labels and no repos
{"properly formed, no envs, labels or repos", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": "ent", "owner": "ownername", "targetWorkflowQueueLength": "1"}, false, false, ""},
{"properly formed, no envs, labels or repos", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": ENT, "owner": "ownername", "targetWorkflowQueueLength": "1"}, false, false, ""},
// formed from env
{"formed from env, no envs", map[string]string{"githubApiURLFromEnv": "GITHUB_API_URL", "ownerFromEnv": "OWNER", "repos": "reponame", "targetWorkflowQueueLength": "1"}, false, true, "no runnerScope given"},
// formed from default env
Expand All @@ -87,23 +87,23 @@ var testGitHubRunnerMetadata = []parseGitHubRunnerMetadataTestData{
// empty runnerScope
{"empty runnerScope, no envs", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": "", "owner": "ownername", "repos": "reponame,otherrepo", "labels": "golang", "targetWorkflowQueueLength": "1"}, false, true, "no runnerScope given"},
// empty owner
{"empty owner, no envs", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": "repo", "owner": "", "repos": "reponame", "targetWorkflowQueueLength": "1"}, false, true, "no owner given"},
{"empty owner, no envs", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": REPO, "owner": "", "repos": "reponame", "targetWorkflowQueueLength": "1"}, false, true, "no owner given"},
// missing owner
{"missing owner, no envs", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": "repo", "repos": "reponame", "targetWorkflowQueueLength": "1"}, false, true, "no owner given"},
{"missing owner, no envs", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": REPO, "repos": "reponame", "targetWorkflowQueueLength": "1"}, false, true, "no owner given"},
// missing labels, no envs
{"missing labels, no envs", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": "org", "owner": "ownername", "repos": "reponame,otherrepo", "targetWorkflowQueueLength": "1"}, false, false, ""},
{"missing labels, no envs", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": ORG, "owner": "ownername", "repos": "reponame,otherrepo", "targetWorkflowQueueLength": "1"}, false, false, ""},
// empty labels, no envs
{"empty labels, no envs", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": "org", "owner": "ownername", "labels": "", "repos": "reponame,otherrepo", "targetWorkflowQueueLength": "1"}, false, false, ""},
{"empty labels, no envs", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": ORG, "owner": "ownername", "labels": "", "repos": "reponame,otherrepo", "targetWorkflowQueueLength": "1"}, false, false, ""},
// missing repos, no envs
{"missing repos, no envs", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": "org", "owner": "ownername", "labels": "golang", "targetWorkflowQueueLength": "1"}, false, false, ""},
{"missing repos, no envs", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": ORG, "owner": "ownername", "labels": "golang", "targetWorkflowQueueLength": "1"}, false, false, ""},
// empty repos, no envs
{"empty repos, no envs", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": "org", "owner": "ownername", "labels": "golang", "repos": "", "targetWorkflowQueueLength": "1"}, false, false, ""},
{"empty repos, no envs", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": ORG, "owner": "ownername", "labels": "golang", "repos": "", "targetWorkflowQueueLength": "1"}, false, false, ""},
// missing installationID
{"missing installationID", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": "org", "owner": "ownername", "repos": "reponame,otherrepo", "labels": "golang", "targetWorkflowQueueLength": "1", "applicationID": "1"}, true, true, "applicationID, installationID and applicationKey must be given"},
{"missing installationID", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": ORG, "owner": "ownername", "repos": "reponame,otherrepo", "labels": "golang", "targetWorkflowQueueLength": "1", "applicationID": "1"}, true, true, "applicationID, installationID and applicationKey must be given"},
// missing applicationID
{"missing applicationID", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": "org", "owner": "ownername", "repos": "reponame,otherrepo", "labels": "golang", "targetWorkflowQueueLength": "1", "installationID": "1"}, true, true, "applicationID, installationID and applicationKey must be given"},
{"missing applicationID", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": ORG, "owner": "ownername", "repos": "reponame,otherrepo", "labels": "golang", "targetWorkflowQueueLength": "1", "installationID": "1"}, true, true, "applicationID, installationID and applicationKey must be given"},
// all good
{"missing applicationKey", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": "org", "owner": "ownername", "repos": "reponame,otherrepo", "labels": "golang", "targetWorkflowQueueLength": "1", "applicationID": "1", "installationID": "1"}, true, true, "applicationID, installationID and applicationKey must be given"},
{"missing applicationKey", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": ORG, "owner": "ownername", "repos": "reponame,otherrepo", "labels": "golang", "targetWorkflowQueueLength": "1", "applicationID": "1", "installationID": "1"}, true, true, "applicationID, installationID and applicationKey must be given"},
}

func TestGitHubRunnerParseMetadata(t *testing.T) {
Expand Down Expand Up @@ -133,7 +133,7 @@ func getGitHubTestMetaData(url string) *githubRunnerMetadata {

meta := githubRunnerMetadata{
githubAPIURL: url,
runnerScope: "repo",
runnerScope: REPO,
owner: "testOwner",
personalAccessToken: &testpat,
targetWorkflowQueueLength: 1,
Expand All @@ -160,19 +160,25 @@ func apiStubHandler(hasRateLeft bool) *httptest.Server {
w.Header().Set("X-RateLimit-Reset", fmt.Sprint(futureReset.Unix()))
if hasRateLeft {
w.Header().Set("X-RateLimit-Remaining", "50")
w.WriteHeader(http.StatusOK)
} else {
w.Header().Set("X-RateLimit-Remaining", "0")
w.WriteHeader(http.StatusForbidden)
}
if strings.HasSuffix(r.URL.String(), "jobs") {
_, _ = w.Write([]byte(testGhWFJobResponse))
w.WriteHeader(http.StatusOK)
}
if strings.HasSuffix(r.URL.String(), "runs") {
_, _ = w.Write(buildQueueJSON())
if strings.Contains(r.URL.String(), "BadRepo") {
w.WriteHeader(http.StatusNotFound)
} else {
_, _ = w.Write(buildQueueJSON())
w.WriteHeader(http.StatusOK)
}
}
if strings.HasSuffix(r.URL.String(), "repos") {
_, _ = w.Write([]byte(testGhUserReposResponse))
w.WriteHeader(http.StatusOK)
}
}))
}
Expand Down Expand Up @@ -290,7 +296,6 @@ func TestNewGitHubRunnerScaler_404(t *testing.T) {
httpClient: http.DefaultClient,
}

mockGitHubRunnerScaler.metadata.repos = []string{"test"}
mockGitHubRunnerScaler.metadata.labels = []string{"foo", "bar"}

_, err := mockGitHubRunnerScaler.GetWorkflowQueueLength(context.TODO())
Expand Down Expand Up @@ -386,7 +391,35 @@ func TestNewGitHubRunnerScaler_QueueLength_MultiRepo_Assigned(t *testing.T) {

tRepo := []string{"test", "test2"}
mockGitHubRunnerScaler.metadata.repos = tRepo
mockGitHubRunnerScaler.metadata.runnerScope = "org"
mockGitHubRunnerScaler.metadata.runnerScope = ORG
mockGitHubRunnerScaler.metadata.labels = []string{"foo", "bar"}

queueLen, err := mockGitHubRunnerScaler.GetWorkflowQueueLength(context.TODO())

if err != nil {
fmt.Println(err)
t.Fail()
}

if queueLen != 2 {
fmt.Println(queueLen)
t.Fail()
}
}

func TestNewGitHubRunnerScaler_QueueLength_MultiRepo_Assigned_OneBad(t *testing.T) {
var apiStub = apiStubHandler(true)

meta := getGitHubTestMetaData(apiStub.URL)

mockGitHubRunnerScaler := githubRunnerScaler{
metadata: meta,
httpClient: http.DefaultClient,
}

tRepo := []string{"test", "test2", "BadRepo"}
mockGitHubRunnerScaler.metadata.repos = tRepo
mockGitHubRunnerScaler.metadata.runnerScope = ORG
mockGitHubRunnerScaler.metadata.labels = []string{"foo", "bar"}

queueLen, err := mockGitHubRunnerScaler.GetWorkflowQueueLength(context.TODO())
Expand Down Expand Up @@ -436,7 +469,7 @@ func TestNewGitHubRunnerScaler_QueueLength_MultiRepo_PulledOrgRepos(t *testing.T
httpClient: http.DefaultClient,
}

mockGitHubRunnerScaler.metadata.runnerScope = "org"
mockGitHubRunnerScaler.metadata.runnerScope = ORG
mockGitHubRunnerScaler.metadata.labels = []string{"foo", "bar"}

queueLen, err := mockGitHubRunnerScaler.GetWorkflowQueueLength(context.TODO())
Expand All @@ -461,7 +494,7 @@ func TestNewGitHubRunnerScaler_QueueLength_MultiRepo_PulledEntRepos(t *testing.T
httpClient: http.DefaultClient,
}

mockGitHubRunnerScaler.metadata.runnerScope = "ent"
mockGitHubRunnerScaler.metadata.runnerScope = ENT
mockGitHubRunnerScaler.metadata.labels = []string{"foo", "bar"}

queueLen, err := mockGitHubRunnerScaler.GetWorkflowQueueLength(context.TODO())
Expand Down

0 comments on commit a1ae203

Please sign in to comment.