Skip to content

Commit

Permalink
review items
Browse files Browse the repository at this point in the history 
* rename 'parsingOrder' tag to 'order'
* rename 'exclusive' tag to 'exclusiveSet'
* improve parsing 'order' tag behaviour + additional coverage

Signed-off-by: Jan Wozniak <[email protected]>
  • Loading branch information
@wozniakjan
wozniakjan committed May 10, 2024
1 parent 8b9f5cf commit 07676ba
Show file tree
Hide file tree
Showing 4 changed files with 134 additions and 78 deletions.
28 changes: 14 additions & 14 deletions pkg/scalers/authentication/authentication_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,37 +66,37 @@ type AuthMeta struct {

// BasicAuth is a basic authentication type
type BasicAuth struct {
Username string `keda:"name=username, parsingOrder=authParams"`
Password string `keda:"name=password, parsingOrder=authParams"`
Username string `keda:"name=username, order=authParams"`
Password string `keda:"name=password, order=authParams"`
}

// CertAuth is a client certificate authentication type
type CertAuth struct {
Cert string `keda:"name=cert, parsingOrder=authParams"`
Key string `keda:"name=key, parsingOrder=authParams"`
CA string `keda:"name=ca, parsingOrder=authParams"`
Cert string `keda:"name=cert, order=authParams"`
Key string `keda:"name=key, order=authParams"`
CA string `keda:"name=ca, order=authParams"`
}

// OAuth is an oAuth2 authentication type
type OAuth struct {
OauthTokenURI string `keda:"name=oauthTokenURI, parsingOrder=authParams"`
Scopes []string `keda:"name=scopes, parsingOrder=authParams"`
ClientID string `keda:"name=clientID, parsingOrder=authParams"`
ClientSecret string `keda:"name=clientSecret, parsingOrder=authParams"`
EndpointParams url.Values `keda:"name=endpointParams, parsingOrder=authParams"`
OauthTokenURI string `keda:"name=oauthTokenURI, order=authParams"`
Scopes []string `keda:"name=scopes, order=authParams"`
ClientID string `keda:"name=clientID, order=authParams"`
ClientSecret string `keda:"name=clientSecret, order=authParams"`
EndpointParams url.Values `keda:"name=endpointParams, order=authParams"`
}

// CustomAuth is a custom header authentication type
type CustomAuth struct {
CustomAuthHeader string `keda:"name=customAuthHeader, parsingOrder=authParams"`
CustomAuthValue string `keda:"name=customAuthValue, parsingOrder=authParams"`
CustomAuthHeader string `keda:"name=customAuthHeader, order=authParams"`
CustomAuthValue string `keda:"name=customAuthValue, order=authParams"`
}

// Config is the configuration for the authentication types
type Config struct {
Modes []Type `keda:"name=authModes, parsingOrder=triggerMetadata, enum=apiKey;basic;tls;bearer;custom;oauth, exclusive=bearer;basic;oauth, optional"`
Modes []Type `keda:"name=authModes, order=triggerMetadata, enum=apiKey;basic;tls;bearer;custom;oauth, exclusiveSet=bearer;basic;oauth, optional"`

BearerToken string `keda:"name=bearerToken, parsingOrder=authParams, optional"`
BearerToken string `keda:"name=bearerToken, order=authParams, optional"`
BasicAuth `keda:"optional"`
CertAuth `keda:"optional"`
OAuth `keda:"optional"`
Expand Down
22 changes: 11 additions & 11 deletions pkg/scalers/prometheus_scaler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,26 +45,26 @@ type prometheusScaler struct {
logger logr.Logger
}

// sometimes should consider there is an error we can accept
// IgnoreNullValues - sometimes should consider there is an error we can accept
// default value is true/t, to ignore the null value return from prometheus
// change to false/f if can not accept prometheus return null values
// https://github.com/kedacore/keda/issues/3065
type prometheusMetadata struct {
triggerIndex int

PrometheusAuth *authentication.Config `keda:"optional"`
ServerAddress string `keda:"name=serverAddress, parsingOrder=triggerMetadata"`
Query string `keda:"name=query, parsingOrder=triggerMetadata"`
QueryParameters map[string]string `keda:"name=queryParameters, parsingOrder=triggerMetadata, optional"`
Threshold float64 `keda:"name=threshold, parsingOrder=triggerMetadata"`
ActivationThreshold float64 `keda:"name=activationThreshold, parsingOrder=triggerMetadata, optional"`
Namespace string `keda:"name=namespace, parsingOrder=triggerMetadata, optional"`
CustomHeaders map[string]string `keda:"name=customHeaders, parsingOrder=triggerMetadata, optional"`
IgnoreNullValues bool `keda:"name=ignoreNullValues, parsingOrder=triggerMetadata, optional, default=true"`
UnsafeSSL bool `keda:"name=unsafeSsl, parsingOrder=triggerMetadata, optional"`
ServerAddress string `keda:"name=serverAddress, order=triggerMetadata"`
Query string `keda:"name=query, order=triggerMetadata"`
QueryParameters map[string]string `keda:"name=queryParameters, order=triggerMetadata, optional"`
Threshold float64 `keda:"name=threshold, order=triggerMetadata"`
ActivationThreshold float64 `keda:"name=activationThreshold, order=triggerMetadata, optional"`
Namespace string `keda:"name=namespace, order=triggerMetadata, optional"`
CustomHeaders map[string]string `keda:"name=customHeaders, order=triggerMetadata, optional"`
IgnoreNullValues bool `keda:"name=ignoreNullValues, order=triggerMetadata, optional, default=true"`
UnsafeSSL bool `keda:"name=unsafeSsl, order=triggerMetadata, optional"`

// deprecated
CortexOrgID string `keda:"name=cortexOrgID, parsingOrder=triggerMetadata, optional, deprecated=use customHeaders instead"`
CortexOrgID string `keda:"name=cortexOrgID, order=triggerMetadata, optional, deprecated=use customHeaders instead"`
}

type promQueryResult struct {
Expand Down
59 changes: 40 additions & 19 deletions pkg/scalers/scalersconfig/typed_config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ import (
"runtime/debug"
"strconv"
"strings"

"golang.org/x/exp/maps"
"golang.org/x/exp/slices"
)

// CustomValidator is an interface that can be implemented to validate the configuration of the typed config
Expand All @@ -42,8 +45,15 @@ const (
AuthParams ParsingOrder = "authParams"
)

// allowedParsingOrderMap is a map with set of valid parsing orders
var allowedParsingOrderMap = map[ParsingOrder]bool{
TriggerMetadata: true,
ResolvedEnv: true,
AuthParams: true,
}

// separators for field tag structure
// e.g. name=stringVal,parsingOrder=triggerMetadata;resolvedEnv;authParams,optional
// e.g. name=stringVal,order=triggerMetadata;resolvedEnv;authParams,optional
const (
tagSeparator = ","
tagKeySeparator = "="
Expand All @@ -61,10 +71,10 @@ const (
optionalTag = "optional"
deprecatedTag = "deprecated"
defaultTag = "default"
parsingOrderTag = "parsingOrder"
orderTag = "order"
nameTag = "name"
enumTag = "enum"
exclusiveTag = "exclusive"
exclusiveSetTag = "exclusiveSet"
)

// Params is a struct that represents the parameter list that can be used in the keda tag
Expand All @@ -78,9 +88,9 @@ type Params struct {
// Optional is the 'optional' tag parameter defining if the parameter is optional
Optional bool

// ParsingOrder is the 'parsingOrder' tag parameter defining the order in which the parameter is looked up
// Order is the 'order' tag parameter defining the parsing order in which the parameter is looked up
// in the triggerMetadata, resolvedEnv or authParams maps
ParsingOrder []ParsingOrder
Order []ParsingOrder

// Default is the 'default' tag parameter defining the default value of the parameter if it's not found
// in any of the maps from ParsingOrder
Expand All @@ -93,8 +103,8 @@ type Params struct {
// Enum is the 'enum' tag parameter defining the list of possible values for the parameter
Enum []string

// Exclusive is the 'exclusive' tag parameter defining the list of values that are mutually exclusive
Exclusive []string
// ExclusiveSet is the 'exclusiveSet' tag parameter defining the list of values that are mutually exclusive
ExclusiveSet []string
}

// IsNested is a function that returns true if the parameter is nested
Expand Down Expand Up @@ -181,7 +191,12 @@ func (sc *ScalerConfig) setValue(field reflect.Value, params Params) error {
return nil
}
if !exists && !(params.Optional || params.IsDeprecated()) {
return fmt.Errorf("missing required parameter %q in %v", params.Name, params.ParsingOrder)
if len(params.Order) == 0 {
apo := maps.Keys(allowedParsingOrderMap)
slices.Sort(apo)
return fmt.Errorf("missing required parameter %q, no 'order' tag, provide any from %v", params.Name, apo)
}
return fmt.Errorf("missing required parameter %q in %v", params.Name, params.Order)
}
if params.Enum != nil {
enumMap := make(map[string]bool)
Expand All @@ -200,9 +215,9 @@ func (sc *ScalerConfig) setValue(field reflect.Value, params Params) error {
return fmt.Errorf("parameter %q value %q must be one of %v", params.Name, valFromConfig, params.Enum)
}
}
if params.Exclusive != nil {
if params.ExclusiveSet != nil {
exclusiveMap := make(map[string]bool)
for _, e := range params.Exclusive {
for _, e := range params.ExclusiveSet {
exclusiveMap[e] = true
}
split := strings.Split(valFromConfig, elemSeparator)
Expand All @@ -214,7 +229,7 @@ func (sc *ScalerConfig) setValue(field reflect.Value, params Params) error {
}
}
if exclusiveCount > 1 {
return fmt.Errorf("parameter %q value %q must contain only one of %v", params.Name, valFromConfig, params.Exclusive)
return fmt.Errorf("parameter %q value %q must contain only one of %v", params.Name, valFromConfig, params.ExclusiveSet)
}
}
if params.IsNested() {
Expand Down Expand Up @@ -326,7 +341,7 @@ func setConfigValueHelper(valFromConfig string, field reflect.Value) error {

// configParamValue is a function that returns the value of the parameter based on the parsing order
func (sc *ScalerConfig) configParamValue(params Params) (string, bool) {
for _, po := range params.ParsingOrder {
for _, po := range params.Order {
var m map[string]string
key := params.Name
switch po {
Expand All @@ -338,7 +353,8 @@ func (sc *ScalerConfig) configParamValue(params Params) (string, bool) {
m = sc.ResolvedEnv
key = sc.TriggerMetadata[fmt.Sprintf("%sFromEnv", params.Name)]
default:
m = sc.TriggerMetadata
// this is checked when parsing the tags but adding as default case to avoid any potential future problems
return "", false
}
if param, ok := m[key]; ok && param != "" {
return strings.TrimSpace(param), true
Expand All @@ -362,12 +378,17 @@ func paramsFromTag(tag string, field reflect.StructField) (Params, error) {
if len(tsplit) > 1 {
params.Optional, _ = strconv.ParseBool(strings.TrimSpace(tsplit[1]))
}
case parsingOrderTag:
case orderTag:
if len(tsplit) > 1 {
parsingOrder := strings.Split(tsplit[1], tagValueSeparator)
for _, po := range parsingOrder {
order := strings.Split(tsplit[1], tagValueSeparator)
for _, po := range order {
poTyped := ParsingOrder(strings.TrimSpace(po))
params.ParsingOrder = append(params.ParsingOrder, poTyped)
if !allowedParsingOrderMap[poTyped] {
apo := maps.Keys(allowedParsingOrderMap)
slices.Sort(apo)
return params, fmt.Errorf("unknown parsing order value %s, has to be one of %s", po, apo)
}
params.Order = append(params.Order, poTyped)
}
}
case nameTag:
Expand All @@ -388,9 +409,9 @@ func paramsFromTag(tag string, field reflect.StructField) (Params, error) {
if len(tsplit) > 1 {
params.Enum = strings.Split(tsplit[1], tagValueSeparator)
}
case exclusiveTag:
case exclusiveSetTag:
if len(tsplit) > 1 {
params.Exclusive = strings.Split(tsplit[1], tagValueSeparator)
params.ExclusiveSet = strings.Split(tsplit[1], tagValueSeparator)
}
case "":
continue
Expand Down
Loading

0 comments on commit 07676ba

Please sign in to comment.