Skip to content

Commit

Permalink
Fix some incorrect parameter parsing in Gitlab Runner (#6173)
Browse files Browse the repository at this point in the history
Signed-off-by: wangrushen <wrs369@88.com>
  • Loading branch information
dovics authored Oct 18, 2024
1 parent 00f4e51 commit 67358af
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 11 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
26 changes: 20 additions & 6 deletions pkg/scalers/github_runner_scaler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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 != "" {
Expand All @@ -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
Expand Down
14 changes: 9 additions & 5 deletions pkg/scalers/github_runner_scaler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit 67358af

Please sign in to comment.