diff --git a/CHANGELOG.md b/CHANGELOG.md index b9712052c54..0e1115b09be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -73,6 +73,7 @@ New deprecation(s): ### Other - **RabbitMQ Scaler:** Move from `streadway/amqp` to `rabbitmq/amqp091-go` ([#4004](https://github.com/kedacore/keda/pull/4039)) +- **General:** Compare error with `errors.Is` ([#4004](https://github.com/kedacore/keda/pull/4004)) ## v2.9.1 diff --git a/pkg/scalers/aws_common.go b/pkg/scalers/aws_common.go index 071bae35e09..5452633da85 100644 --- a/pkg/scalers/aws_common.go +++ b/pkg/scalers/aws_common.go @@ -1,6 +1,7 @@ package scalers import ( + "errors" "fmt" "github.com/aws/aws-sdk-go/aws" @@ -9,6 +10,9 @@ import ( "github.com/aws/aws-sdk-go/aws/session" ) +// ErrAwsNoAccessKey is returned when awsAccessKeyID is missing. +var ErrAwsNoAccessKey = errors.New("awsAccessKeyID not found") + type awsAuthorizationMetadata struct { awsRoleArn string @@ -81,7 +85,7 @@ func getAwsAuthorization(authParams, metadata, resolvedEnv map[string]string) (a } if len(meta.awsAccessKeyID) == 0 { - return meta, fmt.Errorf("awsAccessKeyID not found") + return meta, ErrAwsNoAccessKey } if metadata["awsSecretAccessKeyFromEnv"] != "" { diff --git a/pkg/scalers/aws_dynamodb_scaler.go b/pkg/scalers/aws_dynamodb_scaler.go index 6a5a8022c40..0081975d9da 100644 --- a/pkg/scalers/aws_dynamodb_scaler.go +++ b/pkg/scalers/aws_dynamodb_scaler.go @@ -58,19 +58,48 @@ func NewAwsDynamoDBScaler(config *ScalerConfig) (Scaler, error) { }, nil } +var ( + // ErrAwsDynamoNoTableName is returned when "tableName" is missing from the config. + ErrAwsDynamoNoTableName = errors.New("no tableName given") + + // ErrAwsDynamoNoAwsRegion is returned when "awsRegion" is missing from the config. + ErrAwsDynamoNoAwsRegion = errors.New("no awsRegion given") + + // ErrAwsDynamoNoKeyConditionExpression is returned when "keyConditionExpression" is missing from the config. + ErrAwsDynamoNoKeyConditionExpression = errors.New("no keyConditionExpression given") + + // ErrAwsDynamoEmptyExpressionAttributeNames is returned when "expressionAttributeNames" is empty. + ErrAwsDynamoEmptyExpressionAttributeNames = errors.New("empty map") + + // ErrAwsDynamoInvalidExpressionAttributeNames is returned when "expressionAttributeNames" is an invalid JSON. + ErrAwsDynamoInvalidExpressionAttributeNames = errors.New("invalid expressionAttributeNames") + + // ErrAwsDynamoNoExpressionAttributeNames is returned when "expressionAttributeNames" is missing from the config. + ErrAwsDynamoNoExpressionAttributeNames = errors.New("no expressionAttributeNames given") + + // ErrAwsDynamoInvalidExpressionAttributeValues is returned when "expressionAttributeNames" is missing an invalid JSON. + ErrAwsDynamoInvalidExpressionAttributeValues = errors.New("invalid expressionAttributeValues") + + // ErrAwsDynamoNoExpressionAttributeValues is returned when "expressionAttributeValues" is missing from the config. + ErrAwsDynamoNoExpressionAttributeValues = errors.New("no expressionAttributeValues given") + + // ErrAwsDynamoNoTargetValue is returned when "targetValue" is missing from the config. + ErrAwsDynamoNoTargetValue = errors.New("no targetValue given") +) + func parseAwsDynamoDBMetadata(config *ScalerConfig) (*awsDynamoDBMetadata, error) { meta := awsDynamoDBMetadata{} if val, ok := config.TriggerMetadata["tableName"]; ok && val != "" { meta.tableName = val } else { - return nil, fmt.Errorf("no tableName given") + return nil, ErrAwsDynamoNoTableName } if val, ok := config.TriggerMetadata["awsRegion"]; ok && val != "" { meta.awsRegion = val } else { - return nil, fmt.Errorf("no awsRegion given") + return nil, ErrAwsDynamoNoAwsRegion } if val, ok := config.TriggerMetadata["awsEndpoint"]; ok { @@ -80,48 +109,48 @@ func parseAwsDynamoDBMetadata(config *ScalerConfig) (*awsDynamoDBMetadata, error if val, ok := config.TriggerMetadata["keyConditionExpression"]; ok && val != "" { meta.keyConditionExpression = val } else { - return nil, fmt.Errorf("no keyConditionExpression given") + return nil, ErrAwsDynamoNoKeyConditionExpression } if val, ok := config.TriggerMetadata["expressionAttributeNames"]; ok && val != "" { names, err := json2Map(val) if err != nil { - return nil, fmt.Errorf("error parsing expressionAttributeNames: %s", err) + return nil, fmt.Errorf("error parsing expressionAttributeNames: %w", err) } meta.expressionAttributeNames = names } else { - return nil, fmt.Errorf("no expressionAttributeNames given") + return nil, ErrAwsDynamoNoExpressionAttributeNames } if val, ok := config.TriggerMetadata["expressionAttributeValues"]; ok && val != "" { values, err := json2DynamoMap(val) if err != nil { - return nil, fmt.Errorf("error parsing expressionAttributeValues: %s", err) + return nil, fmt.Errorf("error parsing expressionAttributeValues: %w", err) } meta.expressionAttributeValues = values } else { - return nil, fmt.Errorf("no expressionAttributeValues given") + return nil, ErrAwsDynamoNoExpressionAttributeValues } if val, ok := config.TriggerMetadata["targetValue"]; ok && val != "" { n, err := strconv.ParseInt(val, 10, 64) if err != nil { - return nil, fmt.Errorf("error parsing metadata targetValue") + return nil, fmt.Errorf("error parsing metadata targetValue: %w", err) } meta.targetValue = n } else { - return nil, fmt.Errorf("no targetValue given") + return nil, ErrAwsDynamoNoTargetValue } if val, ok := config.TriggerMetadata["activationTargetValue"]; ok && val != "" { n, err := strconv.ParseInt(val, 10, 64) if err != nil { - return nil, fmt.Errorf("error parsing metadata targetValue") + return nil, fmt.Errorf("error parsing metadata activationTargetValue: %w", err) } meta.activationTargetValue = n @@ -202,11 +231,11 @@ func (s *awsDynamoDBScaler) GetQueryMetrics() (float64, error) { func json2Map(js string) (m map[string]*string, err error) { err = bson.UnmarshalExtJSON([]byte(js), true, &m) if err != nil { - return nil, err + return nil, fmt.Errorf("%w: %v", ErrAwsDynamoInvalidExpressionAttributeNames, err) } if len(m) == 0 { - return nil, errors.New("empty map") + return nil, ErrAwsDynamoEmptyExpressionAttributeNames } return m, err } @@ -216,7 +245,7 @@ func json2DynamoMap(js string) (m map[string]*dynamodb.AttributeValue, err error err = json.Unmarshal([]byte(js), &m) if err != nil { - return nil, err + return nil, fmt.Errorf("%w: %v", ErrAwsDynamoInvalidExpressionAttributeValues, err) } return m, err } diff --git a/pkg/scalers/aws_dynamodb_scaler_test.go b/pkg/scalers/aws_dynamodb_scaler_test.go index b07ccad3684..204833693fb 100644 --- a/pkg/scalers/aws_dynamodb_scaler_test.go +++ b/pkg/scalers/aws_dynamodb_scaler_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "strconv" "testing" "github.com/aws/aws-sdk-go/service/dynamodb" @@ -38,13 +39,13 @@ var dynamoTestCases = []parseDynamoDBMetadataTestData{ name: "no tableName given", metadata: map[string]string{}, authParams: map[string]string{}, - expectedError: errors.New("no tableName given"), + expectedError: ErrAwsDynamoNoTableName, }, { name: "no awsRegion given", metadata: map[string]string{"tableName": "test"}, authParams: map[string]string{}, - expectedError: errors.New("no awsRegion given"), + expectedError: ErrAwsDynamoNoAwsRegion, }, { name: "no keyConditionExpression given", @@ -53,7 +54,7 @@ var dynamoTestCases = []parseDynamoDBMetadataTestData{ "awsRegion": "eu-west-1", }, authParams: map[string]string{}, - expectedError: errors.New("no keyConditionExpression given"), + expectedError: ErrAwsDynamoNoKeyConditionExpression, }, { name: "no expressionAttributeNames given", @@ -63,7 +64,7 @@ var dynamoTestCases = []parseDynamoDBMetadataTestData{ "keyConditionExpression": "#yr = :yyyy", }, authParams: map[string]string{}, - expectedError: errors.New("no expressionAttributeNames given"), + expectedError: ErrAwsDynamoNoExpressionAttributeNames, }, { name: "no expressionAttributeValues given", @@ -74,7 +75,7 @@ var dynamoTestCases = []parseDynamoDBMetadataTestData{ "expressionAttributeNames": "{ \"#yr\" : \"year\" }", }, authParams: map[string]string{}, - expectedError: errors.New("no expressionAttributeValues given"), + expectedError: ErrAwsDynamoNoExpressionAttributeValues, }, { name: "no targetValue given", @@ -86,7 +87,7 @@ var dynamoTestCases = []parseDynamoDBMetadataTestData{ "expressionAttributeValues": "{\":yyyy\": {\"N\": \"1994\"}}", }, authParams: map[string]string{}, - expectedError: errors.New("no targetValue given"), + expectedError: ErrAwsDynamoNoTargetValue, }, { name: "invalid targetValue given", @@ -99,7 +100,7 @@ var dynamoTestCases = []parseDynamoDBMetadataTestData{ "targetValue": "no-valid", }, authParams: map[string]string{}, - expectedError: errors.New("error parsing metadata targetValue"), + expectedError: strconv.ErrSyntax, }, { name: "invalid activationTargetValue given", @@ -113,7 +114,7 @@ var dynamoTestCases = []parseDynamoDBMetadataTestData{ "activationTargetValue": "no-valid", }, authParams: map[string]string{}, - expectedError: errors.New("error parsing metadata targetValue"), + expectedError: strconv.ErrSyntax, }, { name: "malformed expressionAttributeNames", @@ -126,7 +127,7 @@ var dynamoTestCases = []parseDynamoDBMetadataTestData{ "targetValue": "3", }, authParams: map[string]string{}, - expectedError: errors.New("error parsing expressionAttributeNames: invalid JSON input: missing colon after key \"malformed\""), + expectedError: ErrAwsDynamoInvalidExpressionAttributeNames, }, { name: "empty expressionAttributeNames", @@ -139,7 +140,7 @@ var dynamoTestCases = []parseDynamoDBMetadataTestData{ "targetValue": "3", }, authParams: map[string]string{}, - expectedError: errors.New("error parsing expressionAttributeNames: empty map"), + expectedError: ErrAwsDynamoEmptyExpressionAttributeNames, }, { name: "malformed expressionAttributeValues", @@ -152,7 +153,7 @@ var dynamoTestCases = []parseDynamoDBMetadataTestData{ "targetValue": "3", }, authParams: map[string]string{}, - expectedError: errors.New("error parsing expressionAttributeValues: json: cannot unmarshal number into Go struct field AttributeValue.N of type string"), + expectedError: ErrAwsDynamoInvalidExpressionAttributeValues, }, { name: "no aws given", @@ -165,7 +166,7 @@ var dynamoTestCases = []parseDynamoDBMetadataTestData{ "targetValue": "3", }, authParams: map[string]string{}, - expectedError: errors.New("awsAccessKeyID not found"), + expectedError: ErrAwsNoAccessKey, }, { name: "authentication provided", @@ -237,7 +238,7 @@ func TestParseDynamoMetadata(t *testing.T) { ScalerIndex: 1, }) if tc.expectedError != nil { - assert.Contains(t, err.Error(), tc.expectedError.Error()) + assert.ErrorIs(t, err, tc.expectedError) } else { assert.NoError(t, err) fmt.Println(tc.name) diff --git a/pkg/scalers/azure/azure_blob_test.go b/pkg/scalers/azure/azure_blob_test.go index 6197f780a70..da4460b933c 100644 --- a/pkg/scalers/azure/azure_blob_test.go +++ b/pkg/scalers/azure/azure_blob_test.go @@ -2,8 +2,9 @@ package azure import ( "context" + "encoding/base64" + "errors" "net/http" - "strings" "testing" kedav1alpha1 "github.com/kedacore/keda/v2/apis/keda/v1alpha1" @@ -22,7 +23,7 @@ func TestGetBlobLength(t *testing.T) { t.Error("Expected error for empty connection string, but got nil") } - if !strings.Contains(err.Error(), "parse storage connection string") { + if !errors.Is(err, ErrAzureConnectionStringKeyName) { t.Error("Expected error to contain parsing error message, but got", err.Error()) } @@ -37,7 +38,8 @@ func TestGetBlobLength(t *testing.T) { t.Error("Expected error for empty connection string, but got nil") } - if !strings.Contains(err.Error(), "illegal base64") { + var base64Error base64.CorruptInputError + if !errors.As(err, &base64Error) { t.Error("Expected error to contain base64 error message, but got", err.Error()) } } diff --git a/pkg/scalers/azure/azure_queue_test.go b/pkg/scalers/azure/azure_queue_test.go index 966a4b5c803..2b6ebd236e4 100644 --- a/pkg/scalers/azure/azure_queue_test.go +++ b/pkg/scalers/azure/azure_queue_test.go @@ -2,8 +2,9 @@ package azure import ( "context" + "encoding/base64" + "errors" "net/http" - "strings" "testing" kedav1alpha1 "github.com/kedacore/keda/v2/apis/keda/v1alpha1" @@ -19,7 +20,7 @@ func TestGetQueueLength(t *testing.T) { t.Error("Expected error for empty connection string, but got nil") } - if !strings.Contains(err.Error(), "parse storage connection string") { + if !errors.Is(err, ErrAzureConnectionStringKeyName) { t.Error("Expected error to contain parsing error message, but got", err.Error()) } @@ -33,7 +34,8 @@ func TestGetQueueLength(t *testing.T) { t.Error("Expected error for empty connection string, but got nil") } - if !strings.Contains(err.Error(), "illegal base64") { + var base64Error base64.CorruptInputError + if !errors.As(err, &base64Error) { t.Error("Expected error to contain base64 error message, but got", err.Error()) } } diff --git a/pkg/scalers/azure/azure_storage.go b/pkg/scalers/azure/azure_storage.go index 336e0495964..759fa62917c 100644 --- a/pkg/scalers/azure/azure_storage.go +++ b/pkg/scalers/azure/azure_storage.go @@ -55,6 +55,14 @@ const ( storageResource = "https://storage.azure.com/" ) +var ( + // ErrAzureConnectionStringKeyName indicates an error in the connection string AccountKey or AccountName. + ErrAzureConnectionStringKeyName = errors.New("can't parse storage connection string. Missing key or name") + + // ErrAzureConnectionStringEndpoint indicates an error in the connection string DefaultEndpointsProtocol or EndpointSuffix. + ErrAzureConnectionStringEndpoint = errors.New("can't parse storage connection string. Missing DefaultEndpointsProtocol or EndpointSuffix") +) + // Prefix returns prefix for a StorageEndpointType func (e StorageEndpointType) Prefix() string { return [...]string{"BlobEndpoint", "QueueEndpoint", "TableEndpoint", "FileEndpoint"}[e] @@ -169,7 +177,7 @@ func parseAzureStorageConnectionString(connectionString string, endpointType Sto } if name == "" || key == "" { - return nil, "", "", errors.New("can't parse storage connection string. Missing key or name") + return nil, "", "", ErrAzureConnectionStringKeyName } if endpoint != "" { @@ -181,7 +189,7 @@ func parseAzureStorageConnectionString(connectionString string, endpointType Sto } if endpointProtocol == "" || endpointSuffix == "" { - return nil, "", "", errors.New("can't parse storage connection string. Missing DefaultEndpointsProtocol or EndpointSuffix") + return nil, "", "", ErrAzureConnectionStringEndpoint } u, err := url.Parse(fmt.Sprintf("%s://%s.%s.%s", endpointProtocol, name, endpointType.Name(), endpointSuffix)) diff --git a/pkg/scalers/elasticsearch_scaler.go b/pkg/scalers/elasticsearch_scaler.go index e8ea15f3b08..5614717582d 100644 --- a/pkg/scalers/elasticsearch_scaler.go +++ b/pkg/scalers/elasticsearch_scaler.go @@ -5,6 +5,7 @@ import ( "context" "crypto/tls" "encoding/json" + "errors" "fmt" "io" "net/http" @@ -131,6 +132,14 @@ func extractCloudConfig(config *ScalerConfig, meta *elasticsearchMetadata) error return nil } +var ( + // ErrElasticsearchMissingAddressesOrCloudConfig is returned when endpoint addresses or cloud config is missing. + ErrElasticsearchMissingAddressesOrCloudConfig = errors.New("must provide either endpoint addresses or cloud config") + + // ErrElasticsearchConfigConflict is returned when both endpoint addresses and cloud config are provided. + ErrElasticsearchConfigConflict = errors.New("can't provide endpoint addresses and cloud config at the same time") +) + func parseElasticsearchMetadata(config *ScalerConfig) (*elasticsearchMetadata, error) { meta := elasticsearchMetadata{} @@ -138,7 +147,7 @@ func parseElasticsearchMetadata(config *ScalerConfig) (*elasticsearchMetadata, e addresses, err := GetFromAuthOrMeta(config, "addresses") cloudID, errCloudConfig := GetFromAuthOrMeta(config, "cloudID") if err != nil && errCloudConfig != nil { - return nil, fmt.Errorf("must provide either endpoint addresses or cloud config") + return nil, ErrElasticsearchMissingAddressesOrCloudConfig } if err == nil && addresses != "" { @@ -155,7 +164,7 @@ func parseElasticsearchMetadata(config *ScalerConfig) (*elasticsearchMetadata, e } if hasEndpointsConfig(&meta) && hasCloudConfig(&meta) { - return nil, fmt.Errorf("can't provide endpoint addresses and cloud config at the same time") + return nil, ErrElasticsearchConfigConflict } if val, ok := config.TriggerMetadata["unsafeSsl"]; ok { @@ -196,7 +205,7 @@ func parseElasticsearchMetadata(config *ScalerConfig) (*elasticsearchMetadata, e } targetValue, err := strconv.ParseFloat(targetValueString, 64) if err != nil { - return nil, fmt.Errorf("targetValue parsing error %s", err.Error()) + return nil, fmt.Errorf("targetValue parsing error: %w", err) } meta.targetValue = targetValue @@ -204,7 +213,7 @@ func parseElasticsearchMetadata(config *ScalerConfig) (*elasticsearchMetadata, e if val, ok := config.TriggerMetadata["activationTargetValue"]; ok { meta.activationTargetValue, err = strconv.ParseFloat(val, 64) if err != nil { - return nil, fmt.Errorf("activationTargetValue parsing error %s", err.Error()) + return nil, fmt.Errorf("activationTargetValue parsing error: %w", err) } } diff --git a/pkg/scalers/elasticsearch_scaler_test.go b/pkg/scalers/elasticsearch_scaler_test.go index 7ec1f1a4149..ac2100491d8 100644 --- a/pkg/scalers/elasticsearch_scaler_test.go +++ b/pkg/scalers/elasticsearch_scaler_test.go @@ -2,8 +2,8 @@ package scalers import ( "context" - "errors" "fmt" + "strconv" "testing" "github.com/stretchr/testify/assert" @@ -36,25 +36,25 @@ var testCases = []parseElasticsearchMetadataTestData{ name: "must provide either endpoint addresses or cloud config", metadata: map[string]string{}, authParams: map[string]string{}, - expectedError: errors.New("must provide either endpoint addresses or cloud config"), + expectedError: ErrElasticsearchMissingAddressesOrCloudConfig, }, { name: "no apiKey given", metadata: map[string]string{"cloudID": "my-cluster:xxxxxxxxxxx"}, authParams: map[string]string{}, - expectedError: errors.New("no apiKey given"), + expectedError: ErrScalerConfigMissingField, }, { name: "can't provide endpoint addresses and cloud config at the same time", metadata: map[string]string{"addresses": "http://localhost:9200", "cloudID": "my-cluster:xxxxxxxxxxx"}, authParams: map[string]string{"username": "admin", "apiKey": "xxxxxxxxx"}, - expectedError: errors.New("can't provide endpoint addresses and cloud config at the same time"), + expectedError: ErrElasticsearchConfigConflict, }, { name: "no index given", metadata: map[string]string{"addresses": "http://localhost:9200"}, authParams: map[string]string{"username": "admin"}, - expectedError: errors.New("no index given"), + expectedError: ErrScalerConfigMissingField, }, { name: "no searchTemplateName given", @@ -63,7 +63,7 @@ var testCases = []parseElasticsearchMetadataTestData{ "index": "index1", }, authParams: map[string]string{"username": "admin"}, - expectedError: errors.New("no searchTemplateName given"), + expectedError: ErrScalerConfigMissingField, }, { name: "no valueLocation given", @@ -73,7 +73,7 @@ var testCases = []parseElasticsearchMetadataTestData{ "searchTemplateName": "searchTemplateName", }, authParams: map[string]string{"username": "admin"}, - expectedError: errors.New("no valueLocation given"), + expectedError: ErrScalerConfigMissingField, }, { name: "no targetValue given", @@ -84,7 +84,7 @@ var testCases = []parseElasticsearchMetadataTestData{ "valueLocation": "toto", }, authParams: map[string]string{"username": "admin"}, - expectedError: errors.New("no targetValue given"), + expectedError: ErrScalerConfigMissingField, }, { name: "invalid targetValue", @@ -96,7 +96,7 @@ var testCases = []parseElasticsearchMetadataTestData{ "targetValue": "AA", }, authParams: map[string]string{"username": "admin"}, - expectedError: errors.New("targetValue parsing error strconv.ParseFloat: parsing \"AA\": invalid syntax"), + expectedError: strconv.ErrSyntax, }, { name: "invalid activationTargetValue", @@ -109,7 +109,7 @@ var testCases = []parseElasticsearchMetadataTestData{ "activationTargetValue": "AA", }, authParams: map[string]string{"username": "admin"}, - expectedError: errors.New("activationTargetValue parsing error strconv.ParseFloat: parsing \"AA\": invalid syntax"), + expectedError: strconv.ErrSyntax, }, { name: "all fields ok", @@ -301,7 +301,7 @@ func TestParseElasticsearchMetadata(t *testing.T) { ResolvedEnv: tc.resolvedEnv, }) if tc.expectedError != nil { - assert.Contains(t, err.Error(), tc.expectedError.Error()) + assert.ErrorIs(t, err, tc.expectedError) } else { assert.NoError(t, err) fmt.Println(tc.name) @@ -460,7 +460,7 @@ func TestElasticsearchGetMetricSpecForScaling(t *testing.T) { ScalerIndex: testData.scalerIndex, }) if testData.metadataTestData.expectedError != nil { - assert.Equal(t, err, testData.metadataTestData.expectedError) + assert.ErrorIs(t, err, testData.metadataTestData.expectedError) continue } if err != nil { diff --git a/pkg/scalers/gcp_storage_scaler.go b/pkg/scalers/gcp_storage_scaler.go index 5efe3e77c6a..a359096519f 100644 --- a/pkg/scalers/gcp_storage_scaler.go +++ b/pkg/scalers/gcp_storage_scaler.go @@ -2,9 +2,9 @@ package scalers import ( "context" + "errors" "fmt" "strconv" - "strings" "cloud.google.com/go/storage" "github.com/go-logr/logr" @@ -206,7 +206,7 @@ func (s *gcsScaler) getItemCount(ctx context.Context, maxCount int64) (int64, er break } if err != nil { - if strings.Contains(err.Error(), "bucket doesn't exist") { + if errors.Is(err, storage.ErrBucketNotExist) { s.logger.Info("Bucket " + s.metadata.bucketName + " doesn't exist") return 0, nil } diff --git a/pkg/scalers/liiklus_scaler.go b/pkg/scalers/liiklus_scaler.go index f3f8996b945..871c2b17963 100644 --- a/pkg/scalers/liiklus_scaler.go +++ b/pkg/scalers/liiklus_scaler.go @@ -46,6 +46,17 @@ const ( liiklusMetricType = "External" ) +var ( + // ErrLiiklusNoTopic is returned when "topic" in the config is empty. + ErrLiiklusNoTopic = errors.New("no topic provided") + + // ErrLiiklusNoAddress is returned when "address" in the config is empty. + ErrLiiklusNoAddress = errors.New("no liiklus API address provided") + + // ErrLiiklusNoGroup is returned when "group" in the config is empty. + ErrLiiklusNoGroup = errors.New("no consumer group provided") +) + // NewLiiklusScaler creates a new liiklusScaler scaler func NewLiiklusScaler(config *ScalerConfig) (Scaler, error) { metricType, err := GetMetricTargetType(config) @@ -157,7 +168,7 @@ func parseLiiklusMetadata(config *ScalerConfig) (*liiklusMetadata, error) { if val, ok := config.TriggerMetadata[liiklusActivationLagThresholdMetricName]; ok { t, err := strconv.ParseInt(val, 10, 64) if err != nil { - return nil, fmt.Errorf("error parsing %s: %s", liiklusActivationLagThresholdMetricName, err) + return nil, fmt.Errorf("error parsing %s: %w", liiklusActivationLagThresholdMetricName, err) } activationLagThreshold = t } @@ -173,11 +184,11 @@ func parseLiiklusMetadata(config *ScalerConfig) (*liiklusMetadata, error) { switch { case config.TriggerMetadata["topic"] == "": - return nil, errors.New("no topic provided") + return nil, ErrLiiklusNoTopic case config.TriggerMetadata["address"] == "": - return nil, errors.New("no liiklus API address provided") + return nil, ErrLiiklusNoAddress case config.TriggerMetadata["group"] == "": - return nil, errors.New("no consumer group provided") + return nil, ErrLiiklusNoGroup } return &liiklusMetadata{ diff --git a/pkg/scalers/liiklus_scaler_test.go b/pkg/scalers/liiklus_scaler_test.go index b929300a44a..28772f1165a 100644 --- a/pkg/scalers/liiklus_scaler_test.go +++ b/pkg/scalers/liiklus_scaler_test.go @@ -2,11 +2,12 @@ package scalers import ( "context" + "errors" + "strconv" "testing" "github.com/go-logr/logr" "github.com/golang/mock/gomock" - "github.com/pkg/errors" "github.com/kedacore/keda/v2/pkg/scalers/liiklus" mock_liiklus "github.com/kedacore/keda/v2/pkg/scalers/liiklus/mocks" @@ -28,11 +29,11 @@ type liiklusMetricIdentifier struct { } var parseLiiklusMetadataTestDataset = []parseLiiklusMetadataTestData{ - {map[string]string{}, errors.New("no topic provided"), "", "", "", 0}, - {map[string]string{"topic": "foo"}, errors.New("no liiklus API address provided"), "", "", "", 0}, - {map[string]string{"topic": "foo", "address": "bar:6565"}, errors.New("no consumer group provided"), "", "", "", 0}, + {map[string]string{}, ErrLiiklusNoTopic, "", "", "", 0}, + {map[string]string{"topic": "foo"}, ErrLiiklusNoAddress, "", "", "", 0}, + {map[string]string{"topic": "foo", "address": "bar:6565"}, ErrLiiklusNoGroup, "", "", "", 0}, {map[string]string{"topic": "foo", "address": "bar:6565", "group": "mygroup"}, nil, "bar:6565", "mygroup", "foo", 10}, - {map[string]string{"topic": "foo", "address": "bar:6565", "group": "mygroup", "activationLagThreshold": "aa"}, errors.New("error parsing activationLagThreshold: strconv.ParseInt: parsing \"aa\": invalid syntax"), "bar:6565", "mygroup", "foo", 10}, + {map[string]string{"topic": "foo", "address": "bar:6565", "group": "mygroup", "activationLagThreshold": "aa"}, strconv.ErrSyntax, "bar:6565", "mygroup", "foo", 10}, {map[string]string{"topic": "foo", "address": "bar:6565", "group": "mygroup", "lagThreshold": "15"}, nil, "bar:6565", "mygroup", "foo", 15}, } @@ -52,7 +53,7 @@ func TestLiiklusParseMetadata(t *testing.T) { t.Error("Expected error but got success") continue } - if testData.err != nil && err != nil && testData.err.Error() != err.Error() { + if testData.err != nil && err != nil && !errors.Is(err, testData.err) { t.Errorf("Expected error %v but got %v", testData.err, err) continue } diff --git a/pkg/scalers/mssql_scaler.go b/pkg/scalers/mssql_scaler.go index ca5b9c3d050..e86740e83cb 100644 --- a/pkg/scalers/mssql_scaler.go +++ b/pkg/scalers/mssql_scaler.go @@ -3,6 +3,7 @@ package scalers import ( "context" "database/sql" + "errors" "fmt" "net" "net/url" @@ -17,6 +18,14 @@ import ( kedautil "github.com/kedacore/keda/v2/pkg/util" ) +var ( + // ErrMsSQLNoQuery is returned when "query" is missing from the config. + ErrMsSQLNoQuery = errors.New("no query given") + + // ErrMsSQLNoTargetValue is returned when "targetValue" is missing from the config. + ErrMsSQLNoTargetValue = errors.New("no targetValue given") +) + // mssqlScaler exposes a data pointer to mssqlMetadata and sql.DB connection type mssqlScaler struct { metricType v2.MetricTargetType @@ -98,7 +107,7 @@ func parseMSSQLMetadata(config *ScalerConfig) (*mssqlMetadata, error) { if val, ok := config.TriggerMetadata["query"]; ok { meta.query = val } else { - return nil, fmt.Errorf("no query given") + return nil, ErrMsSQLNoQuery } // Target query value @@ -109,7 +118,7 @@ func parseMSSQLMetadata(config *ScalerConfig) (*mssqlMetadata, error) { } meta.targetValue = targetValue } else { - return nil, fmt.Errorf("no targetValue given") + return nil, ErrMsSQLNoTargetValue } // Activation target value diff --git a/pkg/scalers/mssql_scaler_test.go b/pkg/scalers/mssql_scaler_test.go index 9e80a8a31db..2b56d7eab66 100644 --- a/pkg/scalers/mssql_scaler_test.go +++ b/pkg/scalers/mssql_scaler_test.go @@ -92,21 +92,21 @@ var testInputs = []mssqlTestData{ metadata: map[string]string{"targetValue": "1"}, resolvedEnv: map[string]string{}, authParams: map[string]string{"connectionString": "sqlserver://localhost"}, - expectedError: errors.New("no query given"), + expectedError: ErrMsSQLNoQuery, }, // Error: missing targetValue { metadata: map[string]string{"query": "SELECT 1"}, resolvedEnv: map[string]string{}, authParams: map[string]string{"connectionString": "sqlserver://localhost"}, - expectedError: errors.New("no targetValue given"), + expectedError: ErrMsSQLNoTargetValue, }, // Error: missing host { metadata: map[string]string{"query": "SELECT 1", "targetValue": "1"}, resolvedEnv: map[string]string{}, authParams: map[string]string{}, - expectedError: errors.New("no host given"), + expectedError: ErrScalerConfigMissingField, }, } @@ -122,7 +122,7 @@ func TestMSSQLMetadataParsing(t *testing.T) { if err != nil { if testData.expectedError == nil { t.Errorf("Unexpected error parsing input metadata: %v", err) - } else if testData.expectedError.Error() != err.Error() { + } else if !errors.Is(err, testData.expectedError) { t.Errorf("Expected error '%v' but got '%v'", testData.expectedError, err) } diff --git a/pkg/scalers/redis_scaler.go b/pkg/scalers/redis_scaler.go index 7f37733eef9..f917ff738e3 100644 --- a/pkg/scalers/redis_scaler.go +++ b/pkg/scalers/redis_scaler.go @@ -3,6 +3,7 @@ package scalers import ( "context" "crypto/tls" + "errors" "fmt" "net" "strconv" @@ -23,6 +24,17 @@ const ( defaultEnableTLS = false ) +var ( + // ErrRedisNoListName is returned when "listName" is missing from the config. + ErrRedisNoListName = errors.New("no list name given") + + // ErrRedisNoAddresses is returned when the "addresses" in the connection info is empty. + ErrRedisNoAddresses = errors.New("no addresses or hosts given. address should be a comma separated list of host:port or set the host/port values") + + // ErrRedisUnequalHostsAndPorts is returned when the number of hosts and ports are unequal. + ErrRedisUnequalHostsAndPorts = errors.New("not enough hosts or ports given. number of hosts should be equal to the number of ports") +) + type redisAddressParser func(metadata, resolvedEnv, authParams map[string]string) (redisConnectionInfo, error) type redisScaler struct { @@ -207,7 +219,7 @@ func parseRedisMetadata(config *ScalerConfig, parserFn redisAddressParser) (*red if val, ok := config.TriggerMetadata["listLength"]; ok { listLength, err := strconv.ParseInt(val, 10, 64) if err != nil { - return nil, fmt.Errorf("list length parsing error %s", err.Error()) + return nil, fmt.Errorf("list length parsing error: %w", err) } meta.listLength = listLength } @@ -224,7 +236,7 @@ func parseRedisMetadata(config *ScalerConfig, parserFn redisAddressParser) (*red if val, ok := config.TriggerMetadata["listName"]; ok { meta.listName = val } else { - return nil, fmt.Errorf("no list name given") + return nil, ErrRedisNoListName } meta.databaseIndex = defaultDBIdx @@ -357,7 +369,7 @@ func parseRedisMultipleAddress(metadata, resolvedEnv, authParams map[string]stri if len(info.hosts) != 0 && len(info.ports) != 0 { if len(info.hosts) != len(info.ports) { - return info, fmt.Errorf("not enough hosts or ports given. number of hosts should be equal to the number of ports") + return info, ErrRedisUnequalHostsAndPorts } for i := range info.hosts { info.addresses = append(info.addresses, net.JoinHostPort(info.hosts[i], info.ports[i])) @@ -366,7 +378,7 @@ func parseRedisMultipleAddress(metadata, resolvedEnv, authParams map[string]stri } if len(info.addresses) == 0 { - return info, fmt.Errorf("no addresses or hosts given. address should be a comma separated list of host:port or set the host/port values") + return info, ErrRedisNoAddresses } return info, nil diff --git a/pkg/scalers/redis_scaler_test.go b/pkg/scalers/redis_scaler_test.go index 6fcbb76d556..c822725f13f 100644 --- a/pkg/scalers/redis_scaler_test.go +++ b/pkg/scalers/redis_scaler_test.go @@ -2,7 +2,7 @@ package scalers import ( "context" - "errors" + "strconv" "testing" "github.com/go-logr/logr" @@ -114,7 +114,7 @@ func TestParseRedisClusterMetadata(t *testing.T) { { name: "empty metadata", wantMeta: nil, - wantErr: errors.New("no addresses or hosts given. address should be a comma separated list of host:port or set the host/port values"), + wantErr: ErrRedisNoAddresses, }, { name: "unequal number of hosts/ports", @@ -123,7 +123,7 @@ func TestParseRedisClusterMetadata(t *testing.T) { "ports": "1, 2", }, wantMeta: nil, - wantErr: errors.New("not enough hosts or ports given. number of hosts should be equal to the number of ports"), + wantErr: ErrRedisUnequalHostsAndPorts, }, { name: "no list name", @@ -133,7 +133,7 @@ func TestParseRedisClusterMetadata(t *testing.T) { "listLength": "5", }, wantMeta: nil, - wantErr: errors.New("no list name given"), + wantErr: ErrRedisNoListName, }, { name: "invalid list length", @@ -144,7 +144,7 @@ func TestParseRedisClusterMetadata(t *testing.T) { "listLength": "invalid", }, wantMeta: nil, - wantErr: errors.New("list length parsing error"), + wantErr: strconv.ErrSyntax, }, { name: "address is defined in auth params", @@ -345,7 +345,7 @@ func TestParseRedisClusterMetadata(t *testing.T) { } meta, err := parseRedisMetadata(config, parseRedisClusterAddress) if c.wantErr != nil { - assert.Contains(t, err.Error(), c.wantErr.Error()) + assert.ErrorIs(t, err, c.wantErr) } else { assert.NoError(t, err) } @@ -366,7 +366,7 @@ func TestParseRedisSentinelMetadata(t *testing.T) { { name: "empty metadata", wantMeta: nil, - wantErr: errors.New("no addresses or hosts given. address should be a comma separated list of host:port or set the host/port values"), + wantErr: ErrRedisNoAddresses, }, { name: "unequal number of hosts/ports", @@ -375,7 +375,7 @@ func TestParseRedisSentinelMetadata(t *testing.T) { "ports": "1, 2", }, wantMeta: nil, - wantErr: errors.New("not enough hosts or ports given. number of hosts should be equal to the number of ports"), + wantErr: ErrRedisUnequalHostsAndPorts, }, { name: "no list name", @@ -385,7 +385,7 @@ func TestParseRedisSentinelMetadata(t *testing.T) { "listLength": "5", }, wantMeta: nil, - wantErr: errors.New("no list name given"), + wantErr: ErrRedisNoListName, }, { name: "invalid list length", @@ -396,7 +396,7 @@ func TestParseRedisSentinelMetadata(t *testing.T) { "listLength": "invalid", }, wantMeta: nil, - wantErr: errors.New("list length parsing error"), + wantErr: strconv.ErrSyntax, }, { name: "address is defined in auth params", @@ -791,7 +791,7 @@ func TestParseRedisSentinelMetadata(t *testing.T) { } meta, err := parseRedisMetadata(config, parseRedisSentinelAddress) if c.wantErr != nil { - assert.Contains(t, err.Error(), c.wantErr.Error()) + assert.ErrorIs(t, err, c.wantErr) } else { assert.NoError(t, err) } diff --git a/pkg/scalers/redis_streams_scaler.go b/pkg/scalers/redis_streams_scaler.go index 799426cb672..907fa381df1 100644 --- a/pkg/scalers/redis_streams_scaler.go +++ b/pkg/scalers/redis_streams_scaler.go @@ -2,6 +2,7 @@ package scalers import ( "context" + "errors" "fmt" "strconv" @@ -149,6 +150,14 @@ func createScaler(client *redis.Client, meta *redisStreamsMetadata, metricType v }, nil } +var ( + // ErrRedisMissingPendingEntriesCount is returned when "pendingEntriesCount" is missing. + ErrRedisMissingPendingEntriesCount = errors.New("missing pending entries count") + + // ErrRedisMissingStreamName is returned when "stream" is missing. + ErrRedisMissingStreamName = errors.New("missing redis stream name") +) + func parseRedisStreamsMetadata(config *ScalerConfig, parseFn redisAddressParser) (*redisStreamsMetadata, error) { connInfo, err := parseFn(config.TriggerMetadata, config.ResolvedEnv, config.AuthParams) if err != nil { @@ -181,17 +190,17 @@ func parseRedisStreamsMetadata(config *ScalerConfig, parseFn redisAddressParser) if val, ok := config.TriggerMetadata[pendingEntriesCountMetadata]; ok { pendingEntriesCount, err := strconv.ParseInt(val, 10, 64) if err != nil { - return nil, fmt.Errorf("error parsing pending entries count %v", err) + return nil, fmt.Errorf("error parsing pending entries count: %w", err) } meta.targetPendingEntriesCount = pendingEntriesCount } else { - return nil, fmt.Errorf("missing pending entries count") + return nil, ErrRedisMissingPendingEntriesCount } if val, ok := config.TriggerMetadata[streamNameMetadata]; ok { meta.streamName = val } else { - return nil, fmt.Errorf("missing redis stream name") + return nil, ErrRedisMissingStreamName } if val, ok := config.TriggerMetadata[consumerGroupNameMetadata]; ok { diff --git a/pkg/scalers/redis_streams_scaler_test.go b/pkg/scalers/redis_streams_scaler_test.go index 43151884c6c..2eadc19ad3e 100644 --- a/pkg/scalers/redis_streams_scaler_test.go +++ b/pkg/scalers/redis_streams_scaler_test.go @@ -2,7 +2,6 @@ package scalers import ( "context" - "errors" "strconv" "testing" @@ -168,7 +167,7 @@ func TestParseRedisClusterStreamsMetadata(t *testing.T) { { name: "empty metadata", wantMeta: nil, - wantErr: errors.New("no addresses or hosts given. address should be a comma separated list of host:port or set the host/port values"), + wantErr: ErrRedisNoAddresses, }, { name: "unequal number of hosts/ports", @@ -177,7 +176,7 @@ func TestParseRedisClusterStreamsMetadata(t *testing.T) { "ports": "1, 2", }, wantMeta: nil, - wantErr: errors.New("not enough hosts or ports given. number of hosts should be equal to the number of ports"), + wantErr: ErrRedisUnequalHostsAndPorts, }, { name: "no stream name", @@ -187,7 +186,7 @@ func TestParseRedisClusterStreamsMetadata(t *testing.T) { "pendingEntriesCount": "5", }, wantMeta: nil, - wantErr: errors.New("missing redis stream name"), + wantErr: ErrRedisMissingStreamName, }, { name: "missing pending entries count", @@ -197,7 +196,7 @@ func TestParseRedisClusterStreamsMetadata(t *testing.T) { "stream": "my-stream", }, wantMeta: nil, - wantErr: errors.New("missing pending entries count"), + wantErr: ErrRedisMissingPendingEntriesCount, }, { name: "invalid pending entries count", @@ -207,7 +206,7 @@ func TestParseRedisClusterStreamsMetadata(t *testing.T) { "pendingEntriesCount": "invalid", }, wantMeta: nil, - wantErr: errors.New("error parsing pending entries count"), + wantErr: strconv.ErrSyntax, }, { name: "address is defined in auth params", @@ -445,7 +444,7 @@ func TestParseRedisClusterStreamsMetadata(t *testing.T) { } meta, err := parseRedisStreamsMetadata(config, parseRedisClusterAddress) if c.wantErr != nil { - assert.Contains(t, err.Error(), c.wantErr.Error()) + assert.ErrorIs(t, err, c.wantErr) } else { assert.NoError(t, err) } @@ -466,7 +465,7 @@ func TestParseRedisSentinelStreamsMetadata(t *testing.T) { { name: "empty metadata", wantMeta: nil, - wantErr: errors.New("no addresses or hosts given. address should be a comma separated list of host:port or set the host/port values"), + wantErr: ErrRedisNoAddresses, }, { name: "unequal number of hosts/ports", @@ -475,7 +474,7 @@ func TestParseRedisSentinelStreamsMetadata(t *testing.T) { "ports": "1, 2", }, wantMeta: nil, - wantErr: errors.New("not enough hosts or ports given. number of hosts should be equal to the number of ports"), + wantErr: ErrRedisUnequalHostsAndPorts, }, { name: "no stream name", @@ -485,7 +484,7 @@ func TestParseRedisSentinelStreamsMetadata(t *testing.T) { "pendingEntriesCount": "5", }, wantMeta: nil, - wantErr: errors.New("missing redis stream name"), + wantErr: ErrRedisMissingStreamName, }, { name: "missing pending entries count", @@ -495,7 +494,7 @@ func TestParseRedisSentinelStreamsMetadata(t *testing.T) { "stream": "my-stream", }, wantMeta: nil, - wantErr: errors.New("missing pending entries count"), + wantErr: ErrRedisMissingPendingEntriesCount, }, { name: "invalid pending entries count", @@ -505,7 +504,7 @@ func TestParseRedisSentinelStreamsMetadata(t *testing.T) { "pendingEntriesCount": "invalid", }, wantMeta: nil, - wantErr: errors.New("error parsing pending entries count"), + wantErr: strconv.ErrSyntax, }, { name: "address is defined in auth params", @@ -941,7 +940,7 @@ func TestParseRedisSentinelStreamsMetadata(t *testing.T) { } meta, err := parseRedisStreamsMetadata(config, parseRedisSentinelAddress) if c.wantErr != nil { - assert.Contains(t, err.Error(), c.wantErr.Error()) + assert.ErrorIs(t, err, c.wantErr) } else { assert.NoError(t, err) } diff --git a/pkg/scalers/scaler.go b/pkg/scalers/scaler.go index c8dad8e0a90..6afb8ce4a39 100644 --- a/pkg/scalers/scaler.go +++ b/pkg/scalers/scaler.go @@ -18,6 +18,7 @@ package scalers import ( "context" + "errors" "fmt" "strings" "time" @@ -100,6 +101,15 @@ type ScalerConfig struct { MetricType v2.MetricTargetType } +var ( + // ErrScalerUnsupportedUtilizationMetricType is returned when v2.UtilizationMetricType + // is provided as the metric target type for scaler. + ErrScalerUnsupportedUtilizationMetricType = errors.New("'Utilization' metric type is unsupported for external metrics, allowed values are 'Value' or 'AverageValue'") + + // ErrScalerConfigMissingField is returned when a required field is missing from the scaler config. + ErrScalerConfigMissingField = errors.New("missing required field in scaler config") +) + // GetFromAuthOrMeta helps getting a field from Auth or Meta sections func GetFromAuthOrMeta(config *ScalerConfig, field string) (string, error) { var result string @@ -110,7 +120,7 @@ func GetFromAuthOrMeta(config *ScalerConfig, field string) (string, error) { result = config.TriggerMetadata[field] } if result == "" { - err = fmt.Errorf("no %s given", field) + err = fmt.Errorf("%w: no %s given", ErrScalerConfigMissingField, field) } return result, err } @@ -143,7 +153,7 @@ func InitializeLogger(config *ScalerConfig, scalerName string) logr.Logger { func GetMetricTargetType(config *ScalerConfig) (v2.MetricTargetType, error) { switch config.MetricType { case v2.UtilizationMetricType: - return "", fmt.Errorf("'Utilization' metric type is unsupported for external metrics, allowed values are 'Value' or 'AverageValue'") + return "", ErrScalerUnsupportedUtilizationMetricType case "": // Use AverageValue if no metric type was provided return v2.AverageValueMetricType, nil diff --git a/pkg/scalers/scaler_test.go b/pkg/scalers/scaler_test.go index 90b41f20c53..7a6f56f1618 100644 --- a/pkg/scalers/scaler_test.go +++ b/pkg/scalers/scaler_test.go @@ -1,7 +1,6 @@ package scalers import ( - "fmt" "testing" "github.com/stretchr/testify/assert" @@ -20,7 +19,7 @@ func TestGetMetricTargetType(t *testing.T) { name: "utilization metric type", config: &ScalerConfig{MetricType: v2.UtilizationMetricType}, wantmetricType: "", - wantErr: fmt.Errorf("'Utilization' metric type is unsupported for external metrics, allowed values are 'Value' or 'AverageValue'"), + wantErr: ErrScalerUnsupportedUtilizationMetricType, }, { name: "average value metric type", @@ -47,7 +46,7 @@ func TestGetMetricTargetType(t *testing.T) { t.Run(c.name, func(t *testing.T) { metricType, err := GetMetricTargetType(c.config) if c.wantErr != nil { - assert.Contains(t, err.Error(), c.wantErr.Error()) + assert.ErrorIs(t, err, c.wantErr) } else { assert.NoError(t, err) }