Skip to content

Commit

Permalink
Make improvement to getParameterFromConfigV2 (#5391)
Browse files Browse the repository at this point in the history
Signed-off-by: dttung2905 <ttdao.2015@accountancy.smu.edu.sg>
  • Loading branch information
dttung2905 committed Feb 6, 2024
1 parent cc150d4 commit cdbcb9f
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 17 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Expand Down Expand Up @@ -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))
Expand Down
95 changes: 85 additions & 10 deletions pkg/scalers/scaler.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,29 +168,104 @@ 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{}
var foundErr error

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
}
}
Expand All @@ -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")
}
}

Expand Down
12 changes: 6 additions & 6 deletions pkg/scalers/scaler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit cdbcb9f

Please sign in to comment.