Skip to content

Commit

Permalink
scaler typed config: simplify missing parameter error (#6194)
Browse files Browse the repository at this point in the history 
Signed-off-by: Jan Wozniak <[email protected]>
  • Loading branch information
@wozniakjan
wozniakjan authored Sep 29, 2024
1 parent 0b470a6 commit 69a9cb9
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 21 deletions.
10 changes: 5 additions & 5 deletions pkg/scalers/aws_dynamodb_scaler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,13 @@ type parseDynamoDBMetadataTestData struct {

var (
// ErrAwsDynamoNoTableName is returned when "tableName" is missing from the config.
ErrAwsDynamoNoTableName = errors.New("missing required parameter [\"tableName\"]")
ErrAwsDynamoNoTableName = errors.New(`missing required parameter "tableName"`)

// ErrAwsDynamoNoAwsRegion is returned when "awsRegion" is missing from the config.
ErrAwsDynamoNoAwsRegion = errors.New("missing required parameter [\"awsRegion\"]")
ErrAwsDynamoNoAwsRegion = errors.New(`missing required parameter "awsRegion"`)

// ErrAwsDynamoNoKeyConditionExpression is returned when "keyConditionExpression" is missing from the config.
ErrAwsDynamoNoKeyConditionExpression = errors.New("missing required parameter [\"keyConditionExpression\"]")
ErrAwsDynamoNoKeyConditionExpression = errors.New(`missing required parameter "keyConditionExpression"`)
)

var dynamoTestCases = []parseDynamoDBMetadataTestData{
Expand Down Expand Up @@ -114,7 +114,7 @@ var dynamoTestCases = []parseDynamoDBMetadataTestData{
"targetValue": "no-valid",
},
authParams: map[string]string{},
expectedError: errors.New("error parsing DynamoDb metadata: unable to set param [\"targetValue\"] value"),
expectedError: errors.New(`error parsing DynamoDb metadata: unable to set param "targetValue" value`),
},
{
name: "invalid activationTargetValue given",
Expand All @@ -128,7 +128,7 @@ var dynamoTestCases = []parseDynamoDBMetadataTestData{
"activationTargetValue": "no-valid",
},
authParams: map[string]string{},
expectedError: errors.New("unable to set param [\"activationTargetValue\"]"),
expectedError: errors.New(`unable to set param "activationTargetValue"`),
},
{
name: "malformed expressionAttributeNames",
Expand Down
27 changes: 16 additions & 11 deletions pkg/scalers/scalersconfig/typed_config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@ type Params struct {
// FieldName is the name of the field in the struct
FieldName string

// Name is the 'name' tag parameter defining the key in triggerMetadata, resolvedEnv or authParams
Name []string
// Names is the 'name' tag parameter defining the key in triggerMetadata, resolvedEnv or authParams
Names []string

// Optional is the 'optional' tag parameter defining if the parameter is optional
Optional bool
Expand Down Expand Up @@ -114,9 +114,14 @@ type Params struct {
Separator string
}

// Name returns the name of the parameter (or comma separated list of names if it has multiple)
func (p Params) Name() string {
return strings.Join(p.Names, ",")
}

// IsNested is a function that returns true if the parameter is nested
func (p Params) IsNested() bool {
return len(p.Name) == 0
return len(p.Names) == 0
}

// IsDeprecated is a function that returns true if the parameter is deprecated
Expand Down Expand Up @@ -188,7 +193,7 @@ func (sc *ScalerConfig) parseTypedConfig(typedConfig any, parentOptional bool) e
func (sc *ScalerConfig) setValue(field reflect.Value, params Params) error {
valFromConfig, exists := sc.configParamValue(params)
if exists && params.IsDeprecated() {
return fmt.Errorf("parameter %q is deprecated%v", params.Name, params.DeprecatedMessage())
return fmt.Errorf("parameter %q is deprecated%v", params.Name(), params.DeprecatedMessage())
}
if !exists && params.Default != "" {
exists = true
Expand All @@ -201,9 +206,9 @@ func (sc *ScalerConfig) setValue(field reflect.Value, params Params) error {
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, no 'order' tag, provide any from %v", params.Name(), apo)
}
return fmt.Errorf("missing required parameter %q in %v", params.Name, params.Order)
return fmt.Errorf("missing required parameter %q in %v", params.Name(), params.Order)
}
if params.Enum != nil {
enumMap := make(map[string]bool)
Expand All @@ -219,7 +224,7 @@ func (sc *ScalerConfig) setValue(field reflect.Value, params Params) error {
}
}
if len(missingMap) > 0 {
return fmt.Errorf("parameter %q value %q must be one of %v", params.Name, valFromConfig, params.Enum)
return fmt.Errorf("parameter %q value %q must be one of %v", params.Name(), valFromConfig, params.Enum)
}
}
if params.ExclusiveSet != nil {
Expand All @@ -236,7 +241,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.ExclusiveSet)
return fmt.Errorf("parameter %q value %q must contain only one of %v", params.Name(), valFromConfig, params.ExclusiveSet)
}
}
if params.IsNested() {
Expand All @@ -250,7 +255,7 @@ func (sc *ScalerConfig) setValue(field reflect.Value, params Params) error {
return sc.parseTypedConfig(field.Addr().Interface(), params.Optional)
}
if err := setConfigValueHelper(params, valFromConfig, field); err != nil {
return fmt.Errorf("unable to set param %q value %q: %w", params.Name, valFromConfig, err)
return fmt.Errorf("unable to set param %q value %q: %w", params.Name(), valFromConfig, err)
}
return nil
}
Expand Down Expand Up @@ -406,7 +411,7 @@ func setConfigValueHelper(params Params, valFromConfig string, field reflect.Val
func (sc *ScalerConfig) configParamValue(params Params) (string, bool) {
for _, po := range params.Order {
var m map[string]string
for _, key := range params.Name {
for _, key := range params.Names {
switch po {
case TriggerMetadata:
m = sc.TriggerMetadata
Expand Down Expand Up @@ -459,7 +464,7 @@ func paramsFromTag(tag string, field reflect.StructField) (Params, error) {
}
case nameTag:
if len(tsplit) > 1 {
params.Name = strings.Split(strings.TrimSpace(tsplit[1]), tagValueSeparator)
params.Names = strings.Split(strings.TrimSpace(tsplit[1]), tagValueSeparator)
}
case deprecatedTag:
if len(tsplit) == 1 {
Expand Down
10 changes: 5 additions & 5 deletions pkg/scalers/scalersconfig/typed_config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ func TestMissing(t *testing.T) {

ts := testStruct{}
err := sc.TypedConfig(&ts)
Expect(err).To(MatchError(`missing required parameter ["stringVal"] in [triggerMetadata]`))
Expect(err).To(MatchError(`missing required parameter "stringVal" in [triggerMetadata]`))
}

// TestDeprecated tests the deprecated tag
Expand All @@ -152,7 +152,7 @@ func TestDeprecated(t *testing.T) {

ts := testStruct{}
err := sc.TypedConfig(&ts)
Expect(err).To(MatchError(`parameter ["stringVal"] is deprecated`))
Expect(err).To(MatchError(`parameter "stringVal" is deprecated`))

sc2 := &ScalerConfig{
TriggerMetadata: map[string]string{},
Expand Down Expand Up @@ -276,7 +276,7 @@ func TestEnum(t *testing.T) {

ts2 := testStruct{}
err = sc2.TypedConfig(&ts2)
Expect(err).To(MatchError(`parameter ["enumVal"] value "value3" must be one of [value1 value2]`))
Expect(err).To(MatchError(`parameter "enumVal" value "value3" must be one of [value1 value2]`))
}

// TestExclusive tests the exclusiveSet type
Expand Down Expand Up @@ -305,7 +305,7 @@ func TestExclusive(t *testing.T) {

ts2 := testStruct{}
err = sc2.TypedConfig(&ts2)
Expect(err).To(MatchError(`parameter ["intVal"] value "1,4" must contain only one of [1 4 5]`))
Expect(err).To(MatchError(`parameter "intVal" value "1,4" must contain only one of [1 4 5]`))
}

// TestURLValues tests the url.Values type
Expand Down Expand Up @@ -503,7 +503,7 @@ func TestNoParsingOrder(t *testing.T) {
}
tsm := testStructMissing{}
err := sc.TypedConfig(&tsm)
Expect(err).To(MatchError(ContainSubstring(`missing required parameter ["strVal"], no 'order' tag, provide any from [authParams resolvedEnv triggerMetadata]`)))
Expect(err).To(MatchError(ContainSubstring(`missing required parameter "strVal", no 'order' tag, provide any from [authParams resolvedEnv triggerMetadata]`)))

type testStructDefault struct {
DefaultVal string `keda:"name=defaultVal, default=dv"`
Expand Down

0 comments on commit 69a9cb9

Please sign in to comment.