Skip to content

Commit

Permalink
refactor(scaler): compare error with errors.Is (kedacore#4004)
Browse files Browse the repository at this point in the history
  • Loading branch information
Juneezee authored and JorTurFer committed Jan 9, 2023
1 parent f9e84d8 commit 0e447de
Show file tree
Hide file tree
Showing 20 changed files with 211 additions and 105 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,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)
}

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)
}
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

0 comments on commit 0e447de

Please sign in to comment.