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

refactor(scaler): compare error with errors.Is #4004

Merged
merged 8 commits into from
Jan 2, 2023
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
6 changes: 5 additions & 1 deletion pkg/scalers/aws_common.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package scalers

import (
"errors"
"fmt"

"github.com/aws/aws-sdk-go/aws"
Expand All @@ -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

Expand Down Expand Up @@ -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"] != "" {
Expand Down
55 changes: 42 additions & 13 deletions pkg/scalers/aws_dynamodb_scaler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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)
Juneezee marked this conversation as resolved.
Show resolved Hide resolved
}

if len(m) == 0 {
return nil, errors.New("empty map")
return nil, ErrAwsDynamoEmptyExpressionAttributeNames
}
return m, err
}
Expand All @@ -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)
Juneezee marked this conversation as resolved.
Show resolved Hide resolved
}
return m, err
}
27 changes: 14 additions & 13 deletions pkg/scalers/aws_dynamodb_scaler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"errors"
"fmt"
"strconv"
"testing"

"github.com/aws/aws-sdk-go/service/dynamodb"
Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -165,7 +166,7 @@ var dynamoTestCases = []parseDynamoDBMetadataTestData{
"targetValue": "3",
},
authParams: map[string]string{},
expectedError: errors.New("awsAccessKeyID not found"),
expectedError: ErrAwsNoAccessKey,
},
{
name: "authentication provided",
Expand Down Expand Up @@ -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)
Expand Down
8 changes: 5 additions & 3 deletions pkg/scalers/azure/azure_blob_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ package azure

import (
"context"
"encoding/base64"
"errors"
"net/http"
"strings"
"testing"

kedav1alpha1 "github.com/kedacore/keda/v2/apis/keda/v1alpha1"
Expand All @@ -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())
}

Expand All @@ -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())
}
}
8 changes: 5 additions & 3 deletions pkg/scalers/azure/azure_queue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ package azure

import (
"context"
"encoding/base64"
"errors"
"net/http"
"strings"
"testing"

kedav1alpha1 "github.com/kedacore/keda/v2/apis/keda/v1alpha1"
Expand All @@ -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())
}

Expand All @@ -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())
}
}
12 changes: 10 additions & 2 deletions pkg/scalers/azure/azure_storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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 != "" {
Expand All @@ -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))
Expand Down
Loading