From cdbcb9fe7e77aa4b35624ae9813b00b2c2200d70 Mon Sep 17 00:00:00 2001 From: Dao Thanh Tung Date: Tue, 6 Feb 2024 14:43:33 +0000 Subject: [PATCH] Make improvement to `getParameterFromConfigV2` (#5391) Signed-off-by: dttung2905 --- CHANGELOG.md | 2 +- pkg/scalers/scaler.go | 95 ++++++++++++++++++++++++++++++++++---- pkg/scalers/scaler_test.go | 12 ++--- 3 files changed, 92 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 73b4a04231b..42c442d62fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -82,6 +82,7 @@ New deprecation(s): ### Other +- **General**: Improve readability of utility function getParameterFromConfigV2 ([#5037](https://github.com/kedacore/keda/issues/5037)) - **General**: Minor refactor to reduce copy/paste code in ScaledObject webhook ([#5397](https://github.com/kedacore/keda/issues/5397)) - **General**: TODO ([#XXX](https://github.com/kedacore/keda/issues/XXX)) @@ -163,7 +164,6 @@ New deprecation(s): ### Other - **General**: Bump K8s deps to 0.28.5 ([#5346](https://github.com/kedacore/keda/pull/5346)) -- **General**: Create a common utility function to get parameter value from config ([#5037](https://github.com/kedacore/keda/issues/5037)) - **General**: Fix CVE-2023-45142 in OpenTelemetry ([#5089](https://github.com/kedacore/keda/issues/5089)) - **General**: Fix logger in OpenTelemetry collector ([#5094](https://github.com/kedacore/keda/issues/5094)) - **General**: Fix lost commit from the newly created utility function ([#5037](https://github.com/kedacore/keda/issues/5037)) diff --git a/pkg/scalers/scaler.go b/pkg/scalers/scaler.go index e7454681cc1..df37eb210e0 100644 --- a/pkg/scalers/scaler.go +++ b/pkg/scalers/scaler.go @@ -168,8 +168,83 @@ func GenerateMetricInMili(metricName string, value float64) external_metrics.Ext } } -// getParameterFromConfigV2 returns the value of the parameter from the config -func getParameterFromConfigV2(config *scalersconfig.ScalerConfig, parameter string, useMetadata bool, useAuthentication bool, useResolvedEnv bool, isOptional bool, defaultVal string, targetType reflect.Type) (interface{}, error) { +// Option represents a function type that modifies a configOptions instance. +type Option func(*configOptions) + +type configOptions struct { + useMetadata bool // Indicates whether to use metadata. + useAuthentication bool // Indicates whether to use authentication. + useResolvedEnv bool // Indicates whether to use resolved environment variables. + isOptional bool // Indicates whether the configuration is optional. + defaultVal interface{} // Default value for the configuration. +} + +// UseMetadata is an Option function that sets the useMetadata field of configOptions. +func UseMetadata(metadata bool) Option { + return func(opt *configOptions) { + opt.useMetadata = metadata + } +} + +// UseAuthentication is an Option function that sets the useAuthentication field of configOptions. +func UseAuthentication(auth bool) Option { + return func(opt *configOptions) { + opt.useAuthentication = auth + } +} + +// UseResolvedEnv is an Option function that sets the useResolvedEnv field of configOptions. +func UseResolvedEnv(resolvedEnv bool) Option { + return func(opt *configOptions) { + opt.useResolvedEnv = resolvedEnv + } +} + +// IsOptional is an Option function that sets the isOptional field of configOptions. +func IsOptional(optional bool) Option { + return func(opt *configOptions) { + opt.isOptional = optional + } +} + +// WithDefaultVal is an Option function that sets the defaultVal field of configOptions. +func WithDefaultVal(defaultVal interface{}) Option { + return func(opt *configOptions) { + opt.defaultVal = defaultVal + } +} + +// getParameterFromConfigV2 retrieves a parameter value from the provided ScalerConfig object based on the specified parameter name, target type, and optional configuration options. +// +// This method searches for the parameter value in different places within the ScalerConfig object, such as authentication parameters, trigger metadata, and resolved environment variables, based on the provided options. +// It then attempts to convert the found value to the specified target type and returns it. +// +// Parameters: +// +// config: A pointer to a ScalerConfig object from which to retrieve the parameter value. +// parameter: A string representing the name of the parameter to retrieve. +// targetType: A reflect.Type representing the target type to which the parameter value should be converted. +// options: An optional variadic parameter that allows configuring the behavior of the method through Option functions. +// +// Returns: +// - An interface{} representing the retrieved parameter value, converted to the specified target type. +// - An error, if any occurred during the retrieval or conversion process. +// +// Example Usage: +// +// To retrieve a parameter value from a ScalerConfig object, you can call this function with the necessary parameters and options +// +// ``` +// val, err := getParameterFromConfigV2(scalerConfig, "parameterName", reflect.TypeOf(int64(0)), UseMetadata(true), UseAuthentication(true)) +// if err != nil { +// // Handle error +// } +func getParameterFromConfigV2(config *scalersconfig.ScalerConfig, parameter string, targetType reflect.Type, options ...Option) (interface{}, error) { + opt := &configOptions{defaultVal: ""} + for _, option := range options { + option(opt) + } + foundCount := 0 var foundVal string var convertedVal interface{} @@ -177,20 +252,20 @@ func getParameterFromConfigV2(config *scalersconfig.ScalerConfig, parameter stri if val, ok := config.AuthParams[parameter]; ok && val != "" { foundCount++ - if useAuthentication { + if opt.useAuthentication { foundVal = val } } if val, ok := config.TriggerMetadata[parameter]; ok && val != "" { foundCount++ - if useMetadata { + if opt.useMetadata { foundVal = val } } if envFromVal, envFromOk := config.TriggerMetadata[fmt.Sprintf("%sFromEnv", parameter)]; envFromOk { if val, ok := config.ResolvedEnv[envFromVal]; ok && val != "" { foundCount++ - if useResolvedEnv { + if opt.useResolvedEnv { foundVal = val } } @@ -199,16 +274,16 @@ func getParameterFromConfigV2(config *scalersconfig.ScalerConfig, parameter stri convertedVal, foundErr = convertToType(foundVal, targetType) switch { case foundCount > 1: - return "", fmt.Errorf("value for parameter '%s' found in more than one place", parameter) + return opt.defaultVal, fmt.Errorf("value for parameter '%s' found in more than one place", parameter) case foundCount == 1: if foundErr != nil { - return defaultVal, foundErr + return opt.defaultVal, foundErr } return convertedVal, nil - case isOptional: - return defaultVal, nil + case opt.isOptional: + return opt.defaultVal, nil default: - return "", fmt.Errorf("key not found. Either set the correct key or set isOptional to true and set defaultVal") + return opt.defaultVal, fmt.Errorf("key not found. Either set the correct key or set isOptional to true and set defaultVal") } } diff --git a/pkg/scalers/scaler_test.go b/pkg/scalers/scaler_test.go index 2db21abd35e..59508b9ba58 100644 --- a/pkg/scalers/scaler_test.go +++ b/pkg/scalers/scaler_test.go @@ -257,12 +257,12 @@ func TestGetParameterFromConfigV2(t *testing.T) { val, err := getParameterFromConfigV2( &scalersconfig.ScalerConfig{TriggerMetadata: testData.metadata, AuthParams: testData.authParams, ResolvedEnv: testData.resolvedEnv}, testData.parameter, - testData.useMetadata, - testData.useAuthentication, - testData.useResolvedEnv, - testData.isOptional, - testData.defaultVal, testData.targetType, + UseMetadata(testData.useMetadata), + UseAuthentication(testData.useAuthentication), + UseResolvedEnv(testData.useResolvedEnv), + IsOptional(testData.isOptional), + WithDefaultVal(testData.defaultVal), ) if testData.isError { assert.NotNilf(t, err, "test %s: expected error but got success, testData - %+v", testData.name, testData) @@ -466,7 +466,7 @@ func TestConvertStringToType(t *testing.T) { if testData.isError { assert.NotNilf(t, err, "test %s: expected error but got success, testData - %+v", testData.name, testData) - assert.Containsf(t, err.Error(), testData.errorMessage, "test %s", testData.name, testData.errorMessage) + assert.Containsf(t, err.Error(), testData.errorMessage, "test %s: %s", testData.name, testData.errorMessage) } else { assert.Nil(t, err) assert.Equalf(t, testData.expectedOutput, val, "test %s: expected %s but got %s", testData.name, testData.expectedOutput, val)