From 69a9cb9869012bd487972fcbb55bed960b678926 Mon Sep 17 00:00:00 2001 From: Jan Wozniak Date: Sun, 29 Sep 2024 21:58:50 +0200 Subject: [PATCH] scaler typed config: simplify missing parameter error (#6194) Signed-off-by: Jan Wozniak --- pkg/scalers/aws_dynamodb_scaler_test.go | 10 +++---- pkg/scalers/scalersconfig/typed_config.go | 27 +++++++++++-------- .../scalersconfig/typed_config_test.go | 10 +++---- 3 files changed, 26 insertions(+), 21 deletions(-) diff --git a/pkg/scalers/aws_dynamodb_scaler_test.go b/pkg/scalers/aws_dynamodb_scaler_test.go index 869e0b97178..72d7664203d 100644 --- a/pkg/scalers/aws_dynamodb_scaler_test.go +++ b/pkg/scalers/aws_dynamodb_scaler_test.go @@ -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{ @@ -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", @@ -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", diff --git a/pkg/scalers/scalersconfig/typed_config.go b/pkg/scalers/scalersconfig/typed_config.go index 896955ec70a..4e61f3e288d 100644 --- a/pkg/scalers/scalersconfig/typed_config.go +++ b/pkg/scalers/scalersconfig/typed_config.go @@ -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 @@ -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 @@ -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 @@ -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) @@ -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 { @@ -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() { @@ -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 } @@ -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 @@ -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 { diff --git a/pkg/scalers/scalersconfig/typed_config_test.go b/pkg/scalers/scalersconfig/typed_config_test.go index d817f9f108f..866311f574a 100644 --- a/pkg/scalers/scalersconfig/typed_config_test.go +++ b/pkg/scalers/scalersconfig/typed_config_test.go @@ -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 @@ -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{}, @@ -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 @@ -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 @@ -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"`