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

scaler typed config: simplify missing parameter error #6194

Merged
merged 1 commit into from
Sep 29, 2024
Merged

scaler typed config: simplify missing parameter error #6194

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 5 additions & 5 deletions pkg/scalers/aws_dynamodb_scaler_test.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading