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.

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

Make improvement to getParameterFromConfigV2 #5391

Merged
merged 4 commits into from
Feb 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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)
zroubalik marked this conversation as resolved.
Show resolved Hide resolved

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) {
zroubalik marked this conversation as resolved.
Show resolved Hide resolved
zroubalik marked this conversation as resolved.
Show resolved Hide resolved
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
Loading