diff --git a/CHANGELOG.md b/CHANGELOG.md index c948a92b907..50f80909cd1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/pkg/scalers/github_runner_scaler.go b/pkg/scalers/github_runner_scaler.go index 31ec898d5e4..943790b14bb 100644 --- a/pkg/scalers/github_runner_scaler.go +++ b/pkg/scalers/github_runner_scaler.go @@ -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 } @@ -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") @@ -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() @@ -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 { @@ -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 } @@ -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 } @@ -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 diff --git a/pkg/scalers/github_runner_scaler_test.go b/pkg/scalers/github_runner_scaler_test.go index d99c79c7172..d09b1f79ed5 100644 --- a/pkg/scalers/github_runner_scaler_test.go +++ b/pkg/scalers/github_runner_scaler_test.go @@ -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", @@ -49,11 +49,11 @@ 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 @@ -61,23 +61,23 @@ var testGitHubRunnerMetadata = []parseGitHubRunnerMetadataTestData{ // 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 @@ -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) { @@ -133,7 +133,7 @@ func getGitHubTestMetaData(url string) *githubRunnerMetadata { meta := githubRunnerMetadata{ githubAPIURL: url, - runnerScope: "repo", + runnerScope: REPO, owner: "testOwner", personalAccessToken: &testpat, targetWorkflowQueueLength: 1, @@ -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) } })) } @@ -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()) @@ -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()) @@ -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()) @@ -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())