From 67358afe717ae62bf019a30822f27caedf165f17 Mon Sep 17 00:00:00 2001 From: Rushen Wang <45029442+dovics@users.noreply.github.com> Date: Fri, 18 Oct 2024 20:11:03 +0800 Subject: [PATCH] Fix some incorrect parameter parsing in Gitlab Runner (#6173) Signed-off-by: wangrushen --- CHANGELOG.md | 1 + pkg/scalers/github_runner_scaler.go | 26 ++++++++++++++++++------ pkg/scalers/github_runner_scaler_test.go | 14 ++++++++----- 3 files changed, 30 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bea05c1a683..46df9810605 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,6 +61,7 @@ To learn more about active deprecations, we recommend checking [GitHub Discussio - **CloudEventSource**: Introduce ClusterCloudEventSource ([#3533](https://github.com/kedacore/keda/issues/3533)) - **CloudEventSource**: Provide ClusterCloudEventSource around the management of ScaledJobs resources ([#3523](https://github.com/kedacore/keda/issues/3523)) - **CloudEventSource**: Provide ClusterCloudEventSource around the management of TriggerAuthentication/ClusterTriggerAuthentication resources ([#3524](https://github.com/kedacore/keda/issues/3524)) +- **Github Action**: Fix panic when env for runnerScopeFromEnv or ownerFromEnv is empty ([#6156](https://github.com/kedacore/keda/issues/6156)) #### Experimental diff --git a/pkg/scalers/github_runner_scaler.go b/pkg/scalers/github_runner_scaler.go index d53cd21bd2e..c9c93c501b1 100644 --- a/pkg/scalers/github_runner_scaler.go +++ b/pkg/scalers/github_runner_scaler.go @@ -362,8 +362,12 @@ func getValueFromMetaOrEnv(key string, metadata map[string]string, env map[strin if val, ok := metadata[key]; ok && val != "" { return val, nil } else if val, ok := metadata[key+"FromEnv"]; ok && val != "" { - return env[val], nil + if envVal, ok := env[val]; ok && envVal != "" { + return envVal, nil + } + return "", fmt.Errorf("%s %s env variable value is empty", key, val) } + return "", fmt.Errorf("no %s given", key) } @@ -444,12 +448,14 @@ func setupGitHubApp(config *scalersconfig.ScalerConfig) (*int64, *int64, *string var instID *int64 var appKey *string - if val, err := getInt64ValueFromMetaOrEnv("applicationID", config); err == nil && val != -1 { - appID = &val + appIDVal, appIDErr := getInt64ValueFromMetaOrEnv("applicationID", config) + if appIDErr == nil && appIDVal != -1 { + appID = &appIDVal } - if val, err := getInt64ValueFromMetaOrEnv("installationID", config); err == nil && val != -1 { - instID = &val + instIDVal, instIDErr := getInt64ValueFromMetaOrEnv("installationID", config) + if instIDErr == nil && instIDVal != -1 { + instID = &instIDVal } if val, ok := config.AuthParams["appKey"]; ok && val != "" { @@ -458,7 +464,15 @@ func setupGitHubApp(config *scalersconfig.ScalerConfig) (*int64, *int64, *string if (appID != nil || instID != nil || appKey != nil) && (appID == nil || instID == nil || appKey == nil) { - return nil, nil, nil, fmt.Errorf("applicationID, installationID and applicationKey must be given") + if appIDErr != nil { + return nil, nil, nil, appIDErr + } + + if instIDErr != nil { + return nil, nil, nil, instIDErr + } + + return nil, nil, nil, fmt.Errorf("no applicationKey given") } return appID, instID, appKey, nil diff --git a/pkg/scalers/github_runner_scaler_test.go b/pkg/scalers/github_runner_scaler_test.go index fc1babdddc2..808ac78562c 100644 --- a/pkg/scalers/github_runner_scaler_test.go +++ b/pkg/scalers/github_runner_scaler_test.go @@ -73,9 +73,9 @@ var testGitHubRunnerMetadata = []parseGitHubRunnerMetadataTestData{ // empty token {"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, "error parsing installationID: no installationID 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, "error parsing applicationID: no applicationID given"}, // nothing passed {"empty, no envs", map[string]string{}, false, true, "no runnerScope given"}, // empty githubApiURL @@ -105,11 +105,15 @@ var testGitHubRunnerMetadata = []parseGitHubRunnerMetadataTestData{ // 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, ""}, // 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, "error parsing installationID: no installationID 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, "error parsing applicationID: no applicationID 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, "no applicationKey given"}, + {"missing runnerScope Env", map[string]string{"githubApiURL": "https://api.github.com", "owner": "ownername", "repos": "reponame,otherrepo", "labels": "golang", "targetWorkflowQueueLength": "1", "runnerScopeFromEnv": "EMPTY"}, true, true, "runnerScope EMPTY env variable value is empty"}, + {"missing owner Env", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": ORG, "repos": "reponame,otherrepo", "labels": "golang", "targetWorkflowQueueLength": "1", "ownerFromEnv": "EMPTY"}, true, true, "owner EMPTY env variable value is empty"}, + {"wrong applicationID", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": ORG, "owner": "ownername", "repos": "reponame,otherrepo", "labels": "golang", "targetWorkflowQueueLength": "1", "applicationID": "id", "installationID": "1"}, true, true, "error parsing applicationID: strconv.ParseInt: parsing \"id\": invalid syntax"}, + {"wrong installationID", map[string]string{"githubApiURL": "https://api.github.com", "runnerScope": ORG, "owner": "ownername", "repos": "reponame,otherrepo", "labels": "golang", "targetWorkflowQueueLength": "1", "applicationID": "1", "installationID": "id"}, true, true, "error parsing installationID: strconv.ParseInt: parsing \"id\": invalid syntax"}, } func TestGitHubRunnerParseMetadata(t *testing.T) {