Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix some incorrect parameter parsing in Gitlab Runner #6173

Merged
merged 2 commits into from
Oct 18, 2024
Merged

Fix some incorrect parameter parsing in Gitlab Runner #6173

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f93b1a7
fix: panic when owner or runnerScope env is empty
dovics Sep 18, 2024
805451f
fix: Expose errors in parsing applicationID and installationID
dovics Sep 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,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
View file
Open in desktop
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
View file
Open in desktop
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
Loading