Skip to content

Commit

Permalink
updated per review
Browse files Browse the repository at this point in the history
Signed-off-by: Ming Meng <ming.meng@collibra.com>
  • Loading branch information
mingmcb committed Jul 6, 2023
1 parent bbd8b4f commit c513f2a
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 12 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ To learn more about active deprecations, we recommend checking [GitHub Discussio
- [v1.0.0](#v100)

## Unreleased
- **Pulsar Scaler**: Improve error messages for unsuccessful connections ([#4563](https://github.com/kedacore/keda/issues/4563))

### New

Expand Down Expand Up @@ -136,7 +137,6 @@ None.
- **NATS JetStream Scaler**: Add support for pulling AccountID from TriggerAuthentication ([#4586](https://github.com/kedacore/keda/issues/4586))
- **PostgreSQL Scaler**: Replace `lib/pq` with `pgx` ([#4704](https://github.com/kedacore/keda/issues/4704))
- **Prometheus Scaler**: Add support for Google Managed Prometheus ([#467](https://github.com/kedacore/keda/issues/4674))
- **Pulsar Scaler**: Improve error messages for unsuccessful connections ([#4563](https://github.com/kedacore/keda/issues/4563))
- **RabbitMQ Scaler**: Add support for `unsafeSsl` in trigger metadata ([#4448](https://github.com/kedacore/keda/issues/4448))
- **RabbitMQ Scaler**: Add support for `workloadIdentityResource` and utilize AzureAD Workload Identity for HTTP authorization ([#4716](https://github.com/kedacore/keda/issues/4716))
- **Solace Scaler**: Add new `messageReceiveRateTarget` metric to Solace Scaler ([#4665](https://github.com/kedacore/keda/issues/4665))
Expand Down
25 changes: 21 additions & 4 deletions pkg/scalers/authentication/authentication_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,7 @@ func GetAuthConfigs(triggerMetadata, authParams map[string]string) (out *AuthMet
}
out.EnableOAuth = true
out.OauthTokenURI = authParams["oauthTokenURI"]
scope := strings.ReplaceAll(authParams["scope"], " ", "")
if scope != "" {
out.Scopes = strings.Split(scope, ",")
}
out.Scopes = ParseScope(authParams["scope"])
out.ClientID = authParams["clientID"]
out.ClientSecret = authParams["clientSecret"]
default:
Expand All @@ -112,6 +109,26 @@ func GetAuthConfigs(triggerMetadata, authParams map[string]string) (out *AuthMet
return out, err
}

func ParseScope(inputStr string) []string {
scope := strings.TrimSpace(inputStr)
if scope != "" {
scopes := make([]string, 0)
list := strings.Split(scope, ",")
for _, sc := range list {
sc := strings.TrimSpace(sc)
if sc != "" {
scopes = append(scopes, sc)
}
}
if len(scopes) == 0 {
return nil
} else {
return scopes
}
}
return nil
}

func GetBearerToken(auth *AuthMeta) string {
return fmt.Sprintf("Bearer %s", auth.BearerToken)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/scalers/authentication/authentication_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const (
BearerAuthType Type = "bearer"
// CustomAuthType is an auth type using a custom header
CustomAuthType Type = "custom"
// OAuthType is a auth type using a oAuth2
// OAuthType is an auth type using a oAuth2
OAuthType Type = "oauth"
)

Expand Down
5 changes: 2 additions & 3 deletions pkg/scalers/pulsar_scaler.go
Original file line number Diff line number Diff line change
Expand Up @@ -217,9 +217,8 @@ func parsePulsarMetadata(config *ScalerConfig, logger logr.Logger) (pulsarMetada
if auth.OauthTokenURI == "" {
auth.OauthTokenURI = config.TriggerMetadata["oauthTokenURI"]
}
scope := strings.ReplaceAll(config.TriggerMetadata["scope"], " ", "")
if auth.Scopes == nil && scope != "" {
auth.Scopes = strings.Split(scope, ",")
if auth.Scopes == nil {
auth.Scopes = authentication.ParseScope(config.TriggerMetadata["scope"])
}
if auth.ClientID == "" {
auth.ClientID = config.TriggerMetadata["clientID"]
Expand Down
8 changes: 5 additions & 3 deletions pkg/scalers/pulsar_scaler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,13 @@ var parsePulsarMetadataTestAuthTLSDataset = []parsePulsarAuthParamsTestData{
// Passes, oauth config data is set from TriggerAuth if both provided
{map[string]string{"adminURL": "http://172.20.0.151:80", "topic": "persistent://public/default/my-topic", "subscription": "sub1", "authModes": "oauth", "oauthTokenURI": "https1", "scope": "scope1", "clientID": "id1"}, map[string]string{"ca": "cadata", "oauthTokenURI": "https3", "scope": "scope3", "clientID": "id3", "clientSecret": "secret123"}, false, false, "", "", "cadata", "", "", "", false, "https3", "scope3", "id3", "secret123"},
// Passes, with multiple scopes from metadata
{map[string]string{"adminURL": "http://172.20.0.151:80", "topic": "persistent://public/default/my-topic", "subscription": "sub1", "authModes": "oauth", "oauthTokenURI": "https4", "scope": " sc:scope2, sc:scope1 ", "clientID": "id4"}, map[string]string{"ca": "cadata", "oauthTokenURI": "", "scope": "", "clientID": "", "clientSecret": ""}, false, false, "", "", "cadata", "", "", "", false, "https4", "sc:scope1 sc:scope2", "id4", ""},
{map[string]string{"adminURL": "http://172.20.0.151:80", "topic": "persistent://public/default/my-topic", "subscription": "sub1", "authModes": "oauth", "oauthTokenURI": "https4", "scope": " sc:scope2, \tsc:scope1 ", "clientID": "id4"}, map[string]string{"ca": "cadata", "oauthTokenURI": "", "scope": "", "clientID": "", "clientSecret": ""}, false, false, "", "", "cadata", "", "", "", false, "https4", "sc:scope1 sc:scope2", "id4", ""},
// Passes, with multiple scopes from TriggerAuth
{map[string]string{"adminURL": "http://172.20.0.151:80", "topic": "persistent://public/default/my-topic", "subscription": "sub1", "authModes": "oauth"}, map[string]string{"ca": "cadata", "oauthTokenURI": "https5", "scope": " sc:scope2, sc:scope1 ", "clientID": "id5", "clientSecret": "secret123"}, false, false, "", "", "cadata", "", "", "", false, "https5", "sc:scope1 sc:scope2", "id5", "secret123"},
{map[string]string{"adminURL": "http://172.20.0.151:80", "topic": "persistent://public/default/my-topic", "subscription": "sub1", "authModes": "oauth"}, map[string]string{"ca": "cadata", "oauthTokenURI": "https5", "scope": " sc:scope2, \tsc:scope1 \n", "clientID": "id5", "clientSecret": "secret123"}, false, false, "", "", "cadata", "", "", "", false, "https5", "sc:scope1 sc:scope2", "id5", "secret123"},
// Passes, no scope provided
{map[string]string{"adminURL": "http://172.20.0.151:80", "topic": "persistent://public/default/my-topic", "subscription": "sub1", "authModes": "oauth"}, map[string]string{"ca": "cadata", "oauthTokenURI": "https5", "clientID": "id5", "clientSecret": "secret123"}, false, false, "", "", "cadata", "", "", "", false, "https5", "", "id5", "secret123"},
// Passes, invalid scopes provided
{map[string]string{"adminURL": "http://172.20.0.151:80", "topic": "persistent://public/default/my-topic", "subscription": "sub1", "authModes": "oauth", "scope": " "}, map[string]string{"ca": "cadata", "oauthTokenURI": "https5", "scope": " ", "clientID": "id5", "clientSecret": "secret123"}, false, false, "", "", "cadata", "", "", "", false, "https5", "", "id5", "secret123"},
{map[string]string{"adminURL": "http://172.20.0.151:80", "topic": "persistent://public/default/my-topic", "subscription": "sub1", "authModes": "oauth", "scope": " "}, map[string]string{"ca": "cadata", "oauthTokenURI": "https5", "scope": " , \n", "clientID": "id5", "clientSecret": "secret123"}, false, false, "", "", "cadata", "", "", "", false, "https5", "", "id5", "secret123"},
}

var pulsarMetricIdentifiers = []pulsarMetricIdentifier{
Expand Down

0 comments on commit c513f2a

Please sign in to comment.